Skip to content

Allow docker inspect on all types#23614

Merged
icecrime merged 1 commit intomoby:masterfrom
icecrime:inspect_services
Aug 26, 2016
Merged

Allow docker inspect on all types#23614
icecrime merged 1 commit intomoby:masterfrom
icecrime:inspect_services

Conversation

@icecrime
Copy link
Copy Markdown
Contributor

Allow top-level docker inspect command to inspect any kind of resources. The rationale is that inspect already does images and containers for historical reasons, so it might as well support them all. More specialized inspects (e.g., docker network inspect) should exist to support type specific options.

Depends on docker-archive-public/docker.engine-api#283.

@icecrime icecrime changed the title Inspect services Allow docker inspect on all types Jun 16, 2016
@icecrime
Copy link
Copy Markdown
Contributor Author

I should add that this request comes straight from our BDFL.

@icecrime icecrime added this to the 1.12.0 milestone Jun 16, 2016
@vdemeester
Copy link
Copy Markdown
Member

Questions :

  • Node and Swarm have inspects too, shouldn't they be here too ?
  • InspectAll (a.k.a without --type=…) is gonna have a weird behavior now : if you don't give the type, it will only look for containers, and images. Maybe it should go through all of them (with an order to decide). wdyt ?

@justincormack
Copy link
Copy Markdown
Contributor

Makes sense to me.

@icecrime
Copy link
Copy Markdown
Contributor Author

@vdemeester True! Let me update this sometime today :-) Thanks

@vieux
Copy link
Copy Markdown
Contributor

vieux commented Jun 16, 2016

@icecrime I think you can update engine-api to fix vendor (good luck)

@vieux
Copy link
Copy Markdown
Contributor

vieux commented Jun 16, 2016

@icecrime @vdemeester I won't do Swarm for now, only Node.

@icecrime icecrime force-pushed the inspect_services branch 2 times, most recently from f4f6232 to 22bd0a0 Compare June 16, 2016 23:23
@icecrime
Copy link
Copy Markdown
Contributor Author

icecrime commented Jun 16, 2016

Refactored the code and made it available to inspect all (but nodes!): still requires docker-archive-public/docker.engine-api#284 to work properly, as cascading between types is based off the ability to identify not found errors.

@icecrime icecrime force-pushed the inspect_services branch 2 times, most recently from 308c5ae to bc9bc55 Compare June 16, 2016 23:58
@vieux
Copy link
Copy Markdown
Contributor

vieux commented Jun 17, 2016

LGTM

@thaJeztah
Copy link
Copy Markdown
Member

@icecrime needs a rebase

@icecrime
Copy link
Copy Markdown
Contributor Author

Rebased and hopefully good now.

@thaJeztah
Copy link
Copy Markdown
Member

@icecrime tests are failing 😢

@thaJeztah thaJeztah added the status/failing-ci Indicates that the PR in its current state fails the test suite label Jun 17, 2016
@icecrime
Copy link
Copy Markdown
Contributor Author

Thank you all <3

@icecrime icecrime merged commit 6e85b81 into moby:master Aug 26, 2016
@thaJeztah thaJeztah added this to the 1.13.0 milestone Sep 12, 2016
albers added a commit to albers/moby that referenced this pull request Oct 4, 2016
services

This enhances completion of networks, volumes, plugins, runtimes
and local interfaces so that they are consistent with the principles
introduced in the completions of nodes and services.

This serves as a preparation for implementing bash completion for
the enhanced functionality of docker inspeckt (moby#23614).

Signed-off-by: Harald Albers <[email protected]>
albers added a commit to albers/moby that referenced this pull request Oct 25, 2016
…rameterized function

In moby#23614 `docker inspect` was semantically enhanced to inspect "everything".
Therefore moving its logic to `_docker_container_inspect` was not correct.

This commit moves it back to its original top-level location (`_docker_inspect`)
so that it can be called by `_docker_{container,image}_inspect` and others (will
be added in follow-up PRs).
Parameterization was added in order to get caller-specific behavior.

Signed-off-by: Harald Albers <[email protected]>
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.