Skip to content

Commit 5842721

Browse files
csviricsviri
authored
feat: use ssa for status update by default from UpdateControl (#2278)
 Signed-off-by: Attila Mészáros <[email protected]>
1 parent 986d20a commit 5842721

File tree

8 files changed

+240
-50
lines changed

8 files changed

+240
-50
lines changed

‎docs/documentation/v5-0-migration.md

+8-1
Original file line numberDiff line numberDiff line change
@@ -17,5 +17,12 @@ 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. `ManagedDependentResourceContext` has been renamed to `ManagedWorkflowAndDependentResourceContext` and is accessed
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)
23+
!!! IMPORTANT !!!
24+
Migration from a non-SSA based controller to SSA based controller can cause problems, due to known issues.
25+
See the 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) where it is demonstrated.
26+
Also, the related part of a [workaround](https://github.com/operator-framework/java-operator-sdk/blob/main/operator-framework/src/test/java/io/javaoperatorsdk/operator/StatusPatchSSAMigrationIT.java#L110-L116).
27+
4. `ManagedDependentResourceContext` has been renamed to `ManagedWorkflowAndDependentResourceContext` and is accessed
2128
via the accordingly renamed `managedWorkflowAndDependentResourceContext` method.

‎operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationService.java

+10
Original file line numberDiff line numberDiff line change
@@ -391,4 +391,14 @@ default boolean parseResourceVersionsForEventFilteringAndCaching() {
391391
return false;
392392
}
393393

394+
/**
395+
* {@link io.javaoperatorsdk.operator.api.reconciler.UpdateControl} patchStatus can either use
396+
* simple update or SSA for status subresource patching.
397+
*
398+
* @return true by default
399+
*/
400+
default boolean useSSAForResourceStatusPatch() {
401+
return true;
402+
}
403+
394404
}

‎operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationServiceOverrider.java

+15-1
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ public class ConfigurationServiceOverrider {
4040
private Set<Class<? extends HasMetadata>> defaultNonSSAResource;
4141
private Boolean previousAnnotationForDependentResources;
4242
private Boolean parseResourceVersions;
43+
private Boolean useSSAForResourceStatusPatch;
4344
private DependentResourceFactory dependentResourceFactory;
4445

4546
ConfigurationServiceOverrider(ConfigurationService original) {
@@ -174,12 +175,17 @@ public ConfigurationServiceOverrider withPreviousAnnotationForDependentResources
174175
return this;
175176
}
176177

177-
public ConfigurationServiceOverrider wihtParseResourceVersions(
178+
public ConfigurationServiceOverrider withParseResourceVersions(
178179
boolean value) {
179180
this.parseResourceVersions = value;
180181
return this;
181182
}
182183

184+
public ConfigurationServiceOverrider withUseSSAForResourceStatusPatch(boolean value) {
185+
this.useSSAForResourceStatusPatch = value;
186+
return this;
187+
}
188+
183189
public ConfigurationService build() {
184190
return new BaseConfigurationService(original.getVersion(), cloner, client) {
185191
@Override
@@ -312,6 +318,14 @@ public boolean parseResourceVersionsForEventFilteringAndCaching() {
312318
? parseResourceVersions
313319
: super.parseResourceVersionsForEventFilteringAndCaching();
314320
}
321+
322+
@Override
323+
public boolean useSSAForResourceStatusPatch() {
324+
return useSSAForResourceStatusPatch != null
325+
? useSSAForResourceStatusPatch
326+
: super.useSSAForResourceStatusPatch();
327+
328+
}
315329
};
316330
}
317331

‎operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/ReconciliationDispatcher.java

+46-32
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@
1212
import io.fabric8.kubernetes.client.KubernetesClientException;
1313
import io.fabric8.kubernetes.client.dsl.MixedOperation;
1414
import io.fabric8.kubernetes.client.dsl.Resource;
15+
import io.fabric8.kubernetes.client.dsl.base.PatchContext;
16+
import io.fabric8.kubernetes.client.dsl.base.PatchType;
1517
import io.javaoperatorsdk.operator.OperatorException;
1618
import io.javaoperatorsdk.operator.api.ObservedGenerationAware;
1719
import io.javaoperatorsdk.operator.api.config.Cloner;
@@ -25,9 +27,7 @@
2527
import io.javaoperatorsdk.operator.api.reconciler.UpdateControl;
2628
import io.javaoperatorsdk.operator.processing.Controller;
2729

28-
import static io.javaoperatorsdk.operator.processing.KubernetesResourceUtils.getName;
29-
import static io.javaoperatorsdk.operator.processing.KubernetesResourceUtils.getUID;
30-
import static io.javaoperatorsdk.operator.processing.KubernetesResourceUtils.getVersion;
30+
import static io.javaoperatorsdk.operator.processing.KubernetesResourceUtils.*;
3131

3232
/**
3333
* Handles calls and results of a Reconciler and finalizer related logic
@@ -45,8 +45,7 @@ class ReconciliationDispatcher<P extends HasMetadata> {
4545
private final boolean retryConfigurationHasZeroAttempts;
4646
private final Cloner cloner;
4747

48-
ReconciliationDispatcher(Controller<P> controller,
49-
CustomResourceFacade<P> customResourceFacade) {
48+
ReconciliationDispatcher(Controller<P> controller, CustomResourceFacade<P> customResourceFacade) {
5049
this.controller = controller;
5150
this.customResourceFacade = customResourceFacade;
5251
this.cloner = controller.getConfiguration().getConfigurationService().getResourceCloner();
@@ -56,7 +55,8 @@ class ReconciliationDispatcher<P extends HasMetadata> {
5655
}
5756

5857
public ReconciliationDispatcher(Controller<P> controller) {
59-
this(controller, new CustomResourceFacade<>(controller.getCRClient()));
58+
this(controller,
59+
new CustomResourceFacade<>(controller.getCRClient(), controller.getConfiguration()));
6060
}
6161

6262
public PostExecutionControl<P> handleExecution(ExecutionScope<P> executionScope) {
@@ -138,33 +138,25 @@ private PostExecutionControl<P> reconcileExecution(ExecutionScope<P> executionSc
138138

139139
UpdateControl<P> updateControl = controller.reconcile(resourceForExecution, context);
140140
P updatedCustomResource = null;
141+
final var toUpdate =
142+
updateControl.isNoUpdate() ? originalResource : updateControl.getResource();
141143
if (updateControl.isUpdateResourceAndStatus()) {
142-
updatedCustomResource =
143-
updateCustomResource(updateControl.getResource());
144-
updateControl
145-
.getResource()
144+
updatedCustomResource = updateCustomResource(toUpdate);
145+
toUpdate
146146
.getMetadata()
147147
.setResourceVersion(updatedCustomResource.getMetadata().getResourceVersion());
148-
updatedCustomResource =
149-
updateStatusGenerationAware(updateControl.getResource(), originalResource,
150-
updateControl.isPatchStatus());
151-
} else if (updateControl.isUpdateStatus()) {
152-
updatedCustomResource =
153-
updateStatusGenerationAware(updateControl.getResource(), originalResource,
154-
updateControl.isPatchStatus());
155148
} else if (updateControl.isUpdateResource()) {
149+
updatedCustomResource = updateCustomResource(toUpdate);
150+
}
151+
152+
// check if status also needs to be updated
153+
final var updateObservedGeneration = updateControl.isNoUpdate()
154+
? shouldUpdateObservedGenerationAutomatically(resourceForExecution)
155+
: shouldUpdateObservedGenerationAutomatically(updatedCustomResource);
156+
if (updateControl.isUpdateResourceAndStatus() || updateControl.isUpdateStatus()
157+
|| updateObservedGeneration) {
156158
updatedCustomResource =
157-
updateCustomResource(updateControl.getResource());
158-
if (shouldUpdateObservedGenerationAutomatically(updatedCustomResource)) {
159-
updatedCustomResource =
160-
updateStatusGenerationAware(updateControl.getResource(), originalResource,
161-
updateControl.isPatchStatus());
162-
}
163-
} else if (updateControl.isNoUpdate()
164-
&& shouldUpdateObservedGenerationAutomatically(resourceForExecution)) {
165-
updatedCustomResource =
166-
updateStatusGenerationAware(originalResource, originalResource,
167-
updateControl.isPatchStatus());
159+
updateStatusGenerationAware(toUpdate, originalResource, updateControl.isPatchStatus());
168160
}
169161
return createPostExecutionControl(updatedCustomResource, updateControl);
170162
}
@@ -379,10 +371,16 @@ public P conflictRetryingUpdate(P resource, Function<P, Boolean> modificationFun
379371
static class CustomResourceFacade<R extends HasMetadata> {
380372

381373
private final MixedOperation<R, KubernetesResourceList<R>, Resource<R>> resourceOperation;
374+
private final boolean useSSAToUpdateStatus;
375+
private final String fieldManager;
382376

383377
public CustomResourceFacade(
384-
MixedOperation<R, KubernetesResourceList<R>, Resource<R>> resourceOperation) {
378+
MixedOperation<R, KubernetesResourceList<R>, Resource<R>> resourceOperation,
379+
ControllerConfiguration<R> configuration) {
385380
this.resourceOperation = resourceOperation;
381+
this.useSSAToUpdateStatus =
382+
configuration.getConfigurationService().useSSAForResourceStatusPatch();
383+
this.fieldManager = configuration.fieldManager();
386384
}
387385

388386
public R getResource(String namespace, String name) {
@@ -412,14 +410,30 @@ public R updateStatus(R resource) {
412410
}
413411

414412
public R patchStatus(R resource, R originalResource) {
415-
log.trace("Updating status for resource: {}", resource);
413+
log.trace("Patching status for resource: {} with ssa: {}", resource, useSSAToUpdateStatus);
416414
String resourceVersion = resource.getMetadata().getResourceVersion();
417415
// don't do optimistic locking on patch
418416
originalResource.getMetadata().setResourceVersion(null);
419417
resource.getMetadata().setResourceVersion(null);
420418
try {
421-
return resource(originalResource)
422-
.editStatus(r -> resource);
419+
if (useSSAToUpdateStatus) {
420+
var managedFields = resource.getMetadata().getManagedFields();
421+
try {
422+
423+
resource.getMetadata().setManagedFields(null);
424+
var res = resource(resource);
425+
return res.subresource("status").patch(new PatchContext.Builder()
426+
.withFieldManager(fieldManager)
427+
.withForce(true)
428+
.withPatchType(PatchType.SERVER_SIDE_APPLY)
429+
.build());
430+
} finally {
431+
resource.getMetadata().setManagedFields(managedFields);
432+
}
433+
} else {
434+
var res = resource(originalResource);
435+
return res.editStatus(r -> resource);
436+
}
423437
} finally {
424438
// restore initial resource version
425439
originalResource.getMetadata().setResourceVersion(resourceVersion);

‎operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/ReconciliationDispatcherTest.java

+2-12
Original file line numberDiff line numberDiff line change
@@ -43,19 +43,9 @@
4343
import static io.javaoperatorsdk.operator.TestUtils.markForDeletion;
4444
import static io.javaoperatorsdk.operator.processing.event.ReconciliationDispatcher.MAX_UPDATE_RETRY;
4545
import static org.assertj.core.api.Assertions.assertThat;
46-
import static org.junit.jupiter.api.Assertions.assertEquals;
47-
import static org.junit.jupiter.api.Assertions.assertFalse;
48-
import static org.junit.jupiter.api.Assertions.assertTrue;
49-
import static org.junit.jupiter.api.Assertions.fail;
46+
import static org.junit.jupiter.api.Assertions.*;
5047
import static org.mockito.ArgumentMatchers.argThat;
51-
import static org.mockito.Mockito.any;
52-
import static org.mockito.Mockito.eq;
53-
import static org.mockito.Mockito.mock;
54-
import static org.mockito.Mockito.never;
55-
import static org.mockito.Mockito.spy;
56-
import static org.mockito.Mockito.times;
57-
import static org.mockito.Mockito.verify;
58-
import static org.mockito.Mockito.when;
48+
import static org.mockito.Mockito.*;
5949

6050
@SuppressWarnings({"unchecked", "rawtypes"})
6151
class ReconciliationDispatcherTest {

‎operator-framework/src/test/java/io/javaoperatorsdk/operator/StatusPatchNotLockingIT.java

+3-1
Original file line numberDiff line numberDiff line change
@@ -58,10 +58,12 @@ void valuesAreDeletedIfSetToNull() {
5858
assertThat(actual.getStatus().getMessage()).isEqualTo(MESSAGE);
5959
});
6060

61+
// resource needs to be read again to we don't replace the with wrong managed fields
62+
resource = operator.get(StatusPatchLockingCustomResource.class, TEST_RESOURCE_NAME);
6163
resource.getSpec().setMessageInStatus(false);
6264
operator.replace(resource);
6365

64-
await().untilAsserted(() -> {
66+
await().timeout(Duration.ofMinutes(3)).untilAsserted(() -> {
6567
var actual = operator.get(StatusPatchLockingCustomResource.class,
6668
TEST_RESOURCE_NAME);
6769
assertThat(actual.getStatus()).isNotNull();

0 commit comments

Comments
 (0)