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 <santiago@redhat.com>

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/.

contrib/cirrus/build_vm_images.sh

@@ -3,25 +3,7 @@
 set -e
 source $(dirname $0)/lib.sh
 
-req_env_var "
+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
-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
-"
 
 record_timestamp "cache-image build start"
 

contrib/cirrus/integration_test.sh

@@ -3,13 +3,7 @@
 set -e
 source $(dirname $0)/lib.sh
 
-req_env_var "
+req_env_var GOSRC SCRIPT_BASE OS_RELEASE_ID OS_RELEASE_VER CONTAINER_RUNTIME
-GOSRC $GOSRC
-SCRIPT_BASE $SCRIPT_BASE
-OS_RELEASE_ID $OS_RELEASE_ID
-OS_RELEASE_VER $OS_RELEASE_VER
-CONTAINER_RUNTIME $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 \

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
+# Pass in a list of one or more envariable names; exit non-zero with
-# exit non-zero with helpful error message if any value is empty
+# helpful error message if any value is empty
 req_env_var() {
-    echo "$1" | while read NAME VALUE
+    # Provide context. If invoked from function use its name; else script name
-    do
+    local caller=${FUNCNAME[1]}
-        if [[ -n "$NAME" ]] && [[ -z "$VALUE" ]]
+    if [[ -n "$caller" ]]; then
-        then
+        # Indicate that it's a function name
-            echo "Required env. var. \$$NAME is not set"
+        caller="$caller()"
-            exit 9
+    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 "
+    req_env_var UNSET_ENV_VARS
-        UNSET_ENV_VARS $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 "
+    echo "${2:-FATAL ERROR (but no message given!) in ${FUNCNAME[1]}()}"
-        1 $1
+    exit ${1:-1}
-        2 $2
-    "
-    echo "$2"
-    exit $1
 }
 
 # Return a GCE image-name compatible string representation of distribution name
