feat(hub): add the active-hub resolver (#467) - #497
Open
sumanthd032 wants to merge 1 commit 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>
sumanthd032
requested review from
Rahul-D78,
bharath-avesha,
gourishkb and
richiesebastian
as code owners
August 7, 2026 08:29
13 tasks
pnavali
requested review from
rajendra-avesha
and
a lite review from Copilot
and removed request for
richiesebastian
August 19, 2026 12:30
There was a problem hiding this comment.
Pull request overview
Adds a new pkg/hub/resolver package to help the worker operator determine which of two pre-provisioned hub endpoints is currently Active during Active/Standby failover, by probing both hubs’ copies of the worker’s Cluster CR and applying a deterministic, anti-flap decision rule.
Changes:
- Introduces
Resolverdecision logic (trust-boundary enforcement + deterministic tie-break + consecutive-confirmation switching). - Adds a
Probeimplementation that readsstatus.activeControllerunstructured with per-read timeouts. - Adds unit tests covering resolver and probe behavior (steady state, failover, disagreement, timeouts, malformed/missing data).
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| pkg/hub/resolver/resolver.go | Core active-hub selection logic with anti-flap confirmation and trust-boundary checks. |
| pkg/hub/resolver/probe.go | Probe implementation that reads status.activeController from hubs via unstructured GET with bounded timeouts. |
| pkg/hub/resolver/resolver_test.go | Unit tests for resolver decision logic, determinism, and anti-flap behavior. |
| pkg/hub/resolver/probe_test.go | Unit tests for probe decoding, timeout behavior, and error classification. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+107
to
+116
| // Options configures a Resolver. Zero-valued fields fall back to the Default* | ||
| // constants. | ||
| type Options struct { | ||
| // ProbeTimeout bounds each individual candidate read. | ||
| ProbeTimeout time.Duration | ||
| // SwitchConfirmations is how many consecutive agreeing polls are required | ||
| // before a change of winner is reported. | ||
| SwitchConfirmations int | ||
| Log logr.Logger | ||
| } |
Comment on lines
+176
to
+182
| // Resolve probes every candidate once and returns the hub the worker should be | ||
| // talking to, or nil to mean "change nothing". | ||
| // | ||
| // Returning nil is a real answer, not an error: with no usable claim the | ||
| // correct behaviour is to leave the existing connection alone. A worker that | ||
| // disconnected whenever it was unsure would turn every hub blip into a worker | ||
| // outage, which is strictly worse than talking to a hub that might be stale. |
Comment on lines
+200
to
+210
| 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): |
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
Towards #467.
Adds
pkg/hub/resolver: during an Active/Standby controller failover, resolves which hub is Active by readingClusterStatus.ActiveControllerunstructured (avoids a hard dependency on the unreleasedkubeslice/apis#46field before it merges).Resolution rule: unreachable hub gets no say, a missing field gets no say (non-HA path), a claim naming an endpoint outside the configured candidates is rejected (trust boundary), and switching requires N consecutive agreeing polls (anti-flap) before it's accepted.
Part of the Active/Standby HA effort tracked in kubeslice-controller#305.
How Has This Been Tested?
pkg/hub/resolver(21 cases), run with-race -count=2 -shuffle=onChecklist:
go fmtDoes this PR introduce a breaking change?