diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/ReconcilerUtils.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/ReconcilerUtils.java index ea7c58acfb..a2d3d72e5f 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/ReconcilerUtils.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/ReconcilerUtils.java @@ -28,6 +28,8 @@ public class ReconcilerUtils { protected static final String MISSING_GROUP_SUFFIX = ".javaoperatorsdk.io"; private static final String GET_SPEC = "getSpec"; private static final String SET_SPEC = "setSpec"; + private static final String SET_STATUS = "setStatus"; + private static final String GET_STATUS = "getStatus"; private static final Pattern API_URI_PATTERN = Pattern.compile(".*http(s?)://[^/]*/api(s?)/(\\S*).*"); // NOSONAR: input is controlled @@ -135,11 +137,23 @@ public static Object getSpec(HasMetadata resource) { return cr.getSpec(); } + return getSpecOrStatus(resource, GET_SPEC); + } + + public static Object getStatus(HasMetadata resource) { + // optimize CustomResource case + if (resource instanceof CustomResource cr) { + return cr.getStatus(); + } + return getSpecOrStatus(resource, GET_STATUS); + } + + private static Object getSpecOrStatus(HasMetadata resource, String getMethod) { try { - Method getSpecMethod = resource.getClass().getMethod(GET_SPEC); + Method getSpecMethod = resource.getClass().getMethod(getMethod); return getSpecMethod.invoke(resource); } catch (NoSuchMethodException | InvocationTargetException | IllegalAccessException e) { - throw noSpecException(resource, e); + throw noMethodException(resource, e, getMethod); } } @@ -151,31 +165,46 @@ public static Object setSpec(HasMetadata resource, Object spec) { return null; } + return setSpecOrStatus(resource, spec, SET_SPEC); + } + + @SuppressWarnings("unchecked") + public static Object setStatus(HasMetadata resource, Object status) { + // optimize CustomResource case + if (resource instanceof CustomResource cr) { + cr.setStatus(status); + return null; + } + return setSpecOrStatus(resource, status, SET_STATUS); + } + + private static Object setSpecOrStatus( + HasMetadata resource, Object spec, String setterMethodName) { try { Class resourceClass = resource.getClass(); // if given spec is null, find the method just using its name - Method setSpecMethod; + Method setMethod; if (spec != null) { - setSpecMethod = resourceClass.getMethod(SET_SPEC, spec.getClass()); + setMethod = resourceClass.getMethod(setterMethodName, spec.getClass()); } else { - setSpecMethod = + setMethod = Arrays.stream(resourceClass.getMethods()) - .filter(method -> SET_SPEC.equals(method.getName())) + .filter(method -> setterMethodName.equals(method.getName())) .findFirst() - .orElseThrow(() -> noSpecException(resource, null)); + .orElseThrow(() -> noMethodException(resource, null, setterMethodName)); } - return setSpecMethod.invoke(resource, spec); + return setMethod.invoke(resource, spec); } catch (NoSuchMethodException | InvocationTargetException | IllegalAccessException e) { - throw noSpecException(resource, e); + throw noMethodException(resource, e, setterMethodName); } } - private static IllegalStateException noSpecException( - HasMetadata resource, ReflectiveOperationException e) { + private static IllegalStateException noMethodException( + HasMetadata resource, ReflectiveOperationException e, String methodName) { return new IllegalStateException( - "No spec found on resource " + resource.getClass().getName(), e); + "No method: " + methodName + " found on resource " + resource.getClass().getName(), e); } public static T loadYaml(Class clazz, Class loader, String yaml) { 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 41d7a4f493..8e0293f3b4 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 @@ -16,6 +16,7 @@ import io.fabric8.kubernetes.client.dsl.base.PatchContext; import io.fabric8.kubernetes.client.dsl.base.PatchType; import io.javaoperatorsdk.operator.OperatorException; +import io.javaoperatorsdk.operator.ReconcilerUtils; import io.javaoperatorsdk.operator.api.config.Cloner; import io.javaoperatorsdk.operator.api.config.ControllerConfiguration; import io.javaoperatorsdk.operator.api.reconciler.BaseControl; @@ -477,6 +478,7 @@ public R patchStatus(R resource, R originalResource) { } } + @SuppressWarnings("unchecked") private R editStatus(R resource, R originalResource) { String resourceVersion = resource.getMetadata().getResourceVersion(); // the cached resource should not be changed in any circumstances @@ -486,7 +488,11 @@ private R editStatus(R resource, R originalResource) { clonedOriginal.getMetadata().setResourceVersion(null); resource.getMetadata().setResourceVersion(null); var res = resource(clonedOriginal); - return res.editStatus(r -> resource); + return res.editStatus( + r -> { + ReconcilerUtils.setStatus(r, ReconcilerUtils.getStatus(resource)); + return r; + }); } finally { // restore initial resource version clonedOriginal.getMetadata().setResourceVersion(resourceVersion); diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/ReconcilerUtilsTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/ReconcilerUtilsTest.java index abc83b94ff..ad77196068 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/ReconcilerUtilsTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/ReconcilerUtilsTest.java @@ -8,6 +8,7 @@ import io.fabric8.kubernetes.api.model.apps.Deployment; import io.fabric8.kubernetes.api.model.apps.DeploymentBuilder; import io.fabric8.kubernetes.api.model.apps.DeploymentSpec; +import io.fabric8.kubernetes.api.model.apps.DeploymentStatus; import io.fabric8.kubernetes.api.model.rbac.ClusterRole; import io.fabric8.kubernetes.api.model.rbac.ClusterRoleBuilder; import io.fabric8.kubernetes.client.CustomResource; @@ -116,6 +117,29 @@ void setsSpecCustomResourceWithReflection() { assertThat(tomcat.getSpec().getReplicas()).isEqualTo(1); } + @Test + void setsStatusWithReflection() { + Deployment deployment = new Deployment(); + DeploymentStatus status = new DeploymentStatus(); + status.setReplicas(2); + + ReconcilerUtils.setStatus(deployment, status); + + assertThat(deployment.getStatus().getReplicas()).isEqualTo(2); + } + + @Test + void getsStatusWithReflection() { + Deployment deployment = new Deployment(); + DeploymentStatus status = new DeploymentStatus(); + status.setReplicas(2); + deployment.setStatus(status); + + var res = ReconcilerUtils.getStatus(deployment); + + assertThat(((DeploymentStatus) res).getReplicas()).isEqualTo(2); + } + @Test void loadYamlAsBuilder() { DeploymentBuilder builder = diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/patchresourceandstatusnossa/PatchResourceAndStatusNoSSAIT.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/patchresourceandstatusnossa/PatchResourceAndStatusNoSSAIT.java index cd63c708e9..a835dd2de6 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/patchresourceandstatusnossa/PatchResourceAndStatusNoSSAIT.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/patchresourceandstatusnossa/PatchResourceAndStatusNoSSAIT.java @@ -1,5 +1,6 @@ package io.javaoperatorsdk.operator.baseapi.patchresourceandstatusnossa; +import java.util.Map; import java.util.concurrent.TimeUnit; import org.junit.jupiter.api.Test; @@ -9,12 +10,14 @@ import io.javaoperatorsdk.operator.junit.LocallyRunOperatorExtension; import io.javaoperatorsdk.operator.support.TestUtils; +import static io.javaoperatorsdk.operator.baseapi.patchresourceandstatusnossa.PatchResourceAndStatusNoSSAReconciler.TEST_ANNOTATION; +import static io.javaoperatorsdk.operator.baseapi.patchresourceandstatusnossa.PatchResourceAndStatusNoSSAReconciler.TEST_ANNOTATION_VALUE; import static org.assertj.core.api.Assertions.assertThat; import static org.awaitility.Awaitility.await; class PatchResourceAndStatusNoSSAIT { @RegisterExtension - LocallyRunOperatorExtension operator = + LocallyRunOperatorExtension extension = LocallyRunOperatorExtension.builder() .withConfigurationService(o -> o.withUseSSAToPatchPrimaryResource(false)) .withReconciler(PatchResourceAndStatusNoSSAReconciler.class) @@ -22,26 +25,47 @@ class PatchResourceAndStatusNoSSAIT { @Test void updatesSubResourceStatus() { + extension + .getReconcilerOfType(PatchResourceAndStatusNoSSAReconciler.class) + .setRemoveAnnotation(false); PatchResourceAndStatusNoSSACustomResource resource = createTestCustomResource("1"); - operator.create(resource); + extension.create(resource); awaitStatusUpdated(resource.getMetadata().getName()); // wait for sure, there are no more events TestUtils.waitXms(300); PatchResourceAndStatusNoSSACustomResource customResource = - operator.get( + extension.get( PatchResourceAndStatusNoSSACustomResource.class, resource.getMetadata().getName()); - assertThat(TestUtils.getNumberOfExecutions(operator)).isEqualTo(1); + assertThat(TestUtils.getNumberOfExecutions(extension)).isEqualTo(1); assertThat(customResource.getStatus().getState()) .isEqualTo(PatchResourceAndStatusNoSSAStatus.State.SUCCESS); - assertThat( - customResource - .getMetadata() - .getAnnotations() - .get(PatchResourceAndStatusNoSSAReconciler.TEST_ANNOTATION)) - .isNotNull(); + assertThat(customResource.getMetadata().getAnnotations().get(TEST_ANNOTATION)).isNotNull(); + } + + @Test + void removeAnnotationCorrectlyUpdatesStatus() { + extension + .getReconcilerOfType(PatchResourceAndStatusNoSSAReconciler.class) + .setRemoveAnnotation(true); + PatchResourceAndStatusNoSSACustomResource resource = createTestCustomResource("1"); + resource.getMetadata().setAnnotations(Map.of(TEST_ANNOTATION, TEST_ANNOTATION_VALUE)); + extension.create(resource); + + awaitStatusUpdated(resource.getMetadata().getName()); + // wait for sure, there are no more events + TestUtils.waitXms(300); + + PatchResourceAndStatusNoSSACustomResource customResource = + extension.get( + PatchResourceAndStatusNoSSACustomResource.class, resource.getMetadata().getName()); + + assertThat(TestUtils.getNumberOfExecutions(extension)).isEqualTo(1); + assertThat(customResource.getStatus().getState()) + .isEqualTo(PatchResourceAndStatusNoSSAStatus.State.SUCCESS); + assertThat(customResource.getMetadata().getAnnotations().get(TEST_ANNOTATION)).isNull(); } void awaitStatusUpdated(String name) { @@ -50,7 +74,7 @@ void awaitStatusUpdated(String name) { .untilAsserted( () -> { PatchResourceAndStatusNoSSACustomResource cr = - operator.get(PatchResourceAndStatusNoSSACustomResource.class, name); + extension.get(PatchResourceAndStatusNoSSACustomResource.class, name); assertThat(cr).isNotNull(); assertThat(cr.getStatus()).isNotNull(); assertThat(cr.getStatus().getState()) diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/patchresourceandstatusnossa/PatchResourceAndStatusNoSSAReconciler.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/patchresourceandstatusnossa/PatchResourceAndStatusNoSSAReconciler.java index a104ca4185..2d3a282b01 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/patchresourceandstatusnossa/PatchResourceAndStatusNoSSAReconciler.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/patchresourceandstatusnossa/PatchResourceAndStatusNoSSAReconciler.java @@ -22,6 +22,8 @@ public class PatchResourceAndStatusNoSSAReconciler public static final String TEST_ANNOTATION_VALUE = "TestAnnotationValue"; private final AtomicInteger numberOfExecutions = new AtomicInteger(0); + private volatile boolean removeAnnotation = false; + @Override public UpdateControl reconcile( PatchResourceAndStatusNoSSACustomResource resource, @@ -30,8 +32,12 @@ public UpdateControl reconcile( log.info("Value: " + resource.getSpec().getValue()); - resource.getMetadata().setAnnotations(new HashMap<>()); - resource.getMetadata().getAnnotations().put(TEST_ANNOTATION, TEST_ANNOTATION_VALUE); + if (removeAnnotation) { + resource.getMetadata().getAnnotations().remove(TEST_ANNOTATION); + } else { + resource.getMetadata().setAnnotations(new HashMap<>()); + resource.getMetadata().getAnnotations().put(TEST_ANNOTATION, TEST_ANNOTATION_VALUE); + } ensureStatusExists(resource); resource.getStatus().setState(PatchResourceAndStatusNoSSAStatus.State.SUCCESS); @@ -49,4 +55,8 @@ private void ensureStatusExists(PatchResourceAndStatusNoSSACustomResource resour public int getNumberOfExecutions() { return numberOfExecutions.get(); } + + public void setRemoveAnnotation(boolean removeAnnotation) { + this.removeAnnotation = removeAnnotation; + } }