fix: event filtering edge case with no-op updates - extended#3486
fix: event filtering edge case with no-op updates - extended#3486csviri wants to merge 12 commits into
Conversation
Signed-off-by: Attila Mészáros <a_meszaros@apple.com>
Signed-off-by: Attila Mészáros <a_meszaros@apple.com>
Signed-off-by: Attila Mészáros <a_meszaros@apple.com>
Signed-off-by: Attila Mészáros <a_meszaros@apple.com>
Signed-off-by: Attila Mészáros <a_meszaros@apple.com>
There was a problem hiding this comment.
Pull request overview
This PR extends the prior fix for read-cache-after-write event filtering edge cases by refining the API so callers can explicitly control whether an update should (a) open an event-filtering window, (b) only cache, or (c) filter only when optimistic locking is in use. It also expands coverage with a new integration test reproducing a concurrent spec-change-during-status-patch race.
Changes:
- Introduces
ResourceOperations.Options/Modeto control filtering vs cache-only behavior, and threads those options through multipleResourceOperationsentrypoints. - Updates event-source behavior to bypass filtering when
resourceVersionis absent unless explicitly forced, and adds unit tests for that behavior. - Adds a new IT reproducing the spec-change-during-status-patch race; updates multiple existing tests to use the new forced-filtering option where appropriate.
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/readcacheafterwrite/specchangeduringstatuspatch/SpecChangeDuringStatusPatchStatus.java | Adds status POJO for new race-condition integration test. |
| operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/readcacheafterwrite/specchangeduringstatuspatch/SpecChangeDuringStatusPatchSpec.java | Adds spec POJO for new race-condition integration test. |
| operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/readcacheafterwrite/specchangeduringstatuspatch/SpecChangeDuringStatusPatchReconciler.java | Implements a reconciler that holds an update window open to reproduce the race. |
| operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/readcacheafterwrite/specchangeduringstatuspatch/SpecChangeDuringStatusPatchIT.java | New IT validating spec updates aren’t lost during a concurrent status patch. |
| operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/readcacheafterwrite/specchangeduringstatuspatch/SpecChangeDuringStatusPatchCustomResource.java | New CR type used by the IT. |
| operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/readcacheafterwrite/ownsecondaryupdate/OwnSecondaryUpdateReconciler.java | Updates test reconciler to force filtering with the new options API. |
| operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/readcacheafterwrite/onrelistfilter/OnRelistFilterReconciler.java | Updates test reconciler to force filtering / adapt to new signatures. |
| operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/changenamespace/ChangeNamespaceTestReconciler.java | Updates test reconciler to force filtering via new options. |
| operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerEventSourceTest.java | Adds unit tests around skipping/forcing filtering when resourceVersion is null. |
| operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/reconciler/ResourceOperationsTest.java | Updates mocks/verifications for the new event-filtering method signature. |
| operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/ManagedInformerEventSource.java | Adds bypass + “force filtering” behavior based on resourceVersion presence; adds cache-only update helper. |
| operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/ReconciliationDispatcher.java | Adjusts finalizer-add dispatch flow (notably rescheduling behavior). |
| operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/KubernetesDependentResource.java | Forces filtering for certain dependent-resource create/update paths with the new options API. |
| operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/ResourceOperations.java | Introduces Options/Mode and threads options through multiple patch/update/create paths. |
| operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/Matcher.java | Adds a new matcher interface intended for future-proofing API surface. |
| } | ||
| return PostExecutionControl.onlyFinalizerAdded(updatedResource) | ||
| .withReSchedule(BaseControl.INSTANT_RESCHEDULE); | ||
| return PostExecutionControl.onlyFinalizerAdded(updatedResource); |
| } | ||
| } | ||
|
|
||
| // this is designed with forward compatibility in mynd |
| // it is safe to do event filtering for create since check if the resource already exists. | ||
| return create(resource, Options.forcedFiltering()); |
| // it is safe to do event filtering for create since check if the resource already exists. | ||
| return resourcePatch( |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 14 out of 14 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (1)
operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/ResourceOperations.java:950
- The
Optionsinner type is missing closing braces at the end of the file, which will preventResourceOperationsfrom compiling. TheisForcedFiltering()method andModeenum also appear to be outside the intended indentation scope, suggesting the closing braces were accidentally dropped during the edit.
public boolean isForcedFiltering() {
return mode == Mode.FORCED_FILTERING;
}
@Experimental(API_MIGHT_CHANGE)
public enum Mode {
FILTERING_IF_OPTIMISTIC_LOCKING,
FORCED_FILTERING,
ONLY_CACHE,
}
}
| } | ||
| } | ||
|
|
||
| // this is designed with forward compatibility in mynd |
| } | ||
| return PostExecutionControl.onlyFinalizerAdded(updatedResource) | ||
| .withReSchedule(BaseControl.INSTANT_RESCHEDULE); | ||
| return PostExecutionControl.onlyFinalizerAdded(updatedResource); |
Extended version of #3484
Which adds more refined API, explicit option to cache only (not filtering) and future proofed to machers.
TODO:
update javadocs, documentation, release create post.