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

Proposal: Labels for non-container resources #20356

Closed
ehazlett opened this issue Feb 16, 2016 · 23 comments
Closed

Proposal: Labels for non-container resources #20356

ehazlett opened this issue Feb 16, 2016 · 23 comments
Assignees
Labels
kind/feature Functionality or other elements that the project doesn't currently have. Features are new and shiny roadmap
Milestone

Comments

@ehazlett
Copy link
Contributor

I would like to propose having labels for non-container resources. To start with, these would include:

  • Networks
  • Volumes
  • Images (post build)?

For networks and volumes the same syntax as containers could be used. For example:

docker volume create -d local --label env=staging --label backup=1 redis-data

docker network create -d overlay --label env=staging --label app=test-app-v0 backend

For images, it would be best to have something like #18958 to add labels. However, update support / image labels might be out of scope for this proposal.

@thaJeztah thaJeztah added the kind/feature Functionality or other elements that the project doesn't currently have. Features are new and shiny label Feb 16, 2016
@thaJeztah
Copy link
Member

Note that a feature request for adding labels to volumes already exists in #17936

@ehazlett
Copy link
Contributor Author

ah thx @thaJeztah

@thaJeztah
Copy link
Member

I'm +1 for feature parity between resources, so sgtm.

w.r.t. image-labels post/during build; there's some earlier discussion on that in #14144 and #13178

@icecrime
Copy link
Contributor

That's something we'd like for Engine 1.11 (cc @tiborvass).

@icecrime icecrime added this to the 1.11.0 milestone Feb 16, 2016
@stevvooe
Copy link
Contributor

I like the idea of having labels across several objects but we need to be careful to restrict the complexity. Specifically, there are number of problems with updates when labels become transitive. For example, if one starts a container from an image, should the container pick up the image labels? Following, should an image snapshot from a container pick up the container labels? In both cases, we may get incorrect or invalid information.

@cpuguy83
Copy link
Member

To top that off we don't even currently store volume state locally... and also how does this affect objects which the engine doesn't specifically own (like multi-host networks, multi-host volumes).

@thaJeztah
Copy link
Member

if one starts a container from an image, should the container pick up the image labels? Following, should an image snapshot from a container pick up the container labels? In both cases, we may get incorrect or invalid information.

@stevvooe I mentioned some thoughts on that here; #18958 (comment)

@stevvooe
Copy link
Contributor

@thaJeztah I was more talking about the relationship with container and image labels, rather than updating them. I'm a little confused as to how that relates.

Complicated schemes here won't help anyone. Images should not "inherit" container labels and vice versa. Without a full analysis of how transitivity will work and update cycle protection, it will be a sloppy mess of bugs.

@dnephin
Copy link
Member

dnephin commented Feb 24, 2016

Compose would benefit from all of these labels, so +1

@simonvanderveldt
Copy link

Setting labels for images during build time without workarounds which include changing Dockerfiles would really be nice.
It's been mentioned in #13178 for example, but this is a common use case for CI where you want to add the commit hash or build number to an image during build.

Also labels during build time are relevant for builds done using Compose as mentioned by @dnephin above me.
See docker/compose#1403 for the original issue about that.

@tiborvass
Copy link
Contributor

EDIT: No need to read this if you don't care about updating labels, which is out of scope for this issue

One requirement for this feature to be useful is to allow updating the labels.

Before going into technical details, I would like to find a good UX. I could come up with these 3 solutions, none of which I really like, but I prefer the 3rd one among these:

  1. For each new object type (images, networks, volumes), add a new update subcommand to match the docker update that is for containers. Questions: is there anything else we would want to update on images networks and volumes? If so what? If we can't agree on those other things to update, then the problem with this solution is we would add a new subcommand just for labels. Moreover, for historical reasons images is not a subcommand the way network and volume are. So if we were to still go with this solution we should also choose between: 1.a) introduce docker images update subcommand OR 1.b) overload docker update the way we (unfortunately) overload docker inspect and accept: docker update --label foo=bar $imageID.
  2. Overload docker update to non-container objects as well. Problem: how to deal with docker update --memory 2G $networkID ? obviously it should error out not recognizing the memory flag for updating network objects, but we would have the same problem docker machine is having with all the cloud-provider-specific flags and it's not ideal for the help output. Also, if we are overloading docker update for non-container objects, why not docker create and more ? Since this solution is a generalization of 1.b, 1.b also suffers of this problem. Both solutions 1 and 2 would result in accepting Proposal: Add label support to update command #18958.
  3. Introduce docker label command that can act on any object. Problem: we already have docker update that updates the container settings (cgroups only for now) and this would be another toplevel command that would also update container settings, albeit only one setting: the labels. As much as I dislike having two commands do similar things, a counter-argument is to say: labels are special because they are meta. Obviously this would mean, the UX in Proposal: Add label support to update command #18958 would be rejected.

