Skip to content

add -eou pipefail to remaining job templates #6738

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Jan 16, 2020
Merged

add -eou pipefail to remaining job templates #6738

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 6 additions & 3 deletions ci-operator/templates/openshift/endurance/endurance-install.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,8 @@ objects:
- /bin/bash
- -c
- |
#!/bin/bash -x
#!/bin/bash
set -euo pipefail
# fetch the oc binary so we can talk to the cluster.
export PATH=$PATH:/tmp/shared/bin
Expand Down Expand Up @@ -153,7 +154,8 @@ objects:
- /bin/bash
- -c
- |
#!/bin/bash -x
#!/bin/bash
set -euo pipefail
export PATH=$PATH:/tmp/shared/bin
Expand Down Expand Up @@ -377,7 +379,8 @@ objects:
- /bin/sh
- -c
- |
#!/bin/sh -x
#!/bin/bash
set -euo pipefail
Copy link
Member

Choose a reason for hiding this comment

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

do we need to move to Bash shebangs for pipefail? I don't know if it's POSIX

Copy link
Contributor

Choose a reason for hiding this comment

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

we should make the header of the files consistent

Copy link
Contributor Author

Choose a reason for hiding this comment

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

https://github.com/openshift/release/pull/6718/files didn't

Copy link
Contributor Author

Choose a reason for hiding this comment

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

