feat: context getSecondary resource is activation condition aware (#2532

)


Signed-off-by: Attila Mészáros <[email protected]>

operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/DefaultContext.java

@@ -13,6 +13,7 @@
 import io.javaoperatorsdk.operator.api.reconciler.dependent.managed.ManagedWorkflowAndDependentResourceContext;
 import io.javaoperatorsdk.operator.processing.Controller;
 import io.javaoperatorsdk.operator.processing.event.EventSourceRetriever;
+import io.javaoperatorsdk.operator.processing.event.NoEventSourceForClassException;
 import io.javaoperatorsdk.operator.processing.event.ResourceID;
 
 public class DefaultContext<P extends HasMetadata> implements Context<P> {
@@ -62,10 +63,26 @@ public <R> Stream<R> getSecondaryResourcesAsStream(Class<R> expectedType) {
 
   @Override
   public <T> Optional<T> getSecondaryResource(Class<T> expectedType, String eventSourceName) {
-    return controller
-        .getEventSourceManager()
-        .getEventSourceFor(expectedType, eventSourceName)
-        .getSecondaryResource(primaryResource);
+    try {
+      return controller
+          .getEventSourceManager()
+          .getEventSourceFor(expectedType, eventSourceName)
+          .getSecondaryResource(primaryResource);
+    } catch (NoEventSourceForClassException e) {
+      /*
+       * If a workflow has an activation condition there can be event sources which are only
+       * registered if the activation condition holds, but to provide a consistent API we return an
+       * Optional instead of throwing an exception.
+       * 
+       * Note that not only the resource which has an activation condition might not be registered
+       * but dependents which depend on it.
+       */
+      if (eventSourceName == null && controller.workflowContainsDependentForType(expectedType)) {
+        return Optional.empty();
+      } else {
+        throw e;
+      }
+    }
   }
 
   @Override

operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/Controller.java

@@ -473,4 +473,10 @@ public WorkflowCleanupResult cleanupManagedWorkflow(P resource, Context<P> conte
   public boolean isWorkflowExplicitInvocation() {
     return explicitWorkflowInvocation;
   }
+
+  public boolean workflowContainsDependentForType(Class<?> clazz) {
+    return managedWorkflow.getDependentResourcesByName().values().stream()
+        .anyMatch(d -> d.resourceType().equals(clazz));
+  }
+
 }

operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventSources.java

@@ -99,8 +99,7 @@ public <S> EventSource<S, P> get(Class<S> dependentType, String name) {
 
     final var sourcesForType = sources.get(keyFor(dependentType));
     if (sourcesForType == null || sourcesForType.isEmpty()) {
-      throw new IllegalArgumentException(
-          "There is no event source found for class:" + dependentType.getName());
+      throw new NoEventSourceForClassException(dependentType);
     }
 
     final var size = sourcesForType.size();

operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/NoEventSourceForClassException.java

@@ -0,0 +1,16 @@
+package io.javaoperatorsdk.operator.processing.event;
+
+import io.javaoperatorsdk.operator.OperatorException;
+
+public class NoEventSourceForClassException extends OperatorException {
+
+  private Class<?> clazz;
+
+  public NoEventSourceForClassException(Class<?> clazz) {
+    this.clazz = clazz;
+  }
+
+  public Class<?> getClazz() {
+    return clazz;
+  }
+}

operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/reconciler/DefaultContextTest.java

@@ -0,0 +1,37 @@
+package io.javaoperatorsdk.operator.api.reconciler;
+
+import org.junit.jupiter.api.Test;
+
+import io.fabric8.kubernetes.api.model.ConfigMap;
+import io.fabric8.kubernetes.api.model.Secret;
+import io.javaoperatorsdk.operator.processing.Controller;
+import io.javaoperatorsdk.operator.processing.event.EventSourceManager;
+import io.javaoperatorsdk.operator.processing.event.NoEventSourceForClassException;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.junit.jupiter.api.Assertions.*;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+class DefaultContextTest {
+
+  Secret primary = new Secret();
+  Controller<Secret> mockController = mock(Controller.class);
+
+  DefaultContext<?> context = new DefaultContext<>(null, mockController, primary);
+
+  @Test
+  void getSecondaryResourceReturnsEmptyOptionalOnNonActivatedDRType() {
+    var mockManager = mock(EventSourceManager.class);
+    when(mockController.getEventSourceManager()).thenReturn(mockManager);
+    when(mockController.workflowContainsDependentForType(ConfigMap.class)).thenReturn(true);
+    when(mockManager.getEventSourceFor(any(), any()))
+        .thenThrow(new NoEventSourceForClassException(ConfigMap.class));
+
+    var res = context.getSecondaryResource(ConfigMap.class);
+
+    assertThat(res).isEmpty();
+  }
+
+}

operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/EventSourceManagerTest.java

@@ -85,12 +85,12 @@ public void startCascadesToEventSources() {
 
   @Test
   void retrievingEventSourceForClassShouldWork() {
-    assertThatExceptionOfType(IllegalArgumentException.class)
+    assertThatExceptionOfType(NoEventSourceForClassException.class)
         .isThrownBy(() -> eventSourceManager.getEventSourceFor(Class.class));
 
     // manager is initialized with a controller configured to handle HasMetadata
     EventSourceManager manager = initManager();
-    assertThatExceptionOfType(IllegalArgumentException.class)
+    assertThatExceptionOfType(NoEventSourceForClassException.class)
         .isThrownBy(() -> manager.getEventSourceFor(HasMetadata.class, "unknown_name"));
 
     ManagedInformerEventSource eventSource = mock(ManagedInformerEventSource.class);

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

@@ -0,0 +1,28 @@
+package io.javaoperatorsdk.operator.workflow.getnonactivesecondary;
+
+
+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;
+
+public class ConfigMapDependentResource
+    extends CRUDKubernetesDependentResource<ConfigMap, GetNonActiveSecondaryCustomResource> {
+
+  public static final String DATA_KEY = "data";
+
+  public ConfigMapDependentResource() {
+    super(ConfigMap.class);
+  }
+
+  @Override
+  protected ConfigMap desired(GetNonActiveSecondaryCustomResource primary,
+      Context<GetNonActiveSecondaryCustomResource> context) {
+    ConfigMap configMap = new ConfigMap();
+    configMap.setMetadata(new ObjectMetaBuilder()
+        .withName(primary.getMetadata().getName())
+        .withNamespace(primary.getMetadata().getNamespace())
+        .build());
+    return configMap;
+  }
+}

operator-framework/src/test/java/io/javaoperatorsdk/operator/workflow/getnonactivesecondary/FalseActivationCondition.java

@@ -0,0 +1,17 @@
+package io.javaoperatorsdk.operator.workflow.getnonactivesecondary;
+
+import io.fabric8.openshift.api.model.Route;
+import io.javaoperatorsdk.operator.api.reconciler.Context;
+import io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResource;
+import io.javaoperatorsdk.operator.processing.dependent.workflow.Condition;
+
+public class FalseActivationCondition
+    implements Condition<Route, GetNonActiveSecondaryCustomResource> {
+  @Override
+  public boolean isMet(
+      DependentResource<Route, GetNonActiveSecondaryCustomResource> dependentResource,
+      GetNonActiveSecondaryCustomResource primary,
+      Context<GetNonActiveSecondaryCustomResource> context) {
+    return false;
+  }
+}

operator-framework/src/test/java/io/javaoperatorsdk/operator/workflow/getnonactivesecondary/GetNonActiveSecondaryCustomResource.java

@@ -0,0 +1,17 @@
+package io.javaoperatorsdk.operator.workflow.getnonactivesecondary;
+
+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.ShortNames;
+import io.fabric8.kubernetes.model.annotation.Version;
+
+@Group("sample.javaoperatorsdk")
+@Version("v1")
+@ShortNames("gnas")
+public class GetNonActiveSecondaryCustomResource
+    extends CustomResource<Void, Void>
+    implements Namespaced {
+
+
+}

operator-framework/src/test/java/io/javaoperatorsdk/operator/workflow/getnonactivesecondary/RouteDependentResource.java

@@ -0,0 +1,27 @@
+package io.javaoperatorsdk.operator.workflow.getnonactivesecondary;
+
+import io.fabric8.kubernetes.api.model.ObjectMetaBuilder;
+import io.fabric8.openshift.api.model.Route;
+import io.javaoperatorsdk.operator.api.reconciler.Context;
+import io.javaoperatorsdk.operator.processing.dependent.kubernetes.CRUDKubernetesDependentResource;
+
+public class RouteDependentResource
+    extends CRUDKubernetesDependentResource<Route, GetNonActiveSecondaryCustomResource> {
+
+  public RouteDependentResource() {
+    super(Route.class);
+  }
+
+  @Override
+  protected Route desired(GetNonActiveSecondaryCustomResource primary,
+      Context<GetNonActiveSecondaryCustomResource> context) {
+    // basically does not matter since this should not be called
+    Route route = new Route();
+    route.setMetadata(new ObjectMetaBuilder()
+        .withName(primary.getMetadata().getName())
+        .withNamespace(primary.getMetadata().getNamespace())
+        .build());
+
+    return route;
+  }
+}

operator-framework/src/test/java/io/javaoperatorsdk/operator/workflow/getnonactivesecondary/WorkflowActivationConditionIT.java

@@ -0,0 +1,40 @@
+package io.javaoperatorsdk.operator.workflow.getnonactivesecondary;
+
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.RegisterExtension;
+
+import io.fabric8.kubernetes.api.model.ObjectMetaBuilder;
+import io.javaoperatorsdk.operator.junit.LocallyRunOperatorExtension;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.awaitility.Awaitility.await;
+
+public class WorkflowActivationConditionIT {
+
+  public static final String TEST_RESOURCE_NAME = "test1";
+
+  @RegisterExtension
+  LocallyRunOperatorExtension extension =
+      LocallyRunOperatorExtension.builder()
+          .withReconciler(WorkflowActivationConditionReconciler.class)
+          .build();
+
+  @Test
+  void reconciledOnVanillaKubernetesDespiteRouteInWorkflow() {
+    extension.create(testResource());
+
+    await().untilAsserted(() -> {
+      assertThat(extension.getReconcilerOfType(WorkflowActivationConditionReconciler.class)
+          .getNumberOfReconciliationExecution()).isEqualTo(1);
+    });
+  }
+
+  private GetNonActiveSecondaryCustomResource testResource() {
+    var res = new GetNonActiveSecondaryCustomResource();
+    res.setMetadata(new ObjectMetaBuilder()
+        .withName(TEST_RESOURCE_NAME)
+        .build());
+    return res;
+  }
+
+}

operator-framework/src/test/java/io/javaoperatorsdk/operator/workflow/getnonactivesecondary/WorkflowActivationConditionReconciler.java

@@ -0,0 +1,40 @@
+package io.javaoperatorsdk.operator.workflow.getnonactivesecondary;
+
+import java.util.concurrent.atomic.AtomicInteger;
+
+import io.fabric8.openshift.api.model.Route;
+import io.javaoperatorsdk.operator.api.reconciler.Context;
+import io.javaoperatorsdk.operator.api.reconciler.ControllerConfiguration;
+import io.javaoperatorsdk.operator.api.reconciler.Reconciler;
+import io.javaoperatorsdk.operator.api.reconciler.UpdateControl;
+import io.javaoperatorsdk.operator.api.reconciler.Workflow;
+import io.javaoperatorsdk.operator.api.reconciler.dependent.Dependent;
+
+@Workflow(dependents = {
+    @Dependent(type = ConfigMapDependentResource.class),
+    @Dependent(type = RouteDependentResource.class,
+        activationCondition = FalseActivationCondition.class)
+})
+@ControllerConfiguration
+public class WorkflowActivationConditionReconciler
+    implements Reconciler<GetNonActiveSecondaryCustomResource> {
+
+  private final AtomicInteger numberOfReconciliationExecution = new AtomicInteger(0);
+
+  @Override
+  public UpdateControl<GetNonActiveSecondaryCustomResource> reconcile(
+      GetNonActiveSecondaryCustomResource resource,
+      Context<GetNonActiveSecondaryCustomResource> context) {
+
+    // should not throw an exception even if the condition is false
+    var route = context.getSecondaryResource(Route.class);
+
+    numberOfReconciliationExecution.incrementAndGet();
+
+    return UpdateControl.noUpdate();
+  }
+
+  public int getNumberOfReconciliationExecution() {
+    return numberOfReconciliationExecution.get();
+  }
+}