-
Notifications
You must be signed in to change notification settings - Fork 247
make the docker image multi-arch #925
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
|
This issue is currently awaiting triage. If the repository mantainers determine this is a relevant issue, they will accept it by applying the The DetailsInstructions 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-sigs/prow repository. |
|
|
Should be fixed now |
It is, but uses emulation build. I would expect to use build platform and cross compile with GOOS and GOARCH. |
|
The pd CSI driver is built like that too https://github.com/kubernetes-sigs/gcp-compute-persistent-disk-csi-driver/blob/master/Dockerfile The main problem is that the bazel is everywhere. The emulation isn't too slow right now. |
|
The emulation is slow and prone to errors. Not exactly sure what you mean with PD CSI driver, I see there it uses |
|
does this give at least something functional we can iterate later and solve the current issue? removing bazel entirely or doing more complex things in this repo can stall us forever ... trust me ;) |
cloudbuild.yaml
Outdated
| machineType: E2_HIGHCPU_32 | ||
| steps: | ||
| - name: 'gcr.io/cloud-builders/docker' | ||
| - name: "gcr.io/k8s-staging-test-infra/gcb-docker-gcloud:v20251110-7ccd542560" |
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.
Why does this need to change?
Looks like it works fine for NPD: https://github.com/kubernetes/node-problem-detector/blob/master/cloudbuild.yaml.
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.
That image has the latest version of docker in 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.
I doubt the official Google one is old.
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.
@upodroid let's try first with the official image, so we avoid the dependency on the staging ones
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.
I got it working with the official one, reverting this change.
|
@aojea I am good with the approach. I added one more comment regarding an image, otherwise looks good from my end. |
Dockerfile
Outdated
| RUN CGO_ENABLED=0 go build -o /go/bin/cloud-controller-manager ./cmd/cloud-controller-manager | ||
|
|
||
| FROM registry.k8s.io/build-image/go-runner:v2.4.0-go1.24.0-bookworm.0 | ||
| FROM gcr.io/distroless/static-debian12 |
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.
we need go runner, some of the manifest used depend on that
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.
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.
Reverted
|
/lgtm Thanks |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: aojea, upodroid The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/lgtm |
|
Its looking good. @YifeiZhuang Can you cut a new patch release please? Thanks |
…pstream-release-1.34 Automated cherry pick of #925: make the docker image multi-arch
The GCP CCM isn't multi-arch, and arm64 jobs on kops are failing :(
https://testgrid.k8s.io/kops-distro-cosdev#kops-grid-gce-kubenet-cosdevarm64-k34
Can we have this merged and a new patch release cut asap?
Thanks
@justinsb @hakman