Skip to content

Commit

Permalink
bug: not scaling up from 0 replicas using hpa (#544)
Browse files Browse the repository at this point in the history
* bug: not scaling up from 0 replicas using hpa

* clean up logic a bit
  • Loading branch information
martinhny authored Sep 27, 2024
1 parent 12e46cd commit 1817472
Show file tree
Hide file tree
Showing 5 changed files with 67 additions and 2 deletions.
16 changes: 16 additions & 0 deletions pkg/resourcegenerator/deployment/deployment_test.go
Original file line number Diff line number Diff line change
@@ -1,9 +1,12 @@
package deployment

import (
"github.com/kartverket/skiperator/api/v1alpha1"
"github.com/kartverket/skiperator/pkg/testutil"
"github.com/kartverket/skiperator/pkg/util"
"github.com/stretchr/testify/assert"
appsv1 "k8s.io/api/apps/v1"
apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
"testing"
)

Expand All @@ -21,3 +24,16 @@ func TestDeploymentMinimalAppShouldHaveLabels(t *testing.T) {
assert.Equal(t, appLabel["app"], depl.Spec.Selector.MatchLabels["app"])
assert.Equal(t, appLabel["app"], depl.Spec.Template.Labels["app"])
}

func TestHPAReplicasIsZero(t *testing.T) {
// Setup
r := testutil.GetTestMinimalAppReconciliation()
r.GetSKIPObject().(*v1alpha1.Application).Spec.Replicas = &apiextensionsv1.JSON{Raw: []byte(`{"min": 0, "max": 0, "targetCpuUtilization": 200}`)}
// Test
err := Generate(r)

// Assert
assert.Nil(t, err)
depl := r.GetResources()[0].(*appsv1.Deployment)
assert.Equal(t, depl.Spec.Replicas, util.PointTo(int32(0)))
}
8 changes: 8 additions & 0 deletions pkg/resourceprocessor/resource.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,16 @@ func preparePatch(new client.Object, old client.Object) {
case *v1.Deployment:
deployment := old.(*v1.Deployment)
definition := new.(*v1.Deployment)

// Handling HPA.
// If the replicas field is not set in the definition, we should set it to 1 if the deployment has 0 replicas or HPA will not work.
// If the replicas field is set in the definition, we should use that value so we don't scale down.
if definition.Spec.Replicas == nil {
definition.Spec.Replicas = deployment.Spec.Replicas

if *deployment.Spec.Replicas == int32(0) {
definition.Spec.Replicas = util.PointTo(int32(1))
}
}
// The command "kubectl rollout restart" puts an annotation on the deployment template in order to track
// rollouts of different replicasets. This annotation must not trigger a new reconcile, and a quick and easy
Expand Down
7 changes: 5 additions & 2 deletions tests/application/hpa/chainsaw-test.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -31,5 +31,8 @@ spec:
file: patch-application-set-0-assert.yaml
- error:
file: patch-application-set-0-error.yaml


- try:
- apply:
file: patch-application-scale-up-from-0.yaml
- assert:
file: patch-application-scale-up-from-0-assert.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
apiVersion: apps/v1
kind: Deployment
metadata:
name: test-deployment-hpa
spec:
replicas: 2

---
apiVersion: autoscaling/v2
kind: HorizontalPodAutoscaler
metadata:
name: test-deployment-hpa
spec:
minReplicas: 2
maxReplicas: 4
metrics:
- resource:
name: cpu
target:
averageUtilization: 200
type: Utilization
type: Resource
scaleTargetRef:
kind: Deployment
apiVersion: apps/v1
name: test-deployment-hpa

11 changes: 11 additions & 0 deletions tests/application/hpa/patch-application-scale-up-from-0.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
apiVersion: skiperator.kartverket.no/v1alpha1
kind: Application
metadata:
name: test-deployment-hpa
spec:
image: image
port: 8080
replicas:
min: 2
max: 4
targetCpuUtilization: 200

0 comments on commit 1817472

Please sign in to comment.