improve: dependent resources notify all owners by default (#2161)

* improve: dependent resources notify all owners by default

Signed-off-by: Attila Mészáros <csviri@gmail.com>

* improve: idempotent thread-safe start

This allows multiple threads to wait on an informers to start in an idempotent and thread-safe way

Signed-off-by: Attila Mészáros <csviri@gmail.com>

* revert start stop synx

Signed-off-by: Attila Mészáros <csviri@gmail.com>

* fix IT

Signed-off-by: Attila Mészáros <csviri@gmail.com>

---------

Signed-off-by: Attila Mészáros <csviri@gmail.com>

operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/KubernetesDependentResource.java

@@ -87,7 +87,7 @@ private SecondaryToPrimaryMapper<R> getSecondaryToPrimaryMapper() {
     if (this instanceof SecondaryToPrimaryMapper) {
       return (SecondaryToPrimaryMapper<R>) this;
     } else if (garbageCollected) {
-      return Mappers.fromOwnerReference();
+      return Mappers.fromOwnerReferences(false);
     } else if (useNonOwnerRefBasedSecondaryToPrimaryMapping()) {
       return Mappers.fromDefaultAnnotations();
     } else {

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

@@ -5,6 +5,7 @@
 import java.util.Optional;
 
 import io.fabric8.kubernetes.api.model.HasMetadata;
+import io.fabric8.kubernetes.api.model.OwnerReference;
 
 public class ResourceID implements Serializable {
 
@@ -21,13 +22,18 @@ public static Optional<ResourceID> fromFirstOwnerReference(HasMetadata resource,
       boolean clusterScoped) {
     var ownerReferences = resource.getMetadata().getOwnerReferences();
     if (!ownerReferences.isEmpty()) {
-      return Optional.of(new ResourceID(ownerReferences.get(0).getName(),
-          clusterScoped ? null : resource.getMetadata().getNamespace()));
+      return Optional.of(fromOwnerReference(resource, ownerReferences.get(0), clusterScoped));
     } else {
       return Optional.empty();
     }
   }
 
+  public static ResourceID fromOwnerReference(HasMetadata resource, OwnerReference ownerReference,
+      boolean clusterScoped) {
+    return new ResourceID(ownerReference.getName(),
+        clusterScoped ? null : resource.getMetadata().getNamespace());
+  }
+
   private final String name;
   private final String namespace;
 

operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/Mappers.java

@@ -57,6 +57,13 @@ public static <T extends HasMetadata> SecondaryToPrimaryMapper<T> fromOwnerRefer
         .orElseGet(Collections::emptySet);
   }
 
+  public static <T extends HasMetadata> SecondaryToPrimaryMapper<T> fromOwnerReferences(
+      boolean clusterScope) {
+    return resource -> resource.getMetadata().getOwnerReferences().stream()
+        .map(or -> ResourceID.fromOwnerReference(resource, or, clusterScope))
+        .collect(Collectors.toSet());
+  }
+
   private static <T extends HasMetadata> SecondaryToPrimaryMapper<T> fromMetadata(
       String nameKey, String namespaceKey, boolean isLabel) {
     return resource -> {

operator-framework/src/test/java/io/javaoperatorsdk/operator/MultiOwnerDependentTriggeringIT.java

@@ -0,0 +1,82 @@
+package io.javaoperatorsdk.operator;
+
+import java.util.Set;
+
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.RegisterExtension;
+
+import io.fabric8.kubernetes.api.model.ConfigMap;
+import io.fabric8.kubernetes.api.model.ObjectMetaBuilder;
+import io.javaoperatorsdk.operator.junit.LocallyRunOperatorExtension;
+import io.javaoperatorsdk.operator.sample.multipleupdateondependent.MultipleOwnerDependentConfigMap;
+import io.javaoperatorsdk.operator.sample.multipleupdateondependent.MultipleOwnerDependentCustomResource;
+import io.javaoperatorsdk.operator.sample.multipleupdateondependent.MultipleOwnerDependentReconciler;
+import io.javaoperatorsdk.operator.sample.multipleupdateondependent.MultipleOwnerDependentSpec;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.awaitility.Awaitility.await;
+
+class MultiOwnerDependentTriggeringIT {
+
+  public static final String VALUE_1 = "value1";
+  public static final String VALUE_2 = "value2";
+  public static final String NEW_VALUE_1 = "newValue1";
+  public static final String NEW_VALUE_2 = "newValue2";
+
+  @RegisterExtension
+  LocallyRunOperatorExtension extension =
+      LocallyRunOperatorExtension.builder()
+          .withConfigurationService(o -> o.withDefaultNonSSAResource(Set.of()))
+          .withReconciler(MultipleOwnerDependentReconciler.class)
+          .build();
+
+
+  @Test
+  void multiOwnerTriggeringAndManagement() {
+    var res1 = extension.create(testResource("res1", VALUE_1));
+    var res2 = extension.create(testResource("res2", VALUE_2));
+
+    await().untilAsserted(() -> {
+      var cm = extension.get(ConfigMap.class, MultipleOwnerDependentConfigMap.RESOURCE_NAME);
+
+      assertThat(cm).isNotNull();
+      assertThat(cm.getData())
+          .containsEntry(VALUE_1, VALUE_1)
+          .containsEntry(VALUE_2, VALUE_2);
+      assertThat(cm.getMetadata().getOwnerReferences()).hasSize(2);
+    });
+
+    res1.getSpec().setValue(NEW_VALUE_1);
+    extension.replace(res1);
+
+    await().untilAsserted(() -> {
+      var cm = extension.get(ConfigMap.class, MultipleOwnerDependentConfigMap.RESOURCE_NAME);
+      assertThat(cm.getData())
+          .containsEntry(NEW_VALUE_1, NEW_VALUE_1)
+          // note that it will still contain the old value too
+          .containsEntry(VALUE_1, VALUE_1);
+      assertThat(cm.getMetadata().getOwnerReferences()).hasSize(2);
+    });
+
+    res2.getSpec().setValue(NEW_VALUE_2);
+    extension.replace(res2);
+
+    await().untilAsserted(() -> {
+      var cm = extension.get(ConfigMap.class, MultipleOwnerDependentConfigMap.RESOURCE_NAME);
+      assertThat(cm.getData()).containsEntry(NEW_VALUE_2, NEW_VALUE_2);
+      assertThat(cm.getMetadata().getOwnerReferences()).hasSize(2);
+    });
+  }
+
+  MultipleOwnerDependentCustomResource testResource(String name, String value) {
+    var res = new MultipleOwnerDependentCustomResource();
+    res.setMetadata(new ObjectMetaBuilder()
+        .withName(name)
+        .build());
+    res.setSpec(new MultipleOwnerDependentSpec());
+    res.getSpec().setValue(value);
+
+    return res;
+  }
+
+}

operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/multipleupdateondependent/MultipleOwnerDependentConfigMap.java

@@ -0,0 +1,55 @@
+package io.javaoperatorsdk.operator.sample.multipleupdateondependent;
+
+import java.util.HashMap;
+import java.util.List;
+import java.util.Optional;
+
+import io.fabric8.kubernetes.api.model.ConfigMap;
+import io.fabric8.kubernetes.api.model.ConfigMapBuilder;
+import io.javaoperatorsdk.operator.api.reconciler.Context;
+import io.javaoperatorsdk.operator.processing.dependent.kubernetes.BooleanWithUndefined;
+import io.javaoperatorsdk.operator.processing.dependent.kubernetes.CRUDKubernetesDependentResource;
+import io.javaoperatorsdk.operator.processing.dependent.kubernetes.KubernetesDependent;
+import io.javaoperatorsdk.operator.processing.event.ResourceID;
+import io.javaoperatorsdk.operator.processing.event.source.informer.InformerEventSource;
+
+@KubernetesDependent(useSSA = BooleanWithUndefined.TRUE)
+public class MultipleOwnerDependentConfigMap
+    extends
+    CRUDKubernetesDependentResource<ConfigMap, MultipleOwnerDependentCustomResource> {
+
+  public static final String RESOURCE_NAME = "test1";
+
+  public MultipleOwnerDependentConfigMap() {
+    super(ConfigMap.class);
+  }
+
+  @Override
+  protected ConfigMap desired(MultipleOwnerDependentCustomResource primary,
+      Context<MultipleOwnerDependentCustomResource> context) {
+
+    var cm = getSecondaryResource(primary, context);
+
+    var data = cm.map(ConfigMap::getData).orElse(new HashMap<>());
+    data.put(primary.getSpec().getValue(), primary.getSpec().getValue());
+
+    return new ConfigMapBuilder()
+        .withNewMetadata()
+        .withName(RESOURCE_NAME)
+        .withNamespace(primary.getMetadata().getNamespace())
+        .withOwnerReferences(cm.map(c -> c.getMetadata().getOwnerReferences()).orElse(List.of()))
+        .endMetadata()
+        .withData(data)
+        .build();
+  }
+
+  // need to change this since owner reference is present only for the creator primary resource.
+  @Override
+  public Optional<ConfigMap> getSecondaryResource(MultipleOwnerDependentCustomResource primary,
+      Context<MultipleOwnerDependentCustomResource> context) {
+    InformerEventSource<ConfigMap, MultipleOwnerDependentCustomResource> ies =
+        (InformerEventSource<ConfigMap, MultipleOwnerDependentCustomResource>) context
+            .eventSourceRetriever().getResourceEventSourceFor(ConfigMap.class);
+    return ies.get(new ResourceID(RESOURCE_NAME, primary.getMetadata().getNamespace()));
+  }
+}

operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/multipleupdateondependent/MultipleOwnerDependentCustomResource.java

@@ -0,0 +1,16 @@
+package io.javaoperatorsdk.operator.sample.multipleupdateondependent;
+
+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("mod")
+public class MultipleOwnerDependentCustomResource
+    extends CustomResource<MultipleOwnerDependentSpec, Void>
+    implements Namespaced {
+
+}

operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/multipleupdateondependent/MultipleOwnerDependentReconciler.java

@@ -0,0 +1,37 @@
+package io.javaoperatorsdk.operator.sample.multipleupdateondependent;
+
+import java.util.concurrent.atomic.AtomicInteger;
+
+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.dependent.Dependent;
+import io.javaoperatorsdk.operator.support.TestExecutionInfoProvider;
+
+@ControllerConfiguration(dependents = {
+    @Dependent(type = MultipleOwnerDependentConfigMap.class)
+})
+public class MultipleOwnerDependentReconciler
+    implements Reconciler<MultipleOwnerDependentCustomResource>,
+    TestExecutionInfoProvider {
+
+  private final AtomicInteger numberOfExecutions = new AtomicInteger(0);
+
+  public MultipleOwnerDependentReconciler() {}
+
+  @Override
+  public UpdateControl<MultipleOwnerDependentCustomResource> reconcile(
+      MultipleOwnerDependentCustomResource resource,
+      Context<MultipleOwnerDependentCustomResource> context) {
+    numberOfExecutions.getAndIncrement();
+
+    return UpdateControl.noUpdate();
+  }
+
+
+  public int getNumberOfExecutions() {
+    return numberOfExecutions.get();
+  }
+
+}

operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/multipleupdateondependent/MultipleOwnerDependentSpec.java

@@ -0,0 +1,15 @@
+package io.javaoperatorsdk.operator.sample.multipleupdateondependent;
+
+public class MultipleOwnerDependentSpec {
+
+  private String value;
+
+  public String getValue() {
+    return value;
+  }
+
+  public MultipleOwnerDependentSpec setValue(String value) {
+    this.value = value;
+    return this;
+  }
+}