@@ -135,10 +141,8 @@ stub() {
 }
 
 ircmsg() {
-    req_env_var "
+    req_env_var CIRRUS_TASK_ID
-        CIRRUS_TASK_ID $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 "
+    req_env_var ROOTLESS_USER GOSRC ENVLIB
-        ROOTLESS_USER $ROOTLESS_USER
-        GOSRC $GOSRC
-        ENVLIB $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 "
+    req_env_var GOPATH CNI_COMMIT
-        GOPATH $GOPATH
-        CNI_COMMIT $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 "
+    req_env_var GOPATH RUNC_COMMIT OS_RELEASE_ID
-        GOPATH $GOPATH
-        RUNC_COMMIT $RUNC_COMMIT
-        OS_RELEASE_ID $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 "
+    req_env_var GOPATH CRIO_COMMIT
-        GOPATH $GOPATH
-        CRIO_COMMIT $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 "
+    req_env_var CRIU_COMMIT
-    CRIU_COMMIT $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 "
+    req_env_var GOPATH RHSMCMD
-        GOPATH $GOPATH
-        RHSMCMD $RHSMCMD
-    "
     cd /
     sudo rm -rf "$RHSMCMD"
     sudo rm -rf "$GOPATH"
@@ -431,9 +416,7 @@ rhel_exit_handler() {
 }
 
 rhsm_enable() {
-    req_env_var "
+    req_env_var RHSM_COMMAND
-        RHSM_COMMAND $RHSM_COMMAND
-    "
     export GOPATH="$(mktemp -d)"
     export RHSMCMD="$(mktemp)"
     trap "rhel_exit_handler" EXIT

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

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 "
+req_env_var SCRIPT_BASE CNI_COMMIT CRIO_COMMIT CRIU_COMMIT
-SCRIPT_BASE $SCRIPT_BASE
-CNI_COMMIT $CNI_COMMIT
-CRIO_COMMIT $CRIO_COMMIT
-CRIU_COMMIT $CRIU_COMMIT
-"
 
 install_ooe
 

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 "
+req_env_var SCRIPT_BASE
-SCRIPT_BASE $SCRIPT_BASE
-"
 
 install_ooe
 

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 "
+req_env_var SCRIPT_BASE FEDORA_CNI_COMMIT CNI_COMMIT CRIO_COMMIT CRIU_COMMIT RUNC_COMMIT
-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
-"
 
 install_ooe
 

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 "
+req_env_var TIMESTAMP GOSRC SCRIPT_BASE PACKER_BASE
-    TIMESTAMP $TIMESTAMP
-    GOSRC $GOSRC
-    SCRIPT_BASE $SCRIPT_BASE
-    PACKER_BASE $PACKER_BASE
-"
 
 install_ooe
 

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 "
+req_env_var RHSM_COMMAND
-    RHSM_COMMAND $RHSM_COMMAND
-"
 
 install_ooe
 

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 "
+req_env_var SCRIPT_BASE CNI_COMMIT CRIO_COMMIT CRIU_COMMIT RHSM_COMMAND
-SCRIPT_BASE $SCRIPT_BASE
-CNI_COMMIT $CNI_COMMIT
-CRIO_COMMIT $CRIO_COMMIT
-CRIU_COMMIT $CRIU_COMMIT
-RHSM_COMMAND $RHSM_COMMAND
-"
 
 install_ooe
 

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 "
+req_env_var SCRIPT_BASE CNI_COMMIT CRIO_COMMIT CRIU_COMMIT RUNC_COMMIT
-SCRIPT_BASE $SCRIPT_BASE
-CNI_COMMIT $CNI_COMMIT
-CRIO_COMMIT $CRIO_COMMIT
-CRIU_COMMIT $CRIU_COMMIT
-RUNC_COMMIT $RUNC_COMMIT
-"
 
 install_ooe
 

contrib/cirrus/rootless_test.sh

@@ -6,11 +6,7 @@ source $HOME/.bash_profile
 cd $GOSRC
 source $(dirname $0)/lib.sh
 
-req_env_var "
+req_env_var GOSRC OS_RELEASE_ID OS_RELEASE_VER
-GOSRC $GOSRC
-OS_RELEASE_ID $OS_RELEASE_ID
-OS_RELEASE_VER $OS_RELEASE_VER
-"
 
 if [[ "$UID" == "0" ]]
 then

contrib/cirrus/setup_container_environment.sh

@@ -3,11 +3,7 @@ set -e
 
 source $(dirname $0)/lib.sh
 
-req_env_var "
+req_env_var GOSRC OS_RELEASE_ID CONTAINER_RUNTIME
-GOSRC $GOSRC
-OS_RELEASE_ID $OS_RELEASE_ID
-CONTAINER_RUNTIME $CONTAINER_RUNTIME
-"
 
 DIST=$OS_RELEASE_ID
 IMAGE=${DIST}podmanbuild

contrib/cirrus/setup_environment.sh

@@ -6,12 +6,7 @@ source $(dirname $0)/lib.sh
 
 record_timestamp "env. setup start"
 
-req_env_var "
+req_env_var USER HOME ENVLIB SCRIPT_BASE CIRRUS_BUILD_ID
-USER $USER
-HOME $HOME
-ENVLIB $ENVLIB
-SCRIPT_BASE $SCRIPT_BASE
-CIRRUS_BUILD_ID $CIRRUS_BUILD_ID"
 
 [[ "$SHELL" =~ "bash" ]] || chsh -s /bin/bash
 

contrib/cirrus/success.sh

@@ -4,10 +4,7 @@ set -e
 
 source $(dirname $0)/lib.sh
 
-req_env_var "
+req_env_var CIRRUS_BRANCH CIRRUS_BUILD_ID
-    CIRRUS_BRANCH $CIRRUS_BRANCH
-    CIRRUS_BUILD_ID $CIRRUS_BUILD_ID
-"
 
 REF=$(basename $CIRRUS_BRANCH)  # PR number or branch named
 URL="https://cirrus-ci.com/build/$CIRRUS_BUILD_ID"

contrib/cirrus/system_test.sh

@@ -3,11 +3,7 @@
 set -e
 source $(dirname $0)/lib.sh
 
-req_env_var "
+req_env_var GOSRC OS_RELEASE_ID OS_RELEASE_VER
-GOSRC $GOSRC
-OS_RELEASE_ID $OS_RELEASE_ID
-OS_RELEASE_VER $OS_RELEASE_VER
-"
 
 clean_env
 

contrib/cirrus/unit_test.sh

@@ -3,11 +3,7 @@
 set -e
 source $(dirname $0)/lib.sh
 
-req_env_var "
+req_env_var GOSRC OS_RELEASE_ID OS_RELEASE_VER
-GOSRC $GOSRC
-OS_RELEASE_ID $OS_RELEASE_ID
-OS_RELEASE_VER $OS_RELEASE_VER
-"
 
 record_timestamp "unit test start"