Skip to content

feat: add feature switch to use SSA for managing finalizer #2845

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 4 commits into
base: next
Choose a base branch
from
Open

feat: add feature switch to use SSA for managing finalizer #2845

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
5b411e5
feat: add feature switch to use SSA for managing finalizer
csviri Jun 30, 2025
bcf6227
test fix:
csviri Jun 30, 2025
784df22
javadoc
csviri Jun 30, 2025
70e8910
refine
csviri Jun 30, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 13 additions & 2 deletions ...ework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -493,8 +493,7 @@ default boolean parseResourceVersionsForEventFilteringAndCaching() {

/**
* {@link io.javaoperatorsdk.operator.api.reconciler.UpdateControl} patch resource or status can
* either use simple patches or SSA. Setting this to {@code true}, controllers will use SSA for
* adding finalizers, patching resources and status.
* either use simple patches or SSA. Setting this to {@code true}, patching resources and status.
*
* @return {@code true} if Server-Side Apply (SSA) should be used when patching the primary
* resources, {@code false} otherwise
Expand All @@ -505,6 +504,18 @@ default boolean useSSAToPatchPrimaryResource() {
return true;
}

/**
* Setting this to {@code true}, controllers will use SSA for adding finalizers.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might help to document why it might be a good idea to set this to false, in particular point to the Kubernetes issue.

*
* @return {@code true} if Server-Side Apply (SSA) should be used when managing finalizers, {@code
* false} otherwise
* @see ConfigurationServiceOverrider#withUseSSAToAddFinalizer(boolean)
* @since 5.1.2
*/
default boolean useSSAToAddFinalizer() {
return true;
}

/**
* Determines whether resources retrieved from caches such as via calls to {@link
* Context#getSecondaryResource(Class)} should be defensively cloned first.
Expand Down
12 changes: 12 additions & 0 deletions ...e/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationServiceOverrider.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ public class ConfigurationServiceOverrider {
private Boolean previousAnnotationForDependentResources;
private Boolean parseResourceVersions;
private Boolean useSSAToPatchPrimaryResource;
private Boolean useSSAToAddFinalizer;
private Boolean cloneSecondaryResourcesWhenGettingFromCache;
private Set<Class<? extends HasMetadata>> previousAnnotationUsageBlocklist;

Expand Down Expand Up @@ -183,6 +184,11 @@ public ConfigurationServiceOverrider withUseSSAToPatchPrimaryResource(boolean va
return this;
}

public ConfigurationServiceOverrider withUseSSAToAddFinalizer(boolean value) {
this.useSSAToAddFinalizer = value;
return this;
}

public ConfigurationServiceOverrider withCloneSecondaryResourcesWhenGettingFromCache(
boolean value) {
this.cloneSecondaryResourcesWhenGettingFromCache = value;
Expand Down Expand Up @@ -336,6 +342,12 @@ public boolean useSSAToPatchPrimaryResource() {
useSSAToPatchPrimaryResource, ConfigurationService::useSSAToPatchPrimaryResource);
}

@Override
public boolean useSSAToAddFinalizer() {
return overriddenValueOrDefault(
useSSAToAddFinalizer, ConfigurationService::useSSAToPatchPrimaryResource);
}

@Override
public boolean cloneSecondaryResourcesWhenGettingFromCache() {
return overriddenValueOrDefault(
Expand Down
4 changes: 3 additions & 1 deletion .../src/main/java/io/javaoperatorsdk/operator/processing/event/ReconciliationDispatcher.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ class ReconciliationDispatcher<P extends HasMetadata> {
private final boolean retryConfigurationHasZeroAttempts;
private final Cloner cloner;
private final boolean useSSA;
private final boolean useSSAToAddFinalizer;

ReconciliationDispatcher(Controller<P> controller, CustomResourceFacade<P> customResourceFacade) {
this.controller = controller;
Expand All @@ -52,6 +53,7 @@ class ReconciliationDispatcher<P extends HasMetadata> {
var retry = configuration.getRetry();
retryConfigurationHasZeroAttempts = retry == null || retry.initExecution().isLastAttempt();
useSSA = configuration.getConfigurationService().useSSAToPatchPrimaryResource();
useSSAToAddFinalizer = configuration.getConfigurationService().useSSAToAddFinalizer();
}

public ReconciliationDispatcher(Controller<P> controller) {
Expand Down Expand Up @@ -119,7 +121,7 @@ private PostExecutionControl<P> handleReconcile(
* finalizer.
*/
P updatedResource;
if (useSSA) {
if (useSSAToAddFinalizer) {
updatedResource = addFinalizerWithSSA(originalResource);
} else {
updatedResource = updateCustomResourceWithFinalizer(resourceForExecution, originalResource);
Expand Down
17 changes: 9 additions & 8 deletions .../test/java/io/javaoperatorsdk/operator/processing/event/ReconciliationDispatcherTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,18 +62,18 @@ class ReconciliationDispatcherTest {

@BeforeEach
void setup() {
initConfigService(true);
initConfigService(true, true);
testCustomResource = TestUtils.testCustomResource();
reconciler = spy(new TestReconciler());
reconciliationDispatcher =
init(testCustomResource, reconciler, null, customResourceFacade, true);
}

static void initConfigService(boolean useSSA) {
initConfigService(useSSA, true);
static void initConfigService(boolean useSSA, boolean useSSAForFinalizer) {
initConfigService(useSSA, useSSAForFinalizer, true);
}

static void initConfigService(boolean useSSA, boolean noCloning) {
static void initConfigService(boolean useSSA, boolean useSSAForFinalizer, boolean noCloning) {
/*
* We need this for mock reconcilers to properly generate the expected UpdateControl: without
* this, calls such as `when(reconciler.reconcile(eq(testCustomResource),
Expand All @@ -98,7 +98,8 @@ public <R extends HasMetadata> R clone(R object) {
}
}
})
.withUseSSAToPatchPrimaryResource(useSSA));
.withUseSSAToPatchPrimaryResource(useSSA)
.withUseSSAToAddFinalizer(useSSAForFinalizer));
}

private <R extends HasMetadata> ReconciliationDispatcher<R> init(
Expand Down Expand Up @@ -149,7 +150,7 @@ void addFinalizerOnNewResource() {

@Test
void addFinalizerOnNewResourceWithoutSSA() {
initConfigService(false);
initConfigService(false, false);
final ReconciliationDispatcher<TestCustomResource> dispatcher =
init(testCustomResource, reconciler, null, customResourceFacade, true);

Expand Down Expand Up @@ -640,7 +641,7 @@ void canSkipSchedulingMaxDelayIf() {

@Test
void retriesAddingFinalizerWithoutSSA() {
initConfigService(false);
initConfigService(true, false);
reconciliationDispatcher =
init(testCustomResource, reconciler, null, customResourceFacade, true);

Expand Down Expand Up @@ -683,7 +684,7 @@ void reSchedulesFromErrorHandler() {

@Test
void reconcilerContextUsesTheSameInstanceOfResourceAsParam() {
initConfigService(false, false);
initConfigService(false, false, false);

final ReconciliationDispatcher<TestCustomResource> dispatcher =
init(testCustomResource, reconciler, null, customResourceFacade, true);
Expand Down