From 25fa1eb3a7fc9b83469a78be176d03d31ad5fb3b Mon Sep 17 00:00:00 2001 From: csviri Date: Mon, 12 Dec 2022 10:11:37 +0100 Subject: [PATCH] Revert "chore: temporarily revert SSA for finalizers (#1654)" This reverts commit c72baa2ee19c3e2a8a8d6cb2cb39ca5f0f0e7522. --- .../event/ReconciliationDispatcher.java | 47 ++----------------- .../event/ReconciliationDispatcherTest.java | 7 +-- .../operator/CachePruneIT.java | 2 - 3 files changed, 6 insertions(+), 50 deletions(-) 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 0328261c74..4a7ac68082 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 @@ -295,10 +295,8 @@ private PostExecutionControl

handleCleanup(P originalResource, P resource, // cleanup is finished, nothing left to done final var finalizerName = configuration().getFinalizerName(); if (deleteControl.isRemoveFinalizer() && resource.hasFinalizer(finalizerName)) { - P customResource = removeFinalizer(resource, finalizerName); - // todo: restore SSA - // P customResource = conflictRetryingPatch(resource, originalResource, - // r -> r.removeFinalizer(finalizerName)); + P customResource = conflictRetryingPatch(resource, originalResource, + r -> r.removeFinalizer(finalizerName)); return PostExecutionControl.customResourceFinalizerRemoved(customResource); } } @@ -313,49 +311,13 @@ private PostExecutionControl

handleCleanup(P originalResource, P resource, return postExecutionControl; } - // todo: remove after restoring SSA - public P removeFinalizer(P resource, String finalizer) { - if (log.isDebugEnabled()) { - log.debug("Removing finalizer on resource: {}", ResourceID.fromResource(resource)); - } - int retryIndex = 0; - while (true) { - try { - var removed = resource.removeFinalizer(finalizer); - if (!removed) { - return resource; - } - return customResourceFacade.updateResource(resource); - } catch (KubernetesClientException e) { - log.trace("Exception during finalizer removal for resource: {}", resource); - retryIndex++; - // only retry on conflict (HTTP 409), otherwise fail - if (e.getCode() != 409) { - throw e; - } - if (retryIndex >= MAX_FINALIZER_REMOVAL_RETRY) { - throw new OperatorException( - "Exceeded maximum (" + MAX_FINALIZER_REMOVAL_RETRY - + ") retry attempts to remove finalizer '" + finalizer + "' for resource " - + ResourceID.fromResource(resource)); - } - resource = customResourceFacade.getResource(resource.getMetadata().getNamespace(), - resource.getMetadata().getName()); - } - } - } - private P updateCustomResourceWithFinalizer(P resourceForExecution, P originalResource) { log.debug( "Adding finalizer for resource: {} version: {}", getUID(originalResource), getVersion(originalResource)); - originalResource.addFinalizer(configuration().getFinalizerName()); - return customResourceFacade.updateResource(originalResource); - - // todo: restore SSA - // return conflictRetryingPatch(resourceForExecution, originalResource, - // r -> r.addFinalizer(configuration().getFinalizerName())); + return conflictRetryingPatch(resourceForExecution, originalResource, + r -> r.addFinalizer(configuration().getFinalizerName())); } private P updateCustomResource(P resource) { @@ -434,6 +396,7 @@ public R updateResource(R resource) { .replace(); } + @SuppressWarnings({"rawtypes", "unchecked"}) public R updateStatus(R resource) { log.trace("Updating status for resource: {}", resource); return resource(resource) 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 2f43b3ad62..ef7a27677b 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 @@ -9,7 +9,6 @@ import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.BeforeEach; -import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; import org.mockito.ArgumentCaptor; import org.mockito.ArgumentMatcher; @@ -23,10 +22,7 @@ import io.javaoperatorsdk.operator.MockKubernetesClient; import io.javaoperatorsdk.operator.OperatorException; import io.javaoperatorsdk.operator.TestUtils; -import io.javaoperatorsdk.operator.api.config.Cloner; -import io.javaoperatorsdk.operator.api.config.ConfigurationServiceProvider; -import io.javaoperatorsdk.operator.api.config.ControllerConfiguration; -import io.javaoperatorsdk.operator.api.config.MockControllerConfiguration; +import io.javaoperatorsdk.operator.api.config.*; import io.javaoperatorsdk.operator.api.reconciler.Cleaner; import io.javaoperatorsdk.operator.api.reconciler.Context; import io.javaoperatorsdk.operator.api.reconciler.DeleteControl; @@ -59,7 +55,6 @@ import static org.mockito.Mockito.when; @SuppressWarnings({"unchecked", "rawtypes"}) -@Disabled(value = "todo: reactivate when restoring SSA") class ReconciliationDispatcherTest { private static final String DEFAULT_FINALIZER = "javaoperatorsdk.io/finalizer"; diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/CachePruneIT.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/CachePruneIT.java index 15106015ee..9bf57ab372 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/CachePruneIT.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/CachePruneIT.java @@ -3,7 +3,6 @@ import java.time.Duration; import java.util.Map; -import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.RegisterExtension; @@ -18,7 +17,6 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.awaitility.Awaitility.await; -@Disabled(value = "todo: reactivate when restoring SSA") class CachePruneIT { public static final String DEFAULT_DATA = "default_data";