feat(ha): cross-cluster leader election + strict write fencing (Active-only reconcile)#409
Open
sumanthd032 wants to merge 5 commits into
Open
feat(ha): cross-cluster leader election + strict write fencing (Active-only reconcile)#409sumanthd032 wants to merge 5 commits into
sumanthd032 wants to merge 5 commits into
Conversation
Introduce pkg/ha, the foundation for cross-cluster (Active/Standby) high availability per ADR kubeslice#293 (issue kubeslice#294). - HAMode (active|standby|standalone) with fail-safe parsing: empty or unknown input maps to standalone so a misconfig never disables writes. - Lease helpers over coordination.k8s.io/v1: acquire/renew (bumping leaseTransitions on takeover), get, and a leaseDuration+padding staleness check. - ClusterLeaderElector: IsLeader() reads an atomic flag kept current by background loops, so it is cheap enough to call at the top of every Reconcile and always reflects live leadership. StartLeaseRenewal (Active) renews the local Lease and releases leadership once the renew deadline is exceeded (natural fencing). WatchRemoteLease (Standby) reads the Active's Lease and logs staleness but does not promote; promotion is issue kubeslice#297. Standalone is the default and is always the leader, preserving today's single-hub behaviour (no regression). Unit tests are race-clean and cover leadership by mode, renew-deadline loss, lease staleness, and the standby-never-promotes boundary. vendor: add controller-runtime fake client + interceptor packages (test-only) via go mod vendor. Signed-off-by: Sumanth D <sumanthd032@gmail.com>
Wire pkg/ha into the controller so only the Active hub writes (issue kubeslice#294). - Add a LeaderElector field to all nine reconcilers and a per-call guard at the top of every Reconcile: a Standby logs "standby mode, skipping reconcile" and returns without writing. The guard is nil-safe, so a reconciler built without an elector keeps today's behaviour. - main.go: add --ha-mode, --ha-identity, --ha-active-kubeconfig, --ha-lease-duration, --ha-renew-deadline, --ha-retry-period and --ha-padding-seconds; construct the elector (building a remote client from the mounted Active kubeconfig in standby mode); start StartLeaseRenewal (active) or WatchRemoteLease (standby) and pass the shared signal-handler context to the manager. - Add coordination.k8s.io/leases RBAC for the Lease. --ha-mode=standalone is the default and is always the leader, so existing single-hub deployments are unaffected (no regression). The existing --leader-elect (in-cluster pod election) is left untouched. A controller test asserts the Standby skips and logs on every call. vendor: add go.uber.org/zap/zaptest/observer (test-only). Signed-off-by: Sumanth D <sumanthd032@gmail.com>
The HA leader-election Lease lives in the controller's own namespace and is already covered by the existing leader-election Role's leases grant (config/rbac/leader_election_role.yaml). The kubebuilder marker added a redundant cluster-wide grant and left the generated manifests out of sync (make manifests was not run). Replace it with a note pointing at the role that provides the permission, keeping markers and manifests consistent. Signed-off-by: Sumanth D <sumanthd032@gmail.com>
There was a problem hiding this comment.
Pull request overview
This PR introduces cross-cluster (Active/Standby) HA leadership for the KubeSlice controller via a new pkg/ha package and wires a strict “Active-only” write fence into all reconcilers so a Standby hub returns early without performing any mutating work. It also adds unit tests and vendors a few upstream test/util packages needed for the HA tests (fake client interception and zap log observation).
Changes:
- Add
pkg/haClusterLeaderElectorbacked by acoordination.k8s.io/v1Lease (Active renews; Standby observes) with “natural fencing” (drops leadership afterrenewDeadlinewithout renewals). - Wire
LeaderElector.IsLeader()gates into all reconcilers and add a controller-level unit test validating “standby mode, skipping reconcile”. - Add
--ha-*flags and startup wiring inmain.go(including remote client bootstrap for Standby).
Reviewed changes
Copilot reviewed 17 out of 25 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| vendor/sigs.k8s.io/controller-runtime/pkg/internal/objectutil/objectutil.go | Vendored helper to filter runtime objects by label selector (used by vendored fake client). |
| vendor/sigs.k8s.io/controller-runtime/pkg/client/interceptor/intercept.go | Vendored interceptor client used to inject failures in HA tests. |
| vendor/sigs.k8s.io/controller-runtime/pkg/client/fake/doc.go | Vendored docs for controller-runtime fake client package. |
| vendor/sigs.k8s.io/controller-runtime/pkg/client/fake/client.go | Vendored controller-runtime fake client implementation used by HA unit tests. |
| vendor/modules.txt | Vendor index updated for newly vendored packages. |
| vendor/k8s.io/apimachinery/pkg/util/rand/rand.go | Vendored dependency required by the fake client. |
| vendor/go.uber.org/zap/zaptest/observer/observer.go | Vendored zap log observer used by reconcile gating test. |
| vendor/go.uber.org/zap/zaptest/observer/logged_entry.go | Vendored zap observed log entry helper. |
| pkg/ha/mode.go | HA mode enum + parsing/validation helpers. |
| pkg/ha/mode_test.go | Unit tests for HA mode parsing/validation. |
| pkg/ha/lease.go | Lease acquire/renew helpers + staleness check for Standby observation. |
| pkg/ha/lease_test.go | Unit tests for lease helpers and staleness logic. |
| pkg/ha/leader_elector.go | ClusterLeaderElector implementation (Active renew loop; Standby watch loop; in-memory IsLeader()). |
| pkg/ha/leader_elector_test.go | Unit tests for leader behavior (standalone leader; active renew; natural fencing; standby never promotes). |
| main.go | Adds --ha-* flags; constructs HA clients/elector; starts renewal/watch loops; injects elector into reconcilers. |
| controllers/worker/workerslicegateway_controller.go | Adds LeaderElector field + standby write-fence gate. |
| controllers/worker/workersliceconfig_controller.go | Adds LeaderElector field + standby write-fence gate. |
| controllers/worker/workerserviceimport_controller.go | Adds LeaderElector field + standby write-fence gate. |
| controllers/controller/vpnkey_rotation_controller.go | Adds LeaderElector field + standby write-fence gate. |
| controllers/controller/sliceqosconfig_controller.go | Adds LeaderElector field + standby write-fence gate. |
| controllers/controller/sliceconfig_controller.go | Adds LeaderElector field + standby write-fence gate. |
| controllers/controller/serviceexportconfig_controller.go | Adds LeaderElector field + standby write-fence gate. |
| controllers/controller/project_controller.go | Adds LeaderElector field + standby write-fence gate. |
| controllers/controller/leader_gate_test.go | New unit test asserting Standby returns early and logs exactly once per call. |
| controllers/controller/cluster_controller.go | Adds LeaderElector field + standby write-fence gate. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…hutdown - acquireOrRenewLease rounds LeaseDurationSeconds up to whole seconds and clamps to a minimum of 1, so a sub-second --ha-lease-duration is not truncated to 0 (invalid, and skews staleness checks). - StartLeaseRenewal and WatchRemoteLease return nil instead of ctx.Err() on context cancellation, so a graceful shutdown is not logged as an error. Signed-off-by: Sumanth D <sumanthd032@gmail.com>
The Lease namespace defaulted to a hard-coded constant, but the controller runs in a namespace injected at runtime via KUBESLICE_CONTROLLER_MANAGER_NAMESPACE (downward API), and the leader-election Role that grants leases is namespaced to that deploy namespace. Deploying into any other namespace would create the Lease where the controller has no leases permission, so the Active could not renew it and would fence itself permanently. Add --ha-lease-namespace defaulting to KUBESLICE_CONTROLLER_MANAGER_NAMESPACE so the Lease lands in the controller's own namespace. Empty (local runs) falls back to the pkg/ha default. Signed-off-by: Sumanth D <sumanthd032@gmail.com>
Comment on lines
+99
to
+101
| if opts.LeaseNamespace == "" { | ||
| opts.LeaseNamespace = DefaultLeaseNamespace | ||
| } |
Comment on lines
+306
to
+310
| // Set up cross-cluster HA leader election (ADR #293 / issue #294). In | ||
| // standalone mode (the default) the elector is always the leader, so the | ||
| // reconciler write-fence is a no-op and behaviour is unchanged. | ||
| haRunMode := ha.ParseHAMode(haMode) | ||
| localHAClient, err := client.New(mgr.GetConfig(), client.Options{Scheme: scheme}) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Adds cross-cluster (Active/Standby) leader election so only the Active hub writes, per ADR #293. New
pkg/hapackage with aClusterLeaderElector:IsLeader()reads an in-memory flag kept fresh by a background loop.coordination.k8s.io/v1Lease and drops leadership past the renew deadline (natural fencing); logsLeadershipAcquired/LeadershipLost.A per-call
IsLeader()gate at the top of all nine reconcilers makes a Standby logstandby mode, skipping reconcileand return without writing. New--ha-*flags configure mode, identity, the Active kubeconfig, and Lease timings.--ha-mode=standalone(default) is always the leader - no change for existing single-hub deployments. The in-cluster--leader-electflag is untouched. State mirroring (#295) and promotion (#297) are out of scope.Fixes #294
How Has This Been Tested?
Unit tests - race-clean (
go test -race ./pkg/ha/... ./controllers/controller/...):ClusterLeaderElector: standalone is always leader; active becomes leader only after a successful Lease renew and releases leadership once the renew deadline is exceeded (natural fencing); standby stays a non-leader even when it observes a stale Active Lease.leaseTransitions, and staleness for fresh / expired / missing-renewTime/ nil leases.standby mode, skipping reconcileexactly once perReconcilecall (asserted with a zap log observer), with the service left nil to prove the reconcile returns before any write path.End-to-end in a 3-cluster Kind setup (Active hub / Standby hub / worker):
--ha-mode=standby, create aSliceConfig→ logs the skip;kubectl get workersliceconfig,workerslicegatewayreturns nothing (fence produced no writes).--ha-mode=active→ logsLeadershipAcquired; the LeaserenewTimeadvances every retry period.Checklist:
Does this PR introduce a breaking change for other components like worker-operator?
No. Additive and gated behind
--ha-mode; the default preserves today's behaviour.