Adds image automated build #189

claudiubelu · 2020-04-22T15:35:13Z

In order for the secret store CSI image to be automatically built and published to the staging registry (from which it will be promoted), the cloudbuild.yaml file has been added.

The file was added in conformance with [1].

Adds the docker folder, which contains several items:

cloudbuild.yaml
BASEIMAGE: the base image to use when building an image for a certain os/arch
(or os/arch/version for Windows).
BASEIMAGE_CORE: for Windows images, from which image to copy necessary files (DLLs).
Makefile
build.sh: script that can build, push, and create the manifest list.
Dockerfile: needed to build the Linux docker image. Can now accept the a
BASEIMAGE arg, which can be useful when building multi-arch images.
windows.Dockerfile: needed to build the Windows docker image. Can now accept
BASEIMAGE and BASEIMAGE_CORE args, which can be useful when building images for multiple
Windows versions.

The image building process will be triggered when changes to the files in the docker changes
are made (for example, you bump the image version, so a new image is built).

[1] https://github.com/kubernetes/test-infra/blob/master/config/jobs/image-pushing/README.md

Adds support for multiple Windows versions.

The image will be created with the name gcr.io/k8s-staging-csi-secrets-store/driver:VERSION.

Bumps the driver's image version.

Related PR: kubernetes/test-infra#17335

claudiubelu · 2020-04-22T15:35:28Z

/cc @aramase

PS: The reason for the build script, is in case you will want to build multi-arch images in the future. See here an example: https://github.com/kubernetes/kubernetes/blob/master/test/images/image-util.sh#L131

aramase · 2020-04-22T16:20:24Z

@claudiubelu The e2e tests are failing because they're looking for Dockerfile in the repo root. Once you update that to docker/Dockerfile in the Makefile - https://github.com/kubernetes-sigs/secrets-store-csi-driver/blob/master/Makefile#L53-L56 and https://github.com/kubernetes-sigs/secrets-store-csi-driver/blob/master/Makefile#L103-L104 the tests should pass.

claudiubelu · 2020-04-22T19:04:49Z

I've tried building this image with the added script. The image I've built is at claudiubelu/driver:v0.0.10 (it's a manifest list with all the os/arch/versions described in the BASEIMAGE file).

claudiubelu · 2020-05-19T12:44:12Z

FWIW, I've updated the way the image is built, so the Windows nodes are not required at all: https://paste.ubuntu.com/p/phqKvJDDbt/

From what I've tried, containers using the image claudiubelu/driver:v0.0.10 have started on Windows Server 1809, 1903, and 1909. If someone could also check them, that would also be great. The manifest list contains os.version, as you can see: https://paste.ubuntu.com/p/MJjDW4Q3WB/

The more difficult part of this was actually tagging the manifest list with the proper os.version platform entries for the Windows images, which are mandatory for Windows. Without those, a Windows node will try to pull the first image that fits the platform and arch from the manifest list, even if it won't match the OS version, and thus fail to start the container. docker manifest annotate doesn't allow you to set the os.version, at least, in the current version, so I had to use manifest-tool to create and push the manifest list, which supports adding the os.version.

I've also updated the image in cloudbuild.yaml to a newer one that also supports buildx.

aramase · 2020-05-20T00:30:57Z

@claudiubelu We recently merged the PR to use buildx for building multi-arch images - #205.

so I had to use manifest-tool to create and push the manifest list, which supports adding the os.version.

This is great!

aramase · 2020-05-20T00:31:46Z

Makefile

@@ -51,9 +52,15 @@ build: setup
 build-windows: setup
 	CGO_ENABLED=0 GOOS=windows go build -a -ldflags ${LDFLAGS} -o _output/secrets-store-csi.exe ./cmd/secrets-store-csi-driver
 image:
-	docker buildx build --no-cache --build-arg LDFLAGS=${LDFLAGS} -t $(IMAGE_TAG) -f Dockerfile --platform="linux/amd64" --output "type=docker,push=false" .
+	docker buildx build --no-cache --build-arg LDFLAGS=${LDFLAGS} -t $(IMAGE_TAG) -f docker/Dockerfile --platform="linux/amd64" --output "type=docker,push=false" .


there is no docker/ dir?

Indeed, I moved some things on rebase (there was a merge conflict). Because of some recent changes, I had to rewrite some bits of the building process, and I moved everything in the root directory.

I still prefer having the docker folder. The reason is because we're configuring the Image Promoter to build the image when changes to files in the docker/.* are being made (for example, you'd trigger the builder on an image version bump). Having everything on the root directory would mean that the builder would be triggered on every PR, which is unnecessary.

@claudiubelu I'm just wondering if we can make it more architecture natural so that we can build other platforms as well like arm, ppc64le and s390x as well?

I did, in the build.sh below.

As I mentioned before, I've made a separate docker/Makefile which is going to be used by the Image Builder / Promoter. This target is still being used for testing, and is called in this Makefile. This only builds the linux/amd64 image, which is sufficient for testing, while mine builds all images, which takes a lot more time, especially due to the Windows images.

claudiubelu · 2020-05-20T09:55:47Z

@claudiubelu We recently merged the PR to use buildx for building multi-arch images - #205.

so I had to use manifest-tool to create and push the manifest list, which supports adding the os.version.

This is great!

Hmm, from what I see, there is still only linux/amd64 and windows/amd64. There is no arm, amr64, etc. archs added.

mkumatag · 2020-05-21T15:00:09Z

Makefile

@@ -51,9 +52,15 @@ build: setup
 build-windows: setup
 	CGO_ENABLED=0 GOOS=windows go build -a -ldflags ${LDFLAGS} -o _output/secrets-store-csi.exe ./cmd/secrets-store-csi-driver
 image:
-	docker buildx build --no-cache --build-arg LDFLAGS=${LDFLAGS} -t $(IMAGE_TAG) -f Dockerfile --platform="linux/amd64" --output "type=docker,push=false" .
+	docker buildx build --no-cache --build-arg LDFLAGS=${LDFLAGS} -t $(IMAGE_TAG) -f docker/Dockerfile --platform="linux/amd64" --output "type=docker,push=false" .


@claudiubelu I'm just wondering if we can make it more architecture natural so that we can build other platforms as well like arm, ppc64le and s390x as well?

docker/windows.Dockerfile

aramase · 2020-05-28T17:13:11Z

docker/Makefile

+
+REGISTRY?=docker.io/deislabs
+IMAGE_NAME=driver
+IMAGE_VERSION?=v0.0.10


Could you update this to v0.0.11 as that's the last release we cut?

aramase

@claudiubelu The PR looks great! 🎉

Just one small nit to update the IMAGE_VERSION to the latest release version.

In order for the secret store CSI image to be automatically built and published to the staging registry (from which it will be promoted), the cloudbuild.yaml file has been added. The file was added in conformance with [1]. Adds the docker folder, which contains several items: - cloudbuild.yaml - BASEIMAGE: the base image to use when building an image for a certain os/arch (or os/arch/version for Windows). - BASEIMAGE_CORE: for Windows images, from which image to copy necessary files (DLLs). - Makefile - build.sh: script that can build, push, and create the manifest list. - Dockerfile: needed to build the Linux docker image. Can now accept the a BASEIMAGE arg, which can be useful when building multi-arch images. - windows.Dockerfile: needed to build the Windows docker image. Can now accept BASEIMAGE and BASEIMAGE_CORE args, which can be useful when building images for multiple Windows versions. The image building process will be triggered when changes to the files in the docker changes are made (for example, you bump the image version, so a new image is built). Adds support for multiple Windows versions. The image will be created with the name gcr.io/k8s-staging-csi-secrets-store/driver:VERSION. Bumps the driver's image version. [1] https://github.com/kubernetes/test-infra/blob/master/config/jobs/image-pushing/README.md

claudiubelu · 2020-06-02T13:11:55Z

Updated version

aramase

/lgtm

/hold
for @ritazh review

ritazh · 2020-06-02T21:12:54Z

/hold cancel
/lgtm

aramase · 2020-06-02T21:23:17Z

/approve

k8s-ci-robot · 2020-06-02T21:23:31Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: aramase, claudiubelu

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [aramase]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

k8s-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Apr 22, 2020

k8s-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Apr 22, 2020

k8s-ci-robot requested review from aramase and ritazh April 22, 2020 15:35

claudiubelu force-pushed the adds-image-promoter branch 2 times, most recently from 45a6bd6 to 2981794 Compare April 22, 2020 15:43

