feat(hub): follow a controller failover to the promoted hub (#467) - #498
Open
sumanthd032 wants to merge 5 commits into
Open
feat(hub): follow a controller failover to the promoted hub (#467)#498sumanthd032 wants to merge 5 commits into
sumanthd032 wants to merge 5 commits into
Conversation
A worker cannot learn about a controller failover from the hub that just failed, because the promotion is recorded on the other hub. So each hub publishes status.activeController on this worker's own Cluster CR while it holds leadership, and a Standby's mirrored copy repeats the Active's declaration. This is the worker's half: read that field from both pre-provisioned endpoints and decide who to talk to. Wired to nothing. No caller, no behaviour change, and a non-HA worker is byte-identical. The decision logic is the part with real substance, it reviews on its own, and landing it separately keeps the change that touches the hub connection small when it comes. The rule, in order: an unreachable hub has no say; a hub that published nothing has no say, which is what every non-HA deployment looks like from here; a claim naming an endpoint outside the configured candidate set is rejected, because the field selects among endpoints an operator provisioned rather than pointing the worker at arbitrary addresses; agreement between the hubs wins, and agreement is the normal case since the Standby mirrors the Active's declaration; disagreement prefers the fresher declaration, which keeps behaviour single-valued during the split brain the design does not claim to solve. No usable claim means change nothing — a worker that disconnected whenever it was unsure would turn every hub blip into a worker outage. Two properties worth their own tests. A switch needs consecutive confirming polls, so one divergent poll cannot move a worker, and the comparison excludes LastUpdated: the Active republishes on a timer, so including it would reset the counter every poll and no switch could ever confirm. Every read is deadline-bounded, because an API server that accepts a connection and then stops answering hangs until the OS TCP timeout otherwise; the controller side of this feature shipped that bug and measured a single read blocking ~12s against a stopped API server. The field is read unstructured rather than through the shared github.com/kubeslice/apis types. It is four scalars out of one status field, and reading them untyped keeps a third repository's release cadence off the critical path of a package that is otherwise self-contained. Nothing in go.mod, go.sum or vendor/ changes as a result. Part of kubeslice#467 Signed-off-by: Sumanth D <sumanthd032@gmail.com>
…ment The endpoint and the credentials for a hub are only valid as a pair, but they were reaching the client builders by two different routes. HUB_HOST_ENDPOINT was read inside NewHubClientConfig and manager.Start, while HubTokenFile and HubCAFile are package-level vars evaluated before main runs. That difference does not matter with one hub and cannot be worked around with two. Overriding the environment from main would move the address without moving the token, producing a client aimed at one hub authenticating as the other — which fails as a TLS or authorization error and reads like a network fault. Both now take a hub.Connection carrying all three together. Callers with a single hub pass PrimaryConnection(), built from the same environment lookups as before, so the resulting rest.Config is identical field for field. Also exports resolver.Prober, which was unexported and therefore unnameable by the first package to build one. Left alone deliberately: the hub manager's webhook server still takes its Host from the HubEndpoint package var. It registers no webhooks, so the server never starts and the value is dead config; changing it here would mean altering behaviour in a commit whose whole point is not to. Part of kubeslice#467 Signed-off-by: Sumanth D <sumanthd032@gmail.com>
A worker pinned to one hub endpoint cannot survive that hub being promoted away from. With the controller running Active/Standby, leadership can move, and until now the only way to point a worker at the new Active was to edit its deployment. The worker now resolves which hub holds leadership before it opens any connection, and keeps watching. When the answer changes and holds across consecutive polls, it logs, counts it, and shuts down cleanly; the kubelet restarts it and startup resolution picks the hub that is now Active. Restarting rather than rebuilding in place is the deliberate choice. Both hub connections are assembled once from a rest.Config, and manager.Start already exits the process on any hub error, so a clean restart is both the smaller change and the one this process is built for. The data plane is untouched either way: gateways and tunnels run in their own pods. Everything is gated on HUB_SECONDARY_HOST_ENDPOINT. Unset, which is every deployment today, no resolver is built, no extra client is opened, and the connection is the same one the environment has always described. The resolution rule refuses claims naming any endpoint outside the two configured hubs, so a Cluster CR cannot redirect a worker somewhere nobody provisioned. Two metrics: kubeslice_worker_hub_switches_total, and kubeslice_worker_hub_probe_errors_total by hub slot. The second is the one worth alerting on — a hub that has been quietly unreachable for days is a problem to hear about before a failover rather than during one. Not implemented: issue kubeslice#467 asks for a ControllerConnected condition. ClusterStatus has no Conditions field, so that needs a further change to the shared apis module, and a hub-side condition can only be written while the hub is reachable — it can never report the state anyone wants to see. Local metrics and logs carry it instead. A durable local health surface belongs with kubeslice#469. Part of kubeslice#467 Signed-off-by: Sumanth D <sumanthd032@gmail.com>
Describes the resolution rule, the configuration, the metrics, and how to verify the whole thing in Kind. config/manager/manager.yaml gains the env, volume mount and volume for the second hub, commented out, so the shape is visible where an operator would look for it. The part worth being explicit about is the second credential. A worker authenticates with a token the hub it talks to minted, so following a failover needs a Standby-valid credential mounted before the Active fails — afterwards nothing is left to hand it one. The controller side already produces it: the Standby mirrors the worker's ServiceAccount and an empty token Secret shell, and its own token controller fills that shell in. Getting it from there onto the worker belongs to cluster registration and the charts, which live in neither repository, so it currently has no owner. The manual procedure is written out rather than left implied, including a check that the token actually authenticates before it is installed — a credential that is present but invalid is worse than none, because it fails only at failover. Part of kubeslice#467 Signed-off-by: Sumanth D <sumanthd032@gmail.com>
Startup resolution never worked at any confirmation setting above one. The caller resolves once before opening a connection, but the very first winner had to clear the same consecutive-poll threshold as a later switch — and a single call can never reach it. The worker fell back to its configured primary every time, including when the primary is the hub that just lost leadership, which is the one case startup resolution exists for. The threshold guards a *change* of hub: it stops one divergent poll re-pointing a worker that is already connected somewhere. Before anything is established there is no connection to protect and nothing to flap between, so the first winner is now taken as it stands. Later changes are unaffected. Found by running the failover demo against live clusters, where startup logged "candidate active hub not yet confirmed, seen 1 need 2" and then ignored a hub that had plainly declared itself. No unit test caught it because every existing test drove the resolver in a loop, which is exactly what startup does not do. TestResolve_RequiresConsecutiveConfirmations asserted the old behaviour in passing and has been corrected. Part of kubeslice#467 Signed-off-by: Sumanth D <sumanthd032@gmail.com>
sumanthd032
requested review from
Rahul-D78,
bharath-avesha,
gourishkb and
richiesebastian
as code owners
August 7, 2026 08:30
pnavali
requested review from
rajendra-avesha
and
a lite review from Copilot
and removed request for
bharath-avesha and
richiesebastian
August 19, 2026 12:29
There was a problem hiding this comment.
Pull request overview
Adds Active/Standby hub failover-following to the worker operator: it polls both pre-provisioned hub endpoints for status.activeController, resolves the Active hub, and on a confirmed switch shuts down cleanly so the pod restarts against the promoted hub (leaving the data plane untouched). It also fixes a config bug by passing hub endpoint+credentials together as a value object instead of relying on early-evaluated package globals.
Changes:
- Introduces
pkg/hub/resolver(+ probe) to deterministically resolve the active hub with anti-flap confirmations. - Adds
pkg/hub/failoverto wire resolution into startup selection and a watch loop that triggers a graceful restart on switch. - Refactors hub client/manager construction to accept a
hub.Connection(Endpoint/TokenFile/CAFile) and documents/configures the secondary credential.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/hub/resolver/resolver.go | Active hub resolution logic (trust boundary, deterministic tie-break, anti-flap state). |
| pkg/hub/resolver/resolver_test.go | Unit tests for resolver steady state, failover, ties, anti-flap behavior. |
| pkg/hub/resolver/probe.go | Unstructured probe for status.activeController with per-read timeout. |
| pkg/hub/resolver/probe_test.go | Unit tests for probe decoding, timeouts, and error classification. |
| pkg/hub/manager/manager.go | Hub manager now takes an explicit hub connection and builds REST config from it. |
| pkg/hub/hubclient/hubclient.go | Hub client construction now takes a Connection rather than reading env directly. |
| pkg/hub/hubclient/connection.go | Adds Connection value object + PrimaryConnection() + REST config builder. |
| pkg/hub/hubclient/connection_test.go | Tests ensuring endpoint/token/CA travel together and primary config matches legacy behavior. |
| pkg/hub/failover/failover.go | Startup resolution + polling watch loop + metrics and env-driven config for secondary hub. |
| pkg/hub/failover/failover_test.go | Tests for config parsing, credential pairing, startup selection, and watch behavior. |
| main.go | Integrates failover follower, selects hub before building clients, and cancels context to restart on switch. |
| docs/hub-failover.md | Operational documentation for configuration, behavior, and manual secondary credential delivery. |
| config/manager/manager.yaml | Adds commented-out env/volume scaffolding for secondary hub configuration/credential. |
Suppressed comments (1)
pkg/hub/manager/manager.go:82
webhook.Options.Hostis a bind host for the local webhook server; wiring it toHubEndpoint(a remote API server URL/host) can break binding and also won’t follow the selectedconn.Endpointduring failover. Prefer leaving Host unset so controller-runtime uses its default bind address.
webhookServer := webhook.NewServer(webhook.Options{
Host: HubEndpoint,
Port: 9443,
})
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+199
to
+220
| verdict := r.probe(ctx, candidate) | ||
| switch { | ||
| case !verdict.Reachable: | ||
| r.log.V(1).Info("hub candidate unreachable", "hub", candidate.Name, | ||
| "endpoint", candidate.Endpoint, "error", verdict.Err) | ||
| case verdict.Claim == nil: | ||
| // The hub answered and published nothing. This is what a non-HA | ||
| // deployment looks like from here, and it must stay silent at info | ||
| // level or every worker in every existing cluster logs a warning | ||
| // forever. | ||
| r.log.V(1).Info("hub candidate published no activeController", "hub", candidate.Name) | ||
| case !r.known(verdict.Claim.Endpoint): | ||
| // The trust boundary. The field chooses among endpoints an operator | ||
| // pre-provisioned; it does not get to point this worker at an | ||
| // address nobody configured. Anything else would make write access | ||
| // to one Cluster CR enough to redirect a worker's hub connection. | ||
| r.log.Info("ignoring activeController naming an unconfigured endpoint", | ||
| "hub", candidate.Name, "declaredEndpoint", verdict.Claim.Endpoint, | ||
| "declaredIdentity", verdict.Claim.Identity) | ||
| default: | ||
| claims = append(claims, *verdict.Claim) | ||
| } |
Comment on lines
+184
to
+190
| counted := func(ctx context.Context, c resolver.HubCandidate) resolver.Verdict { | ||
| v := probe(ctx, c) | ||
| if !v.Reachable { | ||
| hubProbeErrorsTotal.WithLabelValues(c.Name).Inc() | ||
| } | ||
| return v | ||
| } |
Comment on lines
+175
to
+180
| probe := resolver.NewProbe( | ||
| func(c resolver.HubCandidate) (resolver.ClusterReader, error) { | ||
| return readerFor(hub.Connection{Endpoint: c.Endpoint, TokenFile: c.TokenFile, CAFile: c.CAFile}) | ||
| }, | ||
| resolver.ProbeConfig{ClusterName: cfg.ClusterName, Namespace: cfg.Namespace, Timeout: cfg.Timeout}, | ||
| ) |
7 tasks
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
Fixes #467. Builds on #497 — this branch is stacked on it, so the diff below includes #497's commit until that one merges.
Wires the
pkg/hub/resolverfrom #497 in: on a confirmed hub switch, the manager exits gracefully so kubelet restarts it against the new hub, rather than reconnecting in place. Gateways/tunnels run in separate pods, so the data plane is untouched.Along the way, found and fixed a real bug:
HubTokenFile/HubCAFileare package-level vars evaluated beforemain, whileHUB_HOST_ENDPOINTis read late, insideNewHubClientConfig/manager.Start. So callingos.Setenvfrommainmoved the address but not the token — the client ended up pointed at the new hub while still authenticating as the old one. Fixed by passing ahub.Connection{Endpoint, TokenFile, CAFile}by value instead of reading the globals;PrimaryConnection()reproduces today's config exactly.Also documents the second credential's manual delivery in
docs/hub-failover.md.Part of the Active/Standby HA effort tracked in kubeslice-controller#305.
How Has This Been Tested?
-race -count=2 -shuffle=ongo build ./...,go vet ./...Checklist:
go fmtDoes this PR introduce a breaking change?