Repository: kedacore/http-add-on
Pull request #929 (2024-02-22 16:36:29)

feat: provide support to allow HTTP scaler to work alongside other core KEDA scalers
<!-- Thank you for contributing!

     Read more about how you can contribute in our contribution guide:
     https://github.com/kedacore/http-add-on/blob/main/CONTRIBUTING.md
-->

This change provides support to allow HTTP scaler to work alongside other core KEDA scalers by allowing skipScaledObjectCreation annotation to be set on an HTTPScaledObject.  This will ensure the reconciler to skip the KEDA core ScaledObjects creation which alllows you to create and manage your own ScaledObject with addtional triggers.

### Checklist

- [x] Commits are signed with Developer Certificate of Origin (DCO)
- [x] Changelog has been updated and is aligned with our [changelog requirements](https://github.com/kedacore/keda/blob/main/CONTRIBUTING.md#Changelog)
- [x] Any necessary documentation is added, such as:
  - [`README.md`](/README.md)
  - [The `docs/` directory](./docs)
  - [The docs repo](https://github.com/kedacore/keda-docs)

Fixes #


tomkerkhove: Hey, mind sharing some samples on what that would look like? Is the problem that you'd otherwise have 2 scaledobjects or?
devopsdynamo: @tomkerkhove  Yeah the problem we have at the moment is that we already have a scaledobject with a kafka trigger added for scaling but we also have a requirement to add HTTP scaling to the same deployment so we can scale up on http requests also.  This change will allow you to add skip annotation on the HTTPSO so you can add the external-push trigger to your existing scaledobject as an additional trigger etc.   
JorTurFer: I like the solution but I'm worried about how this will scale in the future. Currently, we support `hosts` and `prefixes` properties within external scaler metadata but maybe we need more info in the future. 
I'm thinking that maybe we should replace them with the HTTPScaledObject name instead, but it enforces the scaler to know about the HTTPScaledObject and permissions for pulling them to selfconfigure it :(
WDYT @tomkerkhove ?
JorTurFer: There are also some static checks failures: https://github.com/kedacore/http-add-on/actions/runs/8230990823/job/22505562138?pr=929
JorTurFer: We need to fix the DCO check too, you can check the steps just clicking the link:
![image](https://github.com/kedacore/http-add-on/assets/36899226/ee6b749b-b2f2-4508-b571-87700ca39f74)

JorTurFer: There are also some linting errors, could you take a look? https://github.com/kedacore/http-add-on/actions/runs/8261625169/job/22724018316?pr=929
devopsdynamo: @tomkerkhove updated docs as per your suggestions, thanks.  @JorTurFer lint errors should be fixed now also,  my bad :)
devopsdynamo: > LGTM! Just the change in the changelog and we are ready to merge this! ❤️

Updated as requested

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 5d8a8a4..3a5918d 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -23,11 +23,10 @@ This changelog keeps track of work items that have been completed and are ready
 ### New
 
 - **General**: Propagate HTTPScaledObject labels and annotations to ScaledObject ([#840](https://github.com/kedacore/http-add-on/issues/840))
+- **General**: Provide support to allow HTTP scaler to work alongside other core KEDA scalers ([#489](https://github.com/kedacore/http-add-on/issues/489))
 
 ### Improvements
 
-- **General**: TODO ([#TODO](https://github.com/kedacore/http-add-on/issues/TODO))
-
 ### Fixes
 
 - **General**: TODO ([#TODO](https://github.com/kedacore/http-add-on/issues/TODO))
diff --git a/docs/walkthrough.md b/docs/walkthrough.md
index 5cd9f57..3cae7ee 100644
--- a/docs/walkthrough.md
+++ b/docs/walkthrough.md
@@ -89,4 +89,42 @@ kubectl port-forward svc/keda-http-add-on-interceptor-proxy -n ${NAMESPACE} 8080
 curl -H "Host: myhost.com" localhost:8080/path1
 ```
 
+### Integrating HTTP Add-On Scaler with other KEDA scalers
+
+For scenerios where you want to integrate HTTP Add-On scaler with other keda scalers, you can set the `SkipScaledObjectCreation` annotation to true on your `HTTPScaledObject`.  The reconciler will then skip the KEDA core ScaledObject creation which will allow you to create your own `ScaledObject` and add HTTP scaler as one of your triggers.
+
+> 💡 Ensure that your ScaledObject is created with a different name than the `HTTPScaledObject` to ensure your ScaledObject is not removed by the reconciler.
+
+It is reccomended that you first deploy your HTTPScaledObject with no annotation set in order to obtain the latest trigger spec to use on your own managed ScaledObject.
+
+1. Deploy your `HTTPScaledObject` with annotation set to false
+
+```console
+annotations:
+  skipScaledObjectCreation: false
+```
+
+2. Take copy of the current generated external-push trigger spec on the generated ScaledObject.
+
+For example:
+
+```console
+  triggers:
+  - type: external-push
+    metadata:
+      hosts: example-service
+      pathPrefixes: ""
+      scalerAddress: keda-http-add-on-external-scaler.keda:9090
+```
+
+3. Apply the `skipScaledObjectCreation` annotation with `true` and apply the change. This will remove the originally created `ScaledObject` allowing you to create your own.
+
+```console
+annotations:
+  skipScaledObjectCreation: true
+```
+
+4. Add the `external-push` trigger taken from step 2 to your own ScaledObject and apply this.
+
+
 [Go back to landing page](./)
diff --git a/operator/controllers/http/app.go b/operator/controllers/http/app.go
index f6b8330..b62c614 100644
--- a/operator/controllers/http/app.go
+++ b/operator/controllers/http/app.go
@@ -2,9 +2,12 @@ package http
 
 import (
 	"context"
+	"strings"
 
 	"github.com/go-logr/logr"
+	kedav1alpha1 "github.com/kedacore/keda/v2/apis/keda/v1alpha1"
 	v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/types"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 
 	"github.com/kedacore/http-add-on/operator/apis/http/v1alpha1"
@@ -41,6 +44,22 @@ func (r *HTTPScaledObjectReconciler) createOrUpdateApplicationResources(
 			"Identified HTTPScaledObject creation signal"),
 	)
 
+	// We want to integrate http scaler with other
+	// scalers. when SkipScaledObjectCreation is set to true,
+	// reconciler will skip the KEDA core ScaledObjects creation or delete scaledObject if it already exists.
+	// you can then create your own SO, and add http scaler as one of your triggers.
+	if httpso.Annotations["skipScaledObjectCreation"] == "true" {
+		logger.Info(
+			"Skip scaled objects creation with flag SkipScaledObjectCreation=true",
+			"HTTPScaledObject", httpso.Name)
+		err := r.deleteScaledObject(ctx, cl, logger, httpso)
+		if err != nil {
+			logger.Info("Failed to delete ScaledObject",
+				"HTTPScaledObject", httpso.Name)
+		}
+		return nil
+	}
+
 	// create the KEDA core ScaledObjects (not the HTTP one) for
 	// the app deployment and the interceptor deployment.
 	// this needs to be submitted so that KEDA will scale both the app and
@@ -53,3 +72,46 @@ func (r *HTTPScaledObjectReconciler) createOrUpdateApplicationResources(
 		httpso,
 	)
 }
+
+func (r *HTTPScaledObjectReconciler) deleteScaledObject(
+	ctx context.Context,
+	cl client.Client,
+	logger logr.Logger,
+	httpso *v1alpha1.HTTPScaledObject,
+) error {
+	var fetchedSO kedav1alpha1.ScaledObject
+
+	objectKey := types.NamespacedName{
+		Namespace: httpso.Namespace,
+		Name:      httpso.Name,
+	}
+
+	if err := cl.Get(ctx, objectKey, &fetchedSO); err != nil {
+		logger.Info("Failed to retrieve ScaledObject",
+			"ScaledObject", &fetchedSO.Name)
+		return err
+	}
+
+	if isOwnerReferenceMatch(&fetchedSO, httpso) {
+		if err := cl.Delete(ctx, &fetchedSO); err != nil {
+			logger.Info("Failed to delete ScaledObject",
+				"ScaledObject", &fetchedSO.Name)
+			return nil
+		}
+		logger.Info("Deleted ScaledObject",
+			"ScaledObject", &fetchedSO.Name)
+	}
+
+	return nil
+}
+
+// function to check if the owner reference of ScaledObject matches the HTTPScaledObject
+func isOwnerReferenceMatch(scaledObject *kedav1alpha1.ScaledObject, httpso *v1alpha1.HTTPScaledObject) bool {
+	for _, ownerRef := range scaledObject.OwnerReferences {
+		if strings.ToLower(ownerRef.Kind) == "httpscaledobject" &&
+			ownerRef.Name == httpso.Name {
+			return true
+		}
+	}
+	return false
+}


Test patch:
diff --git a/operator/controllers/http/httpscaledobject_controller_test.go b/operator/controllers/http/httpscaledobject_controller_test.go
new file mode 100644
index 0000000..389aa25
--- /dev/null
+++ b/operator/controllers/http/httpscaledobject_controller_test.go
@@ -0,0 +1,127 @@
+package http
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/require"
+
+	"github.com/kedacore/http-add-on/operator/controllers/http/config"
+)
+
+func TestHttpScaledObjectControllerWhenSkipAnnotationNotSet(t *testing.T) {
+	r := require.New(t)
+
+	testInfra := newCommonTestInfra("testns", "testapp")
+
+	reconciller := &HTTPScaledObjectReconciler{
+		Client:               testInfra.cl,
+		Scheme:               testInfra.cl.Scheme(),
+		ExternalScalerConfig: config.ExternalScaler{},
+		BaseConfig:           config.Base{},
+	}
+
+	// Create required app objects for the application defined by the CRD
+	err := reconciller.createOrUpdateApplicationResources(
+		testInfra.ctx,
+		testInfra.logger,
+		testInfra.cl,
+		config.Base{},
+		config.ExternalScaler{},
+		&testInfra.httpso,
+	)
+	r.NoError(err)
+
+	// check for scaledobject, expect no error as scaledobject should get created
+	_, err = getSO(
+		testInfra.ctx,
+		testInfra.cl,
+		testInfra.httpso,
+	)
+	r.NoError(err)
+}
+
+func TestHttpScaledObjectControllerWhenSkipAnnotationSet(t *testing.T) {
+	r := require.New(t)
+
+	testInfra := newCommonTestInfraWithSkipScaledObjectCreation("testns", "testapp")
+
+	reconciller := &HTTPScaledObjectReconciler{
+		Client:               testInfra.cl,
+		Scheme:               testInfra.cl.Scheme(),
+		ExternalScalerConfig: config.ExternalScaler{},
+		BaseConfig:           config.Base{},
+	}
+
+	// Create required app objects for the application defined by the CRD
+	err := reconciller.createOrUpdateApplicationResources(
+		testInfra.ctx,
+		testInfra.logger,
+		testInfra.cl,
+		config.Base{},
+		config.ExternalScaler{},
+		&testInfra.httpso,
+	)
+	r.NoError(err)
+
+	// check for scaledobject, expect error as scaledobject should not exist when skipScaledObjectCreation annotation is set
+	_, err = getSO(
+		testInfra.ctx,
+		testInfra.cl,
+		testInfra.httpso,
+	)
+	r.Error(err)
+}
+
+func TestHttpScaledObjectControllerWhenSkipAnnotationAddedToExistingHttpSo(t *testing.T) {
+	r := require.New(t)
+
+	testInfra := newCommonTestInfra("testns", "testapp")
+
+	reconciller := &HTTPScaledObjectReconciler{
+		Client:               testInfra.cl,
+		Scheme:               testInfra.cl.Scheme(),
+		ExternalScalerConfig: config.ExternalScaler{},
+		BaseConfig:           config.Base{},
+	}
+
+	// Create required app objects for the application defined by the CRD
+	err := reconciller.createOrUpdateApplicationResources(
+		testInfra.ctx,
+		testInfra.logger,
+		testInfra.cl,
+		config.Base{},
+		config.ExternalScaler{},
+		&testInfra.httpso,
+	)
+	r.NoError(err)
+
+	// check for scaledobject, expect no error as scaledobject should exist when skipScaledObjectCreation annotation is not set
+	_, err = getSO(
+		testInfra.ctx,
+		testInfra.cl,
+		testInfra.httpso,
+	)
+	r.NoError(err)
+
+	// add skipScaledObjectCreation annotation to HTTPScaledObject
+	testInfra = newCommonTestInfraWithSkipScaledObjectCreation("testns", "testapp")
+
+	// update required app objects for the application defined by the CRD
+	err = reconciller.createOrUpdateApplicationResources(
+		testInfra.ctx,
+		testInfra.logger,
+		testInfra.cl,
+		config.Base{},
+		config.ExternalScaler{},
+		&testInfra.httpso,
+	)
+	r.NoError(err)
+
+	// check for scaledobject, expect error as scaledobject should not exist when skipScaledObjectCreation annotation is set
+	_, err = getSO(
+		testInfra.ctx,
+		testInfra.cl,
+		testInfra.httpso,
+	)
+	r.Error(err)
+}
diff --git a/operator/controllers/http/suite_test.go b/operator/controllers/http/suite_test.go
index 40f6151..4b89abb 100644
--- a/operator/controllers/http/suite_test.go
+++ b/operator/controllers/http/suite_test.go
@@ -131,3 +131,41 @@ func newCommonTestInfra(namespace, appName string) *commonTestInfra {
 		httpso:  httpso,
 	}
 }
+
+func newCommonTestInfraWithSkipScaledObjectCreation(namespace, appName string) *commonTestInfra {
+	localScheme := runtime.NewScheme()
+	utilruntime.Must(clientgoscheme.AddToScheme(localScheme))
+	utilruntime.Must(httpv1alpha1.AddToScheme(localScheme))
+	utilruntime.Must(kedav1alpha1.AddToScheme(localScheme))
+
+	ctx := context.Background()
+	cl := fake.NewClientBuilder().WithScheme(localScheme).Build()
+	logger := logr.Discard()
+
+	httpso := httpv1alpha1.HTTPScaledObject{
+		ObjectMeta: metav1.ObjectMeta{
+			Namespace: namespace,
+			Name:      appName,
+			Annotations: map[string]string{
+				"skipScaledObjectCreation": "true",
+			},
+		},
+		Spec: httpv1alpha1.HTTPScaledObjectSpec{
+			ScaleTargetRef: httpv1alpha1.ScaleTargetRef{
+				Deployment: appName,
+				Service:    appName,
+				Port:       8081,
+			},
+			Hosts: []string{"myhost1.com", "myhost2.com"},
+		},
+	}
+
+	return &commonTestInfra{
+		ns:      namespace,
+		appName: appName,
+		ctx:     ctx,
+		cl:      cl,
+		logger:  logger,
+		httpso:  httpso,
+	}
+}
diff --git a/tests/checks/scaling_phase_deployment_with_skip_so_creation/scaling_phase_deployment_with_skip_so_creation_test.go b/tests/checks/scaling_phase_deployment_with_skip_so_creation/scaling_phase_deployment_with_skip_so_creation_test.go
new file mode 100644
index 0000000..1611143
--- /dev/null
+++ b/tests/checks/scaling_phase_deployment_with_skip_so_creation/scaling_phase_deployment_with_skip_so_creation_test.go
@@ -0,0 +1,215 @@
+//go:build e2e
+// +build e2e
+
+package scaling_phase_test
+
+import (
+	"fmt"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"k8s.io/client-go/kubernetes"
+
+	. "github.com/kedacore/http-add-on/tests/helper"
+)
+
+const (
+	testName = "scaling-phase-deployment-with-skip-so-creation-test"
+)
+
+var (
+	testNamespace        = fmt.Sprintf("%s-ns", testName)
+	deploymentName       = fmt.Sprintf("%s-deployment", testName)
+	serviceName          = fmt.Sprintf("%s-service", testName)
+	httpScaledObjectName = fmt.Sprintf("%s-so", testName)
+	scaledObjectName     = fmt.Sprintf("%s-so", testName)
+	host                 = testName
+	minReplicaCount      = 0
+	maxReplicaCount      = 4
+)
+
+type templateData struct {
+	TestNamespace        string
+	DeploymentName       string
+	ServiceName          string
+	HTTPScaledObjectName string
+	ScaledObjectName     string
+	Host                 string
+	MinReplicas          int
+	MaxReplicas          int
+}
+
+const (
+	serviceTemplate = `
+apiVersion: v1
+kind: Service
+metadata:
+  name: {{.ServiceName}}
+  namespace: {{.TestNamespace}}
+  labels:
+    app: {{.DeploymentName}}
+spec:
+  ports:
+    - port: 8080
+      targetPort: http
+      protocol: TCP
+      name: http
+  selector:
+    app: {{.DeploymentName}}
+`
+
+	workloadTemplate = `
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: {{.DeploymentName}}
+  namespace: {{.TestNamespace}}
+  labels:
+    app: {{.DeploymentName}}
+spec:
+  replicas: 0
+  selector:
+    matchLabels:
+      app: {{.DeploymentName}}
+  template:
+    metadata:
+      labels:
+        app: {{.DeploymentName}}
+    spec:
+      containers:
+        - name: {{.DeploymentName}}
+          image: registry.k8s.io/e2e-test-images/agnhost:2.45
+          args:
+          - netexec
+          ports:
+            - name: http
+              containerPort: 8080
+              protocol: TCP
+          readinessProbe:
+            httpGet:
+              path: /
+              port: http
+`
+
+	loadJobTemplate = `
+apiVersion: batch/v1
+kind: Job
+metadata:
+  name: load-generator
+  namespace: {{.TestNamespace}}
+spec:
+  template:
+    spec:
+      containers:
+      - name: apache-ab
+        image: ghcr.io/kedacore/tests-apache-ab
+        imagePullPolicy: Always
+        args:
+          - "-n"
+          - "2000000"
+          - "-c"
+          - "20"
+          - "-H"
+          - "Host: {{.Host}}"
+          - "http://keda-http-add-on-interceptor-proxy.keda:8080/"
+      restartPolicy: Never
+      terminationGracePeriodSeconds: 5
+  activeDeadlineSeconds: 600
+  backoffLimit: 5
+`
+	httpScaledObjectTemplate = `
+kind: HTTPScaledObject
+apiVersion: http.keda.sh/v1alpha1
+metadata:
+  name: {{.HTTPScaledObjectName}}
+  namespace: {{.TestNamespace}}
+  annotations:
+    skipScaledObjectCreation: "true"
+spec:
+  hosts:
+  - {{.Host}}
+  scaledownPeriod: 10
+  scaleTargetRef:
+    name: {{.DeploymentName}}
+    service: {{.ServiceName}}
+    port: 8080
+  replicas:
+    min: {{ .MinReplicas }}
+    max: {{ .MaxReplicas }}
+`
+	scaledObjectTemplate = `
+apiVersion: keda.sh/v1alpha1
+kind: ScaledObject
+metadata:
+  name: {{.ScaledObjectName}}
+  namespace: {{.TestNamespace}}
+spec:
+  cooldownPeriod: 10
+  idleReplicaCount: 0
+  maxReplicaCount: {{ .MaxReplicas }}
+  minReplicaCount: {{ .MaxReplicas }}
+  pollingInterval: 30
+  scaleTargetRef:
+    apiVersion: apps/v1
+    kind: Deployment
+    name: {{.DeploymentName}}
+  triggers:
+  - type: external-push
+    metadata:
+      hosts: {{.Host}}
+      scalerAddress: keda-http-add-on-external-scaler.keda:9090
+`
+)
+
+func TestCheck(t *testing.T) {
+	// setup
+	t.Log("--- setting up ---")
+	// Create kubernetes resources
+	kc := GetKubernetesClient(t)
+	data, templates := getTemplateData()
+	CreateKubernetesResources(t, kc, testNamespace, data, templates)
+
+	assert.True(t, WaitForDeploymentReplicaReadyCount(t, kc, deploymentName, testNamespace, minReplicaCount, 6, 10),
+		"replica count should be %d after 1 minutes", minReplicaCount)
+
+	testScaleOut(t, kc, data)
+	testScaleIn(t, kc)
+
+	// cleanup
+	DeleteKubernetesResources(t, testNamespace, data, templates)
+}
+
+func testScaleOut(t *testing.T, kc *kubernetes.Clientset, data templateData) {
+	t.Log("--- testing scale out ---")
+
+	KubectlApplyWithTemplate(t, data, "loadJobTemplate", loadJobTemplate)
+
+	assert.True(t, WaitForDeploymentReplicaReadyCount(t, kc, deploymentName, testNamespace, maxReplicaCount, 18, 10),
+		"replica count should be %d after 3 minutes", maxReplicaCount)
+	KubectlDeleteWithTemplate(t, data, "loadJobTemplate", loadJobTemplate)
+}
+
+func testScaleIn(t *testing.T, kc *kubernetes.Clientset) {
+	t.Log("--- testing scale out ---")
+
+	assert.True(t, WaitForDeploymentReplicaReadyCount(t, kc, deploymentName, testNamespace, minReplicaCount, 12, 10),
+		"replica count should be %d after 2 minutes", minReplicaCount)
+}
+
+func getTemplateData() (templateData, []Template) {
+	return templateData{
+			TestNamespace:        testNamespace,
+			DeploymentName:       deploymentName,
+			ServiceName:          serviceName,
+			HTTPScaledObjectName: httpScaledObjectName,
+			ScaledObjectName:     scaledObjectName,
+			Host:                 host,
+			MinReplicas:          minReplicaCount,
+			MaxReplicas:          maxReplicaCount,
+		}, []Template{
+			{Name: "workloadTemplate", Config: workloadTemplate},
+			{Name: "serviceNameTemplate", Config: serviceTemplate},
+			{Name: "httpScaledObjectTemplate", Config: httpScaledObjectTemplate},
+			{Name: "scaledObjectTemplate", Config: scaledObjectTemplate},
+		}
+}