Skip to content

Commit fec055e

Browse files
committed
fix: isCleaner was only returning the cleaner state of last seen DR
1 parent c058c72 commit fec055e

File tree

3 files changed

+18
-9
lines changed

3 files changed

+18
-9
lines changed

operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/ManagedWorkflowSupport.java

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ public <P extends HasMetadata> Workflow<P> createWorkflow(
5050
List<DependentResourceSpec> dependentResourceSpecs) {
5151
var orderedResourceSpecs = orderAndDetectCycles(dependentResourceSpecs);
5252
final var alreadyCreated = new ArrayList<DependentResourceNode>(orderedResourceSpecs.size());
53-
final boolean cleanerHolder[] = {false};
53+
final boolean[] cleanerHolder = {false};
5454
final var nodes = orderedResourceSpecs.stream()
5555
.map(spec -> createFrom(spec, alreadyCreated, cleanerHolder))
5656
.collect(Collectors.toSet());
@@ -61,7 +61,8 @@ private DependentResourceNode createFrom(DependentResourceSpec spec,
6161
List<DependentResourceNode> alreadyCreated, boolean[] cleanerHolder) {
6262
final var node = new SpecDependentResourceNode<>(spec);
6363
alreadyCreated.add(node);
64-
cleanerHolder[0] = Workflow.isDeletable(spec.getDependentResourceClass());
64+
// if any previously checked dependent was a cleaner, no need to check further
65+
cleanerHolder[0] = cleanerHolder[0] || Workflow.isDeletable(spec.getDependentResourceClass());
6566
spec.getDependsOn().forEach(depend -> {
6667
final DependentResourceNode dependsOn = alreadyCreated.stream()
6768
.filter(drn -> depend.equals(drn.getName())).findFirst()

operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/Workflow.java

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -130,7 +130,6 @@ void resolve(KubernetesClient client, List<DependentResourceSpec> dependentResou
130130
if (!resolved) {
131131
dependentResourceNodes.values().forEach(drn -> drn.resolve(client, dependentResources));
132132
resolved = true;
133-
hasCleaner = cleanerHolder[0];
134133
}
135134
}
136135

operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/workflow/ManagedWorkflowTest.java

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@
44

55
import org.junit.jupiter.api.Test;
66

7-
import io.fabric8.kubernetes.client.KubernetesClient;
87
import io.javaoperatorsdk.operator.api.config.ConfigurationServiceProvider;
98
import io.javaoperatorsdk.operator.api.config.ControllerConfiguration;
109
import io.javaoperatorsdk.operator.api.config.dependent.DependentResourceSpec;
@@ -17,7 +16,7 @@
1716
import static org.mockito.Mockito.mock;
1817
import static org.mockito.Mockito.when;
1918

20-
@SuppressWarnings({"rawtypes", "unchecked"})
19+
@SuppressWarnings({"rawtypes"})
2120
class ManagedWorkflowTest {
2221

2322
public static final String NAME = "name";
@@ -39,6 +38,19 @@ void isNotCleanerIfGarbageCollected() {
3938
.isCleaner()).isFalse();
4039
}
4140

41+
@Test
42+
void isCleanerShouldWork() {
43+
assertThat(managedWorkflow(
44+
createDRSWithTraits(NAME, GarbageCollected.class),
45+
createDRSWithTraits("foo", Deleter.class))
46+
.isCleaner()).isTrue();
47+
48+
assertThat(managedWorkflow(
49+
createDRSWithTraits("foo", Deleter.class),
50+
createDRSWithTraits(NAME, GarbageCollected.class))
51+
.isCleaner()).isTrue();
52+
}
53+
4254
@Test
4355
void isCleanerIfHasDeleter() {
4456
var spec = createDRSWithTraits(NAME, Deleter.class);
@@ -49,12 +61,9 @@ ManagedWorkflow managedWorkflow(DependentResourceSpec... specs) {
4961
final var configuration = mock(ControllerConfiguration.class);
5062
final var specList = List.of(specs);
5163

52-
KubernetesClient kubernetesClientMock = mock(KubernetesClient.class);
53-
5464
when(configuration.getDependentResources()).thenReturn(specList);
5565
return ConfigurationServiceProvider.instance().getWorkflowFactory()
56-
.workflowFor(configuration)
57-
.resolve(kubernetesClientMock, specList);
66+
.workflowFor(configuration);
5867
}
5968

6069
}

0 commit comments

Comments
 (0)