Merge pull request #5039 from cevich/fix_gobin_exit_bug

Cirrus: Fix gate image & false-positive exits

9 files changed, 117 insertions, 58 deletions

diff --git a/.cirrus.yml b/.cirrus.yml
index 39b2bdc1a..692282d61 100644
--- a/.cirrus.yml
+++ b/.cirrus.yml
@@ -17,6 +17,7 @@ env:
     DEST_BRANCH: "master"
     # Overrides default location (/tmp/cirrus) for repo clone
     GOPATH: "/var/tmp/go"
+    GOBIN: "${GOPATH}/bin"
     GOSRC: "/var/tmp/go/src/github.com/containers/libpod"
     CIRRUS_WORKING_DIR: "/var/tmp/go/src/github.com/containers/libpod"
     # The default is 'sh' if unspecified
@@ -88,17 +89,18 @@ gce_instance:
 # quick format, lint, and unit tests on the standard platform.
 gating_task:
 
-    # Only run this on PRs, never during post-merge testing.  This is also required
-    # for proper setting of EPOCH_TEST_COMMIT value, required by validation tools.
+    # Only run this on PRs, never during post-merge testing (for speed).
     only_if: $CIRRUS_BRANCH != $DEST_BRANCH
 
     env:
         CIRRUS_WORKING_DIR: "/usr/src/libpod"
-        GOPATH: "/go"
-        GOSRC: "/go/src/github.com/containers/libpod"
+        SRCPATH: "$CIRRUS_WORKING_DIR"
 
     # Runs within Cirrus's "community cluster"
     container:
+        # Note: Image has dual purpose, see contrib/gate/README.md
+        # The entrypoint.sh script ensures a prestine copy of $SRCPATH is
+        # available at $GOSRC before executing make instructions.
         image: "quay.io/libpod/gate:master"
         cpu: 4
         memory: 12
@@ -136,9 +138,9 @@ gating_task:
 
     # Verify some aspects of ci/related scripts
     ci_script:
-        - '${CIRRUS_WORKING_DIR}/${SCRIPT_BASE}/lib.sh.t |& ${TIMESTAMP}'
+        - '${GOSRC}/${SCRIPT_BASE}/lib.sh.t |& ${TIMESTAMP}'
         - '/usr/local/bin/entrypoint.sh -C ${CIRRUS_WORKING_DIR}/${SCRIPT_BASE}/packer test'
-        - '${CIRRUS_WORKING_DIR}/${SCRIPT_BASE}/cirrus_yaml_test.py |& ${TIMESTAMP}'
+        - '${GOSRC}/${SCRIPT_BASE}/cirrus_yaml_test.py |& ${TIMESTAMP}'
 
     # Verify expected bash environment (-o pipefail)
     pipefail_enabledscript: 'if /bin/false | /bin/true; then echo "pipefail fault" && exit 72; fi'
@@ -220,13 +222,14 @@ varlink_api_task:
 
     env:
         CIRRUS_WORKING_DIR: "/usr/src/libpod"
-        GOPATH: "/go"
-        GOSRC: "/go/src/github.com/containers/libpod"
+        SRCPATH: "$CIRRUS_WORKING_DIR"
+        EPOCH_TEST_COMMIT: "${CIRRUS_BASE_SHA}"  # repo clone missing this data
         # Used by tree_status.sh
         SUGGESTION: 'remove API.md, then "make varlink_api_generate" and commit changes.'
 
     # Runs within Cirrus's "community cluster"
     container:
+        # Note: Image has dual purpose, see contrib/gate/README.md
         image: "quay.io/libpod/gate:master"
         cpu: 4
         memory: 12
@@ -757,12 +760,13 @@ success_task:
 
     env:
         CIRRUS_WORKING_DIR: "/usr/src/libpod"
-        GOPATH: "/go"
-        GOSRC: "/go/src/github.com/containers/libpod"
+        SRCPATH: "$CIRRUS_WORKING_DIR"
+        EPOCH_TEST_COMMIT: "${CIRRUS_BASE_SHA}"  # repo clone missing this data
 
     container:
+        # Note: Image has dual purpose, see contrib/gate/README.md
         image: "quay.io/libpod/gate:master"
         cpu: 1
         memory: 1
 
-    success_script: '$CIRRUS_WORKING_DIR/$SCRIPT_BASE/success.sh |& ${TIMESTAMP}'
+    success_script: '/usr/local/bin/entrypoint.sh ./$SCRIPT_BASE/success.sh |& ${TIMESTAMP}'
diff --git a/.dockerignore b/.dockerignore
new file mode 100644
index 000000000..85914ee63
--- /dev/null
+++ b/.dockerignore
@@ -0,0 +1,3 @@
+.git/
+.github/
+bin/
diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md
index be13b6de3..7e94957eb 100644
--- a/CONTRIBUTING.md
+++ b/CONTRIBUTING.md
@@ -261,24 +261,62 @@ commit automatically with `git commit -s`.
 
 All code changes must pass ``make validate`` and ``make lint``, as
 executed in a standard container.  The container image for this
-purpose is provided at: ``quay.io/libpod/gate:latest``.  However,
-for changes to the image itself, it may also be built locally
-from the repository root, with the command:
+purpose is provided at: ``quay.io/libpod/gate:master``.  With
+other tags available for different branches as needed.  These
+images are built automatically after merges to the branch.
+
+#### Building the gate container locally
+
+For local use, debugging, or experimentation, the gate image may
+be built locally from the repository root, with the command:
 
 ```
-sudo podman build -t quay.io/libpod/gate:latest -f contrib/gate/Dockerfile .
+podman build -t gate -f contrib/gate/Dockerfile .
 ```
 
 ***N/B:*** **don't miss the dot (.) at the end, it's really important**
 
-The container executes 'make' by default, on a copy of the repository.
-This avoids changing or leaving build artifacts in your working directory.
+#### Local use of gate container
+
+The gate container's entry-point executes 'make' by default, on a copy of
+the repository made at runtime.  This avoids the container changing or
+leaving build artifacts in your hosts working directory.  It also guarantees
+every execution is based upon pristine code provided from the host.
+
 Execution does not require any special permissions from the host. However,
-the repository root must be bind-mounted into the container at
-'/usr/src/libpod'. For example, running `make lint` is done (from
-the repository root) with the command:
+your libpod repository clone's root must be bind-mounted to the container at
+'/usr/src/libpod'.  The copy will be made into /var/tmp/go (`$GOSRC` in container)
+before running your make target.  For example, running `make lint` from a
+repository clone at $HOME/devel/libpod could be done with the commands:
+
+```bash
+$ cd $HOME/devel/libpod
+$ podman run -it --rm -v $PWD:/usr/src/libpod:ro \
+    --security-opt label=disable quay.io/libpod/gate:master \
+    lint
+```
+
+***N/B:*** Depending on your clone's git remotes-configuration,
+(esp. for `validate` and `lint` targets), you may also need to reference the
+commit which was your upstream fork-point.  Otherwise you may receive an error
+similar to:
 
-``sudo podman run -it --rm -v $PWD:/usr/src/libpod:ro --security-opt label=disable quay.io/libpod/gate:latest lint``
+```
+fatal: Not a valid object name master
+Makefile:152: *** Required variable EPOCH_TEST_COMMIT value is undefined, whitespace, or empty.  Stop.
+```
+
+For example, assuming your have a remote called `upstream` running the
+validate target should be done like this:
+
+```bash
+$ cd $HOME/devel/libpod
+$ git remote update upstream
+$ export EPOCH_TEST_COMMIT=$(git merge-base upstream/master HEAD)
+$ podman run -it --rm -e EPOCH_TEST_COMMIT -v $PWD:/usr/src/libpod:ro \
+    --security-opt label=disable quay.io/libpod/gate:master \
+    validate
+```
 
 ### Integration Tests
 
