feat: provide support to allow HTTP scaler to work alongside other co…

…re KEDA scalers

Signed-off-by: Paul Cooke <Paul.Cooke@10xbanking.com>

CHANGELOG.md

@@ -26,7 +26,7 @@ This changelog keeps track of work items that have been completed and are ready
 
 ### Improvements
 
-- **General**: TODO ([#TODO](https://github.com/kedacore/http-add-on/issues/TODO))
+- **Operator**: Provide support to allow HTTP scaler to work alongside other core KEDA scalers ([#489](https://github.com/kedacore/http-add-on/issues/489))
 
 ### Fixes
 

docs/walkthrough.md

@@ -89,4 +89,43 @@ 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](./)

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
+}

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)
+}

operator/controllers/http/suite_test.go

@@ -92,6 +92,16 @@ type commonTestInfra struct {
 	httpso  httpv1alpha1.HTTPScaledObject
 }
 
+type commonTestInfraWithScaledObject struct {
+	ns      string
+	appName string
+	ctx     context.Context
+	cl      client.Client
+	logger  logr.Logger
+	httpso  httpv1alpha1.HTTPScaledObject
+	so      kedav1alpha1.ScaledObject
+}
+
 func newCommonTestInfra(namespace, appName string) *commonTestInfra {
 	localScheme := runtime.NewScheme()
 	utilruntime.Must(clientgoscheme.AddToScheme(localScheme))
@@ -131,3 +141,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,
+	}
+}