Skip to content

Proposal: add --label to docker ps#11943

Closed
vieux wants to merge 1 commit intomoby:masterfrom
vieux:labels_ps
Closed

Proposal: add --label to docker ps#11943
vieux wants to merge 1 commit intomoby:masterfrom
vieux:labels_ps

Conversation

@vieux
Copy link
Contributor

@vieux vieux commented Mar 31, 2015

I didn't write the documentation yet, because I'd like to know what you think beforehand.

I'd like to add a --label to docker ps so if you do

docker run busybox true
docker run --label storage=ssd busybox true
docker ps --label storage

you get

CONTAINER ID        IMAGE               COMMAND                CREATED             STATUS                      PORTS               NAMES               STORAGE
8abd15ea641f        busybox:latest      "true"                 33 minutes ago      Exited (0) 33 minutes ago                       kickass_pare        ssd
6dd6e51818be        busybox:latest      "true"                 33 minutes ago      Exited (0) 33 minutes ago                       suspicious_bardeen

Everything could be done client side, but I decided to go through the daemon, so, it the future the daemon could add mandatory labels to display. (key Display in the JSON)

If this PR gets merged, we are going to use this feature in Swarm to add a mandatory label node, so docker ps will output:

CONTAINER ID        IMAGE               COMMAND             CREATED              STATUS                          PORTS               NAMES          NODE
24f6565be282        busybox:latest      "ls"                19 seconds ago                                                           berserk_bell   ubuntu-1
481f2a3d6a74        busybox:latest      "ls"                About a minute ago   Exited (0) About a minute ago                       insane_fermi   ubuntu-2

@vieux
add --label to docker ps
89698b3 
Signed-off-by: Victor Vieux <[email protected]>
@vieux
Copy link
Contributor Author

vieux commented Mar 31, 2015

ping @abronan @aluzzardi

@jessfraz
Copy link
Contributor

jessfraz commented Mar 31, 2015

what about --filter label=storage

@jessfraz
Copy link
Contributor

jessfraz commented Mar 31, 2015

or someway to reuse that flag, unless you really think we need another which i guess we kinda do because having two equals is kinda weird

in the case of --filter label=storage=ssd or something

@abronan
Copy link
Contributor

abronan commented Mar 31, 2015

@jfrazelle Agreed I like the --filter better in order to stay consistent with what already exists.

@vieux
Copy link
Contributor Author

vieux commented Mar 31, 2015

@jfrazelle I don't think filtering and displaying a new column are the same, if we use
--filter label=storage then the column is useless (it will be storage everywhere)

@vieux
Copy link
Contributor Author

vieux commented Mar 31, 2015

@jfrazelle sorry I misread what you wrote

@vieux
Copy link
Contributor Author

vieux commented Mar 31, 2015

@jfrazelle yes we could use --filter label=storage --filter label=region

@thaJeztah
Copy link
Member

thaJeztah commented Mar 31, 2015

+1 for --filter to keep things consistent. Bit harder to write, but that seems like a small price to pay.

Also a big +1 for doing this on the daemon; Docker Compose can probably benefit from that as well

@vieux
Copy link
Contributor Author

vieux commented Mar 31, 2015

The thing is, I'm not super confident about using --filter to not filter anything, just to display a new column.

@thaJeztah
Copy link
Member

thaJeztah commented Mar 31, 2015

The thing is, I'm not super confident about using --filter to not filter anything, just to display a new column.

Looks like I'm making a fool of myself here; I was misreading your intent, and got confused in the discussion; we already have filtering on label; this is for displaying.

/me grabs the hat of eternal shame (still have it lying around somewhere) 😊

@thaJeztah
Copy link
Member

thaJeztah commented Apr 1, 2015

@vieux you might want to look at this proposal as well #10255

@thaJeztah thaJeztah mentioned this pull request Apr 1, 2015
Closed
@vieux
Copy link
Contributor Author

vieux commented Apr 6, 2015

@icecrime @tiborvass @jfrazelle Any update on this ? I still believe --filter should only do filtering and not "display" stuff

@duglin
Copy link
Contributor

duglin commented Apr 7, 2015

I'm having a hard time following the indent of this issue.
@vieux what does docker ps --label=foo do ? Show a new column called 'label' and only show the containers that have a label of 'foo' set to something? Or does it just show a new 'label' column? or do it just show a new 'foo' column? Did you want it to do any filtering at all?

I'm guessing you don't want it to filter, rather you JUST want a new column for ALL containers. And that new column shows each container's value for the label "foo". And if it has no label then its blank. Is that correct?

@abronan
Copy link
Contributor

abronan commented Apr 7, 2015

@duglin Yes it's correct. Besides this, we are discussing whether we should use --label as a flag for the column display part only or use the existing --filter to output this new column.

@vieux with a fresher mind after reading the PR again, I think we should use a new flag anyway. The notion of displaying a new column must not be entangled with the filtering of containers output. And using the --filter seems weird from a user experience standpoint in the context of displaying a new column.

@duglin
Copy link
Contributor

duglin commented Apr 7, 2015

@vieux how do you guarantee that a user-defined label isn't going to conflict with a swarm defined label? Or that a user doesn't change the value on you? It feels a bit like there needs to be either a) clear rules for how docker defined extensions (labels) are defined vs user-defined ones to avoid conflicts, or b) a docker/swarm managed set of extension properties that users can't touch.

@thaJeztah
Copy link
Member

thaJeztah commented Apr 7, 2015

how do you guarantee that a user-defined label isn't going to conflict with a swarm defined label

Aren't namespaces meant for that? i.e. com.docker.swarm.storage=ssd. Since swarm wraps the Docker API, it could automatically prepend/strip the namespace.

@duglin
Copy link
Contributor

duglin commented Apr 7, 2015

yes. I was just looking at the example he provided in the first comment ("storage") and didn't realize it was a short-hand for something like com.docker.swarm.storage. Should make for one heck of a column title :-)

@vieux
Copy link
Contributor Author

vieux commented Apr 7, 2015

@duglin this is more of a swarm design problem than docker, but we could also refuse to schedule containers if they already have a storage label for example

@duglin
Copy link
Contributor

duglin commented Apr 17, 2015

needs a rebase

@dnephin
Copy link
Member

dnephin commented Apr 26, 2015

Compose would benefit from this as well. If certain labels could be exposed through the list containers API call, it would remove the need to inspect every container that was returned to get the labels.

@thaJeztah thaJeztah mentioned this pull request Apr 27, 2015
Merged
@calavera calavera mentioned this pull request May 1, 2015
Closed
3 tasks
@tiborvass
Copy link
Contributor

tiborvass commented May 4, 2015

Closing in favor of #12931

@tiborvass tiborvass closed this May 4, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

status/1-design-review

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

8 participants

@vieux @jessfraz @abronan @thaJeztah @duglin @dnephin @tiborvass @GordonTheTurtle