kubernetes-sigs / kind

Kubernetes IN Docker - local clusters for testing Kubernetes
https://kind.sigs.k8s.io/
Apache License 2.0
13.48k stars 1.56k forks source link

Shell completion for loading docker images #2081

Open wilsonehusin opened 3 years ago

wilsonehusin commented 3 years ago

What would you like to be added:

Would be nice if I can get shell auto-completion from kind load docker-image [TAB][TAB] with the list of available images on my machine. Roughly the options can be retrieved by:

❯ docker image ls --format '{{.Repository}}:{{.ID}}{{if .Tag}}\n{{.Repository}}:{{.Tag}}{{end}}'
kube-build:4c4633e52abe
kube-build:build-ea19f83daf-5-v1.15.8-1
<none>:9418e301751d
<none>:<none>
k8s.gcr.io/conformance-amd64:f1d5ca7f8b5f
k8s.gcr.io/conformance-amd64:distroless
k8s.gcr.io/build-image/kube-cross:fc653090dd37
k8s.gcr.io/build-image/kube-cross:v1.15.8-1
kindest/base:13ec3861490d
kindest/base:v20201112-cc74d297
k8s.gcr.io/coredns/coredns:296a6d5035e2
k8s.gcr.io/coredns/coredns:v1.8.0
kindest/kindnetd:298b621990b4
kindest/kindnetd:v20200928-6a47034c
k8s.gcr.io/etcd:0369cf4303ff
k8s.gcr.io/etcd:3.4.13-0
rancher/local-path-provisioner:e422121c9c5f
rancher/local-path-provisioner:v0.0.14
k8s.gcr.io/build-image/debian-base:c7c6c86897b6
k8s.gcr.io/build-image/debian-base:v2.1.0
k8s.gcr.io/pause:0184c1613d92
k8s.gcr.io/pause:3.3

or without the <none> images being included:

❯ docker image ls --format '{{if ne .Repository "<none>"}}{{.Repository}}:{{.ID}}{{if .Tag}}\n{{.Repository}}:{{.Tag}}{{end}}{{end}}'
kindest/node:fd88bf2f1f69
kindest/node:latest
kube-build:4c4633e52abe
kube-build:build-ea19f83daf-5-v1.15.8-1
k8s.gcr.io/conformance-amd64:f1d5ca7f8b5f
k8s.gcr.io/conformance-amd64:distroless
k8s.gcr.io/build-image/kube-cross:fc653090dd37
k8s.gcr.io/build-image/kube-cross:v1.15.8-1
kindest/base:13ec3861490d
kindest/base:v20201112-cc74d297
k8s.gcr.io/coredns/coredns:296a6d5035e2
k8s.gcr.io/coredns/coredns:v1.8.0
kindest/kindnetd:298b621990b4
kindest/kindnetd:v20200928-6a47034c
k8s.gcr.io/etcd:0369cf4303ff
k8s.gcr.io/etcd:3.4.13-0
rancher/local-path-provisioner:e422121c9c5f
rancher/local-path-provisioner:v0.0.14
k8s.gcr.io/build-image/debian-base:c7c6c86897b6
k8s.gcr.io/build-image/debian-base:v2.1.0
k8s.gcr.io/pause:0184c1613d92
k8s.gcr.io/pause:3.3

Why is this needed:

As part of my development iteration, I need to repetitively load docker images. This would make my development iteration easier :smile:

Other notes:

Happy to contribute this if maintainers would not mind reviewing the code!

BenTheElder commented 3 years ago

This sounds interesting, how do you propose to implement it?

I think we will have some interesting portability concerns xref: https://github.com/kubernetes-sigs/kind/issues/2038 depending on how it is implemented

wilsonehusin commented 3 years ago

What I had in mind was using ValidArgsFunction in cobra, where it executes docker image ls or HTTP call to Docker server. However, in context of image-agnostic interface, it might be difficult unless user specifies the prefix with docker:// or podman://.

I suppose if the scope is limited to Docker and Podman, we should be able to achieve this from executing the binary since Podman CLI aims to be backwards-compatible with Docker to a point of alias docker=podman, right?

BenTheElder commented 3 years ago

in the #2038 case we aim to support having both installed, because you may e.g. need to build kubernetes images with buildx but prefer to use podman when you have the choice.

since Podman CLI aims to be backwards-compatible with Docker to a point of alias docker=podman, right?

I don't think they're making this claim as strongly anymore. Within kind we've found that this is ... not the case once you're doing interesting things. We have explicit codepaths for both and they're not identical.

wilsonehusin commented 3 years ago

I see, if the case is to support both, then I think this is something we can iterate on -- add support for one, then the other and append the results?

I have yet to try loading image from podman so maybe I'm rambling nonsense, but if I submit a PR for supporting autocompletion on docker images now while we explore the podman route, would that be acceptable? Or would you prefer to wait until we find a common ground / universal approach for both?

grzesuav commented 3 years ago

I guess it should be done on SHELL side, so create appropriate completion in bash/zsh ?

BenTheElder commented 3 years ago

I see, if the case is to support both, then I think this is something we can iterate on -- add support for one, then the other and append the results?

well we also have completion for other shells atm.

I have yet to try loading image from podman so maybe I'm rambling nonsense, but if I submit a PR for supporting autocompletion on docker images now while we explore the podman route, would that be acceptable? Or would you prefer to wait until we find a common ground / universal approach for both?

that seems reasonable, also it will probably be a different command.

eddiezane commented 3 years ago

https://github.com/kubernetes/kubernetes/pull/96087 might give an idea for this.

BenTheElder commented 3 years ago

cobra just overhauled completion significantly and we will need to update our existing support to handle this, I think kubernetes will also have some issues upgrading but will probably just disable the support from cobra.

marckhouzam commented 3 years ago

cobra just overhauled completion significantly and we will need to update our existing support to handle this, I think kubernetes will also have some issues upgrading but will probably just disable the support from cobra.

If I may chime in, Cobra's completion changes should be fully backwards-compatible, and I believe an upgrade should be transparent.

If you have your own completions written in bash, you would need to write them in Go for them to work with different shells, but I don't believe you do.

Are there specific issues you are referring to @BenTheElder ?

BenTheElder commented 3 years ago

IIRC there was some custom work for ZSH but I have not had time to look at this. It would be great if someone wanted to 😅

marckhouzam commented 3 years ago

IIRC there was some custom work for ZSH but I have not had time to look at this. It would be great if someone wanted to 😅

I've checked how completion was done for kind and you are all good. Specifically, kind uses Cobra's native zsh completion and does not use the bash-to-zsh conversion that kubectl was using up until kubernetes/kubernetes#96087, as can be seen here: https://github.com/kubernetes-sigs/kind/blob/c7f1d202a1408691d6ab1fb8e3112e11832ce500/pkg/cmd/kind/completion/zsh/zsh.go#L34

So, I am confident you can upgrade to Cobra 1.2.1 completely transparently.

Also, using ValidArgsFunction as mentioned by @wilsonehusin to add custom completion logic would work nicely and would automatically work for all shells (bash, zsh, fish). As a side-note, be aware @wilsonehusin that when adding custom completions, you can also include descriptions for them if you are so inclined (https://github.com/spf13/cobra/blob/master/shell_completions.md#descriptions-for-completions).

Beyond adding custom completions, there are two other improvements you may be interested in making: 1- adding support for powershell completions, as provided by Cobra (https://github.com/spf13/cobra/blob/master/shell_completions.md#powershell-completions) 2- using Cobra's new bash completion V2 logic which supports descriptions like the other shells already support (https://github.com/spf13/cobra/blob/master/shell_completions.md#bash-completion-v2)