awslabs / awslabs/operatorpkg

Migrate status controller to events.EventRecorder API

Open
#206 0 comments 0 reactions 0 assignees View on GitHub
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

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.