Return correct exit code when using values files by dcarley · Pull Request #32 · helm/chart-testing · GitHub
Skip to content

Return correct exit code when using values files - #32

Merged
unguiculus merged 2 commits into
helm:masterfrom
dcarley:fix_exit_for_values
Oct 1, 2018
Merged

unguiculus merged 2 commits into
helm:masterfrom
dcarley:fix_exit_for_values

Conversation

@dcarley

@dcarley dcarley commented Sep 28, 2018

Copy link
Copy Markdown
Contributor

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 #29 to
re-implement in another language is a good idea.

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 #29 to
re-implement in another language is a good idea.

Signed-off-by: Dan Carley <dan.carley@gmail.com>
@unguiculus

Copy link
Copy Markdown
Member

@rimusz

rimusz commented Oct 1, 2018

Copy link
Copy Markdown
Contributor

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>
@dcarley

dcarley commented Oct 1, 2018

Copy link
Copy Markdown
Contributor Author

No problem. I've gone with the || error=true pattern since that was already used in a few other places.

@rimusz

rimusz commented Oct 1, 2018

Copy link
Copy Markdown
Contributor

@unguiculus
unguiculus merged commit 4960e30 into helm:master Oct 1, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants