diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowReconcileExecutor.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowReconcileExecutor.java index 9121cf4e07..d0d2435347 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowReconcileExecutor.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowReconcileExecutor.java @@ -246,6 +246,10 @@ private void markDependentsForDelete( // so if the activation condition was false, this node is not meant to be deleted. var dependents = dependentResourceNode.getParents(); if (activationConditionMet) { + // make sure we register the dependent's event source if it hasn't been added already + // this might be needed in corner cases such as + // https://github.com/operator-framework/java-operator-sdk/issues/3249 + registerOrDeregisterEventSourceBasedOnActivation(true, dependentResourceNode); createOrGetResultFor(dependentResourceNode).markForDelete(); if (dependents.isEmpty()) { bottomNodes.add(dependentResourceNode); diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/workflow/bulkactivationcondition/AlwaysFailingPrecondition.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/workflow/bulkactivationcondition/AlwaysFailingPrecondition.java new file mode 100644 index 0000000000..4c21fc019d --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/workflow/bulkactivationcondition/AlwaysFailingPrecondition.java @@ -0,0 +1,34 @@ +/* + * Copyright Java Operator SDK Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.javaoperatorsdk.operator.workflow.bulkactivationcondition; + +import io.fabric8.kubernetes.api.model.ConfigMap; +import io.javaoperatorsdk.operator.api.reconciler.Context; +import io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResource; +import io.javaoperatorsdk.operator.processing.dependent.workflow.Condition; + +/** Reconcile precondition that always fails, simulating e.g. a missing prerequisite resource. */ +public class AlwaysFailingPrecondition + implements Condition { + + @Override + public boolean isMet( + DependentResource dependentResource, + BulkActivationConditionCustomResource primary, + Context context) { + return false; + } +} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/workflow/bulkactivationcondition/AlwaysTrueActivation.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/workflow/bulkactivationcondition/AlwaysTrueActivation.java new file mode 100644 index 0000000000..bc3b70bbb1 --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/workflow/bulkactivationcondition/AlwaysTrueActivation.java @@ -0,0 +1,34 @@ +/* + * Copyright Java Operator SDK Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.javaoperatorsdk.operator.workflow.bulkactivationcondition; + +import io.fabric8.kubernetes.api.model.Secret; +import io.javaoperatorsdk.operator.api.reconciler.Context; +import io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResource; +import io.javaoperatorsdk.operator.processing.dependent.workflow.Condition; + +/** Activation condition that always returns true — event source should always be registered. */ +public class AlwaysTrueActivation + implements Condition { + + @Override + public boolean isMet( + DependentResource dependentResource, + BulkActivationConditionCustomResource primary, + Context context) { + return true; + } +} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/workflow/bulkactivationcondition/BulkActivationConditionCustomResource.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/workflow/bulkactivationcondition/BulkActivationConditionCustomResource.java new file mode 100644 index 0000000000..da5d22f482 --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/workflow/bulkactivationcondition/BulkActivationConditionCustomResource.java @@ -0,0 +1,28 @@ +/* + * Copyright Java Operator SDK Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.javaoperatorsdk.operator.workflow.bulkactivationcondition; + +import io.fabric8.kubernetes.api.model.Namespaced; +import io.fabric8.kubernetes.client.CustomResource; +import io.fabric8.kubernetes.model.annotation.Group; +import io.fabric8.kubernetes.model.annotation.Kind; +import io.fabric8.kubernetes.model.annotation.Version; + +@Group("sample.javaoperatorsdk") +@Version("v1") +@Kind("BulkActivationCondition") +public class BulkActivationConditionCustomResource extends CustomResource + implements Namespaced {} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/workflow/bulkactivationcondition/BulkActivationConditionIT.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/workflow/bulkactivationcondition/BulkActivationConditionIT.java new file mode 100644 index 0000000000..d0370927fb --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/workflow/bulkactivationcondition/BulkActivationConditionIT.java @@ -0,0 +1,91 @@ +/* + * Copyright Java Operator SDK Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.javaoperatorsdk.operator.workflow.bulkactivationcondition; + +import java.time.Duration; + +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; + +import io.fabric8.kubernetes.api.model.ObjectMetaBuilder; +import io.javaoperatorsdk.annotation.Sample; +import io.javaoperatorsdk.operator.junit.LocallyRunOperatorExtension; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.awaitility.Awaitility.await; + +/** + * Reproducer for the bug where NodeDeleteExecutor fires for a BulkDependentResource whose + * activationCondition-gated event source has never been registered. + * + *

Workflow under test: + * + *

+ * ConfigMapDependentResource  (reconcilePrecondition = AlwaysFailingPrecondition)
+ *   └── SecretBulkDependentResource  (activationCondition = AlwaysTrueActivation)
+ * 
+ * + *

On first reconciliation the ConfigMap precondition fails → JOSDK calls + * markDependentsForDelete() → NodeDeleteExecutor fires for SecretBulkDependent → + * SecretBulkDependent.getSecondaryResources() calls eventSourceRetriever.getEventSourceFor() → the + * Secret event source was never registered (NodeReconcileExecutor never ran) → + * NoEventSourceForClassException. + * + *

This test FAILS on unfixed JOSDK, demonstrating the bug. + */ +@Sample( + tldr = "Bulk Dependent Resource with Activation Condition Bug Reproducer", + description = + """ + Reproducer for a bug where NodeDeleteExecutor fires for a BulkDependentResource \ + with an activationCondition before its event source has been registered, \ + causing NoEventSourceForClassException. Triggered when a parent dependent \ + has a failing reconcilePrecondition on first reconciliation. + """) +public class BulkActivationConditionIT { + + static final BulkActivationConditionReconciler reconciler = + new BulkActivationConditionReconciler(); + + @RegisterExtension + static LocallyRunOperatorExtension extension = + LocallyRunOperatorExtension.builder().withReconciler(reconciler).build(); + + @BeforeEach + void reset() { + reconciler.lastError.set(null); + reconciler.callCount.set(0); + } + + @Test + void nodeDeleteExecutorShouldNotThrowWhenEventSourceNotYetRegistered() { + var primary = new BulkActivationConditionCustomResource(); + primary.setMetadata( + new ObjectMetaBuilder() + .withName("test-primary") + .withNamespace(extension.getNamespace()) + .build()); + extension.create(primary); + + // Wait for reconcile() to be called. + // If the bug is present, SecretBulkDependentResource will be in error and lastError will be set + await().atMost(Duration.ofSeconds(10)).until(() -> reconciler.callCount.get() == 1); + + // On unfixed JOSDK this fails: lastError contains NoEventSourceForClassException. + assertThat(reconciler.lastError.get()).isNull(); + } +} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/workflow/bulkactivationcondition/BulkActivationConditionReconciler.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/workflow/bulkactivationcondition/BulkActivationConditionReconciler.java new file mode 100644 index 0000000000..0f658942ef --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/workflow/bulkactivationcondition/BulkActivationConditionReconciler.java @@ -0,0 +1,82 @@ +/* + * Copyright Java Operator SDK Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.javaoperatorsdk.operator.workflow.bulkactivationcondition; + +import java.util.concurrent.atomic.AtomicInteger; +import java.util.concurrent.atomic.AtomicReference; + +import io.javaoperatorsdk.operator.api.reconciler.*; +import io.javaoperatorsdk.operator.api.reconciler.dependent.Dependent; +import io.javaoperatorsdk.operator.processing.retry.GradualRetry; + +/** + * Workflow: + * + *

+ * ConfigMapDependentResource  (reconcilePrecondition = AlwaysFailingPrecondition)
+ *   └── SecretBulkDependentResource  (activationCondition = AlwaysTrueActivation)
+ * 
+ * + *

On first reconciliation: ConfigMap precondition fails → markDependentsForDelete cascades to + * SecretBulkDependentResource → NodeDeleteExecutor fires for Secret before its event source is ever + * registered → NoEventSourceForClassException. + */ +@Workflow( + dependents = { + @Dependent( + name = "configmap", + type = ConfigMapDependentResource.class, + reconcilePrecondition = AlwaysFailingPrecondition.class), + @Dependent( + name = SecretBulkDependentResource.NAME, + type = SecretBulkDependentResource.class, + activationCondition = AlwaysTrueActivation.class, + dependsOn = "configmap") + }, + handleExceptionsInReconciler = true) +@GradualRetry(maxAttempts = 0) +@ControllerConfiguration(maxReconciliationInterval = @MaxReconciliationInterval(interval = 0)) +public class BulkActivationConditionReconciler + implements Reconciler { + + /** Tracks how many times reconcile() or updateErrorStatus() has been called. */ + final AtomicInteger callCount = new AtomicInteger(); + + /** Set when updateErrorStatus() is invoked; null means no error occurred. */ + final AtomicReference lastError = new AtomicReference<>(); + + @Override + public UpdateControl reconcile( + BulkActivationConditionCustomResource primary, + Context context) { + final var workflowResult = + context + .managedWorkflowAndDependentResourceContext() + .getWorkflowReconcileResult() + .orElseThrow(); + final var erroredDependents = workflowResult.getErroredDependents(); + if (!erroredDependents.isEmpty()) { + final var exception = + erroredDependents.get( + workflowResult + .getDependentResourceByName(SecretBulkDependentResource.NAME) + .orElseThrow()); + lastError.set(exception); + } + callCount.incrementAndGet(); + return UpdateControl.noUpdate(); + } +} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/workflow/bulkactivationcondition/ConfigMapDependentResource.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/workflow/bulkactivationcondition/ConfigMapDependentResource.java new file mode 100644 index 0000000000..26b45af98a --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/workflow/bulkactivationcondition/ConfigMapDependentResource.java @@ -0,0 +1,43 @@ +/* + * Copyright Java Operator SDK Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.javaoperatorsdk.operator.workflow.bulkactivationcondition; + +import io.fabric8.kubernetes.api.model.ConfigMap; +import io.fabric8.kubernetes.api.model.ObjectMetaBuilder; +import io.javaoperatorsdk.operator.api.reconciler.Context; +import io.javaoperatorsdk.operator.processing.dependent.kubernetes.CRUDKubernetesDependentResource; + +/** + * Parent dependent resource. Its reconcile precondition always fails, which causes JOSDK to call + * markDependentsForDelete() on its children — triggering NodeDeleteExecutor for SecretBulkDependent + * before that resource's event source has ever been registered. + */ +public class ConfigMapDependentResource + extends CRUDKubernetesDependentResource { + + @Override + protected ConfigMap desired( + BulkActivationConditionCustomResource primary, + Context context) { + var cm = new ConfigMap(); + cm.setMetadata( + new ObjectMetaBuilder() + .withName(primary.getMetadata().getName()) + .withNamespace(primary.getMetadata().getNamespace()) + .build()); + return cm; + } +} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/workflow/bulkactivationcondition/SecretBulkDependentResource.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/workflow/bulkactivationcondition/SecretBulkDependentResource.java new file mode 100644 index 0000000000..d710a5a583 --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/workflow/bulkactivationcondition/SecretBulkDependentResource.java @@ -0,0 +1,80 @@ +/* + * Copyright Java Operator SDK Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.javaoperatorsdk.operator.workflow.bulkactivationcondition; + +import java.util.Map; +import java.util.function.Function; +import java.util.stream.Collectors; + +import io.fabric8.kubernetes.api.model.ObjectMetaBuilder; +import io.fabric8.kubernetes.api.model.Secret; +import io.javaoperatorsdk.operator.api.reconciler.Context; +import io.javaoperatorsdk.operator.processing.ResourceIDMapper; +import io.javaoperatorsdk.operator.processing.dependent.CRUDKubernetesBulkDependentResource; +import io.javaoperatorsdk.operator.processing.dependent.kubernetes.KubernetesDependentResource; +import io.javaoperatorsdk.operator.processing.event.ResourceID; + +/** + * Child bulk dependent resource with an activationCondition. + * + *

The bug: when NodeDeleteExecutor fires for this resource (because its parent + * ConfigMapDependentResource has a failing reconcilePrecondition), the event source for Secret has + * never been registered — NodeReconcileExecutor never ran for this node. NodeDeleteExecutor does + * NOT call registerOrDeregisterEventSourceBasedOnActivation() before calling delete(), so + * getSecondaryResources() → eventSourceRetriever.getEventSourceFor(Secret.class) throws + * NoEventSourceForClassException. + * + *

This implementation intentionally has no try/catch — it exposes the raw bug. + */ +public class SecretBulkDependentResource + extends KubernetesDependentResource + implements CRUDKubernetesBulkDependentResource { + + static final String NAME = "secret"; + static final String LABEL_KEY = "reproducer"; + static final String LABEL_VALUE = "bulk-activation-condition"; + + @Override + public Map desiredResources( + BulkActivationConditionCustomResource primary, + Context context) { + var secret = new Secret(); + secret.setMetadata( + new ObjectMetaBuilder() + .withName(primary.getMetadata().getName()) + .withNamespace(primary.getMetadata().getNamespace()) + .withLabels(Map.of(LABEL_KEY, LABEL_VALUE)) + .build()); + return Map.of(ResourceIDMapper.kubernetesResourceIdMapper().idFor(secret), secret); + } + + @Override + public Map getSecondaryResources( + BulkActivationConditionCustomResource primary, + Context context) { + // Deliberately uses getEventSourceFor (singular) — not getSecondaryResourcesAsStream — + // because the singular variant throws NoEventSourceForClassException when the source is absent. + // This mirrors the Kroxylicious ClusterRouteDependentResource pattern and exposes the bug: + // on first reconciliation NodeDeleteExecutor fires before the event source is registered. + var secrets = + context + .eventSourceRetriever() + .getEventSourceFor(Secret.class) + .getSecondaryResources(primary); + return secrets.stream() + .collect(Collectors.toMap(ResourceID::fromResource, Function.identity())); + } +}