refactor: extract pkg/api/gpu_utilization_worker.go to pkg/api/gpuworker [architect] - #23736
Conversation
…g/api/gpuworker Move the 413 LOC GPU utilization background worker (Start/Stop/collect cycle, DCGM scrape, snapshot retention, threshold alerting) and its 484 LOC test suite out of pkg/api's HTTP-routing package into a dedicated pkg/api/gpuworker subpackage. Rename the exported constructor and type from NewGPUUtilizationWorker/GPUUtilizationWorker to gpuworker.New / gpuworker.Worker to match Go naming conventions in the new namespace. Only two intra-package callers (pkg/api/server.go and server_runtime.go) needed updating. This mirrors the pattern set by the pkg/agent/workers extraction (#18271: PredictionWorker/InsightWorker/DeviceTracker/MetricsHistory) and keeps pkg/api focused on HTTP wiring. No behaviour change: the worker's constructor signature and public methods (Start/Stop) are preserved verbatim. A companion pkg/api/agent_init_test.go blank-imports pkg/agent to preserve a hidden test-scoped side effect: the moved worker_test.go transitively pulled pkg/agent into pkg/api's test binary, and its init() is what assigns ai.SetClusterContextProviders, called unconditionally by NewServer. The underlying anti-pattern (production code depending on init() from a package it never imports) is documented in the follow-up on #23735. Refs #23735 (Step A of the two-step extraction; Step B — move to pkg/gpu with a Metrics interface — remains open). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: hive-bot <bot@hive>
kubestellar-prow
Bot
added
the
dco-signoff: yes
✅ Deploy Preview for kubestellarconsole canceled.
|
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
👋 Hey @hivecommons-hive[bot] — thanks for opening this PR!
This is an automated message. |
kubestellar-prow
Bot
added
the
size/M
|
Thank you for your contribution! Your PR has been merged. Check out what's new:
Stay connected: Slack #kubestellar-dev | Multi-Cluster Survey |
|
Post-merge build verification passed ✅ Both Go and frontend builds compiled successfully against merge commit |
✅ Post-Merge Verification: passedCommit: |
Refactor
Extract
pkg/api/gpu_utilization_worker.go(413 LOC production + 484 LOC tests) — a self-contained periodic background worker — out ofpkg/api, whose surface everywhere else is HTTP routing/wiring, into a dedicatedpkg/api/gpuworker/subpackage. RenameGPUUtilizationWorker→gpuworker.WorkerandNewGPUUtilizationWorker→gpuworker.Newto match Go's package-qualified naming conventions.Only two intra-package callers exist and are updated in-place:
pkg/api/server.go:272— construction site insideNewServer.pkg/api/server_runtime.go:39— thebackground.gpuUtilWorkerfield type.Why
The extraction mirrors an already-established pattern in this repo.
pkg/agent's background workers (PredictionWorker,InsightWorker,DeviceTracker,MetricsHistory) were moved intopkg/agent/workers/in #18271 for the same reason: workers with their own dependency graph do not belong in the HTTP-wiring package. The GPU utilization worker is the last remainingpkg/api-embedded worker of that class. The siblingpkg/gpu/scraper.goheader comment ("extracted from pkg/agent to break the pkg/api → pkg/agent import dependency (#17131, #17640)") shows how sensitive this codebase is to directional coupling — keeping a worker with 6 cross-cutting imports insidepkg/apiinvites the same problem.Hidden test-scoped init discovered during extraction
The pre-extraction
pkg/api/gpu_utilization_worker_test.goimportedgithub.com/kubestellar/console/pkg/agent, and that transitive import is what pulledpkg/agent'sinit()intopkg/api's test binary.pkg/agent/init.gois the only assigner ofai.SetClusterContextProviders, whichNewServer(server.go, line 228 post-refactor) calls unconditionally. Moving the test file out ofpkg/apitherefore causedTestNewServer_ExplicitDevModeStartsWithoutOAuth(and any otherNewServer-based test) to nil-deref an unassignedSetClusterContextProvidersvariable.To preserve the exact test-binary side effect without behaviour drift, this PR adds a two-line
pkg/api/agent_init_test.gowith_ "github.com/kubestellar/console/pkg/agent", with a comment explaining the coupling and pointing at #23735. The underlying production anti-pattern — real callers depending on init() from a package they never import — is not fixed here; it is called out on the tracking issue so a follow-up can wire the var explicitly (constructor injection, or an explicitpkg/agent.Init()call fromcmd/console/main.goplus a matching setup inNewServer's test seam).Validation
go build ./...✅go vet ./...✅ (no warnings)go test ./pkg/api/ ./pkg/api/gpuworker/ ./pkg/api/metrics/✅ok github.com/kubestellar/console/pkg/api 2.069sok github.com/kubestellar/console/pkg/api/gpuworker 0.117sok github.com/kubestellar/console/pkg/api/metrics 0.032sNo workflow (
.github/workflows/) files touched — this PR is safely within the architect agent's push scope.Refs #23735 (needs-human: Step B — moving
pkg/api/gpuworkerfurther topkg/gpuwith aMetricsinterface — remains open; also the underlyingai.SetClusterContextProvidersinit-time coupling deserves its own follow-up).Filed by architect agent (ACMM L6 — full mode)
— hive: agent=architect backend=copilot model=claude-opus-4.7 copilot=1.0.88