Skip to content
This repository was archived by the owner on Mar 11, 2022. It is now read-only.

Add stricter error detection flags in bash scripts#151

Merged
seanmalloy merged 6 commits intoKohlsTechnology:masterfrom
akavel:scripts-preamble
Nov 19, 2019
Merged

Add stricter error detection flags in bash scripts#151
seanmalloy merged 6 commits intoKohlsTechnology:masterfrom
akavel:scripts-preamble

Conversation

@akavel
Copy link
Copy Markdown

@akavel akavel commented Nov 13, 2019

Description

This PR adds error detection flags (set -euo pipefail) in all bash scripts I was able to find in the repository, that were missing it before. This will improve robustness of the scripts.

Adding those flags resulted in a failing command being found in discoverEnvironment.sh. The PR adds a temporary workaround, and I opened a separate issue #152 with regards to fixing the underlying problem in the future.

Additionally, the PR enables execution tracing (set -x) in most of the scripts, including template-processors base scripts. This is intended to help when any debugging of the scripts is needed, and make it easier for contributors to provide better logs when reporting issues.

Closes #74

Type of change

  • Bug fix (non-breaking change which fixes an issue)

Copy link
Copy Markdown

@mkyc mkyc left a comment

Choose a reason for hiding this comment

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

LGTM. One comment to clarify.

Comment thread template-processors/base/bin/resourceManager.sh
Copy link
Copy Markdown
Member

@vinny-sabatini vinny-sabatini left a comment

Choose a reason for hiding this comment

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

LGTM

@seanmalloy seanmalloy added this to the v0.0.4 milestone Nov 18, 2019
@seanmalloy
Copy link
Copy Markdown
Contributor

@akavel looks good. Please resolve the merge conflict, and we can get this merged.

Mateusz Czapliński added 6 commits November 19, 2019 11:47
Add bash flags protecting against common errors in all scripts (in
template-processors/ and in scripts/ too). Also add `set -x` where
possible, to make potentially debugging them easier in future.

As part of adding the flags, a place was found in discoverEnvironment.sh
where `kubectl describe` command was failing. The place was marked with
FIXME note and a separate issue will be opened for fixing it.

Closes KohlsTechnology#74.
The `help set` command gives the following info about the `-u` flag:

  -u  Treat unset variables as an error when substituting.

Calling `set +u` *disables* this protection. This commit removes `set
+u` in blocks where it appears to be spurious, as the variables used are
actually expected to be set.
The blocks handling $TEMPLATE_GITCONFIG and $PARAMETER_GITCONFIG in
gitClone.sh were broken in many ways. For starters, the blocks were
never entered, as their entrance conditions were mutually opposing. This
commit fixes obvious errors in those blocks to make the code run.
Hopefully some tests will be added for this in the future.
Add quotes around environment variables, to improve script robustness.
@akavel
Copy link
Copy Markdown
Author

akavel commented Nov 19, 2019

@seanmalloy rebased & resolved, PTAL

@seanmalloy seanmalloy merged commit c7d506f into KohlsTechnology:master Nov 19, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Development

Successfully merging this pull request may close these issues.

Eunomia-base image scripts don't handle errors well.

4 participants