From c056a77e10261bc8b173de185ffe31cf0e29439a Mon Sep 17 00:00:00 2001 From: csviri Date: Wed, 19 Oct 2022 11:32:49 +0200 Subject: [PATCH 1/3] fix: issue with cluster scoped resource --- .../event/ReconciliationDispatcher.java | 32 ++++++---- .../junit/LocallyRunOperatorExtension.java | 7 +- .../operator/ClusterScopedResourceIT.java | 55 ++++++++++++++++ .../ClusterScopedCustomResource.java | 15 +++++ ...ClusterScopedCustomResourceReconciler.java | 64 +++++++++++++++++++ .../ClusterScopedCustomResourceSpec.java | 15 +++++ .../ClusterScopedCustomResourceStatus.java | 15 +++++ 7 files changed, 188 insertions(+), 15 deletions(-) create mode 100644 operator-framework/src/test/java/io/javaoperatorsdk/operator/ClusterScopedResourceIT.java create mode 100644 operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/clusterscopedresource/ClusterScopedCustomResource.java create mode 100644 operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/clusterscopedresource/ClusterScopedCustomResourceReconciler.java create mode 100644 operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/clusterscopedresource/ClusterScopedCustomResourceSpec.java create mode 100644 operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/clusterscopedresource/ClusterScopedCustomResourceStatus.java diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/ReconciliationDispatcher.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/ReconciliationDispatcher.java index 3db1b3c92a..0239d2cbcc 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/ReconciliationDispatcher.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/ReconciliationDispatcher.java @@ -5,11 +5,11 @@ import io.fabric8.kubernetes.api.model.HasMetadata; import io.fabric8.kubernetes.api.model.KubernetesResourceList; +import io.fabric8.kubernetes.api.model.Namespaced; import io.fabric8.kubernetes.client.CustomResource; import io.fabric8.kubernetes.client.KubernetesClientException; import io.fabric8.kubernetes.client.dsl.MixedOperation; import io.fabric8.kubernetes.client.dsl.Resource; -import io.fabric8.kubernetes.client.dsl.internal.HasMetadataOperationsImpl; import io.javaoperatorsdk.operator.OperatorException; import io.javaoperatorsdk.operator.api.ObservedGenerationAware; import io.javaoperatorsdk.operator.api.config.ConfigurationServiceProvider; @@ -355,7 +355,11 @@ public CustomResourceFacade( } public R getResource(String namespace, String name) { - return resourceOperation.inNamespace(namespace).withName(name).get(); + if (namespace != null) { + return resourceOperation.inNamespace(namespace).withName(name).get(); + } else { + return resourceOperation.withName(name).get(); + } } public R updateResource(R resource) { @@ -363,21 +367,17 @@ public R updateResource(R resource) { "Trying to replace resource {}, version: {}", getName(resource), resource.getMetadata().getResourceVersion()); - return resourceOperation - .inNamespace(resource.getMetadata().getNamespace()) - .resource(resource) - .lockResourceVersion(resource.getMetadata().getResourceVersion()) + + return resource(resource).lockResourceVersion(resource.getMetadata().getResourceVersion()) .replace(); } @SuppressWarnings({"rawtypes", "unchecked"}) public R updateStatus(R resource) { log.trace("Updating status for resource: {}", resource); - HasMetadataOperationsImpl hasMetadataOperation = (HasMetadataOperationsImpl) resourceOperation - .inNamespace(resource.getMetadata().getNamespace()) - .withName(getName(resource)) - .lockResourceVersion(resource.getMetadata().getResourceVersion()); - return (R) hasMetadataOperation.replaceStatus(resource); + return resource(resource) + .lockResourceVersion() + .replaceStatus(); } public R patchStatus(R resource, R originalResource) { @@ -387,9 +387,7 @@ public R patchStatus(R resource, R originalResource) { originalResource.getMetadata().setResourceVersion(null); resource.getMetadata().setResourceVersion(null); try { - return resourceOperation - .inNamespace(resource.getMetadata().getNamespace()) - .resource(originalResource) + return resource(originalResource) .editStatus(r -> resource); } finally { // restore initial resource version @@ -397,5 +395,11 @@ public R patchStatus(R resource, R originalResource) { resource.getMetadata().setResourceVersion(resourceVersion); } } + + private Resource resource(R resource) { + return resource instanceof Namespaced ? resourceOperation + .inNamespace(resource.getMetadata().getNamespace()) + .resource(resource) : resourceOperation.resource(resource); + } } } diff --git a/operator-framework-junit5/src/main/java/io/javaoperatorsdk/operator/junit/LocallyRunOperatorExtension.java b/operator-framework-junit5/src/main/java/io/javaoperatorsdk/operator/junit/LocallyRunOperatorExtension.java index 57201e5146..a4c04ab938 100644 --- a/operator-framework-junit5/src/main/java/io/javaoperatorsdk/operator/junit/LocallyRunOperatorExtension.java +++ b/operator-framework-junit5/src/main/java/io/javaoperatorsdk/operator/junit/LocallyRunOperatorExtension.java @@ -15,6 +15,7 @@ import org.slf4j.LoggerFactory; import io.fabric8.kubernetes.api.model.HasMetadata; +import io.fabric8.kubernetes.api.model.Namespaced; import io.fabric8.kubernetes.client.CustomResource; import io.fabric8.kubernetes.client.KubernetesClient; import io.fabric8.kubernetes.client.LocalPortForward; @@ -128,7 +129,11 @@ protected void before(ExtensionContext context) { for (var ref : reconcilers) { final var config = configurationService.getConfigurationFor(ref.reconciler); - final var oconfig = override(config).settingNamespace(namespace); + final var oconfig = override(config); + + if (Namespaced.class.isAssignableFrom(config.getResourceClass())) { + oconfig.settingNamespace(namespace); + } if (ref.retry != null) { oconfig.withRetry(ref.retry); diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/ClusterScopedResourceIT.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/ClusterScopedResourceIT.java new file mode 100644 index 0000000000..9ab9408af9 --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/ClusterScopedResourceIT.java @@ -0,0 +1,55 @@ +package io.javaoperatorsdk.operator; + +import io.fabric8.kubernetes.api.model.ConfigMap; +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 io.javaoperatorsdk.operator.sample.clusterscopedresource.ClusterScopedCustomResource; +import io.javaoperatorsdk.operator.sample.clusterscopedresource.ClusterScopedCustomResourceReconciler; +import io.javaoperatorsdk.operator.sample.clusterscopedresource.ClusterScopedCustomResourceSpec; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.awaitility.Awaitility.await; + +class ClusterScopedResourceIT { + + public static final String TEST_NAME = "test1"; + @RegisterExtension + LocallyRunOperatorExtension operator = + LocallyRunOperatorExtension.builder() + .withReconciler(new ClusterScopedCustomResourceReconciler()).build(); + + @Test + void crudOperationOnClusterScopedCustomResource() { + var resource = operator.create(testResource()); + + await().untilAsserted(() -> { + var res = operator.get(ClusterScopedCustomResource.class, TEST_NAME); + assertThat(operator.getReconcilerOfType(ClusterScopedCustomResourceReconciler.class) + .getNumberOfExecutions()).isGreaterThan(0); + assertThat(res.getStatus()).isNotNull(); + assertThat(res.getStatus().getCreated()).isTrue(); + var cm = operator.get(ConfigMap.class,TEST_NAME); + assertThat(cm).isNotNull(); + }); + + operator.delete(resource); + + await().untilAsserted(()-> assertThat(operator.get(ConfigMap.class,TEST_NAME)).isNull()); + } + + + ClusterScopedCustomResource testResource() { + var res = new ClusterScopedCustomResource(); + res.setMetadata(new ObjectMetaBuilder() + .withName(TEST_NAME) + .build()); + res.setSpec(new ClusterScopedCustomResourceSpec()); + res.getSpec().setTargetNamespace(operator.getNamespace()); + + return res; + } + +} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/clusterscopedresource/ClusterScopedCustomResource.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/clusterscopedresource/ClusterScopedCustomResource.java new file mode 100644 index 0000000000..957b396df4 --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/clusterscopedresource/ClusterScopedCustomResource.java @@ -0,0 +1,15 @@ +package io.javaoperatorsdk.operator.sample.clusterscopedresource; + +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("csc") +public class ClusterScopedCustomResource + extends CustomResource { + + +} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/clusterscopedresource/ClusterScopedCustomResourceReconciler.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/clusterscopedresource/ClusterScopedCustomResourceReconciler.java new file mode 100644 index 0000000000..3513c2f283 --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/clusterscopedresource/ClusterScopedCustomResourceReconciler.java @@ -0,0 +1,64 @@ +package io.javaoperatorsdk.operator.sample.clusterscopedresource; + +import java.util.concurrent.atomic.AtomicInteger; + +import io.fabric8.kubernetes.api.model.ConfigMap; +import io.fabric8.kubernetes.api.model.ConfigMapBuilder; +import io.fabric8.kubernetes.api.model.ObjectMetaBuilder; +import io.fabric8.kubernetes.client.KubernetesClient; +import io.javaoperatorsdk.operator.api.reconciler.*; +import io.javaoperatorsdk.operator.junit.KubernetesClientAware; +import io.javaoperatorsdk.operator.support.TestExecutionInfoProvider; + +@ControllerConfiguration +public class ClusterScopedCustomResourceReconciler + implements Reconciler, Cleaner, + TestExecutionInfoProvider, + KubernetesClientAware { + + private final AtomicInteger numberOfExecutions = new AtomicInteger(0); + + private KubernetesClient client; + + @Override + public UpdateControl reconcile( + ClusterScopedCustomResource resource, Context context) { + numberOfExecutions.addAndGet(1); + + client.configMaps().resource(desired(resource)).createOrReplace(); + + resource.setStatus(new ClusterScopedCustomResourceStatus()); + resource.getStatus().setCreated(true); + return UpdateControl.patchStatus(resource); + } + + private ConfigMap desired(ClusterScopedCustomResource resource) { + return new ConfigMapBuilder() + .withMetadata(new ObjectMetaBuilder() + .withName(resource.getMetadata().getName()) + .withNamespace(resource.getSpec().getTargetNamespace()) + .build()) + .build(); + } + + public int getNumberOfExecutions() { + return numberOfExecutions.get(); + } + + @Override + public KubernetesClient getKubernetesClient() { + return client; + } + + @Override + public void setKubernetesClient(KubernetesClient kubernetesClient) { + this.client = kubernetesClient; + } + + @Override + public DeleteControl cleanup(ClusterScopedCustomResource resource, + Context context) { + client.configMaps().resource(desired(resource)).delete(); + return DeleteControl.defaultDelete(); + } +} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/clusterscopedresource/ClusterScopedCustomResourceSpec.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/clusterscopedresource/ClusterScopedCustomResourceSpec.java new file mode 100644 index 0000000000..7a15a20d6d --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/clusterscopedresource/ClusterScopedCustomResourceSpec.java @@ -0,0 +1,15 @@ +package io.javaoperatorsdk.operator.sample.clusterscopedresource; + +public class ClusterScopedCustomResourceSpec { + + private String targetNamespace; + + public String getTargetNamespace() { + return targetNamespace; + } + + public ClusterScopedCustomResourceSpec setTargetNamespace(String targetNamespace) { + this.targetNamespace = targetNamespace; + return this; + } +} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/clusterscopedresource/ClusterScopedCustomResourceStatus.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/clusterscopedresource/ClusterScopedCustomResourceStatus.java new file mode 100644 index 0000000000..7c4d49f3a1 --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/clusterscopedresource/ClusterScopedCustomResourceStatus.java @@ -0,0 +1,15 @@ +package io.javaoperatorsdk.operator.sample.clusterscopedresource; + +public class ClusterScopedCustomResourceStatus { + + private Boolean created; + + public Boolean getCreated() { + return created; + } + + public ClusterScopedCustomResourceStatus setCreated(Boolean created) { + this.created = created; + return this; + } +} From fa2bfea4a18e74af7645004dea7e493758323551 Mon Sep 17 00:00:00 2001 From: csviri Date: Wed, 19 Oct 2022 11:36:26 +0200 Subject: [PATCH 2/3] format --- .../javaoperatorsdk/operator/ClusterScopedResourceIT.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/ClusterScopedResourceIT.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/ClusterScopedResourceIT.java index 9ab9408af9..78f0138f45 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/ClusterScopedResourceIT.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/ClusterScopedResourceIT.java @@ -1,9 +1,9 @@ package io.javaoperatorsdk.operator; -import io.fabric8.kubernetes.api.model.ConfigMap; 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.clusterscopedresource.ClusterScopedCustomResource; @@ -31,13 +31,13 @@ void crudOperationOnClusterScopedCustomResource() { .getNumberOfExecutions()).isGreaterThan(0); assertThat(res.getStatus()).isNotNull(); assertThat(res.getStatus().getCreated()).isTrue(); - var cm = operator.get(ConfigMap.class,TEST_NAME); + var cm = operator.get(ConfigMap.class, TEST_NAME); assertThat(cm).isNotNull(); }); operator.delete(resource); - await().untilAsserted(()-> assertThat(operator.get(ConfigMap.class,TEST_NAME)).isNull()); + await().untilAsserted(() -> assertThat(operator.get(ConfigMap.class, TEST_NAME)).isNull()); } From 98891b3f799443ecffcda0dfb790e66b0ad4605c Mon Sep 17 00:00:00 2001 From: csviri Date: Wed, 19 Oct 2022 11:42:53 +0200 Subject: [PATCH 3/3] improved test --- .../operator/ClusterScopedResourceIT.java | 17 ++++++++++++++--- .../ClusterScopedCustomResourceReconciler.java | 12 +++--------- .../ClusterScopedCustomResourceSpec.java | 10 ++++++++++ 3 files changed, 27 insertions(+), 12 deletions(-) diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/ClusterScopedResourceIT.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/ClusterScopedResourceIT.java index 78f0138f45..f1c15f2dec 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/ClusterScopedResourceIT.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/ClusterScopedResourceIT.java @@ -16,6 +16,8 @@ class ClusterScopedResourceIT { public static final String TEST_NAME = "test1"; + public static final String INITIAL_DATA = "initialData"; + public static final String UPDATED_DATA = "updatedData"; @RegisterExtension LocallyRunOperatorExtension operator = LocallyRunOperatorExtension.builder() @@ -27,16 +29,24 @@ void crudOperationOnClusterScopedCustomResource() { await().untilAsserted(() -> { var res = operator.get(ClusterScopedCustomResource.class, TEST_NAME); - assertThat(operator.getReconcilerOfType(ClusterScopedCustomResourceReconciler.class) - .getNumberOfExecutions()).isGreaterThan(0); assertThat(res.getStatus()).isNotNull(); assertThat(res.getStatus().getCreated()).isTrue(); var cm = operator.get(ConfigMap.class, TEST_NAME); assertThat(cm).isNotNull(); + assertThat(cm.getData().get(ClusterScopedCustomResourceReconciler.DATA_KEY)) + .isEqualTo(INITIAL_DATA); }); - operator.delete(resource); + resource.getSpec().setData(UPDATED_DATA); + operator.replace(resource); + await().untilAsserted(() -> { + var cm = operator.get(ConfigMap.class, TEST_NAME); + assertThat(cm).isNotNull(); + assertThat(cm.getData().get(ClusterScopedCustomResourceReconciler.DATA_KEY)) + .isEqualTo(UPDATED_DATA); + }); + operator.delete(resource); await().untilAsserted(() -> assertThat(operator.get(ConfigMap.class, TEST_NAME)).isNull()); } @@ -48,6 +58,7 @@ ClusterScopedCustomResource testResource() { .build()); res.setSpec(new ClusterScopedCustomResourceSpec()); res.getSpec().setTargetNamespace(operator.getNamespace()); + res.getSpec().setData(INITIAL_DATA); return res; } diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/clusterscopedresource/ClusterScopedCustomResourceReconciler.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/clusterscopedresource/ClusterScopedCustomResourceReconciler.java index 3513c2f283..edbf22a69f 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/clusterscopedresource/ClusterScopedCustomResourceReconciler.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/clusterscopedresource/ClusterScopedCustomResourceReconciler.java @@ -1,6 +1,6 @@ package io.javaoperatorsdk.operator.sample.clusterscopedresource; -import java.util.concurrent.atomic.AtomicInteger; +import java.util.Map; import io.fabric8.kubernetes.api.model.ConfigMap; import io.fabric8.kubernetes.api.model.ConfigMapBuilder; @@ -8,22 +8,19 @@ import io.fabric8.kubernetes.client.KubernetesClient; import io.javaoperatorsdk.operator.api.reconciler.*; import io.javaoperatorsdk.operator.junit.KubernetesClientAware; -import io.javaoperatorsdk.operator.support.TestExecutionInfoProvider; @ControllerConfiguration public class ClusterScopedCustomResourceReconciler implements Reconciler, Cleaner, - TestExecutionInfoProvider, KubernetesClientAware { - private final AtomicInteger numberOfExecutions = new AtomicInteger(0); + public static final String DATA_KEY = "data-key"; private KubernetesClient client; @Override public UpdateControl reconcile( ClusterScopedCustomResource resource, Context context) { - numberOfExecutions.addAndGet(1); client.configMaps().resource(desired(resource)).createOrReplace(); @@ -38,13 +35,10 @@ private ConfigMap desired(ClusterScopedCustomResource resource) { .withName(resource.getMetadata().getName()) .withNamespace(resource.getSpec().getTargetNamespace()) .build()) + .withData(Map.of(DATA_KEY, resource.getSpec().getData())) .build(); } - public int getNumberOfExecutions() { - return numberOfExecutions.get(); - } - @Override public KubernetesClient getKubernetesClient() { return client; diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/clusterscopedresource/ClusterScopedCustomResourceSpec.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/clusterscopedresource/ClusterScopedCustomResourceSpec.java index 7a15a20d6d..825b0c443e 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/clusterscopedresource/ClusterScopedCustomResourceSpec.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/clusterscopedresource/ClusterScopedCustomResourceSpec.java @@ -2,8 +2,18 @@ public class ClusterScopedCustomResourceSpec { + private String data; private String targetNamespace; + public String getData() { + return data; + } + + public ClusterScopedCustomResourceSpec setData(String data) { + this.data = data; + return this; + } + public String getTargetNamespace() { return targetNamespace; }