(didn't move to /bin/bash i mean)

Copy link
Member

Choose a reason for hiding this comment

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

No pipefail in POSIX. I think we should move to #!/bin/bash.

export PATH=$PATH:/tmp/shared/bin
Expand Down
6 changes: 4 additions & 2 deletions ci-operator/templates/openshift/installer/cluster-launch-installer-custom-test-image.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -380,8 +380,8 @@ objects:
- /bin/bash
- -c
- |
#!/bin/sh
set -e
#!/bin/bash
set -euo pipefail

trap 'rc=$?; if test "${rc}" -eq 0; then touch /tmp/setup-success; else touch /tmp/exit /tmp/setup-failed; fi; exit "${rc}"' EXIT
trap 'CHILDREN=$(jobs -p); if test -n "${CHILDREN}"; then kill ${CHILDREN} && wait; fi' TERM
Expand Down Expand Up @@ -660,6 +660,8 @@ objects:
- -c
- |
#!/bin/bash
set -euo pipefail

function queue() {
local TARGET="${1}"
shift
Expand Down
4 changes: 3 additions & 1 deletion ci-operator/templates/openshift/installer/cluster-launch-installer-e2e.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -385,7 +385,7 @@ objects:
- /bin/bash
- -c
- |
#!/bin/sh
#!/bin/bash
set -euo pipefail

trap 'rc=$?; if test "${rc}" -eq 0; then touch /tmp/setup-success; else touch /tmp/exit /tmp/setup-failed; fi; exit "${rc}"' EXIT
Expand Down Expand Up @@ -731,6 +731,8 @@ objects:
- -c
- |
#!/bin/bash
set -euo pipefail

function queue() {
local TARGET="${1}"
shift
Expand Down
3 changes: 2 additions & 1 deletion ci-operator/templates/openshift/installer/cluster-launch-installer-libvirt-e2e.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -185,8 +185,9 @@ objects:
- /bin/sh
- -c
- |
#!/bin/sh
#!/bin/bash
set -euo pipefail

trap 'rc=$?; if test "${rc}" -eq 0; then touch "${HOME}"/setup-success; else touch "${HOME}"/exit; fi; exit "${rc}"' EXIT
trap 'CHILDREN=$(jobs -p); if test -n "${CHILDREN}"; then kill ${CHILDREN} && wait; fi' TERM
cat > "${HOME}"/run-tests.sh << 'EOF'
Expand Down
8 changes: 6 additions & 2 deletions ci-operator/templates/openshift/installer/cluster-launch-installer-metal-e2e.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -226,8 +226,10 @@ objects:
- /bin/sh
- -c
- |
#!/bin/sh
set -ex
#!/bin/bash
set -euo pipefail
set -x

trap 'rc=$?; if test "${rc}" -eq 0; then touch /tmp/setup-success; else touch /tmp/exit /tmp/setup-failed; fi; exit "${rc}"' EXIT
trap 'CHILDREN=$(jobs -p); if test -n "${CHILDREN}"; then kill ${CHILDREN} && wait; fi' TERM

Expand Down Expand Up @@ -515,6 +517,8 @@ objects:
- -c
- |
#!/bin/bash
set -euo pipefail

function queue() {
local TARGET="${1}"
shift
Expand Down
2 changes: 2 additions & 0 deletions ci-operator/templates/openshift/installer/cluster-launch-installer-openstack-e2e.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -445,6 +445,8 @@ objects:
- -c
- |
#!/bin/bash
set -euo pipefail

function queue() {
local TARGET="${1}"
shift
Expand Down
6 changes: 5 additions & 1 deletion ci-operator/templates/openshift/installer/cluster-launch-installer-ovirt-e2e.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -323,7 +323,9 @@ objects:
- /bin/sh
- -c
- |
#!/bin/sh
#!/bin/bash
set -euo pipefail

trap 'rc=$?; if test "${rc}" -eq 0; then touch /tmp/setup-success; else touch /tmp/exit; fi; exit "${rc}"' EXIT
trap 'CHILDREN=$(jobs -p); if test -n "${CHILDREN}"; then kill ${CHILDREN} && wait; fi' TERM
# Wait untill lease is acquired
Expand Down Expand Up @@ -449,6 +451,8 @@ objects:
- -c
- |
#!/bin/bash
set -euo pipefail

function queue() {
local TARGET="${1}"
shift
Expand Down
4 changes: 3 additions & 1 deletion ci-operator/templates/openshift/installer/cluster-launch-installer-src.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -243,7 +243,7 @@ objects:
- /bin/bash
- -c
- |
#!/bin/sh
#!/bin/bash
set -euo pipefail

trap 'rc=$?; if test "${rc}" -eq 0; then touch /tmp/setup-success; else touch /tmp/exit /tmp/setup-failed; fi; exit "${rc}"' EXIT
Expand Down Expand Up @@ -566,6 +566,8 @@ objects:
- -c
- |
#!/bin/bash
set -euo pipefail

function queue() {
local TARGET="${1}"
shift
Expand Down
4 changes: 3 additions & 1 deletion ci-operator/templates/openshift/installer/cluster-launch-installer-upi-e2e.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -338,8 +338,9 @@ objects:
- /bin/sh
- -c
- |
#!/bin/sh
#!/bin/bash
set -euo pipefail

trap 'rc=$?; if test "${rc}" -eq 0; then touch /tmp/setup-success; else touch /tmp/exit /tmp/setup-failed; fi; exit "${rc}"' EXIT
trap 'CHILDREN=$(jobs -p); if test -n "${CHILDREN}"; then kill ${CHILDREN} && wait; fi' TERM

Expand Down Expand Up @@ -1855,6 +1856,7 @@ objects:
- -c
- |
#!/bin/bash
set -euo pipefail
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe not fail here... as teardown can have failures but should attempt to try to completion.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the teardown script has the following protections that make this ok i think:

  1. trap 'teardown' EXIT
       function teardown() {
          set +e

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jstuever agree?

Copy link
Contributor

Choose a reason for hiding this comment

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

hold on. teardown shouldn't need to set +e. We have a "correct" teardown on most things - if we have a non standard teardown it would only apply in that context of one job (i.e. don't go adding set +e to teardown that don't have it already)


function queue() {
local TARGET="${1}"
Expand Down
6 changes: 4 additions & 2 deletions ci-operator/templates/openshift/installer/cluster-launch-installer-upi-src.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -333,8 +333,9 @@ objects:
- /bin/sh
- -c
- |
#!/bin/sh
set -e
#!/bin/bash
set -euo pipefail

trap 'rc=$?; if test "${rc}" -eq 0; then touch /tmp/setup-success; else touch /tmp/exit /tmp/setup-failed; fi; exit "${rc}"' EXIT
trap 'CHILDREN=$(jobs -p); if test -n "${CHILDREN}"; then kill ${CHILDREN} && wait; fi' TERM

Expand Down Expand Up @@ -1073,6 +1074,7 @@ objects:
- -c
- |
#!/bin/bash
set -euo pipefail

function queue() {
local TARGET="${1}"
Expand Down
2 changes: 2 additions & 0 deletions ci-operator/templates/openshift/openshift-ansible/cluster-launch-e2e-openshift-ansible.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -255,6 +255,8 @@ objects:
- -c
- |
#!/bin/bash
set -euo pipefail

function teardown() {
set +e
touch /tmp/shared/exit
Expand Down
2 changes: 2 additions & 0 deletions ci-operator/templates/openshift/openshift-ansible/cluster-launch-e2e-openshift-jenkins.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -312,6 +312,8 @@ objects:
- -c
- |
#!/bin/bash
set -euo pipefail

function teardown() {
set +e
touch /tmp/shared/exit
Expand Down
2 changes: 2 additions & 0 deletions ci-operator/templates/openshift/openshift-ansible/cluster-launch-e2e-upgrade.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -260,6 +260,8 @@ objects:
- -c
- |
#!/bin/bash
set -euo pipefail

function teardown() {
set +e
touch /tmp/shared/exit
Expand Down
2 changes: 2 additions & 0 deletions ci-operator/templates/openshift/openshift-ansible/cluster-launch-e2e.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -216,6 +216,8 @@ objects:
- -c
- |
#!/bin/bash
set -euo pipefail

function teardown() {
set +e
touch /tmp/shared/exit
Expand Down
2 changes: 2 additions & 0 deletions ci-operator/templates/openshift/openshift-ansible/cluster-launch-src.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,8 @@ objects:
- -c
- |
#!/bin/bash
set -euo pipefail

function teardown() {
set +e
touch /tmp/shared/exit
Expand Down
6 changes: 5 additions & 1 deletion ci-operator/templates/openshift/openshift-ansible/cluster-scaleup-e2e-40.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,9 @@ objects:
- /bin/sh
- -c
- |
#!/bin/sh
#!/bin/bash
set -euo pipefail

trap 'rc=$?; if test "${rc}" -eq 0; then touch /tmp/setup-success; else touch /tmp/exit /tmp/setup-failed; fi; exit "${rc}"' EXIT
trap 'CHILDREN=$(jobs -p); if test -n "${CHILDREN}"; then kill ${CHILDREN} && wait; fi' TERM
cp "$(command -v openshift-install)" /tmp
Expand Down Expand Up @@ -362,6 +364,8 @@ objects:
- -c
- |
#!/bin/bash
set -euo pipefail

function queue() {
local TARGET="${1}"
shift
Expand Down
4 changes: 2 additions & 2 deletions ci-operator/templates/openshift/openshift-azure/cluster-launch-e2e-azure.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,6 @@ objects:
- -c
- |
#!/bin/bash
# error handling and sync code
set -euo pipefail

trap 'touch /tmp/shared/exit' EXIT
Expand Down Expand Up @@ -271,7 +270,7 @@ objects:
- /bin/bash
- -c
- |
#!/bin/bash
#!/bin/bash
set -euo pipefail

trap 'rc=$?; if [[ $rc -ne 0 ]]; then
Expand Down Expand Up @@ -361,6 +360,7 @@ objects:
- -c
- |
#!/bin/bash
set -euo pipefail

# teardown is collecting debug data and deleting all used resources
function teardown() {
Expand Down