Skip to content

[architect] tech-debt: extract pkg/api/gpu_utilization_worker.go (413 LOC background worker) to its own package #23735

Description

@hivecommons-hive

Architecture Finding

Type: tech-debt / package boundary (background worker in HTTP-wiring package)
Affected area: pkg/api/gpu_utilization_worker.go (413 LOC) + pkg/api/gpu_utilization_worker_test.go (484 LOC)

pkg/api/ is the HTTP surface package — routing, middleware, server startup, and route-group wiring. Everything else in it names its role (server.go, routes_*.go, route_group_*.go, middleware_setup.go, startup.go, buildinfo.go, config.go). The single outlier is gpu_utilization_worker.go, a self-contained periodic background worker with its own goroutine, Start()/Stop(), DCGM scraping, snapshot retention/cleanup, and threshold-alert emission.

Concrete evidence that it does not belong in pkg/api:

  • Zero HTTP surface. The file exposes no http.Handler, no route registration, and no middleware — only NewGPUUtilizationWorker, Start, Stop, collectUtilization, scrapeDCGMPerCluster, collectForReservation, and cleanupOldSnapshots (7 functions total).
  • Only two intra-package callers. pkg/api/server.go:272 constructs it (server.background.gpuUtilWorker = NewGPUUtilizationWorker(...)) and pkg/api/server_runtime.go:39 types the field. No other file in pkg/api touches it.
  • Its own dependency graph is broader than pkg/api's. It imports pkg/gpu, pkg/k8s, pkg/models, pkg/notifications, pkg/store, pkg/safego, and pkg/api/metrics. Of those, only pkg/api/metrics is in-package; the other six are cross-cutting domains a worker naturally reaches, but a routing file should not.
  • Pattern precedent already exists in this repo. The identical shape in pkg/agent was resolved by extraction: PredictionWorker, InsightWorker, DeviceTracker, and MetricsHistory were carved out of pkg/agent/*.go into pkg/agent/workers/ ([architect] refactor: extract background workers to pkg/agent/workers #18271, bead 0075a94d-1ec). The GPU worker is the mirror-image case in pkg/api, with the same size class (413 LOC production + 484 LOC tests, comparable to workers.PredictionWorker).
  • Header comment on the sibling pkg/gpu/scraper.go explicitly warns about direction: "extracted from pkg/agent to break the pkg/api → pkg/agent import dependency ([architect] pkg/api directly imports pkg/agent — violates binary independence, extract shared utilities #17131, [architect] Extract DCGM types from pkg/agent to pkg/gpu #17640)". Keeping a worker under pkg/api risks re-introducing the same directional coupling the prior extraction was designed to prevent.

Impact

  • Test-package confusion. Any change to pkg/api/server.go or route wiring re-triggers pkg/api/gpu_utilization_worker_test.go (484 LOC, DCGM scrape mocks and 90-day retention scenarios) even when nothing about the worker changed, because Go's test unit is the package.
  • Blast-radius for pkg/api compile errors. Any breakage in the worker's dependency chain (pkg/gpu, pkg/notifications, pkg/store) breaks the whole HTTP package's build, blocking route-wiring iteration.
  • Discovery friction. New contributors looking for periodic-worker patterns grep pkg/agent/workers/ and pkg/gpu/, not pkg/api/. The precedent set by the prior extraction is silently violated by this one file.
  • pkg/api/metrics back-reference is the only blocker to a cleaner move to pkg/gpu/. If the metrics counter emission is dependency-injected (WorkerMetrics interface with RecordUtilization, RecordAlert methods, satisfied by pkg/api/metrics), the worker can live in pkg/gpu alongside the scraper it wraps. Without that injection, it stays trapped in pkg/api.

Recommendation

Two-step extraction, either step landable independently:

Step A — cheap move, no cycle risk (recommended first):

  1. Create pkg/api/gpuworker/ package.
  2. Move pkg/api/gpu_utilization_worker.go → pkg/api/gpuworker/worker.go, renaming the exported types from GPUUtilizationWorker/NewGPUUtilizationWorker to gpuworker.Worker/gpuworker.New.
  3. Move pkg/api/gpu_utilization_worker_test.go → pkg/api/gpuworker/worker_test.go (adjust package and any unexported-symbol accesses).
  4. Update pkg/api/server.go:272 and pkg/api/server_runtime.go:39 to use the new package name.
  5. This mirrors the pkg/agent/workers extraction (#18271) exactly and does not touch dependency direction.

Step B — cross-package move (do only after Step A stabilises):

  1. Introduce a Metrics interface in pkg/api/gpuworker covering the two consolemetrics calls the worker makes (grep consolemetrics\. inside the file to enumerate).
  2. Move pkg/api/gpuworker/ → pkg/gpu/utilworker/ with the interface constructor-injected from pkg/api/server.go (which retains the concrete consolemetrics import). This preserves the pkg/api → pkg/gpu direction the pkg/gpu/scraper.go header comment mandates.

No workflow (.github/workflows/) changes required for either step — both stay inside pkg/.

Related historical extractions (for reviewer context):

  • #18271 / bead 0075a94d-1ec: pkg/agent/workers (PredictionWorker, InsightWorker, DeviceTracker, MetricsHistory)
  • #18310 / bead 924f0d4a-91e: pkg/agent/tokentracker
  • #18125 / bead 0f137940-e25: pkg/agent/updater
  • #17131/#17640: pkg/gpu split from pkg/agent to break pkg/api → pkg/agent direction

Filed by architect agent (ACMM L6 — full mode)

🐝 Hive Agent: architect | Instance: hosted-kubestellar-console-4vkt | SHA: unknown

— hive: agent=architect backend=copilot model=claude-opus-4.7 copilot=1.0.88

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions