Update bash-completion for podman inspect #19181

cgiradkar · 2023-07-10T13:49:00Z

The command:
podman inspect <ContainerID> -f "{{."
was not giving any output in console for bash-completion but the same was working fine for the command:
podman container inspect <ContainerID> -f "{{."
So added a flow to handle the former command and additionally handled all types of entities namely container, image, volume, pod and network.
This would ESSENTIALLY make the sub-commands such as follows OBSOLETE:
podman container inspect
podman image inspect
podman volume inspect
podman pod inspect
podman network inspect

Does this PR introduce a user-facing change?

The following command is now enabled with tab-completion:
podman inspect {CONTAINER | IMAGE | VOLUME | POD | NETWORK} -f "{{." <tab>
where CONTAINER can be ContainerID or ContainerName and same for other given entities.

Closes #18672

Signed-off-by: Chetan Giradkar [email protected]

Signed-off-by: Chetan Giradkar <[email protected]>

The command: ```podman inspect <ConatinerID> -f "{{."``` was not giving any output in console for bash-completion but the same was working fine for the command: ```podman container inspect <ConatinerID> -f "{{."``` So added a flow to handle the former command by defaulting to type CONTAINER and restrict the flow ofbash-completion if other types (eg. IMAGE) is encountered. Signed-off-by: Chetan Giradkar <[email protected]>

openshift-ci · 2023-07-10T13:49:08Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: cgiradkar
Once this PR has been reviewed and has the lgtm label, please assign saschagrunert for approval. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

…ontainer_inspection_completion

Luap99 · 2023-07-10T14:22:40Z

cmd/podman/common/completion.go

+		switch reflect.TypeOf(o).String() {
+		case "*define.InspectContainerData":


I don't think this can work, this type depends on what you pass in AutocompleteFormat(). If we want to support more than container than you need to have more than one type and you cannot know that until this function is called.

You need to basically invert the order first check getContainers() if the arg is a container then you can set
o = &define.InspectContainerData{} same for the other types and so on.

Because AutocompleteFormat() is used by a lot of different callers we need to have a check to only do that for the podman inspect command. One way to do that without adding a new parameter is to check for
cmd.Name() == inspect and cmd.Parent() == cmd.Root(). Only then we want to check the arg and overwrite the o value.

Luap99 · 2023-07-10T14:23:45Z

cmd/podman/common/completion.go

+		// add suggestions for: podman inspect (it will default to container inspect)
+		switch reflect.TypeOf(o).String() {
+		case "*define.InspectContainerData":
+			if containers, _ := getContainers(cmd, args[0], completeDefault); len(containers) == 0 { // there might be a panic here when args length is 0. Is it assured that args will not be empty?


you have to make a len(args) > 0 check before, if args == 0 we can assume the container type.

```podman inspect <ConatinerID> -f "{{."``` was not giving any output in console for bash-completion but the same was working fine for the command: ```podman container inspect <ConatinerID> -f "{{."``` So added a flow to handle the former command and additionally handled all types of entities namely container, image, volume, pod and network. This would ESSENTIALLY make the sub-commands such as follows OBSOLETE: podman container inspect podman image inspect podman volume inspect podman pod inspect podman network inspect The following command now enabled with tab-completion: ```podman inspect {CONTAINER | IMAGE | VOLUME | POD | NETWORK} -f "{{." <tab>``` Signed-off-by: Chetan Giradkar <[email protected]>

Luap99 · 2023-07-11T11:56:20Z

cmd/podman/common/completion.go

+		// special(expensive) flow for "podman inspect"
+		if cmd != nil && cmd.Name() == "inspect" && cmd.Parent() == cmd.Root() {
+			if len(args) == 0 {
+				return nil, cobra.ShellCompDirectiveNoFileComp


if no args we should default to &define.InspectContainerData{}

@Luap99 the arg here is the name/Id of the container, image, etc. Without it, we cannot proceed to display corresponding suggestions. Is the explanation good or am I missing something?

It should assume container, consider typing podman inspect --format {{. then we do not yet know what the arg is but I think in most cases it is a container so defaulting to that makes sense to me.

Ok, the use case is somewhat vague but it can be integrated, thanks!

Luap99 · 2023-07-11T12:00:26Z

cmd/podman/common/completion.go

+				found := false
+				// container logic
+				if containers, _ := getContainers(cmd, args[0], completeDefault); len(containers) > 0 {
+					o = &define.InspectContainerData{}
+					found = true
+				}
+
+				// image logic
+				if images, _ := getImages(cmd, args[0]); len(images) > 0 && !found {
+					o = &inspect.ImageData{}
+					found = true
+				}
+
+				// volume logic
+				if volumes, _ := getVolumes(cmd, args[0]); len(volumes) > 0 && !found {
+					o = &define.InspectVolumeData{}
+					found = true
+				}
+
+				// pod logic
+				if pods, _ := getPods(cmd, args[0], completeDefault); len(pods) > 0 && !found {
+					o = &entities.PodInspectReport{}
+					found = true
+				}
+
+				// network logic
+				if networks, _ := getNetworks(cmd, args[0], completeDefault); len(networks) > 0 && !found {
+					o = &types.Network{}
+				}


if you split this out to a new function then you do not need the extra found var and can just return early which would be much more readable

will implement this logic to another function

Luap99 · 2023-07-11T12:08:59Z

As for testing you can add tests here:

podman/test/system/600-completion.bats

Lines 310 to 311 in e6dbb42

	check_shell_completion

Basically you can call run_completion inspect $name --format "{{." where $name is the resources created above, cotnianer, image, network, etc.. then you can check with assert "$output" =~ "<field name here>" that you get the expected completion output

Signed-off-by: Chetan Giradkar <[email protected]>

cgiradkar · 2023-07-18T12:12:43Z

closing as git history is polluted due to multiple rebases. Opened a new PR here: #19261

cgiradkar added 2 commits July 10, 2023 12:57

Add bash-completion for podman inspect and default to container-type

d79aa33

Signed-off-by: Chetan Giradkar <[email protected]>

openshift-ci bot added release-note do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. labels Jul 10, 2023

Merge branch 'main' of https://github.com/cgiradkar/podman into fix_c…

6042dee

…ontainer_inspection_completion

Luap99 reviewed Jul 10, 2023

View reviewed changes

cgiradkar requested a review from Luap99 July 10, 2023 21:33

cgiradkar changed the title ~~Update bash-completion for podman inspect and default to container-type~~ Update bash-completion for podman inspect Jul 10, 2023

Luap99 reviewed Jul 11, 2023

View reviewed changes

Add test for podman inspect with format flag

4dabbb8

Signed-off-by: Chetan Giradkar <[email protected]>

cgiradkar force-pushed the fix_container_inspection_completion branch from 89e11b8 to 4dabbb8 Compare July 17, 2023 12:10

cgiradkar closed this Jul 18, 2023

cgiradkar deleted the fix_container_inspection_completion branch August 1, 2023 18:28

github-actions bot added the locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. label Oct 31, 2023

github-actions bot locked as resolved and limited conversation to collaborators Oct 31, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update bash-completion for podman inspect #19181

Update bash-completion for podman inspect #19181

cgiradkar commented Jul 10, 2023 •

edited

Loading

openshift-ci bot commented Jul 10, 2023

Luap99 Jul 10, 2023

Luap99 Jul 10, 2023

Luap99 Jul 11, 2023

cgiradkar Jul 12, 2023

Luap99 Jul 12, 2023

cgiradkar Jul 12, 2023

Luap99 Jul 11, 2023

cgiradkar Jul 12, 2023

Luap99 commented Jul 11, 2023

cgiradkar commented Jul 18, 2023

		switch reflect.TypeOf(o).String() {
		case "*define.InspectContainerData":

Update bash-completion for podman inspect #19181

Update bash-completion for podman inspect #19181

Conversation

cgiradkar commented Jul 10, 2023 • edited Loading

Does this PR introduce a user-facing change?

openshift-ci bot commented Jul 10, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Luap99 commented Jul 11, 2023

cgiradkar commented Jul 18, 2023

cgiradkar commented Jul 10, 2023 •

edited

Loading