-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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.
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
prow: build images for arm64 #22362
prow: build images for arm64 #22362
Conversation
Welcome @LorbusChris! |
Hi @LorbusChris. Thanks for your PR. I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. 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. |
/ok-to-test |
Right now rules_docker does not have the ability to produce multi-platform images, so we'll either need to pull in the ability to do this, use different tags for other architectures, or something else. |
2ad3fa9
to
d715b77
Compare
the second commit currently contains a reference to a one-off rebuild of the git image. It should be replaced by a properly automated build from #22450 |
prow/def.bzl
Outdated
stamp = True, # stamp by default, but allow overrides | ||
app_name = "app", | ||
**kwargs): | ||
go_image( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there some important reason we need both prow_image
and prow_image_arm64
macros?
It would be nice to just add these two go_image
and container_image
calls to the prow_image
macro, potentially gated by an declare_arm = False
macro which we can set to true for the handful of places you want to use it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My reasoning was that having arm images might not be required for everyone, and this way one can make them separately.
I'm not married to doing it this way, but I'd like to hear @stevekuznetsov's thoughts on this, too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Gating via some bool on the macro would achieve the same, though. I'll go ahead and make the change.
prow/BUILD.bazel
Outdated
prow_push( | ||
name = "release-push-arm64", | ||
bundle_name = "release-arm64", | ||
images = tags_arm64( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about just adding this to release-push
images argument?
This commit updates the `git-base` to tag `v20210618-1017135`, and also adds a `git-base-arm64` image from the same manifest.
276e5a9
to
d688f81
Compare
The arm64 payload only contains the following four images which are the minimal requirements for scheduling Prow builds on an arm64 build farm: - clonerefs - entrypoint - initupload - sidecar The arm64 images will be tagged with an additional `-arm64` suffix.
@fejta I've updated the macro per your review. Please take another look |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks really great! Thanks for all the work here.
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: fejta, LorbusChris 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 |
great, thanks for the review! |
With this commit, a minmal prow payload for arm64 can be built.
The payload only contains the following four images which are
the minimal requirements for scheduling Prow builds on an arm64 build
farm:
The image will be tagged as before but with an additional
-arm64
suffix.Blocked on: #22450
/cc @stevekuznetsov