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

Container labels are merged into the container image node when rendering topology #815

Closed
paulbellamy opened this issue Jan 13, 2016 · 9 comments · Fixed by #1371
Closed
Assignees
Labels
bug Broken end user or developer functionality; not working as the developers intended it
Milestone

Comments

@paulbellamy
Copy link
Contributor

$ docker run -dit --name alpine1 alpine /bin/sh
$ docker run -dit --name alpine2 --label=works.weave.role=system alpine /bin/sh

Then go to container-by-image, and you will see that the "alpine" container image is considered a system image, because all metadata (including labels) has been merged up from the container nodes.

@peterbourgon
Copy link
Contributor

But from the perspective of the container-by-image view, the alpine image is a system image, in the sense that containers using it are system containers. The alternative is to — split images on the axis of classification?

@paulbellamy
Copy link
Contributor Author

... in the sense that containers using it are system containers.

If all of the containers using it were system containers, I could see that, but in this case they aren't.

Container Images should only be "system" if they themselves have that label.

@tomwilkie
Copy link
Contributor

Container Images should only be "system" if they themselves have that label.

+1

The logic for merging the image labels should be being inherited by this rendered, so this is a structural bug in the code. Easy to fix.

@paulbellamy paulbellamy mentioned this issue Jan 13, 2016
18 tasks
@paulbellamy
Copy link
Contributor Author

Actually, the whole determination of whether a container image is "stopped" or "running" is just a random selection from one container.

@tomwilkie
Copy link
Contributor

Yeah we should remove those filters from that view, they make little sense.

@paulbellamy
Copy link
Contributor Author

Stopped makes sense to me, as it's sort of "show only images in use by >= 1 running containers"

@tomwilkie
Copy link
Contributor

I don't mind either way.

@paulbellamy paulbellamy added the bug Broken end user or developer functionality; not working as the developers intended it label Apr 4, 2016
@rade
Copy link
Member

rade commented Apr 4, 2016

Here's how I ran into this...

$ docker run --name=foo -ti alpine /bin/true
$ docker run --name=bar -dti alpine /bin/sh

which gives me one stopped and one running container.

When showing 'By Image' and 'stopped' or 'running', a) the count of the image node is 2, when it should only be 1, and b) the node occasionally vanishes (Paul reckons that depends on which branch "won" the merge).

@tomwilkie
Copy link
Contributor

For pods/services, we use a different prefix for their labels (to prevent this). Could do the same for containers & images.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Broken end user or developer functionality; not working as the developers intended it
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants