feat: Move http scaled object from single host to multi host system (k…

…edacore#674)

Co-authored-by: Somesh Koli <somesh.koli@headout.com>

.github/workflows/e2e-tests.yaml

@@ -51,7 +51,6 @@ jobs:
       - name: Show Kubernetes version
         run: |
           kubectl version
-
       - name: Run e2e test
         run: |
           make e2e-test

CHANGELOG.md

@@ -19,6 +19,8 @@ This changelog keeps track of work items that have been completed and are ready
 
 ### New
 
+- **General**: Add multi-host support to `HTTPScaledObject` ([#552](https://github.com/kedacore/http-add-on/issues/552))
+
 ### Improvements
 
 - **General**: Automatically tag Docker image with commit SHA ([#567](https://github.com/kedacore/http-add-on/issues/567))
@@ -36,7 +38,7 @@ You can find all deprecations in [this overview](https://github.com/kedacore/htt
 
 New deprecation(s):
 
-- TODO
+- **General**: `host` field deprecated in favor of `hosts` in `HTTPScaledObject` ([#552](https://github.com/kedacore/http-add-on/issues/552))
 
 Previously announced deprecation(s):
 

config/crd/bases/http.keda.sh_httpscaledobjects.yaml

@@ -60,10 +60,19 @@ spec:
             description: HTTPScaledObjectSpec defines the desired state of HTTPScaledObject
             properties:
               host:
-                description: The host to route. All requests with this host in the
-                  "Host" header will be routed to the Service and Port specified in
-                  the scaleTargetRef
+                description: (optional) (deprecated) The host to route. All requests
+                  with these hosts in the "Host" header will be routed to the Service
+                  and Port specified in the scaleTargetRef. The host field is mutually
+                  exclusive of the hosts field
                 type: string
+              hosts:
+                description: (optional) The hosts to route. All requests with these
+                  hosts in the "Host" header will be routed to the Service and Port
+                  specified in the scaleTargetRef. The hosts field is mutually exclusive
+                  of the host field.
+                items:
+                  type: string
+                type: array
               replicas:
                 description: (optional) Replica information
                 properties:
@@ -107,7 +116,6 @@ spec:
                 format: int32
                 type: integer
             required:
-            - host
             - scaleTargetRef
             type: object
           status:

examples/xkcd/templates/httpscaledobject.yaml

@@ -3,7 +3,10 @@ apiVersion: http.keda.sh/v1alpha1
 metadata:
     name: {{ include "xkcd.fullname" . }}
 spec:
-    host: {{ .Values.host }}
+    {{- with .Values.hosts }}
+    hosts:
+        {{- toYaml . | nindent 8 }}
+    {{- end }}
     targetPendingRequests: {{ .Values.targetPendingRequests }}
     scaleTargetRef:
         deployment: {{ include "xkcd.fullname" . }}

examples/xkcd/templates/ingress.yaml

@@ -8,7 +8,8 @@ metadata:
     kubernetes.io/ingress.class: nginx
 spec:
   rules:
-  - host: {{ .Values.host }}
+  {{- range .Values.hosts }}
+  - host: {{ . | toString }}
     http:
       paths:
       - path: /
@@ -18,3 +19,4 @@ spec:
             name: keda-add-ons-http-interceptor-proxy
             port:
               number: 8080
+  {{- end }}

examples/xkcd/values.yaml

@@ -1,5 +1,7 @@
 replicaCount: 1
-host: myhost.com
+hosts:
+  - "myhost.com"
+  - "myhost2.com"
 targetPendingRequests: 200
 # This is the namespace that the ingress should be installed
 # into. It should be set to the same namespace as the

operator/apis/http/v1alpha1/httpscaledobject_types.go

@@ -40,9 +40,14 @@ type ReplicaStruct struct {
 
 // HTTPScaledObjectSpec defines the desired state of HTTPScaledObject
 type HTTPScaledObjectSpec struct {
-	// The host to route. All requests with this host in the "Host" header will
-	// be routed to the Service and Port specified in the scaleTargetRef
-	Host string `json:"host"`
+	// (optional) (deprecated) The host to route. All requests with these hosts in the "Host" header will
+	// be routed to the Service and Port specified in the scaleTargetRef. The host field is mutually exclusive of the hosts field
+	// +optional
+	Host *string `json:"host,omitempty"`
+	// (optional) The hosts to route. All requests with these hosts in the "Host" header will
+	// be routed to the Service and Port specified in the scaleTargetRef. The hosts field is mutually exclusive of the host field.
+	// +optional
+	Hosts []string `json:"hosts,omitempty"`
 	// The name of the deployment to route HTTP requests to (and to autoscale).
 	// Either this or Image must be set
 	ScaleTargetRef *ScaleTargetRef `json:"scaleTargetRef"`

operator/controllers/http/app.go

@@ -80,18 +80,14 @@ func removeApplicationResources(
 		v1alpha1.AppScaledObjectTerminated,
 	))
 
-	if err := removeAndUpdateRoutingTable(
+	return removeAndUpdateRoutingTable(
 		ctx,
 		logger,
 		cl,
 		routingTable,
-		httpso.Spec.Host,
+		httpso.Spec.Hosts,
 		baseConfig.CurrentNamespace,
-	); err != nil {
-		return err
-	}
-
-	return nil
+	)
 }
 
 func createOrUpdateApplicationResources(
@@ -144,12 +140,12 @@ func createOrUpdateApplicationResources(
 		targetPendingReqs = *tpr
 	}
 
-	if err := addAndUpdateRoutingTable(
+	return addAndUpdateRoutingTable(
 		ctx,
 		logger,
 		cl,
 		routingTable,
-		httpso.Spec.Host,
+		httpso.Spec.Hosts,
 		routing.NewTarget(
 			httpso.GetNamespace(),
 			httpso.Spec.ScaleTargetRef.Service,
@@ -158,8 +154,5 @@ func createOrUpdateApplicationResources(
 			targetPendingReqs,
 		),
 		baseConfig.CurrentNamespace,
-	); err != nil {
-		return err
-	}
-	return nil
+	)
 }

operator/controllers/http/httpscaledobject_controller.go

@@ -18,9 +18,11 @@ package http
 
 import (
 	"context"
+	"errors"
 	"time"
 
-	"k8s.io/apimachinery/pkg/api/errors"
+	"github.com/go-logr/logr"
+	k8serrors "k8s.io/apimachinery/pkg/api/errors"
 	v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	ctrl "sigs.k8s.io/controller-runtime"
@@ -61,7 +63,7 @@ func (r *HTTPScaledObjectReconciler) Reconcile(ctx context.Context, req ctrl.Req
 
 	httpso := &httpv1alpha1.HTTPScaledObject{}
 	if err := r.Client.Get(ctx, req.NamespacedName, httpso); err != nil {
-		if errors.IsNotFound(err) {
+		if k8serrors.IsNotFound(err) {
 			// If the HTTPScaledObject wasn't found, it might have
 			// been deleted between the reconcile and the get.
 			// It'll automatically get garbage collected, so don't
@@ -108,6 +110,12 @@ func (r *HTTPScaledObjectReconciler) Reconcile(ctx context.Context, req ctrl.Req
 		return ctrl.Result{}, err
 	}
 
+	// ensure only host or hosts is set and if host is set that
+	// it is converted to hosts
+	if err := sanitizeHosts(logger, httpso); err != nil {
+		return ctrl.Result{}, err
+	}
+
 	// httpso is updated now
 	logger.Info(
 		"Reconciling HTTPScaledObject",
@@ -171,3 +179,28 @@ func (r *HTTPScaledObjectReconciler) SetupWithManager(mgr ctrl.Manager) error {
 		For(&httpv1alpha1.HTTPScaledObject{}, builder.WithPredicates(predicate.GenerationChangedPredicate{})).
 		Complete(r)
 }
+
+// sanitize hosts by converting the host definition to hosts are erroring
+// when both fields are set
+func sanitizeHosts(
+	logger logr.Logger,
+	httpso *httpv1alpha1.HTTPScaledObject,
+) error {
+	switch {
+	case httpso.Spec.Hosts != nil && httpso.Spec.Host != nil:
+		err := errors.New("mutually exclusive fields Error")
+		logger.Error(err, "Only one of 'hosts' or 'host' field can be defined")
+		return err
+	case httpso.Spec.Hosts == nil && httpso.Spec.Host == nil:
+		err := errors.New("no host specified Error")
+		logger.Error(err, "At least one of 'hosts' or 'host' field must be defined")
+		return err
+	case httpso.Spec.Hosts == nil:
+		httpso.Spec.Hosts = []string{*httpso.Spec.Host}
+		httpso.Spec.Host = nil
+		logger.Info("Using the 'host' field is deprecated. Please consider switching to the 'hosts' field")
+		return nil
+	default:
+		return nil
+	}
+}

operator/controllers/http/httpscaledobject_test.go

@@ -0,0 +1,65 @@
+package http
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+func TestSanitizeHostsWithOnlyHosts(t *testing.T) {
+	r := require.New(t)
+
+	testInfra := newCommonTestInfra("testns", "testapp")
+	spec := testInfra.httpso.Spec
+
+	r.NoError(sanitizeHosts(
+		testInfra.logger,
+		&testInfra.httpso,
+	))
+
+	r.Equal(spec.Hosts, testInfra.httpso.Spec.Hosts)
+	r.Nil(testInfra.httpso.Spec.Host)
+}
+
+func TestSanitizeHostsWithBothHostAndHosts(t *testing.T) {
+	r := require.New(t)
+
+	testInfra := newBrokenTestInfra("testns", "testapp")
+
+	err := sanitizeHosts(
+		testInfra.logger,
+		&testInfra.httpso,
+	)
+	r.Error(err)
+}
+
+func TestSanitizeHostsWithOnlyHost(t *testing.T) {
+	r := require.New(t)
+
+	testInfra := newDeprecatedTestInfra("testns", "testapp")
+	spec := testInfra.httpso.Spec
+
+	r.NoError(sanitizeHosts(
+		testInfra.logger,
+		&testInfra.httpso,
+	))
+
+	r.NotEqual(spec.Hosts, testInfra.httpso.Spec.Hosts)
+	r.NotEqual(spec.Host, testInfra.httpso.Spec.Host)
+	r.Nil(testInfra.httpso.Spec.Host)
+	r.Equal([]string{*spec.Host}, testInfra.httpso.Spec.Hosts)
+}
+
+func TestSanitizeHostsWithNoHostOrHosts(t *testing.T) {
+	r := require.New(t)
+
+	testInfra := newEmptyHostTestInfra("testns", "testapp")
+
+	err := sanitizeHosts(
+		testInfra.logger,
+		&testInfra.httpso,
+	)
+	r.Error(err)
+	r.Nil(testInfra.httpso.Spec.Host)
+	r.Nil(testInfra.httpso.Spec.Hosts)
+}

operator/controllers/http/routing_table.go

@@ -16,17 +16,19 @@ func removeAndUpdateRoutingTable(
 	lggr logr.Logger,
 	cl client.Client,
 	table *routing.Table,
-	host,
+	hosts []string,
 	namespace string,
 ) error {
 	lggr = lggr.WithName("removeAndUpdateRoutingTable")
-	if err := table.RemoveTarget(host); err != nil {
-		lggr.Error(
-			err,
-			"could not remove host from routing table, progressing anyway",
-			"host",
-			host,
-		)
+	for _, host := range hosts {
+		if err := table.RemoveTarget(host); err != nil {
+			lggr.Error(
+				err,
+				"could not remove host from routing table, progressing anyway",
+				"host",
+				host,
+			)
+		}
 	}
 
 	return updateRoutingMap(ctx, lggr, cl, namespace, table)
@@ -37,18 +39,20 @@ func addAndUpdateRoutingTable(
 	lggr logr.Logger,
 	cl client.Client,
 	table *routing.Table,
-	host string,
+	hosts []string,
 	target routing.Target,
 	namespace string,
 ) error {
 	lggr = lggr.WithName("addAndUpdateRoutingTable")
-	if err := table.AddTarget(host, target); err != nil {
-		lggr.Error(
-			err,
-			"could not add host to routing table, progressing anyway",
-			"host",
-			host,
-		)
+	for _, host := range hosts {
+		if err := table.AddTarget(host, target); err != nil {
+			lggr.Error(
+				err,
+				"could not add host to routing table, progressing anyway",
+				"host",
+				host,
+			)
+		}
 	}
 	return updateRoutingMap(ctx, lggr, cl, namespace, table)
 }