Merge pull request #917 from weaveworks/hosts-no-self-parent

Hosts should not be their own parent

render/detailed/node.go

@@ -5,7 +5,6 @@ import (
 
 	"github.com/weaveworks/scope/probe/docker"
 	"github.com/weaveworks/scope/probe/host"
-	"github.com/weaveworks/scope/probe/kubernetes"
 	"github.com/weaveworks/scope/probe/process"
 	"github.com/weaveworks/scope/render"
 	"github.com/weaveworks/scope/report"
@@ -22,13 +21,6 @@ type Node struct {
 	Parents  []Parent           `json:"parents,omitempty"`
 }
 
-// Parent is the information needed to build a link to the parent of a Node.
-type Parent struct {
-	ID         string `json:"id"`
-	Label      string `json:"label"`
-	TopologyID string `json:"topologyId"`
-}
-
 // ControlInstance contains a control description, and all the info
 // needed to execute it.
 type ControlInstance struct {
@@ -49,7 +41,7 @@ func MakeNode(r report.Report, n render.RenderableNode) Node {
 		Pseudo:      n.Pseudo,
 		Controls:    controls(r, n),
 		Children:    children(n),
-		Parents:     parents(r, n),
+		Parents:     Parents(r, n),
 	}
 }
 
@@ -120,88 +112,3 @@ func children(n render.RenderableNode) []NodeSummaryGroup {
 	}
 	return nodeSummaryGroups
 }
-
-// parents renders the parents of this report.Node, which have been aggregated
-// from the probe reports.
-func parents(r report.Report, n render.RenderableNode) (result []Parent) {
-	topologies := map[string]struct {
-		report.Topology
-		render func(report.Node) Parent
-	}{
-		report.Container:      {r.Container, containerParent},
-		report.Pod:            {r.Pod, podParent},
-		report.Service:        {r.Service, serviceParent},
-		report.ContainerImage: {r.ContainerImage, containerImageParent},
-		report.Host:           {r.Host, hostParent},
-	}
-	topologyIDs := []string{}
-	for topologyID := range topologies {
-		topologyIDs = append(topologyIDs, topologyID)
-	}
-	sort.Strings(topologyIDs)
-	for _, topologyID := range topologyIDs {
-		t := topologies[topologyID]
-		parents, _ := n.Node.Parents.Lookup(topologyID)
-		for _, id := range parents {
-			if topologyID == n.Node.Topology && id == n.ID {
-				continue
-			}
-
-			parent, ok := t.Nodes[id]
-			if !ok {
-				continue
-			}
-
-			result = append(result, t.render(parent))
-		}
-	}
-	return result
-}
-
-func containerParent(n report.Node) Parent {
-	label, _ := render.GetRenderableContainerName(n)
-	containerID, _ := n.Latest.Lookup(docker.ContainerID)
-	return Parent{
-		ID:         render.MakeContainerID(containerID),
-		Label:      label,
-		TopologyID: "containers",
-	}
-}
-
-func podParent(n report.Node) Parent {
-	podID, _ := n.Latest.Lookup(kubernetes.PodID)
-	podName, _ := n.Latest.Lookup(kubernetes.PodName)
-	return Parent{
-		ID:         render.MakePodID(podID),
-		Label:      podName,
-		TopologyID: "pods",
-	}
-}
-
-func serviceParent(n report.Node) Parent {
-	serviceID, _ := n.Latest.Lookup(kubernetes.ServiceID)
-	serviceName, _ := n.Latest.Lookup(kubernetes.ServiceName)
-	return Parent{
-		ID:         render.MakeServiceID(serviceID),
-		Label:      serviceName,
-		TopologyID: "pods-by-service",
-	}
-}
-
-func containerImageParent(n report.Node) Parent {
-	imageName, _ := n.Latest.Lookup(docker.ImageName)
-	return Parent{
-		ID:         render.MakeContainerImageID(render.ImageNameWithoutVersion(imageName)),
-		Label:      imageName,
-		TopologyID: "containers-by-image",
-	}
-}
-
-func hostParent(n report.Node) Parent {
-	hostName, _ := n.Latest.Lookup(host.HostName)
-	return Parent{
-		ID:         render.MakeHostID(hostName),
-		Label:      hostName,
-		TopologyID: "hosts",
-	}
-}

render/detailed/parents.go

