From a71903fe9692ebedf32547a729d010c40739e99e Mon Sep 17 00:00:00 2001 From: Michalis Kargakis Date: Sat, 17 Oct 2026 21:44:41 +0200 Subject: [PATCH] Run the node signal adapters on every replica, not only the leader --- README.md | 7 ++- docs/how-it-works.md | 3 +- internal/controller/standby_test.go | 95 +++++++++++++++++++++++++++++ internal/signal/cordon/cordon.go | 4 ++ internal/signal/taint/taint.go | 4 ++ 5 files changed, 109 insertions(+), 4 deletions(-) create mode 100644 internal/controller/standby_test.go diff --git a/README.md b/README.md index 00a4a9c..21e51cb 100644 --- a/README.md +++ b/README.md @@ -296,9 +296,10 @@ The operator refuses to manage itself, skips targets it cannot help, and stands down while a rollout is in progress. For production, use `replicaCount: 2` and place the replicas on different -nodes. The webhook serves from every replica even though reconciliation is -leader-gated. If the node hosting the only operator pod is drained, its own -eviction is admitted and protection for pods later in that drain can disappear. +nodes. Every replica watches node signals and serves the webhook even though +reconciliation is leader-gated. If the node hosting the only operator pod is +drained, its own eviction is admitted and protection for pods later in that +drain can disappear. ## Limits diff --git a/docs/how-it-works.md b/docs/how-it-works.md index 9660036..4d8af38 100644 --- a/docs/how-it-works.md +++ b/docs/how-it-works.md @@ -158,7 +158,8 @@ holds immediately. An operator restart therefore creates a protection gap rather than a stuck cluster. Run two replicas on different nodes for production. Reconciliation -uses leader election, but every replica serves webhook traffic. +uses leader election, but every replica watches node signals and serves +webhook traffic. The upgrade from 0.3.x runs a leader-gated startup sweep that deletes legacy PodDisruptionBudgets labeled `understudy.sh/owned=true`. PDB read/delete RBAC diff --git a/internal/controller/standby_test.go b/internal/controller/standby_test.go new file mode 100644 index 0000000..c8a82a8 --- /dev/null +++ b/internal/controller/standby_test.go @@ -0,0 +1,95 @@ +/* +Copyright 2026 The Understudy Authors. + +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 controller + +import ( + "context" + "time" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + + coordinationv1 "k8s.io/api/coordination/v1" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/kubernetes/scheme" + "k8s.io/utils/ptr" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/config" + metricsserver "sigs.k8s.io/controller-runtime/pkg/metrics/server" + + "github.com/kylan11/understudy/internal/signal" + "github.com/kylan11/understudy/internal/signal/cordon" + "github.com/kylan11/understudy/internal/signal/taint" +) + +var _ = Describe("A replica without the leader lease", func() { + It("still sees node doom signals, since it serves the webhook too", func() { + const leaseName = "standby.understudy.sh" + + By("leaving the lease to another replica") + now := metav1.NewMicroTime(time.Now()) + Expect(k8sClient.Create(ctx, &coordinationv1.Lease{ + ObjectMeta: metav1.ObjectMeta{Name: leaseName, Namespace: metav1.NamespaceDefault}, + Spec: coordinationv1.LeaseSpec{ + HolderIdentity: ptr.To("leader"), + LeaseDurationSeconds: ptr.To(int32(3600)), + AcquireTime: &now, + RenewTime: &now, + }, + })).To(Succeed()) + + mgr, err := ctrl.NewManager(cfg, ctrl.Options{ + Scheme: scheme.Scheme, + Metrics: metricsserver.Options{BindAddress: "0"}, + LeaderElection: true, + LeaderElectionID: leaseName, + LeaderElectionNamespace: metav1.NamespaceDefault, + Controller: config.Controller{SkipNameValidation: ptr.To(true)}, + }) + Expect(err).NotTo(HaveOccurred()) + standby := signal.NewRegistry() + Expect(cordon.New().SetupWithManager(mgr, standby)).To(Succeed()) + Expect(taint.New(nil).SetupWithManager(mgr, standby)).To(Succeed()) + + mgrCtx, stop := context.WithCancel(ctx) + stopped := make(chan struct{}) + go func() { + defer GinkgoRecover() + defer close(stopped) + // A replica that never led reports "leader election lost" on shutdown. + _ = mgr.Start(mgrCtx) + }() + DeferCleanup(func() { + stop() + Eventually(stopped).Should(BeClosed()) + }) + + By("cordoning one node and tainting another") + cordoned := &corev1.Node{ObjectMeta: metav1.ObjectMeta{Name: "standby-cordoned"}} + tainted := &corev1.Node{ObjectMeta: metav1.ObjectMeta{Name: "standby-tainted"}} + Expect(k8sClient.Create(ctx, cordoned)).To(Succeed()) + Expect(k8sClient.Create(ctx, tainted)).To(Succeed()) + cordonNode(cordoned) + taintNode(tainted, "ToBeDeletedByClusterAutoscaler") + + By("recording both in the standby's registry without ever leading") + Eventually(func() bool { _, doomed := standby.Lookup(cordoned.Name); return doomed }).Should(BeTrue()) + Eventually(func() bool { _, doomed := standby.Lookup(tainted.Name); return doomed }).Should(BeTrue()) + Expect(mgr.Elected()).NotTo(BeClosed()) + }) +}) diff --git a/internal/signal/cordon/cordon.go b/internal/signal/cordon/cordon.go index abe9966..e50e53d 100644 --- a/internal/signal/cordon/cordon.go +++ b/internal/signal/cordon/cordon.go @@ -21,8 +21,10 @@ import ( corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/utils/ptr" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/controller" "sigs.k8s.io/controller-runtime/pkg/event" "sigs.k8s.io/controller-runtime/pkg/predicate" @@ -46,6 +48,8 @@ func (a *Adapter) SetupWithManager(mgr ctrl.Manager, reg *signal.Registry) error return ctrl.NewControllerManagedBy(mgr). For(&corev1.Node{}). Named("signal-cordon"). + // Every replica serves the webhook from its own registry, and this only writes to it. + WithOptions(controller.Options{NeedLeaderElection: ptr.To(false)}). WithEventFilter(predicate.Funcs{ CreateFunc: func(e event.CreateEvent) bool { return true }, DeleteFunc: func(e event.DeleteEvent) bool { return true }, diff --git a/internal/signal/taint/taint.go b/internal/signal/taint/taint.go index fd15bbc..af7f86b 100644 --- a/internal/signal/taint/taint.go +++ b/internal/signal/taint/taint.go @@ -23,8 +23,10 @@ import ( corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/utils/ptr" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/controller" "sigs.k8s.io/controller-runtime/pkg/event" "sigs.k8s.io/controller-runtime/pkg/predicate" @@ -93,6 +95,8 @@ func (a *Adapter) SetupWithManager(mgr ctrl.Manager, reg *signal.Registry) error return ctrl.NewControllerManagedBy(mgr). For(&corev1.Node{}). Named("signal-taint"). + // Every replica serves the webhook from its own registry, and this only writes to it. + WithOptions(controller.Options{NeedLeaderElection: ptr.To(false)}). WithEventFilter(predicate.Funcs{ CreateFunc: func(e event.CreateEvent) bool { return true }, DeleteFunc: func(e event.DeleteEvent) bool { return true },