Skip to content

Commit ae0eb4a

Browse files
committed
fix: register event source when dependents are marked for deletion
Fixes #3249 Signed-off-by: Chris Laprun <metacosm@gmail.com>
1 parent c63f8d1 commit ae0eb4a

File tree

4 files changed

+31
-35
lines changed

4 files changed

+31
-35
lines changed

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -246,6 +246,10 @@ private void markDependentsForDelete(
246246
// so if the activation condition was false, this node is not meant to be deleted.
247247
var dependents = dependentResourceNode.getParents();
248248
if (activationConditionMet) {
249+
// make sure we register the dependent's event source if it hasn't been added already
250+
// this might be needed in corner cases such as
251+
// https://github.com/operator-framework/java-operator-sdk/issues/3249
252+
registerOrDeregisterEventSourceBasedOnActivation(true, dependentResourceNode);
249253
createOrGetResultFor(dependentResourceNode).markForDelete();
250254
if (dependents.isEmpty()) {
251255
bottomNodes.add(dependentResourceNode);

operator-framework/src/test/java/io/javaoperatorsdk/operator/workflow/bulkactivationcondition/BulkActivationConditionIT.java

Lines changed: 4 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -18,14 +18,12 @@
1818
import java.time.Duration;
1919

2020
import org.junit.jupiter.api.BeforeEach;
21-
import org.junit.jupiter.api.Disabled;
2221
import org.junit.jupiter.api.Test;
2322
import org.junit.jupiter.api.extension.RegisterExtension;
2423

2524
import io.fabric8.kubernetes.api.model.ObjectMetaBuilder;
2625
import io.javaoperatorsdk.annotation.Sample;
2726
import io.javaoperatorsdk.operator.junit.LocallyRunOperatorExtension;
28-
import io.javaoperatorsdk.operator.processing.event.NoEventSourceForClassException;
2927

3028
import static org.assertj.core.api.Assertions.assertThat;
3129
import static org.awaitility.Awaitility.await;
@@ -49,7 +47,6 @@
4947
*
5048
* <p>This test FAILS on unfixed JOSDK, demonstrating the bug.
5149
*/
52-
@Disabled("Reproducer for https://github.com/operator-framework/java-operator-sdk/issues/3249")
5350
@Sample(
5451
tldr = "Bulk Dependent Resource with Activation Condition Bug Reproducer",
5552
description =
@@ -84,25 +81,11 @@ void nodeDeleteExecutorShouldNotThrowWhenEventSourceNotYetRegistered() {
8481
.build());
8582
extension.create(primary);
8683

87-
// Wait for reconcile() or updateErrorStatus() to be called — whichever comes first.
88-
// If the bug is present, updateErrorStatus() fires (no reconcile() call); if fixed,
89-
// reconcile() runs cleanly.
90-
await().atMost(Duration.ofSeconds(30)).until(() -> reconciler.callCount.get() > 0);
84+
// Wait for reconcile() to be called.
85+
// If the bug is present, SecretBulkDependentResource will be in error and lastError will be set
86+
await().atMost(Duration.ofSeconds(10)).until(() -> reconciler.callCount.get() == 1);
9187

9288
// On unfixed JOSDK this fails: lastError contains NoEventSourceForClassException.
93-
assertThat(reconciler.lastError.get())
94-
.as(
95-
"NodeDeleteExecutor must not throw NoEventSourceForClassException when the"
96-
+ " activationCondition-gated event source was never registered")
97-
.satisfies(
98-
e -> {
99-
Throwable t = e;
100-
while (t != null) {
101-
assertThat(t)
102-
.as("Cause chain should not contain NoEventSourceForClassException")
103-
.isNotInstanceOf(NoEventSourceForClassException.class);
104-
t = t.getCause();
105-
}
106-
});
89+
assertThat(reconciler.lastError.get()).isNull();
10790
}
10891
}

operator-framework/src/test/java/io/javaoperatorsdk/operator/workflow/bulkactivationcondition/BulkActivationConditionReconciler.java

Lines changed: 20 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020

2121
import io.javaoperatorsdk.operator.api.reconciler.*;
2222
import io.javaoperatorsdk.operator.api.reconciler.dependent.Dependent;
23+
import io.javaoperatorsdk.operator.processing.retry.GradualRetry;
2324

2425
/**
2526
* Workflow:
@@ -40,11 +41,14 @@
4041
type = ConfigMapDependentResource.class,
4142
reconcilePrecondition = AlwaysFailingPrecondition.class),
4243
@Dependent(
44+
name = SecretBulkDependentResource.NAME,
4345
type = SecretBulkDependentResource.class,
4446
activationCondition = AlwaysTrueActivation.class,
4547
dependsOn = "configmap")
46-
})
47-
@ControllerConfiguration
48+
},
49+
handleExceptionsInReconciler = true)
50+
@GradualRetry(maxAttempts = 0)
51+
@ControllerConfiguration(maxReconciliationInterval = @MaxReconciliationInterval(interval = 0))
4852
public class BulkActivationConditionReconciler
4953
implements Reconciler<BulkActivationConditionCustomResource> {
5054

@@ -58,17 +62,21 @@ public class BulkActivationConditionReconciler
5862
public UpdateControl<BulkActivationConditionCustomResource> reconcile(
5963
BulkActivationConditionCustomResource primary,
6064
Context<BulkActivationConditionCustomResource> context) {
65+
final var workflowResult =
66+
context
67+
.managedWorkflowAndDependentResourceContext()
68+
.getWorkflowReconcileResult()
69+
.orElseThrow();
70+
final var erroredDependents = workflowResult.getErroredDependents();
71+
if (!erroredDependents.isEmpty()) {
72+
final var exception =
73+
erroredDependents.get(
74+
workflowResult
75+
.getDependentResourceByName(SecretBulkDependentResource.NAME)
76+
.orElseThrow());
77+
lastError.set(exception);
78+
}
6179
callCount.incrementAndGet();
6280
return UpdateControl.noUpdate();
6381
}
64-
65-
@Override
66-
public ErrorStatusUpdateControl<BulkActivationConditionCustomResource> updateErrorStatus(
67-
BulkActivationConditionCustomResource primary,
68-
Context<BulkActivationConditionCustomResource> context,
69-
Exception e) {
70-
lastError.set(e);
71-
callCount.incrementAndGet();
72-
return ErrorStatusUpdateControl.noStatusUpdate();
73-
}
7482
}

operator-framework/src/test/java/io/javaoperatorsdk/operator/workflow/bulkactivationcondition/SecretBulkDependentResource.java

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -43,8 +43,9 @@ public class SecretBulkDependentResource
4343
extends KubernetesDependentResource<Secret, BulkActivationConditionCustomResource>
4444
implements CRUDKubernetesBulkDependentResource<Secret, BulkActivationConditionCustomResource> {
4545

46-
public static final String LABEL_KEY = "reproducer";
47-
public static final String LABEL_VALUE = "bulk-activation-condition";
46+
static final String NAME = "secret";
47+
static final String LABEL_KEY = "reproducer";
48+
static final String LABEL_VALUE = "bulk-activation-condition";
4849

4950
@Override
5051
public Map<ResourceID, Secret> desiredResources(

0 commit comments

Comments
 (0)