Cirrus: Combine build and code consistency tasks

It's conceivable for CI to spend a lot of time testing code which
otherwise should be rejected due to quality problems.  Previously this
was validated in a dedicated task, however a failure would still fail
the CI run.  Simplify the number of CI tasks by combining the consistency
check at the tail-end of the build task.

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

.cirrus.yml

@@ -148,6 +148,8 @@ build_task:
     # all required external/3rd-party services are available and functional.
     # Standard main execution stage call, used by nearly every task in CI.
     main_script: &main '/usr/bin/time --verbose --output="$STATS_LOGFILE" $GOSRC/$SCRIPT_BASE/runner.sh'
+    # Attempt to catch code-quality and vendoring problems early.
+    postbuild_script: &postbuild $SCRIPT_BASE/postbuild.sh
     # Cirrus-CI is very slow uploading one file at time, and the repo contains
     # thousands of files.  Speed this up by archiving into tarball first.
     repo_prep_script: &repo_prep >-
@@ -160,7 +162,6 @@ build_task:
             path: ./*-${STATS_LOGFILE_SFX}
             type: text/plain
 
-
 build_aarch64_task:
     alias: 'build_aarch64'
     name: 'Build for $DISTRO_NV'
@@ -182,6 +183,7 @@ build_aarch64_task:
     clone_script: *full_clone
     prebuild_script: *prebuild
     setup_script: *setup
+    postbuild_script: *postbuild
     main_script: *main
     # Cirrus-CI is very slow uploading one file at time, and the repo contains
     # thousands of files.  Speed this up by archiving into tarball first.
@@ -325,56 +327,6 @@ swagger_task:
             type: text/plain
 
 
-# Check that all included go modules from other sources match
-# what is expected in `vendor/modules.txt` vs `go.mod`.  Also
-# make sure that the generated bindings in pkg/bindings/...
-# are in sync with the code.
-consistency_task:
-    name: "Test Code Consistency"
-    alias: consistency
-    # Docs: ./contrib/cirrus/CIModes.md
-    only_if: *is_pr
-    depends_on:
-        - build
-    container: &smallcontainer
-        image: ${CTR_FQIN}
-        # Resources are limited across ALL currently executing tasks
-        # ref: https://cirrus-ci.org/guide/linux/#linux-containers
-        cpu: 2
-        memory: 2
-    env:
-        <<: *stdenvars
-        TEST_FLAVOR: consistency
-        TEST_ENVIRON: container
-        CTR_FQIN: ${FEDORA_CONTAINER_FQIN}
-    clone_script: *get_gosrc
-    setup_script: *setup
-    main_script: *main
-    always: *runner_stats
-
-
-# Check that all included go modules from other sources match
-# what is expected in `vendor/modules.txt` vs `go.mod`.  Also
-# make sure that the generated bindings in pkg/bindings/...
-# are in sync with the code.
-consistency_aarch64_task:
-    name: "Test Code Consistency (aarch64)"
-    alias: consistency_aarch64
-    # Docs: ./contrib/cirrus/CIModes.md
-    only_if: *is_pr
-    depends_on:
-        - build_aarch64
-    ec2_instance: *standard_build_ec2_aarch64
-    env:
-        <<: *stdenvars_aarch64
-        TEST_FLAVOR: consistency
-        TEST_ENVIRON: container
-    clone_script: *get_gosrc_aarch64
-    setup_script: *setup
-    main_script: *main
-    always: *runner_stats
-
-
 # There are several other important variations of podman which
 # must always build successfully.  Most of them are handled in
 # this task, though a few need dedicated tasks which follow.
@@ -978,8 +930,6 @@ success_task:
         - validate_aarch64
         - bindings
         - swagger
-        - consistency
-        - consistency_aarch64
         - alt_build
         - osx_alt_build
         - win_installer
@@ -1004,7 +954,12 @@ success_task:
         - upgrade_test
         - image_build
         - meta
-    container: *smallcontainer
+    container: &smallcontainer
+        image: ${CTR_FQIN}
+        # Resources are limited across ALL currently executing tasks
+        # ref: https://cirrus-ci.org/guide/linux/#linux-containers
+        cpu: 2
+        memory: 2
     env:
         CTR_FQIN: ${FEDORA_CONTAINER_FQIN}
         TEST_ENVIRON: container

contrib/cirrus/CIModes.md

@@ -47,7 +47,6 @@ of this document, it's not possible to override the behavior of `$CIRRUS_PR`.
 + validate
 + bindings
 + swagger
-+ consistency
 + *alt_build*
 + osx_alt_build
 + docker-py_test
@@ -77,7 +76,6 @@ of this document, it's not possible to override the behavior of `$CIRRUS_PR`.
 + *build*
 + validate
 + swagger
-+ consistency
 + meta
 + success
 
@@ -85,14 +83,12 @@ of this document, it's not possible to override the behavior of `$CIRRUS_PR`.
 + *build*
 + validate
 + swagger
-+ consistency
 + meta
 + success
 
 ### Intend `[CI:BUILD]` PR Tasks:
 + *build*
 + validate
-+ consistency
 + *alt_build*
 + osx_alt_build
 + test_image_build
@@ -101,6 +97,7 @@ of this document, it's not possible to override the behavior of `$CIRRUS_PR`.
 + artifacts
 
 ### Intended Branch tasks (and Cirrus-cron jobs, except "multiarch"):
++ *build*
 + swagger
 + *alt_build*
 + osx_alt_build

contrib/cirrus/check_go_changes.sh

@@ -36,6 +36,8 @@ then
     exit 0
 fi
 
+# Defined by/in Cirrus-CI config.
+# shellcheck disable=SC2154
 base=$(git merge-base $DEST_BRANCH $CIRRUS_CHANGE_IN_REPO)
 diffs=$(git diff $base $CIRRUS_CHANGE_IN_REPO -- '*.go' ':^vendor/')
 

contrib/cirrus/postbuild.sh

@@ -0,0 +1,30 @@
+#!/bin/bash
+
+set -eo pipefail
+
+# This script attempts to confirm all included go modules from
+# other sources match what is expected in `vendor/modules.txt`
+# vs `go.mod`.  Also make sure that the generated bindings in
+# `pkg/bindings/...` are in sync with the code.  It's intended
+# for use after successfully building podman, to prevent wasting
+# time on tests that might otherwise succeed with bad/ugly/invalid
+# code.
+
+source /etc/automation_environment
+source $AUTOMATION_LIB_PATH/common_lib.sh
+
+# Defined by the CI system
+# shellcheck disable=SC2154
+cd $CIRRUS_WORKING_DIR
+
+showrun make .install.goimports
+showrun make vendor
+SUGGESTION="run 'make vendor' and commit all changes" ./hack/tree_status.sh
+showrun make generate-bindings
+SUGGESTION="run 'make generate-bindings' and commit all changes" ./hack/tree_status.sh
+showrun make completions
+SUGGESTION="run 'make completions' and commit all changes" ./hack/tree_status.sh
+
+# Defined in Cirrus-CI config.
+# shellcheck disable=SC2154
+$SCRIPT_BASE/check_go_changes.sh

contrib/cirrus/prebuild.sh

@@ -17,26 +17,27 @@ req_env_vars CI DEST_BRANCH IMAGE_SUFFIX TEST_FLAVOR TEST_ENVIRON \
              SCRIPT_BASE CIRRUS_WORKING_DIR FEDORA_NAME UBUNTU_NAME \
              VM_IMAGE_NAME
 
-# There's no need to perform further checks on more than one
-# CI platform.  These variables are defined in .cirrus.yml
+# Defined by the CI system
 # shellcheck disable=SC2154
-if [[ ! "${DISTRO_NV}" =~ ${FEDORA_NAME} ]]; then
-    echo "Skipping additional checks on $DISTRO_NV"
-    exit 0
+cd $CIRRUS_WORKING_DIR
+
+# Defined by CI config.
+# shellcheck disable=SC2154
+showrun $SCRIPT_BASE/cirrus_yaml_test.py
+
+# Defined by CI config.
+# shellcheck disable=SC2154
+if [[ "${DISTRO_NV}" =~ fedora ]]; then
+    showrun ooe.sh dnf install -y ShellCheck  # small/quick addition
+    showrun shellcheck --color=always --format=tty \
+        --shell=bash --external-sources \
+        --enable add-default-case,avoid-nullary-conditions,check-unassigned-uppercase \
+        --exclude SC2046,SC2034,SC2090,SC2064 \
+        --wiki-link-count=0 --severity=warning \
+        $SCRIPT_BASE/*.sh hack/get_ci_vm.sh
 fi
 
-# shellcheck disable=SC2154
-$SCRIPT_BASE/cirrus_yaml_test.py
-
-ooe.sh dnf install -y ShellCheck  # small/quick addition
-
-shellcheck --color=always --format=tty \
-    --shell=bash --external-sources \
-    --enable add-default-case,avoid-nullary-conditions,check-unassigned-uppercase \
-    --exclude SC2046,SC2034,SC2090,SC2064 \
-    --wiki-link-count=0 --severity=warning \
-    $SCRIPT_BASE/*.sh hack/get_ci_vm.sh
-
+msg "Checking 3rd party network service connectivity"
 # shellcheck disable=SC2154
 cat ${CIRRUS_WORKING_DIR}/${SCRIPT_BASE}/required_host_ports.txt | \
     while read host port
@@ -66,9 +67,11 @@ TEST_IMGS=(\
     cirros:latest
 )
 
-echo "Checking quay.io test image accessibility"
+msg "Checking quay.io test image accessibility"
 for testimg in "${TEST_IMGS[@]}"; do
     fqin="quay.io/libpod/$testimg"
     echo "    $fqin"
+    # Belt-and-suspenders: Catch skopeo (somehow) returning False or null
+    # in addition to "bad" (invalid) JSON.
     skopeo inspect --retry-times 5 "docker://$fqin" | jq -e . > /dev/null
 done

contrib/cirrus/runner.sh

@@ -211,16 +211,6 @@ eof
     rm -f $envvarsfile
 }
 
-function _run_consistency() {
-    make vendor
-    SUGGESTION="run 'make vendor' and commit all changes" ./hack/tree_status.sh
-    make generate-bindings
-    SUGGESTION="run 'make generate-bindings' and commit all changes" ./hack/tree_status.sh
-    make completions
-    SUGGESTION="run 'make completions' and commit all changes" ./hack/tree_status.sh
-    $SCRIPT_BASE/check_go_changes.sh
-}
-
 function _run_build() {
     # Ensure always start from clean-slate with all vendor modules downloaded
     make clean

contrib/cirrus/setup_environment.sh

@@ -364,10 +364,6 @@ case "$TEST_FLAVOR" in
             docker.io/gitlab/gitlab-runner-helper:x86_64-latest-pwsh
         ;;
     swagger) ;&  # use next item
-    consistency)
-        make clean
-        make .install.goimports
-        ;;
     release) ;;
     *) die_unknown TEST_FLAVOR
 esac