Skip to content
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

addons registry: Use multi-arch kube-registry-proxy image #16577

Merged
merged 2 commits into from
Jun 7, 2023

Conversation

spowelljr
Copy link
Member

The existing kube-registry-proxy image (gcr.io/google_containers/kube-registry-proxy) was only built for amd64 and doesn't work for users using QEMU on arm64 machines.

The source code for the image was removed in kubernetes/kubernetes#58564.

I took the source code, updated the nginx base image and added it to https://github.com/spowelljr/kube-registry-proxy and built the kube-registry-proxy image for our five supported architectures and pushed it to gcr.io/k8s-minikube/kube-registry-proxy.

It was confirmed to work by another user as well.
https://kubernetes.slack.com/archives/C1F5CT6Q1/p1684942501397369?thread_ts=1684931469.550259&cid=C1F5CT6Q1

I'm happy to move the source code into the project or into some minikube owned repo.

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label May 24, 2023
@k8s-ci-robot k8s-ci-robot added approved Indicates a PR has been approved by an approver from all required OWNERS files. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels May 24, 2023
@spowelljr
Copy link
Member Author

/ok-to-test

@k8s-ci-robot k8s-ci-robot added the ok-to-test Indicates a non-member PR verified by an org member that is safe to test. label May 24, 2023
@spowelljr spowelljr mentioned this pull request May 24, 2023
12 tasks
@@ -389,7 +389,7 @@ var Addons = map[string]*Addon{
"0640"),
}, false, "registry", "Google", "", "", map[string]string{
"Registry": "registry:2.8.1@sha256:83bb78d7b28f1ac99c68133af32c93e9a1c149bcd3cb6e683a3ee56e312f1c96",
"KubeRegistryProxy": "google_containers/kube-registry-proxy:0.4@sha256:1040f25a5273de0d72c54865a8efd47e3292de9fb8e5353e3fa76736b854f2da",
"KubeRegistryProxy": "k8s-minikube/kube-registry-proxy:0.0.1@sha256:498e7da3b171e24630615854c4a2924cfb8adbd43b0f252b1458c859ca9b746c",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lets create an issue on Kubernetes and ask them where they plan to maintain the deleted code ?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@BenTheElder mentioned anything under cluster has been depricated, so I think in minikube we should create an issue upstream to see if there is no other sponsor, minikube could host it on our repo

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Kubernetes/Kubernetes will not host this anymore since kube-up CI is not using it and excluding the etcd image this directory is only used for CI and not supported anymore (see cluster/README.md) but minikube could host these sources and iterate on it or SIG cluster lifecycle could sponsor bringing it back as a SIG project repo. I suspect the latter will not be supported though based on past conversations.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SIG Testing + Infra is also trying to phase out the rest as well because we need to be running CI on multiple clouds instead of only the GCE bash. We're starting small with just one scale test on kops-aws but the hope is to use kOps AWS+GCE instead of kube-up GCE more generally, eventually. So there's not much future for anything in cluster/. It's barely maintained as-is out of necessity for CI.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

since there is no owner for this image, we could just publish one last patch of it in arm64 but in a follow up PR we could change the ownership of this addon from Google to minikube

@@ -389,7 +389,7 @@ var Addons = map[string]*Addon{
"0640"),
}, false, "registry", "Google", "", "", map[string]string{
"Registry": "registry:2.8.1@sha256:83bb78d7b28f1ac99c68133af32c93e9a1c149bcd3cb6e683a3ee56e312f1c96",
"KubeRegistryProxy": "google_containers/kube-registry-proxy:0.4@sha256:1040f25a5273de0d72c54865a8efd47e3292de9fb8e5353e3fa76736b854f2da",
"KubeRegistryProxy": "k8s-minikube/kube-registry-proxy:0.0.1@sha256:498e7da3b171e24630615854c4a2924cfb8adbd43b0f252b1458c859ca9b746c",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@BenTheElder mentioned anything under cluster has been depricated, so I think in minikube we should create an issue upstream to see if there is no other sponsor, minikube could host it on our repo

@mprimeaux
Copy link

mprimeaux commented May 25, 2023

@medyagh and @spowelljr I am just catching up with this PR. I had commented here and here in the overall issue.

Let me know if I can help.

@@ -389,7 +389,7 @@ var Addons = map[string]*Addon{
"0640"),
}, false, "registry", "Google", "", "", map[string]string{
"Registry": "registry:2.8.1@sha256:83bb78d7b28f1ac99c68133af32c93e9a1c149bcd3cb6e683a3ee56e312f1c96",
"KubeRegistryProxy": "google_containers/kube-registry-proxy:0.4@sha256:1040f25a5273de0d72c54865a8efd47e3292de9fb8e5353e3fa76736b854f2da",
"KubeRegistryProxy": "k8s-minikube/kube-registry-proxy:0.0.1@sha256:498e7da3b171e24630615854c4a2924cfb8adbd43b0f252b1458c859ca9b746c",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

since there is no owner for this image, we could just publish one last patch of it in arm64 but in a follow up PR we could change the ownership of this addon from Google to minikube

@minikube-pr-bot
Copy link

kvm2 driver with docker runtime

+----------------+----------+---------------------+
|    COMMAND     | MINIKUBE | MINIKUBE (PR 16577) |
+----------------+----------+---------------------+
| minikube start | 53.0s    | 53.4s               |
| enable ingress | 29.7s    | 27.4s               |
+----------------+----------+---------------------+

Times for minikube start: 53.3s 53.1s 51.7s 53.4s 53.3s
Times for minikube (PR 16577) start: 52.3s 52.1s 57.8s 50.7s 54.1s

Times for minikube ingress: 28.2s 28.2s 32.7s 28.2s 31.2s
Times for minikube (PR 16577) ingress: 27.7s 27.7s 28.2s 25.1s 28.2s

docker driver with docker runtime

+----------------+----------+---------------------+
|    COMMAND     | MINIKUBE | MINIKUBE (PR 16577) |
+----------------+----------+---------------------+
| minikube start | 23.6s    | 24.4s               |
| enable ingress | 23.1s    | 22.4s               |
+----------------+----------+---------------------+

Times for minikube (PR 16577) ingress: 26.4s 21.4s 21.9s 20.9s 21.4s
Times for minikube ingress: 24.9s 20.9s 21.9s 21.9s 25.9s

Times for minikube start: 25.4s 22.1s 25.7s 22.3s 22.2s
Times for minikube (PR 16577) start: 25.1s 24.6s 24.3s 25.2s 22.6s

docker driver with containerd runtime

+----------------+----------+---------------------+
|    COMMAND     | MINIKUBE | MINIKUBE (PR 16577) |
+----------------+----------+---------------------+
| minikube start | 22.6s    | 22.1s               |
| enable ingress | 35.6s    | 31.8s               |
+----------------+----------+---------------------+

Times for minikube start: 21.0s 22.6s 22.9s 22.7s 23.6s
Times for minikube (PR 16577) start: 23.0s 20.7s 20.1s 22.8s 23.8s

Times for minikube ingress: 36.4s 31.3s 31.4s 32.4s 46.4s
Times for minikube (PR 16577) ingress: 31.4s 31.4s 31.4s 32.4s 32.4s

@minikube-pr-bot
Copy link

These are the flake rates of all failed tests.

Environment Failed Tests Flake Rate (%)
Hyperkit_macOS TestRunningBinaryUpgrade (gopogh) 11.61 (chart)
Hyper-V_Windows TestNoKubernetes/serial/StartWithK8s (gopogh) 49.12 (chart)

To see the flake rates of all tests by environment, click here.

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: medyagh, spowelljr

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@spowelljr spowelljr merged commit 7d59ace into kubernetes:master Jun 7, 2023
@spowelljr spowelljr deleted the updateKubeRegistryProxyImage branch June 7, 2023 20:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. ok-to-test Indicates a non-member PR verified by an org member that is safe to test. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants