diff --git a/CHANGELOG.md b/CHANGELOG.md index b06c86dda1c..c81aaef32d4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -82,6 +82,8 @@ - **AWS DynamoDB:** Setup AWS DynamoDB test account ([#2803](https://github.com/kedacore/keda/issues/2803)) - **AWS Kinesis Stream:** Adding e2e test ([#1526](https://github.com/kedacore/keda/issues/1526)) - **AWS SQS Queue:** Adding e2e test ([#1527](https://github.com/kedacore/keda/issues/1527)) +- **External Scaler:** Adding e2e test. ([#2697](https://github.com/kedacore/keda/issues/2697)) +- **External Scaler:** Fix issue with internal KEDA core prefix being passed to external scaler. ([#2640](https://github.com/kedacore/keda/issues/2640)) - **GCP Pubsub Scaler:** Adding e2e test ([#1528](https://github.com/kedacore/keda/issues/1528)) - **Hashicorp Vault Secret Provider:** Adding e2e test ([#2842](https://github.com/kedacore/keda/issues/2842)) - **Memory Scaler:** Adding e2e test ([#2220](https://github.com/kedacore/keda/issues/2220)) diff --git a/pkg/scalers/external_scaler.go b/pkg/scalers/external_scaler.go index 384a93ca0a0..a0cd8857c4e 100644 --- a/pkg/scalers/external_scaler.go +++ b/pkg/scalers/external_scaler.go @@ -197,6 +197,12 @@ func (s *externalScaler) GetMetrics(ctx context.Context, metricName string, metr } defer done() + // Remove the sX- prefix as the external scaler shouldn't have to know about it + metricName, err = RemoveIndexFromMetricName(s.metadata.scalerIndex, metricName) + if err != nil { + return metrics, err + } + request := &pb.GetMetricsRequest{ MetricName: metricName, ScaledObjectRef: &s.scaledObjectRef, diff --git a/pkg/scalers/scaler.go b/pkg/scalers/scaler.go index 049e54111d8..2176298ec67 100644 --- a/pkg/scalers/scaler.go +++ b/pkg/scalers/scaler.go @@ -19,6 +19,7 @@ package scalers import ( "context" "fmt" + "strings" "time" "k8s.io/api/autoscaling/v2beta2" @@ -110,6 +111,21 @@ func GenerateMetricNameWithIndex(scalerIndex int, metricName string) string { return fmt.Sprintf("s%d-%s", scalerIndex, metricName) } +// RemoveIndexFromMetricName removes the index prefix from the metric name +func RemoveIndexFromMetricName(scalerIndex int, metricName string) (string, error) { + metricNameSplit := strings.SplitN(metricName, "-", 2) + if len(metricNameSplit) != 2 { + return "", fmt.Errorf("metric name without index prefix") + } + + indexPrefix, metricNameWithoutIndex := metricNameSplit[0], metricNameSplit[1] + if indexPrefix != fmt.Sprintf("s%d", scalerIndex) { + return "", fmt.Errorf("metric name contains incorrect index prefix") + } + + return metricNameWithoutIndex, nil +} + // GetMetricTargetType helps getting the metric target type of the scaler func GetMetricTargetType(config *ScalerConfig) (v2beta2.MetricTargetType, error) { switch config.MetricType { diff --git a/pkg/scalers/scaler_test.go b/pkg/scalers/scaler_test.go index 1a4b40f2e15..fced7eb5969 100644 --- a/pkg/scalers/scaler_test.go +++ b/pkg/scalers/scaler_test.go @@ -85,3 +85,40 @@ func TestGetMetricTarget(t *testing.T) { }) } } + +func TestRemoveIndexFromMetricName(t *testing.T) { + cases := []struct { + scalerIndex int + metricName string + expectedMetricNameWithoutIndexPrefix string + isError bool + }{ + // Proper input + {scalerIndex: 0, metricName: "s0-metricName", expectedMetricNameWithoutIndexPrefix: "metricName", isError: false}, + // Proper input with scalerIndex > 9 + {scalerIndex: 123, metricName: "s123-metricName", expectedMetricNameWithoutIndexPrefix: "metricName", isError: false}, + // Incorrect index prefix + {scalerIndex: 1, metricName: "s0-metricName", expectedMetricNameWithoutIndexPrefix: "", isError: true}, + // Incorrect index prefix + {scalerIndex: 0, metricName: "0-metricName", expectedMetricNameWithoutIndexPrefix: "", isError: true}, + // No index prefix + {scalerIndex: 0, metricName: "metricName", expectedMetricNameWithoutIndexPrefix: "", isError: true}, + } + + for _, testCase := range cases { + metricName, err := RemoveIndexFromMetricName(testCase.scalerIndex, testCase.metricName) + if err != nil && !testCase.isError { + t.Error("Expected success but got error", err) + } + + if testCase.isError && err == nil { + t.Error("Expected error but got success") + } + + if err == nil { + if metricName != testCase.expectedMetricNameWithoutIndexPrefix { + t.Errorf("Expected - %s, Got - %s", testCase.expectedMetricNameWithoutIndexPrefix, metricName) + } + } + } +} diff --git a/tests/scalers/external-scaler.test.ts b/tests/scalers/external-scaler.test.ts new file mode 100644 index 00000000000..450f932e3ad --- /dev/null +++ b/tests/scalers/external-scaler.test.ts @@ -0,0 +1,192 @@ +import * as sh from "shelljs" +import test from "ava" +import { createNamespace, createYamlFile, waitForDeploymentReplicaCount } from "./helpers" + +const testName = "test-external-scaler" +const testNamespace = `${testName}-ns` +const scalerName = `${testName}-scaler` +const serviceName = `${testName}-service` +const deploymentName = `${testName}-deployment` +const scaledObjectName = `${testName}-scaled-object` + +const idleReplicaCount = 0 +const minReplicaCount = 1 +const maxReplicaCount = 2 +const threshold = 10 + +test.before(async t => { + sh.config.silent = true + + // Create Kubernetes Namespace + createNamespace(testNamespace) + + // Create external scaler deployment + t.is( + sh.exec(`kubectl apply -f ${createYamlFile(scalerYaml)} -n ${testNamespace}`).code, + 0, + "Createing a external scaler deployment should work" + ) + + // Create service + t.is( + sh.exec(`kubectl apply -f ${createYamlFile(serviceYaml)} -n ${testNamespace}`).code, + 0, + "Createing a service should work" + ) + + // Create deployment + t.is( + sh.exec(`kubectl apply -f ${createYamlFile(deploymentYaml)} -n ${testNamespace}`).code, + 0, + "Createing a deployment should work" + ) + + // Create scaled object + t.is( + sh.exec(`kubectl apply -f ${createYamlFile(scaledObjectYaml.replace("{{VALUE}}", "0"))} -n ${testNamespace}`).code, + 0, + "Creating a scaled object should work" + ) + + t.true(await waitForDeploymentReplicaCount(idleReplicaCount, deploymentName, testNamespace, 60, 1000), + `Replica count should be ${idleReplicaCount} after 1 minute`) +}) + +test.serial("Deployment should scale up to minReplicaCount", async t => { + // Modify scaled object's metricValue to induce scaling + t.is( + sh.exec(`kubectl apply -f ${createYamlFile(scaledObjectYaml.replace("{{VALUE}}", `${threshold}`))} -n ${testNamespace}`).code, + 0, + "Modifying scaled object should work" + ) + + t.true(await waitForDeploymentReplicaCount(minReplicaCount, deploymentName, testNamespace, 60, 1000), + `Replica count should be ${minReplicaCount} after 1 minute`) +}) + +test.serial("Deployment should scale up to maxReplicaCount", async t => { + // Modify scaled object's metricValue to induce scaling + t.is( + sh.exec(`kubectl apply -f ${createYamlFile(scaledObjectYaml.replace("{{VALUE}}", `${threshold * 2}`))} -n ${testNamespace}`).code, + 0, + "Modifying scaled object should work" + ) + + t.true(await waitForDeploymentReplicaCount(maxReplicaCount, deploymentName, testNamespace, 60, 1000), + `Replica count should be ${maxReplicaCount} after 1 minute`) +}) + +test.serial("Deployment should scale back down to idleReplicaCount", async t => { + // Modify scaled object's metricValue to induce scaling + t.is( + sh.exec(`kubectl apply -f ${createYamlFile(scaledObjectYaml.replace("{{VALUE}}", "0"))} -n ${testNamespace}`).code, + 0, + "Modifying scaled object should work" + ) + + t.true(await waitForDeploymentReplicaCount(idleReplicaCount, deploymentName, testNamespace, 60, 1000), + `Replica count should be ${idleReplicaCount} after 1 minute`) +}) + +test.after.always("Clean up E2E K8s objects", async t => { + const resources = [ + `scaledobject.keda.sh/${scaledObjectName}`, + `deployments.apps/${deploymentName}`, + `service/${serviceName}`, + `deployments.apps/${scalerName}`, + ] + + for (const resource of resources) { + sh.exec(`kubectl delete ${resource} -n ${testNamespace}`) + } + + sh.exec(`kubectl delete ns ${testNamespace}`) +}) + +// YAML Definitions for Kubernetes resources +// External Scaler Deployment +const scalerYaml = +` +apiVersion: apps/v1 +kind: Deployment +metadata: + name: ${scalerName} + namespace: ${testNamespace} +spec: + replicas: 1 + selector: + matchLabels: + app: ${scalerName} + template: + metadata: + labels: + app: ${scalerName} + spec: + containers: + - name: scaler + image: ghcr.io/kedacore/tests-external-scaler-e2e:latest + imagePullPolicy: Always + ports: + - containerPort: 6000 +` + +const serviceYaml = +` +apiVersion: v1 +kind: Service +metadata: + name: ${serviceName} + namespace: ${testNamespace} +spec: + ports: + - port: 6000 + targetPort: 6000 + selector: + app: ${scalerName} +` + +// Deployment +const deploymentYaml = +`apiVersion: apps/v1 +kind: Deployment +metadata: + name: ${deploymentName} + namespace: ${testNamespace} +spec: + replicas: 0 + selector: + matchLabels: + app: ${deploymentName} + template: + metadata: + labels: + app: ${deploymentName} + spec: + containers: + - name: nginx + image: nginx:1.16.1 +` + +// Scaled Object +const scaledObjectYaml = +` +apiVersion: keda.sh/v1alpha1 +kind: ScaledObject +metadata: + name: ${scaledObjectName} + namespace: ${testNamespace} +spec: + scaleTargetRef: + name: ${deploymentName} + pollingInterval: 5 + cooldownPeriod: 10 + idleReplicaCount: ${idleReplicaCount} + minReplicaCount: ${minReplicaCount} + maxReplicaCount: ${maxReplicaCount} + triggers: + - type: external + metadata: + scalerAddress: ${serviceName}.${testNamespace}:6000 + metricThreshold: "${threshold}" + metricValue: "{{VALUE}}" +` diff --git a/tests/scalers/helpers.ts b/tests/scalers/helpers.ts index 45ed5bceaa1..5af7677d0fe 100644 --- a/tests/scalers/helpers.ts +++ b/tests/scalers/helpers.ts @@ -31,6 +31,12 @@ export async function createNamespace(namespace: string) { sh.exec(`kubectl apply -f ${namespaceFile.name}`) } +export function createYamlFile(yaml: string) { + const tmpFile = tmp.fileSync() + fs.writeFileSync(tmpFile.name, yaml) + return tmpFile.name +} + const namespaceTemplate = `apiVersion: v1 kind: Namespace metadata: