Skip to content

(feat): EventRecorder implementation#37

Merged
sandeepkunusoth merged 6 commits into
valkey-io:mainfrom
sandeepkunusoth:main
Jan 9, 2026
Merged

(feat): EventRecorder implementation#37
sandeepkunusoth merged 6 commits into
valkey-io:mainfrom
sandeepkunusoth:main

Conversation

@sandeepkunusoth
Copy link
Copy Markdown
Member

@sandeepkunusoth sandeepkunusoth commented Dec 27, 2025

This PR Implements #27 Kubernetes Event recording for the ValkeyCluster controller to improve operator visibility and debugging. Adds an EventRecorder to the reconciler and wires it via the manager, updates RBAC to allow creating/patching Events, emits Normal/Warning events at key reconciliation points (e.g., node add/join failures/success), and documents event semantics in docs/status-conditions.md.

Added e2e tests for some events. facing issues with validation for all events as events are rate limited kubernetes/kubernetes#136061 (comment)

Signed-off-by: Sandeep Kunusoth <sandeepkunsoth000@gmail.com>
@sandeepkunusoth sandeepkunusoth marked this pull request as ready for review January 7, 2026 21:51
@sandeepkunusoth sandeepkunusoth requested a review from bjosv January 8, 2026 05:31
Copy link
Copy Markdown
Collaborator

@jdheyburn jdheyburn left a comment

Choose a reason for hiding this comment

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

LGTM, thank you!

@sandeepkunusoth sandeepkunusoth merged commit a9f3a0c into valkey-io:main Jan 9, 2026
4 checks passed
Copy link
Copy Markdown
Collaborator

@bjosv bjosv left a comment

Choose a reason for hiding this comment

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

Looks good, added one comment.

return err
}
} else {
return err
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

We have a ConfigMapCreationFailed, but we don't have a ServiceCreationFailed and emit it here.
Should we have one, or is this event implicit in some other way?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Hum, or maybe we have too many events already..
https://book.kubebuilder.io/reference/raising-events#events-should-be-raised-in-certain-circumstances-only

When testing on minikube all events are noted, but on kind many are lost/rate-limited, and I guess that why the test fails intermittently. The status conditions is a better indicator for a healthy cluster.

I guess we need to investigate what level of detail we really need here.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

yeah, i will add new issue for this. we need to agree on what all events should be removed.

hieu2102 pushed a commit to hieu2102/valkey-operator that referenced this pull request Jan 15, 2026
This PR Implements
valkey-io#27 Kubernetes Event
recording for the ValkeyCluster controller to improve operator
visibility and debugging. Adds an EventRecorder to the reconciler and
wires it via the manager, updates RBAC to allow creating/patching
Events, emits Normal/Warning events at key reconciliation points (e.g.,
node add/join failures/success), and documents event semantics in
docs/status-conditions.md.

Added e2e tests for some events. facing issues with validation for all
events as events are rate limited
kubernetes/kubernetes#136061 (comment)

---------

Signed-off-by: Sandeep Kunusoth <sandeepkunsoth000@gmail.com>
hieu2102 pushed a commit to hieu2102/valkey-operator that referenced this pull request Jan 15, 2026
This PR Implements
valkey-io#27 Kubernetes Event
recording for the ValkeyCluster controller to improve operator
visibility and debugging. Adds an EventRecorder to the reconciler and
wires it via the manager, updates RBAC to allow creating/patching
Events, emits Normal/Warning events at key reconciliation points (e.g.,
node add/join failures/success), and documents event semantics in
docs/status-conditions.md.

Added e2e tests for some events. facing issues with validation for all
events as events are rate limited
kubernetes/kubernetes#136061 (comment)

---------

Signed-off-by: Sandeep Kunusoth <31273507+sandeepkunusoth@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants