Repository navigation
Conversation
…namespace - aggregating onto an event with an explicit name also updates type, reason and action, and rejects a change of them when the event carries an eventTime, which the API server treats as immutable - supplied names longer than the Kubernetes limit are shortened with a hash of the full name instead of plainly truncated, so distinct names stay distinct - events about cluster scoped resources in an overridden namespace carry an eventTime (and an action), as the API server refuses them otherwise
openshift-ci
Bot
added
the
do-not-merge/work-in-progress
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the
 |
csviri
marked this pull request as ready for review
October 30, 2026 13:40
openshift-ci
Bot
removed
the
do-not-merge/work-in-progress
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Event handling can drop records that differ only by action or retain stale actions, and namespace-override behavior lacks core-API integration coverage.
3 open issues
What changed in this PR
This PR extracts event-recorder correctness fixes from the virtual-thread work in #3664, improving how the SDK names, aggregates, and namespaces Kubernetes events.
Changes:
- Updates type, reason, and supplied action during aggregation, with checks for immutable fields.
- Hashes oversized supplied names to keep names with shared prefixes distinct.
- Adds
eventTimeand a default action for cluster-scoped events in overridden namespaces, with regression coverage.
| File | Description |
|---|---|
operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/event/EventRecorderRegressionTest.java |
Adds regression tests for naming, aggregation, and namespace overrides. |
operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/event/DefaultEventRecorderTest.java |
Updates shortened-name expectations and tests shared-prefix names. |
operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/event/DefaultEventSink.java |
Updates aggregated fields and rejects immutable-field conflicts. |
operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/event/DefaultEventRecorder.java |
Adds hash-based name shortening and conditional event timestamps and actions. |
Review effort: Balanced
Comment on lines
+269
to
+273
| if (requiresEventTime(regarding, namespace)) { | ||
| builder.withEventTime(new MicroTime(EVENT_TIME_FORMAT.format(Instant.now()))); | ||
| if (record.action().isEmpty()) { | ||
| builder.withAction(record.reason()); | ||
| } |
Comment on lines
+64
to
+65
| if (existing.getEventTime() != null && differsInImmutableFields(existing, event)) { | ||
| throw new IllegalStateException( |
Comment on lines
+78
to
+80
| if (event.getAction() != null) { | ||
| builder.withAction(event.getAction()); | ||
| } |
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Correctness fixes of the event recorder, found while reviewing the virtual thread support PR (#3664) and extracted from it.
1. Aggregating onto a named event updates its type, reason and action
Events with the same explicit name are aggregated onto the recorded one, but only the count, the last timestamp and the message were updated, so a
Warningrepeated under the name of aNormalevent kept theNormaltype and reason. The type, reason and action are updated too now.The API server treats them as immutable for an event that carries an
eventTime. Recording an event that differs in them onto such an event is therefore rejected with anIllegalStateExceptionexplaining that distinct event names have to be used, instead of failing on the patch with an API server error.2. Long supplied names stay distinct
A supplied event name longer than the Kubernetes limit was plainly truncated, so distinct names sharing a long prefix were merged into the same event. They are now shortened with a hash of the full name, as the names the recorder generates itself already are.
3. Events about cluster scoped resources in an overridden namespace are accepted by the API server
The API server only accepts an event without
eventTimeabout a cluster scoped object if it lives in thedefaultnamespace. Events about cluster scoped resources in an overridden namespace now carry aneventTime(in the fixed widthMicroTimeformat the API server requires) and anaction, which defaults to the reason. Events in thedefaultnamespace are unchanged.Tests
EventRecorderRegressionTest:defaultnamespaceeventTimerejects changed immutable fieldsDefaultEventRecorderTestis adapted to the shortened names.