From 8c341ae932a3b8c22aba73b2a6af6757daa1df58 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Attila=20M=C3=A9sz=C3=A1ros?= Date: Wed, 19 Oct 2022 12:45:08 +0200 Subject: [PATCH] fix: issue with cluster scoped resource (#1549) --- .../event/ReconciliationDispatcher.java | 41 +++++++----- .../junit/LocallyRunOperatorExtension.java | 7 +- .../operator/ClusterScopedResourceIT.java | 66 +++++++++++++++++++ .../ClusterScopedCustomResource.java | 15 +++++ ...ClusterScopedCustomResourceReconciler.java | 66 +++++++++++++++++++ .../ClusterScopedCustomResourceSpec.java | 25 +++++++ .../ClusterScopedCustomResourceStatus.java | 15 +++++ 7 files changed, 217 insertions(+), 18 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 d2dc247faf..6d8a612366 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 @@ -9,9 +9,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.NonNamespaceOperation; import io.fabric8.kubernetes.client.dsl.Resource; import io.fabric8.kubernetes.client.dsl.internal.HasMetadataOperationsImpl; import io.fabric8.kubernetes.client.utils.Serialization; @@ -170,7 +172,7 @@ private PostExecutionControl

handleErrorStatusHandler(P resource, P originalR Exception e) throws Exception { if (isErrorStatusHandlerPresent()) { try { - RetryInfo retryInfo = context.getRetryInfo().orElse(new RetryInfo() { + RetryInfo retryInfo = context.getRetryInfo().orElseGet(() -> new RetryInfo() { @Override public int getAttemptCount() { return 0; @@ -362,28 +364,28 @@ 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) { - log.debug( - "Trying to replace resource {}, version: {}", - getName(resource), - resource.getMetadata().getResourceVersion()); - return resourceOperation - .inNamespace(resource.getMetadata().getNamespace()) - .withName(getName(resource)) - .lockResourceVersion(resource.getMetadata().getResourceVersion()) - .replace(resource); + final var resourceVersion = resource.getMetadata().getResourceVersion(); + log.debug("Trying to replace resource {}, version: {}", getName(resource), resourceVersion); + + return resource(resource).withName(resource.getMetadata().getName()) + .lockResourceVersion(resourceVersion).replace(resource); } @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()); + HasMetadataOperationsImpl hasMetadataOperation = + (HasMetadataOperationsImpl) resource(resource) + .withName(getName(resource)) + .lockResourceVersion(resource.getMetadata().getResourceVersion()); return (R) hasMetadataOperation.replaceStatus(resource); } @@ -395,8 +397,7 @@ public R patchStatus(R resource, R originalResource) { resource.getMetadata().setResourceVersion(null); try (var bis = new ByteArrayInputStream( Serialization.asJson(originalResource).getBytes(StandardCharsets.UTF_8))) { - return resourceOperation - .inNamespace(resource.getMetadata().getNamespace()) + return resource(originalResource) // will be simplified in fabric8 v6 .load(bis) .editStatus(r -> resource); @@ -408,5 +409,11 @@ public R patchStatus(R resource, R originalResource) { resource.getMetadata().setResourceVersion(resourceVersion); } } + + private NonNamespaceOperation, Resource> resource(R resource) { + return resource instanceof Namespaced + ? resourceOperation.inNamespace(resource.getMetadata().getNamespace()) + : resourceOperation; + } } } 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 656d160acf..1a0905dfd3 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.LocalPortForward; import io.javaoperatorsdk.operator.Operator; @@ -124,7 +125,11 @@ protected void before(ExtensionContext context) { final var configurationService = ConfigurationServiceProvider.instance(); 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..f1c15f2dec --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/ClusterScopedResourceIT.java @@ -0,0 +1,66 @@ +package io.javaoperatorsdk.operator; + +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; +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"; + public static final String INITIAL_DATA = "initialData"; + public static final String UPDATED_DATA = "updatedData"; + @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(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); + }); + + 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()); + } + + + ClusterScopedCustomResource testResource() { + var res = new ClusterScopedCustomResource(); + res.setMetadata(new ObjectMetaBuilder() + .withName(TEST_NAME) + .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/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..b5e135262d --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/clusterscopedresource/ClusterScopedCustomResourceReconciler.java @@ -0,0 +1,66 @@ +package io.javaoperatorsdk.operator.sample.clusterscopedresource; + +import java.util.Map; + +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.fabric8.kubernetes.client.dsl.Resource; +import io.javaoperatorsdk.operator.api.reconciler.*; +import io.javaoperatorsdk.operator.junit.KubernetesClientAware; + +@ControllerConfiguration +public class ClusterScopedCustomResourceReconciler + implements Reconciler, Cleaner, + KubernetesClientAware { + + public static final String DATA_KEY = "data-key"; + + private KubernetesClient client; + + @Override + public UpdateControl reconcile( + ClusterScopedCustomResource resource, Context context) { + + final var desired = desired(resource); + getConfigMapResource(desired).createOrReplace(desired); + + resource.setStatus(new ClusterScopedCustomResourceStatus()); + resource.getStatus().setCreated(true); + return UpdateControl.patchStatus(resource); + } + + private Resource getConfigMapResource(ConfigMap desired) { + return client.configMaps().inNamespace(desired.getMetadata().getNamespace()) + .withName(desired.getMetadata().getName()); + } + + private ConfigMap desired(ClusterScopedCustomResource resource) { + return new ConfigMapBuilder() + .withMetadata(new ObjectMetaBuilder() + .withName(resource.getMetadata().getName()) + .withNamespace(resource.getSpec().getTargetNamespace()) + .build()) + .withData(Map.of(DATA_KEY, resource.getSpec().getData())) + .build(); + } + + @Override + public KubernetesClient getKubernetesClient() { + return client; + } + + @Override + public void setKubernetesClient(KubernetesClient kubernetesClient) { + this.client = kubernetesClient; + } + + @Override + public DeleteControl cleanup(ClusterScopedCustomResource resource, + Context context) { + final var desired = desired(resource); + getConfigMapResource(desired).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..825b0c443e --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/clusterscopedresource/ClusterScopedCustomResourceSpec.java @@ -0,0 +1,25 @@ +package io.javaoperatorsdk.operator.sample.clusterscopedresource; + +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; + } + + 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; + } +}