Return correct exit code when using values files #32
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The return values from the individual calls to `lint_chart_with_single_config`
are masked by the other operations in the `lint_chart_with_all_configs`
function.
When `helm lint` reports an error to STDERR but the chart is incorrectly
given a success tick and the exit code of `chart_test.sh` is 0 for
success. An example using `quay.io/helmpack/chart-testing:v1.1.0`:
==> Linting charts/nomad-server
[ERROR] templates/: render error in "nomad-server/templates/nomad-server.yaml": template: nomad-server/templates/nomad-server.yaml:55:28: executing "nomad-server/templates/nomad-server.yaml" at <{{template "fullname...>: template "fullname" not defined
Error: 1 chart(s) linted, 1 chart(s) failed
--------------------------------------------------------------------------------
✔︎ charts/nomad-server
--------------------------------------------------------------------------------
dcarley@cci-mbp ~/p/g/s/g/c/circle-external-services-chart master echo $?
0s
Fix this by copying the pattern used in other functions of tracking
failures in a local boolean variable and using that to determine the
return code at the end of the function. This is the result after:
==> Linting charts/nomad-server
[ERROR] templates/: render error in "nomad-server/templates/nomad-server.yaml": template: nomad-server/templates/nomad-server.yaml:55:28: executing "nomad-server/templates/nomad-server.yaml" at <{{template "fullname...>: template "fullname" not defined
Error: 1 chart(s) linted, 1 chart(s) failed
--------------------------------------------------------------------------------
✖︎ charts/nomad-server
--------------------------------------------------------------------------------
✘ dcarley@cci-mbp ~/p/g/s/g/c/circle-external-services-chart master echo $?
1
There are probably other bugs like this. Shell scripting does not
provide safe error handling for these cases and it's difficult to write
unit tests for. This is another good reason why the proposal in helm#29 to
re-implement in another language is a good idea.
Signed-off-by: Dan Carley <dan.carley@gmail.com>
bf065ff to
691f232
Compare
|
Thanks for fixing this. I basically fixed the same thing a while ago in #18 for the install part and didn't see the same problem existed for the linting part. You fixed the problem a little differently than I did. I would like to have this done consistently in both places. Would you mind updating the PR? I don't care whether you choose your way or mine, but it should be solved the same way in both places. Thanks. |
|
yes either way is fine, it just needs to be consistent |
The `|| error=true` pattern is already used in a few other places. This makes the change from 64c1e52 consistent with the others. Signed-off-by: Dan Carley <dan.carley@gmail.com>
425886d to
5bfc0cf
Compare
|
No problem. I've gone with the |
|
/lgtm |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The return values from the individual calls to
lint_chart_with_single_configare masked by the other operations in the
lint_chart_with_all_configsfunction.
When
helm lintreports an error to STDERR but the chart is incorrectlygiven a success tick and the exit code of
chart_test.shis 0 forsuccess. An example using
quay.io/helmpack/chart-testing:v1.1.0:Fix this by copying the pattern used in other functions of tracking
failures in a local boolean variable and using that to determine the
return code at the end of the function. This is the result after:
There are probably other bugs like this. Shell scripting does not
provide safe error handling for these cases and it's difficult to write
unit tests for. This is another good reason why the proposal in #29 to
re-implement in another language is a good idea.