simplify finding missing liveness probes

pkg/api/kubegraph/analysis/hpa.go

@@ -11,6 +11,7 @@ import (
 
 	osgraph "github.com/openshift/origin/pkg/api/graph"
 	"github.com/openshift/origin/pkg/api/kubegraph"
+	kubeedges "github.com/openshift/origin/pkg/api/kubegraph"
 	kubenodes "github.com/openshift/origin/pkg/api/kubegraph/nodes"
 	deploygraph "github.com/openshift/origin/pkg/deploy/graph"
 	deploynodes "github.com/openshift/origin/pkg/deploy/graph/nodes"
@@ -117,6 +118,7 @@ func FindOverlappingHPAs(graph osgraph.Graph, namer osgraph.Namer) []osgraph.Mar
 	edgeFilter := osgraph.EdgesOfKind(
 		kubegraph.ScalingEdgeKind,
 		deploygraph.DeploymentEdgeKind,
+		kubeedges.ManagedByControllerEdgeKind,
 	)
 
 	hpaSubGraph := graph.Subgraph(nodeFilter, edgeFilter)

pkg/api/kubegraph/analysis/podspec.go

@@ -8,8 +8,6 @@ import (
 	osgraph "github.com/openshift/origin/pkg/api/graph"
 	kubeedges "github.com/openshift/origin/pkg/api/kubegraph"
 	kubegraph "github.com/openshift/origin/pkg/api/kubegraph/nodes"
-	deploygraph "github.com/openshift/origin/pkg/deploy/graph"
-	"k8s.io/kubernetes/pkg/util/sets"
 )
 
 const (
@@ -81,62 +79,55 @@ func FindMissingSecrets(g osgraph.Graph, f osgraph.Namer) []osgraph.Marker {
 // FindMissingLivenessProbes inspects all PodSpecs for missing liveness probes and generates a list of non-duplicate markers
 func FindMissingLivenessProbes(g osgraph.Graph, f osgraph.Namer, setProbeCommand string) []osgraph.Marker {
 	markers := []osgraph.Marker{}
-	appendedNodes := sets.NewString()
-	ignoredNodes := findNodesManagedByController(g)
 
 	for _, uncastPodSpecNode := range g.NodesByKind(kubegraph.PodSpecNodeKind) {
 		podSpecNode := uncastPodSpecNode.(*kubegraph.PodSpecNode)
-		podsWithoutLivenessProbes := CheckForLivenessProbes(podSpecNode)
-
 		topLevelNode := osgraph.GetTopLevelContainerNode(g, podSpecNode)
-		topLevelString := f.ResourceName(topLevelNode)
 
-		// prevent duplicate markers
-		if appendedNodes.Has(topLevelString) {
+		if hasLivenessProbe(podSpecNode) {
 			continue
 		}
-
-		// ignore replication controllers owned by DeploymentConfigs
-		if isNodeInList(topLevelNode, ignoredNodes) {
+		if hasControllerRefEdge(g, topLevelNode) {
 			continue
 		}
 
-		appendedNodes.Insert(topLevelString)
-		for _, node := range podsWithoutLivenessProbes {
-			markers = append(markers, osgraph.Marker{
-				Node:         podSpecNode,
-				RelatedNodes: []graph.Node{node},
-
-				Severity: osgraph.WarningSeverity,
-				Key:      MissingLivenessProbeWarning,
-				Message: fmt.Sprintf("%s has no liveness probe to verify pods are still running.",
-					topLevelString),
-				Suggestion: osgraph.Suggestion(fmt.Sprintf("%s %s --liveness ...", setProbeCommand, topLevelString)),
-			})
-		}
+		topLevelString := f.ResourceName(topLevelNode)
+		markers = append(markers, osgraph.Marker{
+			Node: podSpecNode,
+
+			Severity: osgraph.WarningSeverity,
+			Key:      MissingLivenessProbeWarning,
+			Message: fmt.Sprintf("%s has no liveness probe to verify pods are still running.",
+				topLevelString),
+			Suggestion: osgraph.Suggestion(fmt.Sprintf("%s %s --liveness ...", setProbeCommand, topLevelString)),
+		})
 	}
 
 	return markers
 }
 
-// CheckForLivenessProbes iterates through all of the containers in a podSpecNode until it finds one
-// with a liveness probe set. The list of nodes whose containers have no liveness probe set is returned.
-func CheckForLivenessProbes(podSpecNode *kubegraph.PodSpecNode) []*kubegraph.PodSpecNode {
-	noLivenessProbes := []*kubegraph.PodSpecNode{}
-
-	hasLivenessProbe := false
+// hasLivenessProbe iterates through all of the containers in a podSpecNode returning true
+// if at least one container has a liveness probe, or false otherwise
+func hasLivenessProbe(podSpecNode *kubegraph.PodSpecNode) bool {
 	for _, container := range podSpecNode.PodSpec.Containers {
 		if container.LivenessProbe != nil {
-			hasLivenessProbe = true
-			break
+			return true
 		}
 	}
+	return false
+}
 
-	if !hasLivenessProbe {
-		noLivenessProbes = append(noLivenessProbes, podSpecNode)
+// hasControllerRefEdge returns true if a given node contains a "ManagedByController" edge between itself and any of its "To" nodes.
+func hasControllerRefEdge(g osgraph.Graph, node graph.Node) bool {
+	children := g.From(node)
+	for _, child := range children {
+		edge := g.Edge(node, child)
+		edgeKinds := g.EdgeKinds(edge)
+		if edgeKinds.Has(kubeedges.ManagedByControllerEdgeKind) {
+			return true
+		}
 	}
-
-	return noLivenessProbes
+	return false
 }
 
 // CheckForUnmountableSecrets checks to be sure that all the referenced secrets are mountable (by service account)
@@ -186,42 +177,3 @@ func CheckMissingMountedSecrets(g osgraph.Graph, podSpecNode *kubegraph.PodSpecN
 
 	return missingSecrets
 }
-
-// findNodesManagedByController returns a list of nodes that have "controllerRef" edge kinds directed at them
-func findNodesManagedByController(g osgraph.Graph) []graph.Node {
-	managedNodes := []graph.Node{}
-
-	// find and append nodes of ManagedByController and
-	// Deployment edge kinds, ignoring top-level nodes.
-	for _, node := range g.NodesByKind(kubegraph.PodSpecNodeKind) {
-		topLevelNode := osgraph.GetTopLevelContainerNode(g, node)
-		managedNodes = append(managedNodes, findManagedNodes(g, topLevelNode, g.To(topLevelNode))...)
-	}
-	return managedNodes
-}
-
-// findManagedNodes traverses up from a podSpecNode until it finds parents with no "controllerRef" edge kinds
-func findManagedNodes(g osgraph.Graph, node graph.Node, parents []graph.Node) []graph.Node {
-	managedNodes := []graph.Node{}
-	for _, parent := range parents {
-		edge := g.Edge(parent, node)
-		kinds := g.EdgeKinds(edge)
-
-		validEdge := kinds.HasAny(kubeedges.ManagedByControllerEdgeKind, deploygraph.DeploymentEdgeKind)
-		if validEdge && !isNodeInList(node, managedNodes) {
-			managedNodes = append(managedNodes, node)
-			managedNodes = append(managedNodes, findManagedNodes(g, parent, g.To(parent))...)
-		}
-	}
-
-	return managedNodes
-}
-
-func isNodeInList(node graph.Node, nodeList []graph.Node) bool {
-	for _, n := range nodeList {
-		if n == node {
-			return true
-		}
-	}
-	return false
-}

pkg/deploy/graph/edges.go

@@ -6,6 +6,7 @@ import (
 	kapi "k8s.io/kubernetes/pkg/api"
 
 	osgraph "github.com/openshift/origin/pkg/api/graph"
+	kubeedges "github.com/openshift/origin/pkg/api/kubegraph"
 	kubegraph "github.com/openshift/origin/pkg/api/kubegraph/nodes"
 	deployapi "github.com/openshift/origin/pkg/deploy/api"
 	deploygraph "github.com/openshift/origin/pkg/deploy/graph/nodes"
@@ -73,6 +74,7 @@ func AddDeploymentEdges(g osgraph.MutableUniqueGraph, node *deploygraph.Deployme
 			}
 			if BelongsToDeploymentConfig(node.DeploymentConfig, rcNode.ReplicationController) {
 				g.AddEdge(node, rcNode, DeploymentEdgeKind)
+				g.AddEdge(rcNode, node, kubeedges.ManagedByControllerEdgeKind)
 			}
 		}
 	}