test(hub): cover #468's failover robustness scenarios - #501
Open
sumanthd032 wants to merge 11 commits into
Open
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 25, 2026 16:42
7 tasks
Pulls in ActiveController/StorageCapabilities (already merged locally, never vendored) and the new Conditions field kubeslice#469 needs. Pinned to the fork commit behind kubeslice/apis#47; drop the replace once that merges and a release carries the field. Signed-off-by: Sumanth D <sumanthd032@gmail.com>
…icationFailed Distinguishes a TLS/cert-handshake failure from everything else, for issue kubeslice#469's ControllerConnected reason table. Signed-off-by: Sumanth D <sumanthd032@gmail.com>
… worker's Cluster CR Adds the two conditions, four events and two metrics from issue kubeslice#469, wired through main.go/manager.Start/the cluster reconciler. Only ever writes Connected, ReconnectedAfterFailover or EndpointNotConfigured live; DialFailed/CertVerificationFailed are unit-tested but architecturally can't persist on a hub this worker can't reach. Signed-off-by: Sumanth D <sumanthd032@gmail.com>
…metrics Signed-off-by: Sumanth D <sumanthd032@gmail.com>
…t handoff Fills the reconciler-level test kubeslice#469 asked for but the prior commit skipped: non-HA, the one-shot ReconnectedAfterFailover-then-Connected handoff, LastTransitionTime idempotency, and that a live failure reason never gets set. Signed-off-by: Sumanth D <sumanthd032@gmail.com>
Reconnect-signal correctness in both directions, cert-verification-failure classification and safe fallback, a static check that the failover package never touches gateway/tunnel code, and non-HA precedence over a stray Reconnected flag. Signed-off-by: Sumanth D <sumanthd032@gmail.com>
sumanthd032
force-pushed
the
test/468-failover-robustness-tests
branch
from
August 26, 2026 05:15
238302d to
aad9009
Compare
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
1 participant
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
Covers issue #468's four scenarios at whatever level each is actually testable: reconnect-signal correctness in both directions, cert-verification-failure classification with a safe fallback, a static check that the failover-following package never imports gateway/tunnel code, and non-HA precedence over a stray Reconnected flag.
Stacked on #500. Only 1 commit is new here.
Fixes #468
How Has This Been Tested?
go test -race -count=1 ./pkg/hub/...clean, including the envtest/ginkgo suites. Live cross-cloud tunnel continuity was not re-verified here: it is separately blocked by an unrelated worker-CRD-skew defect, unrelated to this change, documented in docs/hub-failover.md.Checklist:
Does this PR introduce a breaking change for other components like kubeslice-controller?
No. Test-only change.