Cirrus: Fix running Validate task on branches

Followup to dbb9943

Despite skipping the `Smoke` check, it was observed on a *new* branch,
the `validate` task (specifically `git-validation`) will fail.  This
is because:

* `$CIRRUS_LAST_GREEN_CHANGE` will be empty on a new branch.
* `$CIRRUS_BASE_SHA` is always empty for runs triggered by branch-push
* `$EPOCH_TEST_COMMIT` will be set to `YOU_FOUND_A_BUG`.

Fix this by eliminating the `Smoke` task entirely, simplifying all
the `make validate` operations into the `validate` cirrus task.  Ensure
this task does not run when a new branch or tag is pushed.

Also, eliminate the `$CIRRUS_BUILD_ID` value as it's confusing and not
actually used anywhere.  It was formerly used for building VM images,
but this has moved to another repo entirely.

Signed-off-by: Chris Evich <cevich@redhat.com>

5 files changed, 54 insertions, 70 deletions

diff --git a/.cirrus.yml b/.cirrus.yml
index 5a4815e1c..1b824909c 100644
--- a/.cirrus.yml
+++ b/.cirrus.yml
@@ -46,7 +46,7 @@ env:
     #### Control variables that determine what to run and how to run it.
     #### N/B: Required ALL of these are set for every single task.
     ####
-    TEST_FLAVOR:             # int, sys, ext_svc, smoke, automation, etc.
+    TEST_FLAVOR:             # int, sys, ext_svc, validate, automation, etc.
     TEST_ENVIRON: host       # 'host' or 'container'
     PODBIN_NAME: podman      # 'podman' or 'remote'
     PRIV_NAME: root          # 'root' or 'rootless'
@@ -78,6 +78,25 @@ ext_svc_check_task:
     env:
         TEST_FLAVOR: ext_svc
         CTR_FQIN: ${FEDORA_CONTAINER_FQIN}
+    # NOTE: The default way Cirrus-CI clones is *NOT* compatible with
+    #       environment expectations in contrib/cirrus/lib.sh.  Specifically
+    #       the 'origin' remote must be defined, and all remote branches/tags
+    #       must be available for reference from CI scripts.
+    clone_script: &full_clone |
+          cd /
+          rm -rf $CIRRUS_WORKING_DIR
+          mkdir -p $CIRRUS_WORKING_DIR
+          git clone --recursive --branch=$DEST_BRANCH https://x-access-token:${CIRRUS_REPO_CLONE_TOKEN}@github.com/${CIRRUS_REPO_FULL_NAME}.git $CIRRUS_WORKING_DIR
+          cd $CIRRUS_WORKING_DIR
+          git remote update origin
+          if [[ -n "$CIRRUS_PR" ]]; then # running for a PR
+              git fetch origin pull/$CIRRUS_PR/head:pull/$CIRRUS_PR
+              git checkout pull/$CIRRUS_PR
+          else
+              git reset --hard $CIRRUS_CHANGE_IN_REPO
+          fi
+          make install.tools
+
     setup_script: &setup '$GOSRC/$SCRIPT_BASE/setup_environment.sh'
     main_script: &main '/usr/bin/time --verbose --output="$STATS_LOGFILE" $GOSRC/$SCRIPT_BASE/runner.sh'
     always: &runner_stats
@@ -97,43 +116,7 @@ automation_task:
         TEST_FLAVOR: automation
         CTR_FQIN: ${FEDORA_CONTAINER_FQIN}
         TEST_ENVIRON: container
