Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Tagging of multiarch images on gcr #82

Merged
merged 7 commits into from
May 21, 2020
Merged

Tagging of multiarch images on gcr #82

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
a5c31f9
Added push-multiarch target
vitt-bagal May 12, 2020
1363600
cleanup cloudbuild
vitt-bagal May 12, 2020
f71ccea
removed push-multiarch target
vitt-bagal May 14, 2020
46a0c88
Added push-multiarch target
vitt-bagal May 14, 2020
bb941d8
cleanup cloudbuild
vitt-bagal May 14, 2020
d5da579
removed extra env variables
vitt-bagal May 15, 2020
3e9e449
removed extra variables
vitt-bagal May 15, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 39 additions & 0 deletions Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,46 @@
# See the License for the specific language governing permissions and
# limitations under the License.

.PHONY: push-multiarch-% push-multiarch

CMDS=csi-node-driver-registrar
all: build

include release-tools/build.make

# This target builds multiarch images using Moby BuildKit builder toolkit.
# Docker Buildx is included in Docker 19.03 and needs DOCKER_CLI_EXPERIMENTAL enabled to run corresponding commands.
# Currently amd, s390x and Windows manifest is pushed for canary, release branch and released tags.
# Images generated from Prow build are pushed to staging area on gcr
push-multiarch-%:
make BUILD_PLATFORMS="windows amd64 .exe"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm still not sure what this make invocation is needed for.

Ah, it's because Dockerfile.Windows doesn't build its binary. We probably should fix this and ideally remove the special Dockerfile for Windows entirely. But this can probably wait until later. We have to sort it out before including it in csi-release-tools.

@ddebroy: is there a chance to use gcr.io/distroless/static:latest as base for a Windows image?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In order to use buildx to build windows image on a linux machine, we cannot include any RUN command in the Dockerfile, so I think this limitation means we cannot build binary as a step in Dockerfile.

set -ex; \
gcloud auth configure-docker; \
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is going to require gcloud? Can we put this in the cloudbuild.yml instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Earlier gcloud command was a part of cloudbuild only but later we have moved it to make target in order to keep cloudbuild simple and minimal.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be preferable to keep gcloud command outside of this otherwise anybody that wants to build a multiarch image is going to require gcloud to be installed.

If you still want put it in the makefile to reduce the commands in cloudbuild, then I would maybe wrap "push-multiarch" with a "cloudbuild-push-multiarch"

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with @msau42 that this belongs into cloudbuild.yml. push-multiarch-% should be independent of the registry. cloudbuild.yml is where we prepare for pushing to gcr.

DOCKER_CLI_EXPERIMENTAL=enabled; \
export DOCKER_CLI_EXPERIMENTAL; \
docker buildx create --use --name multiarchimage-buildertest; \
pushMultiArch () { \
tag=$$1; \
docker buildx build --push -t $(IMAGE_NAME):amd64-linux-$$tag --platform=linux/amd64 -f $(shell if [ -e ./cmd/$*/Dockerfile.multiarch ]; then echo ./cmd/$*/Dockerfile.multiarch; else echo Dockerfile.multiarch; fi) --label revision=$(REV) .; \
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps create a local variable dockerfile=$(shell if [ -e ./cmd/$*/Dockerfile.multiarch ]; then echo ./cmd/$*/Dockerfile.multiarch; else echo Dockerfile.multiarch; fi) above and use it here and below?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can use this once windows is also a part of Dockerfile.multiarch. otherwise we have to set an extra variable for windows dockerfile too.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any way we can make the list of linux platforms as a variable instead of hard coding the command for each platform?

Unfortunately I don't think there's much we can do to generalize windows atm.

Copy link
Contributor Author

@vitt-bagal vitt-bagal May 21, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently with the help of this PR we are just testing that whether push-multiarch (keeping cloudbuild minimal) can be used to correctly tag multiarch images.
Once all these changes are working fine as expected then we can modify docker buildx command as docker buildx build --push -t $(IMAGE_NAME):$$tag --platform=linux/amd64,linux/s390x -f ... which will have single docker buildx command for linux (with multiaple arch)

docker buildx build --push -t $(IMAGE_NAME):s390x-linux-$$tag --platform=linux/s390x -f $(shell if [ -e ./cmd/$*/Dockerfile.multiarch ]; then echo ./cmd/$*/Dockerfile.multiarch; else echo Dockerfile.multiarch; fi) --label revision=$(REV) .; \
docker buildx build --push -t $(IMAGE_NAME):amd64-windows-$$tag --platform=windows -f $(shell if [ -e ./cmd/$*/Dockerfile.Windows ]; then echo ./cmd/$*/Dockerfile.Windows; else echo Dockerfile.Windows; fi) --label revision=$(REV) .; \
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have you tried doing this with Dockerfile.multiarch instead of Dockerfile.Windows?

You must extract the target OS from TARGETPLATFORM instead of hard-coding linux and the .exe suffix may need special handling.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have you tried doing this with Dockerfile.multiarch instead of Dockerfile.Windows?

I haven't tried this. We can't test windows images at our end. May be @jingxu97 can try this out.

Ideally instead of writing different docker buildx build command for each arch build and then creating manifest for them, single command with --push --platform=linux/amd64,linux/s390x should work. However I feel we can try this out later, once current changes are working as expected.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since Dockerfile.multiarch contains a 'RUN' command, we cannot use it to build windows image on a linux machine with buildx.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So we can build Windows binaries on the Linux host, but not inside a Docker container? Or does building for Windows need a Windows host?

docker manifest create --amend $(IMAGE_NAME):$$tag $(IMAGE_NAME):amd64-linux-$$tag \
$(IMAGE_NAME):s390x-linux-$$tag \
$(IMAGE_NAME):amd64-windows-$$tag; \
docker manifest push -p $(IMAGE_NAME):$$tag; \
}; \
if [ $(PULL_BASE_REF) = "master" ]; then \
: "creating or overwriting canary image"; \
pushMultiArch canary; \
elif echo $(PULL_BASE_REF) | grep -q -e 'release-*' ; then \
: "creating or overwriting canary image for release branch"; \
release_canary_tag=$$(echo $(PULL_BASE_REF) | cut -f2 -d '-')-canary; \
pushMultiArch $$release_canary_tag; \
elif docker pull $(IMAGE_NAME):$(PULL_BASE_REF) 2>&1 | tee /dev/stderr | grep -q "manifest for $(IMAGE_NAME):$(PULL_BASE_REF) not found"; then \
: "creating release image"; \
pushMultiArch $(PULL_BASE_REF); \
else \
: "release image $(IMAGE_NAME):$(PULL_BASE_REF) already exists, skipping push"; \
fi; \

push-multiarch: $(CMDS:%=push-multiarch-%)
24 changes: 5 additions & 19 deletions cloudbuild.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,32 +8,18 @@ options:
substitution_option: ALLOW_LOOSE
steps:
- name: 'gcr.io/k8s-testimages/gcb-docker-gcloud:v20200421-a2bf5f8'
entrypoint: 'bash'
entrypoint: make
env:
- DOCKER_CLI_EXPERIMENTAL=enabled
- VERSION=$_GIT_TAG
- BASE_REF=$_PULL_BASE_REF
- PULL_BASE_REF=$_PULL_BASE_REF
- HOME=/root
args:
- '-c'
- |
export DOCKER_CLI_EXPERIMENTAL=enabled \
&& gcloud auth configure-docker \
&& docker buildx create --use --name multiarchimage-builder \
&& docker buildx build --push -t gcr.io/$_STAGING_PROJECT/csi-node-driver-registrar:amd64-linux-$_GIT_TAG --platform=linux/amd64 -f Dockerfile.multiarch . \
&& docker buildx build --push -t gcr.io/$_STAGING_PROJECT/csi-node-driver-registrar:s390x-linux-$_GIT_TAG --platform=linux/s390x -f Dockerfile.multiarch . \
&& make BUILD_PLATFORMS="windows amd64 .exe" \
&& docker buildx build --push -t gcr.io/$_STAGING_PROJECT/csi-node-driver-registrar:amd64-windows-$_GIT_TAG --platform=windows -f Dockerfile.Windows . \
&& docker manifest create --amend gcr.io/$_STAGING_PROJECT/csi-node-driver-registrar:$_GIT_TAG \
gcr.io/$_STAGING_PROJECT/csi-node-driver-registrar:amd64-linux-$_GIT_TAG \
gcr.io/$_STAGING_PROJECT/csi-node-driver-registrar:s390x-linux-$_GIT_TAG \
gcr.io/$_STAGING_PROJECT/csi-node-driver-registrar:amd64-windows-$_GIT_TAG \
&& docker manifest push -p gcr.io/$_STAGING_PROJECT/csi-node-driver-registrar:$_GIT_TAG
# Extract tag-n-hash value from _GIT_TAG(form vYYYYMMDD-tag-n-hash) for REV value
- push-multiarch REV=v$(echo $_GIT_TAG | cut -f3- -d 'v') REGISTRY_NAME=gcr.io/k8s-staging-csi
substitutions:
# _GIT_TAG will be filled with a git-based tag for the image, of the form vYYYYMMDD-hash, and
# can be used as a substitution
_GIT_TAG: '12345'
# _PULL_BASE_REF will contain the ref that was pushed to to trigger this build -
# _PULL_BASE_REF will contain the ref that was pushed to trigger this build -
# a branch like 'master' or 'release-0.2', or a tag like 'v0.2'.
_PULL_BASE_REF: 'master'
_STAGING_PROJECT: 'k8s-staging-csi'