Use buildah instead of docker, support multi-arch builds by maya-r · Pull Request #115 · kubevirt/hostpath-provisioner

maya-r · 2022-07-06T07:34:34Z

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
hostpath-provisioner part of #98, we also need to change make push in project-infra to build arm64 binaries.

Special notes for your reviewer:

Release note:

Use buildah and podman, enable building multi-arch manifests

This passes them to all subprocesses, but they can still be overridden due to the ?= construct. Signed-off-by: Maya Rashish <mrashish@redhat.com>

Avoid failure when the binary is built with Fedora 36 and thus requires newer glibc symbols than the container. Signed-off-by: Maya Rashish <mrashish@redhat.com>

Now we can run: make clean && \ GOARCH=arm64 make manifest && GOARCH=amd64 make manifest && \ make manifest-push And spit out a manifest for both arm64 and amd64, in the same image. Caveats: - We have a special 'manifest-clean' target, as we can add arbitrarily many images to a manifest and don't want the old ones. Delete old image in case a regular non-manifest image exists by the same name, too. - The push and image/manifest creation are split, so we can run the image creation for more than one architecture and push the combined manifest including both. - We keep `make push` behaving the same to avoid breaking CI. - Full DOCKER_REPO name is used, as podman-like tools have odd behavior with short names. Signed-off-by: Maya Rashish <mrashish@redhat.com>

awels · 2022-07-06T16:47:18Z

hack/sanity.sh

+buildah build -t ${DOCKER_REPO}/sanity:test -f ./sanity/Dockerfile .
 # Need privileged so we can bind mount inside container, and hostpath capacity cannot change, so skipping that test
-docker run --privileged ${DOCKER_REPO}/sanity:test -ginkgo.noColor -ginkgo.skip="should fail when requesting to create a volume with already existing name and different capacity"
+podman run --privileged ${DOCKER_REPO}/sanity:test -ginkgo.noColor -ginkgo.skip="should fail when requesting to create a volume with already existing name and different capacity"


Looks like podman is not installed in this container, this is why the test is failing here.

quay.io/kubevirtci/golang is the container that is running this test. Might have to install podman on it during this run.

After a failed attempt to make the CI image include podman (the kubevirtci kind provider would pick it up but then fail to come up, so it immediately broke some CI lanes), this PR now supports both docker & podman, so CI is now passing with no change to project-infra.

maya-r · 2022-07-17T07:28:25Z

/retest

Signed-off-by: Maya Rashish <mrashish@redhat.com>

maya-r · 2022-08-02T09:26:05Z

ping @awels

awels

Looks good just have a concern with tls-verify=false being added when pushing.

awels · 2022-08-03T13:09:32Z

cluster-sync/sync.sh


 registry=${IMAGE_REGISTRY:-localhost:$(_port registry)}
-DOCKER_REPO=${registry} make push
+DOCKER_REPO=${registry} BUILDAH_PUSH_FLAGS="--tls-verify=false" make manifest manifest-push


I understand why you added the tls-verify=false, because we can't really verify the identity of the registry:5000 in the kubevirtci cluster. Can we only add this flag if we are in that particular situation. I would really like to verify the identity when I am pushing an actual release to quay.io for instance.

awels · 2022-08-03T13:09:41Z

hack/k8s-e2e.sh

 registry=${IMAGE_REGISTRY:-localhost:$(_port registry)}
 echo "registry: ${registry}"
-DOCKER_REPO=${registry} make push
+DOCKER_REPO=${registry} BUILDAH_PUSH_FLAGS="--tls-verify=false" make manifest manifest-push


Add a message about this, too Signed-off-by: Maya Rashish <mrashish@redhat.com>

awels · 2022-08-03T15:11:59Z

/lgtm
/approve

kubevirt-bot · 2022-08-03T15:12:07Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: awels

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

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~OWNERS~~ [awels]

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

awels · 2023-06-16T19:43:46Z

/cherrypick release-v0.13

kubevirt-bot · 2023-06-16T19:44:24Z

@awels: new pull request created: #215

Details

In response to this:

/cherrypick release-v0.13

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

awels · 2023-06-16T20:25:05Z

/cherrypick release-v0.12

kubevirt-bot · 2023-06-16T20:25:40Z

@awels: #115 failed to apply on top of branch "release-v0.12":

Applying: Instead of passing environment variables, export them in Makefile
Using index info to reconstruct a base tree...
M	Makefile
Falling back to patching base and 3-way merge...
Auto-merging Makefile
CONFLICT (content): Merge conflict in Makefile
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 Instead of passing environment variables, export them in Makefile
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

Details

In response to this:

/cherrypick release-v0.12

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

* Instead of passing environment variables, export them in Makefile This passes them to all subprocesses, but they can still be overridden due to the ?= construct. Signed-off-by: Maya Rashish <mrashish@redhat.com> * Build sanity.test statically Avoid failure when the binary is built with Fedora 36 and thus requires newer glibc symbols than the container. Signed-off-by: Maya Rashish <mrashish@redhat.com> * Switch to buildah & podman, enable multi-arch builds Now we can run: make clean && \ GOARCH=arm64 make manifest && GOARCH=amd64 make manifest && \ make manifest-push And spit out a manifest for both arm64 and amd64, in the same image. Caveats: - We have a special 'manifest-clean' target, as we can add arbitrarily many images to a manifest and don't want the old ones. Delete old image in case a regular non-manifest image exists by the same name, too. - The push and image/manifest creation are split, so we can run the image creation for more than one architecture and push the combined manifest including both. - We keep `make push` behaving the same to avoid breaking CI. - Full DOCKER_REPO name is used, as podman-like tools have odd behavior with short names. Signed-off-by: Maya Rashish <mrashish@redhat.com> * Tolerate docker instead of podman Signed-off-by: Maya Rashish <mrashish@redhat.com> * Only add --tls-verify=false if the registry matches localhost* Add a message about this, too Signed-off-by: Maya Rashish <mrashish@redhat.com>

