Migrate status controller to events.EventRecorder API
- Dominant language
- Go
- Stars
- 29
- Forks
- 25
- Avg merge
- 1d 11h
- Merged PRs (30d)
- 3
Description
## Background
`controller-runtime` v0.23 deprecated `manager.GetEventRecorderFor` and the legacy `k8s.io/client-go/tools/record.EventRecorder` API in favor of the newer `k8s.io/client-go/tools/events.EventRecorder`. The deprecation notice says the old API will be removed in a future release.
operatorpkg's `status.NewController` and `status.NewGenericObjectController` currently take `record.EventRecorder`, which means downstream consumers can't drop their own usage of `GetEventRecorderFor` — they're forced to suppress the `SA1019` lint at every call site.
For example, kubernetes-sigs/karpenter#2951 currently carries two `//nolint:staticcheck` directives just to keep using operatorpkg after bumping controller-runtime.
## Proposal
Migrate operatorpkg's status controller to the new `events.EventRecorder` API. Two possible approaches:
1. **Hard swap.** Change the `status.NewController` / `status.NewGenericObjectController` signatures to take `events.EventRecorder`. Breaking change for callers, but compile-time, with a one-line migration (`mgr.GetEventRecorderFor(name)` → `mgr.GetEventRecorder(name)`).
2. **Parallel constructor.** Add `NewControllerWithEventsRecorder` (or similar) alongside the existing one, and deprecate the old constructor. Slower migration, no breaking change. Matches the pattern from #190 where a downstream-impacting change was reworked into an additive split.
## Implementation surface (audited)
The change is small:
- `status/controller.go`: import swap, `eventRecorder` field type, two constructor signatures, three internal `eventRecorder.Event(...)` → `Eventf(...)` translations. Reusable action strings: `Finalize` and `TransitionCondition`.
- `status/controller_test.go`: swap `record.FakeRecorder` for `events.FakeRecorder`. The new fake produces the same ` ` channel format, so existing assertion strings stay valid unchanged.
- No other call sites in operatorpkg use `record.EventRecorder`.
A working draft of the hard-swap approach is on https://github.com/jamesmt-aws/operatorpkg/tree/chore/migrate-events-recorder (was opened as #205, closed in favor of this tracking issue so the design question isn't blocked on a specific PR shape).
## Downstream blast radius
~30 repos depend on operatorpkg, but the real blast radius for a `status.NewController` signature change is roughly:
- ~10 karpenter cloud-provider repos that bump operatorpkg via karpenter's `go.sum` and would migrate in lockstep
- ~5 standalone consumers (kaito, eks-node-monitoring-agent, tensor-fusion, gpu-provisioner, grit) that may or may not actually call `status.NewController` directly
A compile-time signature break is loud and easy to fix at every call site, which makes the hard swap defensible. Happy to defer to maintainer preference on which approach to take.
## Downstream cleanup
Once this lands, the karpenter follow-up at https://github.com/jamesmt-aws/karpenter/tree/chore/migrate-event-recorder-api can be opened to drop the two nolints.
Contributor guide
Assessment
This issue has not been assessed yet.