claudiubelu force-pushed the adds-image-promoter branch 2 times, most recently from 646062b to b6df6f7 Compare April 22, 2020 19:03

claudiubelu changed the title ~~WIP: Adds image automated build~~ Adds image automated build Apr 28, 2020

k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Apr 28, 2020

aramase mentioned this pull request May 4, 2020

Move images to using gcr repo #200

Closed

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 17, 2020

claudiubelu force-pushed the adds-image-promoter branch from b6df6f7 to 7fa01cf Compare May 19, 2020 12:36

claudiubelu force-pushed the adds-image-promoter branch from 7fa01cf to 0be100b Compare May 19, 2020 14:24

k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 19, 2020

aramase reviewed May 20, 2020

View reviewed changes

claudiubelu force-pushed the adds-image-promoter branch 2 times, most recently from a64b2d7 to 269fe38 Compare May 20, 2020 09:54

aramase mentioned this pull request May 20, 2020

adds k8s-staging-csi-secrets-store image pushing prow config kubernetes/test-infra#17335

Merged

claudiubelu mentioned this pull request May 21, 2020

[WIP] Support for multi-arch kubernetes-csi/node-driver-registrar#63

Closed

mkumatag suggested changes May 21, 2020

View reviewed changes

jingxu97 reviewed May 21, 2020

View reviewed changes

docker/windows.Dockerfile Show resolved Hide resolved

claudiubelu force-pushed the adds-image-promoter branch from 269fe38 to 1afb7c6 Compare May 25, 2020 12:39

aramase reviewed May 28, 2020

View reviewed changes

claudiubelu force-pushed the adds-image-promoter branch from 1afb7c6 to a2c63e0 Compare June 2, 2020 13:13

aramase reviewed Jun 2, 2020

View reviewed changes

k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jun 2, 2020

k8s-ci-robot assigned aramase Jun 2, 2020

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jun 2, 2020

k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jun 2, 2020

k8s-ci-robot assigned ritazh Jun 2, 2020

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 2, 2020

k8s-ci-robot merged commit ff1dc3b into kubernetes-sigs:master Jun 2, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adds image automated build #189

Adds image automated build #189

claudiubelu commented Apr 22, 2020 •

edited

claudiubelu commented Apr 22, 2020 •

edited

aramase commented Apr 22, 2020

claudiubelu commented Apr 22, 2020

claudiubelu commented May 19, 2020 •

edited

aramase commented May 20, 2020

aramase May 20, 2020

claudiubelu May 20, 2020

mkumatag May 21, 2020

claudiubelu May 21, 2020

mkumatag May 21, 2020

claudiubelu commented May 20, 2020

mkumatag May 21, 2020

aramase May 28, 2020

aramase left a comment

claudiubelu commented Jun 2, 2020

aramase left a comment

ritazh commented Jun 2, 2020

aramase commented Jun 2, 2020

k8s-ci-robot commented Jun 2, 2020

Adds image automated build #189

Adds image automated build #189

Conversation

claudiubelu commented Apr 22, 2020 • edited

claudiubelu commented Apr 22, 2020 • edited

aramase commented Apr 22, 2020

claudiubelu commented Apr 22, 2020

claudiubelu commented May 19, 2020 • edited

aramase commented May 20, 2020

aramase May 20, 2020

Choose a reason for hiding this comment

claudiubelu May 20, 2020

Choose a reason for hiding this comment

mkumatag May 21, 2020

Choose a reason for hiding this comment

claudiubelu May 21, 2020

Choose a reason for hiding this comment

mkumatag May 21, 2020

Choose a reason for hiding this comment

claudiubelu commented May 20, 2020

mkumatag May 21, 2020

Choose a reason for hiding this comment

aramase May 28, 2020

Choose a reason for hiding this comment

aramase left a comment

Choose a reason for hiding this comment

claudiubelu commented Jun 2, 2020

aramase left a comment

Choose a reason for hiding this comment

ritazh commented Jun 2, 2020

aramase commented Jun 2, 2020

k8s-ci-robot commented Jun 2, 2020

claudiubelu commented Apr 22, 2020 •

edited

claudiubelu commented Apr 22, 2020 •

edited

claudiubelu commented May 19, 2020 •

edited