-
Notifications
You must be signed in to change notification settings - Fork 527
Trap errors and remove ec2 instance #854
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
Changes from all commits
e403aaa
047ee2e
5cfa9ab
08ee95e
856eb15
53577f1
2cc681f
5315272
e1229fc
4a6a929
97a5006
bf99d0a
230c5f9
60a67cd
2840963
e3cc634
ccfb096
f4415b6
85c95ec
e0ce7a3
2296157
4aa0da1
1d13349
ff2852f
c2d3ac2
f72f9a7
708ba1b
8e572f4
16ba15b
7130093
9b489d7
47388c3
3ed9316
5ad0517
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -7,17 +7,25 @@ if [ -z "${BUILDTIMESTAMP}" ]; then | |
| BUILDTIMESTAMP=$(cat "${HOME}/buildtimestamp") | ||
| export BUILDTIMESTAMP | ||
| echo run "${0}" with output to "${HOME}/buildlog_${BUILDTIMESTAMP}" | ||
| (bash "${0}" "${1}" 2>&1) | tee "${HOME}/buildlog_${BUILDTIMESTAMP}" | ||
| exit 0 | ||
| bash "${0}" "${1}" 2>&1 | tee "${HOME}/buildlog_${BUILDTIMESTAMP}" | ||
| # http://tldp.org/LDP/abs/html/internalvariables.html#PIPESTATUSREF | ||
| exit "${PIPESTATUS[0]}" | ||
| fi | ||
|
|
||
| echo | ||
| date "+build_release begin SETUP stage %Y%m%d_%H%M%S" | ||
| echo | ||
|
|
||
| # `apt-get` fails randomly when downloading package, this is a hack that "works" reasonably well. | ||
| echo -e "deb http://us.archive.ubuntu.com/ubuntu/ bionic main universe multiverse\ndeb http://archive.ubuntu.com/ubuntu/ bionic main universe multiverse" | sudo tee /etc/apt/sources.list.d/ubuntu | ||
|
|
||
|
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is a hack to ensure that |
||
| sudo apt-get update | ||
| sudo apt-get upgrade -y | ||
| sudo apt-get install -y build-essential automake autoconf awscli docker.io git gpg nfs-common python3 rpm sqlite3 python3-boto3 g++ libtool rng-tools | ||
|
|
||
| # `apt-get` fails randomly when downloading package, this is a hack that "works" reasonably well. | ||
| sudo apt-get update | ||
|
|
||
|
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Like the above comment, this is a needed hack so we don't get stuck. I really think we should leave these in for now.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm quite skeptical that the upgrade -y is needed. ( i.e. if you need specific upgrades for specific components, you should explicitly ask for that.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ok, I'll try removing it. |
||
| sudo apt-get install -y build-essential automake autoconf awscli docker.io git gpg nfs-common python python3 rpm sqlite3 python3-boto3 g++ libtool rng-tools | ||
| sudo rngd -r /dev/urandom | ||
|
|
||
| #umask 0077 | ||
|
|
||
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.
This will get the exit code of the first command in the pipeline, which is what we want.
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.
nice, but that's not what you want. You want to use
set -o pipefailwhich would set the error code you get from the pipe to the first failuire.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.
Ok, will change and test.
Uh oh!
There was an error while loading. Please reload this page.
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.
Oh, I remember now why I used
PIPESTATUSinstead. I agree that in most cases settingpipefailis the better choice, but we do need a return value here or the script will execute one more time than we need.BUILDTIMESTAMPisn't set so we set it and then start capturing the logs.bash "${0}" "${1}" 2>&1BUILDTIMESTAMPis set skip this portion and parse the file.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.
yes, setting it to
pipefailwould set the error flag once thebash "${0}" "${1}" 2>&1 | tee "${HOME}/buildlog_${BUILDTIMESTAMP}"command is complete. Then, given that you already have aset -ex, it will exit right away.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.
note that pipefail would not propagate to the child bash invocation since SHELLOPTS is not exported.
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.
Yes, but the happy path is not to have an error. So, if we don't explicitly return here, the script will be parsed again, which is not what we want.