@@ -0,0 +1,103 @@
+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/render"
+	"github.com/weaveworks/scope/report"
+)
+
+// Parent is the information needed to build a link to the parent of a Node.
+type Parent struct {
+	ID         string `json:"id"`
+	Label      string `json:"label"`
+	TopologyID string `json:"topologyId"`
+}
+
+// Parents renders the parents of this report.Node, which have been aggregated
+// from the probe reports.
+func Parents(r report.Report, n render.RenderableNode) (result []Parent) {
+	topologies := map[string]struct {
+		report.Topology
+		render func(report.Node) Parent
+	}{
+		report.Container:      {r.Container, containerParent},
+		report.Pod:            {r.Pod, podParent},
+		report.Service:        {r.Service, serviceParent},
+		report.ContainerImage: {r.ContainerImage, containerImageParent},
+		report.Host:           {r.Host, hostParent},
+	}
+	topologyIDs := []string{}
+	for topologyID := range topologies {
+		topologyIDs = append(topologyIDs, topologyID)
+	}
+	sort.Strings(topologyIDs)
+	for _, topologyID := range topologyIDs {
+		t := topologies[topologyID]
+		parents, _ := n.Node.Parents.Lookup(topologyID)
+		for _, id := range parents {
+			if topologyID == n.Node.Topology && id == n.Node.ID {
+				continue
+			}
+
+			parent, ok := t.Nodes[id]
+			if !ok {
+				continue
+			}
+
+			result = append(result, t.render(parent))
+		}
+	}
+	return result
+}
+
+func containerParent(n report.Node) Parent {
+	label, _ := render.GetRenderableContainerName(n)
+	containerID, _ := n.Latest.Lookup(docker.ContainerID)
+	return Parent{
+		ID:         render.MakeContainerID(containerID),
+		Label:      label,
+		TopologyID: "containers",
+	}
+}
+
+func podParent(n report.Node) Parent {
+	podID, _ := n.Latest.Lookup(kubernetes.PodID)
+	podName, _ := n.Latest.Lookup(kubernetes.PodName)
+	return Parent{
+		ID:         render.MakePodID(podID),
+		Label:      podName,
+		TopologyID: "pods",
+	}
+}
+
+func serviceParent(n report.Node) Parent {
+	serviceID, _ := n.Latest.Lookup(kubernetes.ServiceID)
+	serviceName, _ := n.Latest.Lookup(kubernetes.ServiceName)
+	return Parent{
+		ID:         render.MakeServiceID(serviceID),
+		Label:      serviceName,
+		TopologyID: "pods-by-service",
+	}
+}
+
+func containerImageParent(n report.Node) Parent {
+	imageName, _ := n.Latest.Lookup(docker.ImageName)
+	return Parent{
+		ID:         render.MakeContainerImageID(render.ImageNameWithoutVersion(imageName)),
+		Label:      imageName,
+		TopologyID: "containers-by-image",
+	}
+}
+
+func hostParent(n report.Node) Parent {
+	hostName, _ := n.Latest.Lookup(host.HostName)
+	return Parent{
+		ID:         render.MakeHostID(hostName),
+		Label:      hostName,
+		TopologyID: "hosts",
+	}
+}

render/detailed/parents_test.go

@@ -0,0 +1,56 @@
+package detailed_test
+
+import (
+	"fmt"
+	"testing"
+
+	"github.com/weaveworks/scope/render"
+	"github.com/weaveworks/scope/render/detailed"
+	"github.com/weaveworks/scope/report"
+	"github.com/weaveworks/scope/test"
+	"github.com/weaveworks/scope/test/fixture"
+	"github.com/weaveworks/scope/test/reflect"
+)
+
+func TestParents(t *testing.T) {
+	for _, c := range []struct {
+		name string
+		node render.RenderableNode
+		want []detailed.Parent
+	}{
+		{
+			name: "Node accidentally tagged with itself",
+			node: render.HostRenderer.Render(fixture.Report)[render.MakeHostID(fixture.ClientHostID)].WithParents(
+				report.EmptySets.Add(report.Host, report.MakeStringSet(fixture.ClientHostNodeID)),
+			),
+			want: nil,
+		},
+		{
+			node: render.HostRenderer.Render(fixture.Report)[render.MakeHostID(fixture.ClientHostID)],
+			want: nil,
+		},
+		{
+			node: render.ContainerRenderer.Render(fixture.Report)[render.MakeContainerID(fixture.ClientContainerID)],
+			want: []detailed.Parent{
+				{ID: render.MakeContainerImageID(fixture.ClientContainerImageName), Label: fixture.ClientContainerImageName, TopologyID: "containers-by-image"},
+				{ID: render.MakeHostID(fixture.ClientHostID), Label: fixture.ClientHostName, TopologyID: "hosts"},
+			},
+		},
+		{
+			node: render.ProcessRenderer.Render(fixture.Report)[render.MakeProcessID(fixture.ClientHostID, fixture.Client1PID)],
+			want: []detailed.Parent{
+				{ID: render.MakeContainerID(fixture.ClientContainerID), Label: fixture.ClientContainerName, TopologyID: "containers"},
+				{ID: render.MakeContainerImageID(fixture.ClientContainerImageName), Label: fixture.ClientContainerImageName, TopologyID: "containers-by-image"},
+				{ID: render.MakeHostID(fixture.ClientHostID), Label: fixture.ClientHostName, TopologyID: "hosts"},
+			},
+		},
+	} {
+		name := c.name
+		if name == "" {
+			name = fmt.Sprintf("Node %q", c.node.ID)
+		}
+		if have := detailed.Parents(fixture.Report, c.node); !reflect.DeepEqual(c.want, have) {
+			t.Errorf("%s: %s", name, test.Diff(c.want, have))
+		}
+	}
+}

test/fixture/report_fixture.go

@@ -73,6 +73,7 @@ var (
 	NonContainerProcessNodeID = report.MakeProcessNodeID(ServerHostID, NonContainerPID)
 
 	ClientContainerID     = "a1b2c3d4e5"
+	ClientContainerName   = "client"
 	ServerContainerID     = "5e4d3c2b1a"
 	ClientContainerNodeID = report.MakeContainerNodeID(ClientContainerID)
 	ServerContainerNodeID = report.MakeContainerNodeID(ServerContainerID)
@@ -257,7 +258,7 @@ var (
 			Nodes: report.Nodes{
 				ClientContainerNodeID: report.MakeNodeWith(map[string]string{
 					docker.ContainerID:                            ClientContainerID,
-					docker.ContainerName:                          "client",
+					docker.ContainerName:                          ClientContainerName,
 					docker.ImageID:                                ClientContainerImageID,
 					report.HostNodeID:                             ClientHostNodeID,
 					docker.LabelPrefix + "io.kubernetes.pod.name": ClientPodID,