WIP - fix: trim so name when is longer than 63 chars

CHANGELOG.md

@@ -49,6 +49,7 @@ To learn more about our roadmap, we recommend reading [this document](ROADMAP.md
 - **General:** Properly handle `restoreToOriginalReplicaCount` if `ScaleTarget` is missing ([#2872](https://github.com/kedacore/keda/issues/2872))
 - **General:** Support for running KEDA secure-by-default as non-root ([#2933](https://github.com/kedacore/keda/issues/2933))
 - **General:** Synchronize HPA annotations from ScaledObject ([#2659](https://github.com/kedacore/keda/pull/2659))
+- **General:** Trim ScaledObject name when is longer than 63 chars ([#2915](https://github.com/kedacore/keda/issues/2915))
 - **General:** Updated HTTPClient to be proxy-aware, if available, from environment variables. ([#2577](https://github.com/kedacore/keda/issues/2577))
 - **General:** Using manager client in KEDA Metrics Server to avoid flush request to Kubernetes Apiserver([2914](https://github.com/kedacore/keda/issues/2914))
 - **ActiveMQ Scaler:** Add CorsHeader information to ActiveMQ Scaler ([#2884](https://github.com/kedacore/keda/issues/2884))

controllers/keda/hpa.go

@@ -20,8 +20,6 @@ import (
 	"context"
 	"fmt"
 	"sort"
-	"strings"
-	"unicode"
 
 	"github.com/go-logr/logr"
 	autoscalingv2beta2 "k8s.io/api/autoscaling/v2beta2"
@@ -73,18 +71,12 @@ func (r *ScaledObjectReconciler) newHPAForScaledObject(ctx context.Context, logg
 		behavior = nil
 	}
 
-	// label can have max 63 chars
 	labelName := getHPAName(scaledObject)
-	if len(labelName) > 63 {
-		labelName = labelName[:63]
-		labelName = strings.TrimRightFunc(labelName, func(r rune) bool {
-			return !unicode.IsLetter(r) && !unicode.IsNumber(r)
-		})
-	}
+	// label can have max 63 chars
 	labels := map[string]string{
 		"app.kubernetes.io/name":       labelName,
 		"app.kubernetes.io/version":    version.Version,
-		"app.kubernetes.io/part-of":    scaledObject.Name,
+		"app.kubernetes.io/part-of":    kedacontrollerutil.Trim(scaledObject.Name, 63),
 		"app.kubernetes.io/managed-by": "keda-operator",
 	}
 	for key, value := range scaledObject.ObjectMeta.Labels {
@@ -199,7 +191,7 @@ func (r *ScaledObjectReconciler) getScaledObjectMetricSpecs(ctx context.Context,
 
 			// add the scaledobject.keda.sh/name label. This is how the MetricsAdapter will know which scaledobject a metric is for when the HPA queries it.
 			metricSpec.External.Metric.Selector = &metav1.LabelSelector{MatchLabels: make(map[string]string)}
-			metricSpec.External.Metric.Selector.MatchLabels["scaledobject.keda.sh/name"] = scaledObject.Name
+			metricSpec.External.Metric.Selector.MatchLabels["scaledobject.keda.sh/name"] = kedacontrollerutil.Trim(scaledObject.Name, 63)
 			externalMetricNames = append(externalMetricNames, externalMetricName)
 		}
 	}
@@ -250,7 +242,8 @@ func (r *ScaledObjectReconciler) checkMinK8sVersionforHPABehavior(logger logr.Lo
 
 // getHPAName returns generated HPA name for ScaledObject specified in the parameter
 func getHPAName(scaledObject *kedav1alpha1.ScaledObject) string {
-	return fmt.Sprintf("keda-hpa-%s", scaledObject.Name)
+	rawName := fmt.Sprintf("keda-hpa-%s", scaledObject.Name)
+	return kedacontrollerutil.Trim(rawName, 63)
 }
 
 // getHPAMinReplicas returns MinReplicas based on definition in ScaledObject or default value if not defined

controllers/keda/scaledobject_controller.go

@@ -257,18 +257,20 @@ func (r *ScaledObjectReconciler) reconcileScaledObject(ctx context.Context, logg
 // This is how the MetricsAdapter will know which ScaledObject a metric is for when the HPA queries it.
 func (r *ScaledObjectReconciler) ensureScaledObjectLabel(ctx context.Context, logger logr.Logger, scaledObject *kedav1alpha1.ScaledObject) error {
 	const labelScaledObjectName = "scaledobject.keda.sh/name"
+	// label can have max 63 chars
+	scaledObjectName := kedacontrollerutil.Trim(scaledObject.Name, 63)
 
 	if scaledObject.Labels == nil {
-		scaledObject.Labels = map[string]string{labelScaledObjectName: scaledObject.Name}
+		scaledObject.Labels = map[string]string{labelScaledObjectName: scaledObjectName}
 	} else {
 		value, found := scaledObject.Labels[labelScaledObjectName]
-		if found && value == scaledObject.Name {
+		if found && value == scaledObjectName {
 			return nil
 		}
-		scaledObject.Labels[labelScaledObjectName] = scaledObject.Name
+		scaledObject.Labels[labelScaledObjectName] = scaledObjectName
 	}
 
-	logger.V(1).Info("Adding \"scaledobject.keda.sh/name\" label on ScaledObject", "value", scaledObject.Name)
+	logger.V(1).Info("Adding \"scaledobject.keda.sh/name\" label on ScaledObject", "value", scaledObjectName)
 	return r.Client.Update(ctx, scaledObject)
 }
 

controllers/keda/util/strings.go

@@ -0,0 +1,34 @@
+/*
+Copyright 2021 The KEDA 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 util
+
+import (
+	"strings"
+	"unicode"
+)
+
+// Trim checks if the given string is longer than given value and trim it in that case.
+func Trim(s string, length int) string {
+	result := s
+	if len(result) > length {
+		result = result[:length]
+		result = strings.TrimRightFunc(result, func(r rune) bool {
+			return !unicode.IsLetter(r) && !unicode.IsNumber(r)
+		})
+	}
+	return result
+}

tests/scalers/kubernetes-workload-long-so-names.test.ts

@@ -0,0 +1,134 @@
+import * as fs from 'fs'
+import * as sh from 'shelljs'
+import * as tmp from 'tmp'
+import test from 'ava'
+import {createNamespace, waitForDeploymentReplicaCount} from "./helpers";
+
+const testNamespace = 'scaled-object-name-trimming'
+const monitoredDeploymentFile = tmp.fileSync()
+const sutDeploymentFile = tmp.fileSync()
+
+test.before(t => {
+  sh.config.silent = true
+	createNamespace(testNamespace)
+
+  fs.writeFileSync(monitoredDeploymentFile.name, monitoredDeploymentYaml)
+	t.is(
+		0,
+		sh.exec(`kubectl apply -f ${monitoredDeploymentFile.name} --namespace ${testNamespace}`).code,
+		'Deploying monitored deployment should work.'
+	)
+
+  fs.writeFileSync(sutDeploymentFile.name, sutDeploymentYaml)
+	t.is(
+		0,
+		sh.exec(`kubectl apply -f ${sutDeploymentFile.name} --namespace ${testNamespace}`).code,
+		'Deploying monitored deployment should work.'
+	)
+})
+
+test.serial('Deployment should have 0 replicas on start', t => {
+  const replicaCount = sh.exec(
+    `kubectl get deployment.apps/sut-deployment --namespace ${testNamespace} -o jsonpath="{.spec.replicas}"`
+  ).stdout
+  t.is(replicaCount, '0', 'replica count should start out as 0')
+})
+
+test.serial(`Deployment should scale to fit the amount of pods which match the selector`, async t => {
+
+  sh.exec(
+    `kubectl scale deployment.apps/monitored-deployment --namespace ${testNamespace} --replicas=5`
+  )
+  t.true(await waitForDeploymentReplicaCount(5, 'sut-deployment', testNamespace, 6, 10000), 'Replica count should be 5 after 60 seconds')
+
+  sh.exec(
+    `kubectl scale deployment.apps/monitored-deployment --namespace ${testNamespace} --replicas=10`
+  )
+  t.true(await waitForDeploymentReplicaCount(10, 'sut-deployment', testNamespace, 6, 10000), 'Replica count should be 10 after 60 seconds')
+
+  sh.exec(
+    `kubectl scale deployment.apps/monitored-deployment --namespace ${testNamespace} --replicas=5`
+  )
+  t.true(await waitForDeploymentReplicaCount(5, 'sut-deployment', testNamespace, 6, 10000), 'Replica count should be 5 after 60 seconds')
+
+  sh.exec(
+    `kubectl scale deployment.apps/monitored-deployment --namespace ${testNamespace} --replicas=0`
+  )
+  t.true(await waitForDeploymentReplicaCount(0, 'sut-deployment', testNamespace, 6, 10000), 'Replica count should be 0 after 60 seconds')
+})
+
+test.after.always.cb('clean up workload test related deployments', t => {
+  const resources = [
+    'scaledobject.keda.sh/sut-scaledobject',
+    'deployment.apps/sut-deployment',
+    'deployment.apps/monitored-deployment',
+  ]
+
+  for (const resource of resources) {
+    sh.exec(`kubectl delete ${resource} --namespace ${testNamespace}`)
+  }
+  sh.exec(`kubectl delete namespace ${testNamespace}`)
+  t.end()
+})
+
+const monitoredDeploymentYaml = `apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: monitored-deployment
+  labels:
+    deploy: workload-test
+spec:
+  replicas: 0
+  selector:
+    matchLabels:
+      pod: workload-test
+  template:
+    metadata:
+      labels:
+        pod: workload-test
+    spec:
+      containers:
+        - name: nginx
+          image: 'nginx'`
+
+const sutDeploymentYaml = `apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: sut-deployment
+  labels:
+    deploy: workload-sut
+spec:
+  replicas: 0
+  selector:
+    matchLabels:
+      pod: workload-sut
+  template:
+    metadata:
+      labels:
+        pod: workload-sut
+    spec:
+      containers:
+        - name: nginx
+          image: 'nginx'
+---
+apiVersion: keda.sh/v1alpha1
+kind: ScaledObject
+metadata:
+  name: this-scaledobject-name-is-longer-than-63-characters-to-check-that-it-is-trimmed
+spec:
+  scaleTargetRef:
+    name: sut-deployment
+  pollingInterval: 5
+  cooldownPeriod: 5
+  minReplicaCount: 0
+  maxReplicaCount: 10
+  advanced:
+    horizontalPodAutoscalerConfig:
+      behavior:
+        scaleDown:
+          stabilizationWindowSeconds: 15
+  triggers:
+  - type: kubernetes-workload
+    metadata:
+      podSelector: 'pod=workload-test'
+      value: '1'`