improve: resource operation support multiple update modes#3491
Open
csviri wants to merge 34 commits into
Open
Conversation
b978daa to
f0d3fcb
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
This PR reworks ResourceOperations to support multiple update modes (match-before-write, optimistic-locking-aware filtering, cache-only, force-filter), introduces a new Matcher SPI plus default matchers per update type, and expands test/doc coverage around own-event filtering correctness and read-cache-after-write behavior.
Changes:
- Adds matcher-aware update/patch/create modes to
ResourceOperations(including default matchers per operation type). - Introduces new matcher implementations (including status-only matching) and expands unit/integration tests for own-event filtering edge cases.
- Updates documentation/migration notes for v5.5 and adjusts CI integration-test timeout.
Reviewed changes
Copilot reviewed 47 out of 47 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/subresource/SubResourceUpdateIT.java | Updates assertions to reflect additional reconciliation triggered by status updates. |
| operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/resourceoperations/SecondaryResourceOperationsReconciler.java | New reconciler exercising secondary-resource ResourceOperations overloads and update modes. |
| operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/resourceoperations/SecondaryResourceOperationsIT.java | New IT covering secondary-resource operations convergence without looping. |
| operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/resourceoperations/SecondaryResourceOperationsCustomResource.java | New CR type for secondary resource operations tests. |
| operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/resourceoperations/ResourceOperationsStatus.java | New status model for ResourceOperations test CRs. |
| operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/resourceoperations/ResourceOperationsSpec.java | New spec model for ResourceOperations test CRs. |
| operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/resourceoperations/ResourceOperationsReconciler.java | New reconciler exercising primary update/patch/SSA + status variants via ResourceOperations. |
| operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/resourceoperations/ResourceOperationsIT.java | New IT covering all primary update/patch/SSA strategies and convergence. |
| operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/resourceoperations/ResourceOperationsCustomResource.java | New CR type for primary resource operations tests. |
| operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/readcacheafterwrite/specchangeduringstatuspatch/SpecChangeDuringStatusPatchStatus.java | New status type for spec-change-during-status-patch reproduction test. |
| operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/readcacheafterwrite/specchangeduringstatuspatch/SpecChangeDuringStatusPatchSpec.java | New spec type for spec-change-during-status-patch reproduction test. |
| operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/readcacheafterwrite/specchangeduringstatuspatch/SpecChangeDuringStatusPatchReconciler.java | New reconciler reproducing concurrent spec change during status patch filtering window. |
| operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/readcacheafterwrite/specchangeduringstatuspatch/SpecChangeDuringStatusPatchIT.java | New repeated IT validating spec change is not swallowed by own status patch filtering. |
| operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/readcacheafterwrite/specchangeduringstatuspatch/SpecChangeDuringStatusPatchCustomResource.java | New CR type for the spec-change-during-status-patch scenario. |
| operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/readcacheafterwrite/ownssastatusupdate/OwnSsaStatusUpdateStatus.java | New status type for own SSA status update scenario. |
| operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/readcacheafterwrite/ownssastatusupdate/OwnSsaStatusUpdateReconciler.java | New reconciler applying status via resourceOperations().serverSideApplyPrimaryStatus(...). |
| operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/readcacheafterwrite/ownssastatusupdate/OwnSsaStatusUpdateIT.java | New IT asserting own SSA status update doesn’t loop and external updates still reconcile. |
| operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/readcacheafterwrite/ownssastatusupdate/OwnSsaStatusUpdateCustomResource.java | New CR type for own SSA status update test. |
| operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/readcacheafterwrite/ownsecondaryupdate/OwnSecondaryUpdateReconciler.java | Switches SSA calls to explicit force-filter options for secondary own-event filtering. |
| operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/readcacheafterwrite/onrelistfilter/OnRelistFilterReconciler.java | Updates SSA calls to force-filter options for relist filtering scenarios. |
| operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/changenamespace/ChangeNamespaceTestReconciler.java | Updates SSA call to force-filter options. |
| operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/matcher/StatusMatchersTest.java | New unit tests for status-only matchers behavior. |
| operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/matcher/PatchMatchersTest.java | New unit tests for JSON patch/merge patch matchers (resource + status). |
| operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerEventSourceTest.java | Adds test ensuring force-filter opens window even without resourceVersion. |
| operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/GenericKubernetesResourceMatcherTest.java | Adds tests for new matchStatus(...) behavior and equality modes. |
| operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/reconciler/ResourceOperationsTest.java | Extends tests for matcher-driven patching, cache-only, and filtering modes. |
| operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/matcher/UpdateType.java | New enum mapping update types to default matchers. |
| operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/matcher/UpdateStatusMatcher.java | New matcher for update-status operations using status-only matching. |
| operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/matcher/UpdateMatcher.java | New matcher for update operations using generic resource matching. |
| operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/matcher/SSAStatusMatcher.java | New matcher for SSA status operations using status-only matching. |
| operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/matcher/SSAMatcher.java | New matcher for SSA operations using SSA-based matcher implementation. |
| operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/matcher/MatcherUtils.java | New shared utilities for JSON patch/merge patch matcher implementations. |
| operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/matcher/JsonPatchStatusMacher.java | New JSON Patch status matcher implementation. |
| operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/matcher/JsonPatchMacher.java | New JSON Patch matcher implementation. |
| operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/matcher/JsonMergePatchStatusMatcher.java | New JSON Merge Patch status matcher implementation. |
| operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/matcher/JsonMergePatchMatcher.java | New JSON Merge Patch matcher implementation. |
| operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/ManagedInformerEventSource.java | Adds update-and-cache helper and annotates update paths as experimental. |
| operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/ReconciliationDispatcher.java | Adjusts primary patch paths to use cache-only ResourceOperations variants. |
| operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/KubernetesDependentResource.java | Updates dependent resource create/update to use explicit force-filter options. |
| operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/GenericKubernetesResourceMatcher.java | Adds matchStatus(...) to compare only /status subtree with configurable equality. |
| operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/ResourceOperations.java | Major API expansion: modes/options, matchers, default matcher selection per operation type. |
| operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/matcher/Matcher.java | New matcher SPI for plug-in match logic. |
| docs/content/en/docs/migration/v5-5-migration.md | New migration doc describing v5.5 behavioral change around UpdateControl. |
| docs/content/en/docs/documentation/reconciler.md | Documents new ResourceOperations.Options modes and UpdateControl behavior change. |
| docs/content/en/blog/releases/v5-5-release.md | New release post for v5.5 summarizing features and migration notes. |
| docs/content/en/blog/news/read-after-write-consistency.md | Updates blog content to reflect explicit facilities for own-event filtering. |
| .github/workflows/integration-tests.yml | Increases integration test job timeout. |
57157d8 to
4ddb3d5
Compare
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>
Signed-off-by: Attila Mészáros <a_meszaros@apple.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.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>
The status assertions could throw NPE on the first Awaitility poll before the reconciler patched the status. untilAsserted retries AssertionErrors but rethrows other exceptions immediately, so the NPE aborted the test instead of retrying. Guard with assertThat(status).isNotNull() to keep polling. Signed-off-by: Attila Mészáros <a_meszaros@apple.com>
440b380 to
2d7c490
Compare
Signed-off-by: Attila Mészáros <a_meszaros@apple.com>
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.
When an event-filtering and caching update results in a no-op (i.e., nothing changes on the resource), and there is a concurrent update that actually modifies the resource, the event from that concurrent update may be filtered out.
This is particularly problematic when we patch the primary resource's status and the patch is a no-op, while another actor concurrently changes the resource's spec. In this case, the spec change would not result in an update event.
Although we could handle this particular case in the code, there are other situations (such as two concurrent updates to a secondary resource without optimistic locking) where we want to avoid similar issues. Therefore, we now limit event filtering to updates performed using optimistic locking or matching the resource before an update / patch operation.
We recognize that this changes the previous behavior. However, since controllers are expected to be idempotent by default, we do not expect this to have a significant impact on users.
The PR extends resource operation with options like:
Adjust the documentation and prepares release notes and migration guide.