-    setup_script: *setup
-    main_script: *main
-    always: *runner_stats
-
-
-# This task use to be called 'gating', however that name is being
-# used downstream for release testing.  Renamed this to avoid confusion.
-# All it does is run basic golang formatting and commit validation checks.
-smoke_task:
-    alias: 'smoke'
-    name: "Smoke Test"
-    skip: *branches_and_tags
-    container: &bigcontainer
-        image: ${CTR_FQIN}
-        # Leave some resources for smallcontainer
-        cpu: 6
-        memory: 22
-    env:
-        TEST_FLAVOR: 'smoke'
-        CTR_FQIN: "${FEDORA_CONTAINER_FQIN}"
-        TEST_ENVIRON: container
-    # This clone script is also used to initially populate gopath_cache (below)
-    clone_script: &full_clone |
-        cd /
-        rm -rf $CIRRUS_WORKING_DIR
-        mkdir -p $CIRRUS_WORKING_DIR
-        git clone --recursive --branch=$DEST_BRANCH https://x-access-token:${CIRRUS_REPO_CLONE_TOKEN}@github.com/${CIRRUS_REPO_FULL_NAME}.git $CIRRUS_WORKING_DIR
-        cd $CIRRUS_WORKING_DIR
-        git remote update origin
-        if [[ -n "$CIRRUS_PR" ]]; then # running for a PR
-            git fetch origin pull/$CIRRUS_PR/head:pull/$CIRRUS_PR
-            git checkout pull/$CIRRUS_PR
-        else
-            git reset --hard $CIRRUS_CHANGE_IN_REPO
-        fi
-        cd $CIRRUS_WORKING_DIR
-        make install.tools
+    clone_script: *full_clone
     setup_script: *setup
     main_script: *main
     always: *runner_stats
@@ -211,11 +194,15 @@ build_task:
 validate_task:
     name: "Validate $DISTRO_NV Build"
     alias: validate
-    skip: *tags
+    # This task is primarily intended to catch human-errors early on, in a
+    # PR.  Skip it for branch-push, branch-create, and tag-push to improve
+    # automation reliability/speed in those contexts.  Any missed errors due
+    # to nonsequential PR merging practices, will be caught on a future PR,
+    # build or test task failures.
+    skip: *branches_and_tags
     depends_on:
         - ext_svc_check
         - automation
-        - smoke
         - build
     # golangci-lint is a very, very hungry beast.
     gce_instance: &bigvm
@@ -646,7 +633,6 @@ success_task:
     depends_on:
         - ext_svc_check
         - automation
-        - smoke
         - build
         - validate
         - bindings
diff --git a/contrib/cirrus/lib.sh b/contrib/cirrus/lib.sh
index bc9a95310..451a267b3 100644
--- a/contrib/cirrus/lib.sh
+++ b/contrib/cirrus/lib.sh
@@ -80,10 +80,19 @@ CIRRUS_CI="${CIRRUS_CI:-false}"
 DEST_BRANCH="${DEST_BRANCH:-master}"
 CONTINUOUS_INTEGRATION="${CONTINUOUS_INTEGRATION:-false}"
 CIRRUS_REPO_NAME=${CIRRUS_REPO_NAME:-podman}
-# N/B: CIRRUS_BASE_SHA is empty on branch and tag push.
-CIRRUS_BASE_SHA=${CIRRUS_BASE_SHA:-${CIRRUS_LAST_GREEN_CHANGE:-YOU_FOUND_A_BUG}}
-CIRRUS_BUILD_ID=${CIRRUS_BUILD_ID:-$RANDOM$(date +%s)}  # must be short and unique
-
+# Cirrus only sets $CIRRUS_BASE_SHA properly for PRs, but $EPOCH_TEST_COMMIT
+# needs to be set from this value in order for `make validate` to run properly.
+# When running get_ci_vm.sh, most $CIRRUS_xyz variables are empty. Attempt
+# to accomidate both branch and get_ci_vm.sh testing by discovering the base
+# branch SHA value.
+# shellcheck disable=SC2154
+if [[ -z "$CIRRUS_BASE_SHA" ]] && [[ -z "$CIRRUS_TAG" ]]
+then  # Operating on a branch, or under `get_ci_vm.sh`
+    CIRRUS_BASE_SHA=$(git rev-parse ${UPSTREAM_REMOTE:-origin}/$DEST_BRANCH)
+elif [[ -z "$CIRRUS_BASE_SHA" ]]
+then  # Operating on a tag
+    CIRRUS_BASE_SHA=$(git rev-parse HEAD)
+fi
 # The starting place for linting and code validation
 EPOCH_TEST_COMMIT="$CIRRUS_BASE_SHA"
 
