From f5b37b1c769824c514ad42c86775fc2e21ded002 Mon Sep 17 00:00:00 2001 From: Steve Nay <265958+snay2@users.noreply.github.com> Date: Wed, 23 Feb 2022 11:55:55 -0600 Subject: [PATCH 1/4] Add support for docker buildx on linux builds This ensures that the platform in the image manifest matches the platform of the binaries. --- Dockerfile | 7 ++++--- Makefile | 2 +- scripts/build-docker-images | 29 ++++++++++++++++++++--------- scripts/push-docker-images | 4 ++++ 4 files changed, 29 insertions(+), 13 deletions(-) diff --git a/Dockerfile b/Dockerfile index 8d6a3e44..8b13b6d1 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,4 +1,4 @@ -FROM golang:1.16 as builder +FROM --platform=$BUILDPLATFORM golang:1.16 as builder ## GOLANG env ARG GOPROXY="https://proxy.golang.org|direct" @@ -11,8 +11,9 @@ COPY go.sum . RUN go mod download ARG CGO_ENABLED=0 -ARG GOOS=linux -ARG GOARCH=amd64 +ARG TARGETOS TARGETARCH +ARG GOOS=$TARGETOS +ARG GOARCH=$TARGETARCH # Build COPY . . diff --git a/Makefile b/Makefile index eaf12c42..738c9cf9 100644 --- a/Makefile +++ b/Makefile @@ -12,7 +12,7 @@ GOARCH ?= amd64 GOPROXY ?= "https://proxy.golang.org,direct" MAKEFILE_PATH = $(dir $(realpath -s $(firstword $(MAKEFILE_LIST)))) BUILD_DIR_PATH = ${MAKEFILE_PATH}/build -SUPPORTED_PLATFORMS_LINUX ?= "linux/amd64,linux/arm64,linux/arm,darwin/amd64" +SUPPORTED_PLATFORMS_LINUX ?= "linux/amd64,linux/arm64" SUPPORTED_PLATFORMS_WINDOWS ?= "windows/amd64" BINARY_NAME ?= "node-termination-handler" diff --git a/scripts/build-docker-images b/scripts/build-docker-images index 74b5b718..95dc1418 100755 --- a/scripts/build-docker-images +++ b/scripts/build-docker-images @@ -47,6 +47,7 @@ while getopts "dp:r:v:" opt; do esac done + for os_arch in "${PLATFORMS[@]}"; do os=$(echo $os_arch | cut -d'/' -f1) arch=$(echo $os_arch | cut -d'/' -f2) @@ -56,13 +57,23 @@ for os_arch in "${PLATFORMS[@]}"; do dockerfile="$DOCKERFILE_PATH" if [[ $os = "windows" ]]; then dockerfile="${dockerfile}.windows" + docker build \ + --file "${dockerfile}" \ + --build-arg GOOS=${os} \ + --build-arg GOARCH=${arch} \ + --build-arg GOPROXY=${GOPROXY} \ + --tag ${img_tag} \ + ${REPO_ROOT_PATH} + else + # Launch a docker buildx instance and save its name so we can terminate it later + buildx_instance_name=$(docker buildx create --use) + docker buildx build \ + --load \ + --file "${dockerfile}" \ + --build-arg GOPROXY=${GOPROXY} \ + --tag ${img_tag} \ + --platform "${os_arch}" \ + ${REPO_ROOT_PATH} + docker buildx rm ${buildx_instance_name} fi - - docker build \ - --file "${dockerfile}" \ - --build-arg GOOS=${os} \ - --build-arg GOARCH=${arch} \ - --build-arg GOPROXY=${GOPROXY} \ - --tag ${img_tag} \ - ${REPO_ROOT_PATH} -done +done \ No newline at end of file diff --git a/scripts/push-docker-images b/scripts/push-docker-images index 730ae419..f0448128 100755 --- a/scripts/push-docker-images +++ b/scripts/push-docker-images @@ -109,6 +109,10 @@ if [[ $MANIFEST == "true" ]]; then echo "creating manifest for $updated_img" docker manifest create $IMAGE_REPO:$VERSION $updated_img --amend + # Theoretically, this will not be necessary if we move all our builds to docker buildx. + # (The Windows build is the only one not using it at the moment.) The manifest create --amend command + # should figure out the OS and architecture automatically if the container was built properly. + # However, our builds in the past required this explicit annotation, and it doesn't hurt to keep it for now. os_arch=$(echo ${updated_img//$IMAGE_REPO:$VERSION-/}) os=$(echo $os_arch | cut -d'-' -f1) arch=$(echo $os_arch | cut -d'-' -f2) From 0aab5ec34bb2f1471fde4c0860f28ed6684fa834 Mon Sep 17 00:00:00 2001 From: Steve Nay <265958+snay2@users.noreply.github.com> Date: Wed, 23 Feb 2022 13:48:30 -0600 Subject: [PATCH 2/4] Use docker buildx in the integration test suite too --- test/eks-cluster-test/provision-cluster | 4 ++-- test/k8s-local-cluster-test/run-test | 4 ++-- test/license-test/run-license-test.sh | 2 +- test/readme-test/run-readme-spellcheck | 2 +- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/test/eks-cluster-test/provision-cluster b/test/eks-cluster-test/provision-cluster index 18ce5749..82ca1aad 100755 --- a/test/eks-cluster-test/provision-cluster +++ b/test/eks-cluster-test/provision-cluster @@ -27,12 +27,12 @@ fi ## Build Docker images echo "🥑 Building the node-termination-handler docker image" -docker build $DOCKER_ARGS -t $DEFAULT_NODE_TERMINATION_HANDLER_DOCKER_IMG "$SCRIPTPATH/../../." +docker buildx build --load $DOCKER_ARGS -t $DEFAULT_NODE_TERMINATION_HANDLER_DOCKER_IMG "$SCRIPTPATH/../../." NODE_TERMINATION_HANDLER_DOCKER_IMG="$DEFAULT_NODE_TERMINATION_HANDLER_DOCKER_IMG" echo "👍 Built the node-termination-handler docker image" echo "🥑 Building the webhook-test-proxy docker image" -docker build $DOCKER_ARGS -t $DEFAULT_WEBHOOK_DOCKER_IMG "$SCRIPTPATH/../webhook-test-proxy/." +docker buildx build --load $DOCKER_ARGS -t $DEFAULT_WEBHOOK_DOCKER_IMG "$SCRIPTPATH/../webhook-test-proxy/." WEBHOOK_DOCKER_IMG="$DEFAULT_WEBHOOK_DOCKER_IMG" echo "👍 Built the webhook-test-proxy docker image" diff --git a/test/k8s-local-cluster-test/run-test b/test/k8s-local-cluster-test/run-test index e6b14165..cd975e40 100755 --- a/test/k8s-local-cluster-test/run-test +++ b/test/k8s-local-cluster-test/run-test @@ -196,7 +196,7 @@ CLUSTER_NAME=$(cat "$TMP_DIR/clustername") if [ -z "$NODE_TERMINATION_HANDLER_DOCKER_IMG" ]; then echo "🥑 Building the node-termination-handler docker image" - docker build $DOCKER_ARGS -t "$DEFAULT_NODE_TERMINATION_HANDLER_DOCKER_IMG" "$SCRIPTPATH/../../." + docker buildx build --load $DOCKER_ARGS -t "$DEFAULT_NODE_TERMINATION_HANDLER_DOCKER_IMG" "$SCRIPTPATH/../../." NODE_TERMINATION_HANDLER_DOCKER_IMG="$DEFAULT_NODE_TERMINATION_HANDLER_DOCKER_IMG" echo "👍 Built the node-termination-handler docker image" else @@ -208,7 +208,7 @@ NODE_TERMINATION_HANDLER_DOCKER_TAG=$(echo "$NODE_TERMINATION_HANDLER_DOCKER_IMG if [ -z "$WEBHOOK_DOCKER_IMG" ]; then echo "🥑 Building the webhook-test-proxy docker image" - docker build $DOCKER_ARGS -t "$DEFAULT_WEBHOOK_DOCKER_IMG" "$SCRIPTPATH/../webhook-test-proxy/." + docker buildx build --load $DOCKER_ARGS -t "$DEFAULT_WEBHOOK_DOCKER_IMG" "$SCRIPTPATH/../webhook-test-proxy/." WEBHOOK_DOCKER_IMG="$DEFAULT_WEBHOOK_DOCKER_IMG" echo "👍 Built the webhook-test-proxy docker image" else diff --git a/test/license-test/run-license-test.sh b/test/license-test/run-license-test.sh index d0d56675..7bf14335 100755 --- a/test/license-test/run-license-test.sh +++ b/test/license-test/run-license-test.sh @@ -10,6 +10,6 @@ LICENSE_TEST_TAG="nth-license-test" LICENSE_REPORT_FILE="$BUILD_PATH/license-report" SUPPORTED_PLATFORMS_LINUX="linux/amd64" make -s -f $SCRIPTPATH/../../Makefile build-binaries -docker build --build-arg=GOPROXY=direct -t $LICENSE_TEST_TAG $SCRIPTPATH/ +docker buildx build --load --build-arg=GOPROXY=direct -t $LICENSE_TEST_TAG $SCRIPTPATH/ docker run -i -e GITHUB_TOKEN --rm -v $SCRIPTPATH/:/test -v $BUILD_BIN/:/nth-bin $LICENSE_TEST_TAG golicense /test/license-config.hcl /nth-bin/$BINARY_NAME | tee $LICENSE_REPORT_FILE $SCRIPTPATH/check-licenses.sh $LICENSE_REPORT_FILE diff --git a/test/readme-test/run-readme-spellcheck b/test/readme-test/run-readme-spellcheck index 48ba1043..e915f293 100755 --- a/test/readme-test/run-readme-spellcheck +++ b/test/readme-test/run-readme-spellcheck @@ -9,6 +9,6 @@ function exit_and_fail() { } trap exit_and_fail INT ERR TERM -docker build -t misspell -f $SCRIPTPATH/spellcheck-Dockerfile $SCRIPTPATH/ +docker buildx build --load -t misspell -f $SCRIPTPATH/spellcheck-Dockerfile $SCRIPTPATH/ docker run -i --rm -v $SCRIPTPATH/../../:/app misspell /bin/bash -c 'find /app/ -type f -name "*.md" -not -path "build" | grep -v "/build/" | xargs misspell -error -debug' echo "✅ Markdown file spell check passed!" \ No newline at end of file From 80f7283f6063ad4695a1c2e7d147c2b5d4f2e855 Mon Sep 17 00:00:00 2001 From: Steve Nay <265958+snay2@users.noreply.github.com> Date: Wed, 23 Feb 2022 15:17:49 -0600 Subject: [PATCH 3/4] Delete the local manifest list before adding the new image to it. This ensures that we will keep any existing images in the remote manifest when we add the new image to that manifest list. --- scripts/push-docker-images | 2 ++ 1 file changed, 2 insertions(+) diff --git a/scripts/push-docker-images b/scripts/push-docker-images index f0448128..6080d061 100755 --- a/scripts/push-docker-images +++ b/scripts/push-docker-images @@ -62,6 +62,8 @@ if [[ $MANIFEST == "true" ]]; then fi cat <<< "$(jq '.+{"experimental":"enabled"}' $DOCKER_CLI_CONFIG)" > $DOCKER_CLI_CONFIG echo "Enabled experimental CLI features to execute docker manifest commands" + # Delete the local version of the manifest so we rely solely on the remote manifest + docker manifest rm $IMAGE_REPO:$VERSION || : manifest_exists=$(docker manifest inspect $IMAGE_REPO:$VERSION > /dev/null ; echo $?) if [[ manifest_exists -eq 0 ]]; then echo "manifest already exists" From acae5bed24dd3d9eccf58ca60b3acef1b7372cfb Mon Sep 17 00:00:00 2001 From: Steve Nay <265958+snay2@users.noreply.github.com> Date: Thu, 24 Feb 2022 17:35:34 -0600 Subject: [PATCH 4/4] Improve local build instructions and remove passive voice --- BUILD.md | 29 +++++++++++++++++++++-------- 1 file changed, 21 insertions(+), 8 deletions(-) diff --git a/BUILD.md b/BUILD.md index 2fb3a119..6912a496 100644 --- a/BUILD.md +++ b/BUILD.md @@ -5,21 +5,26 @@ Clone the repo: ``` git clone https://github.com/aws/aws-node-termination-handler.git ``` -Build the latest version of the docker image: +Build the latest version of the docker image for `linux/amd64`: ``` make docker-build ``` ### Multi-Target -By default a linux/amd64 image will be built. To build for a different target the build-arg `GOARCH` can be changed. +If you instead want to build for all support Linux architectures (`linux/amd64` and `linux/arm64`), you can run this make target: +``` +make build-docker-images +``` +Under the hood, this passes each architecture as the `--platform` argument to `docker buildx build`, like this: ``` -$ docker build --build-arg=GOARCH=amd64 -t ${USER}/aws-node-termination-handler-amd64:v1.0.0 . -$ docker build --build-arg=GOARCH=arm64 -t ${USER}/aws-node-termination-handler-arm64:v1.0.0 . +$ docker buildx create --use +$ docker buildx build --load --platform "linux/amd64" -t ${USER}/aws-node-termination-handler-amd64:v1.0.0 . +$ docker buildx build --load --platform "linux/arm64" -t ${USER}/aws-node-termination-handler-arm64:v1.0.0 . ``` -To push a multi-arch image, the helper tool [manifest-tool](https://github.com/estesp/manifest-tool) can be used. +To push a multi-arch image, you can use the helper tool [manifest-tool](https://github.com/estesp/manifest-tool). ``` $ cat << EOF > manifest.yaml @@ -39,16 +44,24 @@ EOF $ manifest-tool push from-spec manifest.yaml ``` +### Building for Windows + +You can build the Windows docker image with the following command: +``` +make build-docker-images-windows +``` +Currently, our `windows/amd64` builds use the older `docker build` system, not `docker buildx build` because it does not seem to be well supported. We hope to unify them in the future. + ### Go Module Proxy -By default, Go 1.13+ uses the proxy.golang.org proxy for go module downloads. This can be changed to a different go module proxy or revert back to pre-go 1.13 default which was "direct". `GOPROXY=direct` will pull from the VCS provider directly instead of going through a proxy at all. +By default, Go 1.13+ uses the proxy.golang.org proxy for go module downloads. You can change this to a different go module proxy or revert back to pre-go 1.13 default which was "direct". `GOPROXY=direct` will pull from the VCS provider directly instead of going through a proxy at all. ``` ## No Proxy -docker build --build-arg=GOPROXY=direct -t ${USER}/aws-node-termination-handler:v1.0.0 . +docker buildx build --load --build-arg=GOPROXY=direct -t ${USER}/aws-node-termination-handler:v1.0.0 . ## My Corp Proxy -docker build --build-arg=GOPROXY=go-proxy.mycorp.com -t ${USER}/aws-node-termination-handler:v1.0.0 . +docker buildx build --load --build-arg=GOPROXY=go-proxy.mycorp.com -t ${USER}/aws-node-termination-handler:v1.0.0 . ``` ### Kubernetes Object Files