Skip to content

Migrate event recorder to fluxcd/pkg/runtime/events.Recorder - #1621

Open
adri1197 wants to merge 1 commit into
fluxcd:mainfrom
adri1197:event-recorder
Open

Migrate event recorder to fluxcd/pkg/runtime/events.Recorder#1621
adri1197 wants to merge 1 commit into
fluxcd:mainfrom
adri1197:event-recorder

Conversation

@adri1197

@adri1197 adri1197 commented Mar 14, 2026 •
edited
Loading

Copy link
Copy Markdown

Migrate the controller's event recorder to fluxcd/pkg/runtime/events.Recorder and K8s event API from v1beta1 to v1.

  • Replace kuberecorder.EventRecorder with events.Recorder to support structured event metadata (source object reference and action type).
  • Add an action to every event call (Reconciled, Failed, Waiting, Applied, Deleted, Progressing) and thread the source reference through reconcile, apply, prune, checkHealth, and finalize.
  • Bump fluxcd/pkg/runtime to v0.113.0.

Part of - fluxcd/flux2#5761

@adri1197 adri1197 mentioned this pull request Mar 14, 2026
Open
15 tasks
@matheuscscp

matheuscscp commented Sep 18, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

Hey @adri1197 please rebase all your PRs, we have upgraded to k8s 1.37 across all controllers and bumped most fluxcd/pkg packages. The only remaining dep for you to bump across your PRs is fluxcd/pkg/runtime. Please start with kustomize-controller as there's another PR depending on the runtime bump 🙏

@adri1197
adri1197 marked this pull request as ready for review September 18, 2026 13:33
matheuscscp
matheuscscp approved these changes Sep 18, 2026
View reviewed changes

@matheuscscp matheuscscp left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! 🚀

Thanks @adri1197!

@stefanprodan

stefanprodan commented Sep 21, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

Make the success event message stable across reconciliations of the same revision (drop the duration from the message; log it instead) and stamp each event with a unique token annotation, so Kubernetes can deduplicate

This is not true, deduplication will never happen now because ResourceVersion will always differ (due to status patching) and even if we mutate and set that to nil, the new client-go broadcaster hardcodes 6m for aggregation, basically this PR reverts #1743

stefanprodan
stefanprodan reviewed Sep 21, 2026
View reviewed changes
Comment thread internal/controller/kustomization_controller.go Outdated
stefanprodan
stefanprodan reviewed Sep 21, 2026
View reviewed changes
Comment thread internal/controller/kustomization_controller.go Outdated
"revision", obj.Status.LastAttemptedRevision,
"duration", time.Since(reconcileStart).String())
r.event(obj, obj.Status.LastAppliedRevision, obj.Status.LastAppliedOriginRevision, eventv1.EventSeverityInfo, msg,
r.event(obj, nil, obj.Status.LastAppliedRevision, obj.Status.LastAppliedOriginRevision, eventv1.EventSeverityInfo,

@stefanprodan stefanprodan Sep 21, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why we do not set the source on a success reconcile event?

@adri1197 adri1197 Sep 21, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done! 😃

@matheuscscp matheuscscp mentioned this pull request Sep 21, 2026
Closed
@matheuscscp

matheuscscp commented Sep 21, 2026

Copy link
Copy Markdown
Member

We will do what's described here now: #1742 (comment)

Please rebase after that! 🙏

@matheuscscp

matheuscscp commented Sep 21, 2026

Copy link
Copy Markdown
Member

@adri1197 All done, please rebase! 🙏

@adri1197
adri1197 force-pushed the event-recorder branch 2 times, most recently from dcd41bb to 34ff9e9 Compare September 21, 2026 15:03
matheuscscp
matheuscscp previously approved these changes Sep 21, 2026
View reviewed changes

@matheuscscp matheuscscp left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! 🚀

Thanks @adri1197!

@adri1197
Migrate event recorder to fluxcd/pkg/runtime/events.Recorder
92400d2 
- Replace kuberecorder.EventRecorder with events.Recorder to support
  structured event metadata including source object and action type
- Upgrade event API from event/v1beta1 to event/v1
- Add action parameter (Reconciled, Failed, Waiting, Applied, Deleted,
  Progressing) to all event calls for richer event semantics
- Pass source reference through reconcile, apply, prune, checkHealth,
  and finalize methods
- Update tests to use events.Recorder

Signed-off-by: Adrian Fernandez De La Torre <adri1197@gmail.com>
@stefanprodan

stefanprodan commented Sep 22, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

Found some major blockers:

@stefanprodan

stefanprodan commented Sep 23, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

Given the 1024 limit of the event message, we can not ship this as is, all of Flux observability is built on the assumption that the events cary the desired state changes and errors. With 1024 limit, KC can no longer issue events with the list of applied changes, dry-run errors will no longer be surfaced to users, HC can no longer report helm upgrade errors, etc.

@stefanprodan stefanprodan added the hold Issues and pull requests put on hold label Sep 23, 2026
@matheuscscp
matheuscscp self-requested a review September 23, 2026 07:56
@matheuscscp
matheuscscp dismissed their stale review September 23, 2026 07:56

Found major blockers

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

Reviewers

@stefanprodan stefanprodan Awaiting requested review from stefanprodan
@matheuscscp matheuscscp Awaiting requested review from matheuscscp

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

hold Issues and pull requests put on hold

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@adri1197 @matheuscscp @stefanprodan