feat: using annotations to identify primary for a secondary object if no owner reference can be added (#1197)

csviri · csviri · commit f82ab55d088b · 2022-07-14T10:39:21.000+02:00
diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/KubernetesDependentResource.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/KubernetesDependentResource.java
@@ -1,5 +1,6 @@
 package io.javaoperatorsdk.operator.processing.dependent.kubernetes;
 
+import java.util.HashMap;
 import java.util.Optional;
 import java.util.Set;
 
@@ -11,6 +12,7 @@
 import io.fabric8.kubernetes.client.KubernetesClient;
 import io.fabric8.kubernetes.client.dsl.NonNamespaceOperation;
 import io.fabric8.kubernetes.client.dsl.Resource;
+import io.javaoperatorsdk.operator.OperatorException;
 import io.javaoperatorsdk.operator.api.config.informer.InformerConfiguration;
 import io.javaoperatorsdk.operator.api.reconciler.Constants;
 import io.javaoperatorsdk.operator.api.reconciler.Context;
@@ -66,18 +68,29 @@ private void configureWith(String labelSelector, Set<String> namespaces,
       namespaces = context.getControllerConfiguration().getNamespaces();
     }
 
-    final SecondaryToPrimaryMapper<R> primaryResourcesRetriever =
-        (this instanceof SecondaryToPrimaryMapper) ? (SecondaryToPrimaryMapper<R>) this
-            : Mappers.fromOwnerReference();
     var ic = InformerConfiguration.from(resourceType())
         .withLabelSelector(labelSelector)
-        .withSecondaryToPrimaryMapper(primaryResourcesRetriever)
+        .withSecondaryToPrimaryMapper(getSecondaryToPrimaryMapper())
         .withNamespaces(namespaces, inheritNamespacesOnChange)
         .build();
 
     configureWith(new InformerEventSource<>(ic, client));
   }
 
