From 1a582b0c5e9d56b3215b4a679d862562c27e60a7 Mon Sep 17 00:00:00 2001 From: csviri Date: Wed, 27 Oct 2021 10:49:57 +0200 Subject: [PATCH 1/8] feat: observedGeneration support --- .../operator/api/ObservedGenerationAware.java | 15 +++++++++++++ .../api/ObservedGenerationAwareStatus.java | 21 +++++++++++++++++++ .../internal/CustomResourceEventFilters.java | 17 ++++++++++++--- 3 files changed, 50 insertions(+), 3 deletions(-) create mode 100644 operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/ObservedGenerationAware.java create mode 100644 operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/ObservedGenerationAwareStatus.java diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/ObservedGenerationAware.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/ObservedGenerationAware.java new file mode 100644 index 0000000000..a68850c606 --- /dev/null +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/ObservedGenerationAware.java @@ -0,0 +1,15 @@ +package io.javaoperatorsdk.operator.api; + +import java.util.Optional; + +/** + * If the custom resource's status object implements this interface the observed generation will be automatically + * handled. The last observed generation will be set to status when the status is updated or no update comes from + * controller. In addition to that will be checked if the controller is generation aware. + */ +public interface ObservedGenerationAware { + + void setObservedGeneration(Long generation); + + Optional getObservedGeneration(); +} diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/ObservedGenerationAwareStatus.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/ObservedGenerationAwareStatus.java new file mode 100644 index 0000000000..d2e77286dc --- /dev/null +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/ObservedGenerationAwareStatus.java @@ -0,0 +1,21 @@ +package io.javaoperatorsdk.operator.api; + +import java.util.Optional; + +/** + * A helper base class for status sub-resources classes to extend to support generate awareness. + */ +public class ObservedGenerationAwareStatus implements ObservedGenerationAware { + + private Long observedGeneration; + + @Override + public void setObservedGeneration(Long generation) { + this.observedGeneration = generation; + } + + @Override + public Optional getObservedGeneration() { + return Optional.of(observedGeneration); + } +} diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/internal/CustomResourceEventFilters.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/internal/CustomResourceEventFilters.java index 2f3bc72327..6ff2762c33 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/internal/CustomResourceEventFilters.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/internal/CustomResourceEventFilters.java @@ -1,6 +1,7 @@ package io.javaoperatorsdk.operator.processing.event.internal; import io.fabric8.kubernetes.client.CustomResource; +import io.javaoperatorsdk.operator.api.ObservedGenerationAware; /** * Convenience implementations of, and utility methods for, {@link CustomResourceEventFilter}. @@ -21,9 +22,19 @@ public final class CustomResourceEventFilters { }; private static final CustomResourceEventFilter GENERATION_AWARE = - (configuration, oldResource, newResource) -> oldResource == null - || !configuration.isGenerationAware() - || oldResource.getMetadata().getGeneration() < newResource.getMetadata().getGeneration(); + (configuration, oldResource, newResource) -> { + if (configuration.isGenerationAware() && newResource.getStatus() != null && + newResource.getStatus() instanceof ObservedGenerationAware) { + var actualGeneration = newResource.getMetadata().getGeneration(); + var observedGeneration = ((ObservedGenerationAware) newResource.getStatus()) + .getObservedGeneration(); + return observedGeneration.map(aLong -> actualGeneration > aLong).orElse(true); + } else { + return oldResource == null + || !configuration.isGenerationAware() + || oldResource.getMetadata().getGeneration() < newResource.getMetadata().getGeneration(); + } + }; private static final CustomResourceEventFilter PASSTHROUGH = (configuration, oldResource, newResource) -> true; From 7c8e359812009e371b42b4a1d657f0c026d87843 Mon Sep 17 00:00:00 2001 From: csviri Date: Wed, 27 Oct 2021 13:26:49 +0200 Subject: [PATCH 2/8] Basic Implementation --- .../operator/api/ObservedGenerationAware.java | 13 +++++---- .../api/ObservedGenerationAwareStatus.java | 18 ++++++------ .../operator/processing/EventDispatcher.java | 28 ++++++++++++++++--- .../internal/CustomResourceEventFilters.java | 9 +++--- 4 files changed, 46 insertions(+), 22 deletions(-) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/ObservedGenerationAware.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/ObservedGenerationAware.java index a68850c606..3435543c79 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/ObservedGenerationAware.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/ObservedGenerationAware.java @@ -3,13 +3,16 @@ import java.util.Optional; /** - * If the custom resource's status object implements this interface the observed generation will be automatically - * handled. The last observed generation will be set to status when the status is updated or no update comes from - * controller. In addition to that will be checked if the controller is generation aware. + * If the custom resource's status object implements this interface the observed generation will be + * automatically handled. The last observed generation will be set to status when the status is + * updated or no update comes from controller. In addition to that will be checked if the controller + * is generation aware. + * + * In order to work the status object returned by CustomResource.getStatus() should not be null. */ public interface ObservedGenerationAware { - void setObservedGeneration(Long generation); + void setObservedGeneration(Long generation); - Optional getObservedGeneration(); + Optional getObservedGeneration(); } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/ObservedGenerationAwareStatus.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/ObservedGenerationAwareStatus.java index d2e77286dc..843736242b 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/ObservedGenerationAwareStatus.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/ObservedGenerationAwareStatus.java @@ -7,15 +7,15 @@ */ public class ObservedGenerationAwareStatus implements ObservedGenerationAware { - private Long observedGeneration; + private Long observedGeneration; - @Override - public void setObservedGeneration(Long generation) { - this.observedGeneration = generation; - } + @Override + public void setObservedGeneration(Long generation) { + this.observedGeneration = generation; + } - @Override - public Optional getObservedGeneration() { - return Optional.of(observedGeneration); - } + @Override + public Optional getObservedGeneration() { + return Optional.ofNullable(observedGeneration); + } } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/EventDispatcher.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/EventDispatcher.java index dd8ae8dbae..21faf20b37 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/EventDispatcher.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/EventDispatcher.java @@ -117,18 +117,38 @@ private PostExecutionControl handleCreateOrUpdate( .getCustomResource() .getMetadata() .setResourceVersion(updatedCustomResource.getMetadata().getResourceVersion()); - updatedCustomResource = - customResourceFacade.updateStatus(updateControl.getCustomResource()); + updatedCustomResource = updateStatusGenerationAware(updateControl.getCustomResource()); } else if (updateControl.isUpdateStatusSubResource()) { - updatedCustomResource = - customResourceFacade.updateStatus(updateControl.getCustomResource()); + updatedCustomResource = updateStatusGenerationAware(updateControl.getCustomResource()); } else if (updateControl.isUpdateCustomResource()) { updatedCustomResource = updateCustomResource(updateControl.getCustomResource()); + } else if (configuration().isGenerationAware() && isStatusObservedGenerationAware(resource)) { + updatedCustomResource = updateCustomResource(resource); } return createPostExecutionControl(updatedCustomResource, updateControl); } } + private boolean isStatusObservedGenerationAware(R resource) { + return resource.getStatus() instanceof ObservedGenerationAware; + } + + private R updateStatusGenerationAware(R customResource) { + updateStatusObservedGenerationIfRequired(customResource); + return customResourceFacade.updateStatus(customResource); + } + + private void updateStatusObservedGenerationIfRequired(R customResource) { + if (controller.getConfiguration().isGenerationAware()) { + var status = customResource.getStatus(); + // Note that if status is null we won't update the observed generation. + if (status instanceof ObservedGenerationAware) { + ((ObservedGenerationAware) status) + .setObservedGeneration(customResource.getMetadata().getGeneration()); + } + } + } + private PostExecutionControl createPostExecutionControl(R updatedCustomResource, UpdateControl updateControl) { PostExecutionControl postExecutionControl; diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/internal/CustomResourceEventFilters.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/internal/CustomResourceEventFilters.java index 6ff2762c33..219215d801 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/internal/CustomResourceEventFilters.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/internal/CustomResourceEventFilters.java @@ -24,15 +24,16 @@ public final class CustomResourceEventFilters { private static final CustomResourceEventFilter GENERATION_AWARE = (configuration, oldResource, newResource) -> { if (configuration.isGenerationAware() && newResource.getStatus() != null && - newResource.getStatus() instanceof ObservedGenerationAware) { + newResource.getStatus() instanceof ObservedGenerationAware) { var actualGeneration = newResource.getMetadata().getGeneration(); var observedGeneration = ((ObservedGenerationAware) newResource.getStatus()) - .getObservedGeneration(); + .getObservedGeneration(); return observedGeneration.map(aLong -> actualGeneration > aLong).orElse(true); } else { return oldResource == null - || !configuration.isGenerationAware() - || oldResource.getMetadata().getGeneration() < newResource.getMetadata().getGeneration(); + || !configuration.isGenerationAware() + || oldResource.getMetadata().getGeneration() < newResource.getMetadata() + .getGeneration(); } }; From e6c9ba9eafc6ed8c958ec993717d4115f5f5106c Mon Sep 17 00:00:00 2001 From: csviri Date: Wed, 27 Oct 2021 14:29:04 +0200 Subject: [PATCH 3/8] New generation aware filter test --- .../CustomResourceEventFilterTest.java | 64 +++++++++++++++++-- .../ObservedGenCustomResource.java | 21 ++++++ .../observedgeneration/ObservedGenSpec.java | 21 ++++++ .../observedgeneration/ObservedGenStatus.java | 8 +++ 4 files changed, 109 insertions(+), 5 deletions(-) create mode 100644 operator-framework-core/src/test/java/io/javaoperatorsdk/operator/sample/observedgeneration/ObservedGenCustomResource.java create mode 100644 operator-framework-core/src/test/java/io/javaoperatorsdk/operator/sample/observedgeneration/ObservedGenSpec.java create mode 100644 operator-framework-core/src/test/java/io/javaoperatorsdk/operator/sample/observedgeneration/ObservedGenStatus.java diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/internal/CustomResourceEventFilterTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/internal/CustomResourceEventFilterTest.java index 3e9e413f1e..337bb5bde2 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/internal/CustomResourceEventFilterTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/internal/CustomResourceEventFilterTest.java @@ -3,6 +3,9 @@ import java.util.List; import java.util.Objects; +import io.fabric8.kubernetes.api.model.ObjectMeta; +import io.fabric8.kubernetes.client.CustomResource; +import io.javaoperatorsdk.operator.sample.observedgeneration.ObservedGenCustomResource; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -94,6 +97,31 @@ public void eventFilteredByCustomPredicateAndGenerationAware() { verify(eventHandler, times(1)).handleEvent(any()); } + @Test + public void observedGenerationFiltering() { + var config = new ObservedGenControllerConfig(FINALIZER,true,null); + when(config.getConfigurationService().getResourceCloner()) + .thenReturn(ConfigurationService.DEFAULT_CLONER); + + var controller = new ObservedGenConfiguredController(config); + var eventSource = new CustomResourceEventSource<>(controller); + eventSource.setEventHandler(eventHandler); + + ObservedGenCustomResource cr = new ObservedGenCustomResource(); + cr.setMetadata(new ObjectMeta()); + cr.getMetadata().setFinalizers(List.of(FINALIZER)); + cr.getMetadata().setGeneration(5L); + cr.getStatus().setObservedGeneration(5L); + + eventSource.eventReceived(ResourceAction.UPDATED, cr, null); + verify(eventHandler, times(0)).handleEvent(any()); + + cr.getMetadata().setGeneration(6L); + + eventSource.eventReceived(ResourceAction.UPDATED, cr, null); + verify(eventHandler, times(1)).handleEvent(any()); + } + @Test public void eventNotFilteredByCustomPredicateIfFinalizerIsRequired() { var config = new TestControllerConfig( @@ -124,11 +152,24 @@ public void eventNotFilteredByCustomPredicateIfFinalizerIsRequired() { verify(eventHandler, times(2)).handleEvent(any()); } - private static class TestControllerConfig extends - DefaultControllerConfiguration { - + private static class TestControllerConfig extends ControllerConfig { public TestControllerConfig(String finalizer, boolean generationAware, - CustomResourceEventFilter eventFilter) { + CustomResourceEventFilter eventFilter) { + super(finalizer, generationAware, eventFilter, TestCustomResource.class); + } + } + private static class ObservedGenControllerConfig extends ControllerConfig { + public ObservedGenControllerConfig(String finalizer, boolean generationAware, + CustomResourceEventFilter eventFilter) { + super(finalizer, generationAware, eventFilter, ObservedGenCustomResource.class); + } + } + + private static class ControllerConfig> extends + DefaultControllerConfiguration { + + public ControllerConfig(String finalizer, boolean generationAware, + CustomResourceEventFilter eventFilter, Class customResourceClass) { super( null, null, @@ -139,7 +180,7 @@ public TestControllerConfig(String finalizer, boolean generationAware, null, null, eventFilter, - TestCustomResource.class, + customResourceClass, mock(ConfigurationService.class)); when(getConfigurationService().getResourceCloner()) @@ -158,4 +199,17 @@ public MixedOperation { + + public ObservedGenConfiguredController(ControllerConfiguration configuration) { + super(null, configuration, null); + } + + @Override + public MixedOperation, + Resource> getCRClient() { + return mock(MixedOperation.class); + } + } } diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/sample/observedgeneration/ObservedGenCustomResource.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/sample/observedgeneration/ObservedGenCustomResource.java new file mode 100644 index 0000000000..74f58b795f --- /dev/null +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/sample/observedgeneration/ObservedGenCustomResource.java @@ -0,0 +1,21 @@ +package io.javaoperatorsdk.operator.sample.observedgeneration; + +import io.fabric8.kubernetes.client.CustomResource; +import io.fabric8.kubernetes.model.annotation.Group; +import io.fabric8.kubernetes.model.annotation.Version; + +@Group("sample.javaoperatorsdk.io") +@Version("v1") +public class ObservedGenCustomResource + extends CustomResource { + + @Override + protected ObservedGenSpec initSpec() { + return new ObservedGenSpec(); + } + + @Override + protected ObservedGenStatus initStatus() { + return new ObservedGenStatus(); + } +} diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/sample/observedgeneration/ObservedGenSpec.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/sample/observedgeneration/ObservedGenSpec.java new file mode 100644 index 0000000000..5cf25eb3b6 --- /dev/null +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/sample/observedgeneration/ObservedGenSpec.java @@ -0,0 +1,21 @@ +package io.javaoperatorsdk.operator.sample.observedgeneration; + +public class ObservedGenSpec { + + private String value; + + public String getValue() { + return value; + } + + public void setValue(String value) { + this.value = value; + } + + @Override + public String toString() { + return "TestCustomResourceSpec{" + + "value='" + value + '\'' + + '}'; + } +} diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/sample/observedgeneration/ObservedGenStatus.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/sample/observedgeneration/ObservedGenStatus.java new file mode 100644 index 0000000000..bf92d143f8 --- /dev/null +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/sample/observedgeneration/ObservedGenStatus.java @@ -0,0 +1,8 @@ +package io.javaoperatorsdk.operator.sample.observedgeneration; + +import io.javaoperatorsdk.operator.api.ObservedGenerationAware; +import io.javaoperatorsdk.operator.api.ObservedGenerationAwareStatus; + +public class ObservedGenStatus extends ObservedGenerationAwareStatus { + +} From be18b975cc2fda5946862c11a3da2e5e697c9493 Mon Sep 17 00:00:00 2001 From: csviri Date: Wed, 27 Oct 2021 14:31:01 +0200 Subject: [PATCH 4/8] fix: formatting --- .../CustomResourceEventFilterTest.java | 32 ++++++++++--------- .../observedgeneration/ObservedGenSpec.java | 4 +-- .../observedgeneration/ObservedGenStatus.java | 1 - 3 files changed, 19 insertions(+), 18 deletions(-) diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/internal/CustomResourceEventFilterTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/internal/CustomResourceEventFilterTest.java index 337bb5bde2..86081e4af1 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/internal/CustomResourceEventFilterTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/internal/CustomResourceEventFilterTest.java @@ -3,13 +3,12 @@ import java.util.List; import java.util.Objects; -import io.fabric8.kubernetes.api.model.ObjectMeta; -import io.fabric8.kubernetes.client.CustomResource; -import io.javaoperatorsdk.operator.sample.observedgeneration.ObservedGenCustomResource; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import io.fabric8.kubernetes.api.model.KubernetesResourceList; +import io.fabric8.kubernetes.api.model.ObjectMeta; +import io.fabric8.kubernetes.client.CustomResource; import io.fabric8.kubernetes.client.dsl.MixedOperation; import io.fabric8.kubernetes.client.dsl.Resource; import io.javaoperatorsdk.operator.TestUtils; @@ -18,6 +17,7 @@ import io.javaoperatorsdk.operator.api.config.DefaultControllerConfiguration; import io.javaoperatorsdk.operator.processing.ConfiguredController; import io.javaoperatorsdk.operator.processing.event.EventHandler; +import io.javaoperatorsdk.operator.sample.observedgeneration.ObservedGenCustomResource; import io.javaoperatorsdk.operator.sample.simple.TestCustomResource; import static org.mockito.Mockito.any; @@ -99,9 +99,9 @@ public void eventFilteredByCustomPredicateAndGenerationAware() { @Test public void observedGenerationFiltering() { - var config = new ObservedGenControllerConfig(FINALIZER,true,null); + var config = new ObservedGenControllerConfig(FINALIZER, true, null); when(config.getConfigurationService().getResourceCloner()) - .thenReturn(ConfigurationService.DEFAULT_CLONER); + .thenReturn(ConfigurationService.DEFAULT_CLONER); var controller = new ObservedGenConfiguredController(config); var eventSource = new CustomResourceEventSource<>(controller); @@ -117,7 +117,7 @@ public void observedGenerationFiltering() { verify(eventHandler, times(0)).handleEvent(any()); cr.getMetadata().setGeneration(6L); - + eventSource.eventReceived(ResourceAction.UPDATED, cr, null); verify(eventHandler, times(1)).handleEvent(any()); } @@ -154,18 +154,19 @@ public void eventNotFilteredByCustomPredicateIfFinalizerIsRequired() { private static class TestControllerConfig extends ControllerConfig { public TestControllerConfig(String finalizer, boolean generationAware, - CustomResourceEventFilter eventFilter) { + CustomResourceEventFilter eventFilter) { super(finalizer, generationAware, eventFilter, TestCustomResource.class); } } - private static class ObservedGenControllerConfig extends ControllerConfig { + private static class ObservedGenControllerConfig + extends ControllerConfig { public ObservedGenControllerConfig(String finalizer, boolean generationAware, - CustomResourceEventFilter eventFilter) { + CustomResourceEventFilter eventFilter) { super(finalizer, generationAware, eventFilter, ObservedGenCustomResource.class); } } - private static class ControllerConfig> extends + private static class ControllerConfig> extends DefaultControllerConfiguration { public ControllerConfig(String finalizer, boolean generationAware, @@ -180,7 +181,7 @@ public ControllerConfig(String finalizer, boolean generationAware, null, null, eventFilter, - customResourceClass, + customResourceClass, mock(ConfigurationService.class)); when(getConfigurationService().getResourceCloner()) @@ -200,15 +201,16 @@ public MixedOperation { + private static class ObservedGenConfiguredController + extends ConfiguredController { - public ObservedGenConfiguredController(ControllerConfiguration configuration) { + public ObservedGenConfiguredController( + ControllerConfiguration configuration) { super(null, configuration, null); } @Override - public MixedOperation, - Resource> getCRClient() { + public MixedOperation, Resource> getCRClient() { return mock(MixedOperation.class); } } diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/sample/observedgeneration/ObservedGenSpec.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/sample/observedgeneration/ObservedGenSpec.java index 5cf25eb3b6..181204bc1c 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/sample/observedgeneration/ObservedGenSpec.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/sample/observedgeneration/ObservedGenSpec.java @@ -15,7 +15,7 @@ public void setValue(String value) { @Override public String toString() { return "TestCustomResourceSpec{" + - "value='" + value + '\'' + - '}'; + "value='" + value + '\'' + + '}'; } } diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/sample/observedgeneration/ObservedGenStatus.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/sample/observedgeneration/ObservedGenStatus.java index bf92d143f8..d4ffee5416 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/sample/observedgeneration/ObservedGenStatus.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/sample/observedgeneration/ObservedGenStatus.java @@ -1,6 +1,5 @@ package io.javaoperatorsdk.operator.sample.observedgeneration; -import io.javaoperatorsdk.operator.api.ObservedGenerationAware; import io.javaoperatorsdk.operator.api.ObservedGenerationAwareStatus; public class ObservedGenStatus extends ObservedGenerationAwareStatus { From 309ce871a63bc61960362d464276d1c694d4a1f2 Mon Sep 17 00:00:00 2001 From: csviri Date: Wed, 27 Oct 2021 15:21:02 +0200 Subject: [PATCH 5/8] docs improvements and formatting fix --- .../operator/api/ObservedGenerationAware.java | 10 ++++++- .../operator/processing/EventDispatcher.java | 6 ---- .../processing/EventDispatcherTest.java | 29 +++++++++++++++++++ 3 files changed, 38 insertions(+), 7 deletions(-) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/ObservedGenerationAware.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/ObservedGenerationAware.java index 3435543c79..675b15d1ca 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/ObservedGenerationAware.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/ObservedGenerationAware.java @@ -2,17 +2,25 @@ import java.util.Optional; +import io.fabric8.kubernetes.client.CustomResource; + /** * If the custom resource's status object implements this interface the observed generation will be * automatically handled. The last observed generation will be set to status when the status is * updated or no update comes from controller. In addition to that will be checked if the controller * is generation aware. * - * In order to work the status object returned by CustomResource.getStatus() should not be null. + * In order to work the status object returned by CustomResource.getStatus() should not be null. In + * addition to that from the controller that the + * {@link UpdateControl#updateStatusSubResource(CustomResource)} or + * {@link UpdateControl#updateCustomResourceAndStatus(CustomResource)} should be returned. The + * observed generation is not updated in other cases. + * */ public interface ObservedGenerationAware { void setObservedGeneration(Long generation); Optional getObservedGeneration(); + } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/EventDispatcher.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/EventDispatcher.java index 21faf20b37..3000e463b6 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/EventDispatcher.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/EventDispatcher.java @@ -122,17 +122,11 @@ private PostExecutionControl handleCreateOrUpdate( updatedCustomResource = updateStatusGenerationAware(updateControl.getCustomResource()); } else if (updateControl.isUpdateCustomResource()) { updatedCustomResource = updateCustomResource(updateControl.getCustomResource()); - } else if (configuration().isGenerationAware() && isStatusObservedGenerationAware(resource)) { - updatedCustomResource = updateCustomResource(resource); } return createPostExecutionControl(updatedCustomResource, updateControl); } } - private boolean isStatusObservedGenerationAware(R resource) { - return resource.getStatus() instanceof ObservedGenerationAware; - } - private R updateStatusGenerationAware(R customResource) { updateStatusObservedGenerationIfRequired(customResource); return customResourceFacade.updateStatus(customResource); diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/EventDispatcherTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/EventDispatcherTest.java index da32b467e0..87385e61d2 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/EventDispatcherTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/EventDispatcherTest.java @@ -9,6 +9,7 @@ import org.mockito.ArgumentCaptor; import org.mockito.ArgumentMatchers; +import io.fabric8.kubernetes.api.model.ObjectMeta; import io.fabric8.kubernetes.client.CustomResource; import io.javaoperatorsdk.operator.TestUtils; import io.javaoperatorsdk.operator.api.Context; @@ -23,6 +24,7 @@ import io.javaoperatorsdk.operator.processing.event.Event; import io.javaoperatorsdk.operator.processing.event.internal.CustomResourceEvent; import io.javaoperatorsdk.operator.processing.event.internal.ResourceAction; +import io.javaoperatorsdk.operator.sample.observedgeneration.ObservedGenCustomResource; import static io.javaoperatorsdk.operator.processing.event.internal.ResourceAction.ADDED; import static io.javaoperatorsdk.operator.processing.event.internal.ResourceAction.UPDATED; @@ -320,6 +322,33 @@ void reScheduleOnDeleteWithoutFinalizerRemoval() { assertThat(control.getReScheduleDelay().get()).isEqualTo(1000L); } + @Test + void setObservedGenerationForStatusIfNeeded() { + var observedGenResource = createObservedGenCustomResource(); + + when(configuration.isGenerationAware()).thenReturn(true); + when(controller.createOrUpdateResource(eq(observedGenResource), any())) + .thenReturn( + UpdateControl.updateStatusSubResource(observedGenResource)); + + when(customResourceFacade.updateStatus(observedGenResource)).thenReturn(observedGenResource); + + PostExecutionControl control = eventDispatcher.handleExecution( + executionScopeWithCREvent(ADDED, observedGenResource)); + assertThat(control.getUpdatedCustomResource().get().getStatus().getObservedGeneration().get()) + .isEqualTo(1L); + } + + @Test + private ObservedGenCustomResource createObservedGenCustomResource() { + ObservedGenCustomResource observedGenCustomResource = new ObservedGenCustomResource(); + observedGenCustomResource.setMetadata(new ObjectMeta()); + observedGenCustomResource.getMetadata().setGeneration(1L); + observedGenCustomResource.getMetadata().setFinalizers(new ArrayList<>()); + observedGenCustomResource.getMetadata().getFinalizers().add(DEFAULT_FINALIZER); + return observedGenCustomResource; + } + private void markForDeletion(CustomResource customResource) { customResource.getMetadata().setDeletionTimestamp("2019-8-10"); } From 7778236aa250526276861a28f79a35b800e31ff0 Mon Sep 17 00:00:00 2001 From: csviri Date: Wed, 27 Oct 2021 15:35:11 +0200 Subject: [PATCH 6/8] javadoc updates --- .../operator/api/ObservedGenerationAware.java | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/ObservedGenerationAware.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/ObservedGenerationAware.java index 675b15d1ca..59c3e5c2de 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/ObservedGenerationAware.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/ObservedGenerationAware.java @@ -6,9 +6,10 @@ /** * If the custom resource's status object implements this interface the observed generation will be - * automatically handled. The last observed generation will be set to status when the status is - * updated or no update comes from controller. In addition to that will be checked if the controller - * is generation aware. + * automatically handled. The last observed generation will be updated on status when the status is + * instructed to be updated (see below). In addition to that, controller configuration will be + * checked if is set to generation aware. If generation aware config is turned off, this interface + * is ignored. * * In order to work the status object returned by CustomResource.getStatus() should not be null. In * addition to that from the controller that the @@ -16,6 +17,7 @@ * {@link UpdateControl#updateCustomResourceAndStatus(CustomResource)} should be returned. The * observed generation is not updated in other cases. * + * @see ObservedGenerationAwareStatus */ public interface ObservedGenerationAware { From 24ec45430eb4e54b2e4dbd8a7f92c23ca0c4aa58 Mon Sep 17 00:00:00 2001 From: csviri Date: Wed, 27 Oct 2021 15:42:13 +0200 Subject: [PATCH 7/8] fix: small issue with commit message --- .../javaoperatorsdk/operator/api/ObservedGenerationAware.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/ObservedGenerationAware.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/ObservedGenerationAware.java index 59c3e5c2de..946fde48a3 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/ObservedGenerationAware.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/ObservedGenerationAware.java @@ -5,7 +5,7 @@ import io.fabric8.kubernetes.client.CustomResource; /** - * If the custom resource's status object implements this interface the observed generation will be + * If the custom resource's status implements this interface, the observed generation will be * automatically handled. The last observed generation will be updated on status when the status is * instructed to be updated (see below). In addition to that, controller configuration will be * checked if is set to generation aware. If generation aware config is turned off, this interface From 57316fa8fc57c06c7c39688de37310b349ac578e Mon Sep 17 00:00:00 2001 From: Chris Laprun Date: Wed, 27 Oct 2021 19:10:59 +0200 Subject: [PATCH 8/8] refactor: simplify generation-aware filter logic --- .../event/internal/CustomResourceEventFilters.java | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/internal/CustomResourceEventFilters.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/internal/CustomResourceEventFilters.java index 219215d801..7de41c455d 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/internal/CustomResourceEventFilters.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/internal/CustomResourceEventFilters.java @@ -23,18 +23,16 @@ public final class CustomResourceEventFilters { private static final CustomResourceEventFilter GENERATION_AWARE = (configuration, oldResource, newResource) -> { - if (configuration.isGenerationAware() && newResource.getStatus() != null && - newResource.getStatus() instanceof ObservedGenerationAware) { + final var status = newResource.getStatus(); + final var generationAware = configuration.isGenerationAware(); + if (generationAware && status instanceof ObservedGenerationAware) { var actualGeneration = newResource.getMetadata().getGeneration(); - var observedGeneration = ((ObservedGenerationAware) newResource.getStatus()) + var observedGeneration = ((ObservedGenerationAware) status) .getObservedGeneration(); return observedGeneration.map(aLong -> actualGeneration > aLong).orElse(true); - } else { - return oldResource == null - || !configuration.isGenerationAware() - || oldResource.getMetadata().getGeneration() < newResource.getMetadata() - .getGeneration(); } + return oldResource == null || !generationAware || + oldResource.getMetadata().getGeneration() < newResource.getMetadata().getGeneration(); }; private static final CustomResourceEventFilter PASSTHROUGH =