Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: make the required replicas configurable #633

Merged
merged 1 commit into from
Nov 25, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions README_CHECKS.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
| ID | Target | Description | Enabled |
|----|--------|-------------|---------|
| deployment-strategy | Deployment | Makes sure that all Deployments targeted by service use RollingUpdate strategy | default |
| deployment-replicas | Deployment | Makes sure that Deployment has multiple replicas | default |
| deployment-replicas | Deployment | Makes sure that Deployment has multiple replicas. The --min-replicas-deployment flag can be used to specify the required minimum. Default is 2. | default |
| ingress-targets-service | Ingress | Makes sure that the Ingress targets a Service | default |
| cronjob-has-deadline | CronJob | Makes sure that all CronJobs has a configured deadline | default |
| cronjob-restartpolicy | CronJob | Makes sure CronJobs have a valid RestartPolicy | default |
Expand Down Expand Up @@ -37,5 +37,5 @@
| statefulset-pod-selector-labels-match-template-metadata-labels | StatefulSet | Ensure the StatefulSet selector labels match the template metadata labels. | default |
| label-values | all | Validates label values | default |
| horizontalpodautoscaler-has-target | HorizontalPodAutoscaler | Makes sure that the HPA targets a valid object | default |
| horizontalpodautoscaler-replicas | HorizontalPodAutoscaler | Makes sure that the HPA at least 2 replicas | default |
| horizontalpodautoscaler-replicas | HorizontalPodAutoscaler | Makes sure that the HPA has multiple replicas. The --min-replicas-hpa flag can be used to specify the required minimum. Default is 2. | default |
| pod-topology-spread-constraints | Pod | Pod Topology Spread Constraints | default |
4 changes: 4 additions & 0 deletions cmd/kube-score/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,8 @@ func scoreFiles(binName string, args []string) error {
disableOptionalChecksAnnotation := fs.Bool("disable-optional-checks-annotations", false, "Set to true to disable the effect of the 'kube-score/enable' annotations")
allDefaultOptional := fs.Bool("all-default-optional", false, "Set to true to enable all tests")
kubernetesVersion := fs.String("kubernetes-version", "v1.18", "Setting the kubernetes-version will affect the checks ran against the manifests. Set this to the version of Kubernetes that you're using in production for the best results.")
minReplicasDeployment := fs.Int("min-replicas-deployment", 2, "Minimum required number of replicas for a deployment")
minReplicasHPA := fs.Int("min-replicas-hpa", 2, "Minimum required number of replicas for a horizontal pod autoscaler")
setDefault(fs, binName, "score", false)

err := fs.Parse(args)
Expand Down Expand Up @@ -199,6 +201,8 @@ Use "-" as filename to read from STDIN.`, execName(binName))
UseIgnoreChecksAnnotation: !*disableIgnoreChecksAnnotation,
UseOptionalChecksAnnotation: !*disableOptionalChecksAnnotation,
KubernetesVersion: kubeVer,
MinReplicasDeployment: *minReplicasDeployment,
MinReplicasHPA: *minReplicasHPA,
}

p, err := parser.New(&parser.Config{
Expand Down
2 changes: 2 additions & 0 deletions config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ type RunConfiguration struct {
UseIgnoreChecksAnnotation bool
UseOptionalChecksAnnotation bool
KubernetesVersion Semver
MinReplicasDeployment int
MinReplicasHPA int
}

type Semver struct {
Expand Down
11 changes: 6 additions & 5 deletions score/deployment/deployment.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package deployment

import (
"fmt"
ks "github.com/zegl/kube-score/domain"
"github.com/zegl/kube-score/score/checks"
"github.com/zegl/kube-score/score/internal"
Expand All @@ -10,9 +11,9 @@ import (
"k8s.io/utils/ptr"
)

func Register(allChecks *checks.Checks, all ks.AllTypes) {
func Register(allChecks *checks.Checks, all ks.AllTypes, minReplicas int) {
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()))
allChecks.RegisterDeploymentCheck("Deployment Replicas", `Makes sure that Deployment has multiple replicas`, deploymentReplicas(all.Services(), all.HorizontalPodAutoscalers(), minReplicas))
}

// deploymentRolloutStrategy checks if a Deployment has the update strategy on RollingUpdate if targeted by a service
Expand Down Expand Up @@ -52,7 +53,7 @@ func deploymentRolloutStrategy(svcs []ks.Service) func(deployment v1.Deployment)
}

// 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) {
func deploymentReplicas(svcs []ks.Service, hpas []ks.HpaTargeter, minReplicas int) func(deployment v1.Deployment) (scorecard.TestScore, error) {
svcsInNamespace := make(map[string][]map[string]string)
for _, s := range svcs {
svc := s.Service()
Expand Down Expand Up @@ -98,11 +99,11 @@ func deploymentReplicas(svcs []ks.Service, hpas []ks.HpaTargeter) func(deploymen
score.Skipped = true
score.AddComment("", "Skipped as the Deployment is controlled by a HorizontalPodAutoscaler", "")
} else {
if ptr.Deref(deployment.Spec.Replicas, 1) >= 2 {
if ptr.Deref(deployment.Spec.Replicas, 1) >= int32(minReplicas) {
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.")
score.AddComment("", "Deployment few replicas", fmt.Sprintf("Deployments targeted by Services are recommended to have at least %d replicas to prevent unwanted downtime.", minReplicas))
}
}

Expand Down
5 changes: 4 additions & 1 deletion score/deployment_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package score

import (
"github.com/zegl/kube-score/config"
"testing"

"github.com/stretchr/testify/assert"
Expand Down Expand Up @@ -49,7 +50,9 @@ func TestServiceNotTargetsDeploymentReplicasNotRelevant(t *testing.T) {

func TestServiceTargetsDeploymentReplicasNok(t *testing.T) {
t.Parallel()
testExpectedScore(t, "service-target-deployment-replica-1.yaml", "Deployment Replicas", scorecard.GradeWarning)
testExpectedScoreWithConfig(t, []ks.NamedReader{testFile("service-target-deployment-replica-1.yaml")}, nil, &config.RunConfiguration{
MinReplicasDeployment: 2,
}, "Deployment Replicas", scorecard.GradeWarning)
}

func TestHPATargetsDeployment(t *testing.T) {
Expand Down
11 changes: 6 additions & 5 deletions score/hpa/hpa.go
Original file line number Diff line number Diff line change
@@ -1,15 +1,16 @@
package hpa

import (
"fmt"
"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) {
func Register(allChecks *checks.Checks, allTargetableObjs []domain.BothMeta, minReplicas int) {
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())
allChecks.RegisterHorizontalPodAutoscalerCheck("HorizontalPodAutoscaler Replicas", `Makes sure that the HPA has multiple replicas`, hpaHasMultipleReplicas(minReplicas))
}

func hpaHasTarget(allTargetableObjs []domain.BothMeta) func(hpa domain.HpaTargeter) (scorecard.TestScore, error) {
Expand All @@ -36,13 +37,13 @@ func hpaHasTarget(allTargetableObjs []domain.BothMeta) func(hpa domain.HpaTarget
}
}

func hpaHasMultipleReplicas() func(hpa domain.HpaTargeter) (scorecard.TestScore, error) {
func hpaHasMultipleReplicas(minReplicas int) func(hpa domain.HpaTargeter) (scorecard.TestScore, error) {
return func(hpa domain.HpaTargeter) (score scorecard.TestScore, err error) {
if ptr.Deref(hpa.MinReplicas(), 1) >= 2 {
if ptr.Deref(hpa.MinReplicas(), 1) >= int32(minReplicas) {
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.")
score.AddComment("", "HPA few replicas", fmt.Sprintf("HorizontalPodAutoscalers are recommended to have at least %d replicas to prevent unwanted downtime.", minReplicas))
}
return
}
Expand Down
6 changes: 5 additions & 1 deletion score/hpa_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package score

import (
"github.com/zegl/kube-score/config"
ks "github.com/zegl/kube-score/domain"
"testing"

"github.com/zegl/kube-score/scorecard"
Expand Down Expand Up @@ -28,5 +30,7 @@ func TestHorizontalPodAutoscalerMinReplicasOk(t *testing.T) {

func TestHorizontalPodAutoscalerMinReplicasNok(t *testing.T) {
t.Parallel()
testExpectedScore(t, "hpa-min-replicas-nok.yaml", "HorizontalPodAutoscaler Replicas", scorecard.GradeWarning)
testExpectedScoreWithConfig(t, []ks.NamedReader{testFile("hpa-min-replicas-nok.yaml")}, nil, &config.RunConfiguration{
MinReplicasHPA: 2,
}, "HorizontalPodAutoscaler Replicas", scorecard.GradeWarning)
}
4 changes: 2 additions & 2 deletions score/score.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ func RegisterAllChecks(allObjects ks.AllTypes, checksConfig *checks.Config, runC
runConfig = &config.RunConfiguration{}
}

deployment.Register(allChecks, allObjects)
deployment.Register(allChecks, allObjects, runConfig.MinReplicasDeployment)
ingress.Register(allChecks, allObjects)
cronjob.Register(allChecks)
container.Register(allChecks, runConfig.IgnoreContainerCpuLimitRequirement, runConfig.IgnoreContainerMemoryLimitRequirement)
Expand All @@ -44,7 +44,7 @@ func RegisterAllChecks(allObjects ks.AllTypes, checksConfig *checks.Config, runC
stable.Register(runConfig.KubernetesVersion, allChecks)
apps.Register(allChecks, allObjects.HorizontalPodAutoscalers(), allObjects.Services())
meta.Register(allChecks)
hpa.Register(allChecks, allObjects.Metas())
hpa.Register(allChecks, allObjects.Metas(), runConfig.MinReplicasHPA)
podtopologyspreadconstraints.Register(allChecks)

return allChecks
Expand Down
Loading