From 07a11f37011f4824c72c50bc1e59192ca71d296d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Attila=20M=C3=A9sz=C3=A1ros?= Date: Sat, 27 Apr 2024 15:03:34 +0200 Subject: [PATCH 1/5] feat: label selector on resources MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Attila Mészáros --- .../operator/glue/ControllerConfig.java | 2 ++ .../glue/reconciler/glue/GlueReconciler.java | 16 +++++++--- .../reconciler/glue/InformerRegister.java | 32 ++++++++++++++++--- 3 files changed, 41 insertions(+), 9 deletions(-) diff --git a/src/main/java/io/csviri/operator/glue/ControllerConfig.java b/src/main/java/io/csviri/operator/glue/ControllerConfig.java index e213e4d..fb31a31 100644 --- a/src/main/java/io/csviri/operator/glue/ControllerConfig.java +++ b/src/main/java/io/csviri/operator/glue/ControllerConfig.java @@ -9,4 +9,6 @@ public interface ControllerConfig { Map glueOperatorManagedGlueLabels(); + Map resourceLabelSelector(); + } diff --git a/src/main/java/io/csviri/operator/glue/reconciler/glue/GlueReconciler.java b/src/main/java/io/csviri/operator/glue/reconciler/glue/GlueReconciler.java index 0cd23f4..c37d56c 100644 --- a/src/main/java/io/csviri/operator/glue/reconciler/glue/GlueReconciler.java +++ b/src/main/java/io/csviri/operator/glue/reconciler/glue/GlueReconciler.java @@ -29,8 +29,6 @@ import io.javaoperatorsdk.operator.processing.dependent.workflow.KubernetesResourceDeletedCondition; import io.javaoperatorsdk.operator.processing.dependent.workflow.WorkflowBuilder; -import jakarta.inject.Inject; - import static io.csviri.operator.glue.Utils.getResourceForSSAFrom; import static io.csviri.operator.glue.reconciler.operator.GlueOperatorReconciler.PARENT_RELATED_RESOURCE_NAME; @@ -42,14 +40,22 @@ public class GlueReconciler implements Reconciler, Cleaner, ErrorSta public static final String PARENT_GLUE_FINALIZER_PREFIX = "io.csviri.operator.resourceflow.glue/"; public static final String GLUE_RECONCILER_NAME = "glue"; - @Inject - ValidationAndErrorHandler validationAndErrorHandler; + + private final ValidationAndErrorHandler validationAndErrorHandler; + private final InformerRegister informerRegister; private final KubernetesResourceDeletedCondition deletePostCondition = new KubernetesResourceDeletedCondition(); - private final InformerRegister informerRegister = new InformerRegister(); + private final GenericTemplateHandler genericTemplateHandler = new GenericTemplateHandler(); + + public GlueReconciler(ValidationAndErrorHandler validationAndErrorHandler, + InformerRegister informerRegister) { + this.validationAndErrorHandler = validationAndErrorHandler; + this.informerRegister = informerRegister; + } + /** * Handling finalizers for GlueOperator: Glue ids a finalizer to parent, that is necessary since * on clean up the resource name might be calculated based on the parents name, and it this way diff --git a/src/main/java/io/csviri/operator/glue/reconciler/glue/InformerRegister.java b/src/main/java/io/csviri/operator/glue/reconciler/glue/InformerRegister.java index 181dbd7..2f81f1f 100644 --- a/src/main/java/io/csviri/operator/glue/reconciler/glue/InformerRegister.java +++ b/src/main/java/io/csviri/operator/glue/reconciler/glue/InformerRegister.java @@ -7,6 +7,7 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import io.csviri.operator.glue.ControllerConfig; import io.csviri.operator.glue.Utils; import io.csviri.operator.glue.customresource.glue.Glue; import io.csviri.operator.glue.customresource.glue.RelatedResourceSpec; @@ -17,8 +18,11 @@ import io.javaoperatorsdk.operator.processing.event.ResourceID; import io.javaoperatorsdk.operator.processing.event.source.informer.InformerEventSource; +import jakarta.inject.Singleton; + // todo unit test -class InformerRegister { +@Singleton +public class InformerRegister { private static final Logger log = LoggerFactory.getLogger(InformerRegister.class); @@ -27,6 +31,12 @@ class InformerRegister { private final Map relatedResourceMappers = new ConcurrentHashMap<>(); + private final ControllerConfig controllerConfig; + + public InformerRegister(ControllerConfig controllerConfig) { + this.controllerConfig = controllerConfig; + } + // todo test related resources deleting public synchronized void deRegisterInformerOnResourceFlowChange(Context context, Glue primary) { @@ -78,9 +88,13 @@ public InformerEventSource registerInformer( mapper = relatedResourceMappers.get(gvk); markEventSource(gvk, glue); } - var newES = new InformerEventSource<>(InformerConfiguration.from(gvk) - .withSecondaryToPrimaryMapper(mapper) - .build(), context.eventSourceRetriever().eventSourceContextForDynamicRegistration()); + + var configBuilder = InformerConfiguration.from(gvk) + .withSecondaryToPrimaryMapper(mapper); + labelSelectorForGVK(gvk).ifPresent(configBuilder::withLabelSelector); + + var newES = new InformerEventSource<>(configBuilder.build(), + context.eventSourceRetriever().eventSourceContextForDynamicRegistration()); return (InformerEventSource) context .eventSourceRetriever() @@ -147,4 +161,14 @@ private String workflowId(Glue glue) { return glue.getMetadata().getName() + "#" + glue.getMetadata().getNamespace(); } + public Optional labelSelectorForGVK(GroupVersionKind gvk) { + return Optional.ofNullable(controllerConfig.resourceLabelSelector().get(toSimpleString(gvk))); + } + + public static String toSimpleString(GroupVersionKind gvk) { + String groupVersion = + gvk.getGroup() == null ? gvk.getVersion() : gvk.getGroup() + "/" + gvk.getVersion(); + return groupVersion + "#" + gvk.getKind(); + } + } From 07c9cbf21750d2d8caa835dbb4c992e667e66391 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Attila=20M=C3=A9sz=C3=A1ros?= Date: Sat, 27 Apr 2024 15:08:29 +0200 Subject: [PATCH 2/5] unit test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Attila Mészáros --- .../reconciler/glue/InformerRegisterTest.java | 21 +++++++++++++++++++ 1 file changed, 21 insertions(+) create mode 100644 src/test/java/io/csviri/operator/glue/reconciler/glue/InformerRegisterTest.java diff --git a/src/test/java/io/csviri/operator/glue/reconciler/glue/InformerRegisterTest.java b/src/test/java/io/csviri/operator/glue/reconciler/glue/InformerRegisterTest.java new file mode 100644 index 0000000..c357983 --- /dev/null +++ b/src/test/java/io/csviri/operator/glue/reconciler/glue/InformerRegisterTest.java @@ -0,0 +1,21 @@ +package io.csviri.operator.glue.reconciler.glue; + +import org.junit.jupiter.api.Test; + +import io.javaoperatorsdk.operator.processing.GroupVersionKind; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.junit.jupiter.api.Assertions.*; + +class InformerRegisterTest { + + + @Test + void gvkToSimpleString() { + assertThat(InformerRegister.toSimpleString(new GroupVersionKind("apps", "v1", "Deployment"))) + .isEqualTo("apps/v1#Deployment"); + assertThat(InformerRegister.toSimpleString(new GroupVersionKind("v1", "ConfigMap"))) + .isEqualTo("v1#ConfigMap"); + } + +} From 6b50c6496e10898a60c3e269ddf33468e6039e5b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Attila=20M=C3=A9sz=C3=A1ros?= Date: Sat, 27 Apr 2024 15:44:49 +0200 Subject: [PATCH 3/5] unit test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Attila Mészáros --- .../reconciler/glue/InformerProducer.java | 24 ++++++++++ .../reconciler/glue/InformerRegister.java | 12 +++-- .../reconciler/glue/InformerRegisterTest.java | 45 ++++++++++++++++++- 3 files changed, 76 insertions(+), 5 deletions(-) create mode 100644 src/main/java/io/csviri/operator/glue/reconciler/glue/InformerProducer.java diff --git a/src/main/java/io/csviri/operator/glue/reconciler/glue/InformerProducer.java b/src/main/java/io/csviri/operator/glue/reconciler/glue/InformerProducer.java new file mode 100644 index 0000000..bece7e1 --- /dev/null +++ b/src/main/java/io/csviri/operator/glue/reconciler/glue/InformerProducer.java @@ -0,0 +1,24 @@ +package io.csviri.operator.glue.reconciler.glue; + + +import io.csviri.operator.glue.customresource.glue.Glue; +import io.fabric8.kubernetes.api.model.GenericKubernetesResource; +import io.javaoperatorsdk.operator.api.config.informer.InformerConfiguration; +import io.javaoperatorsdk.operator.api.reconciler.Context; +import io.javaoperatorsdk.operator.processing.event.source.informer.InformerEventSource; + +import jakarta.inject.Singleton; + + +/** For mocking purpose only. Too complex to mock InformerEventSource creation. */ +@Singleton +public class InformerProducer { + + public InformerEventSource createInformer( + InformerConfiguration configuration, + Context context) { + return new InformerEventSource<>(configuration, + context.eventSourceRetriever().eventSourceContextForDynamicRegistration()); + } + +} diff --git a/src/main/java/io/csviri/operator/glue/reconciler/glue/InformerRegister.java b/src/main/java/io/csviri/operator/glue/reconciler/glue/InformerRegister.java index 2f81f1f..d8815c3 100644 --- a/src/main/java/io/csviri/operator/glue/reconciler/glue/InformerRegister.java +++ b/src/main/java/io/csviri/operator/glue/reconciler/glue/InformerRegister.java @@ -31,12 +31,17 @@ public class InformerRegister { private final Map relatedResourceMappers = new ConcurrentHashMap<>(); + + private final InformerProducer informerProducer; private final ControllerConfig controllerConfig; - public InformerRegister(ControllerConfig controllerConfig) { + + public InformerRegister(InformerProducer informerProducer, ControllerConfig controllerConfig) { + this.informerProducer = informerProducer; this.controllerConfig = controllerConfig; } + // todo test related resources deleting public synchronized void deRegisterInformerOnResourceFlowChange(Context context, Glue primary) { @@ -93,12 +98,11 @@ public InformerEventSource registerInformer( .withSecondaryToPrimaryMapper(mapper); labelSelectorForGVK(gvk).ifPresent(configBuilder::withLabelSelector); - var newES = new InformerEventSource<>(configBuilder.build(), - context.eventSourceRetriever().eventSourceContextForDynamicRegistration()); + var newInformer = informerProducer.createInformer(configBuilder.build(), context); return (InformerEventSource) context .eventSourceRetriever() - .dynamicallyRegisterEventSource(gvk.toString(), newES); + .dynamicallyRegisterEventSource(gvk.toString(), newInformer); } diff --git a/src/test/java/io/csviri/operator/glue/reconciler/glue/InformerRegisterTest.java b/src/test/java/io/csviri/operator/glue/reconciler/glue/InformerRegisterTest.java index c357983..78ba384 100644 --- a/src/test/java/io/csviri/operator/glue/reconciler/glue/InformerRegisterTest.java +++ b/src/test/java/io/csviri/operator/glue/reconciler/glue/InformerRegisterTest.java @@ -1,15 +1,50 @@ package io.csviri.operator.glue.reconciler.glue; +import java.util.Map; + import org.junit.jupiter.api.Test; +import io.csviri.operator.glue.ControllerConfig; +import io.csviri.operator.glue.customresource.glue.Glue; +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.GroupVersionKind; +import io.javaoperatorsdk.operator.processing.event.EventSourceRetriever; import static org.assertj.core.api.Assertions.assertThat; -import static org.junit.jupiter.api.Assertions.*; +import static org.mockito.Mockito.*; class InformerRegisterTest { + public static final String LABEL_SELECTOR = "myapp=true"; + + @Test + @SuppressWarnings("unchecked") + void registersInformerWithLabelSelectorIfConfigured() { + var gvk = GroupVersionKind.gvkFor(ConfigMap.class); + var labelSelectors = Map.of(InformerRegister.toSimpleString(gvk), + LABEL_SELECTOR); + var config = mock(ControllerConfig.class); + when(config.resourceLabelSelector()).thenReturn(labelSelectors); + var informerProducer = mock(InformerProducer.class); + + var register = new InformerRegister(informerProducer, config); + var mockContext = mock(Context.class); + var mockEventSourceRetriever = mock(EventSourceRetriever.class); + when(mockContext.eventSourceRetriever()).thenReturn(mockEventSourceRetriever); + + register.registerInformer(mockContext, gvk, testGlue()); + + verify(informerProducer).createInformer(argThat(c -> { + + assertThat(c.getLabelSelector()).isEqualTo(LABEL_SELECTOR); + return true; + }), any()); + } + + @Test void gvkToSimpleString() { assertThat(InformerRegister.toSimpleString(new GroupVersionKind("apps", "v1", "Deployment"))) @@ -18,4 +53,12 @@ void gvkToSimpleString() { .isEqualTo("v1#ConfigMap"); } + Glue testGlue() { + Glue glue = new Glue(); + glue.setMetadata(new ObjectMetaBuilder() + .withName("test1") + .build()); + return glue; + } + } From 8b1a324d1ae2c08d12151b7829f8345c0d2365a6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Attila=20M=C3=A9sz=C3=A1ros?= Date: Sat, 27 Apr 2024 15:49:22 +0200 Subject: [PATCH 4/5] decoration MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Attila Mészáros --- .../operator/glue/reconciler/glue/InformerRegisterTest.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/test/java/io/csviri/operator/glue/reconciler/glue/InformerRegisterTest.java b/src/test/java/io/csviri/operator/glue/reconciler/glue/InformerRegisterTest.java index 78ba384..f4f89be 100644 --- a/src/test/java/io/csviri/operator/glue/reconciler/glue/InformerRegisterTest.java +++ b/src/test/java/io/csviri/operator/glue/reconciler/glue/InformerRegisterTest.java @@ -29,7 +29,6 @@ void registersInformerWithLabelSelectorIfConfigured() { var config = mock(ControllerConfig.class); when(config.resourceLabelSelector()).thenReturn(labelSelectors); var informerProducer = mock(InformerProducer.class); - var register = new InformerRegister(informerProducer, config); var mockContext = mock(Context.class); var mockEventSourceRetriever = mock(EventSourceRetriever.class); @@ -38,7 +37,6 @@ void registersInformerWithLabelSelectorIfConfigured() { register.registerInformer(mockContext, gvk, testGlue()); verify(informerProducer).createInformer(argThat(c -> { - assertThat(c.getLabelSelector()).isEqualTo(LABEL_SELECTOR); return true; }), any()); From 713b04ec68bc18b403ce00eb89bf0d52842ba112 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Attila=20M=C3=A9sz=C3=A1ros?= Date: Sat, 27 Apr 2024 16:09:33 +0200 Subject: [PATCH 5/5] integration test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Attila Mészáros --- .../reconciler/glue/InformerRegister.java | 5 ++- .../glue/GlueResourceLabelSelectorTest.java | 44 +++++++++++++++++++ .../glue/SimpleGlueWithLabeledResource.yaml | 17 +++++++ 3 files changed, 65 insertions(+), 1 deletion(-) create mode 100644 src/test/java/io/csviri/operator/glue/GlueResourceLabelSelectorTest.java create mode 100644 src/test/resources/glue/SimpleGlueWithLabeledResource.yaml diff --git a/src/main/java/io/csviri/operator/glue/reconciler/glue/InformerRegister.java b/src/main/java/io/csviri/operator/glue/reconciler/glue/InformerRegister.java index d8815c3..628db2d 100644 --- a/src/main/java/io/csviri/operator/glue/reconciler/glue/InformerRegister.java +++ b/src/main/java/io/csviri/operator/glue/reconciler/glue/InformerRegister.java @@ -96,7 +96,10 @@ public InformerEventSource registerInformer( var configBuilder = InformerConfiguration.from(gvk) .withSecondaryToPrimaryMapper(mapper); - labelSelectorForGVK(gvk).ifPresent(configBuilder::withLabelSelector); + labelSelectorForGVK(gvk).ifPresent(ls -> { + log.debug("Registering label selector: {} for informer for gvk: {}", ls, gvk); + configBuilder.withLabelSelector(ls); + }); var newInformer = informerProducer.createInformer(configBuilder.build(), context); diff --git a/src/test/java/io/csviri/operator/glue/GlueResourceLabelSelectorTest.java b/src/test/java/io/csviri/operator/glue/GlueResourceLabelSelectorTest.java new file mode 100644 index 0000000..e1527c3 --- /dev/null +++ b/src/test/java/io/csviri/operator/glue/GlueResourceLabelSelectorTest.java @@ -0,0 +1,44 @@ +package io.csviri.operator.glue; + +import java.util.Map; + +import org.junit.jupiter.api.Test; + +import io.fabric8.kubernetes.api.model.ConfigMap; +import io.quarkus.test.junit.QuarkusTest; +import io.quarkus.test.junit.QuarkusTestProfile; +import io.quarkus.test.junit.TestProfile; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.awaitility.Awaitility.await; + +@QuarkusTest +@TestProfile(GlueResourceLabelSelectorTest.GlueResourceLabelSelectorTestProfile.class) +public class GlueResourceLabelSelectorTest extends TestBase { + + @Test + void showCreatingResourceWithLabelSelectorAndLabel() { + var glue = create(TestUtils.loadResoureFlow("/glue/SimpleGlueWithLabeledResource.yaml")); + + // this serves more like a sample, hard to test here if the label selector is on informer + await().untilAsserted(() -> { + var cm = get(ConfigMap.class, "configmap"); + assertThat(cm).isNotNull(); + assertThat(cm.getMetadata().getLabels()) + .containsEntry("test-glue", "true"); + }); + + delete(glue); + await().untilAsserted(() -> { + var cm = get(ConfigMap.class, "configmap"); + assertThat(cm).isNull(); + }); + } + + public static class GlueResourceLabelSelectorTestProfile implements QuarkusTestProfile { + @Override + public Map getConfigOverrides() { + return Map.of("glue.operator.resource-label-selector.v1#ConfigMap", "test-glue=true"); + } + } +} diff --git a/src/test/resources/glue/SimpleGlueWithLabeledResource.yaml b/src/test/resources/glue/SimpleGlueWithLabeledResource.yaml new file mode 100644 index 0000000..867fafa --- /dev/null +++ b/src/test/resources/glue/SimpleGlueWithLabeledResource.yaml @@ -0,0 +1,17 @@ +# Invalid GLUE, presents resources with non-unique name +apiVersion: io.csviri.operator.glue/v1beta1 +kind: Glue +metadata: + name: glue +spec: + childResources: + - name: configMap + resource: + apiVersion: v1 + kind: ConfigMap + metadata: + name: configmap + labels: + test-glue: true + data: + key: "value1"