set HPA replica values when workload overrides are set

pkg/reconciler/common/ha.go

@@ -18,32 +18,21 @@ package common
 
 import (
 	mf "github.com/manifestival/manifestival"
-	"go.uber.org/zap"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/util/sets"
 
 	"knative.dev/operator/pkg/apis/operator/base"
 )
 
-func haUnSupported(obj base.KComponent) sets.String {
+func haUnSupported(name string) bool {
 	return sets.NewString(
 		"pingsource-mt-adapter",
-	)
-}
-
-// When Deployment has HPA, the replicas should be controlled by HPA's minReplicas instead of operator.
-// Hence, skip changing the spec.replicas in deployment directory for these Deployments.
-func hasHorizontalPodAutoscaler(obj base.KComponent) sets.String {
-	return sets.NewString(
-		"webhook",
-		"activator",
-		"3scale-kourier-gateway",
-	)
+	).Has(name)
 }
 
 // HighAvailabilityTransform mutates configmaps and replicacounts of certain
 // controllers when HA control plane is specified.
-func HighAvailabilityTransform(obj base.KComponent, log *zap.SugaredLogger) mf.Transformer {
+func HighAvailabilityTransform(obj base.KComponent) mf.Transformer {
 	return func(u *unstructured.Unstructured) error {
 		// Use spec.deployments.replicas for the deployment instead of spec.high-availability.
 		for _, override := range obj.GetSpec().GetWorkloadOverrides() {
@@ -61,39 +50,14 @@ func HighAvailabilityTransform(obj base.KComponent, log *zap.SugaredLogger) mf.T
 		replicas := int64(*ha.Replicas)
 
 		// Transform deployments that support HA.
-		if u.GetKind() == "Deployment" && !haUnSupported(obj).Has(u.GetName()) && !hasHorizontalPodAutoscaler(obj).Has(u.GetName()) {
+		if u.GetKind() == "Deployment" && !haUnSupported(u.GetName()) && !hasHorizontalPodAutoscaler(u.GetName()) {
 			if err := unstructured.SetNestedField(u.Object, replicas, "spec", "replicas"); err != nil {
 				return err
 			}
 		}
 
 		if u.GetKind() == "HorizontalPodAutoscaler" {
-			min, _, err := unstructured.NestedInt64(u.Object, "spec", "minReplicas")
-			if err != nil {
-				return err
-			}
-			// Do nothing if the HPA ships with even more replicas out of the box.
-			if min >= replicas {
-				return nil
-			}
-
-			if err := unstructured.SetNestedField(u.Object, replicas, "spec", "minReplicas"); err != nil {
-				return err
-			}
-
-			max, found, err := unstructured.NestedInt64(u.Object, "spec", "maxReplicas")
-			if err != nil {
-				return err
-			}
-
-			// Do nothing if maxReplicas is not defined.
-			if !found {
-				return nil
-			}
-
-			// Increase maxReplicas to the amount that we increased,
-			// because we need to avoid minReplicas > maxReplicas happenning.
-			if err := unstructured.SetNestedField(u.Object, max+(replicas-min), "spec", "maxReplicas"); err != nil {
+			if err := hpaTransform(u, replicas); err != nil {
 				return err
 			}
 		}

pkg/reconciler/common/ha_test.go

@@ -23,7 +23,6 @@ import (
 	"knative.dev/operator/pkg/apis/operator/v1beta1"
 
 	appsv1 "k8s.io/api/apps/v1"
-	autoscalingv2beta1 "k8s.io/api/autoscaling/v2beta1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/client-go/kubernetes/scheme"
@@ -74,10 +73,10 @@ func TestHighAvailabilityTransform(t *testing.T) {
 		in:       makeUnstructuredHPA(t, "activator", 2, 5),
 		expected: makeUnstructuredHPA(t, "activator", 2, 5),
 	}, {
-		name:     "HA; adjust hpa when replicas is lerger than maxReplicas",
+		name:     "HA; adjust hpa when replicas is larger than maxReplicas",
 		config:   makeHa(6),
 		in:       makeUnstructuredHPA(t, "activator", 2, 5),
-		expected: makeUnstructuredHPA(t, "activator", 6, 9), // maxReplicas is increased by max+(replicas-min) to avoid minReplicas > maxReplicas happenning.
+		expected: makeUnstructuredHPA(t, "activator", 6, 9), // maxReplicas is increased by max+(replicas-min) to avoid minReplicas > maxReplicas happening.
 	}, {
 		name:     "HA; adjust hpa when minReplica is equal to maxReplicas",
 		config:   makeHa(3),
@@ -107,7 +106,7 @@ func TestHighAvailabilityTransform(t *testing.T) {
 					},
 				},
 			}
-			haTransform := HighAvailabilityTransform(instance, log)
+			haTransform := HighAvailabilityTransform(instance)
 			err := haTransform(tc.in)
 
 			util.AssertDeepEqual(t, err, tc.err)
@@ -143,23 +142,3 @@ func makeUnstructuredDeploymentReplicas(t *testing.T, name string, replicas int3
 
 	return result
 }
-
-func makeUnstructuredHPA(t *testing.T, name string, minReplicas, maxReplicas int32) *unstructured.Unstructured {
-	hpa := &autoscalingv2beta1.HorizontalPodAutoscaler{
-		ObjectMeta: metav1.ObjectMeta{
-			Name: name,
-		},
-		Spec: autoscalingv2beta1.HorizontalPodAutoscalerSpec{
-			MinReplicas: &minReplicas,
-			MaxReplicas: maxReplicas,
-		},
-	}
-
-	result := &unstructured.Unstructured{}
-	err := scheme.Scheme.Convert(hpa, result, nil)
-	if err != nil {
-		t.Fatalf("Could not create unstructured HPA: %v, err: %v", hpa, err)
-	}
-
-	return result
-}

pkg/reconciler/common/hpa.go

@@ -0,0 +1,89 @@
+/*
+Copyright 2023 The Knative Authors
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package common
+
+import (
+	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
+	"k8s.io/apimachinery/pkg/util/sets"
+)
+
+// When a Podspecable has HPA, the replicas should be controlled by HPAs minReplicas instead of operator.
+// Hence, skip changing the spec.replicas for these Podspecables.
+func hasHorizontalPodAutoscaler(name string) bool {
+	return sets.NewString(
+		"webhook",
+		"activator",
+		"3scale-kourier-gateway",
+		"eventing-webhook",
+		"mt-broker-ingress",
+		"mt-broker-filter",
+	).Has(name)
+}
+
+// Maps a Podspecables name to the HPAs name.
+// Add overrides here, if your HPA is named differently to the workloads name,
+// if no override is defined, the name of the podspecable is used as HPA name.
+func getHPAName(podspecableName string) string {
+	overrides := map[string]string{
+		"mt-broker-ingress": "broker-ingress-hpa",
+		"mt-broker-filter":  "broker-filter-hpa",
+	}
+	if v, ok := overrides[podspecableName]; ok {
+		return v
+	} else {
+		return podspecableName
+	}
+}
+
+// hpaTransform sets the minReplicas and maxReplicas of an HPA based on a replica override value.
+// If minReplica needs to be increased, the maxReplica is increased by the same value.
+func hpaTransform(u *unstructured.Unstructured, replicas int64) error {
+	if u.GetKind() != "HorizontalPodAutoscaler" {
+		return nil
+	}
+
+	min, _, err := unstructured.NestedInt64(u.Object, "spec", "minReplicas")
+	if err != nil {
+		return err
+	}
+
+	// Do nothing if the HPA ships with even more replicas out of the box.
+	if min >= replicas {
+		return nil
+	}
+
+	if err := unstructured.SetNestedField(u.Object, replicas, "spec", "minReplicas"); err != nil {
+		return err
+	}
+
+	max, found, err := unstructured.NestedInt64(u.Object, "spec", "maxReplicas")
+	if err != nil {
+		return err
+	}
+
+	// Do nothing if maxReplicas is not defined.
+	if !found {
+		return nil
+	}
+
+	// Increase maxReplicas to the amount that we increased,
+	// because we need to avoid minReplicas > maxReplicas happening.
+	if err := unstructured.SetNestedField(u.Object, max+(replicas-min), "spec", "maxReplicas"); err != nil {
+		return err
+	}
+	return nil
+}

pkg/reconciler/common/hpa_test.go

@@ -0,0 +1,90 @@
+/*
+Copyright 2023 The Knative Authors
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package common
+
+import (
+	"testing"
+
+	"k8s.io/api/autoscaling/v2beta1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
+	"k8s.io/client-go/kubernetes/scheme"
+	util "knative.dev/operator/pkg/reconciler/common/testing"
+)
+
+func TestHpaTransform(t *testing.T) {
+	cases := []struct {
+		name     string
+		in       *unstructured.Unstructured
+		replicas int64
+		expected *unstructured.Unstructured
+		err      error
+	}{{
+		name:     "Object is not a HPA",
+		in:       makeUnstructuredDeployment(t, "not-a-hpa"),
+		replicas: 5,
+		expected: makeUnstructuredDeployment(t, "not-a-hpa"),
+		err:      nil,
+	}, {
+		name:     "minReplicas same as override",
+		in:       makeUnstructuredHPA(t, "hpa", 1, 2),
+		replicas: 1,
+		expected: makeUnstructuredHPA(t, "hpa", 1, 2),
+		err:      nil,
+	}, {
+		name:     "minReplicas lower than override",
+		in:       makeUnstructuredHPA(t, "hpa", 1, 2),
+		replicas: 5,
+		expected: makeUnstructuredHPA(t, "hpa", 5, 6),
+		err:      nil,
+	}}
+
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+
+			err := hpaTransform(tc.in, tc.replicas)
+
+			util.AssertDeepEqual(t, err, tc.err)
+			util.AssertDeepEqual(t, tc.in, tc.expected)
+		})
+	}
+}
+
+func TestGetHPAName(t *testing.T) {
+	util.AssertEqual(t, getHPAName("mt-broker-ingress"), "broker-ingress-hpa")
+	util.AssertEqual(t, getHPAName("activator"), "activator")
+}
+
+func makeUnstructuredHPA(t *testing.T, name string, minReplicas, maxReplicas int32) *unstructured.Unstructured {
+	hpa := &v2beta1.HorizontalPodAutoscaler{
+		ObjectMeta: metav1.ObjectMeta{
+			Name: name,
+		},
+		Spec: v2beta1.HorizontalPodAutoscalerSpec{
+			MinReplicas: &minReplicas,
+			MaxReplicas: maxReplicas,
+		},
+	}
+
+	result := &unstructured.Unstructured{}
+	err := scheme.Scheme.Convert(hpa, result, nil)
+	if err != nil {
+		t.Fatalf("Could not create unstructured HPA: %v, err: %v", hpa, err)
+	}
+
+	return result
+}

pkg/reconciler/common/transformers.go

@@ -30,7 +30,7 @@ func transformers(ctx context.Context, obj base.KComponent) []mf.Transformer {
 	return []mf.Transformer{
 		injectOwner(obj),
 		mf.InjectNamespace(obj.GetNamespace()),
-		HighAvailabilityTransform(obj, logger),
+		HighAvailabilityTransform(obj),
 		ImageTransform(obj.GetSpec().GetRegistry(), logger),
 		JobTransform(obj),
 		ConfigMapTransform(obj.GetSpec().GetConfig(), logger),

pkg/reconciler/common/workload_override.go

@@ -47,7 +47,9 @@ func OverridesTransform(overrides []base.WorkloadOverride, log *zap.SugaredLogge
 				}
 				obj = deployment
 				ps = &deployment.Spec.Template
-				if override.Replicas != nil {
+
+				// Do not set replicas, if this resource is controlled by a HPA
+				if override.Replicas != nil && !hasHorizontalPodAutoscaler(override.Name) {
 					deployment.Spec.Replicas = override.Replicas
 				}
 			}
@@ -58,7 +60,9 @@ func OverridesTransform(overrides []base.WorkloadOverride, log *zap.SugaredLogge
 				}
 				obj = ss
 				ps = &ss.Spec.Template
-				if override.Replicas != nil {
+
+				// Do not set replicas, if this resource is controlled by a HPA
+				if override.Replicas != nil && !hasHorizontalPodAutoscaler(override.Name) {
 					ss.Spec.Replicas = override.Replicas
 				}
 			}
@@ -71,6 +75,13 @@ func OverridesTransform(overrides []base.WorkloadOverride, log *zap.SugaredLogge
 				ps = &job.Spec.Template
 			}
 
+			if u.GetKind() == "HorizontalPodAutoscaler" && override.Replicas != nil && u.GetName() == getHPAName(override.Name) {
+				overrideReplicas := int64(*override.Replicas)
+				if err := hpaTransform(u, overrideReplicas); err != nil {
+					return err
+				}
+			}
+
 			if obj == nil {
 				continue
 			}