diff --git a/charts/wave/templates/deployment.yaml b/charts/wave/templates/deployment.yaml index ed83043..e070c92 100644 --- a/charts/wave/templates/deployment.yaml +++ b/charts/wave/templates/deployment.yaml @@ -35,6 +35,12 @@ spec: {{- if .Values.syncPeriod }} - --sync-period={{ .Values.syncPeriod }} {{- end }} + {{- if .Values.updateRate }} + - --update-rate={{ .Values.updateRate }} + {{- end }} + {{- if .Values.updateBurst }} + - --update-burst={{ .Values.updateBurst }} + {{- end }} {{- if .Values.webhooks.enabled }} - --enable-webhooks=true {{- end }} diff --git a/charts/wave/values.yaml b/charts/wave/values.yaml index 45d2507..a0c8297 100644 --- a/charts/wave/values.yaml +++ b/charts/wave/values.yaml @@ -53,6 +53,14 @@ webhooks: # Period for reconciliation # syncPeriod: 5m +# Global rate limiting for updates to prevent rapid deployment churn +# Rate limit is shared across all deployments, statefulsets, and daemonsets +# updateRate: maximum updates per second globally (default: 1.0 = 1 update per second) +# updateBurst: maximum burst size (default: 100 = allow 100 immediate updates) +# Set updateRate to 0 or very high value to disable rate limiting +updateRate: 1.0 +updateBurst: 100 + resources: {} # We usually recommend not to specify default resources and to leave this as a conscious # choice for the user. This also increases chances charts run on environments with little diff --git a/cmd/manager/main.go b/cmd/manager/main.go index 8bde92f..eb1c67f 100644 --- a/cmd/manager/main.go +++ b/cmd/manager/main.go @@ -47,6 +47,8 @@ var ( leaderElectionID = flag.String("leader-election-id", "", "Name of the configmap used by the leader election system") leaderElectionNamespace = flag.String("leader-election-namespace", "", "Namespace for the configmap used by the leader election system") syncPeriod = flag.Duration("sync-period", 10*time.Hour, "Reconcile sync period") + updateRate = flag.Float64("update-rate", core.DefaultUpdateRate, "Global maximum update rate per second (default: 1.0 = 1 update per second)") + updateBurst = flag.Int("update-burst", core.DefaultUpdateBurst, "Global maximum burst size for updates (default: 100 immediate updates allowed)") showVersion = flag.Bool("version", false, "Show version and exit") enableWebhooks = flag.Bool("enable-webhooks", false, "Enable webhooks") namespaces = flag.String("namespaces", "", "Comma-separated list of namespaces to watch. Defaults to all namespaces.") @@ -109,22 +111,26 @@ func main() { // Setup all Controllers setupLog.Info("Setting up controller") - if err := controller.AddToManager(mgr); err != nil { + controllerConfig := controller.Config{ + UpdateRate: *updateRate, + UpdateBurst: *updateBurst, + } + if err := controller.AddToManager(mgr, controllerConfig); err != nil { setupLog.Error(err, "unable to register controllers to the manager") os.Exit(1) } if *enableWebhooks { - if err := deployment.AddDeploymentWebhook(mgr); err != nil { + if err := deployment.AddDeploymentWebhook(mgr, *updateRate, *updateBurst); err != nil { setupLog.Error(err, "unable to create webhook", "webhook", "Deployment") os.Exit(1) } - if err := statefulset.AddStatefulSetWebhook(mgr); err != nil { + if err := statefulset.AddStatefulSetWebhook(mgr, *updateRate, *updateBurst); err != nil { setupLog.Error(err, "unable to create webhook", "webhook", "StatefulSet") os.Exit(1) } - if err := daemonset.AddDaemonSetWebhook(mgr); err != nil { + if err := daemonset.AddDaemonSetWebhook(mgr, *updateRate, *updateBurst); err != nil { setupLog.Error(err, "unable to create webhook", "webhook", "DaemonSet") os.Exit(1) } diff --git a/go.mod b/go.mod index 2bc6c29..8e5f5b4 100644 --- a/go.mod +++ b/go.mod @@ -8,6 +8,7 @@ require ( github.com/onsi/ginkgo/v2 v2.27.1 github.com/onsi/gomega v1.38.2 github.com/prometheus/client_golang v1.22.0 + golang.org/x/time v0.9.0 k8s.io/api v0.34.1 k8s.io/apimachinery v0.34.1 k8s.io/client-go v0.34.1 @@ -71,7 +72,6 @@ require ( golang.org/x/sys v0.37.0 // indirect golang.org/x/term v0.36.0 // indirect golang.org/x/text v0.30.0 // indirect - golang.org/x/time v0.9.0 // indirect golang.org/x/tools v0.38.0 // indirect gomodules.xyz/jsonpatch/v2 v2.4.0 // indirect google.golang.org/protobuf v1.36.10 // indirect diff --git a/hack/test-update-throttling.sh b/hack/test-update-throttling.sh new file mode 100755 index 0000000..b5e578e --- /dev/null +++ b/hack/test-update-throttling.sh @@ -0,0 +1,180 @@ +#!/usr/bin/env bash +# +# Test script for Wave's global rate limiting feature. +# +# Usage: ./hack/test-update-throttling.sh [kubernetes-version] [update-rate] [update-burst] +# +# Arguments: +# kubernetes-version: Kubernetes version for minikube (default: v1.21) +# update-rate: Updates per second globally (default: 0.2 = 1 update per 5 seconds) +# update-burst: Maximum burst size (default: 2) +# +# Examples: +# ./hack/test-update-throttling.sh # Use defaults +# ./hack/test-update-throttling.sh v1.21 0.5 5 # 0.5 updates/sec, burst of 5 +# ./hack/test-update-throttling.sh v1.21 1.0 10 # 1 update/sec, burst of 10 + +set -eu +set -o pipefail + +helm --help > /dev/null 2>&1 || { + echo "helm is not installed" + exit 1 +} + +kubectl --help > /dev/null 2>&1 || { + echo "kubectl is not installed" + exit 1 +} + +MINIKUBE_ALREADY_RUNNING=0 +kubectl get node minikube >/dev/null 2>&1 && MINIKUBE_ALREADY_RUNNING=1 + +minikube --help > /dev/null 2>&1 || { + echo "minikube is not installed" + exit 1 +} + +KUBERNETES_VERSION=${1:-v1.21} +UPDATE_RATE=${2:-0.2} +UPDATE_BURST=${3:-2} + +[ $MINIKUBE_ALREADY_RUNNING -eq 0 ] && { + echo Starting minikube... + minikube start --kubernetes-version "$KUBERNETES_VERSION" + trap "minikube delete" EXIT +} + +eval $(minikube -p minikube docker-env) +docker build -f ./Dockerfile -t wave-local:local . + +echo Installing wave with updateRate=${UPDATE_RATE} updateBurst=${UPDATE_BURST}... +helm install wave charts/wave --set image.name=wave-local --set image.tag=local --set updateRate=${UPDATE_RATE} --set updateBurst=${UPDATE_BURST} + +while [ "$(kubectl get pods -n default | grep -cE 'wave-wave')" -gt 1 ]; do echo Waiting for \"wave\" to be scheduled; sleep 10; done + +while [ "$(kubectl get pods -A | grep -cEv 'Running|Completed')" -gt 1 ]; do echo Waiting for \"cluster\" to start; sleep 10; done + +echo Creating test resources... +kubectl apply -f - <<'EOF' +apiVersion: v1 +kind: ConfigMap +metadata: + name: throttle-test +data: + counter: "0" + timestamp: "0" +EOF + +kubectl apply -f - <<'EOF' +apiVersion: apps/v1 +kind: Deployment +metadata: + name: throttle-test + annotations: + wave.pusher.com/update-on-config-change: "true" +spec: + replicas: 1 + selector: + matchLabels: + app: throttle-test + template: + metadata: + labels: + app: throttle-test + spec: + containers: + - name: test + image: nixery.dev/shell/bash + command: + - /bin/bash + - -c + - | + echo "Pod started at $(date +%s)" + echo "Counter: $(cat /etc/config/counter)" + sleep infinity + volumeMounts: + - name: config + mountPath: /etc/config + volumes: + - name: config + configMap: + name: throttle-test +EOF + +echo Waiting for initial deployment to be ready... +kubectl wait --for=condition=available --timeout=60s deployment/throttle-test + +# Get the initial pod name and creation time +INITIAL_POD=$(kubectl get pods -l app=throttle-test -o jsonpath='{.items[0].metadata.name}') +INITIAL_CREATION=$(kubectl get pod $INITIAL_POD -o jsonpath='{.metadata.creationTimestamp}') +echo "Initial pod: $INITIAL_POD created at $INITIAL_CREATION" + +# Record start time +START_TIME=$(date +%s) + +echo "" +echo "Testing update throttling by rapidly updating the ConfigMap..." +echo "Expected behavior with updateRate=${UPDATE_RATE} updateBurst=${UPDATE_BURST}:" +echo " - First ${UPDATE_BURST} updates will happen immediately (burst tokens)" +echo " - Subsequent updates will be rate-limited to ${UPDATE_RATE} per second" +echo "" + +# Rapidly update the ConfigMap 10 times +for i in 1 2 3 4 5 6 7 8 9 10; do + echo "Update $i at $(date +%H:%M:%S)" + kubectl patch configmap throttle-test --type merge -p "{\"data\":{\"counter\":\"$i\",\"timestamp\":\"$(date +%s)\"}}" + sleep 1 +done + +echo "" +echo "Waiting 20 seconds to observe throttling behavior..." +sleep 20 + +# Check how many pod restarts/updates occurred +echo "" +echo "Checking deployment update history..." +kubectl get replicasets -l app=throttle-test -o wide + +# Get all pods that were created (including terminated ones) +echo "" +echo "Checking pod creation times..." +POD_COUNT=$(kubectl get pods -l app=throttle-test --show-all 2>/dev/null | grep -c throttle-test || kubectl get pods -l app=throttle-test | grep -c throttle-test) +echo "Total pods created: $POD_COUNT" + +# Get the deployment's pod template hash changes +HASH_CHANGES=$(kubectl get replicasets -l app=throttle-test -o jsonpath='{range .items[*]}{.metadata.creationTimestamp}{"\t"}{.spec.template.spec.containers[0].image}{"\t"}{.spec.replicas}{"\n"}{end}') +echo "" +echo "ReplicaSet history (creation time, image, replicas):" +echo "$HASH_CHANGES" + +# Count how many distinct replicasets were created +RS_COUNT=$(kubectl get replicasets -l app=throttle-test --no-headers | wc -l) +echo "" +echo "Number of ReplicaSets created: $RS_COUNT" + +# Check wave controller logs for throttling messages +echo "" +echo "Wave controller logs (throttling messages):" +kubectl logs -l app=wave --tail=50 | grep -i "throttl\|delayed" || echo "No throttling messages found in logs" + +# Verify throttling worked +# Calculate expected updates: burst + (rate * time) +# With rate=0.2, burst=2, and ~30 seconds total time: +# Expected: 2 (burst) + (0.2 * 30) = 2 + 6 = 8 updates max +# We'll allow some margin and expect ≤9 to account for timing variations +echo "" +echo "=== Test Results ===" +EXPECTED_MAX=$((UPDATE_BURST + 7)) +if [ "$RS_COUNT" -le "$EXPECTED_MAX" ]; then + echo "✓ PASS: Rate limiting is working correctly" + echo " - ConfigMap was updated 10 times rapidly" + echo " - $RS_COUNT deployment updates occurred (expected ≤${EXPECTED_MAX} with rate=${UPDATE_RATE}/sec, burst=${UPDATE_BURST})" + echo " - Burst allowed ${UPDATE_BURST} immediate updates, then rate-limited to ${UPDATE_RATE} per second" + exit 0 +else + echo "✗ FAIL: Rate limiting may not be working correctly" + echo " - ConfigMap was updated 10 times rapidly" + echo " - $RS_COUNT deployment updates occurred (expected ≤${EXPECTED_MAX} with rate=${UPDATE_RATE}/sec, burst=${UPDATE_BURST})" + exit 1 +fi diff --git a/pkg/controller/add_daemonset.go b/pkg/controller/add_daemonset.go index 401c4f1..48532a5 100644 --- a/pkg/controller/add_daemonset.go +++ b/pkg/controller/add_daemonset.go @@ -18,9 +18,12 @@ package controller import ( "github.com/wave-k8s/wave/pkg/controller/daemonset" + "sigs.k8s.io/controller-runtime/pkg/manager" ) func init() { // AddToManagerFuncs is a list of functions to create controllers and add them to a manager. - AddToManagerFuncs = append(AddToManagerFuncs, daemonset.Add) + AddToManagerFuncs = append(AddToManagerFuncs, func(mgr manager.Manager, cfg Config) error { + return daemonset.Add(mgr, cfg.UpdateRate, cfg.UpdateBurst) + }) } diff --git a/pkg/controller/add_deployment.go b/pkg/controller/add_deployment.go index 409c39b..3ae8577 100644 --- a/pkg/controller/add_deployment.go +++ b/pkg/controller/add_deployment.go @@ -18,9 +18,12 @@ package controller import ( "github.com/wave-k8s/wave/pkg/controller/deployment" + "sigs.k8s.io/controller-runtime/pkg/manager" ) func init() { // AddToManagerFuncs is a list of functions to create controllers and add them to a manager. - AddToManagerFuncs = append(AddToManagerFuncs, deployment.Add) + AddToManagerFuncs = append(AddToManagerFuncs, func(mgr manager.Manager, cfg Config) error { + return deployment.Add(mgr, cfg.UpdateRate, cfg.UpdateBurst) + }) } diff --git a/pkg/controller/add_statefulset.go b/pkg/controller/add_statefulset.go index dc99d5e..8b06b3f 100644 --- a/pkg/controller/add_statefulset.go +++ b/pkg/controller/add_statefulset.go @@ -18,9 +18,12 @@ package controller import ( "github.com/wave-k8s/wave/pkg/controller/statefulset" + "sigs.k8s.io/controller-runtime/pkg/manager" ) func init() { // AddToManagerFuncs is a list of functions to create controllers and add them to a manager. - AddToManagerFuncs = append(AddToManagerFuncs, statefulset.Add) + AddToManagerFuncs = append(AddToManagerFuncs, func(mgr manager.Manager, cfg Config) error { + return statefulset.Add(mgr, cfg.UpdateRate, cfg.UpdateBurst) + }) } diff --git a/pkg/controller/controller.go b/pkg/controller/controller.go index 3e1b348..97491cf 100644 --- a/pkg/controller/controller.go +++ b/pkg/controller/controller.go @@ -20,13 +20,19 @@ import ( "sigs.k8s.io/controller-runtime/pkg/manager" ) +// Config holds configuration options for controllers +type Config struct { + UpdateRate float64 // updates per second + UpdateBurst int // maximum burst size +} + // AddToManagerFuncs is a list of functions to add all Controllers to the Manager -var AddToManagerFuncs []func(manager.Manager) error +var AddToManagerFuncs []func(manager.Manager, Config) error -// AddToManager adds all Controllers to the Manager -func AddToManager(m manager.Manager) error { +// AddToManager adds all Controllers to the Manager with the given configuration +func AddToManager(m manager.Manager, cfg Config) error { for _, f := range AddToManagerFuncs { - if err := f(m); err != nil { + if err := f(m, cfg); err != nil { return err } } diff --git a/pkg/controller/daemonset/daemonset_controller.go b/pkg/controller/daemonset/daemonset_controller.go index d462b9e..77fcb7a 100644 --- a/pkg/controller/daemonset/daemonset_controller.go +++ b/pkg/controller/daemonset/daemonset_controller.go @@ -33,16 +33,16 @@ import ( // Add creates a new DaemonSet Controller and adds it to the Manager with default RBAC. The Manager will set fields on the Controller // and Start it when the Manager is Started. -func Add(mgr manager.Manager) error { - r := newReconciler(mgr) +func Add(mgr manager.Manager, updateRate float64, updateBurst int) error { + r := newReconciler(mgr, updateRate, updateBurst) return add(mgr, r, r.handler) } // newReconciler returns a new reconcile.Reconciler -func newReconciler(mgr manager.Manager) *ReconcileDaemonSet { +func newReconciler(mgr manager.Manager, updateRate float64, updateBurst int) *ReconcileDaemonSet { return &ReconcileDaemonSet{ scheme: mgr.GetScheme(), - handler: core.NewHandler[*appsv1.DaemonSet](mgr.GetClient(), mgr.GetEventRecorderFor("wave")), + handler: core.NewHandler[*appsv1.DaemonSet](mgr.GetClient(), mgr.GetEventRecorderFor("wave"), updateRate, updateBurst), } } diff --git a/pkg/controller/daemonset/daemonset_controller_suite_test.go b/pkg/controller/daemonset/daemonset_controller_suite_test.go index bc73a29..cb3552e 100644 --- a/pkg/controller/daemonset/daemonset_controller_suite_test.go +++ b/pkg/controller/daemonset/daemonset_controller_suite_test.go @@ -19,6 +19,7 @@ package daemonset import ( "context" "log" + "math" "path/filepath" "testing" @@ -134,12 +135,12 @@ var _ = BeforeSuite(func() { m = utils.Matcher{Client: c} var recFn reconcile.Reconciler - r := newReconciler(mgr) + r := newReconciler(mgr, math.Inf(1), 1) recFn, requestsStart, requests = core.SetupControllerTestReconcile(r) Expect(add(mgr, recFn, r.handler)).NotTo(HaveOccurred()) // register mutating pod webhook - err = AddDaemonSetWebhook(mgr) + err = AddDaemonSetWebhook(mgr, math.Inf(1), 1) Expect(err).ToNot(HaveOccurred()) testCtx, testCancel = context.WithCancel(context.Background()) diff --git a/pkg/controller/daemonset/daemonset_webhook.go b/pkg/controller/daemonset/daemonset_webhook.go index 8d6a55a..9503712 100644 --- a/pkg/controller/daemonset/daemonset_webhook.go +++ b/pkg/controller/daemonset/daemonset_webhook.go @@ -28,11 +28,11 @@ func (a *DaemonSetWebhook) Default(ctx context.Context, obj runtime.Object) erro return err } -func AddDaemonSetWebhook(mgr manager.Manager) error { +func AddDaemonSetWebhook(mgr manager.Manager, updateRate float64, updateBurst int) error { err := builder.WebhookManagedBy(mgr).For(&appsv1.DaemonSet{}).WithDefaulter( &DaemonSetWebhook{ Client: mgr.GetClient(), - Handler: core.NewHandler[*appsv1.DaemonSet](mgr.GetClient(), mgr.GetEventRecorderFor("wave")), + Handler: core.NewHandler[*appsv1.DaemonSet](mgr.GetClient(), mgr.GetEventRecorderFor("wave"), updateRate, updateBurst), }).Complete() return err diff --git a/pkg/controller/deployment/deployment_controller.go b/pkg/controller/deployment/deployment_controller.go index 3366d5a..7dc4197 100644 --- a/pkg/controller/deployment/deployment_controller.go +++ b/pkg/controller/deployment/deployment_controller.go @@ -33,16 +33,16 @@ import ( // Add creates a new Deployment Controller and adds it to the Manager with default RBAC. The Manager will set fields on the Controller // and Start it when the Manager is Started. -func Add(mgr manager.Manager) error { - r := newReconciler(mgr) +func Add(mgr manager.Manager, updateRate float64, updateBurst int) error { + r := newReconciler(mgr, updateRate, updateBurst) return add(mgr, r, r.handler) } // newReconciler returns a new reconcile.Reconciler -func newReconciler(mgr manager.Manager) *ReconcileDeployment { +func newReconciler(mgr manager.Manager, updateRate float64, updateBurst int) *ReconcileDeployment { return &ReconcileDeployment{ scheme: mgr.GetScheme(), - handler: core.NewHandler[*appsv1.Deployment](mgr.GetClient(), mgr.GetEventRecorderFor("wave")), + handler: core.NewHandler[*appsv1.Deployment](mgr.GetClient(), mgr.GetEventRecorderFor("wave"), updateRate, updateBurst), } } diff --git a/pkg/controller/deployment/deployment_controller_suite_test.go b/pkg/controller/deployment/deployment_controller_suite_test.go index 0b87a22..8394a9c 100644 --- a/pkg/controller/deployment/deployment_controller_suite_test.go +++ b/pkg/controller/deployment/deployment_controller_suite_test.go @@ -19,6 +19,7 @@ package deployment import ( "context" "log" + "math" "path/filepath" "testing" @@ -136,12 +137,12 @@ var _ = BeforeSuite(func() { m = utils.Matcher{Client: c} var recFn reconcile.Reconciler - r := newReconciler(mgr) + r := newReconciler(mgr, math.Inf(1), 1) recFn, requestsStart, requests = core.SetupControllerTestReconcile(r) Expect(add(mgr, recFn, r.handler)).NotTo(HaveOccurred()) // register mutating pod webhook - err = AddDeploymentWebhook(mgr) + err = AddDeploymentWebhook(mgr, math.Inf(1), 1) Expect(err).ToNot(HaveOccurred()) testCtx, testCancel = context.WithCancel(context.Background()) diff --git a/pkg/controller/deployment/deployment_webhook.go b/pkg/controller/deployment/deployment_webhook.go index 277ffa4..6b2af98 100644 --- a/pkg/controller/deployment/deployment_webhook.go +++ b/pkg/controller/deployment/deployment_webhook.go @@ -28,11 +28,11 @@ func (a *DeploymentWebhook) Default(ctx context.Context, obj runtime.Object) err return err } -func AddDeploymentWebhook(mgr manager.Manager) error { +func AddDeploymentWebhook(mgr manager.Manager, updateRate float64, updateBurst int) error { err := builder.WebhookManagedBy(mgr).For(&appsv1.Deployment{}).WithDefaulter( &DeploymentWebhook{ Client: mgr.GetClient(), - Handler: core.NewHandler[*appsv1.Deployment](mgr.GetClient(), mgr.GetEventRecorderFor("wave")), + Handler: core.NewHandler[*appsv1.Deployment](mgr.GetClient(), mgr.GetEventRecorderFor("wave"), updateRate, updateBurst), }).Complete() return err diff --git a/pkg/controller/statefulset/statefulset_controller.go b/pkg/controller/statefulset/statefulset_controller.go index 09fecba..4186532 100644 --- a/pkg/controller/statefulset/statefulset_controller.go +++ b/pkg/controller/statefulset/statefulset_controller.go @@ -33,16 +33,16 @@ import ( // Add creates a new StatefulSet Controller and adds it to the Manager with default RBAC. The Manager will set fields on the Controller // and Start it when the Manager is Started. -func Add(mgr manager.Manager) error { - r := newReconciler(mgr) +func Add(mgr manager.Manager, updateRate float64, updateBurst int) error { + r := newReconciler(mgr, updateRate, updateBurst) return add(mgr, r, r.handler) } // newReconciler returns a new reconcile.Reconciler -func newReconciler(mgr manager.Manager) *ReconcileStatefulSet { +func newReconciler(mgr manager.Manager, updateRate float64, updateBurst int) *ReconcileStatefulSet { return &ReconcileStatefulSet{ scheme: mgr.GetScheme(), - handler: core.NewHandler[*appsv1.StatefulSet](mgr.GetClient(), mgr.GetEventRecorderFor("wave")), + handler: core.NewHandler[*appsv1.StatefulSet](mgr.GetClient(), mgr.GetEventRecorderFor("wave"), updateRate, updateBurst), } } diff --git a/pkg/controller/statefulset/statefulset_controller_suite_test.go b/pkg/controller/statefulset/statefulset_controller_suite_test.go index 1a5aff1..06b0e64 100644 --- a/pkg/controller/statefulset/statefulset_controller_suite_test.go +++ b/pkg/controller/statefulset/statefulset_controller_suite_test.go @@ -19,6 +19,7 @@ package statefulset import ( "context" "log" + "math" "path/filepath" "testing" @@ -136,12 +137,12 @@ var _ = BeforeSuite(func() { m = utils.Matcher{Client: c} var recFn reconcile.Reconciler - r := newReconciler(mgr) + r := newReconciler(mgr, math.Inf(1), 1) recFn, requestsStart, requests = core.SetupControllerTestReconcile(r) Expect(add(mgr, recFn, r.handler)).NotTo(HaveOccurred()) // register mutating pod webhook - err = AddStatefulSetWebhook(mgr) + err = AddStatefulSetWebhook(mgr, math.Inf(1), 1) Expect(err).ToNot(HaveOccurred()) testCtx, testCancel = context.WithCancel(context.Background()) diff --git a/pkg/controller/statefulset/statefulset_webhook.go b/pkg/controller/statefulset/statefulset_webhook.go index d187443..77e7e93 100644 --- a/pkg/controller/statefulset/statefulset_webhook.go +++ b/pkg/controller/statefulset/statefulset_webhook.go @@ -28,11 +28,11 @@ func (a *StatefulSetWebhook) Default(ctx context.Context, obj runtime.Object) er return err } -func AddStatefulSetWebhook(mgr manager.Manager) error { +func AddStatefulSetWebhook(mgr manager.Manager, updateRate float64, updateBurst int) error { err := builder.WebhookManagedBy(mgr).For(&appsv1.StatefulSet{}).WithDefaulter( &StatefulSetWebhook{ Client: mgr.GetClient(), - Handler: core.NewHandler[*appsv1.StatefulSet](mgr.GetClient(), mgr.GetEventRecorderFor("wave")), + Handler: core.NewHandler[*appsv1.StatefulSet](mgr.GetClient(), mgr.GetEventRecorderFor("wave"), updateRate, updateBurst), }).Complete() return err diff --git a/pkg/core/backoff.go b/pkg/core/backoff.go new file mode 100644 index 0000000..167d0bc --- /dev/null +++ b/pkg/core/backoff.go @@ -0,0 +1,71 @@ +/* +Copyright 2018 Pusher Ltd. and Wave Contributors + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package core + +import ( + "context" + "math" + + "golang.org/x/time/rate" + "k8s.io/apimachinery/pkg/types" +) + +const ( + // DefaultUpdateRate is the default rate of updates per second (1.0 = 1 update per second) + DefaultUpdateRate = 1.0 + // DefaultUpdateBurst is the default burst size (allows 100 immediate updates) + DefaultUpdateBurst = 100 +) + +// UpdateThrottler manages global rate-limited updates using token bucket algorithm +// All deployments, statefulsets, and daemonsets share the same rate limiter +type UpdateThrottler struct { + limiter *rate.Limiter + rate rate.Limit + burst int +} + +// NewUpdateThrottler creates a new UpdateThrottler with the specified rate and burst +// rate: number of updates per second globally (default 1.0 = 1 update per second) +// burst: maximum number of updates that can happen in quick succession (default 100) +// If rate <= 0, uses DefaultUpdateRate (1.0). If rate is Inf, throttling is disabled. +// If burst <= 0, uses DefaultUpdateBurst (100) +func NewUpdateThrottler(r rate.Limit, burst int) *UpdateThrottler { + if r <= 0 && !math.IsInf(float64(r), 1) { + r = DefaultUpdateRate + } + if burst <= 0 { + burst = DefaultUpdateBurst + } + return &UpdateThrottler{ + limiter: rate.NewLimiter(r, burst), + rate: r, + burst: burst, + } +} + +// Wait blocks until the global rate limiter allows an update +// This stalls the operator pipeline to enforce rate limiting across all instances +func (ut *UpdateThrottler) Wait(ctx context.Context, name types.NamespacedName) error { + // If rate limiting is disabled (infinite rate), return immediately + if math.IsInf(float64(ut.rate), 1) { + return nil + } + + // Block until global rate limiter allows the operation + return ut.limiter.Wait(ctx) +} diff --git a/pkg/core/backoff_test.go b/pkg/core/backoff_test.go new file mode 100644 index 0000000..79bb950 --- /dev/null +++ b/pkg/core/backoff_test.go @@ -0,0 +1,225 @@ +/* +Copyright 2018 Pusher Ltd. and Wave Contributors + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package core + +import ( + "context" + "math" + "testing" + "time" + + "golang.org/x/time/rate" + "k8s.io/apimachinery/pkg/types" +) + +func TestUpdateThrottler_InfiniteRate(t *testing.T) { + // Test that infinite rate disables throttling + ut := NewUpdateThrottler(rate.Limit(math.Inf(1)), 1) + name := types.NamespacedName{Namespace: "test", Name: "deployment"} + ctx := context.Background() + + // Multiple rapid updates should all succeed immediately with no delay + start := time.Now() + for i := 0; i < 10; i++ { + if err := ut.Wait(ctx, name); err != nil { + t.Errorf("Wait %d failed: %v", i+1, err) + } + } + elapsed := time.Since(start) + + // Should complete almost instantly (allow 100ms for overhead) + if elapsed > 100*time.Millisecond { + t.Errorf("Infinite rate should not cause delays, took %v", elapsed) + } +} + +func TestUpdateThrottler_RateLimiting(t *testing.T) { + // Test basic rate limiting: 10 updates per second (rate = 10.0) + // With burst=1, first update is immediate, second should be delayed + ut := NewUpdateThrottler(rate.Limit(10.0), 1) + name := types.NamespacedName{Namespace: "test", Name: "deployment"} + ctx := context.Background() + + // First update should be immediate (uses burst token) + start := time.Now() + if err := ut.Wait(ctx, name); err != nil { + t.Fatalf("First wait failed: %v", err) + } + firstElapsed := time.Since(start) + if firstElapsed > 10*time.Millisecond { + t.Errorf("First update should be immediate, took %v", firstElapsed) + } + + // Second update should be delayed by ~100ms (1/10 second) + start = time.Now() + if err := ut.Wait(ctx, name); err != nil { + t.Fatalf("Second wait failed: %v", err) + } + secondElapsed := time.Since(start) + if secondElapsed < 50*time.Millisecond { + t.Errorf("Second update should be delayed, but took only %v", secondElapsed) + } +} + +func TestUpdateThrottler_BurstAllowance(t *testing.T) { + // Test burst: rate=1.0 (1 per second), burst=3 allows 3 immediate updates + ut := NewUpdateThrottler(rate.Limit(1.0), 3) + name := types.NamespacedName{Namespace: "test", Name: "deployment"} + ctx := context.Background() + + // First 3 updates should be immediate (use burst tokens) + start := time.Now() + for i := 0; i < 3; i++ { + if err := ut.Wait(ctx, name); err != nil { + t.Fatalf("Wait %d failed: %v", i+1, err) + } + } + burstElapsed := time.Since(start) + if burstElapsed > 50*time.Millisecond { + t.Errorf("First 3 updates should be immediate (burst), took %v", burstElapsed) + } + + // 4th update should be delayed by ~1 second + start = time.Now() + if err := ut.Wait(ctx, name); err != nil { + t.Fatalf("4th wait failed: %v", err) + } + fourthElapsed := time.Since(start) + if fourthElapsed < 500*time.Millisecond { + t.Errorf("4th update should be delayed by ~1 second, but took only %v", fourthElapsed) + } +} + +func TestUpdateThrottler_MultipleInstances(t *testing.T) { + // Test that different instances share the same global rate limiter + ut := NewUpdateThrottler(rate.Limit(10.0), 1) + name1 := types.NamespacedName{Namespace: "test", Name: "deployment1"} + name2 := types.NamespacedName{Namespace: "test", Name: "deployment2"} + ctx := context.Background() + + // First instance uses the burst token + start := time.Now() + if err := ut.Wait(ctx, name1); err != nil { + t.Fatalf("Wait for name1 failed: %v", err) + } + firstElapsed := time.Since(start) + if firstElapsed > 10*time.Millisecond { + t.Errorf("First update should be immediate (uses burst), took %v", firstElapsed) + } + + // Second instance should be delayed since burst token was used + start = time.Now() + if err := ut.Wait(ctx, name2); err != nil { + t.Fatalf("Wait for name2 failed: %v", err) + } + secondElapsed := time.Since(start) + if secondElapsed < 50*time.Millisecond { + t.Errorf("Second update should be delayed (global rate limit), took only %v", secondElapsed) + } +} + +func TestUpdateThrottler_ContextCancellation(t *testing.T) { + // Test that context cancellation stops waiting + ut := NewUpdateThrottler(rate.Limit(0.1), 1) // Very slow: 1 update per 10 seconds + name := types.NamespacedName{Namespace: "test", Name: "deployment"} + + // Use up the burst token + if err := ut.Wait(context.Background(), name); err != nil { + t.Fatalf("Initial wait failed: %v", err) + } + + // Create a context that will be cancelled + ctx, cancel := context.WithTimeout(context.Background(), 100*time.Millisecond) + defer cancel() + + // This wait should be cancelled by context timeout + start := time.Now() + err := ut.Wait(ctx, name) + elapsed := time.Since(start) + + if err == nil { + t.Error("Expected context cancellation error, got nil") + } + if elapsed > 200*time.Millisecond { + t.Errorf("Should fail quickly on context cancellation, took %v", elapsed) + } +} + +func TestUpdateThrottler_DefaultValues(t *testing.T) { + // Test that invalid rate/burst values use defaults + + // Zero rate should use default + ut := NewUpdateThrottler(0, 1) + if ut.rate != DefaultUpdateRate { + t.Errorf("Expected default rate %v, got %v", DefaultUpdateRate, ut.rate) + } + + // Negative rate should use default + ut = NewUpdateThrottler(-1, 1) + if ut.rate != DefaultUpdateRate { + t.Errorf("Expected default rate %v, got %v", DefaultUpdateRate, ut.rate) + } + + // Zero burst should use default + ut = NewUpdateThrottler(1.0, 0) + if ut.burst != DefaultUpdateBurst { + t.Errorf("Expected default burst %d, got %d", DefaultUpdateBurst, ut.burst) + } + + // Negative burst should use default + ut = NewUpdateThrottler(1.0, -1) + if ut.burst != DefaultUpdateBurst { + t.Errorf("Expected default burst %d, got %d", DefaultUpdateBurst, ut.burst) + } + + // Valid values should be preserved + ut = NewUpdateThrottler(5.0, 10) + if ut.rate != 5.0 { + t.Errorf("Expected rate 5.0, got %v", ut.rate) + } + if ut.burst != 10 { + t.Errorf("Expected burst 10, got %d", ut.burst) + } +} + +func TestUpdateThrottler_PreventChurn(t *testing.T) { + // Test the original issue #182: rapid updates causing deployment churn + // Rate of 0.1 = 1 update per 10 seconds + ut := NewUpdateThrottler(rate.Limit(0.1), 1) + name := types.NamespacedName{Namespace: "test", Name: "deployment"} + ctx := context.Background() + + // First update should be immediate + start := time.Now() + if err := ut.Wait(ctx, name); err != nil { + t.Fatalf("First wait failed: %v", err) + } + if time.Since(start) > 50*time.Millisecond { + t.Error("First update should be immediate") + } + + // Simulate rapid reconciliations (buggy controller scenario) + // Second update should be delayed by ~10 seconds + // We'll use a timeout to avoid actually waiting 10 seconds + ctx, cancel := context.WithTimeout(context.Background(), 100*time.Millisecond) + defer cancel() + + err := ut.Wait(ctx, name) + if err == nil { + t.Error("Expected rate limiting to delay second update") + } +} diff --git a/pkg/core/children_test.go b/pkg/core/children_test.go index 7fd6c3c..4868038 100644 --- a/pkg/core/children_test.go +++ b/pkg/core/children_test.go @@ -18,6 +18,7 @@ package core import ( "context" + "math" "sync" "time" @@ -70,7 +71,7 @@ var _ = Describe("Wave children Suite", func() { Expect(cerr).NotTo(HaveOccurred()) c = mgr.GetClient() // h = NewHandler(c, mgr.GetEventRecorderFor("wave")) - h = NewHandler[*appsv1.Deployment](mgr.GetClient(), mgr.GetEventRecorderFor("wave")) + h = NewHandler[*appsv1.Deployment](mgr.GetClient(), mgr.GetEventRecorderFor("wave"), math.Inf(1), 1) m = utils.Matcher{Client: c} diff --git a/pkg/core/delete_test.go b/pkg/core/delete_test.go index a4cea58..c6f0c6e 100644 --- a/pkg/core/delete_test.go +++ b/pkg/core/delete_test.go @@ -17,6 +17,8 @@ limitations under the License. package core import ( + "context" + "math" "sync" "time" @@ -55,7 +57,7 @@ var _ = Describe("Wave migration Suite", func() { var cerr error c, cerr = client.New(cfg, client.Options{Scheme: scheme.Scheme}) Expect(cerr).NotTo(HaveOccurred()) - h = NewHandler[*appsv1.Deployment](c, mgr.GetEventRecorderFor("wave")) + h = NewHandler[*appsv1.Deployment](c, mgr.GetEventRecorderFor("wave"), math.Inf(1), 1) m = utils.Matcher{Client: c} // Create some configmaps and secrets @@ -111,7 +113,7 @@ var _ = Describe("Wave migration Suite", func() { return obj }, timeout).Should(Succeed()) - _, err := h.handlePodController(deploymentObject) + _, err := h.handlePodController(context.TODO(), deploymentObject) Expect(err).NotTo(HaveOccurred()) }) diff --git a/pkg/core/handler.go b/pkg/core/handler.go index d7bf6bf..143e212 100644 --- a/pkg/core/handler.go +++ b/pkg/core/handler.go @@ -21,6 +21,7 @@ import ( "fmt" "sync" + "golang.org/x/time/rate" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/types" @@ -36,10 +37,11 @@ type Handler[I InstanceType] struct { recorder record.EventRecorder watchedConfigmaps WatcherList watchedSecrets WatcherList + updateThrottler *UpdateThrottler } // NewHandler constructs a new instance of Handler -func NewHandler[I InstanceType](c client.Client, r record.EventRecorder) *Handler[I] { +func NewHandler[I InstanceType](c client.Client, r record.EventRecorder, updateRate float64, updateBurst int) *Handler[I] { return &Handler[I]{Client: c, recorder: r, watchedConfigmaps: WatcherList{ watchers: make(map[types.NamespacedName]map[types.NamespacedName]bool), @@ -48,7 +50,9 @@ func NewHandler[I InstanceType](c client.Client, r record.EventRecorder) *Handle watchedSecrets: WatcherList{ watchers: make(map[types.NamespacedName]map[types.NamespacedName]bool), watchersMutex: &sync.RWMutex{}, - }} + }, + updateThrottler: NewUpdateThrottler(rate.Limit(updateRate), updateBurst), + } } // HandleWebhook is called by the webhook @@ -69,11 +73,11 @@ func (h *Handler[I]) Handle(ctx context.Context, namespacesName types.Namespaced return reconcile.Result{}, err } - return h.handlePodController(instance) + return h.handlePodController(ctx, instance) } // handlePodController reconciles the state of a podController -func (h *Handler[I]) handlePodController(instance I) (reconcile.Result, error) { +func (h *Handler[I]) handlePodController(ctx context.Context, instance I) (reconcile.Result, error) { log := logf.Log.WithName("wave").WithValues("namespace", instance.GetNamespace(), "name", instance.GetName()) // To cleanup legacy ownerReferences and finalizer @@ -125,6 +129,13 @@ func (h *Handler[I]) handlePodController(instance I) (reconcile.Result, error) { // If the desired state doesn't match the existing state, update it if hash != oldHash || schedulingChange { + instanceName := GetNamespacedNameFromObject(instance) + + // Wait for rate limiter (stalls the pipeline until allowed) + if err := h.updateThrottler.Wait(ctx, instanceName); err != nil { + return reconcile.Result{}, fmt.Errorf("error waiting for rate limit: %v", err) + } + log.V(0).Info("Updating instance hash", "hash", hash) h.recorder.Eventf(instance, corev1.EventTypeNormal, "ConfigChanged", "Configuration hash updated to %s", hash) diff --git a/pkg/core/handler_test.go b/pkg/core/handler_test.go index d2b4564..6801366 100644 --- a/pkg/core/handler_test.go +++ b/pkg/core/handler_test.go @@ -18,6 +18,7 @@ package core import ( "context" + "math" "sync" "time" @@ -76,7 +77,7 @@ var _ = Describe("Wave controller Suite", func() { c, cerr = client.New(cfg, client.Options{Scheme: scheme.Scheme}) Expect(cerr).NotTo(HaveOccurred()) - h = NewHandler[*appsv1.Deployment](c, mgr.GetEventRecorderFor("wave")) + h = NewHandler[*appsv1.Deployment](c, mgr.GetEventRecorderFor("wave"), math.Inf(1), 1) m = utils.Matcher{Client: c} stopMgr, mgrStopped = StartTestManager(mgr) diff --git a/pkg/core/owner_references_test.go b/pkg/core/owner_references_test.go index d71a8e3..8b4ac42 100644 --- a/pkg/core/owner_references_test.go +++ b/pkg/core/owner_references_test.go @@ -18,6 +18,7 @@ package core import ( "context" + "math" "sync" "time" @@ -63,7 +64,7 @@ var _ = Describe("Wave owner references Suite", func() { var cerr error c, cerr = client.New(cfg, client.Options{Scheme: scheme.Scheme}) Expect(cerr).NotTo(HaveOccurred()) - h = NewHandler[*appsv1.Deployment](c, mgr.GetEventRecorderFor("wave")) + h = NewHandler[*appsv1.Deployment](c, mgr.GetEventRecorderFor("wave"), math.Inf(1), 1) m = utils.Matcher{Client: c} // Create some configmaps and secrets