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

[FEATURE] Standard way for K3d to communicate about a local registry #234

Closed
nicks opened this issue May 1, 2020 · 20 comments · Fixed by #334
Closed

[FEATURE] Standard way for K3d to communicate about a local registry #234

nicks opened this issue May 1, 2020 · 20 comments · Fixed by #334
Assignees
Labels
enhancement New feature or request priority/medium
Milestone

Comments

@nicks
Copy link

nicks commented May 1, 2020

KEP


Original Issue Text

Many Kubernetes clusters support a local registry, to make pushing images directly to the cluster fast. But every cluster documents it with manual shell scripts:

Because every cluster is slightly different, this makes it hard for tools to automate this work for users.

The big idea

Clusters should use annotations on the kube-system namespace to document how to configure a registry

Detailed design

I propose 3 new "standard" annotations:

x-k8s.io/registry: the host of the registry (hostname and port), as seen from outside of the cluster
x-k8s.io/registry-from-cluster: the host of the registry (hostname and port), as seen from inside the cluster
x-k8s.io/registry-help: The URL of documentation where users can read more about how to set a registry up.

Tools that would benefit from local registries should read this annotation and display it to the user

What this means for K3d

All K3d clusters would have a registry-help annotation pointing users to https://github.com/rancher/k3d/blob/master/docs/registries.md#using-a-local-registry

We'd also update the example script at https://github.com/rancher/k3d/blob/master/docs/registries.md#using-a-local-registry to add the x-k8s.io/registry and x-k8s.io/registry-from-cluster annotation during setup.

Prior Art

This proposal is based on what we've been recommending to people in Tilt, with a lot of success: https://docs.tilt.dev/choosing_clusters.html#custom-clusters

update 5/4 - changed the proposal to use the kube-system namespace instead of node annotations

@nicks
Copy link
Author

nicks commented May 1, 2020

I also filed this feature request here kubernetes-sigs/kind#1543 and here canonical/microk8s#1173, and would love other ideas on places I should post about this

@iwilltry42
Copy link
Member

Hi @nicks, thanks for opening this issue 👍
This is really an interesting approach. However, there are still some open questions about whether using node annotations is a good idea here when talking about a cluster-wide attribute.
I see that there are already some comments concerning this in the issue on kind, so I'll just hook in there to see what comes out of that.
We're currently not pulling in that many k8s dependencies, so I hope to get this in as lightweight as possible 😁 E.g. a configmap could be created by using auto-deploy manifests.

@nicks
Copy link
Author

nicks commented May 4, 2020

ya, that's fair. I'll follow up on the Kind ticket and make sure this is up-to-date with specifics

@nicks
Copy link
Author

nicks commented May 5, 2020

Just to keep you all in the loop, sig-cluster-lifecycle has suggested I open a KEP for this to kick the tires more on the proposal.

But I also recognize that K3d falls slightly outside the KEP process. Are the K3d maintainers ok with that? Ya, I'll be sure to check in with you all if it starts to veer towards something that would require more k8s deps.

@iwilltry42
Copy link
Member

Hi @nicks thanks for the efforts here 👍
You're right, that the KEPs don't affect us too much, but I guess it would make sense to use a standard also in k3d, if there is one 👍

@nicks
Copy link
Author

nicks commented May 12, 2020

Yay! Thanks @iwilltry42

KEP PR: kubernetes/enhancements#1757
KEP tracking issue: kubernetes/enhancements#1755

@nicks
Copy link
Author

nicks commented May 13, 2020

one question that came up in another thread is if it's worth standardizing on how we document other image-loading options, e.g.,

imageLoadCommand: "k3d import-images $IMAGE_REF"

but maybe that makes more sense in a future KEP

@nicks
Copy link
Author

nicks commented May 20, 2020

Hi @iwilltry42! I'm doing a last call for any major objections or things I overlooked before we merge the initial spec

kubernetes/enhancements#1757
https://github.com/kubernetes/enhancements/blob/c5b6b632811c21ababa9e3565766b2d70614feec/keps/sig-cluster-lifecycle/generic/1755-communicating-a-local-registry/README.md#design-details

If you're OK with this proposal, a thumbs-up on the PR would be a help a lot! Thanks!

@iwilltry42
Copy link
Member

Thanks for your efforts @nicks :) You have my 👍

@nicks
Copy link
Author

nicks commented Jun 11, 2020

The KEP has been marked implementable! I suspect it will be straightforward to implement, but let me know if there's any way I can assist!

@iwilltry42
Copy link
Member

Very cool @nicks ! Congrats for pushing this through 🥳
However, the k3d-managed registry didn't land in v3 yet and will probably make it into v3.1.0, so we'll apply this then 👍

@iwilltry42 iwilltry42 added this to the 3.1.0 milestone Jun 12, 2020
@iwilltry42 iwilltry42 modified the milestones: 3.1.0, 3.2.0 Oct 6, 2020
@iwilltry42 iwilltry42 modified the milestones: 3.2.0, v3.4.0 Nov 24, 2020
@iwilltry42 iwilltry42 linked a pull request Nov 24, 2020 that will close this issue
10 tasks
@iwilltry42 iwilltry42 modified the milestones: v3.4.0, v4.1.0, v4.0.0 Dec 4, 2020
@iwilltry42 iwilltry42 self-assigned this Jan 7, 2021
@iwilltry42
Copy link
Member

Hi @nicks 👋
This is in it's final steps for the v4.0.0 release.
I've read your great KEP, and it's sure, that this specifies exactly one local registry, right?
So k3d supports connecting a cluster to multiple (local) registries. In the current solution draft, it would simply choose the first registry it sees for a cluster as the primary registry, that will be advertised using the new spec.

@nicks
Copy link
Author

nicks commented Jan 9, 2021

Ya, that sounds reasonable! The goal of the Config is to advertise to tools what registry they should push to...so choosing the first registry it sees is fine.

@iwilltry42
Copy link
Member

#441 will land in the next v4.0.0-beta.0 release

@nicks
Copy link
Author

nicks commented Jan 14, 2021

Hi @iwilltry42 ! Thank you for landing this. I tried it out and I think there's a minor issue.

$ k3d cluster create --create-registry
$ kubectl local-registry get
host: 0.0.0.0:45659
hostFromContainerRuntime: k3d-k3s-default-registry:5000
help: https://k3d.io/usage/guides/registries/#using-a-local-registry

"0.0.0.0" isn't a valid host here...

would it be better to file a separate bug? I can play around a bit to figure out what it should be...

@iwilltry42
Copy link
Member

@nicks are you on a Mac or Windows?
There I guess we need to go via the docker IP again. Or just localhost..

@nicks
Copy link
Author

nicks commented Jan 14, 2021

linux!

@iwilltry42
Copy link
Member

And it doesn't work for you in Linux with 0.0.0.0? I know it's not a "real" IP, but should work anyway in many cases on Linux.
In any case, we should open a new issue to tackle this 👍

@nicks
Copy link
Author

nicks commented Jan 14, 2021

Nope. Ya, let me file a new issue with the details.

@nicks
Copy link
Author

nicks commented Jan 14, 2021

filed as #449

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request priority/medium
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants