Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Move http scaled object from single host to multi host system #674

Merged
merged 3 commits into from
May 26, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion .github/workflows/e2e-tests.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,6 @@ jobs:
- name: Show Kubernetes version
run: |
kubectl version

- name: Run e2e test
run: |
make e2e-test
Expand Down
4 changes: 3 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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))
jocelynthode marked this conversation as resolved.
Show resolved Hide resolved

### Improvements

- **General**: Automatically tag Docker image with commit SHA ([#567](https://github.com/kedacore/http-add-on/issues/567))
Expand All @@ -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):

Expand Down
16 changes: 12 additions & 4 deletions config/crd/bases/http.keda.sh_httpscaledobjects.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -107,7 +116,6 @@ spec:
format: int32
type: integer
required:
- host
- scaleTargetRef
type: object
status:
Expand Down
5 changes: 4 additions & 1 deletion examples/xkcd/templates/httpscaledobject.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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" . }}
Expand Down
4 changes: 3 additions & 1 deletion examples/xkcd/templates/ingress.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@ metadata:
kubernetes.io/ingress.class: nginx
spec:
rules:
- host: {{ .Values.host }}
{{- range .Values.hosts }}
- host: {{ . | toString }}
http:
paths:
- path: /
Expand All @@ -18,3 +19,4 @@ spec:
name: keda-add-ons-http-interceptor-proxy
port:
number: 8080
{{- end }}
4 changes: 3 additions & 1 deletion examples/xkcd/values.yaml
Original file line number Diff line number Diff line change
@@ -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
Expand Down
11 changes: 8 additions & 3 deletions operator/apis/http/v1alpha1/httpscaledobject_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"`
Expand Down
19 changes: 6 additions & 13 deletions operator/controllers/http/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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,
Expand All @@ -158,8 +154,5 @@ func createOrUpdateApplicationResources(
targetPendingReqs,
),
baseConfig.CurrentNamespace,
); err != nil {
return err
}
return nil
)
}
37 changes: 35 additions & 2 deletions operator/controllers/http/httpscaledobject_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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(
jocelynthode marked this conversation as resolved.
Show resolved Hide resolved
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
}
}
65 changes: 65 additions & 0 deletions operator/controllers/http/httpscaledobject_test.go
Original file line number Diff line number Diff line change
@@ -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)
}
36 changes: 20 additions & 16 deletions operator/controllers/http/routing_table.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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)
}
Expand Down
Loading