Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Warn if OpenSCAP binary not available #13878

Merged
merged 1 commit into from
Feb 19, 2017

Conversation

moolitayer
Copy link

@moolitayer moolitayer commented Feb 12, 2017

In the appliance and podified ManageIQ the OpenSCAP Binary is installed. I added some warning at require failures so that customers that encounter the problem on a new environment that has it's own dependency management (like we recently had with podified ManageIQ) will be able to easily report what is wrong.

See See ManageIQ/guides#189

@moolitayer
Copy link
Author

@cben Take a look, not sure this is the most elegant way

@miq-bot
Copy link
Member

miq-bot commented Feb 12, 2017

Checked commit moolitayer@d6f45d5 with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0
2 files checked, 0 offenses detected
Everything looks good. 👍

@cben
Copy link
Contributor

cben commented Feb 12, 2017

Thanks 👍

Is "binary missing" correct wording? Does the execute a binary, of do FFI calls to a library?

@cben
Copy link
Contributor

cben commented Feb 12, 2017

Specifically, require openscap was failing with libopenscap.8.so or something like that.
(Ideally, wording would reflect actual error from require, but this is harder with the helper returning false.)

@moolitayer
Copy link
Author

It is a dynamically linked library called from an ffi wrapper (the gem). The propose of this PR is when customers encounter the problem on a new environment that has it's own dependency management (like we recently had with podified ManageIQ as it manages dependencies differently then the appliance) they will be able to provide a meaningful error report. We could log the error in the require time but my first priority is to have the error with the other job logging since that's usually what we get.

@moolitayer
Copy link
Author

BTW @cben we could have potentially raised the error and get it in the log but then the termination is less clean that way and I try to avoid that. In those cases the job keeps thinking it is active and is timed out by the scheduler so we get another error in the log (so sometimes customers miss the important one) and we get the failure not then it happends

@@ -157,6 +157,10 @@ def analyze
end

def collect_compliance_data(image)
unless OpenscapResult.openscap_available?
_log.warn "OpenSCAP Binary missing, skipping scan"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So consider s/Binary/Library/here.

@cben
Copy link
Contributor

cben commented Feb 12, 2017

LGTM.

@moolitayer
Copy link
Author

@simon3z please review, not a priority

Copy link
Contributor

@simon3z simon3z left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

@moolitayer
Copy link
Author

@roliveri Can you please review this change?

@roliveri roliveri merged commit 05d9b6e into ManageIQ:master Feb 19, 2017
@roliveri
Copy link
Member

@moolitayer Should this be back ported anywhere?

@chessbyte chessbyte added this to the Sprint 55 Ending Feb 27, 2017 milestone Feb 20, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants