From d0782e7839888d9eecdc97e8e885d5f787f5b8a7 Mon Sep 17 00:00:00 2001 From: Chris Evich Date: Fri, 31 Jan 2020 12:50:27 -0500 Subject: Cirrus: Fix gate image & false-positive exits A number of scripts relating to tooling used and the gate container image were not exiting upon errors as intended. Coupled with external service unavailability (i.e. downloading golangci-lint) was observed to cause difficult to debug failures. This change corrects the scripts inside/out of the gate container as well as fixes many golang related path consistency problems vs other CI jobs. After this change, all jobs use consistent path names reducing the number of special-case overrides needed. Lastly, I also made a documentation-pass, updating/correcting as needed, including documenting a likely local validation-failure mode, related to `$EPOCH_TEST_COMMIT`. This is dependent on the developers git environment, so documentation is the only possible "fix". Signed-off-by: Chris Evich --- .cirrus.yml | 26 ++++++++++++--------- .dockerignore | 3 +++ CONTRIBUTING.md | 58 ++++++++++++++++++++++++++++++++++++++-------- contrib/cirrus/lib.sh | 4 ++-- contrib/gate/Dockerfile | 29 +++++++++++++---------- contrib/gate/README.md | 6 +++-- contrib/gate/entrypoint.sh | 22 ++++++++++++------ hack/get_release_info.sh | 3 +-- hack/install_golangci.sh | 24 +++++++++---------- 9 files changed, 117 insertions(+), 58 deletions(-) create mode 100644 .dockerignore diff --git a/.cirrus.yml b/.cirrus.yml index 2106ac96d..f1fba220e 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 @@ -85,17 +86,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 @@ -133,9 +135,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' @@ -217,13 +219,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 @@ -749,12 +752,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 -- cgit v1.2.3-54-g00ecf