diff --git a/contrib/cirrus/lib.sh b/contrib/cirrus/lib.sh
index 1ffe554e9..5895d84f4 100644
--- a/contrib/cirrus/lib.sh
+++ b/contrib/cirrus/lib.sh
@@ -68,9 +68,9 @@ export FEDORA_BASE_IMAGE="fedora-cloud-base-31-1-9-1578586410"
 export PRIOR_FEDORA_BASE_IMAGE="fedora-cloud-base-30-1-2-1578586410"
 export BUILT_IMAGE_SUFFIX="${BUILT_IMAGE_SUFFIX:--$CIRRUS_REPO_NAME-${CIRRUS_BUILD_ID}}"
 # IN_PODMAN container image
-IN_PODMAN_IMAGE="quay.io/libpod/in_podman:latest"
+IN_PODMAN_IMAGE="quay.io/libpod/in_podman:$DEST_BRANCH"
 # Image for uploading releases
-UPLDREL_IMAGE="quay.io/libpod/upldrel:latest"
+UPLDREL_IMAGE="quay.io/libpod/upldrel:master"
 
 # Avoid getting stuck waiting for user input
 export DEBIAN_FRONTEND="noninteractive"
diff --git a/contrib/gate/Dockerfile b/contrib/gate/Dockerfile
index 2a904a202..54bd2cbde 100644
--- a/contrib/gate/Dockerfile
+++ b/contrib/gate/Dockerfile
@@ -33,31 +33,36 @@ RUN dnf -y install \
       zip \
       && dnf clean all
 
-ENV GOPATH="/go" \
-    PATH="/go/bin:/bin:/sbin:/usr/bin:/usr/sbin:/usr/local/bin:/usr/local/sbin" \
+ENV GOPATH="/var/tmp/go" \
+    GOBIN="/var/tmp/go/bin" \
+    PATH="/var/tmp/go/bin:/bin:/sbin:/usr/bin:/usr/sbin:/usr/local/bin:/usr/local/sbin" \
     SRCPATH="/usr/src/libpod" \
-    GOSRC="/go/src/github.com/containers/libpod"
+    GOSRC="/var/tmp/go/src/github.com/containers/libpod"
 
-# Only needed for installing build-time dependencies
+# Only needed for installing build-time dependencies, then will be removed
 COPY / $GOSRC
 
-WORKDIR $GOSRC
-
 # Install dependencies
 RUN set -x && \
+    mkdir -p "$GOBIN" && \
+    mkdir -p /etc/cni/net.d && \
+    mkdir -p /etc/containers && \
     install -D -m 755 $GOSRC/contrib/gate/entrypoint.sh /usr/local/bin/ && \
-    python3 -m pip install pre-commit && \
-    rm -rf "$GOSRC"
+    python3 -m pip install pre-commit
 
 # Install cni config
-#RUN make install.cni
-RUN mkdir -p /etc/cni/net.d/
 COPY cni/87-podman-bridge.conflist /etc/cni/net.d/87-podman-bridge.conflist
-
 # Make sure we have some policy for pulling images
-RUN mkdir -p /etc/containers
 COPY test/policy.json /etc/containers/policy.json
 COPY test/redhat_sigstore.yaml /etc/containers/registries.d/registry.access.redhat.com.yaml
 
+WORKDIR "$GOSRC"
+RUN make install.tools && \
+    cd / && \
+    rm -rf "$GOSRC" && \
+    mkdir -p "$GOSRC"
 VOLUME ["/usr/src/libpod"]
+# This entrypoint will synchronize the above volume ($SRCPATH) to $GOSRC before
+# executing make.  This ensures the original source remains prestine and is never
+# modified by any lint/validation checks.
 ENTRYPOINT ["/usr/local/bin/entrypoint.sh"]
diff --git a/contrib/gate/README.md b/contrib/gate/README.md
index 709e6035f..fe1205dc5 100644
--- a/contrib/gate/README.md
+++ b/contrib/gate/README.md
@@ -1,4 +1,6 @@
 ![PODMAN logo](../../logo/podman-logo-source.svg)
 
