Cirrus: Fix running Validate task on branches

Minor: Correct `$DEST_BRANCH` value to actual branch name. Otherwise this is similar change to PR containers#9063. 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 <[email protected]>
giuseppe · Feb 22, 2021 · d61babf · d61babf
1 parent 82e4e66
commit d61babf
Show file tree

Hide file tree

Showing 5 changed files with 56 additions and 74 deletions.
diff --git a/.cirrus.yml b/.cirrus.yml
@@ -6,7 +6,7 @@ env:
     #### Global variables used for all tasks
     ####
     # Name of the ultimate destination branch for this CI run, PR or post-merge.
-    DEST_BRANCH: "v2.2"
+    DEST_BRANCH: "v2.2.1-rhel"
     # Overrides default location (/tmp/cirrus) for repo clone
     GOPATH: &gopath "/var/tmp/go"
     GOBIN: "${GOPATH}/bin"
@@ -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'
@@ -62,45 +62,6 @@ timeout_in: 60m
 gcp_credentials: ENCRYPTED[a28959877b2c9c36f151781b0a05407218cda646c7d047fc556e42f55e097e897ab63ee78369dae141dcf0b46a9d0cdd]
 
 
-# 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"
-    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
-    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
-        runner_stats_artifacts:
-            path: ./*-${STATS_LOGFILE_SFX}
-            type: text/plain
-
-
 # N/B: This task is critical.  It builds all binaries and release archives
 # for the project, using all primary OS platforms and versions.  Assuming
 # the builds are successful, a cache is stored of the entire `$GOPATH`
@@ -152,13 +113,27 @@ build_task:
         folder: *gopath  # Required hard-coded path, no variables.
         fingerprint_script: echo "$_BUILD_CACHE_HANDLE"
         # Cheat: Clone here when cache is empty, guaranteeing consistency.
-        populate_script: *full_clone
-    # A normal clone would invalidate useful cache
+        populate_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
     clone_script: &noop mkdir -p $CIRRUS_WORKING_DIR
-    setup_script: *setup
-    main_script: *main
+    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: &binary_artifacts
-        <<: *runner_stats
+        runner_stats_artifacts: &runner_stats
+            path: ./*-${STATS_LOGFILE_SFX}
+            type: text/plain
         gosrc_artifacts:
             path: ./*  # Grab everything in top-level $GOSRC
             type: application/octet-stream
@@ -173,8 +148,13 @@ build_task:
 validate_task:
     name: "Validate $DISTRO_NV Build"
     alias: validate
+    # 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 "$CIRRUS_PR == '' || $CIRRUS_TAG != ''" # Don't run on branches/tags
     depends_on:
-        - smoke
         - build
     # golangci-lint is a very, very hungry beast.
     gce_instance: &bigvm
@@ -190,7 +170,8 @@ validate_task:
     clone_script: *noop
     setup_script: *setup
     main_script: *main
-    always: *runner_stats
+    always:
+        runner_stats_artifacts: *runner_stats
 
 
 # This task is critical.  It updates the "last-used by" timestamp stored
@@ -227,7 +208,6 @@ success_task:
     alias: success
     # N/B: ALL tasks must be listed here, minus their '_task' suffix.
     depends_on:
-        - smoke
         - build
         - validate
         - meta

diff --git 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
@@ -23,18 +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
-        make .gitvalidation
-    fi
-}
-
 function _run_automation() {
     $SCRIPT_BASE/cirrus_yaml_test.py
 
@@ -47,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
@@ -173,7 +173,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
@@ -149,11 +149,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}}"
@@ -255,7 +258,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