diff --git a/domain/kube-score.go b/domain/kube-score.go index 6e54d1b0..ffe6b0cf 100644 --- a/domain/kube-score.go +++ b/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 } diff --git a/parser/internal/hpa.go b/parser/internal/hpa.go index bd619d18..31108814 100644 --- a/parser/internal/hpa.go +++ b/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) } diff --git a/score/apps/apps_test.go b/score/apps/apps_test.go index 1994ea05..76561d62 100644 --- a/score/apps/apps_test.go +++ b/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 } diff --git a/score/deployment/deployment.go b/score/deployment/deployment.go index 8b202bd7..a1f2f9dd 100644 --- a/score/deployment/deployment.go +++ b/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 diff --git a/score/deployment_test.go b/score/deployment_test.go index 38373cd7..fa5ccf9a 100644 --- a/score/deployment_test.go +++ b/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") } diff --git a/score/hpa/hpa.go b/score/hpa/hpa.go index 7414fbf2..63e56372 100644 --- a/score/hpa/hpa.go +++ b/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 + } +} diff --git a/score/hpa/hpa_test.go b/score/hpa/hpa_test.go index 8b4be191..55ab3c7b 100644 --- a/score/hpa/hpa_test.go +++ b/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 } diff --git a/score/hpa_test.go b/score/hpa_test.go index 6b0307ac..30d47f7b 100644 --- a/score/hpa_test.go +++ b/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) +} diff --git a/score/score_test.go b/score/score_test.go index 4bcfdd6e..10860954 100644 --- a/score/score_test.go +++ b/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 { diff --git a/score/testdata/hpa-min-replicas-nok.yaml b/score/testdata/hpa-min-replicas-nok.yaml new file mode 100644 index 00000000..9a7c03cf --- /dev/null +++ b/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 \ No newline at end of file diff --git a/score/testdata/hpa-min-replicas-ok.yaml b/score/testdata/hpa-min-replicas-ok.yaml new file mode 100644 index 00000000..b40d1782 --- /dev/null +++ b/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 \ No newline at end of file diff --git a/score/testdata/hpa-target-deployment.yaml b/score/testdata/hpa-target-deployment.yaml index 85362697..d2893a2e 100644 --- a/score/testdata/hpa-target-deployment.yaml +++ b/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 \ No newline at end of file + 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 \ No newline at end of file