Use image name without version as id. (#1531)

* Use image name without version as id.

* Review feedback

probe/docker/registry.go

@@ -43,6 +43,7 @@ type Registry interface {
 	WatchContainerUpdates(ContainerUpdateWatcher)
 	GetContainer(string) (Container, bool)
 	GetContainerByPrefix(string) (Container, bool)
+	GetContainerImage(string) (*docker_client.APIImages, bool)
 }
 
 // ContainerUpdateWatcher is the type of functions that get called when containers are updated.
@@ -390,6 +391,13 @@ func (r *registry) GetContainerByPrefix(prefix string) (Container, bool) {
 	return nil, false
 }
 
+func (r *registry) GetContainerImage(id string) (*docker_client.APIImages, bool) {
+	r.RLock()
+	defer r.RUnlock()
+	image, ok := r.images[id]
+	return image, ok
+}
+
 // WalkImages runs f on every image of running containers the registry
 // knows of.  f may be run on the same image more than once.
 func (r *registry) WalkImages(f func(*docker_client.APIImages)) {

probe/docker/reporter.go

@@ -38,7 +38,7 @@ var (
 	}
 
 	ContainerImageMetadataTemplates = report.MetadataTemplates{
-		ImageID:          {ID: ImageID, Label: "Image ID", From: report.FromLatest, Truncate: 12, Priority: 1},
+		ImageID:          {ID: ImageID, Label: "Image ID", From: report.FromSets, Truncate: 12, Priority: 1},
 		report.Container: {ID: report.Container, Label: "# Containers", From: report.FromCounters, Datatype: "number", Priority: 2},
 	}
 

probe/docker/reporter_test.go

@@ -40,6 +40,11 @@ func (r *mockRegistry) GetContainer(_ string) (docker.Container, bool) { return
 
 func (r *mockRegistry) GetContainerByPrefix(_ string) (docker.Container, bool) { return nil, false }
 
+func (r *mockRegistry) GetContainerImage(id string) (*client.APIImages, bool) {
+	image, ok := r.images[id]
+	return image, ok
+}
+
 var (
 	mockRegistryInstance = &mockRegistry{
 		containersByPID: map[int]docker.Container{

probe/docker/tagger.go

@@ -82,12 +82,21 @@ func (t *Tagger) tag(tree process.Tree, topology *report.Topology) {
 			continue
 		}
 
-		topology.AddNode(report.MakeNodeWith(nodeID, map[string]string{
+		node := report.MakeNodeWith(nodeID, map[string]string{
 			ContainerID: c.ID(),
 		}).WithParents(report.EmptySets.
-			Add(report.Container, report.MakeStringSet(report.MakeContainerNodeID(c.ID()))).
-			Add(report.ContainerImage, report.MakeStringSet(report.MakeContainerImageNodeID(c.Image()))),
-		))
+			Add(report.Container, report.MakeStringSet(report.MakeContainerNodeID(c.ID()))),
+		)
 
+		// If we can work out the image name, add a parent tag for it
+		image, ok := t.registry.GetContainerImage(c.Image())
+		if ok && len(image.RepoTags) > 0 {
+			imageName := ImageNameWithoutVersion(image.RepoTags[0])
+			node = node.WithParents(report.EmptySets.
+				Add(report.ContainerImage, report.MakeStringSet(report.MakeContainerImageNodeID(imageName))),
+			)
+		}
+
+		topology.AddNode(node)
 	}
 }

probe/docker/tagger_test.go

@@ -70,8 +70,8 @@ func TestTagger(t *testing.T) {
 		}
 
 		// node should have the container image as a parent
-		if have, ok := node.Parents.Lookup(report.ContainerImage); !ok || !have.Contains(report.MakeContainerImageNodeID("baz")) {
-			t.Errorf("Expected process node %s to have container image %q as a parent, got %q", nodeID, "baz", have)
+		if have, ok := node.Parents.Lookup(report.ContainerImage); !ok || !have.Contains(report.MakeContainerImageNodeID("bang")) {
+			t.Errorf("Expected process node %s to have container image %q as a parent, got %q", nodeID, "bang", have)
 		}
 	}
 }

render/container.go

@@ -79,9 +79,19 @@ func (r containerWithImageNameRenderer) Render(rpt report.Report, dct Decorator)
 		if !ok {
 			continue
 		}
+		imageName, ok := image.Latest.Lookup(docker.ImageName)
+		if !ok {
+			continue
+		}
+		imageNameWithoutVersion := docker.ImageNameWithoutVersion(imageName)
+		imageNodeID := report.MakeContainerImageNodeID(imageNameWithoutVersion)
+
 		output := c.Copy()
 		output = propagateLatest(docker.ImageName, image, output)
 		output = propagateLatest(docker.ImageLabelPrefix+"works.weave.role", image, output)
+		output.Parents = output.Parents.
+			Delete(report.ContainerImage).
+			Add(report.ContainerImage, report.MakeStringSet(imageNodeID))
 		outputs[id] = output
 	}
 	return outputs
@@ -94,12 +104,15 @@ var ContainerWithImageNameRenderer = ApplyDecorators(containerWithImageNameRende
 // ContainerImageRenderer is a Renderer which produces a renderable container
 // image graph by merging the container graph and the container image topology.
 var ContainerImageRenderer = FilterEmpty(report.Container,
-	MakeReduce(
-		MakeMap(
-			MapContainer2ContainerImage,
-			ContainerWithImageNameRenderer,
+	MakeMap(
+		MapContainerImage2Name,
+		MakeReduce(
+			MakeMap(
+				MapContainer2ContainerImage,
+				ContainerWithImageNameRenderer,
+			),
+			SelectContainerImage,
 		),
-		SelectContainerImage,
 	),
 )
 
@@ -305,6 +318,29 @@ func MapContainer2ContainerImage(n report.Node, _ report.Networks) report.Nodes
 	return report.Nodes{id: result}
 }
 
+// MapContainerImage2Name ignores image versions
+func MapContainerImage2Name(n report.Node, _ report.Networks) report.Nodes {
+	// Propagate all pseudo nodes
+	if n.Topology == Pseudo {
+		return report.Nodes{n.ID: n}
+	}
+
+	imageName, ok := n.Latest.Lookup(docker.ImageName)
+	if !ok {
+		return report.Nodes{}
+	}
+
+	imageNameWithoutVersion := docker.ImageNameWithoutVersion(imageName)
+	output := n.Copy()
+	output.ID = report.MakeContainerImageNodeID(imageNameWithoutVersion)
+
+	if imageID, ok := report.ParseContainerImageNodeID(n.ID); ok {
+		output.Sets = output.Sets.Add(docker.ImageID, report.EmptyStringSet.Add(imageID))
+	}
+
+	return report.Nodes{output.ID: output}
+}
+
 // MapContainer2Hostname maps container Nodes to 'hostname' renderabled nodes..
 func MapContainer2Hostname(n report.Node, _ report.Networks) report.Nodes {
 	// Propagate all pseudo nodes

render/detailed/node_test.go

@@ -10,6 +10,7 @@ import (
 	"github.com/weaveworks/scope/probe/process"
 	"github.com/weaveworks/scope/render"
 	"github.com/weaveworks/scope/render/detailed"
+	"github.com/weaveworks/scope/render/expected"
 	"github.com/weaveworks/scope/report"
 	"github.com/weaveworks/scope/test"
 	"github.com/weaveworks/scope/test/fixture"
@@ -29,7 +30,7 @@ func TestMakeDetailedHostNode(t *testing.T) {
 	renderableNode := renderableNodes[fixture.ClientHostNodeID]
 	have := detailed.MakeNode("hosts", fixture.Report, renderableNodes, renderableNode)
 
-	containerImageNodeSummary := child(t, render.ContainerImageRenderer, fixture.ClientContainerImageNodeID)
+	containerImageNodeSummary := child(t, render.ContainerImageRenderer, expected.ClientContainerImageNodeID)
 	containerNodeSummary := child(t, render.ContainerRenderer, fixture.ClientContainerNodeID)
 	process1NodeSummary := child(t, render.ProcessRenderer, fixture.ClientProcess1NodeID)
 	process1NodeSummary.Linkable = true
@@ -176,7 +177,7 @@ func TestMakeDetailedHostNode(t *testing.T) {
 
 func TestMakeDetailedContainerNode(t *testing.T) {
 	id := fixture.ServerContainerNodeID
-	renderableNodes := render.ContainerRenderer.Render(fixture.Report, render.FilterNoop)
+	renderableNodes := render.ContainerWithImageNameRenderer.Render(fixture.Report, render.FilterNoop)
 	renderableNode, ok := renderableNodes[id]
 	if !ok {
 		t.Fatalf("Node not found: %s", id)
@@ -190,6 +191,7 @@ func TestMakeDetailedContainerNode(t *testing.T) {
 			ID:         id,
 			Label:      "server",
 			LabelMinor: "server.hostname.com",
+			Rank:       fixture.ServerContainerImageName,
 			Shape:      "hexagon",
 			Linkable:   true,
 			Pseudo:     false,
@@ -232,7 +234,7 @@ func TestMakeDetailedContainerNode(t *testing.T) {
 		},
 		Parents: []detailed.Parent{
 			{
-				ID:         fixture.ServerContainerImageNodeID,
+				ID:         expected.ServerContainerImageNodeID,
 				Label:      fixture.ServerContainerImageName,
 				TopologyID: "containers-by-image",
 			},

render/detailed/parents.go

@@ -3,7 +3,6 @@ package detailed
 import (
 	"sort"
 
-	"github.com/weaveworks/scope/probe/docker"
 	"github.com/weaveworks/scope/probe/host"
 	"github.com/weaveworks/scope/probe/kubernetes"
 	"github.com/weaveworks/scope/report"
@@ -16,20 +15,31 @@ type Parent struct {
 	TopologyID string `json:"topologyId"`
 }
 
+func node(t report.Topology) func(string) (report.Node, bool) {
+	return func(id string) (report.Node, bool) {
+		n, ok := t.Nodes[id]
+		return n, ok
+	}
+}
+
+func fake(id string) (report.Node, bool) {
+	return report.MakeNode(id), true
+}
+
 // Parents renders the parents of this report.Node, which have been aggregated
 // from the probe reports.
 func Parents(r report.Report, n report.Node) (result []Parent) {
 	topologies := map[string]struct {
-		report.Topology
+		node   func(id string) (report.Node, bool)
 		render func(report.Node) Parent
 	}{
-		report.Container:      {r.Container, containerParent},
-		report.Pod:            {r.Pod, podParent},
-		report.ReplicaSet:     {r.ReplicaSet, replicaSetParent},
-		report.Deployment:     {r.Deployment, deploymentParent},
-		report.Service:        {r.Service, serviceParent},
-		report.ContainerImage: {r.ContainerImage, containerImageParent},
-		report.Host:           {r.Host, hostParent},
+		report.Container:      {node(r.Container), containerParent},
+		report.Pod:            {node(r.Pod), podParent},
+		report.ReplicaSet:     {node(r.ReplicaSet), replicaSetParent},
+		report.Deployment:     {node(r.Deployment), deploymentParent},
+		report.Service:        {node(r.Service), serviceParent},
+		report.ContainerImage: {fake, containerImageParent},
+		report.Host:           {node(r.Host), hostParent},
 	}
 	topologyIDs := []string{}
 	for topologyID := range topologies {
@@ -44,7 +54,7 @@ func Parents(r report.Report, n report.Node) (result []Parent) {
 				continue
 			}
 
-			parent, ok := t.Nodes[id]
+			parent, ok := t.node(id)
 			if !ok {
 				continue
 			}
@@ -83,10 +93,10 @@ func kubernetesParent(topology string) func(report.Node) Parent {
 }
 
 func containerImageParent(n report.Node) Parent {
-	imageName, _ := n.Latest.Lookup(docker.ImageName)
+	name, _ := report.ParseContainerImageNodeID(n.ID)
 	return Parent{
 		ID:         n.ID,
-		Label:      docker.ImageNameWithoutVersion(imageName),
+		Label:      name,
 		TopologyID: "containers-by-image",
 	}
 }

render/detailed/parents_test.go

@@ -6,6 +6,7 @@ import (
 
 	"github.com/weaveworks/scope/render"
 	"github.com/weaveworks/scope/render/detailed"
+	"github.com/weaveworks/scope/render/expected"
 	"github.com/weaveworks/scope/report"
 	"github.com/weaveworks/scope/test"
 	"github.com/weaveworks/scope/test/fixture"
@@ -30,15 +31,17 @@ func TestParents(t *testing.T) {
 			want: nil,
 		},
 		{
-			node: render.ContainerImageRenderer.Render(fixture.Report, render.FilterNoop)[fixture.ClientContainerImageNodeID],
+			name: "Container image",
+			node: render.ContainerImageRenderer.Render(fixture.Report, render.FilterNoop)[expected.ClientContainerImageNodeID],
 			want: []detailed.Parent{
 				{ID: fixture.ClientHostNodeID, Label: fixture.ClientHostName, TopologyID: "hosts"},
 			},
 		},
 		{
-			node: render.ContainerRenderer.Render(fixture.Report, render.FilterNoop)[fixture.ClientContainerNodeID],
+			name: "Container",
+			node: render.ContainerWithImageNameRenderer.Render(fixture.Report, render.FilterNoop)[fixture.ClientContainerNodeID],
 			want: []detailed.Parent{
-				{ID: fixture.ClientContainerImageNodeID, Label: fixture.ClientContainerImageName, TopologyID: "containers-by-image"},
+				{ID: expected.ClientContainerImageNodeID, Label: fixture.ClientContainerImageName, TopologyID: "containers-by-image"},
 				{ID: fixture.ClientHostNodeID, Label: fixture.ClientHostName, TopologyID: "hosts"},
 				{ID: fixture.ClientPodNodeID, Label: "pong-a", TopologyID: "pods"},
 			},
@@ -47,7 +50,6 @@ func TestParents(t *testing.T) {
 			node: render.ProcessRenderer.Render(fixture.Report, render.FilterNoop)[fixture.ClientProcess1NodeID],
 			want: []detailed.Parent{
 				{ID: fixture.ClientContainerNodeID, Label: fixture.ClientContainerName, TopologyID: "containers"},
-				{ID: fixture.ClientContainerImageNodeID, Label: fixture.ClientContainerImageName, TopologyID: "containers-by-image"},
 				{ID: fixture.ClientHostNodeID, Label: fixture.ClientHostName, TopologyID: "hosts"},
 			},
 		},

render/detailed/summary_test.go

@@ -134,10 +134,10 @@ func TestMakeNodeSummary(t *testing.T) {
 		},
 		{
 			name:  "single container image rendering",
-			input: expected.RenderedContainerImages[fixture.ClientContainerImageNodeID],
+			input: expected.RenderedContainerImages[expected.ClientContainerImageNodeID],
 			ok:    true,
 			want: detailed.NodeSummary{
-				ID:         fixture.ClientContainerImageNodeID,
+				ID:         expected.ClientContainerImageNodeID,
 				Label:      fixture.ClientContainerImageName,
 				LabelMinor: "1 container",
 				Rank:       fixture.ClientContainerImageName,
@@ -145,10 +145,9 @@ func TestMakeNodeSummary(t *testing.T) {
 				Linkable:   true,
 				Stack:      true,
 				Metadata: []report.MetadataRow{
-					{ID: docker.ImageID, Label: "Image ID", Value: fixture.ClientContainerImageID, Priority: 1},
 					{ID: report.Container, Label: "# Containers", Value: "1", Priority: 2, Datatype: "number"},
 				},
-				Adjacency: report.MakeIDList(fixture.ServerContainerImageNodeID),
+				Adjacency: report.MakeIDList(expected.ServerContainerImageNodeID),
 			},
 		},
 		{

render/expected/expected.go

@@ -208,8 +208,11 @@ var (
 		render.OutgoingInternetID: theOutgoingInternetNode,
 	}
 
+	ClientContainerImageNodeID = report.MakeContainerImageNodeID(fixture.ClientContainerImageName)
+	ServerContainerImageNodeID = report.MakeContainerImageNodeID(fixture.ServerContainerImageName)
+
 	RenderedContainerImages = report.Nodes{
-		fixture.ClientContainerImageNodeID: containerImage(fixture.ClientContainerImageNodeID, fixture.ServerContainerImageNodeID).
+		ClientContainerImageNodeID: containerImage(ClientContainerImageNodeID, ServerContainerImageNodeID).
 			WithLatests(map[string]string{
 				report.HostNodeID: fixture.ClientHostNodeID,
 				docker.ImageID:    fixture.ClientContainerImageID,
@@ -226,7 +229,7 @@ var (
 				RenderedContainers[fixture.ClientContainerNodeID],
 			)),
 
-		fixture.ServerContainerImageNodeID: containerImage(fixture.ServerContainerImageNodeID).
+		ServerContainerImageNodeID: containerImage(ServerContainerImageNodeID).
 			WithChildren(report.MakeNodeSet(
 				RenderedEndpoints[fixture.Server80NodeID],
 				RenderedProcesses[fixture.ServerProcessNodeID],
@@ -234,7 +237,7 @@ var (
 			)),
 
 		uncontainedServerID:       uncontainedServerNode,
-		render.IncomingInternetID: theIncomingInternetNode(fixture.ServerContainerImageNodeID),
+		render.IncomingInternetID: theIncomingInternetNode(ServerContainerImageNodeID),
 		render.OutgoingInternetID: theOutgoingInternetNode,
 	}
 
@@ -298,7 +301,7 @@ var (
 				RenderedProcesses[fixture.ClientProcess1NodeID],
 				RenderedProcesses[fixture.ClientProcess2NodeID],
 				RenderedContainers[fixture.ClientContainerNodeID],
-				RenderedContainerImages[fixture.ClientContainerImageNodeID],
+				RenderedContainerImages[ClientContainerImageNodeID],
 				RenderedPods[fixture.ClientPodNodeID],
 			)),
 
@@ -309,7 +312,7 @@ var (
 				RenderedProcesses[fixture.ServerProcessNodeID],
 				RenderedProcesses[fixture.NonContainerProcessNodeID],
 				RenderedContainers[fixture.ServerContainerNodeID],
-				RenderedContainerImages[fixture.ServerContainerImageNodeID],
+				RenderedContainerImages[ServerContainerImageNodeID],
 				RenderedPods[fixture.ServerPodNodeID],
 			)),
 

report/sets.go

@@ -46,6 +46,16 @@ func (s Sets) Add(key string, value StringSet) Sets {
 	}
 }
 
+// Delete the given set from the Sets.
+func (s Sets) Delete(key string) Sets {
+	if s.psMap == nil {
+		return EmptySets
+	}
+	return Sets{
+		psMap: s.psMap.Delete(key),
+	}
+}
+
 // Lookup returns the sets stored under key.
 func (s Sets) Lookup(key string) (StringSet, bool) {
 	if s.psMap == nil {

test/fixture/report_fixture.go

@@ -217,8 +217,7 @@ var (
 				}).
 					WithTopology(report.Process).WithParents(report.EmptySets.
 					Add("host", report.MakeStringSet(ClientHostNodeID)).
-					Add("container", report.MakeStringSet(ClientContainerNodeID)).
-					Add("container_image", report.MakeStringSet(ClientContainerImageNodeID)),
+					Add("container", report.MakeStringSet(ClientContainerNodeID)),
 				).WithMetrics(report.Metrics{
 					process.CPUUsage:    ClientProcess1CPUMetric,
 					process.MemoryUsage: ClientProcess1MemoryMetric,
@@ -231,8 +230,7 @@ var (
 				}).
 					WithTopology(report.Process).WithParents(report.EmptySets.
 					Add("host", report.MakeStringSet(ClientHostNodeID)).
-					Add("container", report.MakeStringSet(ClientContainerNodeID)).
-					Add("container_image", report.MakeStringSet(ClientContainerImageNodeID)),
+					Add("container", report.MakeStringSet(ClientContainerNodeID)),
 				),
 				ServerProcessNodeID: report.MakeNodeWith(ServerProcessNodeID, map[string]string{
 					process.PID:        ServerPID,
@@ -242,8 +240,7 @@ var (
 				}).
 					WithTopology(report.Process).WithParents(report.EmptySets.
 					Add("host", report.MakeStringSet(ServerHostNodeID)).
-					Add("container", report.MakeStringSet(ServerContainerNodeID)).
-					Add("container_image", report.MakeStringSet(ServerContainerImageNodeID)),
+					Add("container", report.MakeStringSet(ServerContainerNodeID)),
 				),
 				NonContainerProcessNodeID: report.MakeNodeWith(NonContainerProcessNodeID, map[string]string{
 					process.PID:       NonContainerPID,