Skip to content

Commit

Permalink
fix: managed workflow result is available even when exception is thrown
Browse files Browse the repository at this point in the history
Fixes #2551

Signed-off-by: Chris Laprun <claprun@redhat.com>
  • Loading branch information
metacosm committed Oct 15, 2024
1 parent 41463e1 commit a70ff78
Show file tree
Hide file tree
Showing 6 changed files with 20 additions and 35 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,8 @@

@SuppressWarnings("rawtypes")
public class DefaultManagedDependentResourceContext implements ManagedDependentResourceContext {

private WorkflowReconcileResult workflowReconcileResult;
private WorkflowCleanupResult workflowCleanupResult;
public static final Object RECONCILE_RESULT_KEY = new Object();
public static final Object CLEANUP_RESULT_KEY = new Object();
private final ConcurrentHashMap attributes = new ConcurrentHashMap();

@Override
Expand All @@ -37,25 +36,13 @@ public <T> T getMandatory(Object key, Class<T> expectedType) {
+ ") is missing or not of the expected type"));
}

public DefaultManagedDependentResourceContext setWorkflowExecutionResult(
WorkflowReconcileResult workflowReconcileResult) {
this.workflowReconcileResult = workflowReconcileResult;
return this;
}

public DefaultManagedDependentResourceContext setWorkflowCleanupResult(
WorkflowCleanupResult workflowCleanupResult) {
this.workflowCleanupResult = workflowCleanupResult;
return this;
}

@Override
public Optional<WorkflowReconcileResult> getWorkflowReconcileResult() {
return Optional.ofNullable(workflowReconcileResult);
return get(RECONCILE_RESULT_KEY, WorkflowReconcileResult.class);
}

