Skip to content

feat(ha): publish status.activeController so workers can find the Active hub (#297 PR1) - #3

Open
sumanthd032 wants to merge 4 commits into
feat/294-ha-leader-electionfrom
feat/297-active-controller
Open

feat(ha): publish status.activeController so workers can find the Active hub (#297 PR1)#3
sumanthd032 wants to merge 4 commits into
feat/294-ha-leader-electionfrom
feat/297-active-controller

Conversation

@sumanthd032

@sumanthd032 sumanthd032 commented Jul 29, 2026

Copy link
Copy Markdown
Owner

Description

Adds status.activeController to the Cluster CRD and a publisher that keeps it current on whichever hub currently holds leadership, so a worker can identify the Active hub after a failover.

Per ADR kubeslice#293 Decision 7 each hub writes this field about itself, locally. A Standby's copy is written by the kubeslice#295 mirror and therefore names the Active, which is what lets a worker watching both hubs pick the right one without knowing either hub's role.

This diverges from the issue's step 7, which has the promoted hub push its endpoint into every worker cluster. That needs write credentials from the hub into each worker and reverses the direction of trust used elsewhere, and the fields it names do not exist in this repo. The ADR settled it the other way.

The publisher runs continuously rather than only at promotion, otherwise a worker cannot identify the Active before the first failover. It is not started in standalone mode, so a non-HA deployment leaves the field absent. It refuses to publish the shipped placeholder endpoint.

The same types are being added to github.com/kubeslice/apis (kubeslice/apis#46), which is what worker-operator imports.

Part of kubeslice#297

How Has This Been Tested?

  • 35 unit tests, go test -race ./pkg/ha/... clean.
  • Live on a 3-cluster Kind setup against a real Cluster CR: standalone left the field absent, active published in 2s with the correct values, the placeholder endpoint was refused, a Standby started the publisher but never wrote its own identity, and resourceVersion was unchanged across a full publish interval.
  • go vet ./service/... fails on a pre-existing undefined: util.Client in a test file on the base branch, unrelated to this change.

Checklist:

  • The title of the PR states what changed and the related issues number (used for the release note).
  • Does this PR requires documentation updates?
  • I've updated documentation as required by this PR.
  • I have performed a self-review of my own code.
  • I have commented my code, particularly in hard-to-understand areas.
  • I have tested it for all user roles.
  • I have added all the required unit test cases.

Does this PR introduce a breaking change for other components like kubeslice-controller, worker-operator?

No. The field is additive and omitempty, and the publisher is not started in standalone mode.


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>
Copilot AI review requested due to automatic review settings July 29, 2026 11:10

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants