[architect] refactor: extract explicit poller lifecycle from registerSaaSRoutes (hub testing.Testing() removal) - #5563
Conversation
…SaaSRoutes Production code in pkg/hub branched on testing.Testing() in two places: registerSaaSRoutes spawned four long-lived pollers behind !testing.Testing(), and StartAdvisoryDiagnostics carried a second redundant guard. That coupled the production binary to the testing package and hid a lifecycle decision inside route registration. - New HubServer.StartBackgroundPollers(ctx) (pkg/hub/background_pollers.go) owns poller startup; pollers now share a caller-owned context instead of four detached context.Background() calls. - registerSaaSRoutes is now side-effect free; the testing import is gone from saas.go and advisory_diagnostics.go. - cmd/hive runHub (the composition root) calls StartBackgroundPollers once before serving, so production behavior is unchanged. - TestStartAdvisoryDiagnosticsNoopInTests (which asserted the anti-pattern) is replaced by TestStartAdvisoryDiagnosticsStopsOnCancel, asserting the new contract: pollers stop when the root context is cancelled. No behavior change in production; tests get no background goroutines, as before, but now by construction rather than by test-mode detection. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: architect <architect@hive>
kubestellar-prow
Bot
added
dco-signoff: yes
|
Changelog: this PR changes code but does not touch If it is user-visible — a feature, a fix an operator would notice, a This is a reminder, not a gate; it never blocks a merge. |
clubanderson
added
approved
|
[APPROVALNOTIFIER] This PR is APPROVED Approval requirements bypassed by manually added approval. This pull-request has been approved by: The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
Thank you for your contribution! Your PR has been merged. Check out what's new:
Stay connected: Slack #kubestellar-dev | Multi-Cluster Survey |
1 participant
Refactor
Cluster: pkg/hub poller lifecycle / test-mode coupling —
src/pkg/hub/saas.go(registerSaaSRoutes),src/pkg/hub/advisory_diagnostics.go(StartAdvisoryDiagnostics), newsrc/pkg/hub/background_pollers.go,src/cmd/hive/main.go(runHubonly),src/pkg/hub/reach_diag_wiring_test.go. Disjoint from PR #4032 (proxy auth injection: pkg/proxy, pkg/github token source, pkg/config/proxy_inject,func mainhunk at main.go:2750).Production code in
pkg/hubimportedtestingand branched ontesting.Testing():registerSaaSRoutesspawned four long-lived pollers (startProvisionWatcher,StartLatestSHAPoller,StartAuthAudit,StartAdvisoryDiagnostics) behindif !testing.Testing(), hiding a lifecycle decision inside route registration and using four detachedcontext.Background()calls that nothing could stop.StartAdvisoryDiagnosticscarried a second, redundant guard;TestStartAdvisoryDiagnosticsNoopInTestsasserted the anti-pattern.What changes structurally
HubServer.StartBackgroundPollers(ctx)owns poller startup; all four pollers share one caller-owned context.registerSaaSRoutesis now side-effect free; bothtestingimports are gone from production files.runHubincmd/hive/main.go) callsStartBackgroundPollersonce before serving — production behavior unchanged.TestStartAdvisoryDiagnosticsStopsOnCancelreplaces the noop-assert test, asserting the new contract (pollers stop on context cancellation).Tests still get zero background goroutines — now by construction, not test-mode detection.
Validation
go vet ./pkg/hub ./cmd/hivecleango test ./pkg/hub -count=1green (141s)Fixes #5562
Filed by architect agent (ACMM L5 — hold-gated mode). Hold-gated: human review required.
— hive: agent=architect backend=copilot model=claude-opus-4-6