-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
Replace k8s.gcr.io with registry.k8s.io #15777
Conversation
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: mrbobbytables The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Can one of the admins verify this patch? |
@mrbobbytables : the goal with the code changes, was to have the minikube cache/preload match This is why there was a cut-off, so that we don't have to invalidate old caches and old preloads needlessly ? So it is good that the examples and tests are updated, but not changing e.g. 1.20 was done on purpose... Versions -1.21 use old, versions 1.25+ use new. The others have their cutoff points, 1.22.18, 1.23.15, 1.24.9 |
cache/images/k8s.gcr.io/kube-addon-manager_v9.0 | ||
cache/images/k8s.gcr.io/k8s-dns-kube-dns-amd64_1.14.13 | ||
cache/images/k8s.gcr.io/kube-proxy_v1.14.0 | ||
cache/images/registry.k8s.io/k8s-dns-sidecar-amd64_1.14.13 |
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.
the kubernetes version needs to be updated, in this example (not changing anything for 1.14)
@@ -294,7 +294,7 @@ Steps: | |||
asserts basic "service" command functionality | |||
|
|||
Steps: | |||
- Create a new `k8s.gcr.io/echoserver` deployment | |||
- Create a new `registry.k8s.io/echoserver` deployment |
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.
The echoserver no longer lives in the main registry, it has been forked to multiple locations.
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.
For minikube, I think k8s.gcr.io/echoserver:1.4
was replaced with kicbase/echo-server:1.0
@@ -76,7 +76,7 @@ type ClusterConfig struct { | |||
KubernetesConfig KubernetesConfig | |||
Nodes []Node | |||
Addons map[string]bool | |||
CustomAddonImages map[string]string // Maps image names to the image to use for addons. e.g. Dashboard -> k8s.gcr.io/echoserver:1.4 makes dashboard addon use echoserver for its Dashboard deployment. | |||
CustomAddonImages map[string]string // Maps image names to the image to use for addons. e.g. Dashboard -> registry.k8s.io/echoserver:1.4 makes dashboard addon use echoserver for its Dashboard deployment. |
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.
the echoserver image needs to be updated to the new one
@@ -21,7 +21,7 @@ import ( | |||
) | |||
|
|||
// OldDefaultKubernetesRepo is the old default Kubernetes repository | |||
const OldDefaultKubernetesRepo = "k8s.gcr.io" | |||
const OldDefaultKubernetesRepo = "registry.k8s.io" |
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.
Changing the old registry to be the same as the new, makes this rather meaningless.
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.
ah yeah - sorry, this was a more generic find & replace. Will update
"k8s.gcr.io/coredns:1.6.2", | ||
"k8s.gcr.io/etcd:3.3.15-0", | ||
"k8s.gcr.io/pause:3.1", | ||
"registry.k8s.io/kube-proxy:v1.16.0", |
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.
not changing anything for 1.16 ?
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 must have accidentally just replaced that 1 line vs all in the file, will update
The old echoserver needs to be updated to the new location, @spowelljr I think this will still use the "kicbase" echoserver ? |
@afbjorklund should I just close this? It looks like things are being tackled / tracked separately in #14769 FWIW - theres a policy discussion going on about agining out old images, we're sadly still on track to exceed our 3M in GCP credits (to the tune of an additional 1M 😬 ) and are looking for ways to reduce costs. Outside of policy decisions on removing old images, one of the big things we can do is get as much user facing things to switch over to the registry.k8s.io which will spread the load across multiple providers. Updating k/website and this repo looked like an easy win because they are so frequently used / referenced by end users. |
A little reminder is a good thing, it missed the minikube 1.29.0 release for instance. I think the best approach for minikube, would be to make a "preload" version also for the "none" driver.
Currently it is pulling them one by one from the registry, and storing them in cache (without preload) But when there is a combined tarball, that could be used instead - and also for the "none" driver. Basically: |
I'll close this for now and move things over to separate PRs, and track in #14769 /close |
@mrbobbytables: Closed this PR. In response to this:
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. |
@mrbobbytables if there is any other discussion on how to make the downloads smaller, please invite I don't know if the preload bucket and release bucket are also in the same spreadsheet, somewhere... https://storage.googleapis.com/minikube-preloaded-volume-tarballs/ |
I don't know of any discussion on making the images smaller at this time, most of the discussion is around shutting traffic to other providers or aging out old image. I don't know if image size would impact much of the traffic right now - from various sources it looks like most users are running older images, and thats what drove the backporting discussion / push to get everyone to use registry.k8s.io. |
Ok, for minikube it is mostly those two.
But the URLs should be changed, as well. |
But it would only invalidate old caches? How long will minikube continue to support these old versions?
I have no idea where this is and who pays for it ... 😬 This is billed to google internally / different bill that SIG K8s Infra. There's other work ongoing to migrate that out to fully community control but it's a lot so we're seeking out other funding before doing so ....
We've had pretty active work targeting images like kube-proxy but nothing at this time. distroless kube-proxy shipped a pretty major reduction in 1.25 kubernetes/kubernetes#109406 I think out of the top images, for current tags only ingress-nginx/controller jumps out as something that appears to be particularly large. |
This was just about the default in I don't see why the version support should be different in minikube, from the rest of the project... * the old tutorials will soon be deleted anyway
The preload is an optional pre-installed archive, so I guess it is the same as the kind "node" image. It would be possible to not provide this feature anymore, and use the regular container images.
The preload is all about making the install go faster, so it is actually bigger than the usual images. (it uses lz4) But it would be possible to make a "batteries included" version and optimize for a small download... (using xz) Otherwise I think we are hoping for improvements in the container runtimes and registries will "fix" the problem. |
k8s.gcr.io is in the process of being deprecated with future images no longer being served from that endpoint.
This is a general "find & replace" I ignored the changelog, but updated everything else.
For more information - see this blog post: https://kubernetes.io/blog/2022/11/28/registry-k8s-io-faster-cheaper-ga/