Skip to content

Conversation

thaJeztah
Copy link
Member

docker run, create: add shell completion for CDI devices for "--device"

With this patch:

docker info --format '{{json .DiscoveredDevices}}'
[{"Source":"cdi","ID":"docker.com/gpu=webgpu"}]

docker container create --device=<tab>
docker container create --device=docker.com/gpu=webgpu

docker run --device=docker.com/gpu=webgpu
docker run --device=<tab>

- How to verify it

- Human readable description for the release notes

Add shell completion for CDI devices for "--device" on `docker run` and `docker create`

- A picture of a cute animal (not mandatory but encouraged)

With this patch:

    docker info --format '{{json .DiscoveredDevices}}'
    [{"Source":"cdi","ID":"docker.com/gpu=webgpu"}]

    docker container create --device=<tab>
    docker container create --device=docker.com/gpu=webgpu

    docker run --device=docker.com/gpu=webgpu
    docker run --device=<tab>

Signed-off-by: Sebastiaan van Stijn <[email protected]>
Signed-off-by: Sebastiaan van Stijn <[email protected]>
@codecov-commenter
Copy link

Codecov Report

❌ Patch coverage is 10.52632% with 17 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
cli/command/container/completion.go 10.52% 16 Missing and 1 partial ⚠️

📢 Thoughts on this report? Let us know!

Comment on lines 159 to 165
func completeCDIDevices(dockerCLI completion.APIClientProvider) cobra.CompletionFunc {
return func(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) {
if strings.HasPrefix(toComplete, "/") {
return nil, cobra.ShellCompDirectiveDefault | cobra.ShellCompDirectiveNoSpace
}

info, err := dockerCLI.Client().Info(cmd.Context())
Copy link
Member Author

Choose a reason for hiding this comment

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

Opening as draft, because I had this branch but after implementing CDI completion, I realised we also need to provide completion for file-paths (assuming the daemon and CLI are on the same host), so we need some solid way to distinguish "user wants to complete a CDI device" vs "user wants to add a device (e.g. /dev/foo:/some/path).

Comment on lines 170 to 174
devices = append(devices, di.ID)
devices = append(devices, cobra.CompletionWithDesc(di.ID, "CDI device"))
Copy link
Member Author

Choose a reason for hiding this comment

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

Still on the fence if cobra.CompletionWithDesc is a bit "too much"; it's the equivalent of <value> + \t + <description>. I guess it's slightly more expressive, but also makes it look more magical than it is 😂

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CDI: --device shell completion
2 participants