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.

Sign up for GitHub

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

Jump to bottom

Report docker container labels and render them in the details pane #400

Merged
merged 3 commits into from
Aug 27, 2015
Merged

Report docker container labels and render them in the details pane #400

merged 3 commits into from
Aug 27, 2015

Conversation

2opremio
Copy link
Contributor

Addresses part of #372

@tomwilkie tomwilkie self-assigned this Aug 26, 2015
@tomwilkie
Copy link
Contributor

Failure is due to

./probe/docker/container.go:206:1: exported function ExtractContainerLabels should have comment or be unexported

tomwilkie
tomwilkie reviewed Aug 26, 2015
View reviewed changes
probe/docker/container.go
@@ -215,6 +223,7 @@ func (c *container) GetNodeMetadata() report.NodeMetadata {
ImageID: c.container.Image,
ContainerIPs: strings.Join(append(c.container.NetworkSettings.SecondaryIPAddresses,
c.container.NetworkSettings.IPAddress), " "),
ContainerLabels: string(labels),

This comment was marked as abuse.

Sign in to view

This comment was marked as abuse.

Sign in to view

This comment was marked as abuse.

Sign in to view

@tomwilkie
Copy link
Contributor

Also, docker images can have labels, as can the docker daemon. We should probably fetch these and attach them to the docker image and host topology. In which case, make the prefix docker_label_

@tomwilkie tomwilkie force-pushed the master branch 2 times, most recently from 66be8bd to d9ab9d8 Compare August 26, 2015 14:25
peterbourgon
peterbourgon reviewed Aug 26, 2015
View reviewed changes
probe/docker/container.go
@@ -245,6 +248,13 @@ func (c *container) GetNodeMetadata() report.NodeMetadata {
return result
}

// ExtractContainerLabels returns the list of Docker container labels given a NodeMetadata from the Container topology.
func ExtractContainerLabels(nmd report.NodeMetadata) map[string]string {
result := make(map[string]string)

This comment was marked as abuse.

Sign in to view

This comment was marked as abuse.

Sign in to view

@tomwilkie tomwilkie assigned 2opremio and unassigned tomwilkie Aug 26, 2015
@2opremio
Copy link
Contributor Author

@tomwilkie Unfortunately the fsouza Docker client doesn't support Docker daemon labels yet: see the message from my last commit

@2opremio 2opremio assigned tomwilkie and unassigned 2opremio Aug 26, 2015
tomwilkie
tomwilkie reviewed Aug 26, 2015
View reviewed changes
probe/docker/container.go
ContainerPorts = "docker_container_ports"
ContainerCreated = "docker_container_created"
ContainerIPs = "docker_container_ips"
ContainerLabelPrefix = "docker_container_label_"

This comment was marked as abuse.

Sign in to view

This comment was marked as abuse.

Sign in to view

This comment was marked as abuse.

Sign in to view

This comment was marked as abuse.

Sign in to view

@tomwilkie tomwilkie assigned 2opremio and unassigned tomwilkie Aug 26, 2015
@tomwilkie
Copy link
Contributor

Assign it back to me when the tests pass.

Report/render both image and container labels
9ba3740 
Daemon labels will have to wait since the go client (Docker API v1.14) doesn't support
them yet (daemon labels were exposed in Docker API v1.16)

See https://godoc.org/github.com/fsouza/go-dockerclient#Client.Info for details.
@2opremio 2opremio assigned tomwilkie and unassigned 2opremio Aug 27, 2015
@tomwilkie
Copy link
Contributor

LGTM

@tomwilkie tomwilkie assigned 2opremio and unassigned tomwilkie Aug 27, 2015
2opremio pushed a commit that referenced this pull request Aug 27, 2015
Merge pull request #400 from weaveworks/docker-labels
4db8010 
Report docker container labels and render them in the details pane
@2opremio 2opremio merged commit 4db8010 into master Aug 27, 2015
@2opremio 2opremio deleted the docker-labels branch August 27, 2015 13:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@2opremio 2opremio

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@2opremio @tomwilkie @peterbourgon