* Allow dynamic linking of binaries (#205) * Allow dynamic linking of binaries The release was creating statically linked binaries and there is no particular reason for this. Doing dynamically linked binaries would decrase the container size. Signed-off-by: Alexander Wels <awels@redhat.com> * Updated fedora to 37 so it has a recent enough glibc Added glibc to the Dockerfile so the binaries can run Fixed failing functional test that was not calculating the size properly from the `df -Bk` command. It was calculating from 1000 instead of 1024 and the size was wrong because of it. Signed-off-by: Alexander Wels <awels@redhat.com> --------- Signed-off-by: Alexander Wels <awels@redhat.com> * Use buildah instead of docker, support multi-arch builds (#115) * Instead of passing environment variables, export them in Makefile This passes them to all subprocesses, but they can still be overridden due to the ?= construct. Signed-off-by: Maya Rashish <mrashish@redhat.com> * Build sanity.test statically Avoid failure when the binary is built with Fedora 36 and thus requires newer glibc symbols than the container. Signed-off-by: Maya Rashish <mrashish@redhat.com> * Switch to buildah & podman, enable multi-arch builds Now we can run: make clean && \ GOARCH=arm64 make manifest && GOARCH=amd64 make manifest && \ make manifest-push And spit out a manifest for both arm64 and amd64, in the same image. Caveats: - We have a special 'manifest-clean' target, as we can add arbitrarily many images to a manifest and don't want the old ones. Delete old image in case a regular non-manifest image exists by the same name, too. - The push and image/manifest creation are split, so we can run the image creation for more than one architecture and push the combined manifest including both. - We keep `make push` behaving the same to avoid breaking CI. - Full DOCKER_REPO name is used, as podman-like tools have odd behavior with short names. Signed-off-by: Maya Rashish <mrashish@redhat.com> * Tolerate docker instead of podman Signed-off-by: Maya Rashish <mrashish@redhat.com> * Only add --tls-verify=false if the registry matches localhost* Add a message about this, too Signed-off-by: Maya Rashish <mrashish@redhat.com> * Add go mod vendor Signed-off-by: Alexander Wels <awels@redhat.com> --------- Signed-off-by: Alexander Wels <awels@redhat.com> Signed-off-by: Maya Rashish <mrashish@redhat.com> Co-authored-by: Maya Rashish <mrashish@redhat.com>

maya-r added 3 commits July 6, 2022 00:49

Instead of passing environment variables, export them in Makefile

cea2906

This passes them to all subprocesses, but they can still be overridden due to the ?= construct. Signed-off-by: Maya Rashish <mrashish@redhat.com>

Build sanity.test statically

bd7116f

Avoid failure when the binary is built with Fedora 36 and thus requires newer glibc symbols than the container. Signed-off-by: Maya Rashish <mrashish@redhat.com>

kubevirt-bot added release-note Denotes a PR that will be considered when it comes time to generate release notes. dco-signoff: yes Indicates the PR's author has DCO signed all their commits. size/M labels Jul 6, 2022

kubevirt-bot requested review from aglitke and awels July 6, 2022 07:34

awels reviewed Jul 6, 2022

View reviewed changes

maya-r mentioned this pull request Jul 8, 2022

Add podman to bootstrap images kubevirt/project-infra#2196

Merged

brianmcarey mentioned this pull request Jul 20, 2022

build multiarch bootstrap image kubevirt/project-infra#1149

Merged

Tolerate docker instead of podman

0b585ce

Signed-off-by: Maya Rashish <mrashish@redhat.com>

awels reviewed Aug 3, 2022

View reviewed changes

Only add --tls-verify=false if the registry matches localhost*

22b1206

Add a message about this, too Signed-off-by: Maya Rashish <mrashish@redhat.com>

kubevirt-bot assigned awels Aug 3, 2022

kubevirt-bot added the lgtm Indicates that a PR is ready to be merged. label Aug 3, 2022

kubevirt-bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 3, 2022

kubevirt-bot merged commit f24ea1f into kubevirt:main Aug 3, 2022

kubevirt-bot mentioned this pull request Jun 16, 2023

[release-v0.13] Use buildah instead of docker, support multi-arch builds #215

Merged

Conversation

maya-r commented Jul 6, 2022

Uh oh!

awels Jul 6, 2022

Choose a reason for hiding this comment

Uh oh!

awels Jul 6, 2022

Choose a reason for hiding this comment

Uh oh!

maya-r Jul 20, 2022

Choose a reason for hiding this comment

Uh oh!

maya-r commented Jul 17, 2022

Uh oh!

maya-r commented Aug 2, 2022

Uh oh!

awels left a comment

Choose a reason for hiding this comment

Uh oh!

awels Aug 3, 2022

Choose a reason for hiding this comment

Uh oh!

maya-r Aug 3, 2022

Choose a reason for hiding this comment

Uh oh!

awels Aug 3, 2022

Choose a reason for hiding this comment

Uh oh!

maya-r Aug 3, 2022

Choose a reason for hiding this comment

Uh oh!

awels commented Aug 3, 2022

Uh oh!

kubevirt-bot commented Aug 3, 2022

Uh oh!

awels commented Jun 16, 2023

Uh oh!

kubevirt-bot commented Jun 16, 2023

Uh oh!

awels commented Jun 16, 2023

Uh oh!

kubevirt-bot commented Jun 16, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants