Skip to content

Commit 490c224

Browse files
committed
Address review comments on BulkActivationConditionIT
- Make lastError and callCount instance fields on the reconciler; hold the reconciler instance as a static field in the IT so tests can access state without static leakage between tests - Add callCount so the test can wait for any reconciliation activity (reconcile() or updateErrorStatus()) then assert cleanly, rather than timing out if the bug is fixed - Add @disabled linking to issue #3249 so this reproducer-only test does not break CI - Add @sample annotation to match the pattern of other workflow ITs Assisted-by: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Sam Barker <sam@quadrocket.co.uk>
1 parent d37a0c6 commit 490c224

File tree

2 files changed

+36
-20
lines changed

2 files changed

+36
-20
lines changed

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

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

2020
import org.junit.jupiter.api.BeforeEach;
21+
import org.junit.jupiter.api.Disabled;
2122
import org.junit.jupiter.api.Test;
2223
import org.junit.jupiter.api.extension.RegisterExtension;
2324

2425
import io.fabric8.kubernetes.api.model.ObjectMetaBuilder;
26+
import io.javaoperatorsdk.annotation.Sample;
2527
import io.javaoperatorsdk.operator.junit.LocallyRunOperatorExtension;
2628
import io.javaoperatorsdk.operator.processing.event.NoEventSourceForClassException;
2729

@@ -39,25 +41,37 @@
3941
* └── SecretBulkDependentResource (activationCondition = AlwaysTrueActivation)
4042
* </pre>
4143
*
42-
* <p>On first reconciliation with only a primary resource present: the ConfigMap precondition fails
43-
* → JOSDK calls markDependentsForDelete() → NodeDeleteExecutor fires for SecretBulkDependent →
44+
* <p>On first reconciliation the ConfigMap precondition fails → JOSDK calls
45+
* markDependentsForDelete() → NodeDeleteExecutor fires for SecretBulkDependent →
4446
* SecretBulkDependent.getSecondaryResources() calls eventSourceRetriever.getEventSourceFor() → the
4547
* Secret event source was never registered (NodeReconcileExecutor never ran) →
4648
* NoEventSourceForClassException.
4749
*
4850
* <p>This test FAILS on unfixed JOSDK, demonstrating the bug.
4951
*/
52+
@Disabled("Reproducer for https://github.com/operator-framework/java-operator-sdk/issues/3249")
53+
@Sample(
54+
tldr = "Bulk Dependent Resource with Activation Condition Bug Reproducer",
55+
description =
56+
"""
57+
Reproducer for a bug where NodeDeleteExecutor fires for a BulkDependentResource \
58+
with an activationCondition before its event source has been registered, \
59+
causing NoEventSourceForClassException. Triggered when a parent dependent \
60+
has a failing reconcilePrecondition on first reconciliation.
61+
""")
5062
public class BulkActivationConditionIT {
5163

64+
static final BulkActivationConditionReconciler reconciler =
65+
new BulkActivationConditionReconciler();
66+
5267
@RegisterExtension
5368
static LocallyRunOperatorExtension extension =
54-
LocallyRunOperatorExtension.builder()
55-
.withReconciler(new BulkActivationConditionReconciler())
56-
.build();
69+
LocallyRunOperatorExtension.builder().withReconciler(reconciler).build();
5770

5871
@BeforeEach
59-
void resetError() {
60-
BulkActivationConditionReconciler.lastError.set(null);
72+
void reset() {
73+
reconciler.lastError.set(null);
74+
reconciler.callCount.set(0);
6175
}
6276

6377
@Test
@@ -70,21 +84,16 @@ void nodeDeleteExecutorShouldNotThrowWhenEventSourceNotYetRegistered() {
7084
.build());
7185
extension.create(primary);
7286

73-
// Wait for the error to arrive — the ConfigMap precondition always fails,
74-
// so JOSDK should attempt NodeDeleteExecutor for the Secret bulk dependent.
75-
await()
76-
.atMost(Duration.ofSeconds(30))
77-
.until(() -> BulkActivationConditionReconciler.lastError.get() != null);
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);
7891

79-
// On unfixed JOSDK this fails: lastError is a NoEventSourceForClassException (or wraps one).
80-
// The assertion below demonstrates the bug by asserting the exception should NOT be present.
81-
Exception error = BulkActivationConditionReconciler.lastError.get();
82-
assertThat(error)
92+
// On unfixed JOSDK this fails: lastError contains NoEventSourceForClassException.
93+
assertThat(reconciler.lastError.get())
8394
.as(
8495
"NodeDeleteExecutor must not throw NoEventSourceForClassException when the"
85-
+ " activationCondition-gated event source was never registered."
86-
+ " Actual error: %s",
87-
error)
96+
+ " activationCondition-gated event source was never registered")
8897
.satisfies(
8998
e -> {
9099
Throwable t = e;

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

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
*/
1616
package io.javaoperatorsdk.operator.workflow.bulkactivationcondition;
1717

18+
import java.util.concurrent.atomic.AtomicInteger;
1819
import java.util.concurrent.atomic.AtomicReference;
1920

2021
import io.javaoperatorsdk.operator.api.reconciler.*;
@@ -47,12 +48,17 @@
4748
public class BulkActivationConditionReconciler
4849
implements Reconciler<BulkActivationConditionCustomResource> {
4950

50-
static final AtomicReference<Exception> lastError = new AtomicReference<>();
51+
/** Tracks how many times reconcile() or updateErrorStatus() has been called. */
52+
final AtomicInteger callCount = new AtomicInteger();
53+
54+
/** Set when updateErrorStatus() is invoked; null means no error occurred. */
55+
final AtomicReference<Exception> lastError = new AtomicReference<>();
5156

5257
@Override
5358
public UpdateControl<BulkActivationConditionCustomResource> reconcile(
5459
BulkActivationConditionCustomResource primary,
5560
Context<BulkActivationConditionCustomResource> context) {
61+
callCount.incrementAndGet();
5662
return UpdateControl.noUpdate();
5763
}
5864

@@ -62,6 +68,7 @@ public ErrorStatusUpdateControl<BulkActivationConditionCustomResource> updateErr
6268
Context<BulkActivationConditionCustomResource> context,
6369
Exception e) {
6470
lastError.set(e);
71+
callCount.incrementAndGet();
6572
return ErrorStatusUpdateControl.noStatusUpdate();
6673
}
6774
}

0 commit comments

Comments
 (0)