score/deployment: Adding check for the Deployment replicas to be >=2 …

…if not HPA managed and targeted by Service

Fixes #569

Signed-off-by: BigGold1310 <cyrill.naef@gmail.com>

go.mod

@@ -11,6 +11,7 @@ require (
 	gopkg.in/yaml.v3 v3.0.1
 	k8s.io/api v0.29.0
 	k8s.io/apimachinery v0.29.0
+	k8s.io/utils v0.0.0-20230726121419-3b25d923346b
 )
 
 require (
@@ -29,7 +30,6 @@ require (
 	gopkg.in/inf.v0 v0.9.1 // indirect
 	gopkg.in/yaml.v2 v2.4.0 // indirect
 	k8s.io/klog/v2 v2.110.1 // indirect
-	k8s.io/utils v0.0.0-20230726121419-3b25d923346b // indirect
 	sigs.k8s.io/json v0.0.0-20221116044647-bc3834ca7abd // indirect
 	sigs.k8s.io/structured-merge-diff/v4 v4.4.1 // indirect
 	sigs.k8s.io/yaml v1.3.0 // indirect

score/deployment/deployment.go

@@ -6,10 +6,13 @@ import (
 	"github.com/zegl/kube-score/score/internal"
 	"github.com/zegl/kube-score/scorecard"
 	v1 "k8s.io/api/apps/v1"
+	autoscalingv1 "k8s.io/api/autoscaling/v1"
+	"k8s.io/utils/ptr"
 )
 
 func Register(allChecks *checks.Checks, all ks.AllTypes) {
-	allChecks.RegisterDeploymentCheck("Deployment Strategy", `Makes sure that all Deploymtes targeted by service use RollingUpdate strategy`, deploymentRolloutStrategy(all.Services()))
+	allChecks.RegisterDeploymentCheck("Deployment Strategy", `Makes sure that all Deployments targeted by service use RollingUpdate strategy`, deploymentRolloutStrategy(all.Services()))
+	allChecks.RegisterDeploymentCheck("Deployment Replicas", `Makes sure that Deployment has multiple replicas`, deploymentReplicas(all.Services(), all.HorizontalPodAutoscalers()))
 }
 
 // deploymentRolloutStrategy checks if a Deployment has the update strategy on RollingUpdate if targeted by a service
@@ -33,15 +36,71 @@ func deploymentRolloutStrategy(svcs []ks.Service) func(deployment v1.Deployment)
 		}
 
 		if referencedByService {
-			if deployment.Spec.Strategy.Type == v1.RollingUpdateDeploymentStrategyType {
+			if deployment.Spec.Strategy.Type == v1.RollingUpdateDeploymentStrategyType || deployment.Spec.Strategy.Type == "" {
 				score.Grade = scorecard.GradeAllOK
 			} else {
 				score.Grade = scorecard.GradeWarning
-				score.AddCommentWithURL("", "Deployment update strategy", "The deployment is used by a service but not using rolling update strategy which can cause interruptions", "https://kubernetes.io/docs/concepts/workloads/controllers/deployment/#strategy")
+				score.AddCommentWithURL("", "Deployment update strategy", "The deployment is used by a service but not using the RollingUpdate strategy which can cause interruptions. Set .spec.strategy.type to RollingUpdate.", "https://kubernetes.io/docs/concepts/workloads/controllers/deployment/#strategy")
 			}
 		} else {
 			score.Skipped = true
-			score.AddComment("", "Skipped as the deployment strategy does not matter if not targeted by a service", "")
+			score.AddComment("", "Skipped as the Deployment is not targeted by a service", "")
+		}
+
+		return
+	}
+}
+
+// deploymentReplicas checks if a Deployment has >= 2 replicas if not (targeted by service || has HorizontalPodAutoscaler)
+func deploymentReplicas(svcs []ks.Service, hpas []ks.HpaTargeter) func(deployment v1.Deployment) (scorecard.TestScore, error) {
+	svcsInNamespace := make(map[string][]map[string]string)
+	for _, s := range svcs {
+		svc := s.Service()
+		if _, ok := svcsInNamespace[svc.Namespace]; !ok {
+			svcsInNamespace[svc.Namespace] = []map[string]string{}
+		}
+		svcsInNamespace[svc.Namespace] = append(svcsInNamespace[svc.Namespace], svc.Spec.Selector)
+	}
+	hpasInNamespace := make(map[string][]autoscalingv1.CrossVersionObjectReference)
+	for _, hpa := range hpas {
+		hpaTarget := hpa.HpaTarget()
+		hpaMeta := hpa.GetObjectMeta()
+		if _, ok := hpasInNamespace[hpaMeta.Namespace]; !ok {
+			hpasInNamespace[hpaMeta.Namespace] = []autoscalingv1.CrossVersionObjectReference{}
+		}
+		hpasInNamespace[hpaMeta.Namespace] = append(hpasInNamespace[hpaMeta.Namespace], hpaTarget)
+	}
+
+	return func(deployment v1.Deployment) (score scorecard.TestScore, err error) {
+		referencedByService := false
+		hasHPA := false
+
+		for _, svcSelector := range svcsInNamespace[deployment.Namespace] {
+			if internal.LabelSelectorMatchesLabels(svcSelector, deployment.Spec.Template.Labels) {
+				referencedByService = true
+				break
+			}
+		}
+
+		for _, hpaTarget := range hpasInNamespace[deployment.Namespace] {
+			if deployment.TypeMeta.APIVersion == hpaTarget.APIVersion &&
+				deployment.TypeMeta.Kind == hpaTarget.Kind &&
+				deployment.ObjectMeta.Name == hpaTarget.Name {
+				hasHPA = true
+				break
+			}
+		}
+
+		if !referencedByService || hasHPA {
+			score.Skipped = true
+			score.AddComment("", "Skipped as the Deployment is not targeted by service or is controlled by a HorizontalPodAutoscaler", "")
+		} else {
+			if ptr.Deref(deployment.Spec.Replicas, 1) >= 2 {
+				score.Grade = scorecard.GradeAllOK
+			} else {
+				score.Grade = scorecard.GradeWarning
+				score.AddComment("", "Deployment few replicas", "Deployments targeted by Services are recommended to have at least 2 replicas to prevent unwanted downtime.")
+			}
 		}
 
 		return

score/deployment_test.go

@@ -1,11 +1,11 @@
 package score
 
 import (
+	"testing"
+
 	"github.com/stretchr/testify/assert"
 	"github.com/zegl/kube-score/config"
 	ks "github.com/zegl/kube-score/domain"
-	"testing"
-
 	"github.com/zegl/kube-score/scorecard"
 )
 
@@ -16,7 +16,6 @@ func TestServiceTargetsDeploymentStrategyRolling(t *testing.T) {
 
 func TestServiceNotTargetsDeploymentStrategyNotRelevant(t *testing.T) {
 	t.Parallel()
-	// Expecting score 0 as it didn't cot rated
 	skipped := wasSkipped(t, config.Configuration{
 		AllFiles: []ks.NamedReader{testFile("service-not-target-deployment.yaml")},
 	}, "Deployment Strategy")
@@ -30,5 +29,31 @@ func TestServiceTargetsDeploymentStrategyNotRolling(t *testing.T) {
 
 func TestServiceTargetsDeploymentStrategyNotSet(t *testing.T) {
 	t.Parallel()
-	testExpectedScore(t, "service-target-deployment-strategy-not-set.yaml", "Deployment Strategy", scorecard.GradeWarning)
+	testExpectedScore(t, "service-target-deployment-strategy-not-set.yaml", "Deployment Strategy", scorecard.GradeAllOK)
+}
+
+func TestServiceTargetsDeploymentReplicasOk(t *testing.T) {
+	t.Parallel()
+	testExpectedScore(t, "service-target-deployment.yaml", "Deployment Replicas", scorecard.GradeAllOK)
+}
+
+func TestServiceNotTargetsDeploymentReplicasNotRelevant(t *testing.T) {
+	t.Parallel()
+	skipped := wasSkipped(t, config.Configuration{
+		AllFiles: []ks.NamedReader{testFile("service-not-target-deployment.yaml")},
+	}, "Deployment Replicas")
+	assert.True(t, skipped)
+}
+
+func TestServiceTargetsDeploymentReplicasNok(t *testing.T) {
+	t.Parallel()
+	testExpectedScore(t, "service-target-deployment-replica-1.yaml", "Deployment Replicas", scorecard.GradeWarning)
+}
+
+func TestHPATargetsDeployment(t *testing.T) {
+	t.Parallel()
+	skipped := wasSkipped(t, config.Configuration{
+		AllFiles: []ks.NamedReader{testFile("hpa-target-deployment.yaml")},
+	}, "Deployment Replicas")
+	assert.True(t, skipped)
 }

score/testdata/hpa-target-deployment.yaml

@@ -0,0 +1,32 @@
+apiVersion: autoscaling/v1
+kind: HorizontalPodAutoscaler
+metadata:
+  name: php-apache
+  namespace: default
+spec:
+  scaleTargetRef:
+    apiVersion: apps/v1
+    kind: Deployment
+    name: php-apache
+  minReplicas: 1
+  maxReplicas: 10
+  metrics:
+    - type: Resource
+      resource:
+        name: cpu
+        target:
+          type: Utilization
+          averageUtilization: 50
+---
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: php-apache
+  namespace: default
+spec:
+  replicas: 1
+  template:
+    spec:
+      containers:
+        - name: foo
+          image: foo:latest

score/testdata/service-target-deployment-replica-1.yaml

@@ -0,0 +1,28 @@
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: deployment-test-1
+spec:
+  template:
+    metadata:
+      labels:
+        app: my-app
+    spec:
+      containers:
+      - name: foobar
+        image: foo/bar:123
+  replicas: 1
+  strategy:
+    type: RollingUpdate
+---
+kind: Service
+apiVersion: v1
+metadata:
+  name: my-service
+spec:
+  selector:
+    app: my-app
+  ports:
+  - protocol: TCP
+    port: 80
+    targetPort: 8080

score/testdata/service-target-deployment.yaml

@@ -11,6 +11,7 @@ spec:
       containers:
       - name: foobar
         image: foo/bar:123
+  replicas: 2
   strategy:
     type: RollingUpdate
 ---