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

inspect: add type flag to inspect command #964

Merged
merged 1 commit into from
Apr 10, 2022

Conversation

89luca89
Copy link
Contributor

@89luca89 89luca89 commented Apr 5, 2022

Right now nerdctl inspect does not have a --type flag similar to
docker and podman.

This PR aims to add this, allowing for the basic values:

  • container
  • image

It will default to container to match docker's behaviour

Example of the command:

./nerdctl inspect --format '{{json .}}' --type image registry.fedoraproject.org/fedora-toolbox:35
./nerdctl inspect --format '{{json .}}' --type container fedora-toolbox-35

Signed-off-by: Luca Di Maio luca.dimaio1@gmail.com

@AkihiroSuda
Copy link
Member

Thanks, but please use real name for Signed-off-by line

cmd/nerdctl/inspect.go Outdated Show resolved Hide resolved
cmd/nerdctl/inspect.go Outdated Show resolved Hide resolved
cmd/nerdctl/inspect.go Outdated Show resolved Hide resolved
@AkihiroSuda
Copy link
Member

Please squash commits

cmd/nerdctl/inspect.go Outdated Show resolved Hide resolved
@AkihiroSuda AkihiroSuda added this to the v0.19.0 (tentative) milestone Apr 6, 2022
@89luca89 89luca89 force-pushed the feature/inspect_type branch 2 times, most recently from b5cd2ea to bda31f1 Compare April 6, 2022 09:17
@89luca89
Copy link
Contributor Author

89luca89 commented Apr 6, 2022

Ok I now get what's happening
before with the "all" it would just do image or container, so if unspecified, it would find either of them
now it's either container or you must specify image, so that would make test fail

Docker works by default in the "all" style, so probably will need to find another way to put the logic in inspectAction

@AkihiroSuda
Copy link
Member

Docker works by default in the "all" style

Slightly different from the "all" style.

When both the container and the image exists, Docker only shows the container

$ docker run -d --name nginx nginx
58f268d9a42c0bd861f95d75c396e271be218191e0d94eb3575d58fac26b27f2

$ docker inspect nginx
[
    {
        "Id": "58f268d9a42c0bd861f95d75c396e271be218191e0d94eb3575d58fac26b27f2",
        "Created": "2022-04-06T09:39:48.330069568Z",
        "Path": "/docker-entrypoint.sh",
        "Args": [
            "nginx",
            "-g",
            "daemon off;"
        ],
        "State": {
            "Status": "running",
...
            "MacAddress": "02:42:ac:11:00:02",
            "Networks": {
                "bridge": {
                    "IPAMConfig": null,
                    "Links": null,
                    "Aliases": null,
                    "NetworkID": "bd938a276106bfa3b7b791976dbc16b6f88520ae7c712ed39b1e3c176a59450f",
                    "EndpointID": "18d9f5f42293017de4db6105b8609703ed9a235ec2fba11486ce9540e9794010",
                    "Gateway": "172.17.0.1",
                    "IPAddress": "172.17.0.2",
                    "IPPrefixLen": 16,
                    "IPv6Gateway": "",
                    "GlobalIPv6Address": "",
                    "GlobalIPv6PrefixLen": 0,
                    "MacAddress": "02:42:ac:11:00:02",
                    "DriverOpts": null
                }
            }
        }
    }
]

@89luca89
Copy link
Contributor Author

89luca89 commented Apr 6, 2022

Docker works by default in the "all" style

Slightly different from the "all" style.

When both the container and the image exists, Docker only shows the container

$ docker run -d --name nginx nginx
58f268d9a42c0bd861f95d75c396e271be218191e0d94eb3575d58fac26b27f2

$ docker inspect nginx
[
    {
        "Id": "58f268d9a42c0bd861f95d75c396e271be218191e0d94eb3575d58fac26b27f2",
        "Created": "2022-04-06T09:39:48.330069568Z",
        "Path": "/docker-entrypoint.sh",
        "Args": [
            "nginx",
            "-g",
            "daemon off;"
        ],
        "State": {
            "Status": "running",
...
            "MacAddress": "02:42:ac:11:00:02",
            "Networks": {
                "bridge": {
                    "IPAMConfig": null,
                    "Links": null,
                    "Aliases": null,
                    "NetworkID": "bd938a276106bfa3b7b791976dbc16b6f88520ae7c712ed39b1e3c176a59450f",
                    "EndpointID": "18d9f5f42293017de4db6105b8609703ed9a235ec2fba11486ce9540e9794010",
                    "Gateway": "172.17.0.1",
                    "IPAddress": "172.17.0.2",
                    "IPPrefixLen": 16,
                    "IPv6Gateway": "",
                    "GlobalIPv6Address": "",
                    "GlobalIPv6PrefixLen": 0,
                    "MacAddress": "02:42:ac:11:00:02",
                    "DriverOpts": null
                }
            }
        }
    }
]

I see, I can revert to the "all" flag (maybe call it "any"?) and make it behave this way by default

@AkihiroSuda
Copy link
Member

I see, I can revert to the "all" flag (maybe call it "any"?) and make it behave this way by default

Should be just "" (empty)

@89luca89
Copy link
Contributor Author

89luca89 commented Apr 6, 2022

Ok implemented with empty string and inverted the logic in the last check, so that we prioritize containers over images

@AkihiroSuda
Copy link
Member

AkihiroSuda commented Apr 8, 2022

$ nerdctl -v
nerdctl version 0.18.0-17-g2d3de8c
$ nerdctl run -d --name nginx nginx
55185a099977d453b24e5197bae31cfb828a5e3ea5712ea0afef60b7df4a3bc1
$ nerdctl inspect nginx
FATA[0000] multiple IDs found with provided prefix: nginx 

OTOH, Docker shows the information of the container.

@AkihiroSuda AkihiroSuda removed this from the v0.19.0 (tentative) milestone Apr 8, 2022
Right now nerdctl inspect does not have a --type flag similar to
docker and podman.

This PR aims to add this, allowing for the basic values:

- container
- image

It will default to container to match docker's behaviour

Example of the command:

```sh
./nerdctl inspect --format '{{json .}}' --type image registry.fedoraproject.org/fedora-toolbox:35
./nerdctl inspect --format '{{json .}}' --type container fedora-toolbox-35
```

Signed-off-by: Luca Di Maio <luca.dimaio1@gmail.com>
@89luca89
Copy link
Contributor Author

89luca89 commented Apr 8, 2022

$ nerdctl -v
nerdctl version 0.18.0-17-g2d3de8c
$ nerdctl run -d --name nginx nginx
55185a099977d453b24e5197bae31cfb828a5e3ea5712ea0afef60b7df4a3bc1
$ nerdctl inspect nginx
FATA[0000] multiple IDs found with provided prefix: nginx 

OTOH, Docker shows the information of the container.

mmh I see, this seems to stem from lines 114-116:

if ni != 0 && nc != 0 || nc > 1 {
    return fmt.Errorf("multiple IDs found with provided prefix: %s", req)
}

I've tested simply dropping them, as now we have a default (containers first, like in docker) and a way to disambiguate if needed (the type flag)

Also I've arranged the ifs in a way that we can simply have a hierarchy of defaults if in future we'll add more things to inspect (eg: container, image, network etc...)

Copy link
Member

@AkihiroSuda AkihiroSuda left a comment

Choose a reason for hiding this comment

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

Thanks

@AkihiroSuda AkihiroSuda merged commit c193c53 into containerd:master Apr 10, 2022
@AkihiroSuda AkihiroSuda added this to the v0.19.0 (tentative) milestone Apr 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants