Refactor: streamline Node.WithParents()

Make it take parameters just like all the callers have, instead of
making them create a different structure.

It is now only used in tests.

render/expected/expected.go

@@ -50,28 +50,28 @@ var (
 
 	unknownPseudoNode1 = func(adjacent ...string) report.Node {
 		return pseudo(UnknownPseudoNode1ID, adjacent...).
-			WithChildren(report.MakeNodeSet(
+			WithChildren(
 				RenderedEndpoints[fixture.UnknownClient1NodeID],
 				RenderedEndpoints[fixture.UnknownClient2NodeID],
-			))
+			)
 	}
 	unknownPseudoNode2 = func(adjacent ...string) report.Node {
 		return pseudo(UnknownPseudoNode2ID, adjacent...).
-			WithChildren(report.MakeNodeSet(
+			WithChildren(
 				RenderedEndpoints[fixture.UnknownClient3NodeID],
-			))
+			)
 	}
 
 	theIncomingInternetNode = func(adjacent ...string) report.Node {
 		return pseudo(render.IncomingInternetID, adjacent...).
-			WithChildren(report.MakeNodeSet(
+			WithChildren(
 				RenderedEndpoints[fixture.RandomClientNodeID],
-			))
+			)
 	}
 
-	theOutgoingInternetNode = pseudo(render.OutgoingInternetID).WithChildren(report.MakeNodeSet(
+	theOutgoingInternetNode = pseudo(render.OutgoingInternetID).WithChildren(
 		RenderedEndpoints[fixture.GoogleEndpointNodeID],
-	))
+	)
 
 	RenderedEndpoints = report.Nodes{
 		fixture.Client54001NodeID:    endpoint(fixture.Client54001NodeID, fixture.Server80NodeID),
@@ -92,24 +92,24 @@ var (
 				process.PID:       fixture.Client1PID,
 				process.Name:      fixture.Client1Name,
 			}).
-			WithChildren(report.MakeNodeSet(
+			WithChildren(
 				RenderedEndpoints[fixture.Client54001NodeID],
-			)),
+			),
 
 		fixture.ClientProcess2NodeID: processNode(fixture.ClientProcess2NodeID, fixture.ServerProcessNodeID).
-			WithChildren(report.MakeNodeSet(
+			WithChildren(
 				RenderedEndpoints[fixture.Client54002NodeID],
-			)),
+			),
 
 		fixture.ServerProcessNodeID: processNode(fixture.ServerProcessNodeID).
-			WithChildren(report.MakeNodeSet(
+			WithChildren(
 				RenderedEndpoints[fixture.Server80NodeID],
-			)),
+			),
 
 		fixture.NonContainerProcessNodeID: processNode(fixture.NonContainerProcessNodeID, render.OutgoingInternetID).
-			WithChildren(report.MakeNodeSet(
+			WithChildren(
 				RenderedEndpoints[fixture.NonContainerNodeID],
-			)),
+			),
 
 		// due to https://github.com/weaveworks/scope/issues/1323 we are dropping
 		// all non-internet pseudo nodes for now.
@@ -123,26 +123,26 @@ var (
 		fixture.Client1Name: processNameNode(fixture.Client1Name, fixture.ServerName).
 			WithLatests(map[string]string{process.Name: fixture.Client1Name}).
 			WithCounters(map[string]int{report.Process: 2}).
-			WithChildren(report.MakeNodeSet(
+			WithChildren(
 				RenderedEndpoints[fixture.Client54001NodeID],
 				RenderedEndpoints[fixture.Client54002NodeID],
 				RenderedProcesses[fixture.ClientProcess1NodeID],
 				RenderedProcesses[fixture.ClientProcess2NodeID],
-			)),
+			),
 
 		fixture.ServerName: processNameNode(fixture.ServerName).
 			WithLatests(map[string]string{process.Name: fixture.ServerName}).
 			WithCounters(map[string]int{report.Process: 1}).
-			WithChildren(report.MakeNodeSet(
+			WithChildren(
 				RenderedEndpoints[fixture.Server80NodeID],
 				RenderedProcesses[fixture.ServerProcessNodeID],
-			)),
+			),
 
 		fixture.NonContainerName: processNameNode(fixture.NonContainerName, render.OutgoingInternetID).
-			WithChildren(report.MakeNodeSet(
+			WithChildren(
 				RenderedEndpoints[fixture.NonContainerNodeID],
 				RenderedProcesses[fixture.NonContainerProcessNodeID],
-			)),
+			),
 
 		// due to https://github.com/weaveworks/scope/issues/1323 we are dropping
 		// all non-internet pseudo nodes for now.
@@ -153,10 +153,10 @@ var (
 	}
 
 	uncontainedServerID   = render.MakePseudoNodeID(render.UncontainedID, fixture.ServerHostID)
-	uncontainedServerNode = pseudo(uncontainedServerID, render.OutgoingInternetID).WithChildren(report.MakeNodeSet(
+	uncontainedServerNode = pseudo(uncontainedServerID, render.OutgoingInternetID).WithChildren(
 		RenderedEndpoints[fixture.NonContainerNodeID],
 		RenderedProcesses[fixture.NonContainerProcessNodeID],
-	))
+	)
 
 	RenderedContainers = report.Nodes{
 		fixture.ClientContainerNodeID: container(fixture.ClientContainerNodeID, fixture.ServerContainerNodeID).
@@ -166,18 +166,18 @@ var (
 				docker.ContainerName: fixture.ClientContainerName,
 				docker.ImageName:     fixture.ClientContainerImageName,
 			}).
-			WithChildren(report.MakeNodeSet(
+			WithChildren(
 				RenderedEndpoints[fixture.Client54001NodeID],
 				RenderedEndpoints[fixture.Client54002NodeID],
 				RenderedProcesses[fixture.ClientProcess1NodeID],
 				RenderedProcesses[fixture.ClientProcess2NodeID],
-			)),
+			),
 
 		fixture.ServerContainerNodeID: container(fixture.ServerContainerNodeID).
-			WithChildren(report.MakeNodeSet(
+			WithChildren(
 				RenderedEndpoints[fixture.Server80NodeID],
 				RenderedProcesses[fixture.ServerProcessNodeID],
-			)),
+			),
 
 		uncontainedServerID:       uncontainedServerNode,
 		render.IncomingInternetID: theIncomingInternetNode(fixture.ServerContainerNodeID),
@@ -192,23 +192,23 @@ var (
 			WithCounters(map[string]int{
 				report.Container: 1,
 			}).
-			WithChildren(report.MakeNodeSet(
+			WithChildren(
 				RenderedEndpoints[fixture.Client54001NodeID],
 				RenderedEndpoints[fixture.Client54002NodeID],
 				RenderedProcesses[fixture.ClientProcess1NodeID],
 				RenderedProcesses[fixture.ClientProcess2NodeID],
 				RenderedContainers[fixture.ClientContainerNodeID],
-			)),
+			),
 
 		fixture.ServerContainerHostname: containerHostnameNode(fixture.ServerContainerHostname).
 			WithLatests(map[string]string{
 				docker.ContainerHostname: fixture.ServerContainerHostname,
 			}).
-			WithChildren(report.MakeNodeSet(
+			WithChildren(
 				RenderedEndpoints[fixture.Server80NodeID],
 				RenderedProcesses[fixture.ServerProcessNodeID],
 				RenderedContainers[fixture.ServerContainerNodeID],
-			)),
+			),
 
 		uncontainedServerID:       uncontainedServerNode,
 		render.IncomingInternetID: theIncomingInternetNode(fixture.ServerContainerHostname),
@@ -228,20 +228,20 @@ var (
 			WithCounters(map[string]int{
 				report.Container: 1,
 			}).
-			WithChildren(report.MakeNodeSet(
+			WithChildren(
 				RenderedEndpoints[fixture.Client54001NodeID],
 				RenderedEndpoints[fixture.Client54002NodeID],
 				RenderedProcesses[fixture.ClientProcess1NodeID],
 				RenderedProcesses[fixture.ClientProcess2NodeID],
 				RenderedContainers[fixture.ClientContainerNodeID],
-			)),
+			),
 
 		ServerContainerImageNodeID: containerImage(ServerContainerImageNodeID).
-			WithChildren(report.MakeNodeSet(
+			WithChildren(
 				RenderedEndpoints[fixture.Server80NodeID],
 				RenderedProcesses[fixture.ServerProcessNodeID],
 				RenderedContainers[fixture.ServerContainerNodeID],
-			)),
+			),
 
 		uncontainedServerID:       uncontainedServerNode,
 		render.IncomingInternetID: theIncomingInternetNode(ServerContainerImageNodeID),
@@ -249,28 +249,28 @@ var (
 	}
 
 	UnmanagedServerID   = render.MakePseudoNodeID(render.UnmanagedID, fixture.ServerHostID)
-	unmanagedServerNode = pseudo(UnmanagedServerID, render.OutgoingInternetID).WithChildren(report.MakeNodeSet(
+	unmanagedServerNode = pseudo(UnmanagedServerID, render.OutgoingInternetID).WithChildren(
 		uncontainedServerNode,
 		RenderedEndpoints[fixture.NonContainerNodeID],
 		RenderedProcesses[fixture.NonContainerProcessNodeID],
-	))
+	)
 
 	RenderedPods = report.Nodes{
 		fixture.ClientPodNodeID: pod(fixture.ClientPodNodeID, fixture.ServerPodNodeID).
-			WithChildren(report.MakeNodeSet(
+			WithChildren(
 				RenderedEndpoints[fixture.Client54001NodeID],
 				RenderedEndpoints[fixture.Client54002NodeID],
 				RenderedProcesses[fixture.ClientProcess1NodeID],
 				RenderedProcesses[fixture.ClientProcess2NodeID],
 				RenderedContainers[fixture.ClientContainerNodeID],
-			)),
+			),
 
 		fixture.ServerPodNodeID: pod(fixture.ServerPodNodeID).
-			WithChildren(report.MakeNodeSet(
+			WithChildren(
 				RenderedEndpoints[fixture.Server80NodeID],
 				RenderedProcesses[fixture.ServerProcessNodeID],
 				RenderedContainers[fixture.ServerContainerNodeID],
-			)),
+			),
 
 		fixture.PersistentVolumeClaimNodeID: persistentVolumeClaim(fixture.PersistentVolumeClaimNodeID, fixture.PersistentVolumeNodeID).
 			WithLatests(map[string]string{
@@ -280,7 +280,7 @@ var (
 				kubernetes.VolumeName:       "pongvolume",
 				kubernetes.AccessModes:      "ReadWriteOnce",
 				kubernetes.StorageClassName: "standard",
-			}).WithChild(report.MakeNode(fixture.PersistentVolumeNodeID).WithTopology(report.PersistentVolume)),
+			}).WithChildren(report.MakeNode(fixture.PersistentVolumeNodeID).WithTopology(report.PersistentVolume)),
 
 		fixture.PersistentVolumeNodeID: persistentVolume(fixture.PersistentVolumeNodeID).
 			WithLatests(map[string]string{
@@ -296,7 +296,7 @@ var (
 			WithLatests(map[string]string{
 				kubernetes.Name:        "standard",
 				kubernetes.Provisioner: "pong",
-			}).WithChild(report.MakeNode(fixture.PersistentVolumeClaimNodeID).WithTopology(report.PersistentVolumeClaim)),
+			}).WithChildren(report.MakeNode(fixture.PersistentVolumeClaimNodeID).WithTopology(report.PersistentVolumeClaim)),
 
 		UnmanagedServerID:         unmanagedServerNode,
 		render.IncomingInternetID: theIncomingInternetNode(fixture.ServerPodNodeID),
@@ -305,7 +305,7 @@ var (
 
 	RenderedPodServices = report.Nodes{
 		fixture.ServiceNodeID: service(fixture.ServiceNodeID, fixture.ServiceNodeID).
-			WithChildren(report.MakeNodeSet(
+			WithChildren(
 				RenderedEndpoints[fixture.Client54001NodeID],
 				RenderedEndpoints[fixture.Client54002NodeID],
 				RenderedEndpoints[fixture.Server80NodeID],
@@ -316,7 +316,7 @@ var (
 				RenderedContainers[fixture.ServerContainerNodeID],
 				RenderedPods[fixture.ClientPodNodeID],
 				RenderedPods[fixture.ServerPodNodeID],
-			)),
+			),
 
 		UnmanagedServerID:         unmanagedServerNode,
 		render.IncomingInternetID: theIncomingInternetNode(fixture.ServiceNodeID),
@@ -328,7 +328,7 @@ var (
 			WithLatests(map[string]string{
 				host.HostName: fixture.ClientHostName,
 			}).
-			WithChildren(report.MakeNodeSet(
+			WithChildren(
 				RenderedEndpoints[fixture.Client54001NodeID],
 				RenderedEndpoints[fixture.Client54002NodeID],
 				RenderedProcesses[fixture.ClientProcess1NodeID],
@@ -336,10 +336,10 @@ var (
 				RenderedContainers[fixture.ClientContainerNodeID],
 				RenderedContainerImages[ClientContainerImageNodeID],
 				RenderedPods[fixture.ClientPodNodeID],
-			)),
+			),
 
 		fixture.ServerHostNodeID: hostNode(fixture.ServerHostNodeID, render.OutgoingInternetID).
-			WithChildren(report.MakeNodeSet(
+			WithChildren(
 				RenderedEndpoints[fixture.Server80NodeID],
 				RenderedEndpoints[fixture.NonContainerNodeID],
 				RenderedProcesses[fixture.ServerProcessNodeID],
@@ -347,7 +347,7 @@ var (
 				RenderedContainers[fixture.ServerContainerNodeID],
 				RenderedContainerImages[ServerContainerImageNodeID],
 				RenderedPods[fixture.ServerPodNodeID],
-			)),
+			),
 
 		// due to https://github.com/weaveworks/scope/issues/1323 we are dropping
 		// all non-internet pseudo nodes for now.

render/metrics_test.go

@@ -27,33 +27,44 @@ func TestPropagateSingleMetrics(t *testing.T) {
 		{
 			name: "one child",
 			input: report.MakeNode("a").WithChildren(
-				report.MakeNodeSet(
-					report.MakeNode("child1").
-						WithTopology(report.Container).
-						WithMetrics(report.Metrics{
-							"metric1": report.MakeMetric(nil),
-						}),
-				),
+				report.MakeNode("child1").
+					WithTopology(report.Container).
+					WithMetrics(report.Metrics{
+						"metric1": report.MakeMetric(nil),
+					}),
 			),
 			topology: report.Container,
 			output: report.Nodes{
 				"a": report.MakeNode("a").WithMetrics(report.Metrics{
 					"metric1": report.MakeMetric(nil),
 				}).WithChildren(
-					report.MakeNodeSet(
-						report.MakeNode("child1").
-							WithTopology(report.Container).
-							WithMetrics(report.Metrics{
-								"metric1": report.MakeMetric(nil),
-							}),
-					),
+					report.MakeNode("child1").
+						WithTopology(report.Container).
+						WithMetrics(report.Metrics{
+							"metric1": report.MakeMetric(nil),
+						}),
 				),
 			},
 		},
 		{
 			name: "ignores other topologies",
 			input: report.MakeNode("a").WithChildren(
-				report.MakeNodeSet(
+				report.MakeNode("child1").
+					WithTopology(report.Container).
+					WithMetrics(report.Metrics{
+						"metric1": report.MakeMetric(nil),
+					}),
+				report.MakeNode("child2").
+					WithTopology("otherTopology").
+					WithMetrics(report.Metrics{
+						"metric2": report.MakeMetric(nil),
+					}),
+			),
+			topology: report.Container,
+			output: report.Nodes{
+				"a": report.MakeNode("a").WithMetrics(report.Metrics{
+					"metric1": report.MakeMetric(nil),
+				}).WithChildren(
 					report.MakeNode("child1").
 						WithTopology(report.Container).
 						WithMetrics(report.Metrics{
@@ -65,31 +76,25 @@ func TestPropagateSingleMetrics(t *testing.T) {
 							"metric2": report.MakeMetric(nil),
 						}),
 				),
-			),
-			topology: report.Container,
-			output: report.Nodes{
-				"a": report.MakeNode("a").WithMetrics(report.Metrics{
-					"metric1": report.MakeMetric(nil),
-				}).WithChildren(
-					report.MakeNodeSet(
-						report.MakeNode("child1").
-							WithTopology(report.Container).
-							WithMetrics(report.Metrics{
-								"metric1": report.MakeMetric(nil),
-							}),
-						report.MakeNode("child2").
-							WithTopology("otherTopology").
-							WithMetrics(report.Metrics{
-								"metric2": report.MakeMetric(nil),
-							}),
-					),
-				),
 			},
 		},
 		{
 			name: "two children",
 			input: report.MakeNode("a").WithChildren(
-				report.MakeNodeSet(
+				report.MakeNode("child1").
+					WithTopology(report.Container).
+					WithMetrics(report.Metrics{
+						"metric1": report.MakeMetric(nil),
+					}),
+				report.MakeNode("child2").
+					WithTopology(report.Container).
+					WithMetrics(report.Metrics{
+						"metric2": report.MakeMetric(nil),
+					}),
+			),
+			topology: report.Container,
+			output: report.Nodes{
+				"a": report.MakeNode("a").WithChildren(
 					report.MakeNode("child1").
 						WithTopology(report.Container).
 						WithMetrics(report.Metrics{
@@ -101,29 +106,28 @@ func TestPropagateSingleMetrics(t *testing.T) {
 							"metric2": report.MakeMetric(nil),
 						}),
 				),
-			),
-			topology: report.Container,
-			output: report.Nodes{
-				"a": report.MakeNode("a").WithChildren(
-					report.MakeNodeSet(
-						report.MakeNode("child1").
-							WithTopology(report.Container).
-							WithMetrics(report.Metrics{
-								"metric1": report.MakeMetric(nil),
-							}),
-						report.MakeNode("child2").
-							WithTopology(report.Container).
-							WithMetrics(report.Metrics{
-								"metric2": report.MakeMetric(nil),
-							}),
-					),
-				),
 			},
 		},
 		{
 			name: "ignores k8s pause container",
 			input: report.MakeNode("a").WithChildren(
-				report.MakeNodeSet(
+				report.MakeNode("child1").
+					WithTopology(report.Container).
+					WithMetrics(report.Metrics{
+						"metric1": report.MakeMetric(nil),
+					}),
+				report.MakeNode("child2").
+					WithLatest(report.DoesNotMakeConnections, now, "").
+					WithTopology(report.Container).
+					WithMetrics(report.Metrics{
+						"metric2": report.MakeMetric(nil),
+					}),
+			),
+			topology: report.Container,
+			output: report.Nodes{
+				"a": report.MakeNode("a").WithMetrics(report.Metrics{
+					"metric1": report.MakeMetric(nil),
+				}).WithChildren(
 					report.MakeNode("child1").
 						WithTopology(report.Container).
 						WithMetrics(report.Metrics{
@@ -136,26 +140,6 @@ func TestPropagateSingleMetrics(t *testing.T) {
 							"metric2": report.MakeMetric(nil),
 						}),
 				),
-			),
-			topology: report.Container,
-			output: report.Nodes{
-				"a": report.MakeNode("a").WithMetrics(report.Metrics{
-					"metric1": report.MakeMetric(nil),
-				}).WithChildren(
-					report.MakeNodeSet(
-						report.MakeNode("child1").
-							WithTopology(report.Container).
-							WithMetrics(report.Metrics{
-								"metric1": report.MakeMetric(nil),
-							}),
-						report.MakeNode("child2").
-							WithLatest(report.DoesNotMakeConnections, now, "").
-							WithTopology(report.Container).
-							WithMetrics(report.Metrics{
-								"metric2": report.MakeMetric(nil),
-							}),
-					),
-				),
 			},
 		},
 	} {

report/node.go

@@ -169,14 +169,8 @@ func (n Node) PruneParents() Node {
 }
 
 // WithChildren returns a fresh copy of n, with children merged in.
-func (n Node) WithChildren(children NodeSet) Node {
-	n.Children = n.Children.Merge(children)
-	return n
-}
-
-// WithChild returns a fresh copy of n, with one child merged in.
-func (n Node) WithChild(child Node) Node {
-	n.Children = n.Children.Merge(MakeNodeSet(child))
+func (n Node) WithChildren(children ...Node) Node {
+	n.Children = n.Children.Add(children...)
 	return n
 }
 

report/node_set.go

@@ -5,7 +5,6 @@ import (
 	"fmt"
 
 	"github.com/davecgh/go-spew/spew"
-	"github.com/ugorji/go/codec"
 	"github.com/weaveworks/ps"
 
 	"github.com/weaveworks/scope/test/reflect"
@@ -146,43 +145,3 @@ func (n NodeSet) String() string {
 func (n NodeSet) DeepEqual(o NodeSet) bool {
 	return mapEqual(n.psMap, o.psMap, reflect.DeepEqual)
 }
-
-func (n NodeSet) toIntermediate() []Node {
-	intermediate := make([]Node, 0, n.Size())
-	n.ForEach(func(node Node) {
-		intermediate = append(intermediate, node)
-	})
-	return intermediate
-}
-
-func (n NodeSet) fromIntermediate(nodes []Node) NodeSet {
-	return MakeNodeSet(nodes...)
-}
-
-// CodecEncodeSelf implements codec.Selfer
-func (n *NodeSet) CodecEncodeSelf(encoder *codec.Encoder) {
-	if n.psMap != nil {
-		encoder.Encode(n.toIntermediate())
-	} else {
-		encoder.Encode(nil)
-	}
-}
-
-// CodecDecodeSelf implements codec.Selfer
-func (n *NodeSet) CodecDecodeSelf(decoder *codec.Decoder) {
-	in := []Node{}
-	if err := decoder.Decode(&in); err != nil {
-		return
-	}
-	*n = NodeSet{}.fromIntermediate(in)
-}
-
-// MarshalJSON shouldn't be used, use CodecEncodeSelf instead
-func (NodeSet) MarshalJSON() ([]byte, error) {
-	panic("MarshalJSON shouldn't be used, use CodecEncodeSelf instead")
-}
-
-// UnmarshalJSON shouldn't be used, use CodecDecodeSelf instead
-func (*NodeSet) UnmarshalJSON(b []byte) error {
-	panic("UnmarshalJSON shouldn't be used, use CodecDecodeSelf instead")
-}

test/utils/prune.go

@@ -18,13 +18,13 @@ func Prune(nodes report.Nodes) report.Nodes {
 // necessary for rendering nodes and edges stripped away. Specifically, that
 // means cutting out parts of the Node.
 func PruneNode(node report.Node) report.Node {
-	prunedChildren := report.MakeNodeSet()
+	prunedChildren := []report.Node{}
 	node.Children.ForEach(func(child report.Node) {
-		prunedChildren = prunedChildren.Add(PruneNode(child))
+		prunedChildren = append(prunedChildren, PruneNode(child))
 	})
 	return report.MakeNode(
 		node.ID).
 		WithTopology(node.Topology).
 		WithAdjacent(node.Adjacency...).
-		WithChildren(prunedChildren)
+		WithChildren(prunedChildren...)
 }