+  @SuppressWarnings("unchecked")
+  private SecondaryToPrimaryMapper<R> getSecondaryToPrimaryMapper() {
+    if (this instanceof SecondaryToPrimaryMapper) {
+      return (SecondaryToPrimaryMapper<R>) this;
+    } else if (garbageCollected) {
+      return Mappers.fromOwnerReference();
+    } else if (useDefaultAnnotationsToIdentifyPrimary()) {
+      return Mappers.fromDefaultAnnotations();
+    } else {
+      throw new OperatorException("Provide a SecondaryToPrimaryMapper to associate " +
+          "this resource with the primary resource. DependentResource: " + getClass().getName());
+    }
+  }
+
   /**
    * Use to share informers between event more resources.
    *
@@ -138,6 +151,8 @@ protected NonNamespaceOperation<R, KubernetesResourceList<R>, Resource<R>> prepa
         ResourceID.fromResource(desired));
     if (addOwnerReference()) {
       desired.addOwnerReference(primary);
+    } else if (useDefaultAnnotationsToIdentifyPrimary()) {
+      addDefaultSecondaryToPrimaryMapperAnnotations(desired, primary);
     }
     Class<R> targetClass = (Class<R>) desired.getClass();
     return client.resources(targetClass).inNamespace(desired.getMetadata().getNamespace());
@@ -159,6 +174,24 @@ protected InformerEventSource<R, P> createEventSource(EventSourceContext<P> cont
     return eventSource();
   }
 
+  private boolean useDefaultAnnotationsToIdentifyPrimary() {
+    return !(this instanceof SecondaryToPrimaryMapper) && !garbageCollected && creatable;
+  }
+
+  private void addDefaultSecondaryToPrimaryMapperAnnotations(R desired, P primary) {
+    var annotations = desired.getMetadata().getAnnotations();
+    if (annotations == null) {
+      annotations = new HashMap<>();
+      desired.getMetadata().setAnnotations(annotations);
+    }
+    annotations.put(Mappers.DEFAULT_ANNOTATION_FOR_NAME, primary.getMetadata().getName());
+    var primaryNamespaces = primary.getMetadata().getNamespace();
+    if (primaryNamespaces != null) {
+      annotations.put(
+          Mappers.DEFAULT_ANNOTATION_FOR_NAMESPACE, primary.getMetadata().getNamespace());
+    }
+  }
+
   protected boolean addOwnerReference() {
     return garbageCollected;
   }
diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/KubernetesDependentResourceConfig.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/KubernetesDependentResourceConfig.java
@@ -19,7 +19,7 @@ public KubernetesDependentResourceConfig(Set<String> namespaces, String labelSel
       boolean configuredNS) {
     this.namespaces = namespaces;
     this.labelSelector = labelSelector;
-    namespacesWereConfigured = configuredNS;
+    this.namespacesWereConfigured = configuredNS;
   }
 
   public KubernetesDependentResourceConfig(Set<String> namespaces, String labelSelector) {
@@ -48,4 +48,5 @@ public String labelSelector() {
   public boolean wereNamespacesConfigured() {
     return namespacesWereConfigured;
   }
+
 }
diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/Mappers.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/Mappers.java
@@ -9,6 +9,10 @@
 
 public class Mappers {
 
+  public static final String DEFAULT_ANNOTATION_FOR_NAME = "io.javaoperatorsdk/primary-name";
+  public static final String DEFAULT_ANNOTATION_FOR_NAMESPACE =
+      "io.javaoperatorsdk/primary-namespace";
+
   private Mappers() {}
 
   public static <T extends HasMetadata> SecondaryToPrimaryMapper<T> fromAnnotation(
@@ -26,6 +30,10 @@ public static <T extends HasMetadata> SecondaryToPrimaryMapper<T> fromLabel(
     return fromMetadata(nameKey, null, true);
   }
 
+  public static <T extends HasMetadata> SecondaryToPrimaryMapper<T> fromDefaultAnnotations() {
+    return fromMetadata(DEFAULT_ANNOTATION_FOR_NAME, DEFAULT_ANNOTATION_FOR_NAMESPACE, false);
+  }
+
   public static <T extends HasMetadata> SecondaryToPrimaryMapper<T> fromLabel(
       String nameKey, String namespaceKey) {
     return fromMetadata(nameKey, namespaceKey, true);
diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/DependentAnnotationSecondaryMapperIT.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/DependentAnnotationSecondaryMapperIT.java
@@ -0,0 +1,66 @@
+package io.javaoperatorsdk.operator;
+
+import java.time.Duration;
+
+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.LocalOperatorExtension;
+import io.javaoperatorsdk.operator.sample.dependentannotationsecondarymapper.DependentAnnotationSecondaryMapperReconciler;
+import io.javaoperatorsdk.operator.sample.dependentannotationsecondarymapper.DependentAnnotationSecondaryMapperResource;
+
+import static io.javaoperatorsdk.operator.processing.event.source.informer.Mappers.DEFAULT_ANNOTATION_FOR_NAME;
+import static io.javaoperatorsdk.operator.processing.event.source.informer.Mappers.DEFAULT_ANNOTATION_FOR_NAMESPACE;
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.awaitility.Awaitility.await;
+
+class DependentAnnotationSecondaryMapperIT {
+
+  public static final String TEST_RESOURCE_NAME = "test1";
+
+  @RegisterExtension
+  LocalOperatorExtension operator =
+      LocalOperatorExtension.builder()
+          .withReconciler(DependentAnnotationSecondaryMapperReconciler.class)
+          .build();
+
+  @Test
+  void mapsSecondaryByAnnotation() {
+    operator.create(DependentAnnotationSecondaryMapperResource.class, testResource());
+
+    var reconciler =
+        operator.getReconcilerOfType(DependentAnnotationSecondaryMapperReconciler.class);
+
+    await().pollDelay(Duration.ofMillis(150)).untilAsserted(() -> {
+      assertThat(reconciler.getNumberOfExecutions()).isEqualTo(1);
+    });
+    var configMap = operator.get(ConfigMap.class, TEST_RESOURCE_NAME);
+
+    var annotations = configMap.getMetadata().getAnnotations();
+
+    assertThat(annotations)
+        .containsEntry(DEFAULT_ANNOTATION_FOR_NAME, TEST_RESOURCE_NAME)
+        .containsEntry(DEFAULT_ANNOTATION_FOR_NAMESPACE, operator.getNamespace());
+
+    assertThat(configMap.getMetadata().getOwnerReferences()).isEmpty();
+
+    configMap.getData().put("additional_data", "data");
+    operator.replace(ConfigMap.class, configMap);
+
+    await().pollDelay(Duration.ofMillis(150)).untilAsserted(() -> {
+      assertThat(reconciler.getNumberOfExecutions()).isEqualTo(2);
+    });
+  }
+
+
+  DependentAnnotationSecondaryMapperResource testResource() {
+    var res = new DependentAnnotationSecondaryMapperResource();
+    res.setMetadata(new ObjectMetaBuilder()
+        .withName(TEST_RESOURCE_NAME)
+        .build());
+    return res;
+  }
+
+}
diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/dependentannotationsecondarymapper/DependentAnnotationSecondaryMapperReconciler.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/dependentannotationsecondarymapper/DependentAnnotationSecondaryMapperReconciler.java
@@ -0,0 +1,58 @@
+package io.javaoperatorsdk.operator.sample.dependentannotationsecondarymapper;
+
+import java.util.Map;
+import java.util.concurrent.atomic.AtomicInteger;
+
+import io.fabric8.kubernetes.api.model.ConfigMap;
+import io.fabric8.kubernetes.api.model.ObjectMetaBuilder;
+import io.javaoperatorsdk.operator.api.reconciler.*;
+import io.javaoperatorsdk.operator.api.reconciler.dependent.Deleter;
+import io.javaoperatorsdk.operator.api.reconciler.dependent.Dependent;
+import io.javaoperatorsdk.operator.processing.dependent.Creator;
+import io.javaoperatorsdk.operator.processing.dependent.Updater;
+import io.javaoperatorsdk.operator.processing.dependent.kubernetes.KubernetesDependentResource;
+import io.javaoperatorsdk.operator.support.TestExecutionInfoProvider;
+
+@ControllerConfiguration(dependents = {@Dependent(
+    type = DependentAnnotationSecondaryMapperReconciler.ConfigMapDependentResource.class)})
+public class DependentAnnotationSecondaryMapperReconciler
+    implements Reconciler<DependentAnnotationSecondaryMapperResource>, TestExecutionInfoProvider {
+
+  private final AtomicInteger numberOfExecutions = new AtomicInteger(0);
+
+  @Override
+  public UpdateControl<DependentAnnotationSecondaryMapperResource> reconcile(
+      DependentAnnotationSecondaryMapperResource resource,
+      Context<DependentAnnotationSecondaryMapperResource> context) {
+    numberOfExecutions.addAndGet(1);
+    return UpdateControl.noUpdate();
+  }
+
+  public int getNumberOfExecutions() {
+    return numberOfExecutions.get();
+  }
+
+  public static class ConfigMapDependentResource extends
+      KubernetesDependentResource<ConfigMap, DependentAnnotationSecondaryMapperResource>
+      implements Creator<ConfigMap, DependentAnnotationSecondaryMapperResource>,
+      Updater<ConfigMap, DependentAnnotationSecondaryMapperResource>,
+      Deleter<DependentAnnotationSecondaryMapperResource> {
+
+    public ConfigMapDependentResource() {
+      super(ConfigMap.class);
+    }
+
+    @Override
+    protected ConfigMap desired(DependentAnnotationSecondaryMapperResource primary,
+        Context<DependentAnnotationSecondaryMapperResource> context) {
+      ConfigMap configMap = new ConfigMap();
+      configMap.setMetadata(new ObjectMetaBuilder()
+          .withName(primary.getMetadata().getName())
+          .withNamespace(primary.getMetadata().getNamespace())
+          .build());
+      configMap.setData(Map.of("data", primary.getMetadata().getName()));
+      return configMap;
+    }
+  }
+
+}
diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/dependentannotationsecondarymapper/DependentAnnotationSecondaryMapperResource.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/dependentannotationsecondarymapper/DependentAnnotationSecondaryMapperResource.java
@@ -0,0 +1,17 @@
+package io.javaoperatorsdk.operator.sample.dependentannotationsecondarymapper;
+
+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.ShortNames;
+import io.fabric8.kubernetes.model.annotation.Version;
+
+@Group("sample.javaoperatorsdk")
+@Version("v1")
+@Kind("MaxIntervalTestCustomResource")
+@ShortNames("mit")
+public class DependentAnnotationSecondaryMapperResource
+    extends CustomResource<Void, DependentAnnotationSecondaryMapperResourceStatus>
+    implements Namespaced {
+}
diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/dependentannotationsecondarymapper/DependentAnnotationSecondaryMapperResourceStatus.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/dependentannotationsecondarymapper/DependentAnnotationSecondaryMapperResourceStatus.java
@@ -0,0 +1,5 @@
+package io.javaoperatorsdk.operator.sample.dependentannotationsecondarymapper;
+
+public class DependentAnnotationSecondaryMapperResourceStatus {
+
+}
diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/dependentresourcecrossref/DependentResourceCrossRefReconciler.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/dependentresourcecrossref/DependentResourceCrossRefReconciler.java
@@ -10,7 +10,7 @@
 import io.fabric8.kubernetes.api.model.Secret;
 import io.javaoperatorsdk.operator.api.reconciler.*;
 import io.javaoperatorsdk.operator.api.reconciler.dependent.Dependent;
-import io.javaoperatorsdk.operator.processing.dependent.kubernetes.CRUKubernetesDependentResource;
+import io.javaoperatorsdk.operator.processing.dependent.kubernetes.CRUDKubernetesDependentResource;
 
 @ControllerConfiguration(dependents = {
     @Dependent(type = DependentResourceCrossRefReconciler.SecretDependentResource.class),
@@ -47,7 +47,7 @@ public boolean isErrorHappened() {
   }
 
   public static class SecretDependentResource extends
-      CRUKubernetesDependentResource<Secret, DependentResourceCrossRefResource> {
+      CRUDKubernetesDependentResource<Secret, DependentResourceCrossRefResource> {
 
     public SecretDependentResource() {
       super(Secret.class);
@@ -67,8 +67,7 @@ protected Secret desired(DependentResourceCrossRefResource primary,
   }
 
   public static class ConfigMapDependentResource extends
-      CRUKubernetesDependentResource<ConfigMap, DependentResourceCrossRefResource> {
-
+      CRUDKubernetesDependentResource<ConfigMap, DependentResourceCrossRefResource> {
 
     public ConfigMapDependentResource() {
       super(ConfigMap.class);
diff --git a/sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/ConfigMapDependentResource.java b/sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/ConfigMapDependentResource.java
@@ -19,7 +19,8 @@
 
 // this annotation only activates when using managed dependents and is not otherwise needed
 @KubernetesDependent(labelSelector = SELECTOR)
-class ConfigMapDependentResource extends CRUDKubernetesDependentResource<ConfigMap, WebPage> {
+public class ConfigMapDependentResource
+    extends CRUDKubernetesDependentResource<ConfigMap, WebPage> {
 
   private static final Logger log = LoggerFactory.getLogger(ConfigMapDependentResource.class);
 
diff --git a/sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/DeploymentDependentResource.java b/sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/DeploymentDependentResource.java
@@ -16,7 +16,8 @@
 
 // this annotation only activates when using managed dependents and is not otherwise needed
 @KubernetesDependent(labelSelector = SELECTOR)
-class DeploymentDependentResource extends CRUDKubernetesDependentResource<Deployment, WebPage> {
+public class DeploymentDependentResource
+    extends CRUDKubernetesDependentResource<Deployment, WebPage> {
 
   public DeploymentDependentResource() {
     super(Deployment.class);
diff --git a/sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/ServiceDependentResource.java b/sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/ServiceDependentResource.java
@@ -14,7 +14,7 @@
 
 // this annotation only activates when using managed dependents and is not otherwise needed
 @KubernetesDependent(labelSelector = SELECTOR)
-class ServiceDependentResource extends
+public class ServiceDependentResource extends
     io.javaoperatorsdk.operator.processing.dependent.kubernetes.CRUDKubernetesDependentResource<Service, WebPage> {
 
   public ServiceDependentResource() {

Original file line number	Diff line number	Diff line change
`@@ -19,7 +19,7 @@ public KubernetesDependentResourceConfig(Set<String> namespaces, String labelSel`
`19`	`19`	`boolean configuredNS) {`
`20`	`20`	`this.namespaces = namespaces;`
`21`	`21`	`this.labelSelector = labelSelector;`
`22`		`- namespacesWereConfigured = configuredNS;`
	`22`	`+ this.namespacesWereConfigured = configuredNS;`
`23`	`23`	`}`
`24`	`24`
`25`	`25`	`public KubernetesDependentResourceConfig(Set<String> namespaces, String labelSelector) {`
`@@ -48,4 +48,5 @@ public String labelSelector() {`
`48`	`48`	`public boolean wereNamespacesConfigured() {`
`49`	`49`	`return namespacesWereConfigured;`
`50`	`50`	`}`
	`51`	`+`
`51`	`52`	`}`
-Original file line number
+Diff line change
@@ @@ -0,0 +1,5 @@ @@
 +package io.javaoperatorsdk.operator.sample.dependentannotationsecondarymapper;
++
 +public class DependentAnnotationSecondaryMapperResourceStatus {
++
 +}