build multiarch bootstrap image #1149
Conversation
kubevirt-bot
added
the
dco-signoff: yes
kubevirt-bot
added
the
needs-rebase
|
@zhlhahaha do we still need multi-arch bootstrap images? If possible it would be great if we can stay with the cross-build (and unit-tests arch-free). |
Yes, once we have multi-arch bootstrap images, we can run unit-tests and kind-e2e tests parallelly on the ARM64 server. The prow utility images have already supported multi-arch. kubernetes/test-infra#22362 Anyway, I will rebase this PR tomorrow. |
Ok, let me try to rephrase my question because I am still not sure we talk about the same things. I see that it would be possible for us to run arm64 workloads in CI, but do we really need it now? The unit-tests do not depend on the architecture (thanks to your change) and the whole build process, including container builds is architecture agnostic (no emulation or slow-down). Therefore I wonder if we want to increase our CI complexity. Especially since nested-virt does not work on arm64, we can not even adopt the kubevirtci cluster-spawning model for arm64. But maybe you think about other projects in the kubevirt org which can not cross-compile? |
I am working on making kubevirt officially support the ARM64 arch, and I know there is a lot of work to do. How do you think we can achieve this? I thought enabling arm64 workloads in CI is a part of it.
No, currently I am only working on kubevirt project and the CDI project. They both support crossbuild binary or image on x86 for arm64. |
I think we need a stable build process (we have it thanks to the cross compilatin), we need to be able to run the unit tests in CI and locally for the new architecture (we have that by allowing setting the go arch for the tests from outside and executing all unit tests for all architectures independent of the host platform), we need a stable release process (we have it, we already use it for the developer releases). So what is in my opinion missing: Running enough e2e tests to ensure that what we ship is working on arm. For that you are providing the clusters. They execute the code on the right architecture. There we need to enable more tests and potentially get the chance to run at least a subset of the e2e tests on PRs to guarantee a minimum protection against protections. I am not sure if arm64 base images and CI jobs as such help us there. |
It does not need, I will close is PR. |
|
/open |
kubevirt-bot
removed
the
needs-rebase
kubevirt-bot
added
the
needs-rebase
kubevirt-bot
removed
the
needs-rebase
|
/rehearse |
images/golang/Dockerfile
Outdated
| @@ -1,4 +1,4 @@ | |||
| FROM bootstrap | |||
| FROM bootstrap-amd64 | |||
There was a problem hiding this comment.
Don't we need the ARCH here? If we don't we would just create a regular amd64 golang image and could thus replace the publish_multiarch_image.sh for golang with the regular publish_image.sh no?
There was a problem hiding this comment.
We need multi-arch golang image, I have update the PR.
|
Hi @dhiller the PR has updated, would you like to take a look? |
There was a problem hiding this comment.
/approve
/hold @brianmcarey mind taking another look? If you don't find anything, then let's remove the hold
kubevirt-bot
added
the
do-not-merge/hold
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: dhiller 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:
Approvers can indicate their approval by writing |
kubevirt-bot
added
the
approved
|
/retest-required |
|
Hi @dhiller |
| @@ -85,7 +85,7 @@ presubmits: | |||
| memory: "1Gi" | |||
| limits: | |||
| memory: "1Gi" | |||
| - name: build-bootstrap-image | |||
| - name: build-multiarch-bootstrap-image | |||
| always_run: false | |||
| run_if_changed: "images/bootstrap/.*|images/golang/.*" | |||
| decorate: true | |||
There was a problem hiding this comment.
As we are doubling the number of images that we are building - we may need to update the default 2 hours timeout for this job. I tried running this locally and it timed out - I updated the timeout to 3 hours and the job passed. Here is an example of how to configure the timeout:
Once we have the timeout increased for this and the publish job - I think this looks good.
| for arch in ${archs[*]};do | ||
| amend+=" --amend ${full_image_name}-${arch}" | ||
| done | ||
| docker manifest create ${full_image_name} ${amend} |
There was a problem hiding this comment.
It would be cool to try this with a tool like buildah but we can try doing that later. Other kubevirt projects are starting to adopt buildah for multi-arch builds - kubevirt/hostpath-provisioner#115
Signed-off-by: Howard Zhang <[email protected]>
kubevirt-bot
removed
the
do-not-merge/hold
|
/override build-bootstrap-image Local run of job completed successfully. |
|
@brianmcarey: Overrode contexts on behalf of brianmcarey: build-bootstrap-image In response to this:
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. |
| ./publish_image.sh bootstrap quay.io kubevirtci | ||
| ./publish_image.sh golang quay.io kubevirtci | ||
| ./publish_multiarch_image.sh bootstrap quay.io kubevirtci | ||
| ./publish_multiarch_image.sh -l golang quay.io kubevirtci |
There was a problem hiding this comment.
Good point - we probably don't need this specific local image flag.
There was a problem hiding this comment.
We need this flag. For Dockerfile whose baseimage is from remote repository, we need manually pull correct CPU arch base image, otherwise it would use local image even if the local image has different CPU arch with --platform.
For these Dockerfile we use local built image, like the golang image, we do not need to pull base image.
|
/hold |
kubevirt-bot
added
the
do-not-merge/hold
|
/unhold |
kubevirt-bot
removed
the
do-not-merge/hold
|
@zhlhahaha: The following tests failed, say
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. I understand the commands that are listed here. |
|
/override build-bootstrap-image |
|
@brianmcarey: Overrode contexts on behalf of brianmcarey: build-bootstrap-image In response to this:
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. |
|
@zhlhahaha: Updated the
In response to this:
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. |
build bootstrap image for amd64 and arm64
New Dockerfile for bootstrap has
ARG ARCHwhich is not acquired in the build-bootstrap-image test, so the test would always fail.