-
Notifications
You must be signed in to change notification settings - Fork 519
images: Move kube-cross image building to k/release #1140
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
Conversation
@justaugustus: Adding label: Reasons for blocking this PR:[Changes to certain release tools can affect our ability to test, build, and release Kubernetes. This PR must be explicitly approved by SIG Release repo admins.] 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. |
The title says it is moving the image building, but how come the diff is purely additive? Is this moving us to a state where there are 2 ways to build (old way untouched) with a future follow-up PR finally deprecating the old process? Apologies in advance if I'm missing something obvious. |
@listx -- You haven't missed anything :) I want to get feedback from the people who do go bumps on:
If we agree that that's a reasonable path forward, then I think we'd want to:
WDYT? |
images/build/cross/Dockerfile
Outdated
@@ -0,0 +1,79 @@ | |||
# Copyright 2016 The Kubernetes Authors. |
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.
2020
images/build/cross/Makefile
Outdated
. | ||
|
||
push: | ||
docker push $(STAGING_REGISTRY)/$(IMAGE):$(TAG) |
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.
Let's please add a manifest with a single arch (amd64) for now.
docker push $(STAGING_REGISTRY)/$(IMAGE)-$(ARCH):$(TAG)
docker push $(STAGING_REGISTRY)/$(IMAGE)-$(ARCH):$(KUBE_CROSS_VERSION)
docker manifest annotate $(STAGING_REGISTRY)/$(IMAGE) $(STAGING_REGISTRY)/$(IMAGE)-$(ARCH) --arch amd64
docker manifest push -p $(STAGING_REGISTRY)/$(IMAGE)
This will help folks who end up with issues like kubernetes/kubernetes#78964 and will give us a path to add other arch(es) in the future to support kubernetes/kubernetes#75114 for example.
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.
Thanks @dims! Added.
Signed-off-by: Stephen Augustus <[email protected]>
1de3a08
to
e96924f
Compare
Signed-off-by: Stephen Augustus <[email protected]>
e96924f
to
593eda6
Compare
/approve /hold |
Signed-off-by: Stephen Augustus <[email protected]>
593eda6
to
05b14b6
Compare
Fixed up the manifest push using $ docker manifest inspect gcr.io/k8s-staging-build-image/kube-cross:v1.13.8-2
{
"schemaVersion": 2,
"mediaType": "application/vnd.docker.distribution.manifest.list.v2+json",
"manifests": [
{
"mediaType": "application/vnd.docker.distribution.manifest.v2+json",
"size": 3274,
"digest": "sha256:9816fc4e6e223d4bca55f233bc78d26ab38272dfbab59776bb082b3d35c31502",
"platform": {
"architecture": "amd64",
"os": "linux"
}
}
]
} |
/lgtm |
/lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: BenTheElder, dims, justaugustus 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 |
Thanks y'all! |
What type of PR is this?
/kind feature
What this PR does / why we need it:
Part of #1133.
While we've enabled building kube-cross on K8s Infra, we haven't eliminated the manual intra-PR image building/pushing by some human (Googler or otherwise).
The process (yet to be documented) is something like:
This PR is the first step in proposing we do the following:
pull-kubernetes-cross
andpull-kubernetes-verify
against the new staging image/assign @BenTheElder @cblecker @liggitt @dims @listx
cc: @kubernetes/release-engineering
/hold
Does this PR introduce a user-facing change?: