LGTM

Thanks @icecrime, in hindsight, I should've been more persistent asking about the desired behavior #9882 (comment). Guess the discussion just got too long and I forgot. 😊

ping @icecrime @jfrazelle @duglin and @vieux (for API)

Gave this some more thought, and apologies upfront for being a PITA.

If I understand correctly, this will be a backwards incompatible change and, although I haven't checked if this behavior is actually documented, users will be relying on the current behavior, so we need something to address that. (Envisioning a range of bugs reported by @ibuildthecloud once this change lands in a release);

• Which release will this be in? I don't see a milestone, so I'm guessing 1.8? I think there's plans (still under discussion) that libnetwork will be using labels. From that perspective, and to limit the amount of users relying on the old behavior, I think it would make sense to get this in a release ASAP (perhaps even 1.7 if there will be a 1.7.0-rc3)
• To preserve backward-compatibility; can we keep the old behavior for API versions <= v1.xx (performing this merging inside the API?)
• Check if there are existing examples of descriptions in the documentation describing the old behavior.
• Add a note to the release notes and/or the API docs to document this change?

If any of these points are incorrect or don't make sense, please comment, just want to make sure we don't break existing setups.

Oh this is a terrible change, why???? This breaks me. I use container labels extensively to indicate attributes about the container. By having the labels come from the image I can have images automatically contribute attributes to the container.

For example, I can tell a user to add the label "make-awesome" and in Rancher it will do something cool with that container. But the user is free to put the label on the image and now all of their containers will be made awesome automatically.

This change breaks that behavior. If you want to separate the two then we can add something like CONTAINER_LABEL in the dockerfile, but I honestly don't see a lot of value beyond the theoretical. But please revert this PR until we have a sufficient solution.

Also, this wasn't an oversight on my part, the behavior was quite intentional, but maybe we didn't discuss this enough.

@rhatdan do you have any thoughts on this one?

I also take it for granted that they layers get inherited unless overwritten. I don't believe their is a good solution to this. I believe the president has been set of other fields in layers.

We are telling our vendors who are building content on our images to modify some of the LABELS, that end up being clearly wrong. Vendor, Name, Version, Release for example. But I do not believe that if i examine a container that is based on rhel7 should n ot show that fact in docker inspect.

So I would vote to veto this change.

@ibuildthecloud couldn't you achieve exactly the same thing with a docker label command against the containers of your choice?

It seems weird to me that the author of an image can have such a direct influence on host-specific behavior.

I can see security risks caused by conflating image and container lables, too. For example, let's assume a certain number of Docker deployments are configured to pick up the container label admin, and confer additional privileges to the corresponding containers. What prevents a malicious author from labeling their image with admin, and hope that one of those deployments runs it?

I support @icecrime here, I think it's better to keep container labels and image labels clearly separate, to keep a sane and intuitive data model.

However I don't see any reason why a user couldn't query containers by various properties of the image which it came from - including image labels. But at least it would be a conscious decision by the user to tie behavior on their installation to potentially untrusted or broken image properties.

I agree container labels and image labels should probably be different.

Also, I'd like to point out that it should be simple enough to do the "extra indirection" of grabbing the image name from a running container and inspect its labels.

@shykes @icecrime I get the motivation here. I know this breaks my code, but I can adapt, no big deal. Are we concerned with introducing a breaking change here?

@ibuildthecloud @icecrime @shykes wrt breaking backward compatibility and API stability (it is versioned for a reason), I think my comment #13772 (comment) still contains some things that need to be addressed