Improve renderer for combined view by refactoring Map2Parent

The existing technique of "reducing" the two rendered graphs for daemonsets and deployments
had a glaring issue that no connections would ever be made between nodes of different types,
since that information would've been discarded earlier in the process.
It also makes it hard to identify "parentless" pods.

This commit extends the Map2Parent function, teaching it:
	* To check multiple topologies for parents
	* To pass through nodes with no parents found without modification

Since we already had two 'modes' for what to do with nodes without parents,
and it would've been clunky to try to encode the third option into the existing PseudoNodeID
arg in some way, we instead split it into two args, with the first being an enum specifying
either the old pseudo node behaviour, the old drop behaviour, or the new keep behaviour.

We then use the new Map2Parent to map pods to:
	* A replica set, if it has one
	* A daemonset, if it has one
	* Itself, if neither of the above
and then map again from the results to any deployment, leaving as-is any nodes that
don't map to a deployment. Hence we are left with:
	* Deployments
	* Daemonsets
	* Replica sets, but only if they map to no deployment
	* Pods, but only if they map to none of the above
and connections between all these will be calculated correctly.

render/ecs.go

@@ -10,7 +10,7 @@ var ECSTaskRenderer = ConditionalRenderer(renderECSTopologies,
 		PropagateSingleMetrics(report.Container),
 		MakeReduce(
 			MakeMap(
-				Map2Parent(report.ECSTask, UnmanagedID, nil),
+				Map2Parent([]string{report.ECSTask}, NoParentsPseudo, UnmanagedID, nil),
 				MakeFilter(
 					IsRunning,
 					ContainerWithImageNameRenderer,
@@ -27,7 +27,7 @@ var ECSServiceRenderer = ConditionalRenderer(renderECSTopologies,
 		PropagateSingleMetrics(report.ECSTask),
 		MakeReduce(
 			MakeMap(
-				Map2Parent(report.ECSService, "", nil),
+				Map2Parent([]string{report.ECSService}, NoParentsDrop, "", nil),
 				ECSTaskRenderer,
 			),
 			SelectECSService,

render/pod.go

@@ -26,6 +26,15 @@ func isPauseContainer(n report.Node) bool {
 	return ok && kubernetes.IsPauseImageName(image)
 }
 
+type noParentsActionEnum int
+
+// Constants for specifying noParentsAction in Map2Parent
+const (
+	NoParentsPseudo noParentsActionEnum = iota
+	NoParentsDrop
+	NoParentsKeep
+)
+
 // PodRenderer is a Renderer which produces a renderable kubernetes
 // graph by merging the container graph and the pods topology.
 var PodRenderer = ConditionalRenderer(renderKubernetesTopologies,
@@ -38,7 +47,7 @@ var PodRenderer = ConditionalRenderer(renderKubernetesTopologies,
 			PropagateSingleMetrics(report.Container),
 			MakeReduce(
 				MakeMap(
-					Map2Parent(report.Pod, UnmanagedID, nil),
+					Map2Parent([]string{report.Pod}, NoParentsPseudo, UnmanagedID, nil),
 					MakeFilter(
 						ComposeFilterFuncs(
 							IsRunning,
@@ -61,7 +70,7 @@ var PodServiceRenderer = ConditionalRenderer(renderKubernetesTopologies,
 		PropagateSingleMetrics(report.Pod),
 		MakeReduce(
 			MakeMap(
-				Map2Parent(report.Service, "", nil),
+				Map2Parent([]string{report.Service}, NoParentsDrop, "", nil),
 				PodRenderer,
 			),
 			SelectService,
@@ -76,7 +85,7 @@ var DeploymentRenderer = ConditionalRenderer(renderKubernetesTopologies,
 		PropagateSingleMetrics(report.ReplicaSet),
 		MakeReduce(
 			MakeMap(
-				Map2Parent(report.Deployment, "", mapPodCounts),
+				Map2Parent([]string{report.Deployment}, NoParentsDrop, "", mapPodCounts),
 				ReplicaSetRenderer,
 			),
 			SelectDeployment,
@@ -91,7 +100,7 @@ var ReplicaSetRenderer = ConditionalRenderer(renderKubernetesTopologies,
 		PropagateSingleMetrics(report.Pod),
 		MakeReduce(
 			MakeMap(
-				Map2Parent(report.ReplicaSet, "", nil),
+				Map2Parent([]string{report.ReplicaSet}, NoParentsDrop, "", nil),
 				PodRenderer,
 			),
 			SelectReplicaSet,
@@ -106,7 +115,7 @@ var DaemonSetRenderer = ConditionalRenderer(renderKubernetesTopologies,
 		PropagateSingleMetrics(report.Pod),
 		MakeReduce(
 			MakeMap(
-				Map2Parent(report.DaemonSet, "", nil),
+				Map2Parent([]string{report.DaemonSet}, NoParentsDrop, "", nil),
 				PodRenderer,
 			),
 			SelectDaemonSet,
@@ -114,11 +123,30 @@ var DaemonSetRenderer = ConditionalRenderer(renderKubernetesTopologies,
 	),
 )
 
-// KubeCombinedRenderer is a Renderer which combines the daemonset and deployment views
-// in (for now) a very naive way.
-var KubeCombinedRenderer = MakeReduce(
-	DeploymentRenderer,
-	DaemonSetRenderer,
+// KubeCombinedRenderer is a Renderer which combines the 'top abstraction' of all pods.
+// We first map pods to all possible things they can map to, then we map again to
+// deployments since some things (replica sets) can map to those (the rest are passed through
+// unchanged).
+// We can't simply combine the rendered graphs of the high level objects as they would never
+// have connections to each other.
+var KubeCombinedRenderer = ConditionalRenderer(renderKubernetesTopologies,
+	MakeReduce(
+		MakeMap(
+			Map2Parent([]string{report.Deployment}, NoParentsKeep, "", mapPodCounts),
+			MakeReduce(
+				MakeMap(
+					Map2Parent([]string{
+						report.ReplicaSet,
+						report.DaemonSet,
+					}, NoParentsKeep, "", nil),
+					PodRenderer,
+				),
+				SelectReplicaSet,
+				SelectDaemonSet,
+			),
+		),
+		SelectDeployment,
+	),
 )
 
 func mapPodCounts(parent, original report.Node) report.Node {
@@ -148,18 +176,23 @@ func MapPod2IP(m report.Node) []string {
 
 // Map2Parent returns a MapFunc which maps Nodes to some parent grouping.
 func Map2Parent(
-	// The topology ID of the parents
-	topology string,
-	// Either the ID prefix of the pseudo node to use for nodes without
-	// any parents in the group, eg. UnmanagedID, or "" to drop nodes without any parents.
+	// The topology IDs to look for parents in
+	topologies []string,
+	// Choose what to do in the case of nodes with no parents. One of:
+	//     NoParentsPseudo: Map them to a common pseudo node id with prefix noParentsPseudoID
+	//     NoParentsDrop: Map them to no node.
+	//     NoParentsKeep: Map them to themselves, preserving them in the new graph.
+	noParentsAction noParentsActionEnum,
+	// The ID prefix of the pseudo node to use for nodes without any parents in the group
+	// if noParentsAction == Pseudo, eg. UnmanagedID
 	noParentsPseudoID string,
 	// Optional (can be nil) function to modify any parent nodes,
 	// eg. to copy over details from the original node.
 	modifyMappedNode func(parent, original report.Node) report.Node,
 ) MapFunc {
 	return func(n report.Node, _ report.Networks) report.Nodes {
-		// Uncontained becomes Unmanaged/whatever if noParentsPseudoID is set
-		if noParentsPseudoID != "" && strings.HasPrefix(n.ID, UncontainedIDPrefix) {
+		// Uncontained becomes Unmanaged/whatever if noParentsAction == Pseudo
+		if noParentsAction == NoParentsPseudo && strings.HasPrefix(n.ID, UncontainedIDPrefix) {
 			id := MakePseudoNodeID(noParentsPseudoID, report.ExtractHostID(n))
 			node := NewDerivedPseudoNode(id, n)
 			return report.Nodes{id: node}
@@ -170,28 +203,36 @@ func Map2Parent(
 			return report.Nodes{n.ID: n}
 		}
 
-		// If some some reason the node doesn't have any of these ids
-		// (maybe slightly out of sync reports, or its not in this group),
-		// either drop it or put it in Uncontained/Unmanaged/whatever if one was given
-		groupIDs, ok := n.Parents.Lookup(topology)
-		if !ok || len(groupIDs) == 0 {
-			if noParentsPseudoID == "" {
-				return report.Nodes{}
+		// For each topology, map to any parents we can find
+		result := report.Nodes{}
+		for _, topology := range topologies {
+			if groupIDs, ok := n.Parents.Lookup(topology); ok {
+				for _, id := range groupIDs {
+					node := NewDerivedNode(id, n).WithTopology(topology)
+					node.Counters = node.Counters.Add(n.Topology, 1)
+					if modifyMappedNode != nil {
+						node = modifyMappedNode(node, n)
+					}
+					result[id] = node
+				}
 			}
-			id := MakePseudoNodeID(UnmanagedID, report.ExtractHostID(n))
-			node := NewDerivedPseudoNode(id, n)
-			return report.Nodes{id: node}
 		}
 
-		result := report.Nodes{}
-		for _, id := range groupIDs {
-			node := NewDerivedNode(id, n).WithTopology(topology)
-			node.Counters = node.Counters.Add(n.Topology, 1)
-			if modifyMappedNode != nil {
-				node = modifyMappedNode(node, n)
+		if len(result) == 0 {
+			switch noParentsAction {
+			case NoParentsPseudo:
+				// Map to pseudo node
+				id := MakePseudoNodeID(UnmanagedID, report.ExtractHostID(n))
+				node := NewDerivedPseudoNode(id, n)
+				result[id] = node
+			case NoParentsKeep:
+				// Pass n to output unmodified
+				result[n.ID] = n
+			case NoParentsDrop:
+				// Do nothing, we will return an empty result
 			}
-			result[id] = node
 		}
+
 		return result
 	}
 }

render/swarm.go

@@ -10,7 +10,7 @@ var SwarmServiceRenderer = ConditionalRenderer(renderSwarmTopologies,
 		PropagateSingleMetrics(report.Container),
 		MakeReduce(
 			MakeMap(
-				Map2Parent(report.SwarmService, UnmanagedID, nil),
+				Map2Parent([]string{report.SwarmService}, NoParentsPseudo, UnmanagedID, nil),
 				MakeFilter(
 					IsRunning,
 					ContainerWithImageNameRenderer,