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
Fixes #17268: rudder agent run returns exit code 1 when everything is ok #297
Conversation
OK, merging this PR |
@@ -465,7 +465,7 @@ END { | |||
printf "%s%-80.80s%s\n", white, padding, normal; | |||
|
|||
# Set return code | |||
if (run_error+audit_error+audit_noncompliant+enforce_error != 0) { | |||
if (run_error+audit_error+enforce_error != 0) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i validated, but this means that we want non compliant to be ok ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sur I understand either. Actually, I'm not sure of the semantic of the return code, but non compliance
is as much an error as impossible to enforce
, I think. If my ssh key does't match the expected set of authorized one, it's as important as if in enforce, it was that and I wasn't able to change it. Or is this audit error? And if so, what is noncompliant
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It means we consider non-compliant to be normal and expected agent behavior. Errors or aborted runs are not normal and should be fixed by the user.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In fact I think error reports should not change the exit code to 1. Aborted agents are the only real problems the agent itself can have.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree with that, but if so: why audit error and enforce error change it to 1?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(ie, we diferentiate from "expected" errors that are just part of the business domain, and "unexpect" defaicts that should be processed outside of business domain. An enforce in error is expected, an agent run crash is not)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
They shouldn't :), il make another PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No description provided.