-A standard container image for `gofmt` and lint-checking the libpod
-repository.  The [contributors guide contains the documentation for usage.](https://github.com/containers/libpod/blob/master/CONTRIBUTING.md#go-format-and-lint)
+A standard container image for lint-checking and validating changes to the libpod
+repository.  The
+[contributors guide contains the documentation for usage.](https://github.com/containers/libpod/blob/master/CONTRIBUTING.md#go-format-and-lint).  Note that this container image is also utilized
+in automation, see the file [.cirrus.yml](.cirrus.yml)
diff --git a/contrib/gate/entrypoint.sh b/contrib/gate/entrypoint.sh
index 0189cf7c5..ab6528e00 100755
--- a/contrib/gate/entrypoint.sh
+++ b/contrib/gate/entrypoint.sh
@@ -1,15 +1,23 @@
 #!/bin/bash
 
-[[ -n "$SRCPATH" ]] || \
-    ( echo "ERROR: \$SRCPATH must be non-empty" && exit 1 )
-[[ -n "$GOSRC" ]] || \
-    ( echo "ERROR: \$GOSRC must be non-empty" && exit 2 )
+set -e
+
+die() {
+    echo "${2:-FATAL ERROR (but no message given!)} (gate container entrypoint)"
+    exit ${1:-1}
+}
+
+[[ -n "$SRCPATH" ]] || die 1 "ERROR: \$SRCPATH must be non-empty"
+[[ -n "$GOPATH" ]] || die 2 "ERROR: \$GOPATH must be non-empty"
+[[ -n "$GOSRC" ]] || die 3 "ERROR: \$GOSRC must be non-empty"
 [[ -r "${SRCPATH}/contrib/gate/Dockerfile" ]] || \
-    ( echo "ERROR: Expecting libpod repository root at $SRCPATH" && exit 3 )
+    die 4 "ERROR: Expecting libpod repository root at $SRCPATH"
 
 # Working from a copy avoids needing to perturb the actual source files
-mkdir -p "$GOSRC"
+# if/when developers use gate container for local testing
+echo "Copying $SRCPATH to $GOSRC"
+mkdir -vp "$GOSRC"
 /usr/bin/rsync --recursive --links --quiet --safe-links \
                --perms --times --delete "${SRCPATH}/" "${GOSRC}/"
 cd "$GOSRC"
-make "$@"
+exec make "$@"
diff --git a/hack/get_release_info.sh b/hack/get_release_info.sh
index c2be6a270..c1c694a44 100755
--- a/hack/get_release_info.sh
+++ b/hack/get_release_info.sh
@@ -6,8 +6,7 @@
 
 set -euo pipefail
 
-DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" >/dev/null 2>&1 && pwd )"
-cd "${GOSRC:-${DIR}/../}"
+cd "${GOSRC:-$(dirname $0)/../}"
 
 valid_args() {
     REGEX='^\s+[[:upper:]]+\*[)]'
diff --git a/hack/install_golangci.sh b/hack/install_golangci.sh
index 430685a71..6ef8ce823 100755
--- a/hack/install_golangci.sh
+++ b/hack/install_golangci.sh
@@ -1,17 +1,17 @@
 #!/bin/bash
 
-if [ -z "$VERSION" ]; then
-	echo \$VERSION is empty
-	exit 1
-fi
+set -e
 
-if [ -z "$GOBIN" ]; then
-	echo \$GOBIN is empty
-	exit 1
-fi
+die() { echo "${1:-No error message given} (from $(basename $0))"; exit 1; }
+
+[ -n "$VERSION" ] || die "\$VERSION is empty or undefined"
+[ -n "$GOBIN" ] || die "\$GOBIN is empty or undefined"
 
-$GOBIN/golangci-lint --version | grep $VERSION
-if [ $?  -ne 0 ]; then
-	set -e
-	curl -sfL https://install.goreleaser.com/github.com/golangci/golangci-lint.sh | sh -s -- -b $GOBIN v$VERSION
+BIN="$GOBIN/golangci-lint"
+if [ ! -x "$BIN" ]; then
+    echo "Installing golangci-lint v$VERSION into $GOBIN"
+    curl -sfL https://install.goreleaser.com/github.com/golangci/golangci-lint.sh | sh -s -- -b $GOBIN v$VERSION
+else
+    # Prints it's own file name as part of --verison output
+    echo "Using existing $(dirname $BIN)/$($BIN --version)"
 fi