diff --git a/contrib/cirrus/runner.sh b/contrib/cirrus/runner.sh
index 50bc1102f..50d615105 100755
--- a/contrib/cirrus/runner.sh
+++ b/contrib/cirrus/runner.sh
@@ -23,22 +23,6 @@ function _run_ext_svc() {
     $SCRIPT_BASE/ext_svc_check.sh
 }
 
-function _run_smoke() {
-    make gofmt
-
-    # There is little value to validating commits after tag-push
-    # and it's very difficult to automatically determine a starting commit.
-    # $CIRRUS_TAG is only non-empty when executing due to a tag-push
-    # shellcheck disable=SC2154
-    if [[ -z "$CIRRUS_TAG" ]]; then
-        # If PR consists of multiple commits, test that each compiles cleanly
-        make .gitvalidation
-
-        # PRs should include some way to test.
-        $SCRIPT_BASE/pr-should-include-tests
-    fi
-}
-
 function _run_automation() {
     $SCRIPT_BASE/cirrus_yaml_test.py
 
@@ -51,11 +35,14 @@ function _run_automation() {
 }
 
 function _run_validate() {
-    # Confirm compile via prior task + cache
-    bin/podman --version
-    bin/podman-remote --version
+    # git-validation tool fails if $EPOCH_TEST_COMMIT is empty
+    # shellcheck disable=SC2154
+    if [[ -n "$EPOCH_TEST_COMMIT" ]]; then
+        make validate
+    else
+        warn "Skipping git-validation since \$EPOCH_TEST_COMMIT is empty"
+    fi
 
-    make validate  # Some items require a build
 }
 
 function _run_unit() {
diff --git a/contrib/cirrus/setup_environment.sh b/contrib/cirrus/setup_environment.sh
index 9267b8a1c..4c95d0254 100755
--- a/contrib/cirrus/setup_environment.sh
+++ b/contrib/cirrus/setup_environment.sh
@@ -181,7 +181,6 @@ esac
 # shellcheck disable=SC2154
 case "$TEST_FLAVOR" in
     ext_svc) ;;
-    smoke) ;&
     validate)
         # For some reason, this is also needed for validation
         make .install.pre-commit
diff --git a/hack/get_ci_vm.sh b/hack/get_ci_vm.sh
index d1e38eb35..4f6c42a06 100755
--- a/hack/get_ci_vm.sh
+++ b/hack/get_ci_vm.sh
@@ -157,11 +157,14 @@ parse_args(){
 
     VM_IMAGE_NAME="$1"
 
-    # Word-splitting is desirable in this case
-    # shellcheck disable=SC2207
+    # Word-splitting is desirable in this case.
+    # Values are used literally (with '=') as args to future `env` command.
+    # get_env_vars() will take care of properly quoting it's output.
+    # shellcheck disable=SC2207,SC2191
     ENVS=(
         $(get_env_vars)
-        "VM_IMAGE_NAME=$VM_IMAGE_NAME"
+        VM_IMAGE_NAME="$VM_IMAGE_NAME"
+        UPSTREAM_REMOTE="upstream"
     )
 
     VMNAME="${VMNAME:-${USER}-${VM_IMAGE_NAME}}"
@@ -263,7 +266,7 @@ echo -e "Note: Script can be re-used in another terminal if needed."
 echo -e "${RED}(option to delete VM presented upon exiting).${NOR}"
 # TODO: This is fairly fragile, specifically the quoting for the remote command.
 echo '#!/bin/bash' > $TMPDIR/ssh
-echo "$SSH_CMD -- -t 'cd $GOSRC && exec env \"${ENVS[*]}\" bash -il'" >> $TMPDIR/ssh
+echo "$SSH_CMD -- -t 'cd $GOSRC && exec env ${ENVS[*]} bash -il'" >> $TMPDIR/ssh
 chmod +x $TMPDIR/ssh
 
 showrun $TMPDIR/ssh