Skip to content

Commit 2c16143

Browse files
SamBarkermetacosm
SamBarker
and
metacosm
authored
fix: register event sources when dependents are marked for deletion (#3250)
* Add failing IT reproducing NoEventSourceForClassException in NodeDeleteExecutor When a BulkDependentResource has an activationCondition and its parent dependent has a failing reconcilePrecondition, JOSDK's markDependentsForDelete() cascades to the bulk dependent and fires NodeDeleteExecutor for it. However, NodeDeleteExecutor does not call registerOrDeregisterEventSourceBasedOnActivation() before invoking delete(), so if NodeReconcileExecutor has never run for that node (e.g. on first reconciliation) the event source is never registered. The delete() path calls getSecondaryResources() → eventSourceRetriever .getEventSourceFor() → NoEventSourceForClassException. This IT demonstrates the bug with a minimal workflow: ConfigMapDependentResource (reconcilePrecondition = ALWAYS_FALSE) └── SecretBulkDependentResource (activationCondition = ALWAYS_TRUE) The fix is to call registerOrDeregisterEventSourceBasedOnActivation() in NodeDeleteExecutor.doRun() before calling dependent.delete(), mirroring what NodeReconcileExecutor already does. Assisted-by: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Sam Barker <sam@quadrocket.co.uk> * 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> * fix: register event source when dependents are marked for deletion Fixes #3249 Signed-off-by: Chris Laprun <metacosm@gmail.com> --------- Signed-off-by: Sam Barker <sam@quadrocket.co.uk> Signed-off-by: Chris Laprun <metacosm@gmail.com> Co-authored-by: Chris Laprun <metacosm@gmail.com>
1 parent 09ba1b9 commit 2c16143

File tree

8 files changed

+396
-0
lines changed

8 files changed

+396
-0
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/AlwaysFailingPrecondition.java‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
/*
2+
* Copyright Java Operator SDK Authors
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
package io.javaoperatorsdk.operator.workflow.bulkactivationcondition;
17+
18+
import io.fabric8.kubernetes.api.model.ConfigMap;
19+
import io.javaoperatorsdk.operator.api.reconciler.Context;
20+
import io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResource;
21+
import io.javaoperatorsdk.operator.processing.dependent.workflow.Condition;
22+
23+
/** Reconcile precondition that always fails, simulating e.g. a missing prerequisite resource. */
24+
public class AlwaysFailingPrecondition
25+
implements Condition<ConfigMap, BulkActivationConditionCustomResource> {
26+
27+
@Override
28+
public boolean isMet(
29+
DependentResource<ConfigMap, BulkActivationConditionCustomResource> dependentResource,
30+
BulkActivationConditionCustomResource primary,
31+
Context<BulkActivationConditionCustomResource> context) {
32+
return false;
33+
}
34+
}

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

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
/*
2+
* Copyright Java Operator SDK Authors
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
package io.javaoperatorsdk.operator.workflow.bulkactivationcondition;
17+
18+
import io.fabric8.kubernetes.api.model.Secret;
19+
import io.javaoperatorsdk.operator.api.reconciler.Context;
20+
import io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResource;
21+
import io.javaoperatorsdk.operator.processing.dependent.workflow.Condition;
22+
23+
/** Activation condition that always returns true — event source should always be registered. */
24+
public class AlwaysTrueActivation
25+
implements Condition<Secret, BulkActivationConditionCustomResource> {
26+
27+
@Override
28+
public boolean isMet(
29+
DependentResource<Secret, BulkActivationConditionCustomResource> dependentResource,
30+
BulkActivationConditionCustomResource primary,
31+
Context<BulkActivationConditionCustomResource> context) {
32+
return true;
33+
}
34+
}

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

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
/*
2+
* Copyright Java Operator SDK Authors
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
package io.javaoperatorsdk.operator.workflow.bulkactivationcondition;
17+
18+
import io.fabric8.kubernetes.api.model.Namespaced;
19+
import io.fabric8.kubernetes.client.CustomResource;
20+
import io.fabric8.kubernetes.model.annotation.Group;
21+
import io.fabric8.kubernetes.model.annotation.Kind;
22+
import io.fabric8.kubernetes.model.annotation.Version;
23+
24+
@Group("sample.javaoperatorsdk")
25+
@Version("v1")
26+
@Kind("BulkActivationCondition")
27+
public class BulkActivationConditionCustomResource extends CustomResource<Void, Void>
28+
implements Namespaced {}

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

Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,91 @@
1+
/*
2+
* Copyright Java Operator SDK Authors
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
package io.javaoperatorsdk.operator.workflow.bulkactivationcondition;
17+
18+
import java.time.Duration;
19+
20+
import org.junit.jupiter.api.BeforeEach;
21+
import org.junit.jupiter.api.Test;
22+
import org.junit.jupiter.api.extension.RegisterExtension;
23+
24+
import io.fabric8.kubernetes.api.model.ObjectMetaBuilder;
25+
import io.javaoperatorsdk.annotation.Sample;
26+
import io.javaoperatorsdk.operator.junit.LocallyRunOperatorExtension;
27+
28+
import static org.assertj.core.api.Assertions.assertThat;
29+
import static org.awaitility.Awaitility.await;
30+
31+
/**
32+
* Reproducer for the bug where NodeDeleteExecutor fires for a BulkDependentResource whose
33+
* activationCondition-gated event source has never been registered.
34+
*
35+
* <p>Workflow under test:
36+
*
37+
* <pre>
38+
* ConfigMapDependentResource (reconcilePrecondition = AlwaysFailingPrecondition)
39+
* └── SecretBulkDependentResource (activationCondition = AlwaysTrueActivation)
40+
* </pre>
41+
*
42+
* <p>On first reconciliation the ConfigMap precondition fails → JOSDK calls
43+
* markDependentsForDelete() → NodeDeleteExecutor fires for SecretBulkDependent →
44+
* SecretBulkDependent.getSecondaryResources() calls eventSourceRetriever.getEventSourceFor() → the
45+
* Secret event source was never registered (NodeReconcileExecutor never ran) →
46+
* NoEventSourceForClassException.
47+
*
48+
* <p>This test FAILS on unfixed JOSDK, demonstrating the bug.
49+
*/
50+
@Sample(
51+
tldr = "Bulk Dependent Resource with Activation Condition Bug Reproducer",
52+
description =
53+
"""
54+
Reproducer for a bug where NodeDeleteExecutor fires for a BulkDependentResource \
55+
with an activationCondition before its event source has been registered, \
56+
causing NoEventSourceForClassException. Triggered when a parent dependent \
57+
has a failing reconcilePrecondition on first reconciliation.
58+
""")
59+
public class BulkActivationConditionIT {
60+
61+
static final BulkActivationConditionReconciler reconciler =
62+
new BulkActivationConditionReconciler();
63+
64+
@RegisterExtension
65+
static LocallyRunOperatorExtension extension =
66+
LocallyRunOperatorExtension.builder().withReconciler(reconciler).build();
67+
68+
@BeforeEach
69+
void reset() {
70+
reconciler.lastError.set(null);
71+
reconciler.callCount.set(0);
72+
}
73+
74+
@Test
75+
void nodeDeleteExecutorShouldNotThrowWhenEventSourceNotYetRegistered() {
76+
var primary = new BulkActivationConditionCustomResource();
77+
primary.setMetadata(
78+
new ObjectMetaBuilder()
79+
.withName("test-primary")
80+
.withNamespace(extension.getNamespace())
81+
.build());
82+
extension.create(primary);
83+
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);
87+
88+
// On unfixed JOSDK this fails: lastError contains NoEventSourceForClassException.
89+
assertThat(reconciler.lastError.get()).isNull();
90+
}
91+
}

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

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,82 @@
1+
/*
2+
* Copyright Java Operator SDK Authors
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
package io.javaoperatorsdk.operator.workflow.bulkactivationcondition;
17+
18+
import java.util.concurrent.atomic.AtomicInteger;
19+
import java.util.concurrent.atomic.AtomicReference;
20+
21+
import io.javaoperatorsdk.operator.api.reconciler.*;
22+
import io.javaoperatorsdk.operator.api.reconciler.dependent.Dependent;
23+
import io.javaoperatorsdk.operator.processing.retry.GradualRetry;
24+
25+
/**
26+
* Workflow:
27+
*
28+
* <pre>
29+
* ConfigMapDependentResource (reconcilePrecondition = AlwaysFailingPrecondition)
30+
* └── SecretBulkDependentResource (activationCondition = AlwaysTrueActivation)
31+
* </pre>
32+
*
33+
* <p>On first reconciliation: ConfigMap precondition fails → markDependentsForDelete cascades to
34+
* SecretBulkDependentResource → NodeDeleteExecutor fires for Secret before its event source is ever
35+
* registered → NoEventSourceForClassException.
36+
*/
37+
@Workflow(
38+
dependents = {
39+
@Dependent(
40+
name = "configmap",
41+
type = ConfigMapDependentResource.class,
42+
reconcilePrecondition = AlwaysFailingPrecondition.class),
43+
@Dependent(
44+
name = SecretBulkDependentResource.NAME,
45+
type = SecretBulkDependentResource.class,
46+
activationCondition = AlwaysTrueActivation.class,
47+
dependsOn = "configmap")
48+
},
49+
handleExceptionsInReconciler = true)
50+
@GradualRetry(maxAttempts = 0)
51+
@ControllerConfiguration(maxReconciliationInterval = @MaxReconciliationInterval(interval = 0))
52+
public class BulkActivationConditionReconciler
53+
implements Reconciler<BulkActivationConditionCustomResource> {
54+
55+
/** Tracks how many times reconcile() or updateErrorStatus() has been called. */
56+
final AtomicInteger callCount = new AtomicInteger();
57+
58+
/** Set when updateErrorStatus() is invoked; null means no error occurred. */
59+
final AtomicReference<Exception> lastError = new AtomicReference<>();
60+
61+
@Override
62+
public UpdateControl<BulkActivationConditionCustomResource> reconcile(
63+
BulkActivationConditionCustomResource primary,
64+
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+
}
79+
callCount.incrementAndGet();
80+
return UpdateControl.noUpdate();
81+
}
82+
}

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

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
/*
2+
* Copyright Java Operator SDK Authors
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
package io.javaoperatorsdk.operator.workflow.bulkactivationcondition;
17+
18+
import io.fabric8.kubernetes.api.model.ConfigMap;
19+
import io.fabric8.kubernetes.api.model.ObjectMetaBuilder;
20+
import io.javaoperatorsdk.operator.api.reconciler.Context;
21+
import io.javaoperatorsdk.operator.processing.dependent.kubernetes.CRUDKubernetesDependentResource;
22+
23+
/**
24+
* Parent dependent resource. Its reconcile precondition always fails, which causes JOSDK to call
25+
* markDependentsForDelete() on its children — triggering NodeDeleteExecutor for SecretBulkDependent
26+
* before that resource's event source has ever been registered.
27+
*/
28+
public class ConfigMapDependentResource
29+
extends CRUDKubernetesDependentResource<ConfigMap, BulkActivationConditionCustomResource> {
30+
31+
@Override
32+
protected ConfigMap desired(
33+
BulkActivationConditionCustomResource primary,
34+
Context<BulkActivationConditionCustomResource> context) {
35+
var cm = new ConfigMap();
36+
cm.setMetadata(
37+
new ObjectMetaBuilder()
38+
.withName(primary.getMetadata().getName())
39+
.withNamespace(primary.getMetadata().getNamespace())
40+
.build());
41+
return cm;
42+
}
43+
}

0 commit comments

Comments
 (0)