Skip to content

Commit e39c09f

Browse files
csvirimetacosm
authored andcommitted
feat: using SSA for finalizer and primary patch (#2305)
Signed-off-by: Attila Mészáros <[email protected]>
1 parent 82bf32a commit e39c09f

File tree

48 files changed

+642
-419
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

48 files changed

+642
-419
lines changed

Diff for: docs/documentation/features.md

+35-16
Original file line numberDiff line numberDiff line change
@@ -91,17 +91,31 @@ Those are the typical use cases of resource updates, however in some cases there
9191
the controller wants to update the resource itself (for example to add annotations) or not perform
9292
any updates, which is also supported.
9393

94-
It is also possible to update both the status and the resource with the
95-
`updateResourceAndStatus` method. In this case, the resource is updated first followed by the
96-
status, using two separate requests to the Kubernetes API.
97-
98-
You should always state your intent using `UpdateControl` and let the SDK deal with the actual
99-
updates instead of performing these updates yourself using the actual Kubernetes client so that
100-
the SDK can update its internal state accordingly.
101-
102-
Resource updates are protected using optimistic version control, to make sure that other updates
103-
that might have occurred in the mean time on the server are not overwritten. This is ensured by
104-
setting the `resourceVersion` field on the processed resources.
94+
It is also possible to update both the status and the resource with the `patchResourceAndStatus` method. In this case,
95+
the resource is updated first followed by the status, using two separate requests to the Kubernetes API.
96+
97+
From v5 `UpdateControl` only supports patching the resources, by default
98+
using [Server Side Apply (SSA)](https://kubernetes.io/docs/reference/using-api/server-side-apply/).
99+
It is important to understand how SSA works in Kubernetes. Mainly, resources applied using SSA
100+
should contain only the fields identifying the resource and those the user is interested in (a 'fully specified intent'
101+
in Kubernetes parlance), thus usually using a resource created from scratch, see
102+
[sample](https://github.com/operator-framework/java-operator-sdk/blob/main/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/patchresourcewithssa/PatchResourceWithSSAReconciler.java#L18-L22).
103+
To contrast, see the same sample, this time [without SSA](https://github.com/operator-framework/java-operator-sdk/blob/main/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/patchresourceandstatusnossa/PatchResourceAndStatusNoSSAReconciler.java#L16-L16).
104+
105+
Non-SSA based patch is still supported.
106+
You can control whether or not to use SSA
107+
using [`ConfigurationServcice.useSSAToPatchPrimaryResource()`](https://github.com/operator-framework/java-operator-sdk/blob/main/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationService.java#L385-L385)
108+
and the related `ConfigurationServiceOverrider.withUseSSAToPatchPrimaryResource` method.
109+
Related integration test can be
110+
found [here](https://github.com/operator-framework/java-operator-sdk/blob/main/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/patchresourceandstatusnossa/PatchResourceAndStatusNoSSAReconciler.java).
111+
112+
Handling resources directly using the client, instead of delegating these updates operations to JOSDK by returning
113+
an `UpdateControl` at the end of your reconciliation, should work appropriately. However, we do recommend to
114+
use `UpdateControl` instead since JOSDK makes sure that the operations are handled properly, since there are subtleties
115+
to be aware of. For example, if you are using a finalizer, JOSDK makes sure to include it in your fully specified intent
116+
so that it is not unintentionally removed from the resource (which would happen if you omit it, since your controller is
117+
the designated manager for that field and Kubernetes interprets the finalizer being gone from the specified intent as a
118+
request for removal).
105119

106120
[`DeleteControl`](https://github.com/java-operator-sdk/java-operator-sdk/blob/main/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/DeleteControl.java)
107121
typically instructs the framework to remove the finalizer after the dependent
@@ -170,6 +184,8 @@ You can specify the name of the finalizer to use for your `Reconciler` using the
170184
[`@ControllerConfiguration`](https://github.com/java-operator-sdk/java-operator-sdk/blob/main/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/ControllerConfiguration.java)
171185
annotation. If you do not specify a finalizer name, one will be automatically generated for you.
172186

187+
From v5 by default finalizer is added using Served Side Apply. See also UpdateControl in docs.
188+
173189
## Automatic Observed Generation Handling
174190

175191
Having an `.observedGeneration` value on your resources' status is a best practice to
@@ -187,11 +203,14 @@ In order to have this feature working:
187203
So the status should be instantiated when the object is returned using the `UpdateControl`.
188204

189205
If these conditions are fulfilled and generation awareness is activated, the observed generation
190-
is automatically set by the framework after the `reconcile` method is called. Note that the
191-
observed generation is also updated even when `UpdateControl.noUpdate()` is returned from the
192-
reconciler. See this feature at work in
193-
the [WebPage example](https://github.com/java-operator-sdk/java-operator-sdk/blob/main/sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/WebPageStatus.java#L5)
194-
.
206+
is automatically set by the framework after the `reconcile` method is called.
207+
208+
When using SSA based patches, the observed generation is only updated when `UpdateControl.patchStatus` or
209+
`UpdateControl.patchResourceAndStatus` is returned. In case the of non-SSA based patches
210+
the observed generation is also updated even when `UpdateControl.noUpdate()` is returned from the
211+
reconciler.
212+
See this feature at work in the [WebPage example](https://github.com/java-operator-sdk/java-operator-sdk/blob/main/sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/WebPageStatus.java#L5).
213+
See turning off an on the SSA based patching at [`ConfigurationServcice.useSSAToPatchPrimaryResource()`](https://github.com/operator-framework/java-operator-sdk/blob/main/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationService.java#L385-L385).
195214

196215
```java
197216
public class WebPageStatus extends ObservedGenerationAwareStatus {

Diff for: docs/documentation/v5-0-migration.md

+12-6
Original file line numberDiff line numberDiff line change
@@ -17,15 +17,21 @@ permalink: /docs/v5-0-migration
1717
[`EventSourceUtils`](https://github.com/operator-framework/java-operator-sdk/blob/main/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/EventSourceUtils.java#L11-L11)
1818
now contains all the utility methods used for event sources naming that were previously defined in
1919
the `EventSourceInitializer` interface.
20-
3. Patching status through `UpdateControl` like the `patchStatus` method now by default
21-
uses Server Side Apply instead of simple patch. To use the former approach, use the feature flag
22-
in [`ConfigurationService`](https://github.com/operator-framework/java-operator-sdk/blob/main/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationService.java#L400-L400)
20+
3. Updates through `UpdateControl` now use [Server Side Apply (SSA)](https://kubernetes.io/docs/reference/using-api/server-side-apply/) by default to add the finalizer and for all
21+
the patch operations in `UpdateControl`. The update operations were removed. If you do not wish to use SSA, you can deactivate the feature using `ConfigurationService.useSSAToPatchPrimaryResource` and related `ConfigurationServiceOverrider.withUseSSAToPatchPrimaryResource`.
22+
2323
!!! IMPORTANT !!!
24-
Migration from a non-SSA based controller to SSA based controller can cause problems, due to known issues.
25-
See the
26-
following [integration test](https://github.com/operator-framework/java-operator-sdk/blob/main/operator-framework/src/test/java/io/javaoperatorsdk/operator/StatusPatchSSAMigrationIT.java#L71-L82)
24+
25+
See known issues with migration from non-SSA to SSA based status updates here:
26+
[integration test](https://github.com/operator-framework/java-operator-sdk/blob/main/operator-framework/src/test/java/io/javaoperatorsdk/operator/StatusPatchSSAMigrationIT.java#L71-L82)
2727
where it is demonstrated. Also, the related part of
2828
a [workaround](https://github.com/operator-framework/java-operator-sdk/blob/main/operator-framework/src/test/java/io/javaoperatorsdk/operator/StatusPatchSSAMigrationIT.java#L110-L116).
29+
30+
Related automatic observed generation handling changes:
31+
Automated Observed Generation (see features in docs), is automatically handled for non-SSA, even if
32+
the status sub-resource is not instructed to be updated. This is not true for SSA, observed generation is updated
33+
only when patch status is instructed by `UpdateControl`.
34+
2935
4. `ManagedDependentResourceContext` has been renamed to `ManagedWorkflowAndDependentResourceContext` and is accessed
3036
via the accordingly renamed `managedWorkflowAndDependentResourceContext` method.
3137
5. `ResourceDiscriminator` was removed. In most of the cases you can just delete the discriminator, everything should

Diff for: operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationService.java

+7-4
Original file line numberDiff line numberDiff line change
@@ -386,12 +386,15 @@ default boolean parseResourceVersionsForEventFilteringAndCaching() {
386386
}
387387

388388
/**
389-
* {@link io.javaoperatorsdk.operator.api.reconciler.UpdateControl} patchStatus can either use
390-
* simple update or SSA for status subresource patching.
389+
* {@link io.javaoperatorsdk.operator.api.reconciler.UpdateControl} patch resource or status can
390+
* either use simple patches or SSA. Setting this to {@code true}, controllers will use SSA for
391+
* adding finalizers, managing observed generation, patching resources and status.
391392
*
392-
* @return true by default
393+
* @return {@code true} by default
394+
* @since 5.0.0
395+
* @see ConfigurationServiceOverrider#withUseSSAToPatchPrimaryResource(boolean)
393396
*/
394-
default boolean useSSAForResourceStatusPatch() {
397+
default boolean useSSAToPatchPrimaryResource() {
395398
return true;
396399
}
397400

Diff for: operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationServiceOverrider.java

+7-7
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ public class ConfigurationServiceOverrider {
3939
private Set<Class<? extends HasMetadata>> defaultNonSSAResource;
4040
private Boolean previousAnnotationForDependentResources;
4141
private Boolean parseResourceVersions;
42-
private Boolean useSSAForResourceStatusPatch;
42+
private Boolean useSSAToPatchPrimaryResource;
4343
private DependentResourceFactory dependentResourceFactory;
4444

4545
ConfigurationServiceOverrider(ConfigurationService original) {
@@ -175,8 +175,8 @@ public ConfigurationServiceOverrider withParseResourceVersions(
175175
return this;
176176
}
177177

178-
public ConfigurationServiceOverrider withUseSSAForResourceStatusPatch(boolean value) {
179-
this.useSSAForResourceStatusPatch = value;
178+
public ConfigurationServiceOverrider withUseSSAToPatchPrimaryResource(boolean value) {
179+
this.useSSAToPatchPrimaryResource = value;
180180
return this;
181181
}
182182

@@ -317,10 +317,10 @@ public boolean parseResourceVersionsForEventFilteringAndCaching() {
317317
}
318318

319319
@Override
320-
public boolean useSSAForResourceStatusPatch() {
321-
return useSSAForResourceStatusPatch != null
322-
? useSSAForResourceStatusPatch
323-
: super.useSSAForResourceStatusPatch();
320+
public boolean useSSAToPatchPrimaryResource() {
321+
return useSSAToPatchPrimaryResource != null
322+
? useSSAToPatchPrimaryResource
323+
: super.useSSAToPatchPrimaryResource();
324324

325325
}
326326
};

Diff for: operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/ErrorStatusUpdateControl.java

+3-13
Original file line numberDiff line numberDiff line change
@@ -9,24 +9,18 @@ public class ErrorStatusUpdateControl<P extends HasMetadata>
99
extends BaseControl<ErrorStatusUpdateControl<P>> {
1010

1111
private final P resource;
12-
private final boolean patch;
1312
private boolean noRetry = false;
1413

1514
public static <T extends HasMetadata> ErrorStatusUpdateControl<T> patchStatus(T resource) {
16-
return new ErrorStatusUpdateControl<>(resource, true);
17-
}
18-
19-
public static <T extends HasMetadata> ErrorStatusUpdateControl<T> updateStatus(T resource) {
20-
return new ErrorStatusUpdateControl<>(resource, false);
15+
return new ErrorStatusUpdateControl<>(resource);
2116
}
2217

2318
public static <T extends HasMetadata> ErrorStatusUpdateControl<T> noStatusUpdate() {
24-
return new ErrorStatusUpdateControl<>(null, true);
19+
return new ErrorStatusUpdateControl<>(null);
2520
}
2621

27-
private ErrorStatusUpdateControl(P resource, boolean patch) {
22+
private ErrorStatusUpdateControl(P resource) {
2823
this.resource = resource;
29-
this.patch = patch;
3024
}
3125

3226
/**
@@ -47,10 +41,6 @@ public boolean isNoRetry() {
4741
return noRetry;
4842
}
4943

50-
public boolean isPatch() {
51-
return patch;
52-
}
53-
5444
/**
5545
* If re-scheduled using this method, it is not considered as retry, it effectively cancels retry.
5646
*
Original file line numberDiff line numberDiff line change
@@ -1,150 +1,80 @@
11
package io.javaoperatorsdk.operator.api.reconciler;
22

3+
import java.util.Optional;
4+
35
import io.fabric8.kubernetes.api.model.HasMetadata;
46
import io.fabric8.kubernetes.client.CustomResource;
57

68
public class UpdateControl<P extends HasMetadata> extends BaseControl<UpdateControl<P>> {
79

810
private final P resource;
9-
private final boolean updateStatus;
10-
private final boolean updateResource;
11+
private final boolean patchResource;
1112
private final boolean patchStatus;
1213

1314
private UpdateControl(
14-
P resource, boolean updateStatus, boolean updateResource, boolean patchStatus) {
15-
if ((updateResource || updateStatus) && resource == null) {
15+
P resource, boolean patchResource, boolean patchStatus) {
16+
if ((patchResource || patchStatus) && resource == null) {
1617
throw new IllegalArgumentException("CustomResource cannot be null in case of update");
1718
}
1819
this.resource = resource;
19-
this.updateStatus = updateStatus;
20-
this.updateResource = updateResource;
20+
this.patchResource = patchResource;
2121
this.patchStatus = patchStatus;
2222
}
2323

24-
/**
25-
* Creates an update control instance that instructs the framework to do an update on resource
26-
* itself, not on the status. Note that usually as a results of a reconciliation should be a
27-
* status update not an update to the resource itself.
28-
*
29-
* Using this update makes sure that the resource in the next reconciliation is the updated one -
30-
* this is not guaranteed by default if you do an update on a resource by the Kubernetes client.
31-
*
32-
* @param <T> custom resource type
33-
* @param customResource customResource to use for update
34-
* @return initialized update control
35-
*/
36-
public static <T extends HasMetadata> UpdateControl<T> updateResource(T customResource) {
37-
return new UpdateControl<>(customResource, false, true, false);
38-
}
39-
4024
/**
4125
* Preferred way to update the status. It does not do optimistic locking. Uses JSON Patch to patch
4226
* the resource.
4327
* <p>
44-
* Note that this does not work, if the {@link CustomResource#initStatus() initStatus} is
45-
* implemented, since it breaks the diffing process. Don't implement it if using this method.
28+
* Note that this does not work, if the {@link CustomResource#initStatus()} is implemented, since
29+
* it breaks the diffing process. Don't implement it if using this method.
4630
* </p>
47-
* There is also an issue with setting value to null with older Kubernetes versions (1.19 and
48-
* below). See: <a href=
31+
* There is also an issue with setting value to {@code null} with older Kubernetes versions (1.19
32+
* and below). See: <a href=
4933
* "https://github.com/fabric8io/kubernetes-client/issues/4158">https://github.com/fabric8io/kubernetes-client/issues/4158</a>
5034
*
5135
* @param <T> resource type
5236
* @param customResource the custom resource with target status
5337
* @return UpdateControl instance
5438
*/
5539
public static <T extends HasMetadata> UpdateControl<T> patchStatus(T customResource) {
56-
return new UpdateControl<>(customResource, true, false, true);
57-
}
58-
59-
/**
60-
* Note that usually "patchStatus" is advised to be used instead of this method.
61-
* <p>
62-
* Updates the status with optimistic locking regarding current resource version reconciled. Note
63-
* that this also ensures that on next reconciliation is the most up-to-date custom resource is
64-
* used.
65-
* </p>
66-
*
67-
* @param <T> resource type
68-
* @param customResource the custom resource with target status
69-
* @return UpdateControl instance
70-
*/
71-
public static <T extends HasMetadata> UpdateControl<T> updateStatus(T customResource) {
72-
return new UpdateControl<>(customResource, true, false, false);
73-
}
74-
75-
/**
76-
* As a results of this there will be two call to K8S API. First the custom resource will be
77-
* updates then the status sub-resource.
78-
*
79-
* Using this update makes sure that the resource in the next reconciliation is the updated one -
80-
* this is not guaranteed by default if you do an update on a resource by the Kubernetes client.
81-
*
82-
* @param <T> resource type
83-
* @param customResource - custom resource to use in both API calls
84-
* @return UpdateControl instance
85-
*/
86-
public static <T extends HasMetadata> UpdateControl<T> updateResourceAndStatus(
87-
T customResource) {
88-
return new UpdateControl<>(customResource, true, true, false);
40+
return new UpdateControl<>(customResource, false, true);
8941
}
9042

91-
/**
92-
* Updates the resource - with optimistic locking - and patches the status without optimistic
93-
* locking in place.
94-
*
95-
* Note that using this method, it is not guaranteed that the most recent updated resource will be
96-
* in case for next reconciliation.
97-
*
98-
* @param customResource to update
99-
* @return UpdateControl instance
100-
* @param <T> resource type
101-
*/
102-
public static <T extends HasMetadata> UpdateControl<T> updateResourceAndPatchStatus(
103-
T customResource) {
104-
return new UpdateControl<>(customResource, true, true, true);
43+
public static <T extends HasMetadata> UpdateControl<T> patchResource(T customResource) {
44+
return new UpdateControl<>(customResource, true, false);
10545
}
10646

10747
/**
108-
* Marked for removal, because of confusing name. It does not patch the resource but rather
109-
* updates it.
110-
*
111-
* @deprecated use {@link UpdateControl#updateResourceAndPatchStatus(HasMetadata)}
112-
*
11348
* @param customResource to update
11449
* @return UpdateControl instance
11550
* @param <T> resource type
11651
*/
117-
@Deprecated(forRemoval = true)
11852
public static <T extends HasMetadata> UpdateControl<T> patchResourceAndStatus(T customResource) {
119-
return updateResourceAndStatus(customResource);
53+
return new UpdateControl<>(customResource, true, true);
12054
}
12155

12256
public static <T extends HasMetadata> UpdateControl<T> noUpdate() {
123-
return new UpdateControl<>(null, false, false, false);
124-
}
125-
126-
public P getResource() {
127-
return resource;
57+
return new UpdateControl<>(null, false, false);
12858
}
12959

130-
public boolean isUpdateStatus() {
131-
return updateStatus;
60+
public Optional<P> getResource() {
61+
return Optional.ofNullable(resource);
13262
}
13363

134-
public boolean isUpdateResource() {
135-
return updateResource;
64+
public boolean isPatchResource() {
65+
return patchResource;
13666
}
13767

13868
public boolean isPatchStatus() {
13969
return patchStatus;
14070
}
14171

14272
public boolean isNoUpdate() {
143-
return !updateResource && !updateStatus;
73+
return !patchResource && !patchStatus;
14474
}
14575

146-
public boolean isUpdateResourceAndStatus() {
147-
return updateResource && updateStatus;
76+
public boolean isPatchResourceAndStatus() {
77+
return patchResource && patchStatus;
14878
}
14979

15080
}

Diff for: operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/Controller.java

+2-2
Original file line numberDiff line numberDiff line change
@@ -122,10 +122,10 @@ public String controllerName() {
122122
@Override
123123
public String successTypeName(UpdateControl<P> result) {
124124
String successType = RESOURCE;
125-
if (result.isUpdateStatus()) {
125+
if (result.isPatchStatus()) {
126126
successType = STATUS;
127127
}
128-
if (result.isUpdateResourceAndStatus()) {
128+
if (result.isPatchResourceAndStatus()) {
129129
successType = BOTH;
130130
}
131131
return successType;

Diff for: operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventProcessor.java

-11
Original file line numberDiff line numberDiff line change
@@ -245,17 +245,6 @@ synchronized void eventProcessingFinished(
245245
state.markProcessedMarkForDeletion();
246246
metrics.cleanupDoneFor(resourceID, metricsMetadata);
247247
} else {
248-
postExecutionControl
249-
.getUpdatedCustomResource()
250-
.ifPresent(
251-
p -> {
252-
if (!postExecutionControl.updateIsStatusPatch()) {
253-
eventSourceManager
254-
.getControllerResourceEventSource()
255-
.handleRecentResourceUpdate(
256-
ResourceID.fromResource(p), p, executionScope.getResource());
257-
}
258-
});
259248
if (state.eventPresent()) {
260249
submitReconciliationExecution(state);
261250
} else {

0 commit comments

Comments
 (0)