If you can think of another solution that would have none of these UX issues, I would be glad to read about it.

Otherwise, feel free to vote for one of those solutions. I'm voting 3.

I deliberately didn't discuss what the docker label UX would look like (--get/--set ?) because that is an orthogonal UX issue we can agree on once we know whether we want to do solution 3.

Once we get agreement on the UX, we can discuss the implementation details.

@tiborvass
Copy link
Contributor

Forgot to say that, that UX proposal should cover the usecases mentioned here.
But just to be sure I understood @simonvanderveldt: would it work for you if you to use docker build followed by docker label to set the label on the image you just built? I assume yes, but I would like to be sure.

@ehazlett
Copy link
Contributor Author

ehazlett commented Mar 9, 2016

FWIW I also like 3. I think trying to force it into the other commands wouldn't be ideal. The first class "label" concept also gives us more flexibility down the road.

@dnephin
Copy link
Member

dnephin commented Mar 9, 2016

@tiborvass I'm confused. Why are these the options? What's wrong with the option from the OP :

docker volume create --label env=staging ...
docker network create -d overlay --label env=staging ...

Making labels mutable seems like it's an additional feature that can be added later. Having immutable labels is significantly more valuable. The --label flag would match container labels as well.

(personally I don't think there are any strong use cases for #18958 and that having labels for all resources is a lot more important than being able to edit a label on an immutable resource.)

@tiborvass
Copy link
Contributor

Thanks @dnephin for making me realize I forgot an important point in my reasoning: your solution works well, IF you don't need to update labels. As soon as you need to update them, it doesn't work.

Now a 3.b) solution could be: accept 3) but add some "sugar" to allow for creating labels the way you described it. Even on docker build then. That's a possibility we can look into, but it relies on still having a way of updating SOMEHOW the labels. And that best "somehow" right now is 3, imho.

Does that make more sense?

@dnephin
Copy link
Member

dnephin commented Mar 9, 2016

I'm still missing why "update" is required for this at all. Why can't we just add labels that are immutable, which will work for 95% of use cases, and worry about the 5% that need label updates later (or decide we don't need to update them).

We've had immutable container labels for around a year, and they've been extremely valuable without any update command.

Maybe it would help to separate the idea of post-build image labels and volume/network labels, since these act very differently. volume/network labels seem pretty straightfoward.

post-build image labels are going to require changes to the distribution metadata, won't they? Unless we're talking about post-build labels that don't transfer with the image on push, which doesn't seem very useful.

@ehazlett
Copy link
Contributor Author

ehazlett commented Mar 9, 2016

@dnephin totally valid point. How about we keep update out of scope for this one and just add the create labels? Does that sound reasonable?

@justincormack
Copy link
Contributor

Yes, create only would be fine.

@duglin
Copy link
Contributor

duglin commented Mar 9, 2016

Only supporting create for now is a valid approach as long as we know what the update will look like when we do support it and it won't case us to revisit the create step because we know people will want.

I think @dnephin's approach for create (docker network create ... --label foo=bar) is good, and for update I think consistency with containers (docker network update --label foo=zoo) works.

@tiborvass
Copy link
Contributor

@dnephin yeah sorry, I had some offline conversations with @ehazlett and it was my (mis)understanding that update was a requirement for this.

It makes perfect sense to me to just do volume create --label, network create --label, build --label for 1.11 and continue the discussion about update in another issue. However, this would also mean, we prioritize extending labels to non-container objects, above allowing labels to be updated. Which would mean we should probably freeze (i.e. close for now) #18958 .

I do wonder about load and commit though, do they need --label ? commit has --change "LABEL foo=bar" but its weird.

@tiborvass
Copy link
Contributor

@duglin except that docker network update would be a new subcommand for network. See solution 1 in my big comment above: #20356 (comment)

But again, we can discuss that for another issue.

@duglin
Copy link
Contributor

duglin commented Mar 9, 2016

@tiborvass yup saw that but I still think its the right way to go - and yea we can discuss more later. But, in case you didn't see it: #13509 - an oldie.

@thaJeztah
Copy link
Member

I think adding the --label option to those commands would work well from a UX perspective, so +1

But, in case you didn't see it: #13509 - an oldie.

I still like that one, and would definitely be +1 (but probably a separate discussion), same for containers

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature Functionality or other elements that the project doesn't currently have. Features are new and shiny roadmap
Projects
None yet
Development

No branches or pull requests

10 participants