score/hpa: added a check for hpa min replicas > 1

Fixes #581

Author: ReuDa

domain/kube-score.go

@@ -2,9 +2,9 @@ package domain
 
 import (
 	"io"
+	autoscalingv1 "k8s.io/api/autoscaling/v1"
 
 	appsv1 "k8s.io/api/apps/v1"
-	autoscalingv1 "k8s.io/api/autoscaling/v1"
 	corev1 "k8s.io/api/core/v1"
 	networkingv1 "k8s.io/api/networking/v1"
 	policyv1 "k8s.io/api/policy/v1"
@@ -49,6 +49,7 @@ type FileLocationer interface {
 type HpaTargeter interface {
 	GetTypeMeta() metav1.TypeMeta
 	GetObjectMeta() metav1.ObjectMeta
+	MinReplicas() *int32
 	HpaTarget() autoscalingv1.CrossVersionObjectReference
 	FileLocationer
 }

parser/internal/hpa.go

@@ -26,6 +26,9 @@ func (d HPAv1) GetTypeMeta() metav1.TypeMeta {
 func (d HPAv1) GetObjectMeta() metav1.ObjectMeta {
 	return d.ObjectMeta
 }
+func (d HPAv1) MinReplicas() *int32 {
+	return d.Spec.MinReplicas
+}
 
 func (d HPAv1) HpaTarget() autoscalingv1.CrossVersionObjectReference {
 	return d.Spec.ScaleTargetRef
@@ -48,6 +51,10 @@ func (d HPAv2beta1) GetObjectMeta() metav1.ObjectMeta {
 	return d.ObjectMeta
 }
 
+func (d HPAv2beta1) MinReplicas() *int32 {
+	return d.Spec.MinReplicas
+}
+
 func (d HPAv2beta1) HpaTarget() autoscalingv1.CrossVersionObjectReference {
 	return autoscalingv1.CrossVersionObjectReference(d.Spec.ScaleTargetRef)
 }
@@ -69,6 +76,10 @@ func (d HPAv2beta2) GetObjectMeta() metav1.ObjectMeta {
 	return d.ObjectMeta
 }
 
+func (d HPAv2beta2) MinReplicas() *int32 {
+	return d.Spec.MinReplicas
+}
+
 func (d HPAv2beta2) HpaTarget() autoscalingv1.CrossVersionObjectReference {
 	return autoscalingv1.CrossVersionObjectReference(d.Spec.ScaleTargetRef)
 }
@@ -90,6 +101,10 @@ func (d HPAv2) GetObjectMeta() metav1.ObjectMeta {
 	return d.ObjectMeta
 }
 
+func (d HPAv2) MinReplicas() *int32 {
+	return d.Spec.MinReplicas
+}
+
 func (d HPAv2) HpaTarget() autoscalingv1.CrossVersionObjectReference {
 	return autoscalingv1.CrossVersionObjectReference(d.Spec.ScaleTargetRef)
 }

score/apps/apps_test.go

@@ -355,6 +355,10 @@ func (d hpav1) GetObjectMeta() metav1.ObjectMeta {
 	return d.ObjectMeta
 }
 
+func (d hpav1) MinReplicas() *int32 {
+	return d.Spec.MinReplicas
+}
+
 func (d hpav1) HpaTarget() autoscalingv1.CrossVersionObjectReference {
 	return d.Spec.ScaleTargetRef
 }

score/deployment/deployment.go

@@ -91,9 +91,12 @@ func deploymentReplicas(svcs []ks.Service, hpas []ks.HpaTargeter) func(deploymen
 			}
 		}
 
-		if !referencedByService || hasHPA {
+		if !referencedByService {
 			score.Skipped = true
-			score.AddComment("", "Skipped as the Deployment is not targeted by service or is controlled by a HorizontalPodAutoscaler", "")
+			score.AddComment("", "Skipped as the Deployment is not targeted by service", "")
+		} else if hasHPA {
+			score.Skipped = true
+			score.AddComment("", "Skipped as the Deployment is controlled by a HorizontalPodAutoscaler", "")
 		} else {
 			if ptr.Deref(deployment.Spec.Replicas, 1) >= 2 {
 				score.Grade = scorecard.GradeAllOK

score/deployment_test.go

@@ -39,10 +39,14 @@ func TestServiceTargetsDeploymentReplicasOk(t *testing.T) {
 
 func TestServiceNotTargetsDeploymentReplicasNotRelevant(t *testing.T) {
 	t.Parallel()
-	skipped := wasSkipped(t, config.Configuration{
+	assert.True(t, wasSkipped(t, config.Configuration{
+		AllFiles: []ks.NamedReader{testFile("service-not-target-deployment.yaml")},
+	}, "Deployment Replicas"))
+
+	summaries := getSummaries(t, config.Configuration{
 		AllFiles: []ks.NamedReader{testFile("service-not-target-deployment.yaml")},
 	}, "Deployment Replicas")
-	assert.True(t, skipped)
+	assert.Contains(t, summaries, "Skipped as the Deployment is not targeted by service")
 }
 
 func TestServiceTargetsDeploymentReplicasNok(t *testing.T) {
@@ -52,8 +56,12 @@ func TestServiceTargetsDeploymentReplicasNok(t *testing.T) {
 
 func TestHPATargetsDeployment(t *testing.T) {
 	t.Parallel()
-	skipped := wasSkipped(t, config.Configuration{
+	assert.True(t, wasSkipped(t, config.Configuration{
+		AllFiles: []ks.NamedReader{testFile("hpa-target-deployment.yaml")},
+	}, "Deployment Replicas"))
+
+	summaries := getSummaries(t, config.Configuration{
 		AllFiles: []ks.NamedReader{testFile("hpa-target-deployment.yaml")},
 	}, "Deployment Replicas")
-	assert.True(t, skipped)
+	assert.Contains(t, summaries, "Skipped as the Deployment is controlled by a HorizontalPodAutoscaler")
 }

score/hpa/hpa.go

@@ -4,10 +4,12 @@ import (
 	"github.com/zegl/kube-score/domain"
 	"github.com/zegl/kube-score/score/checks"
 	"github.com/zegl/kube-score/scorecard"
+	"k8s.io/utils/ptr"
 )
 
 func Register(allChecks *checks.Checks, allTargetableObjs []domain.BothMeta) {
 	allChecks.RegisterHorizontalPodAutoscalerCheck("HorizontalPodAutoscaler has target", `Makes sure that the HPA targets a valid object`, hpaHasTarget(allTargetableObjs))
+	allChecks.RegisterHorizontalPodAutoscalerCheck("HorizontalPodAutoscaler Replicas", `Makes sure that the HPA has multiple replicas`, hpaHasMultipleReplicas())
 }
 
 func hpaHasTarget(allTargetableObjs []domain.BothMeta) func(hpa domain.HpaTargeter) (scorecard.TestScore, error) {
@@ -33,3 +35,15 @@ func hpaHasTarget(allTargetableObjs []domain.BothMeta) func(hpa domain.HpaTarget
 		return
 	}
 }
+
+func hpaHasMultipleReplicas() func(hpa domain.HpaTargeter) (scorecard.TestScore, error) {
+	return func(hpa domain.HpaTargeter) (score scorecard.TestScore, err error) {
+		if ptr.Deref(hpa.MinReplicas(), 1) >= 2 {
+			score.Grade = scorecard.GradeAllOK
+		} else {
+			score.Grade = scorecard.GradeWarning
+			score.AddComment("", "HPA few replicas", "HorizontalPodAutoscalers are recommended to have at least 2 replicas to prevent unwanted downtime.")
+		}
+		return
+	}
+}

score/hpa/hpa_test.go

@@ -177,6 +177,10 @@ func (d hpav1) GetObjectMeta() metav1.ObjectMeta {
 	return d.ObjectMeta
 }
 
+func (d hpav1) MinReplicas() *int32 {
+	return d.Spec.MinReplicas
+}
+
 func (d hpav1) HpaTarget() v1.CrossVersionObjectReference {
 	return d.Spec.ScaleTargetRef
 }

score/hpa_test.go

@@ -20,3 +20,13 @@ func TestHorizontalPodAutoscalerHasNoTarget(t *testing.T) {
 	t.Parallel()
 	testExpectedScore(t, "hpa-has-no-target.yaml", "HorizontalPodAutoscaler has target", scorecard.GradeCritical)
 }
+
+func TestHorizontalPodAutoscalerMinReplicasOk(t *testing.T) {
+	t.Parallel()
+	testExpectedScore(t, "hpa-min-replicas-ok.yaml", "HorizontalPodAutoscaler Replicas", scorecard.GradeAllOK)
+}
+
+func TestHorizontalPodAutoscalerMinReplicasNok(t *testing.T) {
+	t.Parallel()
+	testExpectedScore(t, "hpa-min-replicas-nok.yaml", "HorizontalPodAutoscaler Replicas", scorecard.GradeWarning)
+}

score/score_test.go

@@ -54,6 +54,25 @@ func wasSkipped(t *testing.T, config config.Configuration, testcase string) bool
 	return false
 }
 
+func getSummaries(t *testing.T, config config.Configuration, testcase string) []string {
+	sc, err := testScore(config)
+	assert.NoError(t, err)
+	var summaries []string
+	for _, objectScore := range sc {
+		for _, s := range objectScore.Checks {
+			if s.Check.Name == testcase {
+				for _, c := range s.Comments {
+					summaries = append(summaries, c.Summary)
+				}
+				return summaries
+			}
+		}
+	}
+
+	assert.Fail(t, "test was not run")
+	return summaries
+}
+
 func testScore(config config.Configuration) (scorecard.Scorecard, error) {
 	p, err := parser.New()
 	if err != nil {

score/testdata/hpa-min-replicas-nok.yaml

@@ -0,0 +1,31 @@
+apiVersion: autoscaling/v2
+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:
+  template:
+    spec:
+      containers:
+        - name: foo
+          image: foo:latest

score/testdata/hpa-min-replicas-ok.yaml

@@ -0,0 +1,31 @@
+apiVersion: autoscaling/v2
+kind: HorizontalPodAutoscaler
+metadata:
+  name: php-apache
+  namespace: default
+spec:
+  scaleTargetRef:
+    apiVersion: apps/v1
+    kind: Deployment
+    name: php-apache
+  minReplicas: 2
+  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:
+  template:
+    spec:
+      containers:
+        - name: foo
+          image: foo:latest

score/testdata/hpa-target-deployment.yaml

@@ -26,7 +26,23 @@ metadata:
 spec:
   replicas: 1
   template:
+    metadata:
+      labels:
+        app: my-app
     spec:
       containers:
         - name: foo
-          image: foo:latest
+          image: foo:latest
+---
+kind: Service
+apiVersion: v1
+metadata:
+  name: my-service
+  namespace: default
+spec:
+  selector:
+    app: my-app
+  ports:
+    - protocol: TCP
+      port: 80
+      targetPort: 8080