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

The installer should fail on errors #119

Open
4 tasks
vorakl opened this issue May 16, 2019 · 2 comments
Open
4 tasks

The installer should fail on errors #119

vorakl opened this issue May 16, 2019 · 2 comments

Comments

@vorakl
Copy link

vorakl commented May 16, 2019

Summary
I would like to bring in to the discussion a quite serious issue that the installer doesn't fail on any errors.

Steps to reproduce
idt-installer VERSION="1.2.3"

Operating System
Specify: MacOS, Linux

Supporting details
Yesterday, my build pipeline for docker images with cloud tools failed on a testing stage with the error "kubectl: command not found". It was a bit unexpectedly. The first question in the mind was "why the image was even built??!". After a short investigation, it turned out, that all IBM Cloud tools are being installed by only one bash installer and it doesn't fail if cannot install something along the way. I submitted a related pull-request #118. But, it won't fix the main issue. Yes, it does fix the curl's options set and curl will exit with exit code 22 instead of 0, but the installer doesn't check this condition and keep working.
I din't submit a pull-request for this issue as long as it's a matter of adding "set -e" or "set -o errexit" closer to the beginning of the script and run it through your testing suite.

It is more about the understanding of what a behavior is right.
I suppose, most people expect to get all cloud tools installed if the installer finishes with no errors.

What do you guys think?


Development "done" checklist

  • Test case to verify
  • Public Documentation updated
  • Change added to "release notes" as appropropriate
  • Notification to stakeholders (OM, other squads, etc)
@rcj4747
Copy link

rcj4747 commented Sep 11, 2019

set -e won't work for pipes and all of the installs (docker and help scripts are piped to shell)

https://www.gnu.org/software/bash/manual/html_node/Pipelines.html

Keep in mind that not all pipes are obvious, here's one:
export FOO=$(grep bar filename) # set -e will not cause an exit

@vorakl
Copy link
Author

vorakl commented Sep 21, 2019

@rcj4747 , then the script has more issues than I thought but it doesn't change my point.

Yes, "set -o errexit" doesn't work for pipelines, but "set -o pipefail" does.
Moreover, "export FOO=$(grep bar filename)" is not the best way to work with exported variables, because making a variable exported and assigning some value to it in the same command, makes it hard to catch an error. But, it could be rewritten in a much more clear way, like

FOO=$(grep bar filename)
export FOO

or even better

declare -x FOO
...
...
FOO=$(grep bar filename)

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

No branches or pull requests

2 participants