feat(ha): Active/Standby high availability for the KubeSlice Controller - #432
Open
sumanthd032 wants to merge 80 commits into
Open
feat(ha): Active/Standby high availability for the KubeSlice Controller#432sumanthd032 wants to merge 80 commits into
sumanthd032 wants to merge 80 commits into
Conversation
Add the design record for multi-cluster Active/Standby HA of the kubeslice-controller: one Active hub holds a Lease and does all writes while a Standby mirrors its state and watches the Lease, promoting itself on Active failure. Signed-off-by: Sumanth D <sumanthd032@gmail.com>
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>
…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>
Signed-off-by: Sumanth D <sumanthd032@gmail.com>
- Clarify the Lease/Secret namespace is read from the downward API (KUBESLICE_CONTROLLER_MANAGER_NAMESPACE), not a hardcoded literal; confirm kubeslice-controller (not kubeslice-system) is correct, and add the missing --ha-lease-namespace flag. - Resolve the finalizer/stuck-Terminating open issue: StateMirror strips finalizers on write, with periodic prune-on-resync as a backstop for missed deletes. - Enumerate every process main.go starts and mark each as always-runs/Active-only/Standby-only; add a security note that ENABLE_WEBHOOKS=false is a deployment convenience, not a security boundary. - Fix --ha-mode's documented default and align the Decision 3/4 code snippets with the merged kubeslice#294 implementation (LeaderElector field name, nil-check, and the actual tested "skipping reconcile" log line). - Add an open issue on the worker's failover-time detection of status.activeController across both hubs, for #467 to settle. Signed-off-by: Sumanth D <sumanthd032@gmail.com>
NewClusterLeaderElector fell back to the hard-coded DefaultLeaseNamespace whenever LeaseNamespace was empty, independent of main.go's flag default. Since the controller already exposes KUBESLICE_CONTROLLER_MANAGER_NAMESPACE to represent its actual runtime namespace, check that env var first so the package resolves correctly on its own, not only by accident of how main.go wires the --ha-lease-namespace flag default. Only falls back to DefaultLeaseNamespace when the env var is unset too (e.g. running outside a pod). Regression tests included. Signed-off-by: Sumanth D <sumanthd032@gmail.com>
StartLeaseRenewal/WatchRemoteLease returning nil (not ctx.Err()) on context cancellation was fixed in 0da6d82 but never actually got a regression test. Also add coverage for: the mode-guard no-op branches, WatchRemoteLease failing fast without a remote client, getLease/ checkRemoteLeaseOnce propagating a missing-Lease error instead of reporting fresh, renewOnce keeping leadership on a transient failure within renewDeadline, and setLeader logging Acquired/Lost exactly once per transition (F4 in 294-evaluation.md). No production code changes. Signed-off-by: Sumanth D <sumanthd032@gmail.com>
…uard, ownerRef strip, status mirroring) Part of kubeslice#295 — the write path RemoteSyncer's workqueue will drive (next commit) to mirror KubeSlice CRDs from the Active hub onto the Standby. - MirroredResource + CRDMirrorSet (pkg/ha/mirror_set.go): the hub-side resource table (Project, Cluster, SliceConfig, ServiceExportConfig, SliceQoSConfig, VpnKeyRotation, WorkerSliceConfig, WorkerSliceGateway, WorkerServiceImport, plus core Namespace). Deliberately does not match issue kubeslice#295's own CRD table, which names Slice/SliceGateway/ ServiceExport — none of those types exist in this repo; they are worker-cluster data-plane CRDs owned by the separate worker-operator repo, irrelevant to hub-to-hub mirroring. - mirrorCreateOrUpdate/mirrorDelete (pkg/ha/mirror.go): strip resourceVersion/uid/managedFields/finalizers (+ ownerReferences only for VpnKeyRotation, the one mirrored type that carries one) before writing, label mirrored objects ha.kubeslice.io/synced-from=active, and only ever overwrite or delete a target object that already carries that label — the conflict guard that keeps the syncer off anything the Standby's own reconcilers or an operator created directly, including pre-existing namespaces like kube-system/default now that Namespace is an ordinary mirrored type rather than a special-cased cold-start step. - Every mirrored type has a status subresource. A plain Update() never touches .status once one is registered, so mirrorCreateOrUpdate always follows up with an explicit Status().Update() when the source object has a non-empty status — matching this repo's own UpdateStatus/CleanupUpdateStatus convention. Regression-tested. Fake-client unit tests cover create, update-of-existing, the conflict guard on both update and delete, delete-idempotent-on-NotFound, StripOwnerRefs true/false, and status mirroring. Signed-off-by: Sumanth D <sumanthd032@gmail.com>
Part of kubeslice#295. Registered via prometheus.NewHistogramVec/NewCounterVec + prometheus.MustRegister in pkg/ha's own init(), the same self-registration idiom metrics/prometheus.go already uses for KubeSliceEventsCounter — not routed through metrics.StartMetricsCollector, whose default labels are slice-specific and don't apply to a cross-cluster mirror. - ha_sync_lag_seconds{kind,operation}: time.Now() minus CreationTimestamp for creates; minus first-enqueue time for update/delete (RemoteSyncer's workqueue coalesces repeated events for the same object, so there's no single "delivery time" once a retry has backed off a few times — first-enqueue is the more useful number to alert on, since it reflects total time since the triggering change). - ha_sync_errors_total{kind,operation}: counts mirror failures. The syncer keeps running and retries via its workqueue on every increment; this metric never indicates a crash. vendor: add prometheus/client_golang/prometheus/testutil (test-only) via go mod vendor, used to assert metric samples/labels directly. Signed-off-by: Sumanth D <sumanthd032@gmail.com>
Part of kubeslice#295. RemoteSyncer runs only in standby mode: builds a controller-runtime cache.Cache against the Active hub's rest.Config, registers one informer per CRDMirrorSet entry, and starts a small worker pool draining a rate-limited workqueue.TypedRateLimitingInterface [syncKey] — the same primitive controller-runtime's own Controller uses internally (internal/controller.Controller). Informer callbacks (handlersFor) only enqueue a syncKey; no mirror logic runs on the informer's own goroutine. A burst of Update events for the same object coalesces into one queued item, and a worker determines the real action at dequeue time by re-reading the Active cache (found -> mirrorCreateOrUpdate, NotFound -> mirrorDelete) — the same way a Reconcile call would. This is the commit that satisfies issue kubeslice#295's own acceptance criterion that the syncer "retries without crashing": on any mirror failure, processOnce calls queue.AddRateLimited instead of dropping the key. The concrete failure mode this fixes: a namespaced object (e.g. a new Project's Cluster) created on Active after the Standby's initial sync has already completed previously had no path to retry — nothing fires again for an object that didn't change on Active once its first mirror attempt failed on "namespace not found". Now it just gets retried a few seconds later once the Namespace mirror (an ordinary CRDMirrorSet row, no special-casing needed) has landed. Start(ctx) delegates its blocking wait directly to remoteCache.Start(ctx), which itself blocks on <-ctx.Done() and returns nil — giving the "return nil, not ctx.Err(), on graceful shutdown" contract StartLeaseRenewal/WatchRemoteLease already use, for free. remoteGetFunc is a small seam (defaulted to a real cache.Cache.Get in the constructor) so the retry engine's tests exercise real workqueue backoff/redelivery behaviour without a *rest.Config or live cluster. Signed-off-by: Sumanth D <sumanthd032@gmail.com>
Part of kubeslice#295. Constructs ha.RemoteSyncer alongside the existing ClusterLeaderElector, reusing the same remote *rest.Config and local client the elector already builds rather than loading the Active kubeconfig twice — hoisted the standby block's remoteCfg to an outer remoteHACfg variable so both consumers can see it. Starts remoteSyncer.Start(ctx) in its own goroutine alongside the existing leaderElector.WatchRemoteLease(ctx) in the ha.ModeStandby switch case; a no-op in any other mode, matching RemoteSyncer's own mode check. New flag: --ha-sync-workers (default ha.DefaultSyncWorkers), matching the existing --ha-* flag style. --ha-sync-interval is intentionally not added yet — it belongs to the periodic prune backstop (pkg/ha/ prune.go), which is out of scope for this PR and would otherwise be a flag with no consumer. Signed-off-by: Sumanth D <sumanthd032@gmail.com>
…mote read identity Part of kubeslice#295. --ha-active-kubeconfig's RBAC scope on the Active cluster has been undefined since kubeslice#294 introduced the flag — the dev demo uses a full-admin kubeconfig, and config/rbac/leader_election_role.yaml only ever granted configmaps/leases, nothing for the CRD/Namespace reads RemoteSyncer now needs. The ADR (kubeslice#293) itself names this as an unaddressed deployment-level boundary without resolving it. config/ha/active-cluster-clusterrole.yaml: a read-only (get/list/watch) ClusterRole covering Namespace plus every pkg/ha.CRDMirrorSet type, plus a ClusterRoleBinding template (subject left as a placeholder — it's deployment-specific, either a ServiceAccount for in-cluster dialing or a client-cert User for a flattened kubeconfig, and can't be hardcoded). config/ha/README.md explains this must be applied on the Active cluster by whoever provisions it, not through this repo's own deploy/kustomize flow — confirmed neither config/rbac/kustomization.yaml nor config/default/kustomization.yaml reference this directory, so it can never be accidentally auto-applied to the Standby's own cluster, where it would be meaningless. Also documents, ahead of time, that a later credential-mirroring PR appending Secrets to this grant exposes every Secret in the project namespaces on Active, not just the ones actually mirrored — RBAC can't scope Secrets by .type — a real tradeoff to weigh when that lands. Signed-off-by: Sumanth D <sumanthd032@gmail.com>
Start gave up permanently on the first GetInformer/AddEventHandler error instead of retrying, unlike StartLeaseRenewal/WatchRemoteLease. Split setup into registerInformers (retries with backoff) wrapping registerInformersOnce (one attempt). A naive retry would double-register handlers on resources that already succeeded, since AddEventHandlerWithResyncPeriod isn't idempotent. Added a handlerRegistered map so retries skip resources already done. CRDMirrorSet's Namespace entry had no filter, so it mirrored every namespace on the Active hub, not just kubeslice ones — and mirrorDelete's label-only guard meant an unrelated Active-side delete could cascade-delete one of those on the Standby. Scoped the Namespace informer to util.LabelsKubeSliceController via cache.Options.ByObject. Signed-off-by: Sumanth D <sumanthd032@gmail.com>
An Active-side object mid-Terminating (deletionTimestamp set, contents still being garbage-collected) is delivered by the informer as an ordinary Update, not a Delete yet. mirrorCreateOrUpdate's payload carried deletionTimestamp straight through, and updating the Standby's non-terminating mirror with it set fails real API server immutable- field validation. Stripping it surfaced a second, related failure: the status-mirror block was still copying a Terminating status onto that now-non-terminating payload, which a real server also rejects (status.Phase may only be Terminating if deletionTimestamp is set). Both are stripped now: deletionTimestamp/deletionGracePeriodSeconds unconditionally alongside the other identity fields already cleared there, and status explicitly via delete(payload.Object, "status") rather than relying on a real API server silently ignoring .status on the main resource endpoint for subresource-registered types — a fake client does not replicate that behavior, so the code's correctness would otherwise depend on which client it's running against. The Standby still converges correctly once Active reports NotFound and mirrorDelete takes over; there's nothing useful to reflect about the in-between Terminating state. New tests: TestMirrorCreateOrUpdate_StripsDeletionTimestampFromTerminatingSource, TestMirrorCreateOrUpdate_SkipsStatusMirrorWhenSourceIsTerminating. Signed-off-by: Sumanth D <sumanthd032@gmail.com>
Dockerfile never copied pkg/ into the build context, so no image has been buildable since pkg/ha was introduced — go build works directly, docker build did not. Added COPY pkg/ pkg/ alongside the other source directories. config/rbac/role.yaml never granted namespaces/status: every other CRDMirrorSet type already has an explicit <kind>/status rule, but Namespace never needed one before it became a mirrored type with explicit status mirroring. Under real RBAC this permanently fails the Namespace status write. Added the +kubebuilder:rbac marker in main.go next to the existing namespaces rule and regenerated via make manifests. config/ha/active-cluster-clusterrole.yaml never granted coordination.k8s.io/leases: it was scoped only to what RemoteSyncer itself reads (Namespace + CRDMirrorSet), but the same --ha-active-kubeconfig identity is also used by WatchRemoteLease to read the Active's own Lease directly. Under a least-privilege identity using exactly this sample, the Standby could mirror correctly but never observe Lease staleness at all. Added the leases rule and updated the README to describe both consumers of this grant. Signed-off-by: Sumanth D <sumanthd032@gmail.com>
Informers self-heal missed updates via periodic resync and the workqueue
owns retry-on-failure, but neither can remove a mirror whose Active-side
original was deleted while the Standby wasn't watching (e.g. between two
Standby runs): cold-start informers only deliver what currently exists,
so such an orphan would survive forever.
Add a prune loop to RemoteSyncer that periodically lists Standby objects
carrying the ha.kubeslice.io/synced-from label per mirrored type, diffs
them against the remote cache, and enqueues anything no longer present
on the Active hub onto the ordinary mirror workqueue. The worker re-reads
Active at dequeue time, so the existing conflict guard, NotFound->delete
semantics, and rate-limited retry apply unchanged, and no second write
path races the workers.
Fail-safe choices:
- The first pass waits for the remote cache to sync; an unsynced cache
lists empty, which would otherwise read as "everything was deleted"
and prune every mirror on the Standby.
- A failed list (remote or local) skips that kind for the round and
increments ha_sync_errors_total{kind,"prune"} instead of pruning on
partial information.
Configurable via --ha-sync-interval (default 60s).
Part of kubeslice#295
Signed-off-by: Sumanth D <sumanthd032@gmail.com>
…lure Surface mirror failures as Kubernetes events on the Standby, attached to the object that failed to sync, using the EventRecorder main.go already builds for the reconcilers. The entry lives in config/events/controller.yaml with the generated map and config-map output from make generate-events committed alongside — RecordEvent hard-fails for any EventName missing from the generated EventsMap, so skipping that step would silently no-op the whole feature. A regression test pins the entry's presence in the generated map (and that a missing entry errors loudly), so an accidental revert of the generated code fails in go test rather than at runtime. One event per failure episode, not per retry attempt: NumRequeues is 0 only on the first failure since the last success, and early workqueue backoff retries arrive milliseconds apart — although the recorder aggregates repeats into one Event's Count, every call is still an API-server write. ha_sync_errors_total continues to count every attempt. The recorder is called directly rather than through util.RecordEvent: that helper logs via util.CtxLogger, which panics on any context that didn't pass through a reconciler's request-context setup — true for the syncer's own context (main.go's signal-handler context). Caught by the new event-emission test before it could crash a live Standby. Part of kubeslice#295 Signed-off-by: Sumanth D <sumanthd032@gmail.com>
…Account, Role, RoleBinding)
Mirror the credential objects a promoted Standby needs to serve its
worker clusters without manual re-provisioning: worker-identity RBAC
(Role/RoleBinding/ServiceAccount — the only RBAC kinds
access_control_service.go ever creates; no ClusterRole or
ClusterRoleBinding exists to mirror, despite ADR Decision 6's broader
wording) and Secrets such as the gateway certificates the ovpn job
generates.
kubernetes.io/service-account-token Secrets are excluded: SA tokens are
signed by the issuing cluster's service-account key, so an Active-minted
token is cryptographically invalid on the Standby. Mirroring the
ServiceAccount is what matters — the Standby's own token controller
mints a locally-valid token for it.
Unlike the CRD set, these are core types that exist cluster-wide, so
every row is scoped in two layers:
- Server-side, the remote informers are scoped in cache.Options:
ServiceAccount/Role/RoleBinding by the same
util.LabelsKubeSliceController selector the Namespace informer already
uses (util.GetOwnerLabel embeds that exact key/value pair on every
credential object the controller creates), and Secret — which cannot
be label-scoped, the cert-generator job creates its Secrets unlabeled
— by a field selector excluding the SA-token type.
- Client-side, every row sets RequireMirroredNamespace: the object only
mirrors if its namespace is in the remote cache's label-scoped
Namespace view, i.e. a namespace the syncer itself mirrors. The
boundary is deliberately NOT name-based: under the Helm chart's
real-world --project-namespace-prefix ("kubeslice-"), the
controller's own kubeslice-controller namespace matches the
project-namespace naming pattern, and a prefix rule would have
mirrored its webhook TLS key, image-pull credentials, and Helm
release Secrets onto the Standby — found by running the Standby
against a Helm-installed Active hub. The label boundary is the one
ReconcileProjectNamespace actually maintains. A transient failure
reading the namespace surfaces as an error (workqueue retry), never
as a silent skip.
StripOwnerRefs is set on every row: ownerReferences resolve by UID,
which never survives a cross-cluster copy, and credential objects are
also written by actors outside this repo (token controller, cert job),
so the CRD set's audited only-VpnKeyRotation-needs-it reasoning cannot
hold here.
Verified live against the real two-hub Kind topology: worker SAs,
Roles, RoleBindings, and dashboard Secrets in both project namespaces
mirror with the sync label; SA-token Secrets and everything in
kubeslice-controller and kube-system stay off the Standby.
Part of kubeslice#295
Signed-off-by: Sumanth D <sumanthd032@gmail.com>
The prune loop so far only walked one direction — Standby mirrors whose Active-side original disappeared. Walk the other direction too: an Active-side object with no Standby mirror gets re-enqueued onto the ordinary workqueue, where the worker re-reads Active and runs the full Skip/namespace/conflict-guard chain. Three real cases produce that state, none of which the forward pass or the informers handle promptly: - a mirror someone deleted directly on the Standby (previously healed only by the informer's resync period, default 10 minutes); - an object whose RequireMirroredNamespace verdict was decided before its namespace's informer had delivered on cold start — a skip is terminal for that queue item, so without this it also waited for resync; - a key stuck deep in rate-limiter backoff after repeated failures (observed live: a fixed failure cause still took minutes to heal because the next retry was scheduled ~164s out; enqueue bypasses the rate limiter's delay, so recovery lands within one prune tick). Re-enqueueing is always safe: objects that should not mirror simply no-op through the guards again. Verified live: a mirrored Secret deleted directly on the Standby was re-created 15s later, within one --ha-sync-interval, with zero errors logged. Part of kubeslice#295 Signed-off-by: Sumanth D <sumanthd032@gmail.com>
…C sample The Standby now mirrors ha.FullMirrorSet — CRDMirrorSet plus CredentialMirrorSet. No new configuration: the mirrored-namespace gate derives entirely from the label boundary the controller already maintains, so the syncer needs no knowledge of the deployment's --project-namespace-prefix. config/ha/active-cluster-clusterrole.yaml gains read access to secrets/serviceaccounts (core) and roles/rolebindings (rbac), and the README's forward-looking credential-mirroring note becomes present-tense documentation. The Secret grant's security tradeoff is stated where it will be read rather than buried: RBAC cannot scope Secrets by .type or namespace label, and a ClusterRole binding is cluster-wide, so the Standby's identity can read every Secret on the Active hub — the syncer's field selector and mirrored-namespace gate narrow what gets copied, not what the identity could read. The narrower per-namespace RoleBinding alternative is documented alongside, with its maintenance cost. Grant coverage verified live via an impersonation can-i matrix: get/list/watch allowed for every mirrored type plus the HA Lease, writes and unrelated resources denied. Fixes kubeslice#295 Signed-off-by: Sumanth D <sumanthd032@gmail.com>
…slice#297) Designing the promotion path against the implemented leader election and state mirror surfaced four places where this ADR was wrong or silent, and two where its wording invited an incorrect implementation. Amending it here so the design document and the code being built from it agree. Decision 5 — what triggers promotion. Rewritten. * Detection only covered "Lease readable but renewTime frozen". It said nothing about a Lease that cannot be read at all, so the loss of an entire hub - API server, node, or cluster - was undetectable. Both failures are the same event from the Standby's seat: the newest proof of life it holds stops advancing. Restated as one rule over the newest renewTime ever successfully read, which covers both with one threshold. * Added the two conditions that were missing entirely. A Standby must have read the Active's Lease at least once before it may promote, so a bad kubeconfig or a missing RBAC grant cannot masquerade as a dead Active; and it must confirm its own API server answers, so its own network failing is not mistaken for the Active's. Both are recorded with their costs, including that the first means a Standby restarting mid-outage will not promote. * The final dial was framed as the safety mechanism. It is not - in a partition it travels the same broken path and fails identically, so the Standby promotes anyway. Described as what it is: a guard against the Active having renewed between polls. * Step order corrected. Stopping the mirror came after leadership was granted. In the most common trigger the Active's API server is still healthy, so the mirror is still running: it would overwrite what the promoted reconcilers write, and the resync's reverse diff would re-create what they delete. The mirror must stop, and be confirmed stopped, before the write fence opens. * Added the re-enqueue step. The Decision 4 fence drops requests rather than requeuing them, so flipping it causes no reconcile at all and pre-existing mirrored state - which carries no finalizers until a reconciler re-adds them - would sit untouched until the informer resync period. Noted that a test which only creates new objects passes without this, since a new object generates its own event. * Specified the Event: the controller's own namespace, with the acquired Lease as the involved object. The namespace it belongs in was stated correctly in Decision 1 but never restated here, and kubeslice-system exists only on workers. Decision 6 — "each hub mints its own token" was written as a property the system has. It is a requirement on the registration path, which today creates the token Secret only when the ServiceAccount is absent. A mirrored ServiceAccount arrives without its Secret, so that branch never runs and a promoted hub fails every cluster reconcile while reporting success. Stated as the requirement it is. Decision 7 — the activeController field was described as something a promoted hub sets, which leaves a worker unable to identify the Active before any failover has happened. Open Issue 4 already requires each hub to self-declare while it holds leadership; made that a continuous local publisher rather than a promotion step, stated that the write never crosses into a worker cluster, and required a hub still carrying the placeholder endpoint to refuse to publish rather than advertise an unreachable failover target. Decision 8 — recorded why a recovered Active cannot be demoted automatically. The two Leases are separate objects on separate API servers, so there is no shared record of who won; a recovered hub reads its own Lease, sees itself as holder, and resumes. Flags — the two promotion durations were described in near-synonymous terms and are not the same setting: one decides when the Active is considered dead, the other how long promotion waits on its own publication step. Rewritten to distinguish them, and noted that issue kubeslice#297's --ha-promotion-grace is an alias of the shipped --ha-padding-seconds and is deliberately not implemented. Added the dial timeout and self CA bundle path. The worker's pre-failover credential for the Standby remains open and is deliberately not settled here; it spans this repo and worker-operator and needs maintainer agreement on where it lands. Signed-off-by: Sumanth D <sumanthd032@gmail.com>
The signal a worker uses to find the Active hub after a failover, per ADR kubeslice#293 Decision 7. Each hub writes this field about itself, on its own API server, and only while it holds leadership. A Standby's copy is populated by the state mirror from the Active, so it names the Active rather than itself. That is what lets a worker watching both hub endpoints resolve which one is Active by the rule "trust whichever endpoint is reachable and reports an ActiveIdentity matching that endpoint's own identity", without needing to know which role either hub currently holds, and without inferring a death from a timeout. LastUpdated is not in the ADR's YAML sketch. It is added deliberately: Decision 7's open tie-break question needs a freshness signal if a partition causes both hubs to self-declare at once, and comparing a timestamp already on the object is cheaper than making the worker read coordination.k8s.io Leases across clusters. StorageCapabilities.LastUpdated in this same struct is existing precedent for the pattern. The field is additive and omitempty throughout, so a non-HA deployment never populates it and an existing worker sees no behaviour change. The same types are being added to github.com/kubeslice/apis, which is what worker-operator imports; this repo carries its own copy of them. Note on the CRD manifest: only the activeController schema is included. make manifests also rewrites the controller-gen version annotation in all ten CRD files, because the committed manifests were generated with v0.19.0 while the Makefile pins v0.17.3. That pre-existing drift is left alone rather than folded into this change. Part of kubeslice#297 Signed-off-by: Sumanth D <sumanthd032@gmail.com>
…ship ADR kubeslice#293 Decision 7 requires each hub to declare itself on its own API server while it holds leadership, so a worker watching both hub endpoints can tell which one is Active without knowing either hub's role. Publishing only at promotion would leave a worker unable to identify the Active before the first-ever failover, so this is a continuous loop rather than a step in the promotion sequence. It is also standalone rather than part of ClusterService.ReconcileCluster, because it has to converge independently of reconciler traffic — and reconciler traffic is exactly what is absent right after a promotion, when the write fence has just opened but nothing has re-enqueued the pre-existing objects yet. PublishOnce is exported so promotion can run one synchronous pass and not tie failover latency to the tick. Details worth calling out: - The convergence check deliberately excludes LastUpdated. Including it would make every pass differ from itself and turn a convergence check into a write to every Cluster CR on every tick. - The publisher refuses to write an empty endpoint or the shipped placeholder (https://controller.cisco.com:6443/), because advertising an unreachable address as the failover target is worse than advertising nothing. Refusing is not an error: a hub that cannot describe itself should keep reconciling. - The placeholder literal is duplicated in pkg/ha rather than imported, because main.go overwrites service.ControllerEndpoint with the flag value at startup and the default is unrecoverable afterwards. TestPlaceholderMatchesServiceDefault fails if the two ever drift. - An unreadable CA bundle is logged and publication continues without it. The endpoint and identity are what select a hub, and a worker that already pins the hub's CA does not need it republished. - Nothing ever clears the field. A hub stops publishing only by losing leadership, which means it stopped renewing its Lease and is unreachable, so a worker cannot read the stale declaration anyway. Auto-demotion of a recovered hub is an explicit ADR non-goal (Decision 8), and LastUpdated is what lets a consumer prefer the fresher of two claims if it ever does see both. The elector is taken as a narrow two-method interface so the publisher is testable without a live Lease. 12 tests, covering the not-leader no-op, the converged-pass-writes-nothing property, both endpoint refusals, CA bundle encoding and absence, partial failure across clusters, and graceful shutdown. Part of kubeslice#297 Signed-off-by: Sumanth D <sumanthd032@gmail.com>
Adds --ha-self-ca-bundle-path (default the in-pod service account CA path) and starts the publisher alongside the existing HA loops. Two wiring decisions worth stating: It is deliberately not started in standalone mode. Standalone is always the leader, so the publisher would run and start writing status.activeController on every existing non-HA deployment. Leaving the field absent there is what keeps an existing worker's behaviour unchanged, which is the no-regression guarantee HA is built on. A Standby does start it. The publisher no-ops while the hub is not the leader, so it costs one list per interval and needs no extra wiring when promotion flips leadership in a later change. It writes through localHAClient — the same direct, uncached client the elector uses — rather than the manager's cached client, so it does not depend on the manager cache having started. Part of kubeslice#297 Signed-off-by: Sumanth D <sumanthd032@gmail.com>
Found in live testing against a Kind hub: a freshly started Active took 31 seconds to advertise itself, not the ~2 seconds intended. Start ran its first pass immediately, but an Active does not hold its Lease yet at that instant — acquisition lands a second or two later. So the first pass saw IsLeader() false, skipped, and the next attempt was a full publish interval away. Any worker booting inside that window could not identify the hub. Unit tests could not catch this: the test double is the leader from the first call, so the race does not exist there. The fix is driven by the loop now waiting on the short leadership interval whenever a pass found this hub was not the leader, and on the publish interval only once it is. A non-leader returns before touching the API server, so polling at 2s costs nothing while idle — and it means a Standby also picks up leadership promptly at promotion, independently of promotion remembering to call PublishOnce. The regression test then caught a second, narrower version of the same bug in the first fix: choosing the wait from its own IsLeader() call meant leadership arriving between the publish check and the wait check still cost a full interval. publishOnce now reports whether it held leadership, and the wait is chosen from what the pass actually did rather than from a second read. Verified live after the fix: published in 2s. resourceVersion held steady across a full publish interval, so the convergence check still writes nothing once converged. Part of kubeslice#297 Signed-off-by: Sumanth D <sumanthd032@gmail.com>
Signed-off-by: Sumanth D <sumanthd032@gmail.com> # Conflicts: # main.go
…ed pod checkRemoteLeaseOnce returned (false, err) when the read failed — reporting "not stale". So only one failure mode was ever detectable: the controller pod dying while its API server stayed up. An Active that lost its API server, its node, or the whole cluster was invisible, forever. That is the disaster this feature exists for, and it is the one case every demo so far could not have caught, because they all killed the process. The fix follows from noticing that the two cases are the same event. When the pod dies, reads succeed and renewTime is frozen at T. When the API server dies, reads fail, so the newest renewTime this hub has ever seen is frozen at T. In both, the newest proof of life stops advancing; the difference is at the transport layer, not in the meaning. So the elector now retains the last successfully-read Lease. A successful read replaces it; a failed read leaves it alone and logs. The verdict is then a single isLeaseStale call against that retained view, which ages on its own against a moving clock — covering both modes with one threshold, no second timer, and the already-tested staleness helper doing the work. Detection lands at roughly leaseDuration + padding + one poll in both. checkRemoteLeaseOnce still never changes leadership. It reports candidacy; the guards and the promotion sequence are separate commits, so "we think the Active is gone" and "we took over" stay independently testable, and kubeslice#294's tests asserting a Standby does not promote continue to hold.⚠️ The nil check for the retained Lease is deliberately a separate statement and must never be folded into the isLeaseStale call. isLeaseStale(nil, ...) returns TRUE — correct for its original caller, where a Lease absent from your own cluster should be created — but here it would mean a Standby that has never once read the Active's Lease promotes itself on its first tick. A broken kubeconfig, a missing RBAC grant or a mistyped namespace would each become a guaranteed split brain. TestNeverArmed_NeverBecomesCandidate fails if anyone merges the two conditions. That nil case doubles as the arming rule: never promote without having proved, at least once, that the Active's Lease is reachable. It separates "it worked, then it stopped" from "it never worked". The cost is real and accepted: a Standby restarting during an outage can never arm and so will not promote. That is the safer failure — a missed promotion is visible downtime an operator can resolve, a false one is silent dual writes — and it is now documented in the ADR beside the split-brain non-goal. lastGoodRead is recorded but not used by the verdict, which anchors on the Lease's own renewTime. It is carried so an optional local-only staleness floor stays available without a redesign if clock skew between hubs ever becomes a practical problem. Part of kubeslice#297 Signed-off-by: Sumanth D <sumanthd032@gmail.com>
Promotion does not rewrite the Deployment, so restarting a promoted hub brought it back as a Standby and left the pair with no Active. A stale lease still defers to the configured mode, which keeps a deliberate demotion working. Signed-off-by: Sumanth D <sumanthd032@gmail.com>
An accepted status write is not proof the field was stored: a Cluster CRD predating status.activeController makes the API server prune it and still return success, so the hub reported publishing while no worker could discover it. The read-back runs until it succeeds once, since this is a property of the CRD schema rather than of any single write. Signed-off-by: Sumanth D <sumanthd032@gmail.com>
The generated file lists every event under disabledEvents by template default; the Helm chart ships an empty list, so nothing is actually disabled live. Signed-off-by: Sumanth D <sumanthd032@gmail.com>
Shells out to kind/docker/kubectl to create disposable, e2e-ha-prefixed clusters and deploy this branch's controller image onto them, mirroring the proven external suite instead of adding a new orchestration dependency. Signed-off-by: Sumanth D <sumanthd032@gmail.com>
Exercises baseline mirror sync, a transient RBAC blip that must not trigger promotion, real failover promotion, and reconciliation resuming on the promoted hub, all against real disposable Kind clusters. Signed-off-by: Sumanth D <sumanthd032@gmail.com>
Covers every test behind the Active/Standby HA controller: all 182 pkg/ha unit tests by component, the reconciler write-fencing test, and the kubeslice#299 e2e suite's 4 scenarios, with commands to run each layer. Signed-off-by: Sumanth D <sumanthd032@gmail.com>
A Standby is woken by its own mirror's writes, so at info level every mirrored object produced a skip line and buried the rest of the log. The line now carries the request key it dropped and appears under --log-level=debug. Signed-off-by: Sumanth D <sumanthd032@gmail.com>
A RequireMirroredNamespace row reads an unscoped remote cache, so the Active-side listing carried every Secret on the hub and re-enqueued each out-of-scope one on every pass, forever. The reverse diff now applies the same namespace gate the worker would, memoised per pass. Signed-off-by: Sumanth D <sumanthd032@gmail.com>
Under a Deployment the hostname is the pod name, so it changes on every restart, and the worker's resolver compares identity to decide whether the hub it is talking to has changed. Active and standby now warn that --ha-identity is unpinned; standalone, which publishes no identity, stays silent. Signed-off-by: Sumanth D <sumanthd032@gmail.com>
config/ha/README.md still described SA-token Secrets as filtered out of the watch, which stopped being true once the Standby began carrying their sanitized shells. The --ha-sync-interval help likewise described only the prune half of the pass, not the reverse-diff re-enqueue. Signed-off-by: Sumanth D <sumanthd032@gmail.com>
The message was misspelled and used Errorf with nothing to format, and it named no Secret, so a failure gave an operator nothing to look up. The three older occurrences of the same typo elsewhere in this file predate this work and are left alone. Signed-off-by: Sumanth D <sumanthd032@gmail.com>
The worker operator reports its hub-connection health as conditions on its own Cluster CR, but this repository owns both the Go type and the generated CRD. Without the field the schema pruned every write and the controller's typed round-trip stripped what survived. Signed-off-by: Sumanth D <sumanthd032@gmail.com>
docs(adr): add ADR for Active/Standby HA controller (kubeslice#293)
…election feat(ha): cross-cluster leader election + strict write fencing (Active-only reconcile)
feat(): Standby remote CRD mirroring for cross-cluster HA
…-events feat(ha): drift backstop and failure events for the state mirror
…-mirror feat(ha): mirror worker credentials (Secrets/RBAC) to the Standby
feat(ha): promote a Standby to Active on Active-hub failure
fix(ha): reconcile pre-existing state after promotion, not just new events
…ken-shells feat(ha): mirror service-account token Secret shells to the Standby
fix(ha): audit follow-ups for promotion, activeController, and --ha-mode validation
feat(ha): observability metrics, events and operator runbook (kubeslice#298)
fix(ha): stop mirroring Namespace status, resume promoted hubs, and verify activeController persists
test(e2e): Active/Standby HA end-to-end suite
…ditions feat(ha): add status.conditions to the Cluster CRD
There was a problem hiding this comment.
🟡 Changes recommended
The new waitFor helper in the HA e2e suite can treat a condition as successful even when it returns an error, which can mask test failures.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR merges the Active/Standby high-availability implementation for the KubeSlice Controller, including cross-cluster leader election with write fencing, state/credential mirroring to a Standby hub, promotion/failover behavior, observability via metrics/events, and a Kind-based HA e2e suite.
Changes:
- Adds/expands the
pkg/hasubsystem (lease handling, promotion guards, resume-on-restart, mirror pruning, reconcile “kick” after promotion, events). - Gates reconcilers on HA leadership and wires an optional post-promotion requeue mechanism via
source.Channel. - Introduces an HA e2e test suite and updates manifests/CRDs/RBAC/events generation and vendored dependencies.
File summaries
| File | Description |
|---|---|
| vendor/sigs.k8s.io/controller-runtime/pkg/internal/objectutil/objectutil.go | Vendored controller-runtime helper added (label filtering utility). |
| vendor/sigs.k8s.io/controller-runtime/pkg/client/interceptor/intercept.go | Vendored controller-runtime client interceptor added (test/client utilities). |
| vendor/sigs.k8s.io/controller-runtime/pkg/client/fake/doc.go | Vendored controller-runtime fake client docs added. |
| vendor/modules.txt | Vendor index updated for new vendored packages. |
| vendor/k8s.io/apimachinery/pkg/util/rand/rand.go | Vendored apimachinery rand utilities added. |
| vendor/go.uber.org/zap/zaptest/observer/observer.go | Vendored zap observer core added (log assertions in tests). |
| vendor/go.uber.org/zap/zaptest/observer/logged_entry.go | Vendored zap observer LoggedEntry added. |
| vendor/github.com/prometheus/client_golang/prometheus/testutil/promlint/validations/units.go | Vendored Prometheus promlint validation utilities added. |
| vendor/github.com/prometheus/client_golang/prometheus/testutil/promlint/validations/histogram_validations.go | Vendored Prometheus promlint histogram validations added. |
| vendor/github.com/prometheus/client_golang/prometheus/testutil/promlint/validations/help_validations.go | Vendored Prometheus promlint help validations added. |
| vendor/github.com/prometheus/client_golang/prometheus/testutil/promlint/validations/generic_name_validations.go | Vendored Prometheus promlint naming/unit validations added. |
| vendor/github.com/prometheus/client_golang/prometheus/testutil/promlint/validations/counter_validations.go | Vendored Prometheus promlint counter validations added. |
| vendor/github.com/prometheus/client_golang/prometheus/testutil/promlint/validation.go | Vendored Prometheus promlint validation registry added. |
| vendor/github.com/prometheus/client_golang/prometheus/testutil/promlint/promlint.go | Vendored Prometheus promlint core added. |
| vendor/github.com/prometheus/client_golang/prometheus/testutil/promlint/problem.go | Vendored Prometheus promlint problem type added. |
| vendor/github.com/prometheus/client_golang/prometheus/testutil/lint.go | Vendored Prometheus testutil lint helpers added. |
| test/e2e/scenario_reconnect_test.go | HA e2e scenario: verify reconciles resume on promoted hub. |
| test/e2e/scenario_failover_test.go | HA e2e scenario: failover/promotion assertions (logs, timing, event). |
| test/e2e/scenario_blip_test.go | HA e2e scenario: transient read blip must not trigger promotion. |
| test/e2e/scenario_baseline_test.go | HA e2e baseline: steady-state lease ownership + mirror liveness. |
| test/e2e/kind_test.go | Kind orchestration + shared helpers for HA e2e suite. |
| test/e2e/ha_credentials_test.go | HA e2e: builds Standby remote-read kubeconfig Secret from Active SA token. |
| test/e2e/e2e_test.go | HA e2e entrypoint + fixture setup + scenario sequencing. |
| test/e2e/cr_helpers_test.go | HA e2e: creates minimal Project/Cluster CRs used across scenarios. |
| test/e2e/client_test.go | HA e2e: controller-runtime client scheme wiring for CR reads/writes. |
| test/e2e/assertions_test.go | HA e2e: log assertions, lease helpers, and event presence checks. |
| service/access_control_service.go | Ensures ServiceAccount token Secret creation is independent of SA existence (supports mirrored SA shells). |
| pkg/ha/resume.go | “Resume-as-Active” logic on restart for a promoted hub based on local lease state. |
| pkg/ha/resume_test.go | Unit tests for resume-on-restart behavior. |
| pkg/ha/prune.go | Prune backstop for mirror drift + reverse-diff re-enqueue behavior. |
| pkg/ha/promotion_guards.go | Promotion guards (self-health + final active-lease dial) before takeover. |
| pkg/ha/promotion_guards_test.go | Unit tests for promotion guard behavior + dial timeout enforcement. |
| pkg/ha/promotion_event.go | Emits PromotedToActive event attached to the HA Lease. |
| pkg/ha/promotion_event_test.go | Unit tests for promotion event emission and schema registration. |
| pkg/ha/mode.go | HA mode enum + strict parsing (reject typos) to avoid unfenced dual-writer. |
| pkg/ha/mode_test.go | Tests for HA mode parsing/validation. |
| pkg/ha/mirror.go | Mirror create/update/delete engine with conflict guard + status handling. |
| pkg/ha/mirror_set.go | Defines mirrored resource sets (CRDs + credentials) + sanitization rules. |
| pkg/ha/lifecycle_events.go | Emits HA lifecycle events (startup mode, promotion aborted, etc.). |
| pkg/ha/lease.go | Lease acquire/renew + staleness evaluation helpers. |
| pkg/ha/lease_test.go | Unit tests for lease helpers and transitions. |
| pkg/ha/kicker.go | Reconcile “kick” mechanism to re-enqueue existing objects post-promotion. |
| pkg/ha/kicker_test.go | Unit tests for kicker delivery semantics and safety properties. |
| pkg/ha/events_test.go | Tests for mirror sync failure event emission episode-gating. |
| Makefile | Adds test-e2e-ha target to run Kind-based HA e2e suite. |
| events/events_generated.go | Registers new HA-related Events in generated EventsMap. |
| Dockerfile | Copies pkg/ into build context so HA code is included in image build. |
| controllers/worker/workerslicegateway_controller.go | Adds HA leader gate + optional promotion kick watch. |
| controllers/worker/workersliceconfig_controller.go | Adds HA leader gate + optional promotion kick watch. |
| controllers/worker/workerserviceimport_controller.go | Adds HA leader gate + optional promotion kick watch. |
| controllers/controller/vpnkey_rotation_controller.go | Adds HA leader gate + optional promotion kick watch. |
| controllers/controller/sliceqosconfig_controller.go | Adds HA leader gate + optional promotion kick watch. |
| controllers/controller/sliceconfig_controller.go | Adds HA leader gate + optional promotion kick watch. |
| controllers/controller/serviceexportconfig_controller.go | Adds HA leader gate + optional promotion kick watch. |
| controllers/controller/promotion_kick_test.go | Tests SetupWithManager behavior with nil vs non-nil PromotionKick. |
| controllers/controller/project_controller.go | Adds HA leader gate + optional promotion kick watch. |
| controllers/controller/leader_gate_test.go | Unit test ensuring Standby reconcilers no-op and log at debug. |
| controllers/controller/cluster_controller.go | Adds HA leader gate + optional promotion kick watch. |
| config/rbac/role.yaml | Grants namespaces/status update for mirror/status behavior. |
| config/ha/README.md | Documents cross-cluster RBAC for Standby reader identity on Active cluster. |
| config/ha/active-cluster-clusterrole.yaml | Adds template ClusterRole/Binding for Standby’s remote-read permissions. |
| config/events/events_config_map.yaml | Includes HA events in events ConfigMap list. |
| config/events/controller.yaml | Adds HA event schemas (mirror failure, startup, promotion, abort, leadership lost). |
| config/default/manager_auth_proxy_patch.yaml | Sets --metrics-secure=false to align with kube-rbac-proxy upstream HTTP config. |
| config/crd/bases/controller.kubeslice.io_clusters.yaml | Adds Cluster status schema for activeController and conditions. |
| apis/controller/v1alpha1/zz_generated.deepcopy.go | Adds deepcopy support for new ClusterStatus fields/types. |
| apis/controller/v1alpha1/cluster_types.go | Adds status.activeController and status.conditions to ClusterStatus. |
| apis/controller/v1alpha1/cluster_status_conditions_test.go | Pins that ClusterStatus fields survive typed round-trip + are present in CRD schema. |
Review details
Files not reviewed (1)
- apis/controller/v1alpha1/zz_generated.deepcopy.go: Generated file
- Files reviewed: 70/89 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+133
to
+140
| for time.Now().Before(deadline) { | ||
| ok, err := cond() | ||
| if ok { | ||
| return | ||
| } | ||
| lastErr = err | ||
| time.Sleep(2 * time.Second) | ||
| } |
Conditions written as `return x == want, err` can yield (true, err) on a failed read, which let a wait converge on a value it never read. Signed-off-by: Sumanth D <sumanthd032@gmail.com>
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
Merges the completed Active/Standby HA work from
ha-integration-branch. Two controllers run against separate hub clusters and contend for one cross-cluster Lease. Only the holder reconciles, the Standby mirrors the Active's KubeSlice CRDs and worker credentials, and it promotes itself when the Active hub is gone. This covers the ADR, leader election with strict write fencing, the state and credential mirror, promotion, observability, and the e2e suite.All 16 PRs behind this are already reviewed and merged into
ha-integration-branch. The one commit not on that branch is a review follow-up tightening the e2ewaitForhelper.Needs kubeslice/kubeslice#96 so a Helm-installed Cluster CRD carries the new status fields instead of pruning them.
Fixes #293, #294, #295, #297, #298, #299, #423
How Has This Been Tested?
go test -race ./pkg/ha/... ./apis/...clean. The HA e2e suite passes on Kind across its 9 phases, and the 93-assertion scenario suite passes end to end with the worker operator. The same suite was run against real multicloud clusters, a Linode LKE Active hub and an Oracle OKE Standby, where cross-cloud failover completed in 25 seconds.Checklist:
Does this PR introduce a breaking change for other components like worker-operator?
No.
--ha-modedefaults tostandalone, which preserves today's single-controller behaviour, and the two new Cluster status fields are optional.