-
Notifications
You must be signed in to change notification settings - Fork 507
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
images: fix Archs expansion in makefile #1865
Conversation
/priority important-soon |
/lgtm |
/hold for more eyes |
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 reasonable to me, thanks @cpanato 👍
/lgtm
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: ameukam, cpanato, hasheddan 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 |
will lift the hold since got a review from a sig-release TL 🚀 /hold cancel will observe the kube-cross job |
What type of PR is this?
/kind bug
What this PR does / why we need it:
When we start the new build for
kube-cross
image we got this errorwhich looks like when we run the
docker manifest create --amend
we need to add all the images for the archs we built.This piece of the make file https://github.com/kubernetes/release/blob/master/images/Makefile.common-image#L59 add the images to the command, but now we have more
platforms
and the commandsubst
just replace one entry and not all entries in thePLATFORMS
variablefor kube-cross we used to have just one platform and that works fine, but now we introduce more (can see in this PR: https://github.com/kubernetes/release/pull/1853/files) and the command broke.
this PR fix this issue by parsing the
PLATFORMS
and removing thelinux/
from thePLATFORMS
variable.before:
after the fix:
/assign @justaugustus @saschagrunert @hasheddan @ameukam
Which issue(s) this PR fixes:
None
Special notes for your reviewer:
Does this PR introduce a user-facing change?