@Override
public Optional<WorkflowCleanupResult> getWorkflowCleanupResult() {
return Optional.ofNullable(workflowCleanupResult);
return get(CLEANUP_RESULT_KEY, WorkflowCleanupResult.class);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@
import io.javaoperatorsdk.operator.api.reconciler.dependent.EventSourceNotFoundException;
import io.javaoperatorsdk.operator.api.reconciler.dependent.EventSourceProvider;
import io.javaoperatorsdk.operator.api.reconciler.dependent.EventSourceReferencer;
import io.javaoperatorsdk.operator.api.reconciler.dependent.managed.DefaultManagedDependentResourceContext;
import io.javaoperatorsdk.operator.health.ControllerHealthInfo;
import io.javaoperatorsdk.operator.processing.dependent.workflow.Workflow;
import io.javaoperatorsdk.operator.processing.dependent.workflow.WorkflowCleanupResult;
Expand Down Expand Up @@ -145,10 +144,7 @@ public Map<String, Object> metadata() {
public UpdateControl<P> execute() throws Exception {
initContextIfNeeded(resource, context);
if (!managedWorkflow.isEmpty()) {
var res = managedWorkflow.reconcile(resource, context);
((DefaultManagedDependentResourceContext) context.managedDependentResourceContext())
.setWorkflowExecutionResult(res);
res.throwAggregateExceptionIfErrorsPresent();
managedWorkflow.reconcile(resource, context);
}
return reconciler.reconcile(resource, context);
}
Expand Down Expand Up @@ -191,9 +187,6 @@ public DeleteControl execute() {
WorkflowCleanupResult workflowCleanupResult = null;
if (managedWorkflow.hasCleaner()) {
workflowCleanupResult = managedWorkflow.cleanup(resource, context);
((DefaultManagedDependentResourceContext) context.managedDependentResourceContext())
.setWorkflowCleanupResult(workflowCleanupResult);
workflowCleanupResult.throwAggregateExceptionIfErrorsPresent();
}
if (isCleaner) {
var cleanupResult = ((Cleaner<P>) reconciler).cleanup(resource, context);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
import io.javaoperatorsdk.operator.api.reconciler.dependent.Deleter;
import io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResource;
import io.javaoperatorsdk.operator.api.reconciler.dependent.GarbageCollected;
import io.javaoperatorsdk.operator.api.reconciler.dependent.managed.DefaultManagedDependentResourceContext;
import io.javaoperatorsdk.operator.processing.dependent.kubernetes.KubernetesDependentResource;

/**
Expand Down Expand Up @@ -79,7 +80,7 @@ private Map<String, DependentResourceNode> toMap(Set<DependentResourceNode> node
}
map.put(node.getName(), node);
}
if (topLevelResources.size() == 0) {
if (topLevelResources.isEmpty()) {
throw new IllegalStateException(
"No top-level dependent resources found. This might indicate a cyclic Set of DependentResourceNode has been provided.");
}
Expand All @@ -91,6 +92,8 @@ public WorkflowReconcileResult reconcile(P primary, Context<P> context) {
WorkflowReconcileExecutor<P> workflowReconcileExecutor =
new WorkflowReconcileExecutor<>(this, primary, context);
var result = workflowReconcileExecutor.reconcile();
context.managedDependentResourceContext()
.put(DefaultManagedDependentResourceContext.RECONCILE_RESULT_KEY, result);
if (throwExceptionAutomatically) {
result.throwAggregateExceptionIfErrorsPresent();
}
Expand All @@ -102,6 +105,8 @@ public WorkflowCleanupResult cleanup(P primary, Context<P> context) {
WorkflowCleanupExecutor<P> workflowCleanupExecutor =
new WorkflowCleanupExecutor<>(this, primary, context);
var result = workflowCleanupExecutor.cleanup();
context.managedDependentResourceContext()
.put(DefaultManagedDependentResourceContext.CLEANUP_RESULT_KEY, result);
if (throwExceptionAutomatically) {
result.throwAggregateExceptionIfErrorsPresent();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,7 @@
@SuppressWarnings({"rawtypes", "unchecked"})
public class WorkflowBuilder<P extends HasMetadata> {

private final Map<String, DependentResourceNode<?, P>> dependentResourceNodes =
new HashMap<>();
private final Map<String, DependentResourceNode<?, P>> dependentResourceNodes = new HashMap<>();
private boolean throwExceptionAutomatically = THROW_EXCEPTION_AUTOMATICALLY_DEFAULT;
private DependentResourceNode currentNode;
private boolean isCleaner = false;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,13 @@
import io.javaoperatorsdk.operator.api.config.ControllerConfiguration;
import io.javaoperatorsdk.operator.api.reconciler.Context;
import io.javaoperatorsdk.operator.api.reconciler.EventSourceContext;
import io.javaoperatorsdk.operator.api.reconciler.dependent.managed.ManagedDependentResourceContext;
import io.javaoperatorsdk.operator.processing.event.EventSourceRetriever;
import io.javaoperatorsdk.operator.sample.simple.TestCustomResource;

import static io.javaoperatorsdk.operator.processing.dependent.workflow.ExecutionAssert.assertThat;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;
import static org.mockito.Mockito.*;

class WorkflowCleanupExecutorTest extends AbstractWorkflowExecutorTest {

Expand All @@ -29,8 +29,7 @@ class WorkflowCleanupExecutorTest extends AbstractWorkflowExecutorTest {
protected TestDeleterDependent dd3 = new TestDeleterDependent("DR_DELETER_3");
protected TestDeleterDependent dd4 = new TestDeleterDependent("DR_DELETER_4");
@SuppressWarnings("unchecked")

Context<TestCustomResource> mockContext = mock(Context.class);
Context<TestCustomResource> mockContext = spy(Context.class);
ExecutorService executorService = Executors.newCachedThreadPool();

@BeforeEach
Expand All @@ -45,7 +44,8 @@ void setup() {
when(eventSourceContextMock.getControllerConfiguration()).thenReturn(mockControllerConfig);
when(mockControllerConfig.getConfigurationService())
.thenReturn(mock(ConfigurationService.class));

when(mockContext.managedDependentResourceContext())
.thenReturn(mock(ManagedDependentResourceContext.class));
when(mockContext.getWorkflowExecutorService()).thenReturn(executorService);
when(mockContext.eventSourceRetriever()).thenReturn(eventSourceRetrieverMock);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,25 +9,26 @@

import io.javaoperatorsdk.operator.AggregatedOperatorException;
import io.javaoperatorsdk.operator.api.reconciler.Context;
import io.javaoperatorsdk.operator.api.reconciler.dependent.managed.ManagedDependentResourceContext;
import io.javaoperatorsdk.operator.processing.event.EventSourceRetriever;
import io.javaoperatorsdk.operator.sample.simple.TestCustomResource;

import static io.javaoperatorsdk.operator.processing.dependent.workflow.ExecutionAssert.assertThat;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.mockito.Mockito.*;

@SuppressWarnings("rawtypes")
class WorkflowReconcileExecutorTest extends AbstractWorkflowExecutorTest {

@SuppressWarnings("unchecked")
Context<TestCustomResource> mockContext = mock(Context.class);
Context<TestCustomResource> mockContext = spy(Context.class);
ExecutorService executorService = Executors.newCachedThreadPool();

TestDependent dr3 = new TestDependent("DR_3");
TestDependent dr4 = new TestDependent("DR_4");

@BeforeEach
void setup() {
when(mockContext.managedDependentResourceContext()).thenReturn(mock(ManagedDependentResourceContext.class));
when(mockContext.getWorkflowExecutorService()).thenReturn(executorService);
when(mockContext.eventSourceRetriever()).thenReturn(mock(EventSourceRetriever.class));
}
Expand Down

0 comments on commit a70ff78

Please sign in to comment.