From 295c531ecec8661408f01ac4e015d75fec865376 Mon Sep 17 00:00:00 2001 From: Ed Santiago Date: Thu, 2 May 2019 05:49:50 -0600 Subject: cirrus lib.sh: refactor req_env_var() Existing code was not working due to a bash gotcha ('exit' from a pipeline). It also had unnecessary duplication. New version is safer; also includes unit tests run under localunit. Existing invocations of req_env_var replaced via: $ [ edit setup_environment.sh, move one closing quote to its own line ] $ perl -ni -e 's/(?<=req_env_var )"(\S+)\s+\$\1"/$1/; if (/req_env_var "$/ .. /^\s*"/) { chomp; s/(?<=\S)\s.*//; if (/^\s*"/) { print "\n" } else { unless (/req_env_var/) { s/^\s+//; print " ";} print;} } else { print }' $(ack -l req_env_var) $ [ hand-massage an incorrect instance of '@' in lib.sh:ircmsg() ] Signed-off-by: Ed Santiago --- Makefile | 1 + contrib/cirrus/build_vm_images.sh | 20 +---- contrib/cirrus/integration_test.sh | 10 +-- contrib/cirrus/lib.sh | 87 +++++++++------------- contrib/cirrus/lib.sh.t | 81 ++++++++++++++++++++ contrib/cirrus/packer/centos_setup.sh | 7 +- contrib/cirrus/packer/fah_setup.sh | 4 +- contrib/cirrus/packer/fedora_setup.sh | 9 +-- .../packer/image-builder-image_base-setup.sh | 7 +- contrib/cirrus/packer/rhel_base-setup.sh | 4 +- contrib/cirrus/packer/rhel_setup.sh | 8 +- contrib/cirrus/packer/ubuntu_setup.sh | 8 +- contrib/cirrus/rootless_test.sh | 6 +- contrib/cirrus/setup_container_environment.sh | 6 +- contrib/cirrus/setup_environment.sh | 7 +- contrib/cirrus/success.sh | 5 +- contrib/cirrus/system_test.sh | 6 +- contrib/cirrus/unit_test.sh | 6 +- 18 files changed, 133 insertions(+), 149 deletions(-) create mode 100755 contrib/cirrus/lib.sh.t diff --git a/Makefile b/Makefile index 9228ec711..0085537d0 100644 --- a/Makefile +++ b/Makefile @@ -188,6 +188,7 @@ localunit: test/goecho/goecho varlink_generate --tags "$(BUILDTAGS)" \ --succinct $(MAKE) -C contrib/cirrus/packer test + ./contrib/cirrus/lib.sh.t ginkgo: ginkgo -v -tags "$(BUILDTAGS)" $(GINKGOTIMEOUT) -cover -flakeAttempts 3 -progress -trace -noColor -nodes 3 test/e2e/. diff --git a/contrib/cirrus/build_vm_images.sh b/contrib/cirrus/build_vm_images.sh index 6b86aa4d4..fc8cbb404 100755 --- a/contrib/cirrus/build_vm_images.sh +++ b/contrib/cirrus/build_vm_images.sh @@ -3,25 +3,7 @@ set -e source $(dirname $0)/lib.sh -req_env_var " -CNI_COMMIT $CNI_COMMIT -CRIO_COMMIT $CRIO_COMMIT -RUNC_COMMIT $RUNC_COMMIT -PACKER_BUILDS $PACKER_BUILDS -BUILT_IMAGE_SUFFIX $BUILT_IMAGE_SUFFIX -CENTOS_BASE_IMAGE $CENTOS_BASE_IMAGE -UBUNTU_BASE_IMAGE $UBUNTU_BASE_IMAGE -FEDORA_BASE_IMAGE $FEDORA_BASE_IMAGE -FAH_BASE_IMAGE $FAH_BASE_IMAGE -RHEL_BASE_IMAGE $RHEL_BASE_IMAGE -RHSM_COMMAND $RHSM_COMMAND -SERVICE_ACCOUNT $SERVICE_ACCOUNT -GCE_SSH_USERNAME $GCE_SSH_USERNAME -GCP_PROJECT_ID $GCP_PROJECT_ID -PACKER_VER $PACKER_VER -SCRIPT_BASE $SCRIPT_BASE -PACKER_BASE $PACKER_BASE -" +req_env_var CNI_COMMIT CRIO_COMMIT RUNC_COMMIT PACKER_BUILDS BUILT_IMAGE_SUFFIX CENTOS_BASE_IMAGE UBUNTU_BASE_IMAGE FEDORA_BASE_IMAGE FAH_BASE_IMAGE RHEL_BASE_IMAGE RHSM_COMMAND SERVICE_ACCOUNT GCE_SSH_USERNAME GCP_PROJECT_ID PACKER_VER SCRIPT_BASE PACKER_BASE record_timestamp "cache-image build start" diff --git a/contrib/cirrus/integration_test.sh b/contrib/cirrus/integration_test.sh index 95387ff49..71223803c 100755 --- a/contrib/cirrus/integration_test.sh +++ b/contrib/cirrus/integration_test.sh @@ -3,13 +3,7 @@ set -e source $(dirname $0)/lib.sh -req_env_var " -GOSRC $GOSRC -SCRIPT_BASE $SCRIPT_BASE -OS_RELEASE_ID $OS_RELEASE_ID -OS_RELEASE_VER $OS_RELEASE_VER -CONTAINER_RUNTIME $CONTAINER_RUNTIME -" +req_env_var GOSRC SCRIPT_BASE OS_RELEASE_ID OS_RELEASE_VER CONTAINER_RUNTIME exit_handler() { set +ex @@ -39,7 +33,7 @@ then exit $? elif [[ "$SPECIALMODE" == "rootless" ]] then - req_env_var "ROOTLESS_USER $ROOTLESS_USER" + req_env_var ROOTLESS_USER set -x ssh $ROOTLESS_USER@localhost \ -o UserKnownHostsFile=/dev/null -o StrictHostKeyChecking=no -o CheckHostIP=no \ diff --git a/contrib/cirrus/lib.sh b/contrib/cirrus/lib.sh index d663616b2..1c459ebc7 100644 --- a/contrib/cirrus/lib.sh +++ b/contrib/cirrus/lib.sh @@ -33,15 +33,27 @@ then source "$HOME/$ENVLIB" fi -# Pass in a line delimited list of, space delimited name/value pairs -# exit non-zero with helpful error message if any value is empty +# Pass in a list of one or more envariable names; exit non-zero with +# helpful error message if any value is empty req_env_var() { - echo "$1" | while read NAME VALUE - do - if [[ -n "$NAME" ]] && [[ -z "$VALUE" ]] - then - echo "Required env. var. \$$NAME is not set" - exit 9 + # Provide context. If invoked from function use its name; else script name + local caller=${FUNCNAME[1]} + if [[ -n "$caller" ]]; then + # Indicate that it's a function name + caller="$caller()" + else + # Not called from a function: use script name + caller=$(basename $0) + fi + + # Usage check + [[ -n "$1" ]] || die 1 "FATAL: req_env_var: invoked without arguments" + + # Each input arg is an envariable name, e.g. HOME PATH etc. Expand each. + # If any is empty, bail out and explain why. + for i; do + if [[ -z "${!i}" ]]; then + die 9 "FATAL: $caller requires \$$i to be non-empty" fi done } @@ -97,20 +109,14 @@ PACKER_BUILDS $PACKER_BUILDS # Unset environment variables not needed for testing purposes clean_env() { - req_env_var " - UNSET_ENV_VARS $UNSET_ENV_VARS - " + req_env_var UNSET_ENV_VARS echo "Unsetting $(echo $UNSET_ENV_VARS | wc -w) environment variables" unset -v UNSET_ENV_VARS $UNSET_ENV_VARS || true # don't fail on read-only } die() { - req_env_var " - 1 $1 - 2 $2 - " - echo "$2" - exit $1 + echo "${2:-FATAL ERROR (but no message given!) in ${FUNCNAME[1]}()}" + exit ${1:-1} } # Return a GCE image-name compatible string representation of distribution name @@ -135,10 +141,8 @@ stub() { } ircmsg() { - req_env_var " - CIRRUS_TASK_ID $CIRRUS_TASK_ID - @ $@ - " + req_env_var CIRRUS_TASK_ID + [[ -n "$*" ]] || die 9 "ircmsg() invoked without args" # Sometimes setup_environment.sh didn't run SCRIPT="$(dirname $0)/podbot.py" NICK="podbot_$CIRRUS_TASK_ID" @@ -151,7 +155,7 @@ ircmsg() { record_timestamp() { set +x # sometimes it's turned on - req_env_var "TIMESTAMPS_FILEPATH $TIMESTAMPS_FILEPATH" + req_env_var TIMESTAMPS_FILEPATH echo "." # cirrus webui strips blank-lines STAMPMSG="The $1 time at the tone will be:" echo -e "$STAMPMSG\t$(date --iso-8601=seconds)" | \ @@ -160,11 +164,7 @@ record_timestamp() { } setup_rootless() { - req_env_var " - ROOTLESS_USER $ROOTLESS_USER - GOSRC $GOSRC - ENVLIB $ENVLIB - " + req_env_var ROOTLESS_USER GOSRC ENVLIB if passwd --status $ROOTLESS_USER then @@ -220,7 +220,7 @@ setup_rootless() { # Helper/wrapper script to only show stderr/stdout on non-zero exit install_ooe() { - req_env_var "SCRIPT_BASE $SCRIPT_BASE" + req_env_var SCRIPT_BASE echo "Installing script to mask stdout/stderr unless non-zero exit." sudo install -D -m 755 "/tmp/libpod/$SCRIPT_BASE/ooe.sh" /usr/local/bin/ooe.sh } @@ -241,10 +241,7 @@ EOF install_cni_plugins() { echo "Installing CNI Plugins from commit $CNI_COMMIT" - req_env_var " - GOPATH $GOPATH - CNI_COMMIT $CNI_COMMIT - " + req_env_var GOPATH CNI_COMMIT DEST="$GOPATH/src/github.com/containernetworking/plugins" rm -rf "$DEST" ooe.sh git clone "https://github.com/containernetworking/plugins.git" "$DEST" @@ -272,11 +269,7 @@ install_runc(){ OS_RELEASE_ID=$(os_release_id) echo "Installing RunC from commit $RUNC_COMMIT" echo "Platform is $OS_RELEASE_ID" - req_env_var " - GOPATH $GOPATH - RUNC_COMMIT $RUNC_COMMIT - OS_RELEASE_ID $OS_RELEASE_ID - " + req_env_var GOPATH RUNC_COMMIT OS_RELEASE_ID if [[ "$OS_RELEASE_ID" =~ "ubuntu" ]]; then echo "Running make install.libseccomp.sudo for ubuntu" if ! [[ -d "/tmp/libpod" ]] @@ -295,7 +288,7 @@ install_runc(){ install_buildah() { echo "Installing buildah from latest upstream master" - req_env_var "GOPATH $GOPATH" + req_env_var GOPATH DEST="$GOPATH/src/github.com/containers/buildah" rm -rf "$DEST" ooe.sh git clone https://github.com/containers/buildah "$DEST" @@ -307,10 +300,7 @@ install_buildah() { # Requires $GOPATH and $CRIO_COMMIT to be set install_conmon(){ echo "Installing conmon from commit $CRIO_COMMIT" - req_env_var " - GOPATH $GOPATH - CRIO_COMMIT $CRIO_COMMIT - " + req_env_var GOPATH CRIO_COMMIT DEST="$GOPATH/src/github.com/kubernetes-sigs/cri-o.git" rm -rf "$DEST" ooe.sh git clone https://github.com/kubernetes-sigs/cri-o.git "$DEST" @@ -327,9 +317,7 @@ install_criu(){ echo "Installing CRIU" echo "Installing CRIU from commit $CRIU_COMMIT" echo "Platform is $OS_RELEASE_ID" - req_env_var " - CRIU_COMMIT $CRIU_COMMIT - " + req_env_var CRIU_COMMIT if [[ "$OS_RELEASE_ID" =~ "ubuntu" ]]; then ooe.sh sudo -E add-apt-repository -y ppa:criu/ppa @@ -418,10 +406,7 @@ ubuntu_finalize(){ rhel_exit_handler() { set +ex - req_env_var " - GOPATH $GOPATH - RHSMCMD $RHSMCMD - " + req_env_var GOPATH RHSMCMD cd / sudo rm -rf "$RHSMCMD" sudo rm -rf "$GOPATH" @@ -431,9 +416,7 @@ rhel_exit_handler() { } rhsm_enable() { - req_env_var " - RHSM_COMMAND $RHSM_COMMAND - " + req_env_var RHSM_COMMAND export GOPATH="$(mktemp -d)" export RHSMCMD="$(mktemp)" trap "rhel_exit_handler" EXIT diff --git a/contrib/cirrus/lib.sh.t b/contrib/cirrus/lib.sh.t new file mode 100755 index 000000000..ce51f8ad2 --- /dev/null +++ b/contrib/cirrus/lib.sh.t @@ -0,0 +1,81 @@ +#!/bin/bash +# +# Unit tests for some functions in lib.sh +# +source $(dirname $0)/lib.sh + +# Iterator and return code; updated in test functions +testnum=0 +rc=0 + +function check_result { + testnum=$(expr $testnum + 1) + if [ "$1" = "$2" ]; then + echo "ok $testnum $3 = $1" + else + echo "not ok $testnum $3" + echo "# expected: $2" + echo "# actual: $1" + rc=1 + fi +} + +############################################################################### +# tests for die() + +function test_die() { + local input_status=$1 + local input_msg=$2 + local expected_status=$3 + local expected_msg=$4 + + local msg + msg=$(die $input_status "$input_msg") + local status=$? + + check_result "$msg" "$expected_msg" "die $input_status $input_msg" +} + +test_die 1 "a message" 1 "a message" +test_die 2 "" 2 "FATAL ERROR (but no message given!) in test_die()" +test_die '' '' 1 "FATAL ERROR (but no message given!) in test_die()" + +############################################################################### +# tests for req_env_var() + +function test_rev() { + local input_args=$1 + local expected_status=$2 + local expected_msg=$3 + + # bash gotcha: doing 'local msg=...' on one line loses exit status + local msg + msg=$(req_env_var $input_args) + local status=$? + + check_result "$msg" "$expected_msg" "req_env_var $input_args" + check_result "$status" "$expected_status" "req_env_var $input_args (rc)" +} + +# error if called with no args +test_rev '' 1 'FATAL: req_env_var: invoked without arguments' + +# error if desired envariable is unset +unset FOO BAR +test_rev FOO 9 'FATAL: test_rev() requires $FOO to be non-empty' +test_rev BAR 9 'FATAL: test_rev() requires $BAR to be non-empty' + +# OK if desired envariable is unset +FOO=1 +test_rev FOO 0 '' + +# ...but error if any single desired one is unset +test_rev "FOO BAR" 9 'FATAL: test_rev() requires $BAR to be non-empty' + +# ...and OK if all args are set +BAR=1 +test_rev "FOO BAR" 0 '' + +############################################################################### + +exit $rc diff --git a/contrib/cirrus/packer/centos_setup.sh b/contrib/cirrus/packer/centos_setup.sh index d947a1d7f..91b1963c2 100644 --- a/contrib/cirrus/packer/centos_setup.sh +++ b/contrib/cirrus/packer/centos_setup.sh @@ -8,12 +8,7 @@ set -e # Load in library (copied by packer, before this script was run) source /tmp/libpod/$SCRIPT_BASE/lib.sh -req_env_var " -SCRIPT_BASE $SCRIPT_BASE -CNI_COMMIT $CNI_COMMIT -CRIO_COMMIT $CRIO_COMMIT -CRIU_COMMIT $CRIU_COMMIT -" +req_env_var SCRIPT_BASE CNI_COMMIT CRIO_COMMIT CRIU_COMMIT install_ooe diff --git a/contrib/cirrus/packer/fah_setup.sh b/contrib/cirrus/packer/fah_setup.sh index 2e053b396..18c4db0af 100644 --- a/contrib/cirrus/packer/fah_setup.sh +++ b/contrib/cirrus/packer/fah_setup.sh @@ -8,9 +8,7 @@ set -e # Load in library (copied by packer, before this script was run) source /tmp/libpod/$SCRIPT_BASE/lib.sh -req_env_var " -SCRIPT_BASE $SCRIPT_BASE -" +req_env_var SCRIPT_BASE install_ooe diff --git a/contrib/cirrus/packer/fedora_setup.sh b/contrib/cirrus/packer/fedora_setup.sh index 84aee7667..36a65eb71 100644 --- a/contrib/cirrus/packer/fedora_setup.sh +++ b/contrib/cirrus/packer/fedora_setup.sh @@ -8,14 +8,7 @@ set -e # Load in library (copied by packer, before this script was run) source /tmp/libpod/$SCRIPT_BASE/lib.sh -req_env_var " -SCRIPT_BASE $SCRIPT_BASE -FEDORA_CNI_COMMIT $FEDORA_CNI_COMMIT -CNI_COMMIT $CNI_COMMIT -CRIO_COMMIT $CRIO_COMMIT -CRIU_COMMIT $CRIU_COMMIT -RUNC_COMMIT $RUNC_COMMIT -" +req_env_var SCRIPT_BASE FEDORA_CNI_COMMIT CNI_COMMIT CRIO_COMMIT CRIU_COMMIT RUNC_COMMIT install_ooe diff --git a/contrib/cirrus/packer/image-builder-image_base-setup.sh b/contrib/cirrus/packer/image-builder-image_base-setup.sh index 8cf9fd8ab..43cfa7180 100644 --- a/contrib/cirrus/packer/image-builder-image_base-setup.sh +++ b/contrib/cirrus/packer/image-builder-image_base-setup.sh @@ -11,12 +11,7 @@ set -e # Load in library (copied by packer, before this script was run) source $GOSRC/$SCRIPT_BASE/lib.sh -req_env_var " - TIMESTAMP $TIMESTAMP - GOSRC $GOSRC - SCRIPT_BASE $SCRIPT_BASE - PACKER_BASE $PACKER_BASE -" +req_env_var TIMESTAMP GOSRC SCRIPT_BASE PACKER_BASE install_ooe diff --git a/contrib/cirrus/packer/rhel_base-setup.sh b/contrib/cirrus/packer/rhel_base-setup.sh index fbf9f61af..8d5892d7d 100644 --- a/contrib/cirrus/packer/rhel_base-setup.sh +++ b/contrib/cirrus/packer/rhel_base-setup.sh @@ -10,9 +10,7 @@ set -e # Load in library (copied by packer, before this script was run) source $GOSRC/$SCRIPT_BASE/lib.sh -req_env_var " - RHSM_COMMAND $RHSM_COMMAND -" +req_env_var RHSM_COMMAND install_ooe diff --git a/contrib/cirrus/packer/rhel_setup.sh b/contrib/cirrus/packer/rhel_setup.sh index 20be97f9b..45f5c3e9b 100644 --- a/contrib/cirrus/packer/rhel_setup.sh +++ b/contrib/cirrus/packer/rhel_setup.sh @@ -8,13 +8,7 @@ set -e # Load in library (copied by packer, before this script was run) source /tmp/libpod/$SCRIPT_BASE/lib.sh -req_env_var " -SCRIPT_BASE $SCRIPT_BASE -CNI_COMMIT $CNI_COMMIT -CRIO_COMMIT $CRIO_COMMIT -CRIU_COMMIT $CRIU_COMMIT -RHSM_COMMAND $RHSM_COMMAND -" +req_env_var SCRIPT_BASE CNI_COMMIT CRIO_COMMIT CRIU_COMMIT RHSM_COMMAND install_ooe diff --git a/contrib/cirrus/packer/ubuntu_setup.sh b/contrib/cirrus/packer/ubuntu_setup.sh index e84566ce3..d3ac8bddb 100644 --- a/contrib/cirrus/packer/ubuntu_setup.sh +++ b/contrib/cirrus/packer/ubuntu_setup.sh @@ -8,13 +8,7 @@ set -e # Load in library (copied by packer, before this script was run) source /tmp/libpod/$SCRIPT_BASE/lib.sh -req_env_var " -SCRIPT_BASE $SCRIPT_BASE -CNI_COMMIT $CNI_COMMIT -CRIO_COMMIT $CRIO_COMMIT -CRIU_COMMIT $CRIU_COMMIT -RUNC_COMMIT $RUNC_COMMIT -" +req_env_var SCRIPT_BASE CNI_COMMIT CRIO_COMMIT CRIU_COMMIT RUNC_COMMIT install_ooe diff --git a/contrib/cirrus/rootless_test.sh b/contrib/cirrus/rootless_test.sh index 09c57f993..2803d4318 100755 --- a/contrib/cirrus/rootless_test.sh +++ b/contrib/cirrus/rootless_test.sh @@ -6,11 +6,7 @@ source $HOME/.bash_profile cd $GOSRC source $(dirname $0)/lib.sh -req_env_var " -GOSRC $GOSRC -OS_RELEASE_ID $OS_RELEASE_ID -OS_RELEASE_VER $OS_RELEASE_VER -" +req_env_var GOSRC OS_RELEASE_ID OS_RELEASE_VER if [[ "$UID" == "0" ]] then diff --git a/contrib/cirrus/setup_container_environment.sh b/contrib/cirrus/setup_container_environment.sh index 23df4fe8b..eda6f6167 100755 --- a/contrib/cirrus/setup_container_environment.sh +++ b/contrib/cirrus/setup_container_environment.sh @@ -3,11 +3,7 @@ set -e source $(dirname $0)/lib.sh -req_env_var " -GOSRC $GOSRC -OS_RELEASE_ID $OS_RELEASE_ID -CONTAINER_RUNTIME $CONTAINER_RUNTIME -" +req_env_var GOSRC OS_RELEASE_ID CONTAINER_RUNTIME DIST=$OS_RELEASE_ID IMAGE=${DIST}podmanbuild diff --git a/contrib/cirrus/setup_environment.sh b/contrib/cirrus/setup_environment.sh index 3818abbc7..3bc6c2290 100755 --- a/contrib/cirrus/setup_environment.sh +++ b/contrib/cirrus/setup_environment.sh @@ -6,12 +6,7 @@ source $(dirname $0)/lib.sh record_timestamp "env. setup start" -req_env_var " -USER $USER -HOME $HOME -ENVLIB $ENVLIB -SCRIPT_BASE $SCRIPT_BASE -CIRRUS_BUILD_ID $CIRRUS_BUILD_ID" +req_env_var USER HOME ENVLIB SCRIPT_BASE CIRRUS_BUILD_ID [[ "$SHELL" =~ "bash" ]] || chsh -s /bin/bash diff --git a/contrib/cirrus/success.sh b/contrib/cirrus/success.sh index 2b0cf4655..c4e150514 100755 --- a/contrib/cirrus/success.sh +++ b/contrib/cirrus/success.sh @@ -4,10 +4,7 @@ set -e source $(dirname $0)/lib.sh -req_env_var " - CIRRUS_BRANCH $CIRRUS_BRANCH - CIRRUS_BUILD_ID $CIRRUS_BUILD_ID -" +req_env_var CIRRUS_BRANCH CIRRUS_BUILD_ID REF=$(basename $CIRRUS_BRANCH) # PR number or branch named URL="https://cirrus-ci.com/build/$CIRRUS_BUILD_ID" diff --git a/contrib/cirrus/system_test.sh b/contrib/cirrus/system_test.sh index cb179407a..dd5ef511d 100755 --- a/contrib/cirrus/system_test.sh +++ b/contrib/cirrus/system_test.sh @@ -3,11 +3,7 @@ set -e source $(dirname $0)/lib.sh -req_env_var " -GOSRC $GOSRC -OS_RELEASE_ID $OS_RELEASE_ID -OS_RELEASE_VER $OS_RELEASE_VER -" +req_env_var GOSRC OS_RELEASE_ID OS_RELEASE_VER clean_env diff --git a/contrib/cirrus/unit_test.sh b/contrib/cirrus/unit_test.sh index 4ace19d10..a0964061f 100755 --- a/contrib/cirrus/unit_test.sh +++ b/contrib/cirrus/unit_test.sh @@ -3,11 +3,7 @@ set -e source $(dirname $0)/lib.sh -req_env_var " -GOSRC $GOSRC -OS_RELEASE_ID $OS_RELEASE_ID -OS_RELEASE_VER $OS_RELEASE_VER -" +req_env_var GOSRC OS_RELEASE_ID OS_RELEASE_VER record_timestamp "unit test start" -- cgit v1.2.3-54-g00ecf