diff --git a/.github/workflows/integration-tests.yml b/.github/workflows/integration-tests.yml index 385c5b67da..6264ba3aa2 100644 --- a/.github/workflows/integration-tests.yml +++ b/.github/workflows/integration-tests.yml @@ -27,7 +27,7 @@ jobs: name: Integration tests (${{ inputs.java-version }}, ${{ inputs.kube-version }}, ${{ inputs.http-client }}) runs-on: ubuntu-latest continue-on-error: ${{ inputs.experimental }} - timeout-minutes: 40 + timeout-minutes: 120 steps: - uses: actions/checkout@v7 with: diff --git a/docs/content/en/blog/news/read-after-write-consistency.md b/docs/content/en/blog/news/read-after-write-consistency.md index 29e3c5f626..36edf97693 100644 --- a/docs/content/en/blog/news/read-after-write-consistency.md +++ b/docs/content/en/blog/news/read-after-write-consistency.md @@ -30,8 +30,8 @@ public UpdateControl reconcile(WebPage webPage, Context contex } ``` -In addition to that, the framework will automatically filter events for your own updates, -so they don't trigger the reconciliation again. +In addition to that, the framework will provide facilities to filter out +events for own updates so they don't trigger the reconciliation again. {{% alert color=success %}} **This should significantly simplify controller development, and will make reconciliation @@ -180,12 +180,6 @@ From this point the idea of the algorithm is very simple: the one in the TRC. If yes, evict the resource from the TRC. 3. When the controller reads a resource from cache, it checks the TRC first, then falls back to the Informer's cache. -The actual filtering of events for our own writes is more nuanced than a simple -"evict on RV ≥ TRC version" rule — it is driven by a per-resource state machine -that tracks in-flight writes and the events received around them. See -[Filtering events for our own updates](#filtering-events-for-our-own-updates) below. - - ```mermaid sequenceDiagram box rgba(50,108,229,0.1) @@ -226,10 +220,30 @@ sequenceDiagram When we update a resource, the informer will eventually propagate an event that would trigger a reconciliation. In most cases, however, this is not desirable. Since we already have the up-to-date resource at that point, we want to be notified only when the change originates outside our reconciler. -Therefore, in addition to caching the resource, we filter out events caused by our own updates. +Therefore, in addition to caching the resource, provide utilities to so you can optimize the update/patch +operations to filter out those events. + +See [ResourceOperations](https://github.com/operator-framework/java-operator-sdk/blob/main/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/ResourceOperations.java#L49) +for details. + +```java +public UpdateControl reconcile(WebPage webPage, Context context) { + + ConfigMap managedConfigMap = prepareConfigMap(webPage); + + // resource operation in this case will resource only if + // it does not match the actual, and will filter our the related event + context.resourceOperations().serverSideApply(managedConfigMap); + + // UpdateControl.patchStatus would only cache the resource to filter out events too + // you have to use resourceOperations. + context.resourceOperations().serverSideApplyPrimaryStatus(alterStatusObject(webPage)); + return UpdateControl.noUpdate(); +} +``` -Note that the implementation of this is relatively complex: while performing the update, we record all the -events received in the meantime and decide whether to propagate them further once the update request completes. +Note that the implementation of this is relatively complex and has some caveats: +while performing the update, we record all the events received in the meantime and decide whether to propagate them further once the update request completes. This way, we significantly reduce the number of reconciliations, making the whole process much more efficient. diff --git a/docs/content/en/blog/releases/v5-5-release.md b/docs/content/en/blog/releases/v5-5-release.md new file mode 100644 index 0000000000..17126317a1 --- /dev/null +++ b/docs/content/en/blog/releases/v5-5-release.md @@ -0,0 +1,120 @@ +--- +title: Version 5.5 Released! +date: 2026-07-14 +author: >- + [Attila Mészáros](https://github.com/csviri) +--- + +We're pleased to announce the release of Java Operator SDK v5.5.0! This minor version reworks how +the framework filters the events caused by a controller's own writes, making it more correct and +more efficient, and exposes a richer, matcher-aware `ResourceOperations` API. There are **no +breaking API changes**, but there is one behavioral change around `UpdateControl` — see the +migration notes. + +## Key Features + +### Matcher-based updates in `ResourceOperations` + +`ResourceOperations` (available from the reconciliation `Context` via `context.resourceOperations()`) +now offers a complete, consistent family of update/patch/create methods for every write strategy — +**server-side apply, update (PUT), JSON Patch (RFC 6902) and JSON Merge Patch (RFC 7386)** — each +available for the whole resource and the `status` subresource, and with dedicated `primary` +variants. + +Every method comes in two flavors: a default one, and one taking an `Options` argument that controls +how the resulting own event is handled: + +```java +public UpdateControl reconcile(WebPage webPage, Context context) { + makeStatusChanges(webPage); + // updates the status, filters the own event, and skips the write entirely if nothing changed + context.resourceOperations().serverSideApplyPrimaryStatus(webPage); + return UpdateControl.noUpdate(); +} +``` + +By default these operations **match the desired state against the actual (cached) state before +writing**: if they already match, the write is skipped; otherwise the write is performed and its own +event is filtered. This is the most efficient option — it covers full event filtering *and* avoids a +request to the Kubernetes API server when nothing changed. Default matchers are provided for every +operation type; when one does not fit, supply your own via `Options.matchAndFilter(matcher)`. + +`Options` exposes the available strategies: + +- `matchAndFilter(...)` / `matchAndFilterWithDefaultMatcher(...)` — match, then write-and-filter only + if needed (the default). +- `filterIfOptimisticLocking()` — filter the own event only when the write uses optimistic locking, + otherwise just cache the response. +- `cacheOnly()` — only cache the response (read-cache-after-write consistency), no filtering. +- `forceFilterEvents()` — always filter (mostly internal usage). + +> **Correctness note**: filtering an own event is only safe if the framework can tell an own write +> apart from a concurrent third-party write. This requires **either** a matcher **or** optimistic +> locking; otherwise a concurrent external update inside the filter window may be missed until the +> next resync. + +### More correct own-event filtering (no-op update edge case) + +The read-cache-after-write own-event filter has been reworked to fix an edge case where a legitimate +external change could be filtered out together with a controller's own **no-op** update — for +example, when the spec was changed externally while the controller patched its status and that status +patch turned out to be a no-op. The new matcher-based operations avoid issuing such no-op writes in +the first place, and the filtering itself is now correct in these concurrent scenarios. + +## Additional Improvements + +- **`GenericKubernetesResourceMatcher.matchStatus(...)`**: a status-only counterpart to `match(...)` + that compares just the `/status` subtree. +- **New `Matcher` SPI**: `io.javaoperatorsdk.operator.api.reconciler.matcher.Matcher` lets you plug a + custom matching strategy into `Options.matchAndFilter(...)`. +- Extensive integration tests covering every `ResourceOperations` update/patch operation (primary, + status, and secondary resources) against a real cluster. + +## Migration Notes + +There are **no breaking API changes**; existing code compiles and runs unchanged. However: + +### `UpdateControl` no longer filters own events by default + +Returning an `UpdateControl` (or `ErrorStatusUpdateControl`) still updates the resource and keeps the +cache read-after-write consistent, but it **no longer filters the resulting own event by default** — +so the write may cause an additional (idempotent) reconciliation. This change fixes correctness edge +cases where an event that should have propagated was previously swallowed. + +If you relied on the previous filtering, perform the update through `ResourceOperations` and return +`UpdateControl.noUpdate()`: + +```java +// before (v5.4) +resource.setStatus(new MyStatus().setReady(true)); +return UpdateControl.patchStatus(resource); + +// after (v5.5) — filter the own event explicitly +resource.setStatus(new MyStatus().setReady(true)); +context.resourceOperations().serverSideApplyPrimaryStatus(resource); +return UpdateControl.noUpdate(); +``` + +See the [migration guide](/docs/migration/v5-5-migration) for details. + +## Getting Started + +```xml + + io.javaoperatorsdk + operator-framework + 5.5.0 + +``` + +## All Changes + +See the [comparison view](https://github.com/operator-framework/java-operator-sdk/compare/v5.4.0...v5.5.0) +for the full list of changes. + +## Feedback + +Please report issues or suggest improvements on our +[GitHub repository](https://github.com/operator-framework/java-operator-sdk/issues). + +Happy operator building! 🚀 diff --git a/docs/content/en/docs/documentation/reconciler.md b/docs/content/en/docs/documentation/reconciler.md index 7b24c23e3b..6151cdcb08 100644 --- a/docs/content/en/docs/documentation/reconciler.md +++ b/docs/content/en/docs/documentation/reconciler.md @@ -192,6 +192,37 @@ supports stronger guarantees, both for primary and secondary resources. If this they would otherwise look like own echoes, since the relist may have hidden events. +#### Requesting event filtering and its correctness requirements + +Own-event filtering is only safe if the framework can tell an own write apart from a concurrent +third-party write. This requires **either**: + +- a *matcher* to be provided (so a filtered event can be confirmed to reflect the desired state we + just wrote), **or** +- the update to be done using *optimistic locking* (a resource version set on the written resource), + so a conflicting concurrent change is rejected by the API server rather than silently swallowed. + +`ResourceOperations` methods accept an `Options` argument to select the behavior; each maps to a +`Mode`: + +- `Options.matchAndFilter(matcher)` / `Options.matchAndFilterWithDefaultMatcher(updateType)` — compare + the desired state to the actual (cached) state; if they already match, skip the write entirely, + otherwise write and filter the own event. This is the **default** for the `ResourceOperations` + update/patch methods, and generally the most efficient option: it filters the own event *and* + avoids a request to the API server when nothing changed. Default matchers are provided for every + operation type, but they are heuristics — a workflow relying on them should be tested against the + concrete resources it manages, or a custom matcher supplied. +- `Options.filterIfOptimisticLocking()` — filter the own event only when the write uses optimistic + locking, otherwise just cache the response. +- `Options.cacheOnly()` — only cache the response (read-cache-after-write consistency), no own-event + filtering. +- `Options.forceFilterEvents()` — always filter, regardless of optimistic locking. Only safe when + correctness is otherwise guaranteed (mostly for internal usage); a concurrent external update in + the filter window may otherwise be missed until the next resync. + +See the [`ResourceOperations`](https://github.com/operator-framework/java-operator-sdk/blob/main/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/ResourceOperations.java) +and `ResourceOperations.Options` documentation for details. + In order to benefit from these stronger guarantees, use [`ResourceOperations`](https://github.com/operator-framework/java-operator-sdk/blob/main/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/ResourceOperations.java) from the context of the reconciliation: @@ -208,20 +239,26 @@ public UpdateControl reconcile(WebPage webPage, Context contex var upToDateResource = context.getSecondaryResource(ConfigMap.class); makeStatusChanges(webPage); - - // built in update methods by default use this feature + // patches the status and caches the response (does not filter the own event by default, see below) return UpdateControl.patchStatus(webPage); } ``` -`UpdateControl` and `ErrorStatusUpdateControl` by default use this functionality, but you can also update your primary resource at any time during the reconciliation using `ResourceOperations`: +{{% alert title="UpdateControl and event filtering" %}} +Since v5.5, returning an `UpdateControl` (or `ErrorStatusUpdateControl`) updates the resource and +keeps the cache read-after-write consistent, but by default it **no longer filters the own event** — +the resulting update may cause an additional (idempotent) reconciliation. To also filter the own +event, perform the update through `ResourceOperations` instead and return `UpdateControl.noUpdate()`. +{{% /alert %}} + +You can update your primary resource at any time during the reconciliation using `ResourceOperations`: ```java public UpdateControl reconcile(WebPage webPage, Context context) { makeStatusChanges(webPage); - // this is equivalent to UpdateControl.patchStatus(webpage) + // updates the status, filters the own event and skips the write if nothing changed context.resourceOperations().serverSideApplyPrimaryStatus(webPage); return UpdateControl.noUpdate(); } diff --git a/docs/content/en/docs/migration/v5-5-migration.md b/docs/content/en/docs/migration/v5-5-migration.md new file mode 100644 index 0000000000..0d9194e7e3 --- /dev/null +++ b/docs/content/en/docs/migration/v5-5-migration.md @@ -0,0 +1,69 @@ +--- +title: Migrating from v5.4 to v5.5 +description: Migrating from v5.4 to v5.5 +--- + +## No breaking API changes + +v5.5 does **not** contain breaking API changes: existing code compiles and continues to work without +modification. There is, however, one **behavioral** change around own-event filtering that you should +be aware of (see below). + +## `UpdateControl` no longer filters own events by default + +In previous versions, updating the primary resource (or its status) by returning an `UpdateControl` +from the reconciler would filter out the event caused by that own update, so the write did not +trigger an additional reconciliation. Unfortunately, in some edge cases this could lead an +incorrect behavior, thus filtering out events which should be propagated. + +More precisely in case where for example the spec part of the primary resource was updated, while +controller patched the status, but that patch status was a no-op operation. Note that +these no-op operations are causing issue, which should not be done in first place. +From 5.5 we provide methods in `ResourceOperations` which allow only operations which are +correct. + +From v5.5, `UpdateControl` **no longer filters these own events by default**. Returning an +`UpdateControl` still updates the resource and keeps the cache read-after-write consistent, but the +resulting update event is now delivered like any other event, which may cause an additional +reconciliation. + +This is safe (reconciliations are expected to be idempotent), but if you relied on the previous +filtering — for example to avoid an extra reconciliation after a status update — use +[`ResourceOperations`](https://github.com/operator-framework/java-operator-sdk/blob/main/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/ResourceOperations.java) +directly, which does filter own events. + +### Using `ResourceOperations` instead + +`ResourceOperations` is available from the reconciliation `Context` via +`context.resourceOperations()`. Instead of returning an `UpdateControl`, perform the update through +it and return `UpdateControl.noUpdate()`: + +```java +// before (v5.4): the own status update event was filtered +@Override +public UpdateControl reconcile(MyResource resource, Context context) { + resource.setStatus(new MyStatus().setReady(true)); + return UpdateControl.patchStatus(resource); +} +``` + +```java +// after (v5.5): filter the own event explicitly via ResourceOperations +@Override +public UpdateControl reconcile(MyResource resource, Context context) { + resource.setStatus(new MyStatus().setReady(true)); + context.resourceOperations().serverSideApplyPrimaryStatus(resource); + return UpdateControl.noUpdate(); +} +``` + +`ResourceOperations` covers every update/patch strategy (server-side apply, update, JSON Patch, JSON +Merge Patch) for both the whole resource and the status subresource, as well as their primary +variants. By default these operations match the desired state against the actual (cached) state +before writing and filter the own event, so they only issue a request to the Kubernetes API server +when something actually changed. + +> **Note**: Safe own-event filtering requires either a matcher (used by default) or the update to be +> done with optimistic locking. See the `ResourceOperations` and `ResourceOperations.Options` +> documentation for the available modes, the correctness requirements, and the caveats of the +> default matchers. diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/ResourceOperations.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/ResourceOperations.java index b9ef475509..c84e0a55b9 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/ResourceOperations.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/ResourceOperations.java @@ -16,6 +16,7 @@ package io.javaoperatorsdk.operator.api.reconciler; import java.lang.reflect.InvocationTargetException; +import java.util.Optional; import java.util.function.Predicate; import java.util.function.UnaryOperator; @@ -27,20 +28,77 @@ import io.fabric8.kubernetes.client.dsl.base.PatchContext; import io.fabric8.kubernetes.client.dsl.base.PatchType; import io.javaoperatorsdk.operator.OperatorException; +import io.javaoperatorsdk.operator.api.reconciler.matcher.Matcher; import io.javaoperatorsdk.operator.processing.event.ResourceID; import io.javaoperatorsdk.operator.processing.event.source.informer.InformerEventSource; import io.javaoperatorsdk.operator.processing.event.source.informer.ManagedInformerEventSource; +import io.javaoperatorsdk.operator.processing.matcher.UpdateType; +import static io.javaoperatorsdk.operator.api.reconciler.Experimental.API_MIGHT_CHANGE; import static io.javaoperatorsdk.operator.processing.KubernetesResourceUtils.getUID; import static io.javaoperatorsdk.operator.processing.KubernetesResourceUtils.getVersion; /** - * Provides useful operations to manipulate resources (server-side apply, patch, etc.) in an - * idiomatic way, in particular to make sure that the latest version of the resource is present in - * the caches for the next reconciliation. In other words, it provides read-cache-after-write - * consistency. + * Provides various, useful operations to manipulate resources (server-side apply, patch, etc.) in + * an idiomatic ways. Provides improved update/patch/create operations to make sure that the latest + * version of the resource is present in the caches for the next reconciliation, and filter own + * update events. You can still use kubernetes client directly, these methods are however useful to + * achieve better efficiency. * - * @param

the resource type on which this object operates + *

Every update/patch/create method comes in two flavors: a default one, and one that takes an + * {@link Options} argument to control how the resulting own event is handled. The behavior is + * selected through {@link Mode}: + * + *

    + *
  • {@link Options#filterIfOptimisticLocking()} ({@link Mode#FILTER_IF_OPTIMISTIC_LOCKING}) - + * the own event is filtered only when the write uses optimistic locking (i.e. the resource + * version is set on the resource being written); otherwise the response is only cached. This + * is the safe default for plain update/patch operations. + *
  • {@link Options#matchAndFilter(Matcher)} / {@link + * Options#matchAndFilterWithDefaultMatcher(io.javaoperatorsdk.operator.processing.matcher.UpdateType)} + * ({@link Mode#FILTER_IF_NOT_MATCHING}) - before writing, the desired state is compared to + * the actual (cached) state using the provided {@link Matcher}; if they already match the + * write is skipped, otherwise it is performed and the own event is filtered. + *
  • {@link Options#cacheOnly()} ({@link Mode#CACHE_ONLY}) - the response is only put into the + * cache (read-cache-after-write consistency) and no own-event filtering is done. + *
  • {@link Options#forceFilterEvents()} ({@link Mode#FORCE_FILTER}) - the own event is always + * filtered, regardless of optimistic locking. Use only when correctness is otherwise + * guaranteed (see the note below). This is mostly for internal usage, like in {@link + * io.javaoperatorsdk.operator.processing.dependent.kubernetes.KubernetesDependentResource} + * where we match the resource explicitly before update. + *
+ * + *

Correctness of event filtering. Filtering an own update event is only safe if + * the framework can tell an own write apart from a concurrent third-party write. This requires + * either: + * + *

    + *
  • a {@link Matcher} to be provided (so a filtered event can be confirmed to match the desired + * state we just wrote), or + *
  • the update to be done using optimistic locking (a resource version set on the + * written resource), so a conflicting concurrent change is rejected by the API server rather + * than silently swallowed. + *
+ * + *

If neither holds - for example {@link Options#forceFilterEvents()} on a write without + * optimistic locking and without a matcher - a concurrent external update happening within the + * filtering window may be filtered out and thus missed until the next resync. Prefer providing a + * matcher or using optimistic locking whenever own-event filtering is desired. + * + *

Default matchers. For the matching modes a default {@link Matcher} is + * provided for every {@link io.javaoperatorsdk.operator.processing.matcher.UpdateType} (see {@link + * Options#matchAndFilterWithDefaultMatcher(io.javaoperatorsdk.operator.processing.matcher.UpdateType)}), + * so matching works out of the box. Note however that these default matchers are heuristics and may + * have issues in some edge cases, so a workflow relying on them should be tested against the + * concrete resources it manages. When a default matcher does not fit, provide your own via {@link + * Options#matchAndFilter(Matcher)}. + * + *

Despite that caveat, matching is generally the most efficient way to handle updates: it covers + * full event filtering and only performs a write when the actual state actually differs + * from the desired one, thus also reducing the number of requests made against the Kubernetes API + * server. + * + * @param

the primary resource type on which this object operates */ public class ResourceOperations

{ @@ -55,19 +113,35 @@ public ResourceOperations(Context

context) { } /** - * Updates the resource and caches the response if needed, thus making sure that next - * reconciliation will see to updated resource - or more recent one if additional update happened - * after this update; In addition to that it filters out the event from the update, so - * reconciliation is not triggered by own update. + * Server-Side Applies the resource, then caches the response so the next reconciliation observes + * it (read-cache-after-write consistency). * - *

You are free to control the optimistic locking by setting the resource version in resource - * metadata. In case of SSA we advise not to do updates with optimistic locking. + *

Uses the {@link UpdateType#SSA} default {@link Matcher}: the apply is skipped when the + * actual (cached) state already matches the desired one, otherwise it is performed and the + * resulting own event is filtered so it does not trigger a reconciliation. Use {@link + * #serverSideApply( HasMetadata, Options)} to change this behavior. SSA does not require + * optimistic locking. * - * @param resource fresh resource for server side apply - * @return updated resource - * @param resource type + * @param resource the desired resource to server-side apply + * @param the resource type + * @return the applied resource as returned by the API server */ public R serverSideApply(R resource) { + return serverSideApply(resource, Options.matchAndFilterWithDefaultMatcher(UpdateType.SSA)); + } + + /** + * Server-Side Applies the resource, controlling caching and own-event handling through the given + * {@link Options}. + * + * @param resource the desired resource to server-side apply + * @param options controls caching and own-event filtering; see {@link Options} and the class + * documentation + * @param the resource type + * @return the applied resource as returned by the API server + */ + @Experimental(API_MIGHT_CHANGE) + public R serverSideApply(R resource, Options options) { return resourcePatch( resource, r -> @@ -79,25 +153,40 @@ public R serverSideApply(R resource) { .withForce(true) .withFieldManager(context.getControllerConfiguration().fieldManager()) .withPatchType(PatchType.SERVER_SIDE_APPLY) - .build())); + .build()), + options); } /** - * Updates the resource and caches the response if needed, thus making sure that next - * reconciliation will see to updated resource - or more recent one if additional update happened - * after this update; In addition to that it filters out the event from the update, so - * reconciliation is not triggered by own update. - * - *

You are free to control the optimistic locking by setting the resource version in resource - * metadata. In case of SSA we advise not to do updates with optimistic locking. + * Server-Side Applies the resource, caching and filtering the own event through the given {@link + * InformerEventSource} (instead of the controller's own event source). Uses the {@link + * UpdateType#SSA} default {@link Matcher}. * - * @param resource fresh resource for server side apply - * @return updated resource - * @param informerEventSource InformerEventSource to use for resource caching and filtering - * @param resource type + * @param resource the desired resource to server-side apply + * @param informerEventSource the event source used for caching and own-event filtering + * @param the resource type + * @return the applied resource as returned by the API server */ public R serverSideApply( R resource, InformerEventSource informerEventSource) { + return serverSideApply( + resource, informerEventSource, Options.matchAndFilterWithDefaultMatcher(UpdateType.SSA)); + } + + /** + * Server-Side Applies the resource, caching and filtering the own event through the given {@link + * InformerEventSource} and using the given {@link Options}. When {@code informerEventSource} is + * {@code null} this falls back to {@link #serverSideApply(HasMetadata)}. + * + * @param resource the desired resource to server-side apply + * @param informerEventSource the event source used for caching and own-event filtering + * @param options controls caching and own-event filtering; see {@link Options} + * @param the resource type + * @return the applied resource as returned by the API server + */ + @Experimental(API_MIGHT_CHANGE) + public R serverSideApply( + R resource, InformerEventSource informerEventSource, Options options) { if (informerEventSource == null) { return serverSideApply(resource); } @@ -113,25 +202,20 @@ public R serverSideApply( .withFieldManager(context.getControllerConfiguration().fieldManager()) .withPatchType(PatchType.SERVER_SIDE_APPLY) .build()), - informerEventSource); + informerEventSource, + options); } /** - * Server-Side Apply the resource status subresource. - * - *

Updates the resource and caches the response if needed, thus making sure that next - * reconciliation will see to updated resource - or more recent one if additional update happened - * after this update; In addition to that it filters out the event from this update, so - * reconciliation is not triggered by own update. - * - *

You are free to control the optimistic locking by setting the resource version in resource - * metadata. In case of SSA we advise not to do updates with optimistic locking. + * Server-Side Applies the {@code status} subresource, controlling caching and own-event handling + * through the given {@link Options}. * - * @param resource fresh resource for server side apply - * @return updated resource - * @param resource type + * @param resource the desired resource (with the status to apply) + * @param options controls caching and own-event filtering; see {@link Options} + * @param the resource type + * @return the applied resource as returned by the API server */ - public R serverSideApplyStatus(R resource) { + public R serverSideApplyStatus(R resource, Options options) { return resourcePatch( resource, r -> @@ -144,24 +228,44 @@ public R serverSideApplyStatus(R resource) { .withForce(true) .withFieldManager(context.getControllerConfiguration().fieldManager()) .withPatchType(PatchType.SERVER_SIDE_APPLY) - .build())); + .build()), + options); } /** - * Server-Side Apply the primary resource. + * Server-Side Applies the {@code status} subresource, caching the response and filtering the own + * event. Uses the {@link UpdateType#SSA_STATUS} default {@link Matcher}. * - *

Updates the resource and caches the response if needed, thus making sure that next - * reconciliation will see to updated resource - or more recent one if additional update happened - * after this update; In addition to that it filters out the event from this update, so - * reconciliation is not triggered by own update. - * - *

You are free to control the optimistic locking by setting the resource version in resource - * metadata. In case of SSA we advise not to do updates with optimistic locking. + * @param resource the desired resource (with the status to apply) + * @param the resource type + * @return the applied resource as returned by the API server + */ + public R serverSideApplyStatus(R resource) { + return serverSideApplyStatus( + resource, Options.matchAndFilterWithDefaultMatcher(UpdateType.SSA_STATUS)); + } + + /** + * Server-Side Applies the primary resource, caching and filtering the own event through the + * controller's own event source. Uses the {@link UpdateType#SSA} default {@link Matcher}. * - * @param resource primary resource for server side apply - * @return updated resource + * @param resource the desired primary resource to server-side apply + * @return the applied resource as returned by the API server */ public P serverSideApplyPrimary(P resource) { + return serverSideApplyPrimary( + resource, Options.matchAndFilterWithDefaultMatcher(UpdateType.SSA)); + } + + /** + * Server-Side Applies the primary resource, caching and filtering the own event through the + * controller's own event source and using the given {@link Options}. + * + * @param resource the desired primary resource to server-side apply + * @param options controls caching and own-event filtering; see {@link Options} + * @return the applied resource as returned by the API server + */ + public P serverSideApplyPrimary(P resource, Options options) { return resourcePatch( resource, r -> @@ -174,26 +278,34 @@ public P serverSideApplyPrimary(P resource) { .withFieldManager(context.getControllerConfiguration().fieldManager()) .withPatchType(PatchType.SERVER_SIDE_APPLY) .build()), - context.eventSourceRetriever().getControllerEventSource()); + context.eventSourceRetriever().getControllerEventSource(), + options); } /** - * Server-Side Apply the primary resource status subresource. - * - *

Updates the resource and caches the response if needed, thus making sure that next - * reconciliation will see to updated resource - or more recent one if additional update happened - * after this update; In addition to that it filters out the event from this update, so - * reconciliation is not triggered by own update. + * Server-Side Applies the primary resource {@code status} subresource, caching and filtering the + * own event through the controller's own event source. Uses the {@link UpdateType#SSA_STATUS} + * default {@link Matcher}. * - *

You are free to control the optimistic locking by setting the resource version in resource - * metadata. In case of SSA we advise not to do updates with optimistic locking. + * @param desired the desired primary resource (with the status to apply) + * @return the applied resource as returned by the API server + */ + public P serverSideApplyPrimaryStatus(P desired) { + return serverSideApplyPrimaryStatus( + desired, Options.matchAndFilterWithDefaultMatcher(UpdateType.SSA_STATUS)); + } + + /** + * Server-Side Applies the primary resource {@code status} subresource, caching and filtering the + * own event through the controller's own event source and using the given {@link Options}. * - * @param resource primary resource for server side apply - * @return updated resource + * @param desired the desired primary resource (with the status to apply) + * @param options controls caching and own-event filtering; see {@link Options} + * @return the applied resource as returned by the API server */ - public P serverSideApplyPrimaryStatus(P resource) { + public P serverSideApplyPrimaryStatus(P desired, Options options) { return resourcePatch( - resource, + desired, r -> context .getClient() @@ -205,346 +317,589 @@ public P serverSideApplyPrimaryStatus(P resource) { .withFieldManager(context.getControllerConfiguration().fieldManager()) .withPatchType(PatchType.SERVER_SIDE_APPLY) .build()), - context.eventSourceRetriever().getControllerEventSource()); + context.eventSourceRetriever().getControllerEventSource(), + options); } /** - * Updates the resource and caches the response if needed, thus making sure that next - * reconciliation will see to updated resource - or more recent one if additional update happened - * after this update; In addition to that it filters out the event from this update, so - * reconciliation is not triggered by own update. + * Updates (HTTP PUT) the resource, then caches the response so the next reconciliation observes + * it (read-cache-after-write consistency). * - *

You are free to control the optimistic locking by setting the resource version in resource - * metadata. + *

Uses the {@link UpdateType#UPDATE} default {@link Matcher}: the update is skipped when the + * actual (cached) state already matches the desired one, otherwise it is performed and the + * resulting own event is filtered. Use {@link #update(HasMetadata, Options)} to change this. * - * @param resource resource to update - * @return updated resource - * @param resource type + * @param resource the desired resource to update + * @param the resource type + * @return the updated resource as returned by the API server */ public R update(R resource) { - return resourcePatch(resource, r -> context.getClient().resource(r).update()); + return update(resource, Options.matchAndFilterWithDefaultMatcher(UpdateType.UPDATE)); } /** - * Updates the resource and caches the response if needed, thus making sure that next - * reconciliation will see to updated resource - or more recent one if additional update happened - * after this update; In addition to that it filters out the event from this update, so - * reconciliation is not triggered by own update. + * Updates (HTTP PUT) the resource, controlling caching and own-event handling through the given + * {@link Options}. * - *

You are free to control the optimistic locking by setting the resource version in resource - * metadata. + * @param desired the desired resource to update + * @param options controls caching and own-event filtering; see {@link Options} + * @param the resource type + * @return the updated resource as returned by the API server + */ + @Experimental(API_MIGHT_CHANGE) + public R update(R desired, Options options) { + return resourcePatch(desired, r -> context.getClient().resource(r).update(), options); + } + + /** + * Updates (HTTP PUT) the resource, caching and filtering the own event through the given {@link + * InformerEventSource}. Uses the {@link UpdateType#UPDATE} default {@link Matcher}. * - * @param resource resource to update - * @return updated resource - * @param informerEventSource InformerEventSource to use for resource caching and filtering - * @param resource type + * @param resource the desired resource to update + * @param informerEventSource the event source used for caching and own-event filtering + * @param the resource type + * @return the updated resource as returned by the API server */ public R update( R resource, InformerEventSource informerEventSource) { + return update( + resource, informerEventSource, Options.matchAndFilterWithDefaultMatcher(UpdateType.UPDATE)); + } + + /** + * Updates (HTTP PUT) the resource, caching and filtering the own event through the given {@link + * InformerEventSource} and using the given {@link Options}. When {@code informerEventSource} is + * {@code null} this falls back to {@link #update(HasMetadata)}. + * + * @param resource the desired resource to update + * @param informerEventSource the event source used for caching and own-event filtering + * @param options controls caching and own-event filtering; see {@link Options} + * @param the resource type + * @return the updated resource as returned by the API server + */ + @Experimental(API_MIGHT_CHANGE) + public R update( + R resource, InformerEventSource informerEventSource, Options options) { if (informerEventSource == null) { return update(resource); } return resourcePatch( - resource, r -> context.getClient().resource(r).update(), informerEventSource); + resource, r -> context.getClient().resource(r).update(), informerEventSource, options); } /** - * Creates the resource and caches the response if needed, thus making sure that next - * reconciliation will see to updated resource - or more recent one if additional update happened - * after this update; In addition to that it filters out the event from this update, so - * reconciliation is not triggered by own update. - * - *

You are free to control the optimistic locking by setting the resource version in resource - * metadata. + * Creates the resource, then caches the response so the next reconciliation observes it + * (read-cache-after-write consistency). The resulting own event is filtered. * - * @param resource resource to update - * @return updated resource - * @param resource type + * @param resource the resource to create + * @param the resource type + * @return the created resource as returned by the API server */ public R create(R resource) { - return resourcePatch(resource, r -> context.getClient().resource(r).create()); + // it is safe to do event filtering for create since check if the resource already exists. + return create(resource, Options.forceFilterEvents()); } /** - * Creates the resource and caches the response if needed, thus making sure that next - * reconciliation will see to updated resource - or more recent one if additional update happened - * after this update; In addition to that it filters out the event from this update, so - * reconciliation is not triggered by own update. + * Creates the resource, controlling caching and own-event handling through the given {@link + * Options}. A matcher is not supported for create (there is nothing to match against) and passing + * one throws {@link IllegalArgumentException}. * - *

You are free to control the optimistic locking by setting the resource version in resource - * metadata. + * @param resource the resource to create + * @param options controls caching and own-event filtering; see {@link Options} + * @param the resource type + * @return the created resource as returned by the API server + * @throws IllegalArgumentException if the options carry a {@link Matcher} + */ + public R create(R resource, Options options) { + if (options.getMatcher().isPresent()) { + throw new IllegalArgumentException( + "Create operation does not support matcher. There is nothing to match."); + } + return resourcePatch(resource, r -> context.getClient().resource(r).create(), options); + } + + /** + * Creates the resource, caching and filtering the own event through the given {@link + * InformerEventSource} and using the given {@link Options}. When {@code informerEventSource} is + * {@code null} this falls back to {@link #create(HasMetadata)}. * - * @param resource resource to update - * @return updated resource - * @param informerEventSource InformerEventSource to use for resource caching and filtering - * @param resource type + * @param resource the resource to create + * @param informerEventSource the event source used for caching and own-event filtering + * @param options controls caching and own-event filtering; see {@link Options} + * @param the resource type + * @return the created resource as returned by the API server */ public R create( - R resource, InformerEventSource informerEventSource) { + R resource, InformerEventSource informerEventSource, Options options) { if (informerEventSource == null) { return create(resource); } + // it is safe to do event filtering for create since check if the resource already exists. return resourcePatch( - resource, r -> context.getClient().resource(r).create(), informerEventSource); + resource, r -> context.getClient().resource(r).create(), informerEventSource, options); } /** - * Updates the resource status subresource. - * - *

Updates the resource and caches the response if needed, thus making sure that next - * reconciliation will see to updated resource - or more recent one if additional update happened - * after this update; In addition to that it filters out the event from this update, so - * reconciliation is not triggered by own update. + * Updates (HTTP PUT) the resource {@code status} subresource, caching the response and filtering + * the own event. Uses the {@link UpdateType#UPDATE_STATUS} default {@link Matcher}. * - *

You are free to control the optimistic locking by setting the resource version in resource - * metadata. - * - * @param resource resource to update - * @return updated resource - * @param resource type + * @param resource the desired resource (with the status to update) + * @param the resource type + * @return the updated resource as returned by the API server */ public R updateStatus(R resource) { - return resourcePatch(resource, r -> context.getClient().resource(r).updateStatus()); + return updateStatus( + resource, Options.matchAndFilterWithDefaultMatcher(UpdateType.UPDATE_STATUS)); } /** - * Updates the primary resource. + * Updates (HTTP PUT) the resource {@code status} subresource, controlling caching and own-event + * handling through the given {@link Options}. * - *

Updates the resource and caches the response if needed, thus making sure that next - * reconciliation will see to updated resource - or more recent one if additional update happened - * after this update; In addition to that it filters out the event from this update, so - * reconciliation is not triggered by own update. + * @param resource the desired resource (with the status to update) + * @param options controls caching and own-event filtering; see {@link Options} + * @param the resource type + * @return the updated resource as returned by the API server + */ + public R updateStatus(R resource, Options options) { + return resourcePatch(resource, r -> context.getClient().resource(r).updateStatus(), options); + } + + /** + * Updates (HTTP PUT) the primary resource, caching and filtering the own event through the + * controller's own event source. Uses the {@link UpdateType#UPDATE} default {@link Matcher}. * - *

You are free to control the optimistic locking by setting the resource version in resource - * metadata. + * @param desired the desired primary resource to update + * @return the updated resource as returned by the API server + */ + public P updatePrimary(P desired) { + return updatePrimary(desired, Options.matchAndFilterWithDefaultMatcher(UpdateType.UPDATE)); + } + + /** + * Updates (HTTP PUT) the primary resource, caching and filtering the own event through the + * controller's own event source and using the given {@link Options}. * - * @param resource primary resource to update - * @return updated resource + * @param desired the desired primary resource to update + * @param options controls caching and own-event filtering; see {@link Options} + * @return the updated resource as returned by the API server */ - public P updatePrimary(P resource) { + public P updatePrimary(P desired, Options options) { return resourcePatch( - resource, + desired, r -> context.getClient().resource(r).update(), - context.eventSourceRetriever().getControllerEventSource()); + context.eventSourceRetriever().getControllerEventSource(), + options); } /** - * Updates the primary resource status subresource. + * Updates (HTTP PUT) the primary resource {@code status} subresource, caching and filtering the + * own event through the controller's own event source. Uses the {@link UpdateType#UPDATE_STATUS} + * default {@link Matcher}. * - *

Updates the resource and caches the response if needed, thus making sure that next - * reconciliation will see to updated resource - or more recent one if additional update happened - * after this update; In addition to that it filters out the event from this update, so - * reconciliation is not triggered by own update. - * - *

You are free to control the optimistic locking by setting the resource version in resource - * metadata. - * - * @param resource primary resource to update - * @return updated resource + * @param desired the desired primary resource (with the status to update) + * @return the updated resource as returned by the API server */ - public P updatePrimaryStatus(P resource) { + public P updatePrimaryStatus(P desired) { return resourcePatch( - resource, + desired, r -> context.getClient().resource(r).updateStatus(), - context.eventSourceRetriever().getControllerEventSource()); + context.eventSourceRetriever().getControllerEventSource(), + Options.matchAndFilterWithDefaultMatcher(UpdateType.UPDATE_STATUS)); } /** - * Applies a JSON Patch to the resource. The unaryOperator function is used to modify the - * resource, and the differences are sent as a JSON Patch to the Kubernetes API server. + * Applies a JSON Patch (RFC 6902) to the resource: the {@code unaryOperator} mutates the resource + * and the diff is sent as a patch. Caches the response and filters the own event using the {@link + * UpdateType#JSON_PATCH} default {@link Matcher}. Use {@link #jsonPatch(HasMetadata, + * UnaryOperator, Options)} to change this behavior. * - *

Updates the resource and caches the response if needed, thus making sure that next - * reconciliation will see to updated resource - or more recent one if additional update happened - * after this update; In addition to that it filters out the event from this update, so - * reconciliation is not triggered by own update. - * - *

You are free to control the optimistic locking by setting the resource version in resource - * metadata. - * - * @param resource resource to patch - * @param unaryOperator function to modify the resource - * @return updated resource - * @param resource type + * @param actualResource the current resource used as the base of the patch + * @param unaryOperator function that mutates the resource into its desired state + * @param the resource type + * @return the patched resource as returned by the API server */ - public R jsonPatch(R resource, UnaryOperator unaryOperator) { - return resourcePatch(resource, r -> context.getClient().resource(r).edit(unaryOperator)); + public R jsonPatch(R actualResource, UnaryOperator unaryOperator) { + return jsonPatch( + actualResource, + unaryOperator, + Options.matchAndFilterWithDefaultMatcher(UpdateType.JSON_PATCH)); } /** - * Applies a JSON Patch to the resource status subresource. The unaryOperator function is used to - * modify the resource status, and the differences are sent as a JSON Patch. - * - *

Updates the resource and caches the response if needed, thus making sure that next - * reconciliation will see to updated resource - or more recent one if additional update happened - * after this update; In addition to that it filters out the event from this update, so - * reconciliation is not triggered by own update. - * - *

You are free to control the optimistic locking by setting the resource version in resource - * metadata. + * Applies a JSON Patch (RFC 6902) to the resource, controlling caching and own-event handling + * through the given {@link Options}. * - * @param resource resource to patch - * @param unaryOperator function to modify the resource - * @return updated resource - * @param resource type + * @param actualResource the current resource used as the base of the patch + * @param unaryOperator function that mutates the resource into its desired state + * @param options controls caching and own-event filtering; see {@link Options} + * @param the resource type + * @return the patched resource as returned by the API server */ - public R jsonPatchStatus(R resource, UnaryOperator unaryOperator) { - return resourcePatch(resource, r -> context.getClient().resource(r).editStatus(unaryOperator)); + public R jsonPatch( + R actualResource, UnaryOperator unaryOperator, Options options) { + R desired = desiredForJsonPatch(actualResource, unaryOperator, options); + return resourcePatch( + desired, + actualResource, + r -> context.getClient().resource(actualResource).edit(rr -> desired), + options); } /** - * Applies a JSON Patch to the primary resource. + * Applies a JSON Patch (RFC 6902) to the resource, caching and filtering the own event through + * the given {@link InformerEventSource} and using the given {@link Options}. * - *

Updates the resource and caches the response if needed, thus making sure that next - * reconciliation will see to updated resource - or more recent one if additional update happened - * after this update; In addition to that it filters out the event from this update, so - * reconciliation is not triggered by own update. + * @param actualResource the current resource used as the base of the patch + * @param unaryOperator function that mutates the resource into its desired state + * @param informerEventSource the event source used for caching and own-event filtering + * @param options controls caching and own-event filtering; see {@link Options} + * @param the resource type + * @return the patched resource as returned by the API server + */ + public R jsonPatch( + R actualResource, + UnaryOperator unaryOperator, + InformerEventSource informerEventSource, + Options options) { + R desired = desiredForJsonPatch(actualResource, unaryOperator, options); + return resourcePatch( + desired, + actualResource, + r -> context.getClient().resource(actualResource).edit(unaryOperator), + informerEventSource, + options); + } + + /** + * Applies a JSON Patch (RFC 6902) to the resource {@code status} subresource: the {@code + * unaryOperator} mutates the resource status and the diff is sent as a patch. Caches the response + * and filters the own event using the {@link UpdateType#JSON_PATCH_STATUS} default {@link + * Matcher}. * - *

You are free to control the optimistic locking by setting the resource version in resource - * metadata. + * @param actualResource the current resource used as the base of the patch + * @param unaryOperator function that mutates the resource status into its desired state + * @param the resource type + * @return the patched resource as returned by the API server + */ + public R jsonPatchStatus( + R actualResource, UnaryOperator unaryOperator) { + return jsonPatchStatus( + actualResource, + unaryOperator, + Options.matchAndFilterWithDefaultMatcher(UpdateType.JSON_PATCH_STATUS)); + } + + /** + * Applies a JSON Patch (RFC 6902) to the resource {@code status} subresource, controlling caching + * and own-event handling through the given {@link Options}. * - * @param resource primary resource to patch - * @param unaryOperator function to modify the resource - * @return updated resource + * @param actualResource the current resource used as the base of the patch + * @param unaryOperator function that mutates the resource status into its desired state + * @param options controls caching and own-event filtering; see {@link Options} + * @param the resource type + * @return the patched resource as returned by the API server */ - public P jsonPatchPrimary(P resource, UnaryOperator

unaryOperator) { + public R jsonPatchStatus( + R actualResource, UnaryOperator unaryOperator, Options options) { + R desired = desiredForJsonPatch(actualResource, unaryOperator, options); return resourcePatch( - resource, - r -> context.getClient().resource(r).edit(unaryOperator), - context.eventSourceRetriever().getControllerEventSource()); + desired, + actualResource, + r -> context.getClient().resource(actualResource).editStatus(rr -> desired), + options); } /** - * Applies a JSON Patch to the primary resource status subresource. + * Applies a JSON Patch (RFC 6902) to the resource {@code status} subresource, caching and + * filtering the own event through the given {@link InformerEventSource} and using the given + * {@link Options}. * - *

Updates the resource and caches the response if needed, thus making sure that next - * reconciliation will see to updated resource - or more recent one if additional update happened - * after this update; In addition to that it filters out the event from this update, so - * reconciliation is not triggered by own update. + * @param actualResource the current resource used as the base of the patch + * @param unaryOperator function that mutates the resource status into its desired state + * @param informerEventSource the event source used for caching and own-event filtering + * @param options controls caching and own-event filtering; see {@link Options} + * @param the resource type + * @return the patched resource as returned by the API server + */ + public R jsonPatchStatus( + R actualResource, + UnaryOperator unaryOperator, + InformerEventSource informerEventSource, + Options options) { + R desired = desiredForJsonPatch(actualResource, unaryOperator, options); + return resourcePatch( + desired, + actualResource, + r -> context.getClient().resource(actualResource).editStatus(rr -> desired), + informerEventSource, + options); + } + + /** + * Applies a JSON Patch (RFC 6902) to the primary resource, caching and filtering the own event + * through the controller's own event source. Uses the {@link UpdateType#JSON_PATCH} default + * {@link Matcher}. * - *

You are free to control the optimistic locking by setting the resource version in resource - * metadata. + * @param actualResource the current primary resource used as the base of the patch + * @param unaryOperator function that mutates the resource into its desired state + * @return the patched resource as returned by the API server + */ + public P jsonPatchPrimary(P actualResource, UnaryOperator

unaryOperator) { + return jsonPatchPrimary( + actualResource, + unaryOperator, + Options.matchAndFilterWithDefaultMatcher(UpdateType.JSON_PATCH)); + } + + /** + * Applies a JSON Patch (RFC 6902) to the primary resource, caching and filtering the own event + * through the controller's own event source and using the given {@link Options}. * - * @param resource primary resource to patch - * @param unaryOperator function to modify the resource - * @return updated resource + * @param actualResource the current primary resource used as the base of the patch + * @param unaryOperator function that mutates the resource into its desired state + * @param options controls caching and own-event filtering; see {@link Options} + * @return the patched resource as returned by the API server */ - public P jsonPatchPrimaryStatus(P resource, UnaryOperator

unaryOperator) { + public P jsonPatchPrimary(P actualResource, UnaryOperator

unaryOperator, Options options) { + P desired = desiredForJsonPatch(actualResource, unaryOperator, options); return resourcePatch( - resource, - r -> context.getClient().resource(r).editStatus(unaryOperator), - context.eventSourceRetriever().getControllerEventSource()); + desired, + actualResource, + r -> context.getClient().resource(actualResource).edit(rr -> desired), + context.eventSourceRetriever().getControllerEventSource(), + options); } /** - * Applies a JSON Merge Patch to the resource. JSON Merge Patch (RFC 7386) is a simpler patching - * strategy that merges the provided resource with the existing resource on the server. + * Applies a JSON Patch (RFC 6902) to the primary resource {@code status} subresource, caching and + * filtering the own event through the controller's own event source. Uses the {@link + * UpdateType#JSON_PATCH_STATUS} default {@link Matcher}. * - *

Updates the resource and caches the response if needed, thus making sure that next - * reconciliation will see to updated resource - or more recent one if additional update happened - * after this update; In addition to that it filters out the event from this update, so - * reconciliation is not triggered by own update. + * @param actualResource the current primary resource used as the base of the patch + * @param unaryOperator function that mutates the resource status into its desired state + * @return the patched resource as returned by the API server + */ + public P jsonPatchPrimaryStatus(P actualResource, UnaryOperator

unaryOperator) { + return jsonPatchPrimaryStatus( + actualResource, + unaryOperator, + Options.matchAndFilterWithDefaultMatcher(UpdateType.JSON_PATCH_STATUS)); + } + + /** + * Applies a JSON Patch (RFC 6902) to the primary resource {@code status} subresource, caching and + * filtering the own event through the controller's own event source and using the given {@link + * Options}. * - *

You are free to control the optimistic locking by setting the resource version in resource - * metadata. + * @param actualResource the current primary resource used as the base of the patch + * @param unaryOperator function that mutates the resource status into its desired state + * @param options controls caching and own-event filtering; see {@link Options} + * @return the patched resource as returned by the API server + */ + public P jsonPatchPrimaryStatus( + P actualResource, UnaryOperator

unaryOperator, Options options) { + P desired = desiredForJsonPatch(actualResource, unaryOperator, options); + return resourcePatch( + desired, + actualResource, + r -> context.getClient().resource(actualResource).status().edit(unaryOperator), + context.eventSourceRetriever().getControllerEventSource(), + options); + } + + /** + * Applies a JSON Merge Patch (RFC 7386) to the resource, merging the provided resource with the + * one on the server. Caches the response and filters the own event using the {@link + * UpdateType#JSON_MERGE_PATCH} default {@link Matcher}. Use {@link #jsonMergePatch(HasMetadata, + * Options)} to change this behavior. * - * @param resource resource to patch - * @return updated resource - * @param resource type + * @param desired the desired resource to merge-patch + * @param the resource type + * @return the patched resource as returned by the API server */ - public R jsonMergePatch(R resource) { - return resourcePatch(resource, r -> context.getClient().resource(r).patch()); + public R jsonMergePatch(R desired) { + return jsonMergePatch( + desired, Options.matchAndFilterWithDefaultMatcher(UpdateType.JSON_MERGE_PATCH)); } /** - * Applies a JSON Merge Patch to the resource status subresource. + * Applies a JSON Merge Patch (RFC 7386) to the resource, controlling caching and own-event + * handling through the given {@link Options}. * - *

Updates the resource and caches the response if needed, thus making sure that next - * reconciliation will see to updated resource - or more recent one if additional update happened - * after this update; In addition to that it filters out the event from this update, so - * reconciliation is not triggered by own update. + * @param desired the desired resource to merge-patch + * @param options controls caching and own-event filtering; see {@link Options} + * @param the resource type + * @return the patched resource as returned by the API server + */ + public R jsonMergePatch(R desired, Options options) { + return resourcePatch(desired, r -> context.getClient().resource(r).patch(), options); + } + + /** + * Applies a JSON Merge Patch (RFC 7386) to the resource, caching and filtering the own event + * through the given {@link InformerEventSource} and using the given {@link Options}. * - *

You are free to control the optimistic locking by setting the resource version in resource - * metadata. + * @param desired the desired resource to merge-patch + * @param informerEventSource the event source used for caching and own-event filtering + * @param options controls caching and own-event filtering; see {@link Options} + * @param the resource type + * @return the patched resource as returned by the API server + */ + public R jsonMergePatch( + R desired, InformerEventSource informerEventSource, Options options) { + return resourcePatch( + desired, r -> context.getClient().resource(r).patch(), informerEventSource, options); + } + + /** + * Applies a JSON Merge Patch (RFC 7386) to the resource {@code status} subresource, caching the + * response and filtering the own event. Uses the {@link UpdateType#JSON_MERGE_PATCH_STATUS} + * default {@link Matcher}. * - * @param resource resource to patch - * @return updated resource - * @param resource type + * @param resource the desired resource (with the status to merge-patch) + * @param the resource type + * @return the patched resource as returned by the API server */ public R jsonMergePatchStatus(R resource) { - return resourcePatch(resource, r -> context.getClient().resource(r).patchStatus()); + return jsonMergePatchStatus( + resource, Options.matchAndFilterWithDefaultMatcher(UpdateType.JSON_MERGE_PATCH_STATUS)); } /** - * Applies a JSON Merge Patch to the primary resource. Caches the response using the controller's - * event source. + * Applies a JSON Merge Patch (RFC 7386) to the resource {@code status} subresource, controlling + * caching and own-event handling through the given {@link Options}. * - *

Updates the resource and caches the response if needed, thus making sure that next - * reconciliation will see to updated resource - or more recent one if additional update happened - * after this update; In addition to that it filters out the event from this update, so - * reconciliation is not triggered by own update. + * @param resource the desired resource (with the status to merge-patch) + * @param options controls caching and own-event filtering; see {@link Options} + * @param the resource type + * @return the patched resource as returned by the API server + */ + public R jsonMergePatchStatus(R resource, Options options) { + return resourcePatch(resource, r -> context.getClient().resource(r).patchStatus(), options); + } + + /** + * Applies a JSON Merge Patch (RFC 7386) to the resource {@code status} subresource, caching and + * filtering the own event through the given {@link InformerEventSource} and using the given + * {@link Options}. * - *

You are free to control the optimistic locking by setting the resource version in resource - * metadata. + * @param resource the desired resource (with the status to merge-patch) + * @param informerEventSource the event source used for caching and own-event filtering + * @param options controls caching and own-event filtering; see {@link Options} + * @param the resource type + * @return the patched resource as returned by the API server + */ + public R jsonMergePatchStatus( + R resource, InformerEventSource informerEventSource, Options options) { + return resourcePatch( + resource, r -> context.getClient().resource(r).patchStatus(), informerEventSource, options); + } + + /** + * Applies a JSON Merge Patch (RFC 7386) to the primary resource, caching and filtering the own + * event through the controller's own event source. Uses the {@link UpdateType#JSON_MERGE_PATCH} + * default {@link Matcher}. * - * @param resource primary resource to patch reconciliation - * @return updated resource + * @param resource the desired primary resource to merge-patch + * @return the patched resource as returned by the API server */ public P jsonMergePatchPrimary(P resource) { + return jsonMergePatchPrimary( + resource, Options.matchAndFilterWithDefaultMatcher(UpdateType.JSON_MERGE_PATCH)); + } + + /** + * Applies a JSON Merge Patch (RFC 7386) to the primary resource, caching and filtering the own + * event through the controller's own event source and using the given {@link Options}. + * + * @param resource the desired primary resource to merge-patch + * @param options controls caching and own-event filtering; see {@link Options} + * @return the patched resource as returned by the API server + */ + public P jsonMergePatchPrimary(P resource, Options options) { return resourcePatch( resource, r -> context.getClient().resource(r).patch(), - context.eventSourceRetriever().getControllerEventSource()); + context.eventSourceRetriever().getControllerEventSource(), + options); } /** - * Applies a JSON Merge Patch to the primary resource. + * Applies a JSON Merge Patch (RFC 7386) to the primary resource {@code status} subresource, + * caching and filtering the own event through the controller's own event source. Uses the {@link + * UpdateType#JSON_MERGE_PATCH_STATUS} default {@link Matcher}. * - *

Updates the resource and caches the response if needed, thus making sure that next - * reconciliation will see to updated resource - or more recent one if additional update happened - * after this update; In addition to that it filters out the event from this update, so - * reconciliation is not triggered by own update. - * - *

You are free to control the optimistic locking by setting the resource version in resource - * metadata. - * - * @param resource primary resource to patch - * @return updated resource - * @see #jsonMergePatchPrimaryStatus(HasMetadata) + * @param resource the desired primary resource (with the status to merge-patch) + * @return the patched resource as returned by the API server */ public P jsonMergePatchPrimaryStatus(P resource) { + return jsonMergePatchPrimaryStatus( + resource, Options.matchAndFilterWithDefaultMatcher(UpdateType.JSON_MERGE_PATCH_STATUS)); + } + + /** + * Applies a JSON Merge Patch (RFC 7386) to the primary resource {@code status} subresource, + * caching and filtering the own event through the controller's own event source and using the + * given {@link Options}. + * + * @param resource the desired primary resource (with the status to merge-patch) + * @param options controls caching and own-event filtering; see {@link Options} + * @return the patched resource as returned by the API server + */ + public P jsonMergePatchPrimaryStatus(P resource, Options options) { return resourcePatch( resource, r -> context.getClient().resource(r).patchStatus(), - context.eventSourceRetriever().getControllerEventSource()); + context.eventSourceRetriever().getControllerEventSource(), + options); } /** - * Utility method to patch a resource and cache the result. Automatically discovers the event - * source for the resource type and delegates to {@link #resourcePatch(HasMetadata, UnaryOperator, - * ManagedInformerEventSource)}. + * Low-level building block behind all the update/patch operations above: runs the given {@code + * updateOperation} against the API server, then caches the response and (depending on the {@link + * Options}) filters the resulting own event. The target event source is resolved automatically + * from the desired resource's type. Uses {@link Options#filterIfOptimisticLocking()}. * - * @param resource resource to patch - * @param updateOperation operation to perform (update, patch, edit, etc.) - * @return updated resource - * @param resource type - * @throws IllegalStateException if no event source or multiple event sources are found + * @param resource the desired resource being written + * @param updateOperation the actual write to perform (update, patch, edit, ...) returning the + * server response + * @param the resource type + * @return the resource as returned by {@code updateOperation} + * @throws IllegalStateException if no (or no matching) event source is found for the resource + * type */ - @SuppressWarnings({"rawtypes", "unchecked"}) public R resourcePatch(R resource, UnaryOperator updateOperation) { + return resourcePatch(resource, updateOperation, Options.filterIfOptimisticLocking()); + } + + @SuppressWarnings({"rawtypes", "unchecked"}) + private R resourcePatch( + R desired, UnaryOperator updateOperation, Options options) { + return resourcePatch(desired, null, updateOperation, options); + } + + @SuppressWarnings({"rawtypes", "unchecked"}) + private R resourcePatch( + R desired, R actual, UnaryOperator updateOperation, Options options) { - var esList = context.eventSourceRetriever().getEventSourcesFor(resource.getClass()); + Class targetClass = desired == null ? actual.getClass() : desired.getClass(); + + var esList = context.eventSourceRetriever().getEventSourcesFor(targetClass); if (esList.isEmpty()) { - throw new IllegalStateException("No event source found for type: " + resource.getClass()); + throw new IllegalStateException("No event source found for type: " + targetClass); } var es = esList.get(0); if (esList.size() > 1) { log.warn( "Multiple event sources found for type: {}, selecting first with name {}", - resource.getClass(), + targetClass, es.name()); } if (es instanceof ManagedInformerEventSource mes) { - return resourcePatch(resource, updateOperation, (ManagedInformerEventSource) mes); + return resourcePatch( + desired, actual, updateOperation, (ManagedInformerEventSource) mes, options); } else { throw new IllegalStateException( "Target event source must be a subclass off " @@ -553,19 +908,88 @@ public R resourcePatch(R resource, UnaryOperator upda } /** - * Utility method to patch a resource and cache the result using the specified event source. This - * method either filters out the resulting event or allows it to trigger reconciliation based on - * the filterEvent parameter. + * This method is public to ensure backward compatibility, we will make it private in next major + * release. * - * @param resource resource to patch - * @param updateOperation operation to perform (update, patch, edit, etc.) - * @param ies the managed informer event source to use for caching - * @return updated resource - * @param resource type + * @deprecated use one of the higher-level update/patch methods, or {@link #resourcePatch( + * HasMetadata, HasMetadata, UnaryOperator, ManagedInformerEventSource, Options)} */ + @Deprecated(forRemoval = true) public R resourcePatch( - R resource, UnaryOperator updateOperation, ManagedInformerEventSource ies) { - return ies.eventFilteringUpdateAndCacheResource(resource, updateOperation); + R desired, UnaryOperator updateOperation, ManagedInformerEventSource ies) { + return resourcePatch(desired, updateOperation, ies, Options.filterIfOptimisticLocking()); + } + + private R resourcePatch( + R desired, + UnaryOperator updateOperation, + ManagedInformerEventSource ies, + Options options) { + + return resourcePatch(desired, null, updateOperation, ies, options); + } + + /** + * The core update/patch routine used by all operations. Optionally matches the desired against + * the actual (cached) state, performs the write through {@code updateOperation}, and caches the + * response while filtering the own event according to the {@link Options}/{@link Mode}. + * + *

When a {@link Matcher} is present but {@code actualResource} is {@code null}, the actual + * state is looked up from the given event source's cache. If the states already match, the write + * is skipped and the actual resource is returned unchanged. + * + * @param desiredResource the desired resource; may be {@code null} for JSON Patch operations, + * where {@code actualResource} is used as the base instead + * @param actualResource the actual (current) resource used for matching; may be {@code null} + * @param updateOperation the actual write to perform, returning the server response + * @param ies the event source used for caching and own-event filtering + * @param options controls matching, caching and own-event filtering; see {@link Options} + * @param the resource type + * @return the resource as returned by {@code updateOperation}, or {@code actualResource} when the + * desired state already matches + * @throws IllegalArgumentException if the mode requires a matcher but none is provided + */ + // visible for testing + R resourcePatch( + R desiredResource, + R actualResource, + UnaryOperator updateOperation, + ManagedInformerEventSource ies, + Options options) { + + var matcher = options.getMatcher().orElse(null); + boolean matches = false; + if (matcher != null) { + if (actualResource == null) { + actualResource = ies.get(ResourceID.fromResource(desiredResource)).orElse(null); + } + if (actualResource != null) { + matches = matcher.matches(desiredResource, actualResource, context); + } + } + if (options.requiresMatcher() && matcher == null) { + throw new IllegalArgumentException("Mode : " + options.mode + " requires matcher"); + } + // this is to cover special case for jsonPatch were we should use actual resource as base + if (matches) { + if (log.isDebugEnabled()) { + log.debug( + "Resource match resource id: {}, type: {}, version: {}", + ResourceID.fromResource(desiredResource), + desiredResource.getClass().getSimpleName(), + desiredResource.getMetadata().getResourceVersion()); + } + return actualResource; + } + + boolean optimisticLocking = desiredResource.getMetadata().getResourceVersion() != null; + + if (options.getMode() == Mode.CACHE_ONLY + || (options.getMode() == Mode.FILTER_IF_OPTIMISTIC_LOCKING && !optimisticLocking)) { + return ies.updateAndCacheResource(desiredResource, updateOperation); + } else { + return ies.eventFilteringUpdateAndCacheResource(desiredResource, updateOperation); + } } /** @@ -578,7 +1002,33 @@ public R resourcePatch( * @see #addFinalizer(String) */ public P addFinalizer() { - return addFinalizer(context.getControllerConfiguration().getFinalizerName()); + return addFinalizer(context.getControllerConfiguration().getFinalizerName(), false); + } + + /** + * Adds the default finalizer (from controller configuration) to the primary resource. This is a + * convenience method that calls {@link #addFinalizer(String, boolean)} with the configured + * finalizer name. + * + * @param cacheOnly if {@code true} the finalizer patch does not filter its own event (see {@link + * #addFinalizer(String, boolean)}) + * @return updated resource from the server response + * @see #addFinalizer(String, boolean) + */ + public P addFinalizer(boolean cacheOnly) { + return addFinalizer(context.getControllerConfiguration().getFinalizerName(), cacheOnly); + } + + /** + * Adds the given finalizer to the primary resource, filtering the own event (equivalent to {@link + * #addFinalizer(String, boolean)} with {@code cacheOnly = false}). + * + * @param finalizerName name of the finalizer to add + * @return updated resource from the server response + * @see #addFinalizer(String, boolean) + */ + public P addFinalizer(String finalizerName) { + return addFinalizer(finalizerName, false); } /** @@ -587,9 +1037,14 @@ public P addFinalizer() { * marked for deletion. Note that explicitly adding/removing finalizer is required only if * "Trigger reconciliation on all event" mode is on. * + * @param finalizerName name of the finalizer to add + * @param cacheOnly if {@code true} the finalizer patch only caches the response and does + * not filter the resulting own event, so a subsequent reconciliation is triggered by + * the finalizer addition; if {@code false} the default JSON Patch matcher is used and the own + * event is filtered * @return updated resource from the server response */ - public P addFinalizer(String finalizerName) { + public P addFinalizer(String finalizerName, boolean cacheOnly) { var resource = context.getPrimaryResource(); if (resource.isMarkedForDeletion() || resource.hasFinalizer(finalizerName)) { return resource; @@ -599,7 +1054,8 @@ public P addFinalizer(String finalizerName) { r.addFinalizer(finalizerName); return r; }, - r -> !r.hasFinalizer(finalizerName)); + r -> !r.hasFinalizer(finalizerName), + cacheOnly); } /** @@ -621,6 +1077,7 @@ public P removeFinalizer() { * explicitly adding/removing finalizer is required only if "Trigger reconciliation on all event" * mode is on. * + * @param finalizerName name of the finalizer to remove * @return updated resource from the server response */ public P removeFinalizer(String finalizerName) { @@ -643,18 +1100,40 @@ public P removeFinalizer(String finalizerName) { } /** - * Patches the resource using JSON Patch. In case the server responds with conflict (HTTP 409) or - * unprocessable content (HTTP 422) it retries the operation up to the maximum number defined in - * {@link ResourceOperations#DEFAULT_MAX_RETRY}. + * Patches the primary resource using JSON Patch, retrying on conflict, filtering the own event + * via the default JSON Patch matcher (equivalent to {@link + * #conflictRetryingPatchPrimary(UnaryOperator, Predicate, boolean)} with {@code cacheOnly = + * false}). * * @param resourceChangesOperator changes to be done on the resource before update * @param preCondition condition to check if the patch operation still needs to be performed or - * not. - * @return updated resource from the server or unchanged if the precondition does not hold. + * not + * @return updated resource from the server or unchanged if the precondition does not hold */ - @SuppressWarnings("unchecked") public P conflictRetryingPatchPrimary( UnaryOperator

resourceChangesOperator, Predicate

preCondition) { + return conflictRetryingPatchPrimary(resourceChangesOperator, preCondition, false); + } + + /** + * Patches the primary resource using JSON Patch, retrying on conflict. The {@code + * resourceChangesOperator} is applied to the current resource before each attempt; if the server + * responds with conflict (HTTP 409) or unprocessable content (HTTP 422) the latest version is + * re-fetched and the operation is retried, up to {@link ResourceOperations#DEFAULT_MAX_RETRY} + * times. + * + * @param resourceChangesOperator changes to be done on the resource before update + * @param preCondition condition to check if the patch operation still needs to be performed or + * not; evaluated against the (possibly re-fetched) resource before each attempt + * @param cacheOnly if {@code true} the patch only caches the response and does not + * filter the resulting own event; if {@code false} the default JSON Patch matcher is used and + * the own event is filtered + * @return updated resource from the server or unchanged if the precondition does not hold + * @throws OperatorException if the maximum number of retry attempts is exceeded + */ + @SuppressWarnings("unchecked") + public P conflictRetryingPatchPrimary( + UnaryOperator

resourceChangesOperator, Predicate

preCondition, boolean cacheOnly) { var resource = context.getPrimaryResource(); var client = context.getClient(); if (log.isDebugEnabled()) { @@ -666,7 +1145,12 @@ public P conflictRetryingPatchPrimary( if (!preCondition.test(resource)) { return resource; } - return jsonPatchPrimary(resource, resourceChangesOperator); + return jsonPatchPrimary( + resource, + resourceChangesOperator, + cacheOnly + ? Options.cacheOnly() + : Options.matchAndFilterWithDefaultMatcher(UpdateType.JSON_PATCH)); } catch (KubernetesClientException e) { log.trace("Exception during patch for resource: {}", resource); retryIndex++; @@ -742,7 +1226,7 @@ public P addFinalizerWithSSA(String finalizerName) { resource.initNameAndNamespaceFrom(originalResource); resource.addFinalizer(finalizerName); - return serverSideApplyPrimary(resource); + return serverSideApplyPrimary(resource, Options.cacheOnly()); } catch (InstantiationException | IllegalAccessException | InvocationTargetException @@ -754,4 +1238,154 @@ public P addFinalizerWithSSA(String finalizerName) { e); } } + + /** + * Controls how an update/patch/create operation of {@link ResourceOperations} handles the own + * event resulting from the write. This is the entry point users interact with to tune caching and + * event filtering; instances are created through the static factory methods rather than a + * constructor, and each maps to a {@link Mode}. + * + *

See the {@link ResourceOperations} class documentation for a full description of the + * available strategies, their correctness requirements (a {@link Matcher} or optimistic locking + * is needed for safe own-event filtering), and the trade-offs of the default matchers. + * + *

    + *
  • {@link #filterIfOptimisticLocking()} - filter the own event only when the write uses + * optimistic locking, otherwise only cache the response. + *
  • {@link #matchAndFilter(Matcher)} / {@link #matchAndFilterWithDefaultMatcher(UpdateType)} + * - skip the write when the desired state already matches the actual state, otherwise write + * and filter the own event. + *
  • {@link #cacheOnly()} / {@link #cacheOnly(Matcher)} - only cache the response, no + * own-event filtering. + *
  • {@link #forceFilterEvents()} - always filter the own event (mostly for internal usage). + *
+ */ + @Experimental(API_MIGHT_CHANGE) + public static class Options { + + private static final Options ALWAYS_FILTER = new Options(Mode.FORCE_FILTER, null); + private static final Options ONLY_CACHE = new Options(Mode.CACHE_ONLY, null); + private static final Options FILTER_IF_OPTIMISTIC_LOCKING = + new Options(Mode.FILTER_IF_OPTIMISTIC_LOCKING, null); + + private final Mode mode; + private final Matcher matcher; + + private Options(Mode mode, Matcher matcher) { + this.mode = mode; + this.matcher = matcher; + } + + /** + * Always filters the own event resulting from the write, regardless of optimistic locking. Safe + * only when correctness is otherwise guaranteed (see {@link ResourceOperations}); mostly for + * internal usage. + * + * @return options that always filter the own event + */ + public static Options forceFilterEvents() { + return ALWAYS_FILTER; + } + + /** + * Only caches the response of the write for read-cache-after-write consistency, without doing + * any own-event filtering. + * + * @return options that only cache the response + */ + public static Options cacheOnly() { + return ONLY_CACHE; + } + + /** + * Like {@link #cacheOnly()} but additionally skips the write when the desired state already + * matches the actual (cached) state according to the given {@link Matcher}. + * + * @param matcher the matcher used to decide whether the actual state already matches the + * desired + * @return options that cache only, skipping the write when already matching + */ + public static Options cacheOnly(Matcher matcher) { + return new Options(Mode.CACHE_ONLY, matcher); + } + + /** + * Filters the own event only when the write uses optimistic locking (a resource version is set + * on the written resource); otherwise only caches the response. This is the safe default for + * plain update/patch operations. + * + * @return options that filter the own event only when optimistic locking is used + */ + public static Options filterIfOptimisticLocking() { + return FILTER_IF_OPTIMISTIC_LOCKING; + } + + /** + * Filters own updates and, before doing so, checks whether the actual resource already matches + * the desired state using the given {@link Matcher}. + * + * @param matcher the matcher used to decide whether actual already matches desired + * @return options that match using the given matcher and filter the own event + */ + public static Options matchAndFilter(Matcher matcher) { + if (matcher == null) { + throw new IllegalArgumentException("Matcher cannot be null for matchAndFilterOperation"); + } + return new Options(Mode.FILTER_IF_NOT_MATCHING, matcher); + } + + /** + * Same as {@link #matchAndFilter(Matcher)} but uses the default {@link Matcher} registered for + * the given {@link UpdateType}. See the {@link ResourceOperations} class documentation for the + * caveats of the default matchers. + * + * @param updateType the update type whose default matcher should be used + * @return options that match using the default matcher and filter the own event + */ + public static Options matchAndFilterWithDefaultMatcher(UpdateType updateType) { + return new Options(Mode.FILTER_IF_NOT_MATCHING, updateType.getMatcher()); + } + + public Mode getMode() { + return mode; + } + + public Optional getMatcher() { + return Optional.ofNullable(matcher); + } + + public boolean requiresMatcher() { + return mode == Mode.FILTER_IF_NOT_MATCHING; + } + } + + /** + * The strategy used to decide how the own event resulting from a write is handled. This is a + * low-level enum; users should not reference it directly but select the desired behavior through + * the {@link Options} factory methods (e.g. {@link Options#filterIfOptimisticLocking()}, {@link + * Options#matchAndFilter(Matcher)}, {@link Options#cacheOnly()}, {@link + * Options#forceFilterEvents()}), which is why each constant links to its corresponding factory. + */ + @Experimental(API_MIGHT_CHANGE) + enum Mode { + /** See {@link Options#filterIfOptimisticLocking()}. */ + FILTER_IF_OPTIMISTIC_LOCKING, + /** See {@link Options#matchAndFilter(Matcher)}. */ + FILTER_IF_NOT_MATCHING, + /** See {@link Options#cacheOnly()}. */ + CACHE_ONLY, + /** See {@link Options#forceFilterEvents()}. */ + FORCE_FILTER, + } + + private T desiredForJsonPatch( + T actualResource, UnaryOperator unaryOperator, Options options) { + var cloned = + context + .getControllerConfiguration() + .getConfigurationService() + .getResourceCloner() + .clone(actualResource); + return unaryOperator.apply(cloned); + } } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/matcher/Matcher.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/matcher/Matcher.java new file mode 100644 index 0000000000..5da71ea8eb --- /dev/null +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/matcher/Matcher.java @@ -0,0 +1,28 @@ +/* + * Copyright Java Operator SDK Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.javaoperatorsdk.operator.api.reconciler.matcher; + +import io.fabric8.kubernetes.api.model.HasMetadata; +import io.javaoperatorsdk.operator.api.reconciler.Context; + +public interface Matcher { + + /** + * Matcher if the desired state matches the actual state in respective to underlying update/patch + * method + */ + boolean matches(HasMetadata desired, HasMetadata actual, Context context); +} diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/GenericKubernetesResourceMatcher.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/GenericKubernetesResourceMatcher.java index 5562c883e2..b5a0728e16 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/GenericKubernetesResourceMatcher.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/GenericKubernetesResourceMatcher.java @@ -20,6 +20,7 @@ import java.util.List; import io.fabric8.kubernetes.api.model.HasMetadata; +import io.fabric8.zjsonpatch.DiffFlags; import io.fabric8.zjsonpatch.JsonDiff; import io.javaoperatorsdk.operator.api.reconciler.Context; import io.javaoperatorsdk.operator.processing.dependent.Matcher; @@ -29,6 +30,7 @@ public class GenericKubernetesResourceMatcher { private static final String SPEC = "/spec"; + private static final String STATUS = "/status"; private static final String METADATA = "/metadata"; private static final String ADD = "add"; private static final String OP = "op"; @@ -195,6 +197,58 @@ public static Matcher.Result m return Matcher.Result.computed(matched, desired); } + /** + * Determines whether the {@code status} subresource of the specified actual resource matches the + * {@code status} of the specified desired resource. Unlike {@link #match(HasMetadata, + * HasMetadata, Context)}, which ignores the status, this method considers only the + * {@code /status} subtree; all other differences (spec, metadata, ...) are ignored. + * + * @param desired the desired resource + * @param actualResource the actual resource + * @param context the {@link Context} instance within which this method is called + * @param resource + * @return results of matching + */ + public static Matcher.Result matchStatus( + R desired, R actualResource, Context

context) { + return matchStatus(desired, actualResource, false, context); + } + + /** + * Determines whether the {@code status} subresource of the specified actual resource matches the + * {@code status} of the specified desired resource. Unlike {@link #match(HasMetadata, + * HasMetadata, Context)}, which ignores the status, this method considers only the + * {@code /status} subtree; all other differences (spec, metadata, ...) are ignored. + * + * @param desired the desired resource + * @param actualResource the actual resource + * @param valuesEquality if {@code false}, the algorithm only checks that the values present in + * the desired status are the same in the actual status, allowing the actual status to contain + * additional values (for example defaults added by the server). If {@code true}, the statuses + * match only if they are fully equal. + * @param context the {@link Context} instance within which this method is called + * @param resource + * @return results of matching + */ + public static Matcher.Result matchStatus( + R desired, R actualResource, boolean valuesEquality, Context

context) { + final var kubernetesSerialization = context.getClient().getKubernetesSerialization(); + var desiredNode = kubernetesSerialization.convertValue(desired, JsonNode.class); + var actualNode = kubernetesSerialization.convertValue(actualResource, JsonNode.class); + var wholeDiffJsonPatch = + JsonDiff.asJson(desiredNode, actualNode, DiffFlags.dontNormalizeOpIntoMoveAndCopy()); + + boolean matched = true; + for (int i = 0; i < wholeDiffJsonPatch.size() && matched; i++) { + var node = wholeDiffJsonPatch.get(i); + if (nodeIsChildOf(node, List.of(STATUS))) { + matched = match(valuesEquality, node, Collections.emptyList()); + } + } + + return Matcher.Result.computed(matched, desired); + } + private static boolean match(boolean equality, JsonNode diff, final List ignoreList) { if (equality) { return false; diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/KubernetesDependentResource.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/KubernetesDependentResource.java index f8d7c07b01..bb59d6eed6 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/KubernetesDependentResource.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/KubernetesDependentResource.java @@ -30,6 +30,7 @@ import io.javaoperatorsdk.operator.api.reconciler.Context; import io.javaoperatorsdk.operator.api.reconciler.EventSourceContext; import io.javaoperatorsdk.operator.api.reconciler.Ignore; +import io.javaoperatorsdk.operator.api.reconciler.ResourceOperations; import io.javaoperatorsdk.operator.api.reconciler.dependent.GarbageCollected; import io.javaoperatorsdk.operator.api.reconciler.dependent.managed.ConfiguredDependentResource; import io.javaoperatorsdk.operator.processing.GroupVersionKind; @@ -90,10 +91,14 @@ public R create(R desired, P primary, Context

context) { desired.getClass(), ResourceID.fromResource(desired), ssa); - return ssa ? context.resourceOperations().serverSideApply(desired, eventSource().orElse(null)) - : context.resourceOperations().create(desired, eventSource().orElse(null)); + : context + .resourceOperations() + .create( + desired, + eventSource().orElse(null), + ResourceOperations.Options.forceFilterEvents()); } public R update(R actual, R desired, P primary, Context

context) { @@ -114,7 +119,12 @@ public R update(R actual, R desired, P primary, Context

context) { ssa); if (ssa) { updatedResource = - context.resourceOperations().serverSideApply(desired, eventSource().orElse(null)); + context + .resourceOperations() + .serverSideApply( + desired, + eventSource().orElse(null), + ResourceOperations.Options.forceFilterEvents()); } else { var updatedActual = GenericResourceUpdater.updateResource(actual, desired, context); updatedResource = diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/ReconciliationDispatcher.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/ReconciliationDispatcher.java index 6e7ace0447..dfdf37d3e1 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/ReconciliationDispatcher.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/ReconciliationDispatcher.java @@ -31,6 +31,7 @@ import io.javaoperatorsdk.operator.api.reconciler.Context; import io.javaoperatorsdk.operator.api.reconciler.DefaultContext; import io.javaoperatorsdk.operator.api.reconciler.DeleteControl; +import io.javaoperatorsdk.operator.api.reconciler.ResourceOperations; import io.javaoperatorsdk.operator.api.reconciler.RetryInfo; import io.javaoperatorsdk.operator.api.reconciler.UpdateControl; import io.javaoperatorsdk.operator.processing.Controller; @@ -136,10 +137,9 @@ private PostExecutionControl

handleReconcile( if (useSSA) { updatedResource = context.resourceOperations().addFinalizerWithSSA(); } else { - updatedResource = context.resourceOperations().addFinalizer(); + updatedResource = context.resourceOperations().addFinalizer(true); } - return PostExecutionControl.onlyFinalizerAdded(updatedResource) - .withReSchedule(BaseControl.INSTANT_RESCHEDULE); + return PostExecutionControl.onlyFinalizerAdded(updatedResource); } else { try { return reconcileExecution(executionScope, resourceForExecution, originalResource, context); @@ -366,9 +366,14 @@ public R patchResource(Context context, R resource, R originalResource) { log.debug("Trying to replace resource"); } if (useSSA) { - return context.resourceOperations().serverSideApplyPrimary(resource); + return context + .resourceOperations() + .serverSideApplyPrimary(resource, ResourceOperations.Options.cacheOnly()); } else { - return context.resourceOperations().jsonPatchPrimary(originalResource, r -> resource); + return context + .resourceOperations() + .jsonPatchPrimary( + originalResource, r -> resource, ResourceOperations.Options.cacheOnly()); } } @@ -378,7 +383,9 @@ public R patchStatus(Context context, R resource, R originalResource) { var managedFields = resource.getMetadata().getManagedFields(); try { resource.getMetadata().setManagedFields(null); - return context.resourceOperations().serverSideApplyPrimaryStatus(resource); + return context + .resourceOperations() + .serverSideApplyPrimaryStatus(resource, ResourceOperations.Options.cacheOnly()); } finally { resource.getMetadata().setManagedFields(managedFields); } @@ -402,7 +409,8 @@ private R editStatus(Context context, R resource, R originalResource) { r -> { ReconcilerUtilsInternal.setStatus(r, ReconcilerUtilsInternal.getStatus(resource)); return r; - }); + }, + ResourceOperations.Options.cacheOnly()); } finally { // restore initial resource version clonedOriginal.getMetadata().setResourceVersion(resourceVersion); diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/ManagedInformerEventSource.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/ManagedInformerEventSource.java index 5a239a7377..8352bef665 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/ManagedInformerEventSource.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/ManagedInformerEventSource.java @@ -39,6 +39,7 @@ import io.javaoperatorsdk.operator.api.config.ControllerConfiguration; import io.javaoperatorsdk.operator.api.config.Informable; import io.javaoperatorsdk.operator.api.config.NamespaceChangeable; +import io.javaoperatorsdk.operator.api.reconciler.Experimental; import io.javaoperatorsdk.operator.api.reconciler.dependent.RecentOperationCacheFiller; import io.javaoperatorsdk.operator.health.InformerHealthIndicator; import io.javaoperatorsdk.operator.health.InformerWrappingEventSourceHealthIndicator; @@ -48,6 +49,8 @@ import io.javaoperatorsdk.operator.processing.event.source.*; import io.javaoperatorsdk.operator.processing.event.source.ResourceAction; +import static io.javaoperatorsdk.operator.api.reconciler.Experimental.API_MIGHT_CHANGE; + @SuppressWarnings("rawtypes") public abstract class ManagedInformerEventSource< R extends HasMetadata, P extends HasMetadata, C extends Informable> @@ -93,11 +96,13 @@ public void changeNamespaces(Set namespaces) { * Also makes sure that the even produced by this update is filtered, thus does not trigger the * reconciliation. */ + @Experimental(API_MIGHT_CHANGE) @SuppressWarnings("unchecked") public R eventFilteringUpdateAndCacheResource(R resourceToUpdate, UnaryOperator updateMethod) { + ResourceID id = ResourceID.fromResource(resourceToUpdate); - log.debug("Starting event filtering and caching update for id={}", id); - R updatedResource = null; + log.debug("Starting event filtering and caching update for id: {}", id); + R updatedResource; Set relatedPrimaryIds = null; try { temporaryResourceCache.startEventFilteringModify(id); @@ -134,6 +139,19 @@ public R eventFilteringUpdateAndCacheResource(R resourceToUpdate, UnaryOperator< } } + @Experimental(API_MIGHT_CHANGE) + public R updateAndCacheResource(R resourceToUpdate, UnaryOperator updateOperation) { + if (log.isDebugEnabled()) { + log.debug( + "Updating and caching resource without filtering. id: {} type: {}", + ResourceID.fromResource(resourceToUpdate), + resourceType()); + } + var result = updateOperation.apply(resourceToUpdate); + handleRecentResourceUpdate(ResourceID.fromResource(resourceToUpdate), result, resourceToUpdate); + return result; + } + protected abstract void handleEvent( ResourceAction action, R resource, diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/matcher/JsonMergePatchMatcher.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/matcher/JsonMergePatchMatcher.java new file mode 100644 index 0000000000..6b2ac586df --- /dev/null +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/matcher/JsonMergePatchMatcher.java @@ -0,0 +1,37 @@ +/* + * Copyright Java Operator SDK Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.javaoperatorsdk.operator.processing.matcher; + +import io.fabric8.kubernetes.api.model.HasMetadata; +import io.javaoperatorsdk.operator.api.reconciler.Context; +import io.javaoperatorsdk.operator.api.reconciler.matcher.Matcher; + +public class JsonMergePatchMatcher implements Matcher { + + private static final JsonMergePatchMatcher INSTANCE = new JsonMergePatchMatcher(); + + public static JsonMergePatchMatcher getInstance() { + return INSTANCE; + } + + private JsonMergePatchMatcher() {} + + @Override + public boolean matches(HasMetadata desired, HasMetadata actual, Context context) { + return MatcherUtils.mergePatchMatches( + MatcherUtils.toNode(actual, context), MatcherUtils.toNode(desired, context)); + } +} diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/matcher/JsonMergePatchStatusMatcher.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/matcher/JsonMergePatchStatusMatcher.java new file mode 100644 index 0000000000..49be36c80e --- /dev/null +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/matcher/JsonMergePatchStatusMatcher.java @@ -0,0 +1,37 @@ +/* + * Copyright Java Operator SDK Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.javaoperatorsdk.operator.processing.matcher; + +import io.fabric8.kubernetes.api.model.HasMetadata; +import io.javaoperatorsdk.operator.api.reconciler.Context; +import io.javaoperatorsdk.operator.api.reconciler.matcher.Matcher; + +public class JsonMergePatchStatusMatcher implements Matcher { + + private static final JsonMergePatchStatusMatcher INSTANCE = new JsonMergePatchStatusMatcher(); + + public static JsonMergePatchStatusMatcher getInstance() { + return INSTANCE; + } + + private JsonMergePatchStatusMatcher() {} + + @Override + public boolean matches(HasMetadata desired, HasMetadata actual, Context context) { + return MatcherUtils.mergePatchMatches( + MatcherUtils.statusNode(actual, context), MatcherUtils.statusNode(desired, context)); + } +} diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/matcher/JsonPatchMatcher.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/matcher/JsonPatchMatcher.java new file mode 100644 index 0000000000..7ac65be9a6 --- /dev/null +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/matcher/JsonPatchMatcher.java @@ -0,0 +1,37 @@ +/* + * Copyright Java Operator SDK Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.javaoperatorsdk.operator.processing.matcher; + +import io.fabric8.kubernetes.api.model.HasMetadata; +import io.javaoperatorsdk.operator.api.reconciler.Context; +import io.javaoperatorsdk.operator.api.reconciler.matcher.Matcher; + +public class JsonPatchMatcher implements Matcher { + + private static final JsonPatchMatcher INSTANCE = new JsonPatchMatcher(); + + public static JsonPatchMatcher getInstance() { + return INSTANCE; + } + + private JsonPatchMatcher() {} + + @Override + public boolean matches(HasMetadata desired, HasMetadata actual, Context context) { + return MatcherUtils.jsonPatchMatches( + MatcherUtils.toNode(actual, context), MatcherUtils.toNode(desired, context)); + } +} diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/matcher/JsonPatchStatusMatcher.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/matcher/JsonPatchStatusMatcher.java new file mode 100644 index 0000000000..2fcc1aebe4 --- /dev/null +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/matcher/JsonPatchStatusMatcher.java @@ -0,0 +1,37 @@ +/* + * Copyright Java Operator SDK Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.javaoperatorsdk.operator.processing.matcher; + +import io.fabric8.kubernetes.api.model.HasMetadata; +import io.javaoperatorsdk.operator.api.reconciler.Context; +import io.javaoperatorsdk.operator.api.reconciler.matcher.Matcher; + +public class JsonPatchStatusMatcher implements Matcher { + + private static final JsonPatchStatusMatcher INSTANCE = new JsonPatchStatusMatcher(); + + public static JsonPatchStatusMatcher getInstance() { + return INSTANCE; + } + + private JsonPatchStatusMatcher() {} + + @Override + public boolean matches(HasMetadata desired, HasMetadata actual, Context context) { + return MatcherUtils.jsonPatchMatches( + MatcherUtils.statusNode(actual, context), MatcherUtils.statusNode(desired, context)); + } +} diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/matcher/MatcherUtils.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/matcher/MatcherUtils.java new file mode 100644 index 0000000000..413a1c4acc --- /dev/null +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/matcher/MatcherUtils.java @@ -0,0 +1,83 @@ +/* + * Copyright Java Operator SDK Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.javaoperatorsdk.operator.processing.matcher; + +import io.fabric8.kubernetes.api.model.HasMetadata; +import io.fabric8.zjsonpatch.JsonDiff; +import io.javaoperatorsdk.operator.api.reconciler.Context; +import io.javaoperatorsdk.operator.api.reconciler.matcher.Matcher; + +import com.fasterxml.jackson.databind.JsonNode; +import com.fasterxml.jackson.databind.node.JsonNodeFactory; +import com.fasterxml.jackson.databind.node.NullNode; +import com.fasterxml.jackson.databind.node.ObjectNode; + +/** Shared helpers for the client-side patch based {@link Matcher} implementations. */ +final class MatcherUtils { + + private static final String STATUS = "status"; + + private MatcherUtils() {} + + static JsonNode toNode(HasMetadata resource, Context context) { + return context.getClient().getKubernetesSerialization().convertValue(resource, JsonNode.class); + } + + /** The {@code status} subresource as a node, or a {@link NullNode} if it is not present. */ + static JsonNode statusNode(HasMetadata resource, Context context) { + var status = toNode(resource, context).get(STATUS); + return status == null ? NullNode.getInstance() : status; + } + + /** + * @return {@code true} if applying the desired state as a JSON Patch (RFC 6902) to the actual + * state would be a no-op, i.e. the computed patch contains no operations. + */ + static boolean jsonPatchMatches(JsonNode actual, JsonNode desired) { + return JsonDiff.asJson(actual, desired).isEmpty(); + } + + /** + * @return {@code true} if applying the desired state as a JSON Merge Patch (RFC 7386) to the + * actual state would be a no-op, i.e. every value present in the desired state already equals + * the actual state (additional values only present in the actual state are allowed). + */ + static boolean mergePatchMatches(JsonNode actual, JsonNode desired) { + return applyMergePatch(actual.deepCopy(), desired).equals(actual); + } + + // See https://datatracker.ietf.org/doc/html/rfc7386#section-2 + private static JsonNode applyMergePatch(JsonNode target, JsonNode patch) { + if (!patch.isObject()) { + return patch; + } + var targetObject = + target.isObject() ? (ObjectNode) target : JsonNodeFactory.instance.objectNode(); + var fields = patch.fields(); + while (fields.hasNext()) { + var entry = fields.next(); + if (entry.getValue().isNull()) { + targetObject.remove(entry.getKey()); + } else { + var current = targetObject.get(entry.getKey()); + targetObject.set( + entry.getKey(), + applyMergePatch(current == null ? NullNode.getInstance() : current, entry.getValue())); + } + } + return targetObject; + } +} diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/matcher/SSAMatcher.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/matcher/SSAMatcher.java new file mode 100644 index 0000000000..c241ccfad5 --- /dev/null +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/matcher/SSAMatcher.java @@ -0,0 +1,37 @@ +/* + * Copyright Java Operator SDK Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.javaoperatorsdk.operator.processing.matcher; + +import io.fabric8.kubernetes.api.model.HasMetadata; +import io.javaoperatorsdk.operator.api.reconciler.Context; +import io.javaoperatorsdk.operator.api.reconciler.matcher.Matcher; +import io.javaoperatorsdk.operator.processing.dependent.kubernetes.SSABasedGenericKubernetesResourceMatcher; + +public class SSAMatcher implements Matcher { + + private static final SSAMatcher INSTANCE = new SSAMatcher(); + + public static SSAMatcher getInstance() { + return INSTANCE; + } + + private SSAMatcher() {} + + @Override + public boolean matches(HasMetadata desired, HasMetadata actual, Context context) { + return SSABasedGenericKubernetesResourceMatcher.getInstance().matches(actual, desired, context); + } +} diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/matcher/SSAStatusMatcher.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/matcher/SSAStatusMatcher.java new file mode 100644 index 0000000000..a6a59459f5 --- /dev/null +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/matcher/SSAStatusMatcher.java @@ -0,0 +1,37 @@ +/* + * Copyright Java Operator SDK Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.javaoperatorsdk.operator.processing.matcher; + +import io.fabric8.kubernetes.api.model.HasMetadata; +import io.javaoperatorsdk.operator.api.reconciler.Context; +import io.javaoperatorsdk.operator.api.reconciler.matcher.Matcher; +import io.javaoperatorsdk.operator.processing.dependent.kubernetes.GenericKubernetesResourceMatcher; + +public class SSAStatusMatcher implements Matcher { + + private static final SSAStatusMatcher INSTANCE = new SSAStatusMatcher(); + + public static SSAStatusMatcher getInstance() { + return INSTANCE; + } + + private SSAStatusMatcher() {} + + @Override + public boolean matches(HasMetadata desired, HasMetadata actual, Context context) { + return GenericKubernetesResourceMatcher.matchStatus(desired, actual, context).matched(); + } +} diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/matcher/UpdateMatcher.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/matcher/UpdateMatcher.java new file mode 100644 index 0000000000..b47a4b79d5 --- /dev/null +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/matcher/UpdateMatcher.java @@ -0,0 +1,37 @@ +/* + * Copyright Java Operator SDK Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.javaoperatorsdk.operator.processing.matcher; + +import io.fabric8.kubernetes.api.model.HasMetadata; +import io.javaoperatorsdk.operator.api.reconciler.Context; +import io.javaoperatorsdk.operator.api.reconciler.matcher.Matcher; +import io.javaoperatorsdk.operator.processing.dependent.kubernetes.GenericKubernetesResourceMatcher; + +public class UpdateMatcher implements Matcher { + + private static final UpdateMatcher INSTANCE = new UpdateMatcher(); + + public static UpdateMatcher getInstance() { + return INSTANCE; + } + + private UpdateMatcher() {} + + @Override + public boolean matches(HasMetadata desired, HasMetadata actual, Context context) { + return GenericKubernetesResourceMatcher.match(desired, actual, context).matched(); + } +} diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/matcher/UpdateStatusMatcher.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/matcher/UpdateStatusMatcher.java new file mode 100644 index 0000000000..cc3b045e9e --- /dev/null +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/matcher/UpdateStatusMatcher.java @@ -0,0 +1,37 @@ +/* + * Copyright Java Operator SDK Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.javaoperatorsdk.operator.processing.matcher; + +import io.fabric8.kubernetes.api.model.HasMetadata; +import io.javaoperatorsdk.operator.api.reconciler.Context; +import io.javaoperatorsdk.operator.api.reconciler.matcher.Matcher; +import io.javaoperatorsdk.operator.processing.dependent.kubernetes.GenericKubernetesResourceMatcher; + +public class UpdateStatusMatcher implements Matcher { + + private static final UpdateStatusMatcher INSTANCE = new UpdateStatusMatcher(); + + public static UpdateStatusMatcher getInstance() { + return INSTANCE; + } + + private UpdateStatusMatcher() {} + + @Override + public boolean matches(HasMetadata desired, HasMetadata actual, Context context) { + return GenericKubernetesResourceMatcher.matchStatus(desired, actual, context).matched(); + } +} diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/matcher/UpdateType.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/matcher/UpdateType.java new file mode 100644 index 0000000000..327a9144df --- /dev/null +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/matcher/UpdateType.java @@ -0,0 +1,39 @@ +/* + * Copyright Java Operator SDK Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.javaoperatorsdk.operator.processing.matcher; + +import io.javaoperatorsdk.operator.api.reconciler.matcher.Matcher; + +public enum UpdateType { + UPDATE(UpdateMatcher.getInstance()), + UPDATE_STATUS(UpdateStatusMatcher.getInstance()), + SSA(SSAMatcher.getInstance()), + SSA_STATUS(SSAStatusMatcher.getInstance()), + JSON_PATCH(JsonPatchMatcher.getInstance()), + JSON_PATCH_STATUS(JsonPatchStatusMatcher.getInstance()), + JSON_MERGE_PATCH(JsonMergePatchMatcher.getInstance()), + JSON_MERGE_PATCH_STATUS(JsonMergePatchStatusMatcher.getInstance()); + + private final Matcher defaultMatcher; + + UpdateType(Matcher defaultMatcher) { + this.defaultMatcher = defaultMatcher; + } + + public Matcher getMatcher() { + return defaultMatcher; + } +} diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/reconciler/ResourceOperationsTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/reconciler/ResourceOperationsTest.java index 8d0176cd4a..7cb26ce187 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/reconciler/ResourceOperationsTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/reconciler/ResourceOperationsTest.java @@ -22,12 +22,17 @@ import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import io.fabric8.kubernetes.api.model.HasMetadata; import io.fabric8.kubernetes.client.KubernetesClient; import io.fabric8.kubernetes.client.KubernetesClientException; import io.fabric8.kubernetes.client.dsl.MixedOperation; import io.fabric8.kubernetes.client.dsl.Resource; +import io.fabric8.kubernetes.client.utils.KubernetesSerialization; import io.javaoperatorsdk.operator.TestUtils; +import io.javaoperatorsdk.operator.api.config.Cloner; +import io.javaoperatorsdk.operator.api.config.ConfigurationService; import io.javaoperatorsdk.operator.api.config.ControllerConfiguration; +import io.javaoperatorsdk.operator.api.reconciler.matcher.Matcher; import io.javaoperatorsdk.operator.processing.event.EventSourceRetriever; import io.javaoperatorsdk.operator.processing.event.source.EventSource; import io.javaoperatorsdk.operator.processing.event.source.controller.ControllerEventSource; @@ -63,9 +68,20 @@ void setupMocks() { var eventSourceRetriever = mock(EventSourceRetriever.class); when(context.getClient()).thenReturn(client); + when(client.getKubernetesSerialization()).thenReturn(new KubernetesSerialization()); when(context.eventSourceRetriever()).thenReturn(eventSourceRetriever); when(context.getControllerConfiguration()).thenReturn(controllerConfiguration); when(controllerConfiguration.getFinalizerName()).thenReturn(FINALIZER_NAME); + var configService = mock(ConfigurationService.class); + when(controllerConfiguration.getConfigurationService()).thenReturn(configService); + when(configService.getResourceCloner()) + .thenReturn( + new Cloner() { + @Override + public R clone(R object) { + return new KubernetesSerialization().clone(object); + } + }); when(eventSourceRetriever.getControllerEventSource()).thenReturn(controllerEventSource); when(client.resources(TestCustomResource.class)).thenReturn(mixedOperation); @@ -110,8 +126,7 @@ void addsFinalizerWithSSA() { when(context.getPrimaryResource()).thenReturn(resource); // Mock successful SSA finalizer addition - when(controllerEventSource.eventFilteringUpdateAndCacheResource( - any(), any(UnaryOperator.class))) + when(controllerEventSource.updateAndCacheResource(any(), any(UnaryOperator.class))) .thenAnswer( invocation -> { var res = TestUtils.testCustomResource1(); @@ -125,8 +140,7 @@ void addsFinalizerWithSSA() { assertThat(result).isNotNull(); assertThat(result.hasFinalizer(FINALIZER_NAME)).isTrue(); assertThat(result.getMetadata().getResourceVersion()).isEqualTo("2"); - verify(controllerEventSource, times(1)) - .eventFilteringUpdateAndCacheResource(any(), any(UnaryOperator.class)); + verify(controllerEventSource, times(1)).updateAndCacheResource(any(), any(UnaryOperator.class)); } @Test @@ -324,4 +338,114 @@ void resourcePatchThrowsWhenEventSourceIsNotManagedInformer() { assertThat(exception.getMessage()).contains("Target event source must be a subclass off"); assertThat(exception.getMessage()).contains("ManagedInformerEventSource"); } + + @Test + void matcherMatchingSkipsUpdateAndReturnsActual() { + var desired = TestUtils.testCustomResource1(); + var actual = TestUtils.testCustomResource1(); + var ies = mock(ManagedInformerEventSource.class); + var matcher = mock(Matcher.class); + when(matcher.matches(desired, actual, context)).thenReturn(true); + + var result = + resourceOperations.resourcePatch( + desired, + actual, + UnaryOperator.identity(), + ies, + ResourceOperations.Options.matchAndFilter(matcher)); + + assertThat(result).isSameAs(actual); + verify(ies, never()).eventFilteringUpdateAndCacheResource(any(), any(UnaryOperator.class)); + verify(ies, never()).updateAndCacheResource(any(), any(UnaryOperator.class)); + } + + @Test + void matcherNotMatchingProceedsToEventFilteringUpdate() { + var desired = TestUtils.testCustomResource1(); + var actual = TestUtils.testCustomResource1(); + var updated = TestUtils.testCustomResource1(); + var ies = mock(ManagedInformerEventSource.class); + var matcher = mock(Matcher.class); + when(matcher.matches(desired, actual, context)).thenReturn(false); + when(ies.eventFilteringUpdateAndCacheResource(any(), any(UnaryOperator.class))) + .thenReturn(updated); + + var result = + resourceOperations.resourcePatch( + desired, + actual, + UnaryOperator.identity(), + ies, + ResourceOperations.Options.matchAndFilter(matcher)); + + assertThat(result).isSameAs(updated); + verify(ies, times(1)) + .eventFilteringUpdateAndCacheResource(eq(desired), any(UnaryOperator.class)); + } + + @Test + void onlyCacheModeSkipsEventFiltering() { + var desired = TestUtils.testCustomResource1(); + var ies = mock(ManagedInformerEventSource.class); + when(ies.updateAndCacheResource(any(), any(UnaryOperator.class))).thenReturn(desired); + + resourceOperations.resourcePatch( + desired, null, UnaryOperator.identity(), ies, ResourceOperations.Options.cacheOnly()); + + verify(ies, times(1)).updateAndCacheResource(eq(desired), any(UnaryOperator.class)); + verify(ies, never()).eventFilteringUpdateAndCacheResource(any(), any(UnaryOperator.class)); + } + + @Test + void filterIfOptimisticLockingCachesWithoutFilteringWhenNoResourceVersion() { + var desired = TestUtils.testCustomResource1(); + desired.getMetadata().setResourceVersion(null); + var ies = mock(ManagedInformerEventSource.class); + when(ies.updateAndCacheResource(any(), any(UnaryOperator.class))).thenReturn(desired); + + resourceOperations.resourcePatch( + desired, + null, + UnaryOperator.identity(), + ies, + ResourceOperations.Options.filterIfOptimisticLocking()); + + verify(ies, times(1)).updateAndCacheResource(eq(desired), any(UnaryOperator.class)); + verify(ies, never()).eventFilteringUpdateAndCacheResource(any(), any(UnaryOperator.class)); + } + + @Test + void filterIfOptimisticLockingFiltersWhenResourceVersionPresent() { + var desired = TestUtils.testCustomResource1(); + desired.getMetadata().setResourceVersion("1"); + var ies = mock(ManagedInformerEventSource.class); + when(ies.eventFilteringUpdateAndCacheResource(any(), any(UnaryOperator.class))) + .thenReturn(desired); + + resourceOperations.resourcePatch( + desired, + null, + UnaryOperator.identity(), + ies, + ResourceOperations.Options.filterIfOptimisticLocking()); + + verify(ies, times(1)) + .eventFilteringUpdateAndCacheResource(eq(desired), any(UnaryOperator.class)); + verify(ies, never()).updateAndCacheResource(any(), any(UnaryOperator.class)); + } + + @Test + void createRejectsMatcher() { + var resource = TestUtils.testCustomResource1(); + var matcher = mock(Matcher.class); + + var exception = + assertThrows( + IllegalArgumentException.class, + () -> + resourceOperations.create(resource, ResourceOperations.Options.cacheOnly(matcher))); + + assertThat(exception.getMessage()).contains("does not support matcher"); + } } diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/GenericKubernetesResourceMatcherTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/GenericKubernetesResourceMatcherTest.java index 8dd7283fb9..0c2583d594 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/GenericKubernetesResourceMatcherTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/GenericKubernetesResourceMatcherTest.java @@ -190,6 +190,69 @@ void matchConfigMap() { assertThat(match.matched()).isTrue(); } + @Test + void matchStatusMatchesEqualStatus() { + var desiredWithStatus = createDeployment(); + desiredWithStatus.setStatus(new DeploymentStatusBuilder().withReplicas(1).build()); + var actualWithStatus = createDeployment(); + actualWithStatus.setStatus(new DeploymentStatusBuilder().withReplicas(1).build()); + assertThat( + GenericKubernetesResourceMatcher.matchStatus( + desiredWithStatus, actualWithStatus, context) + .matched()) + .isTrue(); + } + + @Test + void matchStatusIgnoresNonStatusChanges() { + var desiredWithStatus = createDeployment(); + desiredWithStatus.getSpec().setReplicas(5); + desiredWithStatus.setStatus(new DeploymentStatusBuilder().withReplicas(1).build()); + var actualWithStatus = createDeployment(); + actualWithStatus.setStatus(new DeploymentStatusBuilder().withReplicas(1).build()); + assertThat( + GenericKubernetesResourceMatcher.matchStatus( + desiredWithStatus, actualWithStatus, context) + .matched()) + .withFailMessage("Only the status subresource should be considered") + .isTrue(); + } + + @Test + void matchStatusDoesNotMatchDifferentStatus() { + var desiredWithStatus = createDeployment(); + desiredWithStatus.setStatus(new DeploymentStatusBuilder().withReplicas(1).build()); + var actualWithStatus = createDeployment(); + actualWithStatus.setStatus(new DeploymentStatusBuilder().withReplicas(2).build()); + assertThat( + GenericKubernetesResourceMatcher.matchStatus( + desiredWithStatus, actualWithStatus, context) + .matched()) + .isFalse(); + } + + @Test + void matchStatusAllowsAdditionalActualStatusValuesByDefault() { + var desiredWithStatus = createDeployment(); + desiredWithStatus.setStatus(new DeploymentStatusBuilder().withReplicas(1).build()); + var actualWithStatus = createDeployment(); + actualWithStatus.setStatus( + new DeploymentStatusBuilder().withReplicas(1).withAvailableReplicas(1).build()); + assertThat( + GenericKubernetesResourceMatcher.matchStatus( + desiredWithStatus, actualWithStatus, context) + .matched()) + .withFailMessage( + "Additional status values in the actual state should be allowed by default") + .isTrue(); + assertThat( + GenericKubernetesResourceMatcher.matchStatus( + desiredWithStatus, actualWithStatus, true, context) + .matched()) + .withFailMessage("Additional status values should fail when strong equality is required") + .isFalse(); + } + ConfigMap createConfigMap() { return new ConfigMapBuilder() .withMetadata(new ObjectMetaBuilder().withName("tes1").withNamespace("default").build()) diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/ReconciliationDispatcherTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/ReconciliationDispatcherTest.java index c7d9458695..e874731657 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/ReconciliationDispatcherTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/ReconciliationDispatcherTest.java @@ -169,7 +169,7 @@ void addFinalizerOnNewResourceWithoutSSA() throws Exception { dispatcher.handleDispatch(executionScopeWithCREvent(testCustomResource), createTestContext()); verify(reconciler, never()).reconcile(ArgumentMatchers.eq(testCustomResource), any()); - verify(mockResourceOperations, times(1)).addFinalizer(); + verify(mockResourceOperations, times(1)).addFinalizer(true); } @Test diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerEventSourceTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerEventSourceTest.java index c62a1d1a3a..210ce52fcc 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerEventSourceTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerEventSourceTest.java @@ -780,6 +780,27 @@ void filteringUpdateFallsBackToMapperWhenNoPrimaryToSecondaryIndex() { verify(eventHandlerMock, times(1)).handleEvent(any()); } + @Test + void forceUpdateFilterOpensFilterWindowEvenWhenResourceVersionIsNull() { + // A write without a resourceVersion (e.g. an SSA finalizer add, which uses no optimistic + // locking) would normally skip event filtering. forceUpdateFilter=true forces filtering + // anyway so the resulting own event is correlated and does not trigger a spurious + // reconciliation. + var resourceToUpdate = testDeployment(); + resourceToUpdate.getMetadata().setResourceVersion(null); + var updated = deploymentWithResourceVersion(3); + when(temporaryResourceCache.doneEventFilterModify(any())).thenReturn(Optional.empty()); + + var result = + informerEventSource.eventFilteringUpdateAndCacheResource(resourceToUpdate, r -> updated); + + assertThat(result).isSameAs(updated); + verify(temporaryResourceCache, times(1)).startEventFilteringModify(any()); + verify(temporaryResourceCache, times(1)).doneEventFilterModify(any()); + verify(temporaryResourceCache, times(1)).putResource(updated); + verify(eventHandlerMock, never()).handleEvent(any()); + } + private PrimaryToSecondaryIndex injectIndexMock() throws Exception { @SuppressWarnings("unchecked") PrimaryToSecondaryIndex indexMock = mock(PrimaryToSecondaryIndex.class); diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/matcher/PatchMatchersTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/matcher/PatchMatchersTest.java new file mode 100644 index 0000000000..b52d0fc250 --- /dev/null +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/matcher/PatchMatchersTest.java @@ -0,0 +1,169 @@ +/* + * Copyright Java Operator SDK Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.javaoperatorsdk.operator.processing.matcher; + +import org.junit.jupiter.api.Test; + +import io.fabric8.kubernetes.api.model.HasMetadata; +import io.fabric8.kubernetes.api.model.apps.Deployment; +import io.fabric8.kubernetes.api.model.apps.DeploymentBuilder; +import io.fabric8.kubernetes.client.KubernetesClient; +import io.javaoperatorsdk.operator.MockKubernetesClient; +import io.javaoperatorsdk.operator.api.reconciler.Context; +import io.javaoperatorsdk.operator.api.reconciler.DefaultContext; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.mock; + +class PatchMatchersTest { + + private static final Context context = new TestContext(); + + private final JsonPatchMatcher jsonPatchMatcher = JsonPatchMatcher.getInstance(); + private final JsonMergePatchMatcher mergePatchMatcher = JsonMergePatchMatcher.getInstance(); + private final JsonPatchStatusMatcher jsonPatchStatusMatcher = + JsonPatchStatusMatcher.getInstance(); + private final JsonMergePatchStatusMatcher mergePatchStatusMatcher = + JsonMergePatchStatusMatcher.getInstance(); + + // ---- JSON Patch (whole resource) ---- + + @Test + void jsonPatchMatchesIdenticalResources() { + assertThat(jsonPatchMatcher.matches(deployment(), deployment(), context)).isTrue(); + } + + @Test + void jsonPatchDoesNotMatchWhenActualHasAdditionalField() { + var actual = deployment(); + actual.getMetadata().getLabels().put("extra", "value"); + assertThat(jsonPatchMatcher.matches(deployment(), actual, context)).isFalse(); + } + + @Test + void jsonPatchDoesNotMatchWhenDesiredHasAdditionalField() { + var desired = deployment(); + desired.getMetadata().getLabels().put("extra", "value"); + assertThat(jsonPatchMatcher.matches(desired, deployment(), context)).isFalse(); + } + + @Test + void jsonPatchDoesNotMatchOnDifferentValue() { + var desired = deployment(); + desired.getSpec().setReplicas(5); + assertThat(jsonPatchMatcher.matches(desired, deployment(), context)).isFalse(); + } + + // ---- JSON Merge Patch (whole resource) ---- + + @Test + void mergePatchMatchesIdenticalResources() { + assertThat(mergePatchMatcher.matches(deployment(), deployment(), context)).isTrue(); + } + + @Test + void mergePatchMatchesWhenActualHasAdditionalField() { + // a merge patch of the desired state leaves fields only present in the actual state untouched + var actual = deployment(); + actual.getMetadata().getLabels().put("extra", "value"); + assertThat(mergePatchMatcher.matches(deployment(), actual, context)).isTrue(); + } + + @Test + void mergePatchDoesNotMatchWhenDesiredHasAdditionalField() { + var desired = deployment(); + desired.getMetadata().getLabels().put("extra", "value"); + assertThat(mergePatchMatcher.matches(desired, deployment(), context)).isFalse(); + } + + @Test + void mergePatchDoesNotMatchOnDifferentValue() { + var desired = deployment(); + desired.getSpec().setReplicas(5); + assertThat(mergePatchMatcher.matches(desired, deployment(), context)).isFalse(); + } + + // ---- Status matchers ---- + + @Test + void statusMatchersIgnoreChangesOutsideStatus() { + var desired = deploymentWithStatus(); + desired.getSpec().setReplicas(5); + var actual = deploymentWithStatus(); + assertThat(jsonPatchStatusMatcher.matches(desired, actual, context)).isTrue(); + assertThat(mergePatchStatusMatcher.matches(desired, actual, context)).isTrue(); + } + + @Test + void statusMatchersDoNotMatchOnDifferentStatus() { + var desired = deploymentWithStatus(); + desired.getStatus().setReplicas(9); + var actual = deploymentWithStatus(); + assertThat(jsonPatchStatusMatcher.matches(desired, actual, context)).isFalse(); + assertThat(mergePatchStatusMatcher.matches(desired, actual, context)).isFalse(); + } + + @Test + void statusMatchersDifferOnAdditionalActualStatusField() { + var actual = deploymentWithStatus(); + actual.getStatus().setAvailableReplicas(1); + var desired = deploymentWithStatus(); + + // json patch sees the extra actual field as a removal, so it does not match + assertThat(jsonPatchStatusMatcher.matches(desired, actual, context)).isFalse(); + // merge patch tolerates fields only present in the actual state + assertThat(mergePatchStatusMatcher.matches(desired, actual, context)).isTrue(); + } + + @Test + void statusMatchersMatchWhenBothStatusesAbsent() { + assertThat(jsonPatchStatusMatcher.matches(deployment(), deployment(), context)).isTrue(); + assertThat(mergePatchStatusMatcher.matches(deployment(), deployment(), context)).isTrue(); + } + + private static Deployment deployment() { + return new DeploymentBuilder() + .withNewMetadata() + .withName("test") + .withNamespace("default") + .addToLabels("app", "test") + .endMetadata() + .withNewSpec() + .withReplicas(1) + .endSpec() + .build(); + } + + private static Deployment deploymentWithStatus() { + var deployment = deployment(); + deployment.setStatus(new io.fabric8.kubernetes.api.model.apps.DeploymentStatus()); + deployment.getStatus().setReplicas(1); + return deployment; + } + + private static class TestContext extends DefaultContext { + private final KubernetesClient client = MockKubernetesClient.client(HasMetadata.class); + + TestContext() { + super(mock(), mock(), null, false, false); + } + + @Override + public KubernetesClient getClient() { + return client; + } + } +} diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/matcher/StatusMatchersTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/matcher/StatusMatchersTest.java new file mode 100644 index 0000000000..5ce3a9fc40 --- /dev/null +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/matcher/StatusMatchersTest.java @@ -0,0 +1,89 @@ +/* + * Copyright Java Operator SDK Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.javaoperatorsdk.operator.processing.matcher; + +import org.junit.jupiter.api.Test; + +import io.fabric8.kubernetes.api.model.HasMetadata; +import io.fabric8.kubernetes.api.model.apps.Deployment; +import io.fabric8.kubernetes.api.model.apps.DeploymentBuilder; +import io.fabric8.kubernetes.api.model.apps.DeploymentStatusBuilder; +import io.fabric8.kubernetes.client.KubernetesClient; +import io.javaoperatorsdk.operator.MockKubernetesClient; +import io.javaoperatorsdk.operator.api.reconciler.Context; +import io.javaoperatorsdk.operator.api.reconciler.DefaultContext; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.mock; + +class StatusMatchersTest { + + private static final Context context = new TestContext(); + + private final UpdateStatusMatcher updateStatusMatcher = UpdateStatusMatcher.getInstance(); + private final SSAStatusMatcher ssaStatusMatcher = SSAStatusMatcher.getInstance(); + + @Test + void matchesEqualStatus() { + var desired = deploymentWithStatus(1); + var actual = deploymentWithStatus(1); + assertThat(updateStatusMatcher.matches(desired, actual, context)).isTrue(); + assertThat(ssaStatusMatcher.matches(desired, actual, context)).isTrue(); + } + + @Test + void doesNotMatchDifferentStatus() { + var desired = deploymentWithStatus(1); + var actual = deploymentWithStatus(2); + assertThat(updateStatusMatcher.matches(desired, actual, context)).isFalse(); + assertThat(ssaStatusMatcher.matches(desired, actual, context)).isFalse(); + } + + @Test + void ignoresChangesOutsideStatus() { + var desired = deploymentWithStatus(1); + desired.getSpec().setReplicas(5); + var actual = deploymentWithStatus(1); + assertThat(updateStatusMatcher.matches(desired, actual, context)).isTrue(); + assertThat(ssaStatusMatcher.matches(desired, actual, context)).isTrue(); + } + + private static Deployment deploymentWithStatus(int statusReplicas) { + return new DeploymentBuilder() + .withNewMetadata() + .withName("test") + .withNamespace("default") + .endMetadata() + .withNewSpec() + .withReplicas(1) + .endSpec() + .withStatus(new DeploymentStatusBuilder().withReplicas(statusReplicas).build()) + .build(); + } + + private static class TestContext extends DefaultContext { + private final KubernetesClient client = MockKubernetesClient.client(HasMetadata.class); + + TestContext() { + super(mock(), mock(), null, false, false); + } + + @Override + public KubernetesClient getClient() { + return client; + } + } +} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/changenamespace/ChangeNamespaceTestReconciler.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/changenamespace/ChangeNamespaceTestReconciler.java index d05364fc44..c8bee56793 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/changenamespace/ChangeNamespaceTestReconciler.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/changenamespace/ChangeNamespaceTestReconciler.java @@ -53,7 +53,9 @@ public UpdateControl reconcile( ChangeNamespaceTestCustomResource primary, Context context) { - context.resourceOperations().serverSideApply(configMap(primary)); + context + .resourceOperations() + .serverSideApply(configMap(primary), ResourceOperations.Options.forceFilterEvents()); if (primary.getStatus() == null) { primary.setStatus(new ChangeNamespaceTestCustomResourceStatus()); diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/manualobservedgeneration/ManualObservedGenerationIT.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/manualobservedgeneration/ManualObservedGenerationIT.java index 9de22c4a2a..40331bb3b3 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/manualobservedgeneration/ManualObservedGenerationIT.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/manualobservedgeneration/ManualObservedGenerationIT.java @@ -52,6 +52,7 @@ void observedGenerationUpdated() { () -> { var r = extension.get(ManualObservedGenerationCustomResource.class, RESOURCE_NAME); assertThat(r).isNotNull(); + assertThat(r.getStatus()).isNotNull(); assertThat(r.getStatus().getObservedGeneration()).isEqualTo(1); assertThat(r.getStatus().getObservedGeneration()) .isEqualTo(r.getMetadata().getGeneration()); @@ -65,6 +66,8 @@ void observedGenerationUpdated() { .untilAsserted( () -> { var r = extension.get(ManualObservedGenerationCustomResource.class, RESOURCE_NAME); + assertThat(r).isNotNull(); + assertThat(r.getStatus()).isNotNull(); assertThat(r.getStatus().getObservedGeneration()).isEqualTo(2); assertThat(r.getStatus().getObservedGeneration()) .isEqualTo(r.getMetadata().getGeneration()); diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/readcacheafterwrite/filterpatchevent/FilterPatchEventTestReconciler.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/readcacheafterwrite/filterpatchevent/FilterPatchEventTestReconciler.java index 1f19015dcd..59bf33cafc 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/readcacheafterwrite/filterpatchevent/FilterPatchEventTestReconciler.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/readcacheafterwrite/filterpatchevent/FilterPatchEventTestReconciler.java @@ -42,11 +42,12 @@ public UpdateControl reconcile( resource.setStatus(new FilterPatchEventTestCustomResourceStatus()); resource.getStatus().setValue(UPDATED); - var uc = UpdateControl.patchStatus(resource); + context.resourceOperations().jsonMergePatchPrimaryStatus(resource); if (!filterPatchEvent.get()) { - uc = uc.reschedule(); + return UpdateControl.noUpdate().reschedule(); + } else { + return UpdateControl.noUpdate(); } - return uc; } public int getNumberOfExecutions() { diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/readcacheafterwrite/onrelistfilter/OnRelistFilterReconciler.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/readcacheafterwrite/onrelistfilter/OnRelistFilterReconciler.java index 13e8e72d74..5f3ead43ff 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/readcacheafterwrite/onrelistfilter/OnRelistFilterReconciler.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/readcacheafterwrite/onrelistfilter/OnRelistFilterReconciler.java @@ -34,6 +34,7 @@ import io.javaoperatorsdk.operator.api.reconciler.ControllerConfiguration; import io.javaoperatorsdk.operator.api.reconciler.EventSourceContext; import io.javaoperatorsdk.operator.api.reconciler.Reconciler; +import io.javaoperatorsdk.operator.api.reconciler.ResourceOperations; import io.javaoperatorsdk.operator.api.reconciler.UpdateControl; import io.javaoperatorsdk.operator.processing.event.ResourceID; import io.javaoperatorsdk.operator.processing.event.source.EventSource; @@ -79,7 +80,11 @@ public UpdateControl reconcile( if (execution == 1) { var cm = prepareConfigMap(resource); switch (mode.get()) { - case NO_RELIST -> context.resourceOperations().serverSideApply(cm, configMapEventSource); + case NO_RELIST -> + context + .resourceOperations() + .serverSideApply( + cm, configMapEventSource, ResourceOperations.Options.forceFilterEvents()); case RELIST_AROUND_UPDATE -> { configMapEventSource.simulateOnBeforeList(); var applied = context.resourceOperations().serverSideApply(cm, configMapEventSource); @@ -94,7 +99,10 @@ public UpdateControl reconcile( case RELIST_COMPLETES_BEFORE_UPDATE -> { configMapEventSource.simulateOnBeforeList(); configMapEventSource.simulateOnList(); - context.resourceOperations().serverSideApply(cm, configMapEventSource); + context + .resourceOperations() + .serverSideApply( + cm, configMapEventSource, ResourceOperations.Options.forceFilterEvents()); } case RELIST_STARTS_DURING_UPDATE -> { // Drive the event-filtering update path manually so we can fire onBeforeList AFTER the diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/readcacheafterwrite/ownsecondaryupdate/OwnSecondaryUpdateReconciler.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/readcacheafterwrite/ownsecondaryupdate/OwnSecondaryUpdateReconciler.java index f0ac28b955..8a95f6fed8 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/readcacheafterwrite/ownsecondaryupdate/OwnSecondaryUpdateReconciler.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/readcacheafterwrite/ownsecondaryupdate/OwnSecondaryUpdateReconciler.java @@ -27,6 +27,7 @@ import io.javaoperatorsdk.operator.api.reconciler.ControllerConfiguration; import io.javaoperatorsdk.operator.api.reconciler.EventSourceContext; import io.javaoperatorsdk.operator.api.reconciler.Reconciler; +import io.javaoperatorsdk.operator.api.reconciler.ResourceOperations; import io.javaoperatorsdk.operator.api.reconciler.UpdateControl; import io.javaoperatorsdk.operator.processing.event.source.EventSource; import io.javaoperatorsdk.operator.processing.event.source.informer.InformerEventSource; @@ -50,7 +51,12 @@ public UpdateControl reconcile( // version actually advances. With the read-cache-after-write filter in place, none of the // resulting watch events should trigger a fresh reconciliation. for (int i = 1; i <= OWN_SSA_COUNT; i++) { - context.resourceOperations().serverSideApply(prepareCM(resource, i), configMapEventSource); + context + .resourceOperations() + .serverSideApply( + prepareCM(resource, i), + configMapEventSource, + ResourceOperations.Options.forceFilterEvents()); } return UpdateControl.noUpdate(); } diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/readcacheafterwrite/ownssastatusupdate/OwnSsaStatusUpdateCustomResource.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/readcacheafterwrite/ownssastatusupdate/OwnSsaStatusUpdateCustomResource.java new file mode 100644 index 0000000000..e44c863aec --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/readcacheafterwrite/ownssastatusupdate/OwnSsaStatusUpdateCustomResource.java @@ -0,0 +1,28 @@ +/* + * Copyright Java Operator SDK Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.javaoperatorsdk.operator.baseapi.readcacheafterwrite.ownssastatusupdate; + +import io.fabric8.kubernetes.api.model.Namespaced; +import io.fabric8.kubernetes.client.CustomResource; +import io.fabric8.kubernetes.model.annotation.Group; +import io.fabric8.kubernetes.model.annotation.ShortNames; +import io.fabric8.kubernetes.model.annotation.Version; + +@Group("sample.javaoperatorsdk") +@Version("v1") +@ShortNames("ossasu") +public class OwnSsaStatusUpdateCustomResource extends CustomResource + implements Namespaced {} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/readcacheafterwrite/ownssastatusupdate/OwnSsaStatusUpdateIT.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/readcacheafterwrite/ownssastatusupdate/OwnSsaStatusUpdateIT.java new file mode 100644 index 0000000000..fc00cc3fae --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/readcacheafterwrite/ownssastatusupdate/OwnSsaStatusUpdateIT.java @@ -0,0 +1,103 @@ +/* + * Copyright Java Operator SDK Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.javaoperatorsdk.operator.baseapi.readcacheafterwrite.ownssastatusupdate; + +import java.time.Duration; +import java.util.HashMap; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; + +import io.fabric8.kubernetes.api.model.ObjectMetaBuilder; +import io.javaoperatorsdk.operator.junit.LocallyRunOperatorExtension; + +import static io.javaoperatorsdk.operator.baseapi.readcacheafterwrite.ownssastatusupdate.OwnSsaStatusUpdateReconciler.EXTERNAL_LABEL_KEY; +import static io.javaoperatorsdk.operator.baseapi.readcacheafterwrite.ownssastatusupdate.OwnSsaStatusUpdateReconciler.EXTERNAL_LABEL_VALUE; +import static io.javaoperatorsdk.operator.baseapi.readcacheafterwrite.ownssastatusupdate.OwnSsaStatusUpdateReconciler.STATUS_VALUE; +import static org.assertj.core.api.Assertions.assertThat; +import static org.awaitility.Awaitility.await; + +/** + * Verifies that updating the status through {@code + * resourceOperations().serverSideApplyPrimaryStatus(...)} (instead of returning an {@code + * UpdateControl}) is read-cache-after-write consistent and does not loop: the own SSA status write + * is filtered as an own event, so the controller converges. A subsequent external label change must + * still be picked up by a fresh reconciliation. + */ +class OwnSsaStatusUpdateIT { + + static final String RESOURCE_NAME = "test-resource"; + + OwnSsaStatusUpdateReconciler reconciler = new OwnSsaStatusUpdateReconciler(); + + @RegisterExtension + LocallyRunOperatorExtension extension = + LocallyRunOperatorExtension.builder().withReconciler(reconciler).build(); + + @Test + void ssaStatusUpdateIsConsistentAndDoesNotLoop() { + extension.create(testResource()); + + // the status is persisted via the own SSA status update + await() + .atMost(Duration.ofSeconds(30)) + .untilAsserted( + () -> { + var actual = extension.get(OwnSsaStatusUpdateCustomResource.class, RESOURCE_NAME); + assertThat(actual.getStatus()).isNotNull(); + assertThat(actual.getStatus().getValue()).isEqualTo(STATUS_VALUE); + }); + + // the own status write must be filtered: no reconciliation loop + await() + .during(Duration.ofSeconds(2)) + .atMost(Duration.ofSeconds(5)) + .untilAsserted( + () -> + assertThat(reconciler.numberOfExecutions.get()) + .as("own SSA status update must not trigger a reconciliation loop") + .isLessThanOrEqualTo(2)); + + var executionsBeforeExternalUpdate = reconciler.numberOfExecutions.get(); + + // an external party changes a label; this must still trigger a fresh reconciliation + var current = extension.get(OwnSsaStatusUpdateCustomResource.class, RESOURCE_NAME); + var labels = new HashMap(); + if (current.getMetadata().getLabels() != null) { + labels.putAll(current.getMetadata().getLabels()); + } + labels.put(EXTERNAL_LABEL_KEY, EXTERNAL_LABEL_VALUE); + current.getMetadata().setLabels(labels); + extension.replace(current); + + await() + .atMost(Duration.ofSeconds(30)) + .untilAsserted( + () -> { + assertThat(reconciler.numberOfExecutions.get()) + .isGreaterThan(executionsBeforeExternalUpdate); + assertThat(reconciler.externalLabelSeenInLaterReconciliation.get()) + .as("a later reconciliation must observe the externally-applied label") + .isTrue(); + }); + } + + OwnSsaStatusUpdateCustomResource testResource() { + var r = new OwnSsaStatusUpdateCustomResource(); + r.setMetadata(new ObjectMetaBuilder().withName(RESOURCE_NAME).build()); + return r; + } +} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/readcacheafterwrite/ownssastatusupdate/OwnSsaStatusUpdateReconciler.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/readcacheafterwrite/ownssastatusupdate/OwnSsaStatusUpdateReconciler.java new file mode 100644 index 0000000000..472aeb47d6 --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/readcacheafterwrite/ownssastatusupdate/OwnSsaStatusUpdateReconciler.java @@ -0,0 +1,66 @@ +/* + * Copyright Java Operator SDK Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.javaoperatorsdk.operator.baseapi.readcacheafterwrite.ownssastatusupdate; + +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.atomic.AtomicInteger; + +import io.javaoperatorsdk.operator.api.reconciler.Context; +import io.javaoperatorsdk.operator.api.reconciler.ControllerConfiguration; +import io.javaoperatorsdk.operator.api.reconciler.Reconciler; +import io.javaoperatorsdk.operator.api.reconciler.UpdateControl; + +/** + * Updates its own status via {@link + * io.javaoperatorsdk.operator.api.reconciler.ResourceOperations#serverSideApplyPrimaryStatus( + * io.fabric8.kubernetes.api.model.HasMetadata)} instead of returning an {@link UpdateControl}. The + * status is server-side applied through the controller's own event source, so the resulting own + * event must be filtered and must not trigger a reconciliation loop. A later external update (a + * label change) must still be observed by a fresh reconciliation. + */ +@ControllerConfiguration(generationAwareEventProcessing = false) +public class OwnSsaStatusUpdateReconciler implements Reconciler { + + static final String STATUS_VALUE = "ready"; + static final String EXTERNAL_LABEL_KEY = "externally-set"; + static final String EXTERNAL_LABEL_VALUE = "yes"; + + final AtomicInteger numberOfExecutions = new AtomicInteger(); + final AtomicBoolean externalLabelSeenInLaterReconciliation = new AtomicBoolean(); + + @Override + public UpdateControl reconcile( + OwnSsaStatusUpdateCustomResource resource, + Context context) { + numberOfExecutions.incrementAndGet(); + + var labels = resource.getMetadata().getLabels(); + if (labels != null && EXTERNAL_LABEL_VALUE.equals(labels.get(EXTERNAL_LABEL_KEY))) { + externalLabelSeenInLaterReconciliation.set(true); + } + + // Only apply the status when it is not already set - the SSA status matcher makes repeated + // applies no-ops anyway, but this keeps the intent explicit and the reconciliation idempotent. + if (resource.getStatus() == null || !STATUS_VALUE.equals(resource.getStatus().getValue())) { + resource.setStatus(new OwnSsaStatusUpdateStatus().setValue(STATUS_VALUE)); + // SSA does not use optimistic locking + resource.getMetadata().setResourceVersion(null); + context.resourceOperations().serverSideApplyPrimaryStatus(resource); + } + + return UpdateControl.noUpdate(); + } +} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/readcacheafterwrite/ownssastatusupdate/OwnSsaStatusUpdateStatus.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/readcacheafterwrite/ownssastatusupdate/OwnSsaStatusUpdateStatus.java new file mode 100644 index 0000000000..48e2e1d73c --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/readcacheafterwrite/ownssastatusupdate/OwnSsaStatusUpdateStatus.java @@ -0,0 +1,30 @@ +/* + * Copyright Java Operator SDK Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.javaoperatorsdk.operator.baseapi.readcacheafterwrite.ownssastatusupdate; + +public class OwnSsaStatusUpdateStatus { + + private String value; + + public String getValue() { + return value; + } + + public OwnSsaStatusUpdateStatus setValue(String value) { + this.value = value; + return this; + } +} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/readcacheafterwrite/specchangeduringstatuspatch/SpecChangeDuringStatusPatchCustomResource.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/readcacheafterwrite/specchangeduringstatuspatch/SpecChangeDuringStatusPatchCustomResource.java new file mode 100644 index 0000000000..dff33b454d --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/readcacheafterwrite/specchangeduringstatuspatch/SpecChangeDuringStatusPatchCustomResource.java @@ -0,0 +1,29 @@ +/* + * Copyright Java Operator SDK Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.javaoperatorsdk.operator.baseapi.readcacheafterwrite.specchangeduringstatuspatch; + +import io.fabric8.kubernetes.api.model.Namespaced; +import io.fabric8.kubernetes.client.CustomResource; +import io.fabric8.kubernetes.model.annotation.Group; +import io.fabric8.kubernetes.model.annotation.ShortNames; +import io.fabric8.kubernetes.model.annotation.Version; + +@Group("sample.javaoperatorsdk") +@Version("v1") +@ShortNames("scdsp") +public class SpecChangeDuringStatusPatchCustomResource + extends CustomResource + implements Namespaced {} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/readcacheafterwrite/specchangeduringstatuspatch/SpecChangeDuringStatusPatchIT.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/readcacheafterwrite/specchangeduringstatuspatch/SpecChangeDuringStatusPatchIT.java new file mode 100644 index 0000000000..30df830e58 --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/readcacheafterwrite/specchangeduringstatuspatch/SpecChangeDuringStatusPatchIT.java @@ -0,0 +1,107 @@ +/* + * Copyright Java Operator SDK Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.javaoperatorsdk.operator.baseapi.readcacheafterwrite.specchangeduringstatuspatch; + +import java.time.Duration; +import java.util.concurrent.TimeUnit; + +import org.junit.jupiter.api.RepeatedTest; +import org.junit.jupiter.api.extension.RegisterExtension; + +import io.fabric8.kubernetes.api.model.ObjectMetaBuilder; +import io.fabric8.kubernetes.client.dsl.base.PatchContext; +import io.fabric8.kubernetes.client.dsl.base.PatchType; +import io.javaoperatorsdk.operator.junit.LocallyRunOperatorExtension; + +import static io.javaoperatorsdk.operator.baseapi.readcacheafterwrite.specchangeduringstatuspatch.SpecChangeDuringStatusPatchReconciler.STATUS_VALUE; +import static org.assertj.core.api.Assertions.assertThat; +import static org.awaitility.Awaitility.await; + +/** + * Reproduces a concurrent spec change happening while the controller patches its own status. When + * the reconciler patches the status it opens an event filtering window so it does not re-trigger + * itself. This test changes the spec on the cluster while that window is open and verifies that the + * spec change is still reconciled - it must not be silently absorbed together with the controller's + * own status update. + */ +class SpecChangeDuringStatusPatchIT { + + static final String RESOURCE_NAME = "test-resource"; + static final String SPEC_VALUE = "initial"; + public static final String UPDATED_SPEC_VALUE = "updated-val"; + + SpecChangeDuringStatusPatchReconciler reconciler = new SpecChangeDuringStatusPatchReconciler(); + + @RegisterExtension + LocallyRunOperatorExtension extension = + LocallyRunOperatorExtension.builder().withReconciler(reconciler).build(); + + @RepeatedTest(3) + void specChangeDuringStatusPatchIsReconciled() throws InterruptedException { + var res = extension.create(testResource()); + var statusRes = testResource(); + statusRes.getMetadata().setNamespace(res.getMetadata().getNamespace()); + extension + .getKubernetesClient() + .resource(statusRes) + .status() + .patch( + new PatchContext.Builder() + .withForce(true) + .withFieldManager( + SpecChangeDuringStatusPatchReconciler.class.getSimpleName().toLowerCase()) + .withPatchType(PatchType.SERVER_SIDE_APPLY) + .build()); + + // wait until the reconciler is inside its own status patch, holding the filtering window open + assertThat(reconciler.statusPatchStartedLatch.await(30, TimeUnit.SECONDS)) + .as("reconciler should enter its own status patch operation") + .isTrue(); + + // change the spec on the cluster while the controller's status patch is still in flight + var current = extension.get(SpecChangeDuringStatusPatchCustomResource.class, RESOURCE_NAME); + current.getSpec().setValue(UPDATED_SPEC_VALUE); + extension.replace(current); + + // let the reconciler finish its own status patch + reconciler.specChangeDoneLatch.countDown(); + + // the spec change must be picked up by a fresh reconciliation and not lost with the own update + await() + .atMost(Duration.ofSeconds(5)) + .untilAsserted( + () -> { + assertThat(reconciler.numberOfExecutions.get()).isGreaterThanOrEqualTo(2); + assertThat(reconciler.lastObservedSpecValue.get()) + .as("a later reconciliation must observe the externally-applied spec change") + .isEqualTo(UPDATED_SPEC_VALUE); + }); + + // sanity check: the status the controller set is still present after the concurrent spec change + var updated = extension.get(SpecChangeDuringStatusPatchCustomResource.class, RESOURCE_NAME); + assertThat(updated.getSpec().getValue()).isEqualTo(UPDATED_SPEC_VALUE); + assertThat(updated.getStatus()).isNotNull(); + assertThat(updated.getStatus().getValue()).isEqualTo(STATUS_VALUE); + } + + SpecChangeDuringStatusPatchCustomResource testResource() { + var r = new SpecChangeDuringStatusPatchCustomResource(); + r.setMetadata(new ObjectMetaBuilder().withName(RESOURCE_NAME).build()); + r.setSpec(new SpecChangeDuringStatusPatchSpec().setValue(SPEC_VALUE)); + r.setStatus(new SpecChangeDuringStatusPatchStatus().setValue(STATUS_VALUE)); + return r; + } +} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/readcacheafterwrite/specchangeduringstatuspatch/SpecChangeDuringStatusPatchReconciler.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/readcacheafterwrite/specchangeduringstatuspatch/SpecChangeDuringStatusPatchReconciler.java new file mode 100644 index 0000000000..c639838f00 --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/readcacheafterwrite/specchangeduringstatuspatch/SpecChangeDuringStatusPatchReconciler.java @@ -0,0 +1,71 @@ +/* + * Copyright Java Operator SDK Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.javaoperatorsdk.operator.baseapi.readcacheafterwrite.specchangeduringstatuspatch; + +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicInteger; +import java.util.concurrent.atomic.AtomicReference; + +import io.javaoperatorsdk.operator.api.reconciler.Context; +import io.javaoperatorsdk.operator.api.reconciler.ControllerConfiguration; +import io.javaoperatorsdk.operator.api.reconciler.Reconciler; +import io.javaoperatorsdk.operator.api.reconciler.UpdateControl; + +/** + * On the first reconciliation the reconciler patches its own status, but keeps the event filtering + * window open until the test signals that it has changed the spec on the cluster. This reproduces + * the race where a spec change lands while the controller's own status patch is in flight: the spec + * change event must still propagate as a fresh reconciliation, it must not be absorbed as if it + * were our own status update. + */ +@ControllerConfiguration +public class SpecChangeDuringStatusPatchReconciler + implements Reconciler { + + static final String STATUS_VALUE = "reconciled"; + + final AtomicInteger numberOfExecutions = new AtomicInteger(); + final CountDownLatch statusPatchStartedLatch = new CountDownLatch(1); + final CountDownLatch specChangeDoneLatch = new CountDownLatch(1); + final AtomicReference lastObservedSpecValue = new AtomicReference<>(); + + @Override + public UpdateControl reconcile( + SpecChangeDuringStatusPatchCustomResource resource, + Context context) { + int execution = numberOfExecutions.incrementAndGet(); + lastObservedSpecValue.set(resource.getSpec().getValue()); + + if (execution == 1) { + resource.setStatus(new SpecChangeDuringStatusPatchStatus().setValue(STATUS_VALUE)); + resource.getMetadata().setResourceVersion(null); + // Patch our own status, but hold the filtering window open with a hook that lets the test + // change the spec on the cluster WHILE the status patch is still in flight. + statusPatchStartedLatch.countDown(); + try { + if (!specChangeDoneLatch.await(30, TimeUnit.SECONDS)) { + throw new IllegalStateException("timed out waiting for external spec change"); + } + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + throw new IllegalStateException(e); + } + return UpdateControl.patchStatus(resource); + } + return UpdateControl.noUpdate(); + } +} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/readcacheafterwrite/specchangeduringstatuspatch/SpecChangeDuringStatusPatchSpec.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/readcacheafterwrite/specchangeduringstatuspatch/SpecChangeDuringStatusPatchSpec.java new file mode 100644 index 0000000000..64a6e7f0ef --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/readcacheafterwrite/specchangeduringstatuspatch/SpecChangeDuringStatusPatchSpec.java @@ -0,0 +1,30 @@ +/* + * Copyright Java Operator SDK Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.javaoperatorsdk.operator.baseapi.readcacheafterwrite.specchangeduringstatuspatch; + +public class SpecChangeDuringStatusPatchSpec { + + private String value; + + public String getValue() { + return value; + } + + public SpecChangeDuringStatusPatchSpec setValue(String value) { + this.value = value; + return this; + } +} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/readcacheafterwrite/specchangeduringstatuspatch/SpecChangeDuringStatusPatchStatus.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/readcacheafterwrite/specchangeduringstatuspatch/SpecChangeDuringStatusPatchStatus.java new file mode 100644 index 0000000000..4cb857a4cc --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/readcacheafterwrite/specchangeduringstatuspatch/SpecChangeDuringStatusPatchStatus.java @@ -0,0 +1,30 @@ +/* + * Copyright Java Operator SDK Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.javaoperatorsdk.operator.baseapi.readcacheafterwrite.specchangeduringstatuspatch; + +public class SpecChangeDuringStatusPatchStatus { + + private String value; + + public String getValue() { + return value; + } + + public SpecChangeDuringStatusPatchStatus setValue(String value) { + this.value = value; + return this; + } +} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/resourceoperations/ResourceOperationsCustomResource.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/resourceoperations/ResourceOperationsCustomResource.java new file mode 100644 index 0000000000..93e2ebe7fc --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/resourceoperations/ResourceOperationsCustomResource.java @@ -0,0 +1,29 @@ +/* + * Copyright Java Operator SDK Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.javaoperatorsdk.operator.baseapi.resourceoperations; + +import io.fabric8.kubernetes.api.model.Namespaced; +import io.fabric8.kubernetes.client.CustomResource; +import io.fabric8.kubernetes.model.annotation.Group; +import io.fabric8.kubernetes.model.annotation.ShortNames; +import io.fabric8.kubernetes.model.annotation.Version; + +@Group("sample.javaoperatorsdk") +@Version("v1") +@ShortNames("ropres") +public class ResourceOperationsCustomResource + extends CustomResource + implements Namespaced {} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/resourceoperations/ResourceOperationsIT.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/resourceoperations/ResourceOperationsIT.java new file mode 100644 index 0000000000..d655ef7e4f --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/resourceoperations/ResourceOperationsIT.java @@ -0,0 +1,150 @@ +/* + * Copyright Java Operator SDK Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.javaoperatorsdk.operator.baseapi.resourceoperations; + +import java.time.Duration; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; + +import io.fabric8.kubernetes.api.model.ObjectMetaBuilder; +import io.javaoperatorsdk.operator.baseapi.resourceoperations.ResourceOperationsReconciler.Operation; +import io.javaoperatorsdk.operator.junit.LocallyRunOperatorExtension; + +import static io.javaoperatorsdk.operator.baseapi.resourceoperations.ResourceOperationsReconciler.SPEC_MARKER; +import static io.javaoperatorsdk.operator.baseapi.resourceoperations.ResourceOperationsReconciler.STATUS_VALUE; +import static org.assertj.core.api.Assertions.assertThat; +import static org.awaitility.Awaitility.await; + +/** + * Integration test that drives every primary update/patch operation exposed by {@link + * io.javaoperatorsdk.operator.api.reconciler.ResourceOperations} against a real cluster through the + * reconciler. For each operation it asserts: + * + *

    + *
  • the intended change is actually persisted on the server (spec marker or status value), and + *
  • the operation converges: since every operation uses its default matcher and filters its own + * event, the controller must not loop on the write it just performed. + *
+ */ +class ResourceOperationsIT { + + static final String RESOURCE_NAME = "test-resource"; + static final String INITIAL_SPEC_VALUE = "initial"; + + ResourceOperationsReconciler reconciler = new ResourceOperationsReconciler(); + + @RegisterExtension + LocallyRunOperatorExtension extension = + LocallyRunOperatorExtension.builder().withReconciler(reconciler).build(); + + @Test + void serverSideApply() { + assertSpecOperation(Operation.SSA); + } + + @Test + void serverSideApplyStatus() { + assertStatusOperation(Operation.SSA_STATUS); + } + + @Test + void update() { + assertSpecOperation(Operation.UPDATE); + } + + @Test + void updateStatus() { + assertStatusOperation(Operation.UPDATE_STATUS); + } + + @Test + void jsonPatch() { + assertSpecOperation(Operation.JSON_PATCH); + } + + @Test + void jsonPatchStatus() { + assertStatusOperation(Operation.JSON_PATCH_STATUS); + } + + @Test + void jsonMergePatch() { + assertSpecOperation(Operation.JSON_MERGE_PATCH); + } + + @Test + void jsonMergePatchStatus() { + assertStatusOperation(Operation.JSON_MERGE_PATCH_STATUS); + } + + private void assertSpecOperation(Operation operation) { + runOperation(operation); + await() + .atMost(Duration.ofSeconds(30)) + .untilAsserted( + () -> + assertThat(actual().getSpec().getValue()) + .as("operation %s should persist the spec change", operation) + .isEqualTo(SPEC_MARKER)); + assertConvergesWithoutLooping(operation); + } + + private void assertStatusOperation(Operation operation) { + runOperation(operation); + await() + .atMost(Duration.ofSeconds(30)) + .untilAsserted( + () -> { + assertThat(actual().getStatus()).isNotNull(); + assertThat(actual().getStatus().getValue()) + .as("operation %s should persist the status value", operation) + .isEqualTo(STATUS_VALUE); + }); + assertConvergesWithoutLooping(operation); + } + + private void runOperation(Operation operation) { + reconciler.setOperation(operation); + extension.create(testResource()); + } + + /** + * The write must be filtered as an own event and any further reconciliation must match the + * desired state, so the controller does not loop on its own write. + */ + private void assertConvergesWithoutLooping(Operation operation) { + await() + .during(Duration.ofSeconds(2)) + .atMost(Duration.ofSeconds(5)) + .untilAsserted( + () -> + assertThat(reconciler.numberOfExecutions.get()) + .as("operation %s should converge without triggering an event loop", operation) + .isLessThanOrEqualTo(3)); + } + + private ResourceOperationsCustomResource actual() { + return extension.get(ResourceOperationsCustomResource.class, RESOURCE_NAME); + } + + ResourceOperationsCustomResource testResource() { + var r = new ResourceOperationsCustomResource(); + r.setMetadata(new ObjectMetaBuilder().withName(RESOURCE_NAME).build()); + r.setSpec(new ResourceOperationsSpec().setValue(INITIAL_SPEC_VALUE)); + return r; + } +} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/resourceoperations/ResourceOperationsReconciler.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/resourceoperations/ResourceOperationsReconciler.java new file mode 100644 index 0000000000..bbfb7cd098 --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/resourceoperations/ResourceOperationsReconciler.java @@ -0,0 +1,122 @@ +/* + * Copyright Java Operator SDK Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.javaoperatorsdk.operator.baseapi.resourceoperations; + +import java.util.concurrent.atomic.AtomicInteger; + +import io.fabric8.kubernetes.api.model.ObjectMetaBuilder; +import io.javaoperatorsdk.operator.api.reconciler.Context; +import io.javaoperatorsdk.operator.api.reconciler.ControllerConfiguration; +import io.javaoperatorsdk.operator.api.reconciler.Reconciler; +import io.javaoperatorsdk.operator.api.reconciler.UpdateControl; + +/** + * Exercises every primary update/patch operation exposed by {@link + * io.javaoperatorsdk.operator.api.reconciler.ResourceOperations}. On each reconciliation it applies + * the change dictated by the currently selected {@link Operation} using {@code + * context.resourceOperations()} and then returns {@link UpdateControl#noUpdate()} (the write was + * already performed via the operations helper). + * + *

Status operations write a fixed value into the status subresource; whole-resource operations + * write a fixed value into the spec. Every operation uses its default matcher, so once the change + * is present on the server subsequent reconciliations must match and become no-ops - which, + * together with own event filtering, is what keeps the controller from looping on its own writes. + */ +@ControllerConfiguration +public class ResourceOperationsReconciler implements Reconciler { + + public static final String STATUS_VALUE = "reconciled"; + public static final String SPEC_MARKER = "applied"; + + public enum Operation { + SSA, + SSA_STATUS, + UPDATE, + UPDATE_STATUS, + JSON_PATCH, + JSON_PATCH_STATUS, + JSON_MERGE_PATCH, + JSON_MERGE_PATCH_STATUS + } + + private volatile Operation operation; + final AtomicInteger numberOfExecutions = new AtomicInteger(); + + @Override + public UpdateControl reconcile( + ResourceOperationsCustomResource resource, + Context context) { + numberOfExecutions.incrementAndGet(); + var ops = context.resourceOperations(); + + switch (operation) { + case SSA -> { + markResource(resource); + // SSA does not use optimistic locking + resource.getMetadata().setManagedFields(null); + resource.getMetadata().setResourceVersion(null); + ops.serverSideApplyPrimary(resource); + } + case SSA_STATUS -> { + ResourceOperationsCustomResource fresh = new ResourceOperationsCustomResource(); + fresh.setMetadata( + new ObjectMetaBuilder() + .withName(resource.getMetadata().getName()) + .withNamespace(resource.getMetadata().getNamespace()) + .build()); + + fresh.setStatus(new ResourceOperationsStatus().setValue(STATUS_VALUE)); + fresh.getMetadata().setResourceVersion(null); + ops.serverSideApplyPrimaryStatus(fresh); + } + case UPDATE -> { + markResource(resource); + ops.updatePrimary(resource); + } + case UPDATE_STATUS -> { + resource.setStatus(new ResourceOperationsStatus().setValue(STATUS_VALUE)); + ops.updatePrimaryStatus(resource); + } + case JSON_PATCH -> ops.jsonPatchPrimary(resource, ResourceOperationsReconciler::markResource); + case JSON_PATCH_STATUS -> + ops.jsonPatchPrimaryStatus( + resource, + r -> { + r.setStatus(new ResourceOperationsStatus().setValue(STATUS_VALUE)); + return r; + }); + case JSON_MERGE_PATCH -> { + markResource(resource); + ops.jsonMergePatchPrimary(resource); + } + case JSON_MERGE_PATCH_STATUS -> { + resource.setStatus(new ResourceOperationsStatus().setValue(STATUS_VALUE)); + ops.jsonMergePatchPrimaryStatus(resource); + } + } + return UpdateControl.noUpdate(); + } + + private static ResourceOperationsCustomResource markResource( + ResourceOperationsCustomResource resource) { + resource.getSpec().setValue(SPEC_MARKER); + return resource; + } + + public void setOperation(Operation operation) { + this.operation = operation; + } +} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/resourceoperations/ResourceOperationsSpec.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/resourceoperations/ResourceOperationsSpec.java new file mode 100644 index 0000000000..50ce143d7e --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/resourceoperations/ResourceOperationsSpec.java @@ -0,0 +1,30 @@ +/* + * Copyright Java Operator SDK Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.javaoperatorsdk.operator.baseapi.resourceoperations; + +public class ResourceOperationsSpec { + + private String value; + + public String getValue() { + return value; + } + + public ResourceOperationsSpec setValue(String value) { + this.value = value; + return this; + } +} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/resourceoperations/ResourceOperationsStatus.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/resourceoperations/ResourceOperationsStatus.java new file mode 100644 index 0000000000..fdb046b5c0 --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/resourceoperations/ResourceOperationsStatus.java @@ -0,0 +1,30 @@ +/* + * Copyright Java Operator SDK Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.javaoperatorsdk.operator.baseapi.resourceoperations; + +public class ResourceOperationsStatus { + + private String value; + + public String getValue() { + return value; + } + + public ResourceOperationsStatus setValue(String value) { + this.value = value; + return this; + } +} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/resourceoperations/SecondaryResourceOperationsCustomResource.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/resourceoperations/SecondaryResourceOperationsCustomResource.java new file mode 100644 index 0000000000..787495a3ca --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/resourceoperations/SecondaryResourceOperationsCustomResource.java @@ -0,0 +1,29 @@ +/* + * Copyright Java Operator SDK Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.javaoperatorsdk.operator.baseapi.resourceoperations; + +import io.fabric8.kubernetes.api.model.Namespaced; +import io.fabric8.kubernetes.client.CustomResource; +import io.fabric8.kubernetes.model.annotation.Group; +import io.fabric8.kubernetes.model.annotation.ShortNames; +import io.fabric8.kubernetes.model.annotation.Version; + +@Group("sample.javaoperatorsdk") +@Version("v1") +@ShortNames("secropres") +public class SecondaryResourceOperationsCustomResource + extends CustomResource + implements Namespaced {} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/resourceoperations/SecondaryResourceOperationsIT.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/resourceoperations/SecondaryResourceOperationsIT.java new file mode 100644 index 0000000000..346a264da1 --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/resourceoperations/SecondaryResourceOperationsIT.java @@ -0,0 +1,120 @@ +/* + * Copyright Java Operator SDK Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.javaoperatorsdk.operator.baseapi.resourceoperations; + +import java.time.Duration; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; + +import io.fabric8.kubernetes.api.model.ConfigMap; +import io.fabric8.kubernetes.api.model.ObjectMetaBuilder; +import io.javaoperatorsdk.operator.baseapi.resourceoperations.SecondaryResourceOperationsReconciler.Operation; +import io.javaoperatorsdk.operator.junit.LocallyRunOperatorExtension; + +import static io.javaoperatorsdk.operator.baseapi.resourceoperations.SecondaryResourceOperationsReconciler.APPLIED_VALUE; +import static io.javaoperatorsdk.operator.baseapi.resourceoperations.SecondaryResourceOperationsReconciler.CREATE_VALUE; +import static io.javaoperatorsdk.operator.baseapi.resourceoperations.SecondaryResourceOperationsReconciler.DATA_KEY; +import static org.assertj.core.api.Assertions.assertThat; +import static org.awaitility.Awaitility.await; + +/** + * Integration test for the secondary-resource variants of {@link + * io.javaoperatorsdk.operator.api.reconciler.ResourceOperations} (the overloads taking an {@code + * InformerEventSource}). For each operation it asserts: + * + *

    + *
  • the managed {@code ConfigMap} secondary is created/updated to the expected value, and + *
  • the write on the secondary is filtered as an own event, so the controller converges and + * does not loop on its own secondary write. + *
+ */ +class SecondaryResourceOperationsIT { + + static final String RESOURCE_NAME = "test-resource"; + + SecondaryResourceOperationsReconciler reconciler = new SecondaryResourceOperationsReconciler(); + + @RegisterExtension + LocallyRunOperatorExtension extension = + LocallyRunOperatorExtension.builder().withReconciler(reconciler).build(); + + @Test + void create() { + reconciler.setOperation(Operation.CREATE); + extension.create(testResource()); + awaitConfigMapValue(CREATE_VALUE); + assertConvergesWithoutLooping(Operation.CREATE); + } + + @Test + void serverSideApply() { + assertAppliedOperation(Operation.SSA); + } + + @Test + void update() { + assertAppliedOperation(Operation.UPDATE); + } + + @Test + void jsonPatch() { + assertAppliedOperation(Operation.JSON_PATCH); + } + + @Test + void jsonMergePatch() { + assertAppliedOperation(Operation.JSON_MERGE_PATCH); + } + + private void assertAppliedOperation(Operation operation) { + reconciler.setOperation(operation); + extension.create(testResource()); + awaitConfigMapValue(APPLIED_VALUE); + assertConvergesWithoutLooping(operation); + } + + private void awaitConfigMapValue(String expected) { + await() + .atMost(Duration.ofSeconds(30)) + .untilAsserted( + () -> { + var cm = extension.get(ConfigMap.class, RESOURCE_NAME); + assertThat(cm).isNotNull(); + assertThat(cm.getData()).containsEntry(DATA_KEY, expected); + }); + } + + private void assertConvergesWithoutLooping(Operation operation) { + await() + .during(Duration.ofSeconds(2)) + .atMost(Duration.ofSeconds(5)) + .untilAsserted( + () -> + assertThat(reconciler.numberOfExecutions.get()) + .as( + "operation %s should converge without looping on its own secondary write", + operation) + .isLessThanOrEqualTo(4)); + } + + SecondaryResourceOperationsCustomResource testResource() { + var r = new SecondaryResourceOperationsCustomResource(); + r.setMetadata(new ObjectMetaBuilder().withName(RESOURCE_NAME).build()); + r.setSpec(new ResourceOperationsSpec().setValue("initial")); + return r; + } +} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/resourceoperations/SecondaryResourceOperationsReconciler.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/resourceoperations/SecondaryResourceOperationsReconciler.java new file mode 100644 index 0000000000..6a694c9751 --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/resourceoperations/SecondaryResourceOperationsReconciler.java @@ -0,0 +1,154 @@ +/* + * Copyright Java Operator SDK Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.javaoperatorsdk.operator.baseapi.resourceoperations; + +import java.util.List; +import java.util.Map; +import java.util.concurrent.atomic.AtomicInteger; + +import io.fabric8.kubernetes.api.model.ConfigMap; +import io.fabric8.kubernetes.api.model.ConfigMapBuilder; +import io.fabric8.kubernetes.api.model.ObjectMetaBuilder; +import io.javaoperatorsdk.operator.api.config.informer.InformerEventSourceConfiguration; +import io.javaoperatorsdk.operator.api.reconciler.Context; +import io.javaoperatorsdk.operator.api.reconciler.ControllerConfiguration; +import io.javaoperatorsdk.operator.api.reconciler.EventSourceContext; +import io.javaoperatorsdk.operator.api.reconciler.Reconciler; +import io.javaoperatorsdk.operator.api.reconciler.ResourceOperations.Options; +import io.javaoperatorsdk.operator.api.reconciler.UpdateControl; +import io.javaoperatorsdk.operator.processing.event.source.EventSource; +import io.javaoperatorsdk.operator.processing.event.source.informer.InformerEventSource; + +/** + * Exercises the secondary-resource variants of {@link + * io.javaoperatorsdk.operator.api.reconciler.ResourceOperations} - the overloads that take an + * {@link InformerEventSource} so the written secondary is cached (and its own event filtered) + * against that source. On every reconciliation it ensures a {@code ConfigMap} secondary exists with + * the value dictated by the selected {@link Operation}, using {@code context.resourceOperations()}. + * + *

The operations are applied idempotently: the secondary is only written when missing or when + * its data differs from the desired value. Together with own event filtering this keeps the + * controller from looping on the secondary writes it performs itself. + */ +@ControllerConfiguration +public class SecondaryResourceOperationsReconciler + implements Reconciler { + + public static final String DATA_KEY = "value"; + public static final String CREATE_VALUE = "created"; + public static final String APPLIED_VALUE = "applied"; + + public enum Operation { + CREATE, + SSA, + UPDATE, + JSON_PATCH, + JSON_MERGE_PATCH + } + + private volatile Operation operation; + final AtomicInteger numberOfExecutions = new AtomicInteger(); + + private InformerEventSource + configMapEventSource; + + @Override + public UpdateControl reconcile( + SecondaryResourceOperationsCustomResource resource, + Context context) { + numberOfExecutions.incrementAndGet(); + var ops = context.resourceOperations(); + var actual = context.getSecondaryResource(ConfigMap.class).orElse(null); + + if (operation == Operation.CREATE) { + if (actual == null) { + ops.create( + desiredConfigMap(resource, CREATE_VALUE), configMapEventSource, Options.cacheOnly()); + } + return UpdateControl.noUpdate(); + } + + // for the update/patch variants the secondary must exist first + if (actual == null) { + ops.create( + desiredConfigMap(resource, CREATE_VALUE), configMapEventSource, Options.cacheOnly()); + return UpdateControl.noUpdate(); + } + + // idempotency guard: only write when the secondary does not yet hold the desired value + if (APPLIED_VALUE.equals(actual.getData().get(DATA_KEY))) { + return UpdateControl.noUpdate(); + } + + switch (operation) { + case SSA -> { + var desired = desiredConfigMap(resource, APPLIED_VALUE); + desired.getMetadata().setResourceVersion(null); + ops.serverSideApply(desired, configMapEventSource, Options.forceFilterEvents()); + } + case UPDATE -> { + actual.getData().put(DATA_KEY, APPLIED_VALUE); + ops.update(actual, configMapEventSource, Options.filterIfOptimisticLocking()); + } + case JSON_PATCH -> + ops.jsonPatch( + actual, + cm -> { + cm.getData().put(DATA_KEY, APPLIED_VALUE); + return cm; + }, + configMapEventSource, + Options.filterIfOptimisticLocking()); + case JSON_MERGE_PATCH -> { + var desired = desiredConfigMap(resource, APPLIED_VALUE); + ops.jsonMergePatch(desired, configMapEventSource, Options.filterIfOptimisticLocking()); + } + default -> throw new IllegalStateException("Unexpected operation: " + operation); + } + return UpdateControl.noUpdate(); + } + + @Override + public List> prepareEventSources( + EventSourceContext context) { + configMapEventSource = + new InformerEventSource<>( + InformerEventSourceConfiguration.from( + ConfigMap.class, SecondaryResourceOperationsCustomResource.class) + .build(), + context); + return List.of(configMapEventSource); + } + + private static ConfigMap desiredConfigMap( + SecondaryResourceOperationsCustomResource primary, String value) { + var cm = + new ConfigMapBuilder() + .withMetadata( + new ObjectMetaBuilder() + .withName(primary.getMetadata().getName()) + .withNamespace(primary.getMetadata().getNamespace()) + .build()) + .withData(Map.of(DATA_KEY, value)) + .build(); + cm.addOwnerReference(primary); + return cm; + } + + public void setOperation(Operation operation) { + this.operation = operation; + } +} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/simple/ReconcilerExecutorIT.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/simple/ReconcilerExecutorIT.java index 54d639c05a..b6790e4085 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/simple/ReconcilerExecutorIT.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/simple/ReconcilerExecutorIT.java @@ -52,7 +52,7 @@ void configMapGetsCreatedForTestCustomResource() { awaitResourcesCreatedOrUpdated(); awaitStatusUpdated(); - assertThat(TestUtils.getNumberOfExecutions(operator)).isEqualTo(1); + assertThat(TestUtils.getNumberOfExecutions(operator)).isEqualTo(2); } @Test diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/subresource/SubResourceTestCustomReconciler.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/subresource/SubResourceTestCustomReconciler.java index 9c270d6164..a4a2f6398a 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/subresource/SubResourceTestCustomReconciler.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/subresource/SubResourceTestCustomReconciler.java @@ -44,9 +44,17 @@ public UpdateControl reconcile( log.info("Value: " + resource.getSpec().getValue()); ensureStatusExists(resource); - resource.getStatus().setState(SubResourceTestCustomResourceStatus.State.SUCCESS); + waitXms(RECONCILER_MIN_EXEC_TIME); - return UpdateControl.patchStatus(resource); + context + .resourceOperations() + .jsonPatchPrimaryStatus( + resource, + r -> { + r.getStatus().setState(SubResourceTestCustomResourceStatus.State.SUCCESS); + return r; + }); + return UpdateControl.noUpdate(); } private void ensureStatusExists(SubResourceTestCustomResource resource) { diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/subresource/SubResourceUpdateIT.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/subresource/SubResourceUpdateIT.java index a86220439c..1ea9ca96ce 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/subresource/SubResourceUpdateIT.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/subresource/SubResourceUpdateIT.java @@ -59,7 +59,7 @@ void updatesSubResourceStatus() { // wait for sure, there are no more events waitXms(WAIT_AFTER_EXECUTION); // there is no event on status update processed - assertThat(TestUtils.getNumberOfExecutions(operator)).isEqualTo(1); + assertThat(TestUtils.getNumberOfExecutions(operator)).isEqualTo(2); } @Test @@ -73,7 +73,7 @@ void updatesSubResourceStatusNoFinalizer() { // wait for sure, there are no more events waitXms(WAIT_AFTER_EXECUTION); // there is no event on status update processed - assertThat(TestUtils.getNumberOfExecutions(operator)).isEqualTo(1); + assertThat(TestUtils.getNumberOfExecutions(operator)).isEqualTo(2); } /** Note that we check on controller impl if there is finalizer on execution. */ @@ -87,7 +87,7 @@ void ifNoFinalizerPresentFirstAddsTheFinalizerThenExecutesControllerAgain() { // wait for sure, there are no more events waitXms(WAIT_AFTER_EXECUTION); // there is no event on status update processed - assertThat(TestUtils.getNumberOfExecutions(operator)).isEqualTo(1); + assertThat(TestUtils.getNumberOfExecutions(operator)).isEqualTo(2); } /** diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/workflow/workflowmultipleactivation/WorkflowMultipleActivationIT.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/workflow/workflowmultipleactivation/WorkflowMultipleActivationIT.java index ce80198c62..93cd2c5e06 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/workflow/workflowmultipleactivation/WorkflowMultipleActivationIT.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/workflow/workflowmultipleactivation/WorkflowMultipleActivationIT.java @@ -16,6 +16,7 @@ package io.javaoperatorsdk.operator.workflow.workflowmultipleactivation; import java.time.Duration; +import java.util.concurrent.atomic.AtomicInteger; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.RegisterExtension; @@ -115,10 +116,7 @@ void deactivatingAndReactivatingDependent() { assertThat(cm.getData()).containsEntry(DATA_KEY, CHANGED_VALUE); }); - var numOfReconciliation = - extension - .getReconcilerOfType(WorkflowMultipleActivationReconciler.class) - .getNumberOfReconciliationExecution(); + var numOfReconciliation = awaitStableReconciliationCount(); var actualCM = extension.get(ConfigMap.class, TEST_RESOURCE1); actualCM.getData().put("data2", "additionaldata"); extension.replace(actualCM); @@ -146,6 +144,27 @@ void deactivatingAndReactivatingDependent() { }); } + /** + * Snapshots the reconciliation count only once it has stopped changing. The framework adds the + * finalizer with a cache-only (non event-filtered) write, so each (re)creation of the resource + * emits a finalizer-add event that drives a follow-up reconciliation. A few of these may still be + * trailing from the preceding create/delete/recreate and spec changes; capturing the count before + * they settle would race with them and inflate the later assertion. + */ + private int awaitStableReconciliationCount() { + var reconciler = extension.getReconcilerOfType(WorkflowMultipleActivationReconciler.class); + var lastSeen = new AtomicInteger(-1); + await() + .pollInterval(Duration.ofMillis(POLL_DELAY)) + .atMost(Duration.ofSeconds(10)) + .until( + () -> { + int current = reconciler.getNumberOfReconciliationExecution(); + return current == lastSeen.getAndSet(current); + }); + return lastSeen.get(); + } + WorkflowMultipleActivationCustomResource testResource(String name) { var res = new WorkflowMultipleActivationCustomResource(); res.setMetadata(new ObjectMetaBuilder().withName(name).build());