Merge pull request #10363 from juanvallejo/jvallejo_emit-warning-no-liveness-probe

Merged by openshift-bot

Author: OpenShift Bot

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

@@ -11,8 +11,9 @@ import (
 )
 
 const (
-	UnmountableSecretWarning = "UnmountableSecret"
-	MissingSecretWarning     = "MissingSecret"
+	UnmountableSecretWarning    = "UnmountableSecret"
+	MissingSecretWarning        = "MissingSecret"
+	MissingLivenessProbeWarning = "MissingLivenessProbe"
 )
 
 // FindUnmountableSecrets inspects all PodSpecs for any Secret reference that isn't listed as mountable by the referenced ServiceAccount
@@ -75,6 +76,58 @@ func FindMissingSecrets(g osgraph.Graph, f osgraph.Namer) []osgraph.Marker {
 	return markers
 }
 
+// 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{}
+
+	for _, uncastPodSpecNode := range g.NodesByKind(kubegraph.PodSpecNodeKind) {
+		podSpecNode := uncastPodSpecNode.(*kubegraph.PodSpecNode)
+		if hasLivenessProbe(podSpecNode) {
+			continue
+		}
+
+		topLevelNode := osgraph.GetTopLevelContainerNode(g, podSpecNode)
+
+		// skip any podSpec nodes that are managed by other nodes.
+		// Liveness probes should only be applied to a controlling
+		// podSpec node, and not to any of its children.
+		if hasControllerRefEdge(g, topLevelNode) {
+			continue
+		}
+
+		topLevelString := f.ResourceName(topLevelNode)
+		markers = append(markers, osgraph.Marker{
+			Node:         podSpecNode,
+			RelatedNodes: []graph.Node{topLevelNode},
+
+			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
+}
+
+// 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 {
+			return true
+		}
+	}
+	return false
+}
+
+// hasControllerRefEdge returns true if a given node contains one or more "ManagedByController" outbound edges
+func hasControllerRefEdge(g osgraph.Graph, node graph.Node) bool {
+	managedEdges := g.OutboundEdges(node, kubeedges.ManagedByControllerEdgeKind)
+	return len(managedEdges) > 0
+}
+
 // CheckForUnmountableSecrets checks to be sure that all the referenced secrets are mountable (by service account)
 func CheckForUnmountableSecrets(g osgraph.Graph, podSpecNode *kubegraph.PodSpecNode) []*kubegraph.SecretNode {
 	saNodes := g.SuccessorNodesByNodeAndEdgeKind(podSpecNode, kubegraph.ServiceAccountNodeKind, kubeedges.ReferencedServiceAccountEdgeKind)

pkg/api/kubegraph/analysis/podspec_test.go

@@ -80,3 +80,23 @@ func TestUnmountableSecrets(t *testing.T) {
 		t.Errorf("expected %v, got %v", expectedSecret2, markers)
 	}
 }
+
+func TestMissingLivenessProbes(t *testing.T) {
+	g, _, err := osgraphtest.BuildGraph("../../../api/graph/test/simple-deployment.yaml")
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+
+	kubeedges.AddAllExposedPodEdges(g)
+
+	markers := FindMissingLivenessProbes(g, osgraph.DefaultNamer, "oc set probe")
+	if e, a := 1, len(markers); e != a {
+		t.Fatalf("expected %v, got %v", e, a)
+	}
+
+	actualDC := osgraph.GetTopLevelContainerNode(g, markers[0].Node)
+	expectedDC := g.Find(osgraph.UniqueName("DeploymentConfig|/simple-deployment"))
+	if e, a := expectedDC.ID(), actualDC.ID(); e != a {
+		t.Errorf("expected %v, got %v", e, a)
+	}
+}

pkg/cmd/cli/describe/projectstatus.go

@@ -383,6 +383,9 @@ func getMarkerScanners(logsCommandName, securityPolicyCommandFormat, setProbeCom
 		func(g osgraph.Graph, f osgraph.Namer) []osgraph.Marker {
 			return deployanalysis.FindDeploymentConfigReadinessWarnings(g, f, setProbeCommandName)
 		},
+		func(g osgraph.Graph, f osgraph.Namer) []osgraph.Marker {
+			return kubeanalysis.FindMissingLivenessProbes(g, f, setProbeCommandName)
+		},
 		routeanalysis.FindPortMappingIssues,
 		routeanalysis.FindMissingTLSTerminationType,
 		routeanalysis.FindPathBasedPassthroughRoutes,

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)
 			}
 		}
 	}

test/cmd/set-liveness-probe.sh

@@ -0,0 +1,21 @@
+#!/bin/bash
+source "$(dirname "${BASH_SOURCE}")/../../hack/lib/init.sh"
+trap os::test::junit::reconcile_output EXIT
+
+# Cleanup cluster resources created by this test
+(
+  set +e
+  oc delete all,templates --all
+  exit 0
+) &>/dev/null
+
+
+os::test::junit::declare_suite_start "cmd/set-probe-liveness"
+# This test setting a liveness probe, without warning about replication controllers whose deployment depends on deployment configs
+os::cmd::expect_success_and_text 'oc create -f pkg/api/graph/test/simple-deployment.yaml' 'deploymentconfig "simple-deployment" created'
+os::cmd::expect_success_and_text 'oc status -v' 'dc/simple-deployment has no liveness probe'
+os::cmd::expect_success_and_not_text 'oc status -v' 'rc/simple-deployment-1 has no liveness probe'
+os::cmd::expect_success_and_text 'oc set probe dc/simple-deployment --liveness --get-url=http://google.com:80' 'deploymentconfig "simple-deployment" updated'
+os::cmd::expect_success_and_not_text 'oc status -v' 'dc/simple-deployment has no liveness probe'
+echo "set-probe-liveness: ok"
+os::test::junit::declare_suite_end