From 2e8f219437e53b9ca58f146a24397dbf38f0bfb8 Mon Sep 17 00:00:00 2001 From: Ioannis Canellos Date: Wed, 6 Oct 2021 14:05:39 +0300 Subject: [PATCH 01/11] chore: renaming vars named k8sClient to kubernetsClient --- .../operator/processing/ConfiguredController.java | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/ConfiguredController.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/ConfiguredController.java index d34317fdb4..120a05bb65 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/ConfiguredController.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/ConfiguredController.java @@ -26,15 +26,15 @@ public class ConfiguredController> implements Res Closeable { private final ResourceController controller; private final ControllerConfiguration configuration; - private final KubernetesClient k8sClient; + private final KubernetesClient kubernetesClient; private EventSourceManager eventSourceManager; public ConfiguredController(ResourceController controller, ControllerConfiguration configuration, - KubernetesClient k8sClient) { + KubernetesClient kubernetesClient) { this.controller = controller; this.configuration = configuration; - this.k8sClient = k8sClient; + this.kubernetesClient = kubernetesClient; } @Override @@ -140,11 +140,11 @@ public ControllerConfiguration getConfiguration() { } public KubernetesClient getClient() { - return k8sClient; + return kubernetesClient; } public MixedOperation, Resource> getCRClient() { - return k8sClient.resources(configuration.getCustomResourceClass()); + return kubernetesClient.resources(configuration.getCustomResourceClass()); } /** @@ -164,7 +164,8 @@ public void start() throws OperatorException { // check that the custom resource is known by the cluster if configured that way final CustomResourceDefinition crd; // todo: check proper CRD spec version based on config if (configuration.getConfigurationService().checkCRDAndValidateLocalModel()) { - crd = k8sClient.apiextensions().v1().customResourceDefinitions().withName(crdName).get(); + crd = + kubernetesClient.apiextensions().v1().customResourceDefinitions().withName(crdName).get(); if (crd == null) { throwMissingCRDException(crdName, specVersion, controllerName); } From 9ebfff090bd0ba70553c1b87fea6574481e7c388 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Fri, 8 Oct 2021 09:08:53 +0200 Subject: [PATCH 02/11] chore(deps): bump jandex-maven-plugin from 1.1.1 to 1.2.1 (#592) Bumps [jandex-maven-plugin](https://github.com/wildfly/jandex-maven-plugin) from 1.1.1 to 1.2.1. - [Release notes](https://github.com/wildfly/jandex-maven-plugin/releases) - [Commits](https://github.com/wildfly/jandex-maven-plugin/compare/1.1.1...1.2.1) --- updated-dependencies: - dependency-name: org.jboss.jandex:jandex-maven-plugin dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index 470cc27b92..d9d8da3a9b 100644 --- a/pom.xml +++ b/pom.xml @@ -67,7 +67,7 @@ 2.8.2 2.5.2 5.0.0 - 1.1.1 + 1.2.1 2.16.0 1.0 1.6.2 From f1f3867c6dd7c270de5a7d6ce3c1a2d187cc79cb Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Fri, 8 Oct 2021 09:09:08 +0200 Subject: [PATCH 03/11] chore(deps-dev): bump mockito-core from 3.12.4 to 4.0.0 (#591) Bumps [mockito-core](https://github.com/mockito/mockito) from 3.12.4 to 4.0.0. - [Release notes](https://github.com/mockito/mockito/releases) - [Commits](https://github.com/mockito/mockito/compare/v3.12.4...v4.0.0) --- updated-dependencies: - dependency-name: org.mockito:mockito-core dependency-type: direct:development update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index d9d8da3a9b..ac2324c699 100644 --- a/pom.xml +++ b/pom.xml @@ -44,7 +44,7 @@ 5.8.0 1.7.32 2.14.1 - 3.12.4 + 4.0.0 3.12.0 1.0 0.19 From b99ff10323cdca0d218c9206eb0f54a4ed43057c Mon Sep 17 00:00:00 2001 From: csviri Date: Thu, 7 Oct 2021 13:55:03 +0200 Subject: [PATCH 04/11] feature: Build PR on v2 --- .github/workflows/pr.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/pr.yml b/.github/workflows/pr.yml index 2f031c070e..6c33c1a710 100644 --- a/.github/workflows/pr.yml +++ b/.github/workflows/pr.yml @@ -8,7 +8,7 @@ concurrency: cancel-in-progress: true on: pull_request: - branches: [ master ] + branches: [ master, v2 ] workflow_dispatch: jobs: build: From 326f82f18b242f9f1f5ea1bc3921b09077c0e066 Mon Sep 17 00:00:00 2001 From: Chris Laprun Date: Thu, 23 Sep 2021 23:17:06 +0200 Subject: [PATCH 05/11] chore(ci): use Java 17 --- .github/workflows/master-snapshot-release.yml | 2 +- .github/workflows/pr.yml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/master-snapshot-release.yml b/.github/workflows/master-snapshot-release.yml index 8c1d1d0049..305cf37279 100644 --- a/.github/workflows/master-snapshot-release.yml +++ b/.github/workflows/master-snapshot-release.yml @@ -15,7 +15,7 @@ jobs: runs-on: ubuntu-latest strategy: matrix: - java: [11, 16] + java: [ 11, 17 ] distribution: [temurin, adopt-openj9] kubernetes: [ 'v1.17.13','v1.18.20','v1.19.14','v1.20.10','v1.21.4', 'v1.22.1' ] steps: diff --git a/.github/workflows/pr.yml b/.github/workflows/pr.yml index 6c33c1a710..77ca08961a 100644 --- a/.github/workflows/pr.yml +++ b/.github/workflows/pr.yml @@ -15,7 +15,7 @@ jobs: runs-on: ubuntu-latest strategy: matrix: - java: [ 11 ] + java: [ 11, 17 ] distribution: [ temurin, adopt-openj9 ] kubernetes: [ 'v1.17.13','v1.18.20','v1.19.14','v1.20.10','v1.21.4', 'v1.22.1' ] steps: From 98e3def23afebeee11062735dc1c4269f8bfe0c8 Mon Sep 17 00:00:00 2001 From: Chris Laprun Date: Fri, 24 Sep 2021 09:53:10 +0200 Subject: [PATCH 06/11] chore(ci): use only Temurin distribution --- .github/workflows/master-snapshot-release.yml | 2 +- .github/workflows/pr.yml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/master-snapshot-release.yml b/.github/workflows/master-snapshot-release.yml index 305cf37279..1fb5ac7e2e 100644 --- a/.github/workflows/master-snapshot-release.yml +++ b/.github/workflows/master-snapshot-release.yml @@ -16,7 +16,7 @@ jobs: strategy: matrix: java: [ 11, 17 ] - distribution: [temurin, adopt-openj9] + distribution: [ temurin ] kubernetes: [ 'v1.17.13','v1.18.20','v1.19.14','v1.20.10','v1.21.4', 'v1.22.1' ] steps: - uses: actions/checkout@v2 diff --git a/.github/workflows/pr.yml b/.github/workflows/pr.yml index 77ca08961a..a1ee4de399 100644 --- a/.github/workflows/pr.yml +++ b/.github/workflows/pr.yml @@ -16,7 +16,7 @@ jobs: strategy: matrix: java: [ 11, 17 ] - distribution: [ temurin, adopt-openj9 ] + distribution: [ temurin ] kubernetes: [ 'v1.17.13','v1.18.20','v1.19.14','v1.20.10','v1.21.4', 'v1.22.1' ] steps: - uses: actions/checkout@v2 From 746f0c1fae41454d6af613dd812351af30a5b4c6 Mon Sep 17 00:00:00 2001 From: Luca Burgazzoli Date: Mon, 11 Oct 2021 14:47:40 +0200 Subject: [PATCH 07/11] chore: add generics to PostExecutionControl to reduce IDEs noise (#594) --- .../operator/processing/PostExecutionControl.java | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/PostExecutionControl.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/PostExecutionControl.java index 763e01c181..60d0f4bb34 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/PostExecutionControl.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/PostExecutionControl.java @@ -21,12 +21,12 @@ private PostExecutionControl( this.runtimeException = runtimeException; } - public static PostExecutionControl onlyFinalizerAdded() { - return new PostExecutionControl(true, null, null); + public static > PostExecutionControl onlyFinalizerAdded() { + return new PostExecutionControl<>(true, null, null); } - public static PostExecutionControl defaultDispatch() { - return new PostExecutionControl(false, null, null); + public static > PostExecutionControl defaultDispatch() { + return new PostExecutionControl<>(false, null, null); } public static > PostExecutionControl customResourceUpdated( @@ -34,8 +34,9 @@ public static PostExecutionControl defaultDispatch() { return new PostExecutionControl<>(false, updatedCustomResource, null); } - public static PostExecutionControl exceptionDuringExecution(RuntimeException exception) { - return new PostExecutionControl(false, null, exception); + public static > PostExecutionControl exceptionDuringExecution( + RuntimeException exception) { + return new PostExecutionControl<>(false, null, exception); } public boolean isOnlyFinalizerHandled() { @@ -54,7 +55,7 @@ public boolean exceptionDuringExecution() { return runtimeException != null; } - public PostExecutionControl withReSchedule(long delay) { + public PostExecutionControl withReSchedule(long delay) { this.reScheduleDelay = delay; return this; } From bb6c00ea654a3c95d1e4b685b2851ac429735963 Mon Sep 17 00:00:00 2001 From: Luca Burgazzoli Date: Mon, 11 Oct 2021 14:48:19 +0200 Subject: [PATCH 08/11] chore: polish the junit5 extension (#593) --- .../operator/junit/OperatorExtension.java | 11 ++++++----- .../io/javaoperatorsdk/operator/ConcurrencyIT.java | 2 +- .../operator/ControllerExecutionIT.java | 4 ++-- .../operator/InformerEventSourceIT.java | 2 +- .../java/io/javaoperatorsdk/operator/RetryIT.java | 2 +- .../javaoperatorsdk/operator/SubResourceUpdateIT.java | 2 +- .../operator/UpdatingResAndSubResIT.java | 4 ++-- 7 files changed, 14 insertions(+), 13 deletions(-) diff --git a/operator-framework-junit5/src/main/java/io/javaoperatorsdk/operator/junit/OperatorExtension.java b/operator-framework-junit5/src/main/java/io/javaoperatorsdk/operator/junit/OperatorExtension.java index 743e2c7b72..b48aca918b 100644 --- a/operator-framework-junit5/src/main/java/io/javaoperatorsdk/operator/junit/OperatorExtension.java +++ b/operator-framework-junit5/src/main/java/io/javaoperatorsdk/operator/junit/OperatorExtension.java @@ -67,6 +67,11 @@ private OperatorExtension( this.waitForNamespaceDeletion = waitForNamespaceDeletion; } + /** + * Creates a {@link Builder} to set up an {@link OperatorExtension} instance. + * + * @return the builder. + */ public static Builder builder() { return new Builder(); } @@ -123,7 +128,7 @@ public NonNamespaceOperation T getNamedResource(Class type, String name) { + public T get(Class type, String name) { return kubernetesClient.resources(type).inNamespace(namespace).withName(name).get(); } @@ -135,10 +140,6 @@ public T replace(Class type, T resource) { return kubernetesClient.resources(type).inNamespace(namespace).replace(resource); } - public T get(Class type, String name) { - return kubernetesClient.resources(type).inNamespace(namespace).withName(name).get(); - } - @SuppressWarnings("unchecked") protected void before(ExtensionContext context) { diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/ConcurrencyIT.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/ConcurrencyIT.java index 6fa248a3d9..4332faf413 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/ConcurrencyIT.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/ConcurrencyIT.java @@ -58,7 +58,7 @@ public void manyResourcesGetCreatedUpdatedAndDeleted() throws InterruptedExcepti // update some resources for (int i = 0; i < NUMBER_OF_RESOURCES_UPDATED; i++) { TestCustomResource tcr = - operator.getNamedResource(TestCustomResource.class, + operator.get(TestCustomResource.class, TestUtils.TEST_CUSTOM_RESOURCE_PREFIX + i); tcr.getSpec().setValue(i + UPDATED_SUFFIX); operator.resources(TestCustomResource.class) diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/ControllerExecutionIT.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/ControllerExecutionIT.java index 4855e61f49..cb3cdbb704 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/ControllerExecutionIT.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/ControllerExecutionIT.java @@ -52,7 +52,7 @@ void awaitResourcesCreatedOrUpdated() { .untilAsserted( () -> { ConfigMap configMap = - operator.getNamedResource(ConfigMap.class, "test-config-map"); + operator.get(ConfigMap.class, "test-config-map"); assertThat(configMap).isNotNull(); assertThat(configMap.getData().get("test-key")).isEqualTo("test-value"); }); @@ -68,7 +68,7 @@ void awaitStatusUpdated(int timeout) { .untilAsserted( () -> { TestCustomResource cr = - operator.getNamedResource(TestCustomResource.class, + operator.get(TestCustomResource.class, TestUtils.TEST_CUSTOM_RESOURCE_NAME); assertThat(cr).isNotNull(); assertThat(cr.getStatus()).isNotNull(); diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/InformerEventSourceIT.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/InformerEventSourceIT.java index 66b9d73b98..1cb3fa7096 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/InformerEventSourceIT.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/InformerEventSourceIT.java @@ -70,7 +70,7 @@ private InformerEventSourceTestCustomResource initialCustomResource() { private void waitForCRStatusValue(String value) { await().atMost(10, TimeUnit.SECONDS).untilAsserted(() -> { var cr = - operator.getNamedResource(InformerEventSourceTestCustomResource.class, RESOURCE_NAME); + operator.get(InformerEventSourceTestCustomResource.class, RESOURCE_NAME); assertThat(cr.getStatus()).isNotNull(); assertThat(cr.getStatus().getConfigMapValue()).isEqualTo(value); }); diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/RetryIT.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/RetryIT.java index a68ba0bc7f..e5fcde6934 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/RetryIT.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/RetryIT.java @@ -52,7 +52,7 @@ public void retryFailedExecution() { .isEqualTo(RetryTestCustomResourceController.NUMBER_FAILED_EXECUTIONS + 1); RetryTestCustomResource finalResource = - operator.getNamedResource(RetryTestCustomResource.class, + operator.get(RetryTestCustomResource.class, resource.getMetadata().getName()); assertThat(finalResource.getStatus().getState()) .isEqualTo(RetryTestCustomResourceStatus.State.SUCCESS); diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/SubResourceUpdateIT.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/SubResourceUpdateIT.java index c09982b639..1cee81c054 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/SubResourceUpdateIT.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/SubResourceUpdateIT.java @@ -99,7 +99,7 @@ void awaitStatusUpdated(String name) { .untilAsserted( () -> { SubResourceTestCustomResource cr = - operator.getNamedResource(SubResourceTestCustomResource.class, name); + operator.get(SubResourceTestCustomResource.class, name); assertThat(cr.getMetadata().getFinalizers()).hasSize(1); assertThat(cr).isNotNull(); assertThat(cr.getStatus()).isNotNull(); diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/UpdatingResAndSubResIT.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/UpdatingResAndSubResIT.java index 7d513ddd33..7d08ca6110 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/UpdatingResAndSubResIT.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/UpdatingResAndSubResIT.java @@ -36,7 +36,7 @@ public void updatesSubResourceStatus() { DoubleUpdateTestCustomResource customResource = operator - .getNamedResource(DoubleUpdateTestCustomResource.class, + .get(DoubleUpdateTestCustomResource.class, resource.getMetadata().getName()); assertThat(TestUtils.getNumberOfExecutions(operator)) @@ -57,7 +57,7 @@ void awaitStatusUpdated(String name) { .untilAsserted( () -> { DoubleUpdateTestCustomResource cr = - operator.getNamedResource(DoubleUpdateTestCustomResource.class, name); + operator.get(DoubleUpdateTestCustomResource.class, name); assertThat(cr) .isNotNull(); From 76f4c0043a83f7bbc11fa610ff5f9c3057d95683 Mon Sep 17 00:00:00 2001 From: csviri Date: Tue, 12 Oct 2021 13:56:21 +0200 Subject: [PATCH 09/11] feat: Use informers as CustomResourceEventSource backbone and cache This is a major change and backbone of v2. We change also how the Resources are addressed both internally and from event source with CustomResourceID. Additional improvements also added. --- .gitignore | 2 + .../micrometer/MicrometerMetrics.java | 5 +- .../operator/ControllerUtils.java | 2 +- .../operator/EventListUtils.java | 4 +- .../operator/api/Controller.java | 8 +- .../AbstractControllerConfiguration.java | 35 ---- .../DefaultControllerConfiguration.java | 25 +-- .../processing/CustomResourceCache.java | 116 -------------- .../processing/DefaultEventHandler.java | 142 ++++++----------- .../operator/processing/EventBuffer.java | 21 ++- .../operator/processing/EventDispatcher.java | 1 - .../operator/processing/ExecutionScope.java | 5 +- .../operator/processing/ResourceCache.java | 12 ++ .../processing/event/AbstractEvent.java | 21 --- .../processing/event/CustomResourceID.java | 50 ++++++ .../processing/event/DefaultEvent.java | 62 +------- .../event/DefaultEventSourceManager.java | 75 +++------ .../operator/processing/event/Event.java | 22 +-- .../processing/event/EventSource.java | 2 +- .../processing/event/EventSourceManager.java | 8 +- .../event/internal/CustomResourceEvent.java | 50 ++---- .../internal/CustomResourceEventFilters.java | 12 +- .../internal/CustomResourceEventSource.java | 149 ++++++++++-------- .../event/internal/InformerEvent.java | 31 ++-- .../event/internal/InformerEventSource.java | 21 +-- .../event/internal/LabelSelectorParser.java | 23 +++ .../processing/event/internal/Mappers.java | 33 ++-- .../event/internal/ResourceAction.java | 5 + .../processing/event/internal/TimerEvent.java | 5 +- .../event/internal/TimerEventSource.java | 25 ++- .../javaoperatorsdk/operator/TestUtils.java | 15 +- .../CustomResourceSelectorTest.java | 110 ------------- .../processing/DefaultEventHandlerTest.java | 84 +++++----- .../operator/processing/EventBufferTest.java | 20 +-- .../processing/EventDispatcherTest.java | 48 +++--- .../event/DefaultEventSourceManagerTest.java | 15 +- .../processing/event/EventListTest.java | 5 +- .../CustomResourceEventFilterTest.java | 26 +-- .../CustomResourceEventSourceTest.java | 39 +++-- .../internal/CustomResourceSelectorTest.java | 21 ++- .../internal/LabelSelectorParserTest.java | 29 ++++ .../event/internal/TimerEventSourceTest.java | 17 +- .../runtime/AccumulativeMappingWriter.java | 2 +- .../runtime/AnnotationConfiguration.java | 40 +++-- .../config/runtime/ClassMappingProvider.java | 2 +- .../operator/EventSourceIT.java | 6 +- ...entSourceTestCustomResourceController.java | 2 +- pom.xml | 3 +- 48 files changed, 579 insertions(+), 877 deletions(-) delete mode 100644 operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/AbstractControllerConfiguration.java delete mode 100644 operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/CustomResourceCache.java create mode 100644 operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/ResourceCache.java delete mode 100644 operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/AbstractEvent.java create mode 100644 operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/CustomResourceID.java create mode 100644 operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/internal/LabelSelectorParser.java create mode 100644 operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/internal/ResourceAction.java delete mode 100644 operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/CustomResourceSelectorTest.java create mode 100644 operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/internal/LabelSelectorParserTest.java diff --git a/.gitignore b/.gitignore index d82a982579..1f3ddc0c32 100644 --- a/.gitignore +++ b/.gitignore @@ -10,3 +10,5 @@ target/ # VSCode .factorypath + +.mvn/wrapper/maven-wrapper.jar diff --git a/micrometer-support/src/main/java/io/javaoperatorsdk/operator/micrometer/MicrometerMetrics.java b/micrometer-support/src/main/java/io/javaoperatorsdk/operator/micrometer/MicrometerMetrics.java index 7dd5515173..3c8074f8f8 100644 --- a/micrometer-support/src/main/java/io/javaoperatorsdk/operator/micrometer/MicrometerMetrics.java +++ b/micrometer-support/src/main/java/io/javaoperatorsdk/operator/micrometer/MicrometerMetrics.java @@ -5,6 +5,7 @@ import io.javaoperatorsdk.operator.Metrics; import io.javaoperatorsdk.operator.processing.DefaultEventHandler.EventMonitor; +import io.javaoperatorsdk.operator.processing.event.CustomResourceID; import io.javaoperatorsdk.operator.processing.event.Event; import io.micrometer.core.instrument.MeterRegistry; import io.micrometer.core.instrument.Timer; @@ -15,12 +16,12 @@ public class MicrometerMetrics implements Metrics { private final MeterRegistry registry; private final EventMonitor monitor = new EventMonitor() { @Override - public void processedEvent(String uid, Event event) { + public void processedEvent(CustomResourceID uid, Event event) { incrementProcessedEventsNumber(); } @Override - public void failedEvent(String uid, Event event) { + public void failedEvent(CustomResourceID uid, Event event) { incrementControllerRetriesNumber(); } }; diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/ControllerUtils.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/ControllerUtils.java index 0097cb128a..023b333758 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/ControllerUtils.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/ControllerUtils.java @@ -18,7 +18,7 @@ public static String getNameFor(Class controllerCl final var annotation = controllerClass.getAnnotation(Controller.class); if (annotation != null) { final var name = annotation.name(); - if (!Controller.NULL.equals(name)) { + if (!Controller.EMPTY_STRING.equals(name)) { return name; } } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/EventListUtils.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/EventListUtils.java index 852b630af0..0fe18bdae0 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/EventListUtils.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/EventListUtils.java @@ -2,9 +2,9 @@ import java.util.List; -import io.fabric8.kubernetes.client.Watcher; import io.javaoperatorsdk.operator.processing.event.Event; import io.javaoperatorsdk.operator.processing.event.internal.CustomResourceEvent; +import io.javaoperatorsdk.operator.processing.event.internal.ResourceAction; public class EventListUtils { @@ -13,7 +13,7 @@ public static boolean containsCustomResourceDeletedEvent(List events) { .anyMatch( e -> { if (e instanceof CustomResourceEvent) { - return ((CustomResourceEvent) e).getAction() == Watcher.Action.DELETED; + return ((CustomResourceEvent) e).getAction() == ResourceAction.DELETED; } else { return false; } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/Controller.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/Controller.java index ebf9c65d42..bf03b03309 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/Controller.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/Controller.java @@ -11,11 +11,11 @@ @Target({ElementType.TYPE}) public @interface Controller { - String NULL = ""; + String EMPTY_STRING = ""; String WATCH_CURRENT_NAMESPACE = "JOSDK_WATCH_CURRENT"; String NO_FINALIZER = "JOSDK_NO_FINALIZER"; - String name() default NULL; + String name() default EMPTY_STRING; /** * Optional finalizer name, if it is not provided, one will be automatically generated. If the @@ -24,7 +24,7 @@ * * @return the finalizer name */ - String finalizerName() default NULL; + String finalizerName() default EMPTY_STRING; /** * If true, will dispatch new event to the controller if generation increased since the last @@ -50,7 +50,7 @@ * * @return the label selector */ - String labelSelector() default NULL; + String labelSelector() default EMPTY_STRING; /** diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/AbstractControllerConfiguration.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/AbstractControllerConfiguration.java deleted file mode 100644 index 65b4a7ebc3..0000000000 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/AbstractControllerConfiguration.java +++ /dev/null @@ -1,35 +0,0 @@ -package io.javaoperatorsdk.operator.api.config; - -import java.util.Set; - -import io.fabric8.kubernetes.client.CustomResource; -import io.javaoperatorsdk.operator.processing.event.internal.CustomResourceEventFilter; - -/** - * @deprecated use {@link DefaultControllerConfiguration} instead - * @param - */ -@Deprecated -public class AbstractControllerConfiguration> - extends DefaultControllerConfiguration { - - @Deprecated - public AbstractControllerConfiguration(String associatedControllerClassName, String name, - String crdName, String finalizer, boolean generationAware, - Set namespaces, - RetryConfiguration retryConfiguration) { - super(associatedControllerClassName, name, crdName, finalizer, generationAware, namespaces, - retryConfiguration, null, null, null, null); - } - - public AbstractControllerConfiguration(String associatedControllerClassName, String name, - String crdName, String finalizer, boolean generationAware, - Set namespaces, - RetryConfiguration retryConfiguration, String labelSelector, - CustomResourceEventFilter customResourcePredicate, - Class customResourceClass, - ConfigurationService service) { - super(associatedControllerClassName, name, crdName, finalizer, generationAware, namespaces, - retryConfiguration, labelSelector, customResourcePredicate, customResourceClass, service); - } -} diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/DefaultControllerConfiguration.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/DefaultControllerConfiguration.java index 9775ce2151..672704f3c4 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/DefaultControllerConfiguration.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/DefaultControllerConfiguration.java @@ -19,7 +19,7 @@ public class DefaultControllerConfiguration> private final RetryConfiguration retryConfiguration; private final String labelSelector; private final CustomResourceEventFilter customResourceEventFilter; - private Class customResourceClass; + private final Class customResourceClass; private ConfigurationService service; public DefaultControllerConfiguration( @@ -54,29 +54,6 @@ public DefaultControllerConfiguration( setConfigurationService(service); } - @Deprecated - public DefaultControllerConfiguration( - String associatedControllerClassName, - String name, - String crdName, - String finalizer, - boolean generationAware, - Set namespaces, - RetryConfiguration retryConfiguration) { - this( - associatedControllerClassName, - name, - crdName, - finalizer, - generationAware, - namespaces, - retryConfiguration, - null, - null, - null, - null); - } - @Override public String getName() { return name; diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/CustomResourceCache.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/CustomResourceCache.java deleted file mode 100644 index 0e6d313f37..0000000000 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/CustomResourceCache.java +++ /dev/null @@ -1,116 +0,0 @@ -package io.javaoperatorsdk.operator.processing; - -import java.util.List; -import java.util.Optional; -import java.util.Set; -import java.util.concurrent.ConcurrentHashMap; -import java.util.concurrent.ConcurrentMap; -import java.util.concurrent.locks.Lock; -import java.util.concurrent.locks.ReentrantLock; -import java.util.function.Predicate; -import java.util.stream.Collectors; - -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - -import io.fabric8.kubernetes.client.CustomResource; -import io.javaoperatorsdk.operator.Metrics; - -import com.fasterxml.jackson.core.JsonProcessingException; -import com.fasterxml.jackson.databind.ObjectMapper; - -import static io.javaoperatorsdk.operator.processing.KubernetesResourceUtils.getName; -import static io.javaoperatorsdk.operator.processing.KubernetesResourceUtils.getUID; - -@SuppressWarnings("rawtypes") -public class CustomResourceCache> { - - private static final Logger log = LoggerFactory.getLogger(CustomResourceCache.class); - private static final Predicate passthrough = o -> true; - - private final ObjectMapper objectMapper; - private final ConcurrentMap resources; - private final Lock lock = new ReentrantLock(); - - public CustomResourceCache() { - this(new ObjectMapper(), Metrics.NOOP); - } - - public CustomResourceCache(ObjectMapper objectMapper, Metrics metrics) { - this.objectMapper = objectMapper; - if (metrics == null) { - metrics = Metrics.NOOP; - } - resources = metrics.monitorSizeOf(new ConcurrentHashMap<>(), "cache"); - } - - @SuppressWarnings("unchecked") - public void cacheResource(T resource) { - cacheResource(resource, passthrough); - } - - public void cacheResource(T resource, Predicate predicate) { - try { - lock.lock(); - final var uid = getUID(resource); - if (predicate.test(resources.get(uid))) { - if (passthrough != predicate) { - log.trace("Update cache after condition is true: {}", getName(resource)); - } - // defensive copy - resources.put(getUID(resource), clone(resource)); - } - } finally { - lock.unlock(); - } - } - - /** - * We clone the object so the one in the cache is not changed by the controller or dispatcher. - * Therefore the cached object always represents the object coming from the API server. - * - * @param uuid identifier of resource - * @return resource if found in cache - */ - public Optional getLatestResource(String uuid) { - return Optional.ofNullable(resources.get(uuid)).map(this::clone); - } - - public List getLatestResources(Predicate selector) { - try { - lock.lock(); - return resources.values().stream() - .filter(selector) - .map(this::clone) - .collect(Collectors.toList()); - } finally { - lock.unlock(); - } - } - - public Set getLatestResourcesUids(Predicate selector) { - try { - lock.lock(); - return resources.values().stream() - .filter(selector) - .map(r -> r.getMetadata().getUid()) - .collect(Collectors.toSet()); - } finally { - lock.unlock(); - } - } - - @SuppressWarnings("unchecked") - private T clone(CustomResource customResource) { - try { - return (T) objectMapper.readValue( - objectMapper.writeValueAsString(customResource), customResource.getClass()); - } catch (JsonProcessingException e) { - throw new IllegalStateException(e); - } - } - - public T cleanup(String customResourceUid) { - return resources.remove(customResourceUid); - } -} diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/DefaultEventHandler.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/DefaultEventHandler.java index 27843a669a..5d440193df 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/DefaultEventHandler.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/DefaultEventHandler.java @@ -8,16 +8,15 @@ import java.util.concurrent.ExecutorService; import java.util.concurrent.ScheduledThreadPoolExecutor; import java.util.concurrent.locks.ReentrantLock; -import java.util.function.Predicate; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import io.fabric8.kubernetes.client.CustomResource; -import io.javaoperatorsdk.operator.Metrics; import io.javaoperatorsdk.operator.api.RetryInfo; import io.javaoperatorsdk.operator.api.config.ConfigurationService; import io.javaoperatorsdk.operator.api.config.ExecutorServiceManager; +import io.javaoperatorsdk.operator.processing.event.CustomResourceID; import io.javaoperatorsdk.operator.processing.event.DefaultEventSourceManager; import io.javaoperatorsdk.operator.processing.event.Event; import io.javaoperatorsdk.operator.processing.event.EventHandler; @@ -27,7 +26,6 @@ import static io.javaoperatorsdk.operator.EventListUtils.containsCustomResourceDeletedEvent; import static io.javaoperatorsdk.operator.processing.KubernetesResourceUtils.getName; -import static io.javaoperatorsdk.operator.processing.KubernetesResourceUtils.getVersion; /** * Event handler that makes sure that events are processed in a "single threaded" way per resource @@ -41,31 +39,36 @@ public class DefaultEventHandler> implements Even private static EventMonitor monitor = EventMonitor.NOOP; private final EventBuffer eventBuffer; - private final Set underProcessing = new HashSet<>(); + private final Set underProcessing = new HashSet<>(); private final EventDispatcher eventDispatcher; private final Retry retry; - private final Map retryState = new HashMap<>(); + private final Map retryState = new HashMap<>(); private final ExecutorService executor; private final String controllerName; private final ReentrantLock lock = new ReentrantLock(); private final EventMonitor eventMonitor; private volatile boolean running; + private final ResourceCache resourceCache; private DefaultEventSourceManager eventSourceManager; - public DefaultEventHandler(ConfiguredController controller) { - this(ExecutorServiceManager.instance().executorService(), + public DefaultEventHandler(ConfiguredController controller, ResourceCache resourceCache) { + this( + resourceCache, + ExecutorServiceManager.instance().executorService(), controller.getConfiguration().getName(), new EventDispatcher<>(controller), GenericRetry.fromConfiguration(controller.getConfiguration().getRetryConfiguration()), controller.getConfiguration().getConfigurationService().getMetrics().getEventMonitor()); } - DefaultEventHandler(EventDispatcher eventDispatcher, String relatedControllerName, + DefaultEventHandler(EventDispatcher eventDispatcher, ResourceCache resourceCache, + String relatedControllerName, Retry retry) { - this(null, relatedControllerName, eventDispatcher, retry, null); + this(resourceCache, null, relatedControllerName, eventDispatcher, retry, null); } - private DefaultEventHandler(ExecutorService executor, String relatedControllerName, + private DefaultEventHandler(ResourceCache resourceCache, ExecutorService executor, + String relatedControllerName, EventDispatcher eventDispatcher, Retry retry, EventMonitor monitor) { this.running = true; this.executor = @@ -76,7 +79,8 @@ private DefaultEventHandler(ExecutorService executor, String relatedControllerNa this.controllerName = relatedControllerName; this.eventDispatcher = eventDispatcher; this.retry = retry; - this.eventBuffer = new EventBuffer(); + eventBuffer = new EventBuffer(); + this.resourceCache = resourceCache; this.eventMonitor = monitor != null ? monitor : EventMonitor.NOOP; } @@ -84,31 +88,21 @@ public void setEventSourceManager(DefaultEventSourceManager eventSourceManage this.eventSourceManager = eventSourceManager; } - /** - * @deprecated the EventMonitor to be used should now be retrieved from - * {@link Metrics#getEventMonitor()} - * @param monitor - */ - @Deprecated - public static void setEventMonitor(EventMonitor monitor) { - DefaultEventHandler.monitor = monitor; - } - /* * TODO: promote this interface to top-level, probably create a `monitoring` package? */ public interface EventMonitor { EventMonitor NOOP = new EventMonitor() { @Override - public void processedEvent(String uid, Event event) {} + public void processedEvent(CustomResourceID uid, Event event) {} @Override - public void failedEvent(String uid, Event event) {} + public void failedEvent(CustomResourceID uid, Event event) {} }; - void processedEvent(String uid, Event event); + void processedEvent(CustomResourceID uid, Event event); - void failedEvent(String uid, Event event); + void failedEvent(CustomResourceID uid, Event event); } private EventMonitor monitor() { @@ -119,24 +113,17 @@ private EventMonitor monitor() { @Override public void handleEvent(Event event) { - try { lock.lock(); - + log.debug("Received event: {}", event); if (!this.running) { log.debug("Skipping event: {} because the event handler is shutting down", event); return; } - - log.debug("Received event: {}", event); - - final Predicate selector = event.getCustomResourcesSelector(); final var monitor = monitor(); - for (String uid : eventSourceManager.getLatestResourceUids(selector)) { - eventBuffer.addEvent(uid, event); - monitor.processedEvent(uid, event); - executeBufferedEvents(uid); - } + eventBuffer.addEvent(event.getRelatedCustomResourceID(), event); + monitor.processedEvent(event.getRelatedCustomResourceID(), event); + executeBufferedEvents(event.getRelatedCustomResourceID()); } finally { lock.unlock(); } @@ -152,11 +139,11 @@ public void close() { } } - private boolean executeBufferedEvents(String customResourceUid) { + private boolean executeBufferedEvents(CustomResourceID customResourceUid) { boolean newEventForResourceId = eventBuffer.containsEvents(customResourceUid); boolean controllerUnderExecution = isControllerUnderExecution(customResourceUid); - Optional latestCustomResource = - eventSourceManager.getLatestResource(customResourceUid); + Optional latestCustomResource = + resourceCache.getCustomResource(customResourceUid); if (!controllerUnderExecution && newEventForResourceId && latestCustomResource.isPresent()) { setUnderExecutionProcessing(customResourceUid); @@ -176,11 +163,14 @@ private boolean executeBufferedEvents(String customResourceUid) { newEventForResourceId, controllerUnderExecution, latestCustomResource.isPresent()); + if (latestCustomResource.isEmpty()) { + log.warn("no custom resource found in cache for CustomResourceID: {}", customResourceUid); + } return false; } } - private RetryInfo retryInfo(String customResourceUid) { + private RetryInfo retryInfo(CustomResourceID customResourceUid) { return retryState.get(customResourceUid); } @@ -196,13 +186,13 @@ void eventProcessingFinished( "Event processing finished. Scope: {}, PostExecutionControl: {}", executionScope, postExecutionControl); - unsetUnderExecution(executionScope.getCustomResourceUid()); + unsetUnderExecution(executionScope.getCustomResourceID()); if (retry != null && postExecutionControl.exceptionDuringExecution()) { handleRetryOnException(executionScope); final var monitor = monitor(); executionScope.getEvents() - .forEach(e -> monitor.failedEvent(executionScope.getCustomResourceUid(), e)); + .forEach(e -> monitor.failedEvent(executionScope.getCustomResourceID(), e)); return; } @@ -210,10 +200,9 @@ void eventProcessingFinished( markSuccessfulExecutionRegardingRetry(executionScope); } if (containsCustomResourceDeletedEvent(executionScope.getEvents())) { - cleanupAfterDeletedEvent(executionScope.getCustomResourceUid()); + cleanupAfterDeletedEvent(executionScope.getCustomResourceID()); } else { - cacheUpdatedResourceIfChanged(executionScope, postExecutionControl); - var executed = executeBufferedEvents(executionScope.getCustomResourceUid()); + var executed = executeBufferedEvents(executionScope.getCustomResourceID()); if (!executed) { reScheduleExecutionIfInstructed(postExecutionControl, executionScope.getCustomResource()); } @@ -237,12 +226,14 @@ private void reScheduleExecutionIfInstructed(PostExecutionControl postExecuti */ private void handleRetryOnException(ExecutionScope executionScope) { RetryExecution execution = getOrInitRetryExecution(executionScope); - boolean newEventsExists = eventBuffer.newEventsExists(executionScope.getCustomResourceUid()); - eventBuffer.putBackEvents(executionScope.getCustomResourceUid(), executionScope.getEvents()); + var customResourceID = executionScope.getCustomResourceID(); + boolean newEventsExists = eventBuffer + .newEventsExists(customResourceID); + eventBuffer.putBackEvents(customResourceID, executionScope.getEvents()); if (newEventsExists) { - log.debug("New events exists for for resource id: {}", executionScope.getCustomResourceUid()); - executeBufferedEvents(executionScope.getCustomResourceUid()); + log.debug("New events exists for for resource id: {}", customResourceID); + executeBufferedEvents(customResourceID); return; } Optional nextDelay = execution.nextDelay(); @@ -252,7 +243,7 @@ private void handleRetryOnException(ExecutionScope executionScope) { log.debug( "Scheduling timer event for retry with delay:{} for resource: {}", delay, - executionScope.getCustomResourceUid()); + customResourceID); eventSourceManager .getRetryTimerEventSource() .scheduleOnce(executionScope.getCustomResource(), delay); @@ -264,70 +255,35 @@ private void markSuccessfulExecutionRegardingRetry(ExecutionScope executionSc log.debug( "Marking successful execution for resource: {}", getName(executionScope.getCustomResource())); - retryState.remove(executionScope.getCustomResourceUid()); + retryState.remove(executionScope.getCustomResourceID()); eventSourceManager .getRetryTimerEventSource() - .cancelOnceSchedule(executionScope.getCustomResourceUid()); + .cancelOnceSchedule(executionScope.getCustomResourceID()); } private RetryExecution getOrInitRetryExecution(ExecutionScope executionScope) { - RetryExecution retryExecution = retryState.get(executionScope.getCustomResourceUid()); + RetryExecution retryExecution = retryState.get(executionScope.getCustomResourceID()); if (retryExecution == null) { retryExecution = retry.initExecution(); - retryState.put(executionScope.getCustomResourceUid(), retryExecution); + retryState.put(executionScope.getCustomResourceID(), retryExecution); } return retryExecution; } - /** - * Here we try to cache the latest resource after an update. The goal is to solve a concurrency - * issue we've seen: If an execution is finished, where we updated a custom resource, but there - * are other events already buffered for next execution, we might not get the newest custom - * resource from CustomResource event source in time. Thus we execute the next batch of events but - * with a non up to date CR. Here we cache the latest CustomResource from the update execution so - * we make sure its already used in the up-coming execution. - * - *

- * Note that this is an improvement, not a bug fix. This situation can happen naturally, we just - * make the execution more efficient, and avoid questions about conflicts. - * - *

- * Note that without the conditional locking in the cache, there is a very minor chance that we - * would override an additional change coming from a different client. - */ - private void cacheUpdatedResourceIfChanged( - ExecutionScope executionScope, PostExecutionControl postExecutionControl) { - if (postExecutionControl.customResourceUpdatedDuringExecution()) { - R originalCustomResource = executionScope.getCustomResource(); - R customResourceAfterExecution = postExecutionControl.getUpdatedCustomResource().get(); - String originalResourceVersion = getVersion(originalCustomResource); - - log.debug( - "Trying to update resource cache from update response for resource: {} new version: {} old version: {}", - getName(originalCustomResource), - getVersion(customResourceAfterExecution), - getVersion(originalCustomResource)); - eventSourceManager.cacheResource( - customResourceAfterExecution, - customResource -> getVersion(customResource).equals(originalResourceVersion) - && !originalResourceVersion.equals(getVersion(customResourceAfterExecution))); - } - } - - private void cleanupAfterDeletedEvent(String customResourceUid) { + private void cleanupAfterDeletedEvent(CustomResourceID customResourceUid) { eventSourceManager.cleanup(customResourceUid); eventBuffer.cleanup(customResourceUid); } - private boolean isControllerUnderExecution(String customResourceUid) { + private boolean isControllerUnderExecution(CustomResourceID customResourceUid) { return underProcessing.contains(customResourceUid); } - private void setUnderExecutionProcessing(String customResourceUid) { + private void setUnderExecutionProcessing(CustomResourceID customResourceUid) { underProcessing.add(customResourceUid); } - private void unsetUnderExecution(String customResourceUid) { + private void unsetUnderExecution(CustomResourceID customResourceUid) { underProcessing.remove(customResourceUid); } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/EventBuffer.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/EventBuffer.java index c9d248acf2..b9c565a001 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/EventBuffer.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/EventBuffer.java @@ -2,45 +2,44 @@ import java.util.*; +import io.javaoperatorsdk.operator.processing.event.CustomResourceID; import io.javaoperatorsdk.operator.processing.event.Event; class EventBuffer { - private final Map> events = new HashMap<>(); + private final Map> events = new HashMap<>(); - /** @deprecated use {@link #addEvent(String, Event)} */ - @Deprecated public void addEvent(Event event) { - addEvent(event.getRelatedCustomResourceUid(), event); + addEvent(event.getRelatedCustomResourceID(), event); } - public void addEvent(String uid, Event event) { + public void addEvent(CustomResourceID uid, Event event) { Objects.requireNonNull(uid, "uid"); Objects.requireNonNull(event, "event"); - List crEvents = events.computeIfAbsent(uid, (id) -> new LinkedList<>()); + List crEvents = events.computeIfAbsent(uid, (customResourceID) -> new LinkedList<>()); crEvents.add(event); } - public boolean newEventsExists(String resourceId) { + public boolean newEventsExists(CustomResourceID resourceId) { return events.get(resourceId) != null && !events.get(resourceId).isEmpty(); } - public void putBackEvents(String resourceUid, List oldEvents) { + public void putBackEvents(CustomResourceID resourceUid, List oldEvents) { List crEvents = events.computeIfAbsent(resourceUid, (id) -> new LinkedList<>()); crEvents.addAll(0, oldEvents); } - public boolean containsEvents(String customResourceId) { + public boolean containsEvents(CustomResourceID customResourceId) { return events.get(customResourceId) != null; } - public List getAndRemoveEventsForExecution(String resourceUid) { + public List getAndRemoveEventsForExecution(CustomResourceID resourceUid) { List crEvents = events.remove(resourceUid); return crEvents == null ? Collections.emptyList() : crEvents; } - public void cleanup(String resourceUid) { + public void cleanup(CustomResourceID resourceUid) { events.remove(resourceUid); } } 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 85fcbefb9d..92347d5460 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 @@ -168,7 +168,6 @@ private PostExecutionControl handleDelete(R resource, Context context) { if (deleteControl == DeleteControl.DEFAULT_DELETE && resource.hasFinalizer(configuration().getFinalizer())) { R customResource = removeFinalizer(resource); - // todo: should we patch the resource to remove the finalizer instead of updating it return PostExecutionControl.customResourceUpdated(customResource); } } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/ExecutionScope.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/ExecutionScope.java index 53917cc094..4f5f0ca7f7 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/ExecutionScope.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/ExecutionScope.java @@ -4,6 +4,7 @@ import io.fabric8.kubernetes.client.CustomResource; import io.javaoperatorsdk.operator.api.RetryInfo; +import io.javaoperatorsdk.operator.processing.event.CustomResourceID; import io.javaoperatorsdk.operator.processing.event.Event; public class ExecutionScope> { @@ -27,8 +28,8 @@ public R getCustomResource() { return customResource; } - public String getCustomResourceUid() { - return customResource.getMetadata().getUid(); + public CustomResourceID getCustomResourceID() { + return CustomResourceID.fromResource(customResource); } @Override diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/ResourceCache.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/ResourceCache.java new file mode 100644 index 0000000000..127926c724 --- /dev/null +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/ResourceCache.java @@ -0,0 +1,12 @@ +package io.javaoperatorsdk.operator.processing; + +import java.util.Optional; + +import io.fabric8.kubernetes.client.CustomResource; +import io.javaoperatorsdk.operator.processing.event.CustomResourceID; + +public interface ResourceCache> { + + Optional getCustomResource(CustomResourceID resourceID); + +} diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/AbstractEvent.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/AbstractEvent.java deleted file mode 100644 index 3429301fe1..0000000000 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/AbstractEvent.java +++ /dev/null @@ -1,21 +0,0 @@ -package io.javaoperatorsdk.operator.processing.event; - -import java.util.function.Predicate; - -import io.fabric8.kubernetes.client.CustomResource; - -/** - * @deprecated use {@link DefaultEvent} instead - */ -@Deprecated -public class AbstractEvent extends DefaultEvent { - - public AbstractEvent(String relatedCustomResourceUid, EventSource eventSource) { - super(relatedCustomResourceUid, eventSource); - } - - public AbstractEvent( - Predicate customResourcesSelector, EventSource eventSource) { - super(customResourcesSelector, eventSource); - } -} diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/CustomResourceID.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/CustomResourceID.java new file mode 100644 index 0000000000..d405e48a5a --- /dev/null +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/CustomResourceID.java @@ -0,0 +1,50 @@ +package io.javaoperatorsdk.operator.processing.event; + +import java.util.Objects; +import java.util.Optional; + +import io.fabric8.kubernetes.api.model.HasMetadata; + +public class CustomResourceID { + + public static CustomResourceID fromResource(HasMetadata resource) { + return new CustomResourceID(resource.getMetadata().getName(), + resource.getMetadata().getNamespace()); + } + + private final String name; + private final String namespace; + + public CustomResourceID(String name, String namespace) { + this.name = name; + this.namespace = namespace; + } + + public CustomResourceID(String name) { + this(name, null); + } + + public String getName() { + return name; + } + + public Optional getNamespace() { + return Optional.ofNullable(namespace); + } + + @Override + public boolean equals(Object o) { + if (this == o) + return true; + if (o == null || getClass() != o.getClass()) + return false; + CustomResourceID that = (CustomResourceID) o; + return Objects.equals(name, that.name) && Objects.equals(namespace, + that.namespace); + } + + @Override + public int hashCode() { + return Objects.hash(name, namespace); + } +} diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/DefaultEvent.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/DefaultEvent.java index b966c06609..c445f2bf27 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/DefaultEvent.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/DefaultEvent.java @@ -1,70 +1,24 @@ package io.javaoperatorsdk.operator.processing.event; -import java.util.Objects; -import java.util.function.Predicate; - -import io.fabric8.kubernetes.client.CustomResource; - @SuppressWarnings("rawtypes") public class DefaultEvent implements Event { - private final Predicate customResourcesSelector; - private final EventSource eventSource; - - public DefaultEvent(String relatedCustomResourceUid, EventSource eventSource) { - this.customResourcesSelector = new UIDMatchingPredicate(relatedCustomResourceUid); - this.eventSource = eventSource; - } + private final CustomResourceID relatedCustomResource; - public DefaultEvent(Predicate customResourcesSelector, EventSource eventSource) { - this.customResourcesSelector = customResourcesSelector; - this.eventSource = eventSource; - } - @Override - public String getRelatedCustomResourceUid() { - if (customResourcesSelector instanceof UIDMatchingPredicate) { - UIDMatchingPredicate resourcesSelector = (UIDMatchingPredicate) customResourcesSelector; - return resourcesSelector.uid; - } else { - return null; - } + public DefaultEvent(CustomResourceID targetCustomResource) { + this.relatedCustomResource = targetCustomResource; } - public Predicate getCustomResourcesSelector() { - return customResourcesSelector; - } @Override - public EventSource getEventSource() { - return eventSource; + public CustomResourceID getRelatedCustomResourceID() { + return relatedCustomResource; } @Override public String toString() { - return "{ class=" - + this.getClass().getName() - + ", customResourcesSelector=" - + customResourcesSelector - + ", eventSource=" - + eventSource - + " }"; - } - - public static class UIDMatchingPredicate implements Predicate { - private final String uid; - - public UIDMatchingPredicate(String uid) { - this.uid = uid; - } - - @Override - public boolean test(CustomResource customResource) { - return Objects.equals(uid, customResource.getMetadata().getUid()); - } - - @Override - public String toString() { - return "UIDMatchingPredicate{uid='" + uid + "'}"; - } + return "DefaultEvent{" + + "relatedCustomResource=" + relatedCustomResource + + '}'; } } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/DefaultEventSourceManager.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/DefaultEventSourceManager.java index 0ca4ffd289..8142544c01 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/DefaultEventSourceManager.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/DefaultEventSourceManager.java @@ -2,14 +2,11 @@ import java.io.IOException; import java.util.Collections; -import java.util.List; import java.util.Map; import java.util.Objects; import java.util.Optional; -import java.util.Set; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.locks.ReentrantLock; -import java.util.function.Predicate; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -18,7 +15,6 @@ import io.javaoperatorsdk.operator.MissingCRDException; import io.javaoperatorsdk.operator.OperatorException; import io.javaoperatorsdk.operator.processing.ConfiguredController; -import io.javaoperatorsdk.operator.processing.CustomResourceCache; import io.javaoperatorsdk.operator.processing.DefaultEventHandler; import io.javaoperatorsdk.operator.processing.event.internal.CustomResourceEventSource; import io.javaoperatorsdk.operator.processing.event.internal.TimerEventSource; @@ -27,27 +23,31 @@ public class DefaultEventSourceManager> implements EventSourceManager { public static final String RETRY_TIMER_EVENT_SOURCE_NAME = "retry-timer-event-source"; - private static final String CUSTOM_RESOURCE_EVENT_SOURCE_NAME = "custom-resource-event-source"; + public static final String CUSTOM_RESOURCE_EVENT_SOURCE_NAME = "custom-resource-event-source"; private static final Logger log = LoggerFactory.getLogger(DefaultEventSourceManager.class); private final ReentrantLock lock = new ReentrantLock(); private final Map eventSources = new ConcurrentHashMap<>(); - private final DefaultEventHandler defaultEventHandler; + private DefaultEventHandler defaultEventHandler; private TimerEventSource retryTimerEventSource; - DefaultEventSourceManager(DefaultEventHandler defaultEventHandler, boolean supportRetry) { - this.defaultEventHandler = defaultEventHandler; - defaultEventHandler.setEventSourceManager(this); - if (supportRetry) { - this.retryTimerEventSource = new TimerEventSource<>(); - registerEventSource(RETRY_TIMER_EVENT_SOURCE_NAME, retryTimerEventSource); - } + DefaultEventSourceManager(DefaultEventHandler defaultEventHandler) { + init(defaultEventHandler); } public DefaultEventSourceManager(ConfiguredController controller) { - this(new DefaultEventHandler<>(controller), true); - registerEventSource(CUSTOM_RESOURCE_EVENT_SOURCE_NAME, - new CustomResourceEventSource<>(controller)); + CustomResourceEventSource customResourceEventSource = + new CustomResourceEventSource<>(controller); + init(new DefaultEventHandler<>(controller, customResourceEventSource)); + registerEventSource(CUSTOM_RESOURCE_EVENT_SOURCE_NAME, customResourceEventSource); + } + + private void init(DefaultEventHandler defaultEventHandler) { + this.defaultEventHandler = defaultEventHandler; + defaultEventHandler.setEventSourceManager(this); + + this.retryTimerEventSource = new TimerEventSource<>(); + registerEventSource(RETRY_TIMER_EVENT_SOURCE_NAME, retryTimerEventSource); } @Override @@ -122,7 +122,7 @@ public Optional deRegisterEventSource(String name) { @Override public Optional deRegisterCustomResourceFromEventSource( - String eventSourceName, String customResourceUid) { + String eventSourceName, CustomResourceID customResourceUid) { try { lock.lock(); EventSource eventSource = this.eventSources.get(eventSourceName); @@ -150,42 +150,15 @@ public Map getRegisteredEventSources() { return Collections.unmodifiableMap(eventSources); } - public void cleanup(String customResourceUid) { + @Override + public CustomResourceEventSource getCustomResourceEventSource() { + return (CustomResourceEventSource) getRegisteredEventSources() + .get(CUSTOM_RESOURCE_EVENT_SOURCE_NAME); + } + + public void cleanup(CustomResourceID customResourceUid) { getRegisteredEventSources() .keySet() .forEach(k -> deRegisterCustomResourceFromEventSource(k, customResourceUid)); - eventSources.remove(customResourceUid); - CustomResourceCache cache = getCache(); - if (cache != null) { - cache.cleanup(customResourceUid); - } - } - - // todo: remove - public CustomResourceCache getCache() { - final var source = - (CustomResourceEventSource) getRegisteredEventSources() - .get(CUSTOM_RESOURCE_EVENT_SOURCE_NAME); - return source.getCache(); - } - - // todo: remove - public Optional getLatestResource(String customResourceUid) { - return getCache().getLatestResource(customResourceUid); - } - - // todo: remove - public List getLatestResources(Predicate selector) { - return getCache().getLatestResources(selector); - } - - // todo: remove - public Set getLatestResourceUids(Predicate selector) { - return getCache().getLatestResourcesUids(selector); - } - - // todo: remove - public void cacheResource(CustomResource resource, Predicate predicate) { - getCache().cacheResource(resource, predicate); } } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/Event.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/Event.java index f36ca6caa0..b0d51a37c0 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/Event.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/Event.java @@ -1,27 +1,7 @@ package io.javaoperatorsdk.operator.processing.event; -import java.util.function.Predicate; - -import io.fabric8.kubernetes.client.CustomResource; - public interface Event { - /** - * @return the UID of the the {@link CustomResource} for which a reconcile loop should be - * triggered. - * @deprecated use {@link #getCustomResourcesSelector()} - */ - @Deprecated - String getRelatedCustomResourceUid(); - - /** - * The selector used to determine the {@link CustomResource} for which a reconcile loop should be - * triggered. - * - * @return predicate used to match the target CustomResource - */ - Predicate getCustomResourcesSelector(); + CustomResourceID getRelatedCustomResourceID(); - /** @return the {@link EventSource} that has generated the event. */ - EventSource getEventSource(); } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventSource.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventSource.java index 149e0acc1c..1cc05f632c 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventSource.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventSource.java @@ -20,5 +20,5 @@ default void close() throws IOException {} void setEventHandler(EventHandler eventHandler); - default void eventSourceDeRegisteredForResource(String customResourceUid) {} + default void eventSourceDeRegisteredForResource(CustomResourceID customResourceUid) {} } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventSourceManager.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventSourceManager.java index b59491042b..86638c2786 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventSourceManager.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventSourceManager.java @@ -5,9 +5,11 @@ import java.util.Map; import java.util.Optional; +import io.fabric8.kubernetes.client.CustomResource; import io.javaoperatorsdk.operator.OperatorException; +import io.javaoperatorsdk.operator.processing.event.internal.CustomResourceEventSource; -public interface EventSourceManager extends Closeable { +public interface EventSourceManager> extends Closeable { /** * Add the {@link EventSource} identified by the given name to the event manager. @@ -32,10 +34,12 @@ void registerEventSource(String name, EventSource eventSource) Optional deRegisterEventSource(String name); Optional deRegisterCustomResourceFromEventSource( - String name, String customResourceUid); + String name, CustomResourceID customResourceUid); Map getRegisteredEventSources(); + CustomResourceEventSource getCustomResourceEventSource(); + @Override default void close() throws IOException {} } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/internal/CustomResourceEvent.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/internal/CustomResourceEvent.java index 007ce49edc..15a67108db 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/internal/CustomResourceEvent.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/internal/CustomResourceEvent.java @@ -1,54 +1,36 @@ package io.javaoperatorsdk.operator.processing.event.internal; import io.fabric8.kubernetes.client.CustomResource; -import io.fabric8.kubernetes.client.Watcher; -import io.javaoperatorsdk.operator.processing.KubernetesResourceUtils; +import io.javaoperatorsdk.operator.processing.event.CustomResourceID; import io.javaoperatorsdk.operator.processing.event.DefaultEvent; -import static io.javaoperatorsdk.operator.processing.KubernetesResourceUtils.getName; - public class CustomResourceEvent extends DefaultEvent { - private final Watcher.Action action; + private final ResourceAction action; private final CustomResource customResource; - public CustomResourceEvent( - Watcher.Action action, - CustomResource resource, - CustomResourceEventSource customResourceEventSource) { - super(KubernetesResourceUtils.getUID(resource), customResourceEventSource); - this.action = action; - this.customResource = resource; - } - - public Watcher.Action getAction() { - return action; - } - public String resourceUid() { - return getCustomResource().getMetadata().getUid(); + public CustomResourceEvent(ResourceAction action, + CustomResource resource) { + super(CustomResourceID.fromResource(resource)); + this.customResource = resource; + this.action = action; } @Override public String toString() { - return "CustomResourceEvent{" - + "action=" - + action - + ", resource=[ name=" - + getName(getCustomResource()) - + ", kind=" - + getCustomResource().getKind() - + ", apiVersion=" - + getCustomResource().getApiVersion() - + " ,resourceVersion=" - + getCustomResource().getMetadata().getResourceVersion() - + ", markedForDeletion: " - + (getCustomResource().getMetadata().getDeletionTimestamp() != null - && !getCustomResource().getMetadata().getDeletionTimestamp().isEmpty()) - + " ]}"; + return "CustomResourceEvent{" + + "action=" + action + + ", customResource=" + customResource + + '}'; } public CustomResource getCustomResource() { return customResource; } + + public ResourceAction getAction() { + return action; + } + } 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 306cde0888..2f3bc72327 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 @@ -113,12 +113,12 @@ public static CustomResourceEventFilter markedForD } return (configuration, oldResource, newResource) -> { - for (int i = 0; i < items.length; i++) { - if (items[i] == null) { + for (CustomResourceEventFilter item : items) { + if (item == null) { continue; } - if (!items[i].acceptChange(configuration, oldResource, newResource)) { + if (!item.acceptChange(configuration, oldResource, newResource)) { return false; } } @@ -147,12 +147,12 @@ public static CustomResourceEventFilter markedForD } return (configuration, oldResource, newResource) -> { - for (int i = 0; i < items.length; i++) { - if (items[i] == null) { + for (CustomResourceEventFilter item : items) { + if (item == null) { continue; } - if (items[i].acceptChange(configuration, oldResource, newResource)) { + if (item.acceptChange(configuration, oldResource, newResource)) { return true; } } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/internal/CustomResourceEventSource.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/internal/CustomResourceEventSource.java index c7a959061b..9964e7b450 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/internal/CustomResourceEventSource.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/internal/CustomResourceEventSource.java @@ -1,49 +1,51 @@ package io.javaoperatorsdk.operator.processing.event.internal; import java.io.IOException; -import java.util.LinkedList; -import java.util.List; +import java.util.*; +import java.util.concurrent.ConcurrentHashMap; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import io.fabric8.kubernetes.api.model.ListOptions; import io.fabric8.kubernetes.client.CustomResource; import io.fabric8.kubernetes.client.KubernetesClientException; -import io.fabric8.kubernetes.client.Watch; -import io.fabric8.kubernetes.client.Watcher; -import io.fabric8.kubernetes.client.WatcherException; -import io.fabric8.kubernetes.client.utils.Utils; +import io.fabric8.kubernetes.client.informers.ResourceEventHandler; +import io.fabric8.kubernetes.client.informers.SharedIndexInformer; +import io.fabric8.kubernetes.client.informers.cache.Cache; import io.javaoperatorsdk.operator.MissingCRDException; import io.javaoperatorsdk.operator.api.config.ControllerConfiguration; import io.javaoperatorsdk.operator.processing.ConfiguredController; -import io.javaoperatorsdk.operator.processing.CustomResourceCache; -import io.javaoperatorsdk.operator.processing.KubernetesResourceUtils; +import io.javaoperatorsdk.operator.processing.ResourceCache; import io.javaoperatorsdk.operator.processing.event.AbstractEventSource; +import io.javaoperatorsdk.operator.processing.event.CustomResourceID; + +import com.fasterxml.jackson.core.JsonProcessingException; +import com.fasterxml.jackson.databind.ObjectMapper; import static io.javaoperatorsdk.operator.processing.KubernetesResourceUtils.getName; import static io.javaoperatorsdk.operator.processing.KubernetesResourceUtils.getUID; import static io.javaoperatorsdk.operator.processing.KubernetesResourceUtils.getVersion; +import static io.javaoperatorsdk.operator.processing.event.internal.LabelSelectorParser.parseSimpleLabelSelector; /** * This is a special case since is not bound to a single custom resource */ public class CustomResourceEventSource> extends AbstractEventSource - implements Watcher { + implements ResourceEventHandler, ResourceCache { + + public static final String ANY_NAMESPACE_MAP_KEY = "anyNamespace"; private static final Logger log = LoggerFactory.getLogger(CustomResourceEventSource.class); private final ConfiguredController controller; - private final List watches; - private final CustomResourceCache customResourceCache; + private final Map> sharedIndexInformers = + new ConcurrentHashMap<>(); + private final ObjectMapper cloningObjectMapper; public CustomResourceEventSource(ConfiguredController controller) { this.controller = controller; - this.watches = new LinkedList<>(); - - this.customResourceCache = new CustomResourceCache<>( - controller.getConfiguration().getConfigurationService().getObjectMapper(), - controller.getConfiguration().getConfigurationService().getMetrics()); + this.cloningObjectMapper = + controller.getConfiguration().getConfigurationService().getObjectMapper(); } @Override @@ -52,25 +54,25 @@ public void start() { final var targetNamespaces = configuration.getEffectiveNamespaces(); final var client = controller.getCRClient(); final var labelSelector = configuration.getLabelSelector(); - var options = new ListOptions(); - if (Utils.isNotNullOrEmpty(labelSelector)) { - options.setLabelSelector(labelSelector); - } try { if (ControllerConfiguration.allNamespacesWatched(targetNamespaces)) { - var w = client.inAnyNamespace().watch(options, this); - watches.add(w); - log.debug("Registered {} -> {} for any namespace", controller, w); + var informer = client.inAnyNamespace() + .withLabels(parseSimpleLabelSelector(labelSelector)).inform(this); + sharedIndexInformers.put(ANY_NAMESPACE_MAP_KEY, informer); + log.debug("Registered {} -> {} for any namespace", controller, informer); } else { targetNamespaces.forEach( ns -> { - var w = client.inNamespace(ns).watch(options, this); - watches.add(w); - log.debug("Registered {} -> {} for namespace: {}", controller, w, ns); + var informer = client.inNamespace(ns) + .withLabels(parseSimpleLabelSelector(labelSelector)).inform(this); + sharedIndexInformers.put(ns, informer); + log.debug("Registered {} -> {} for namespace: {}", controller, informer, + ns); }); } } catch (Exception e) { + // todo double check this if still applies for informers if (e instanceof KubernetesClientException) { KubernetesClientException ke = (KubernetesClientException) e; if (404 == ke.getCode()) { @@ -88,35 +90,19 @@ public void start() { @Override public void close() throws IOException { eventHandler.close(); - for (Watch watch : this.watches) { + for (SharedIndexInformer informer : sharedIndexInformers.values()) { try { - log.info("Closing watch {} -> {}", controller, watch); - watch.close(); + log.info("Closing informer {} -> {}", controller, informer); + informer.close(); } catch (Exception e) { - log.warn("Error closing watcher {} -> {}", controller, watch, e); + log.warn("Error closing informer {} -> {}", controller, informer, e); } } } - @Override - public void eventReceived(Watcher.Action action, T customResource) { + public void eventReceived(ResourceAction action, T customResource, T oldResource) { log.debug( - "Event received for action: {}, resource: {}", action.name(), getName(customResource)); - - final String uuid = KubernetesResourceUtils.getUID(customResource); - final T oldResource = customResourceCache.getLatestResource(uuid).orElse(null); - - // cache the latest version of the CR - customResourceCache.cacheResource(customResource); - - if (action == Action.ERROR) { - log.debug( - "Skipping {} event for custom resource uid: {}, version: {}", - action, - getUID(customResource), - getVersion(customResource)); - return; - } + "Event received for resource: {}", getName(customResource)); final CustomResourceEventFilter filter = CustomResourceEventFilters.or( CustomResourceEventFilters.finalizerNeededAndApplied(), @@ -126,7 +112,7 @@ public void eventReceived(Watcher.Action action, T customResource) { CustomResourceEventFilters.generationAware())); if (filter.acceptChange(controller.getConfiguration(), oldResource, customResource)) { - eventHandler.handleEvent(new CustomResourceEvent(action, customResource, this)); + eventHandler.handleEvent(new CustomResourceEvent(action, clone(customResource))); } else { log.debug( "Skipping event handling resource {} with version: {}", @@ -136,29 +122,52 @@ public void eventReceived(Watcher.Action action, T customResource) { } @Override - public void onClose(WatcherException e) { - if (e == null) { - return; - } - if (e.isHttpGone()) { - log.warn("Received error for watch, will try to reconnect.", e); - try { - close(); - start(); - } catch (Throwable ex) { - log.error("Unexpected error happened with watch reconnect. Will exit.", e); - System.exit(1); - } + public void onAdd(T resource) { + eventReceived(ResourceAction.ADDED, resource, null); + } + + @Override + public void onUpdate(T oldCustomResource, T newCustomResource) { + eventReceived(ResourceAction.UPDATED, newCustomResource, oldCustomResource); + } + + @Override + public void onDelete(T resource, boolean b) { + eventReceived(ResourceAction.DELETED, resource, null); + } + + @Override + public Optional getCustomResource(CustomResourceID resourceID) { + var sharedIndexInformer = + sharedIndexInformers.get(resourceID.getNamespace().orElse(ANY_NAMESPACE_MAP_KEY)); + var resource = sharedIndexInformer.getStore() + .getByKey(Cache.namespaceKeyFunc(resourceID.getNamespace().orElse(null), + resourceID.getName())); + if (resource == null) { + return Optional.empty(); } else { - // Note that this should not happen normally, since fabric8 client handles reconnect. - // In case it tries to reconnect this method is not called. - log.error("Unexpected error happened with watch. Will exit.", e); - System.exit(1); + return Optional.of(clone(resource)); } } - // todo: remove - public CustomResourceCache getCache() { - return customResourceCache; + /** + * @return shared informers by namespace. If custom resource is not namespace scoped use + * CustomResourceEventSource.ANY_NAMESPACE_MAP_KEY + */ + public Map> getInformers() { + return Collections.unmodifiableMap(sharedIndexInformers); + } + + public SharedIndexInformer getInformer(String namespace) { + return getInformers().get(Objects.requireNonNullElse(namespace, ANY_NAMESPACE_MAP_KEY)); + } + + private T clone(T customResource) { + try { + return (T) cloningObjectMapper.readValue( + cloningObjectMapper.writeValueAsString(customResource), customResource.getClass()); + } catch (JsonProcessingException e) { + throw new IllegalStateException(e); + } } } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/internal/InformerEvent.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/internal/InformerEvent.java index ecfcece338..81c78d31b4 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/internal/InformerEvent.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/internal/InformerEvent.java @@ -1,30 +1,20 @@ package io.javaoperatorsdk.operator.processing.event.internal; import java.util.Optional; -import java.util.function.Predicate; -import io.fabric8.kubernetes.client.CustomResource; +import io.fabric8.kubernetes.api.model.HasMetadata; +import io.javaoperatorsdk.operator.processing.event.CustomResourceID; import io.javaoperatorsdk.operator.processing.event.DefaultEvent; -import io.javaoperatorsdk.operator.processing.event.EventSource; -public class InformerEvent extends DefaultEvent { +public class InformerEvent extends DefaultEvent { - private Action action; - private T resource; - private T oldResource; + private final ResourceAction action; + private final T resource; + private final T oldResource; - public InformerEvent(String relatedCustomResourceUid, EventSource eventSource, Action action, - T resource, - T oldResource) { - this(new UIDMatchingPredicate(relatedCustomResourceUid), eventSource, action, resource, - oldResource); - - } - - public InformerEvent(Predicate customResourcesSelector, EventSource eventSource, - Action action, + public InformerEvent(ResourceAction action, T resource, T oldResource) { - super(customResourcesSelector, eventSource); + super(CustomResourceID.fromResource(resource)); this.action = action; this.resource = resource; this.oldResource = oldResource; @@ -38,11 +28,8 @@ public Optional getOldResource() { return Optional.ofNullable(oldResource); } - public Action getAction() { + public ResourceAction getAction() { return action; } - public enum Action { - ADD, UPDATE, DELETE - } } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/internal/InformerEventSource.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/internal/InformerEventSource.java index 37e1add66e..34e409fa17 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/internal/InformerEventSource.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/internal/InformerEventSource.java @@ -12,33 +12,34 @@ import io.fabric8.kubernetes.client.informers.cache.Cache; import io.fabric8.kubernetes.client.informers.cache.Store; import io.javaoperatorsdk.operator.processing.event.AbstractEventSource; +import io.javaoperatorsdk.operator.processing.event.CustomResourceID; public class InformerEventSource extends AbstractEventSource { private final SharedInformer sharedInformer; - private final Function> resourceToUIDs; + private final Function> resourceToUIDs; private final Function associatedWith; private final boolean skipUpdateEventPropagationIfNoChange; public InformerEventSource(SharedInformer sharedInformer, - Function> resourceToUIDs) { + Function> resourceToUIDs) { this(sharedInformer, resourceToUIDs, null, true); } public InformerEventSource(KubernetesClient client, Class type, - Function> resourceToUIDs) { + Function> resourceToUIDs) { this(client, type, resourceToUIDs, false); } InformerEventSource(KubernetesClient client, Class type, - Function> resourceToUIDs, + Function> resourceToUIDs, boolean skipUpdateEventPropagationIfNoChange) { this(client.informers().sharedIndexInformerFor(type, 0), resourceToUIDs, null, skipUpdateEventPropagationIfNoChange); } public InformerEventSource(SharedInformer sharedInformer, - Function> resourceToUIDs, + Function> resourceToUIDs, Function associatedWith, boolean skipUpdateEventPropagationIfNoChange) { this.sharedInformer = sharedInformer; @@ -54,7 +55,7 @@ public InformerEventSource(SharedInformer sharedInformer, sharedInformer.addEventHandler(new ResourceEventHandler<>() { @Override public void onAdd(T t) { - propagateEvent(InformerEvent.Action.ADD, t, null); + propagateEvent(ResourceAction.ADDED, t, null); } @Override @@ -64,23 +65,23 @@ public void onUpdate(T oldObject, T newObject) { .equals(newObject.getMetadata().getResourceVersion())) { return; } - propagateEvent(InformerEvent.Action.UPDATE, newObject, oldObject); + propagateEvent(ResourceAction.UPDATED, newObject, oldObject); } @Override public void onDelete(T t, boolean b) { - propagateEvent(InformerEvent.Action.DELETE, t, null); + propagateEvent(ResourceAction.DELETED, t, null); } }); } - private void propagateEvent(InformerEvent.Action action, T object, T oldObject) { + private void propagateEvent(ResourceAction action, T object, T oldObject) { var uids = resourceToUIDs.apply(object); if (uids.isEmpty()) { return; } uids.forEach(uid -> { - InformerEvent event = new InformerEvent(uid, this, action, object, oldObject); + InformerEvent event = new InformerEvent(action, object, oldObject); this.eventHandler.handleEvent(event); }); } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/internal/LabelSelectorParser.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/internal/LabelSelectorParser.java new file mode 100644 index 0000000000..9c2061b642 --- /dev/null +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/internal/LabelSelectorParser.java @@ -0,0 +1,23 @@ +package io.javaoperatorsdk.operator.processing.event.internal; + +import java.util.Arrays; +import java.util.Collections; +import java.util.HashMap; +import java.util.Map; + +public class LabelSelectorParser { + + public static Map parseSimpleLabelSelector(String labelSelector) { + if (labelSelector == null || labelSelector.isBlank()) { + return Collections.EMPTY_MAP; + } + String[] selectors = labelSelector.split(","); + Map labels = new HashMap<>(selectors.length); + Arrays.stream(selectors).forEach(s -> { + var kv = s.split("="); + labels.put(kv[0], kv[1]); + }); + return labels; + } + +} diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/internal/Mappers.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/internal/Mappers.java index dc0b4b4501..7351b80d4f 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/internal/Mappers.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/internal/Mappers.java @@ -5,27 +5,42 @@ import java.util.function.Function; import io.fabric8.kubernetes.api.model.HasMetadata; +import io.javaoperatorsdk.operator.processing.event.CustomResourceID; public class Mappers { - public static Function> fromAnnotation( - String annotationKey) { - return fromMetadata(annotationKey, false); + + public static Function> fromAnnotation( + String nameKey) { + return fromMetadata(nameKey, null, false); + } + + public static Function> fromAnnotation( + String nameKey, String namespaceKey) { + return fromMetadata(nameKey, namespaceKey, false); + } + + public static Function> fromLabel( + String nameKey) { + return fromMetadata(nameKey, null, true); } - public static Function> fromLabel( - String labelKey) { - return fromMetadata(labelKey, true); + public static Function> fromLabel( + String nameKey, String namespaceKey) { + return fromMetadata(nameKey, namespaceKey, true); } - private static Function> fromMetadata( - String key, boolean isLabel) { + private static Function> fromMetadata( + String nameKey, String namespaceKey, boolean isLabel) { return resource -> { final var metadata = resource.getMetadata(); if (metadata == null) { return Collections.emptySet(); } else { final var map = isLabel ? metadata.getLabels() : metadata.getAnnotations(); - return map != null ? Set.of(map.get(key)) : Collections.emptySet(); + var namespace = + namespaceKey == null ? resource.getMetadata().getNamespace() : map.get(namespaceKey); + return map != null ? Set.of(new CustomResourceID(map.get(nameKey), namespace)) + : Collections.emptySet(); } }; } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/internal/ResourceAction.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/internal/ResourceAction.java new file mode 100644 index 0000000000..6fbc233133 --- /dev/null +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/internal/ResourceAction.java @@ -0,0 +1,5 @@ +package io.javaoperatorsdk.operator.processing.event.internal; + +public enum ResourceAction { + ADDED, UPDATED, DELETED +} diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/internal/TimerEvent.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/internal/TimerEvent.java index 6fa38674bf..9e9bfb5040 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/internal/TimerEvent.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/internal/TimerEvent.java @@ -1,10 +1,11 @@ package io.javaoperatorsdk.operator.processing.event.internal; +import io.javaoperatorsdk.operator.processing.event.CustomResourceID; import io.javaoperatorsdk.operator.processing.event.DefaultEvent; public class TimerEvent extends DefaultEvent { - public TimerEvent(String relatedCustomResourceUid, TimerEventSource eventSource) { - super(relatedCustomResourceUid, eventSource); + public TimerEvent(CustomResourceID relatedCustomResourceUid) { + super(relatedCustomResourceUid); } } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/internal/TimerEventSource.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/internal/TimerEventSource.java index 9ad15e47f9..51f21fc4d4 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/internal/TimerEventSource.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/internal/TimerEventSource.java @@ -11,23 +11,23 @@ import org.slf4j.LoggerFactory; import io.fabric8.kubernetes.client.CustomResource; -import io.javaoperatorsdk.operator.processing.KubernetesResourceUtils; import io.javaoperatorsdk.operator.processing.event.AbstractEventSource; +import io.javaoperatorsdk.operator.processing.event.CustomResourceID; public class TimerEventSource> extends AbstractEventSource { private static final Logger log = LoggerFactory.getLogger(TimerEventSource.class); private final Timer timer = new Timer(); private final AtomicBoolean running = new AtomicBoolean(); - private final Map onceTasks = new ConcurrentHashMap<>(); - private final Map timerTasks = new ConcurrentHashMap<>(); + private final Map onceTasks = new ConcurrentHashMap<>(); + private final Map timerTasks = new ConcurrentHashMap<>(); public void schedule(R customResource, long delay, long period) { if (!running.get()) { throw new IllegalStateException("The TimerEventSource is not running"); } - String resourceUid = KubernetesResourceUtils.getUID(customResource); + CustomResourceID resourceUid = CustomResourceID.fromResource(customResource); if (timerTasks.containsKey(resourceUid)) { return; } @@ -40,8 +40,7 @@ public void scheduleOnce(R customResource, long delay) { if (!running.get()) { throw new IllegalStateException("The TimerEventSource is not running"); } - - String resourceUid = KubernetesResourceUtils.getUID(customResource); + CustomResourceID resourceUid = CustomResourceID.fromResource(customResource); if (onceTasks.containsKey(resourceUid)) { cancelOnceSchedule(resourceUid); } @@ -51,19 +50,19 @@ public void scheduleOnce(R customResource, long delay) { } @Override - public void eventSourceDeRegisteredForResource(String customResourceUid) { + public void eventSourceDeRegisteredForResource(CustomResourceID customResourceUid) { cancelSchedule(customResourceUid); cancelOnceSchedule(customResourceUid); } - public void cancelSchedule(String customResourceUid) { - TimerTask timerTask = timerTasks.remove(customResourceUid); + public void cancelSchedule(CustomResourceID customResourceID) { + TimerTask timerTask = timerTasks.remove(customResourceID); if (timerTask != null) { timerTask.cancel(); } } - public void cancelOnceSchedule(String customResourceUid) { + public void cancelOnceSchedule(CustomResourceID customResourceUid) { TimerTask timerTask = onceTasks.remove(customResourceUid); if (timerTask != null) { timerTask.cancel(); @@ -85,9 +84,9 @@ public void close() throws IOException { public class EventProducerTimeTask extends TimerTask { - protected final String customResourceUid; + protected final CustomResourceID customResourceUid; - public EventProducerTimeTask(String customResourceUid) { + public EventProducerTimeTask(CustomResourceID customResourceUid) { this.customResourceUid = customResourceUid; } @@ -95,7 +94,7 @@ public EventProducerTimeTask(String customResourceUid) { public void run() { if (running.get()) { log.debug("Producing event for custom resource id: {}", customResourceUid); - eventHandler.handleEvent(new TimerEvent(customResourceUid, TimerEventSource.this)); + eventHandler.handleEvent(new TimerEvent(customResourceUid)); } } } diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/TestUtils.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/TestUtils.java index 93e6e57492..c1e887de86 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/TestUtils.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/TestUtils.java @@ -6,16 +6,14 @@ import io.fabric8.kubernetes.api.model.ObjectMetaBuilder; import io.fabric8.kubernetes.api.model.apiextensions.v1.CustomResourceDefinition; import io.fabric8.kubernetes.api.model.apiextensions.v1.CustomResourceDefinitionBuilder; +import io.javaoperatorsdk.operator.processing.event.CustomResourceID; import io.javaoperatorsdk.operator.sample.simple.TestCustomResource; import io.javaoperatorsdk.operator.sample.simple.TestCustomResourceSpec; public class TestUtils { - public static final String TEST_CUSTOM_RESOURCE_NAME = "test-custom-resource"; - public static final String TEST_NAMESPACE = "java-operator-sdk-int-test"; - public static TestCustomResource testCustomResource() { - return testCustomResource(UUID.randomUUID().toString()); + return testCustomResource(new CustomResourceID(UUID.randomUUID().toString(), "test")); } public static CustomResourceDefinition testCRD(String scope) { @@ -29,14 +27,13 @@ public static CustomResourceDefinition testCRD(String scope) { .build(); } - public static TestCustomResource testCustomResource(String uid) { + public static TestCustomResource testCustomResource(CustomResourceID id) { TestCustomResource resource = new TestCustomResource(); resource.setMetadata( new ObjectMetaBuilder() - .withName(TEST_CUSTOM_RESOURCE_NAME) - .withUid(uid) + .withName(id.getName()) .withGeneration(1L) - .withNamespace(TEST_NAMESPACE) + .withNamespace(id.getNamespace().orElse(null)) .build()); resource.getMetadata().setAnnotations(new HashMap<>()); resource.setKind("CustomService"); @@ -46,4 +43,6 @@ public static TestCustomResource testCustomResource(String uid) { resource.getSpec().setValue("test-value"); return resource; } + + } diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/CustomResourceSelectorTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/CustomResourceSelectorTest.java deleted file mode 100644 index 33f8347f13..0000000000 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/CustomResourceSelectorTest.java +++ /dev/null @@ -1,110 +0,0 @@ -package io.javaoperatorsdk.operator.processing; - -import java.util.Objects; -import java.util.UUID; - -import org.junit.jupiter.api.BeforeEach; -import org.junit.jupiter.api.Test; -import org.mockito.ArgumentCaptor; - -import io.javaoperatorsdk.operator.processing.event.DefaultEvent; -import io.javaoperatorsdk.operator.processing.event.DefaultEventSourceManager; -import io.javaoperatorsdk.operator.sample.simple.TestCustomResource; - -import static io.javaoperatorsdk.operator.TestUtils.testCustomResource; -import static org.assertj.core.api.Assertions.assertThat; -import static org.mockito.Mockito.any; -import static org.mockito.Mockito.doAnswer; -import static org.mockito.Mockito.doCallRealMethod; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.timeout; -import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.when; - -class CustomResourceSelectorTest { - - public static final int SEPARATE_EXECUTION_TIMEOUT = 450; - - private final EventDispatcher eventDispatcherMock = mock(EventDispatcher.class); - private final CustomResourceCache customResourceCache = new CustomResourceCache(); - - private final DefaultEventSourceManager defaultEventSourceManagerMock = - mock(DefaultEventSourceManager.class); - - private final DefaultEventHandler defaultEventHandler = - new DefaultEventHandler(eventDispatcherMock, "Test", null); - - @BeforeEach - public void setup() { - defaultEventHandler.setEventSourceManager(defaultEventSourceManagerMock); - - // todo: remove - when(defaultEventSourceManagerMock.getCache()).thenReturn(customResourceCache); - doCallRealMethod().when(defaultEventSourceManagerMock).getLatestResource(any()); - doCallRealMethod().when(defaultEventSourceManagerMock).getLatestResources(any()); - doCallRealMethod().when(defaultEventSourceManagerMock).getLatestResourceUids(any()); - doCallRealMethod().when(defaultEventSourceManagerMock).cacheResource(any(), any()); - doAnswer( - invocation -> { - final var resourceId = (String) invocation.getArgument(0); - customResourceCache.cleanup(resourceId); - return null; - }) - .when(defaultEventSourceManagerMock) - .cleanup(any()); - } - - @Test - public void dispatchEventsWithPredicate() { - TestCustomResource cr1 = testCustomResource(UUID.randomUUID().toString()); - cr1.getSpec().setValue("1"); - TestCustomResource cr2 = testCustomResource(UUID.randomUUID().toString()); - cr2.getSpec().setValue("2"); - TestCustomResource cr3 = testCustomResource(UUID.randomUUID().toString()); - cr3.getSpec().setValue("3"); - - customResourceCache.cacheResource(cr1); - customResourceCache.cacheResource(cr2); - customResourceCache.cacheResource(cr3); - - defaultEventHandler.handleEvent( - new DefaultEvent( - c -> { - var tcr = ((TestCustomResource) c); - return Objects.equals("1", tcr.getSpec().getValue()) - || Objects.equals("3", tcr.getSpec().getValue()); - }, - null)); - - verify(eventDispatcherMock, timeout(SEPARATE_EXECUTION_TIMEOUT).times(2)) - .handleExecution(any()); - - waitMinimalTime(); - - ArgumentCaptor executionScopeArgumentCaptor = - ArgumentCaptor.forClass(ExecutionScope.class); - - verify(eventDispatcherMock, timeout(SEPARATE_EXECUTION_TIMEOUT).times(2)) - .handleExecution(executionScopeArgumentCaptor.capture()); - - assertThat(executionScopeArgumentCaptor.getAllValues()) - .hasSize(2) - .allSatisfy( - s -> { - assertThat(s.getEvents()).isNotEmpty().hasOnlyElementsOfType(DefaultEvent.class); - assertThat(s) - .satisfiesAnyOf( - e -> Objects.equals(cr1.getMetadata().getUid(), e.getCustomResourceUid()), - e -> Objects.equals(cr3.getMetadata().getUid(), e.getCustomResourceUid())); - }); - } - - private void waitMinimalTime() { - try { - Thread.sleep(1000); - } catch (InterruptedException e) { - throw new IllegalStateException(e); - } - } - -} diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/DefaultEventHandlerTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/DefaultEventHandlerTest.java index a03f4bd8a1..620a7c9ac7 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/DefaultEventHandlerTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/DefaultEventHandlerTest.java @@ -1,7 +1,7 @@ package io.javaoperatorsdk.operator.processing; -import java.util.Arrays; import java.util.List; +import java.util.Optional; import java.util.UUID; import org.junit.jupiter.api.BeforeEach; @@ -11,21 +11,22 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import io.fabric8.kubernetes.client.Watcher; +import io.fabric8.kubernetes.client.CustomResource; +import io.javaoperatorsdk.operator.processing.event.CustomResourceID; import io.javaoperatorsdk.operator.processing.event.DefaultEventSourceManager; 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.processing.event.internal.TimerEvent; import io.javaoperatorsdk.operator.processing.event.internal.TimerEventSource; import io.javaoperatorsdk.operator.processing.retry.GenericRetry; import io.javaoperatorsdk.operator.sample.simple.TestCustomResource; import static io.javaoperatorsdk.operator.TestUtils.testCustomResource; +import static io.javaoperatorsdk.operator.processing.event.internal.ResourceAction.DELETED; import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.any; -import static org.mockito.Mockito.doAnswer; -import static org.mockito.Mockito.doCallRealMethod; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; import static org.mockito.Mockito.timeout; @@ -39,18 +40,19 @@ class DefaultEventHandlerTest { public static final int FAKE_CONTROLLER_EXECUTION_DURATION = 250; public static final int SEPARATE_EXECUTION_TIMEOUT = 450; + public static final String TEST_NAMESPACE = "default-event-handler-test"; private EventDispatcher eventDispatcherMock = mock(EventDispatcher.class); - private CustomResourceCache customResourceCache = new CustomResourceCache(); private DefaultEventSourceManager defaultEventSourceManagerMock = mock(DefaultEventSourceManager.class); + private ResourceCache resourceCache = mock(ResourceCache.class); private TimerEventSource retryTimerEventSourceMock = mock(TimerEventSource.class); private DefaultEventHandler defaultEventHandler = - new DefaultEventHandler(eventDispatcherMock, "Test", null); + new DefaultEventHandler(eventDispatcherMock, resourceCache, "Test", null); private DefaultEventHandler defaultEventHandlerWithRetry = - new DefaultEventHandler(eventDispatcherMock, "Test", + new DefaultEventHandler(eventDispatcherMock, resourceCache, "Test", GenericRetry.defaultLimitedExponentialRetry()); @BeforeEach @@ -59,22 +61,6 @@ public void setup() { .thenReturn(retryTimerEventSourceMock); defaultEventHandler.setEventSourceManager(defaultEventSourceManagerMock); defaultEventHandlerWithRetry.setEventSourceManager(defaultEventSourceManagerMock); - - // todo: remove - when(defaultEventSourceManagerMock.getCache()).thenReturn(customResourceCache); - doCallRealMethod().when(defaultEventSourceManagerMock).getLatestResource(any()); - doCallRealMethod().when(defaultEventSourceManagerMock).getLatestResource(any()); - doCallRealMethod().when(defaultEventSourceManagerMock).getLatestResources(any()); - doCallRealMethod().when(defaultEventSourceManagerMock).getLatestResourceUids(any()); - doCallRealMethod().when(defaultEventSourceManagerMock).cacheResource(any(), any()); - doAnswer( - invocation -> { - final var resourceId = (String) invocation.getArgument(0); - customResourceCache.cleanup(resourceId); - return null; - }) - .when(defaultEventSourceManagerMock) - .cleanup(any()); } @Test @@ -87,7 +73,8 @@ public void dispatchesEventsIfNoExecutionInProgress() { @Test public void skipProcessingIfLatestCustomResourceNotInCache() { Event event = prepareCREvent(); - customResourceCache.cleanup(event.getRelatedCustomResourceUid()); + when(resourceCache.getCustomResource(event.getRelatedCustomResourceID())) + .thenReturn(Optional.empty()); defaultEventHandler.handleEvent(event); @@ -96,7 +83,7 @@ public void skipProcessingIfLatestCustomResourceNotInCache() { @Test public void ifExecutionInProgressWaitsUntilItsFinished() throws InterruptedException { - String resourceUid = eventAlreadyUnderProcessing(); + CustomResourceID resourceUid = eventAlreadyUnderProcessing(); defaultEventHandler.handleEvent(nonCREvent(resourceUid)); @@ -106,7 +93,7 @@ public void ifExecutionInProgressWaitsUntilItsFinished() throws InterruptedExcep @Test public void buffersAllIncomingEventsWhileControllerInExecution() { - String resourceUid = eventAlreadyUnderProcessing(); + CustomResourceID resourceUid = eventAlreadyUnderProcessing(); defaultEventHandler.handleEvent(nonCREvent(resourceUid)); defaultEventHandler.handleEvent(prepareCREvent(resourceUid)); @@ -123,17 +110,16 @@ public void buffersAllIncomingEventsWhileControllerInExecution() { @Test public void cleanUpAfterDeleteEvent() { TestCustomResource customResource = testCustomResource(); - customResourceCache.cacheResource(customResource); + when(resourceCache.getCustomResource(CustomResourceID.fromResource(customResource))) + .thenReturn(Optional.of(customResource)); CustomResourceEvent event = - new CustomResourceEvent(Watcher.Action.DELETED, customResource, null); - String uid = customResource.getMetadata().getUid(); + new CustomResourceEvent(DELETED, customResource); defaultEventHandler.handleEvent(event); waitMinimalTime(); - - verify(defaultEventSourceManagerMock, times(1)).cleanup(uid); - assertThat(customResourceCache.getLatestResource(uid)).isNotPresent(); + verify(defaultEventSourceManagerMock, times(1)) + .cleanup(CustomResourceID.fromResource(customResource)); } @Test @@ -141,7 +127,7 @@ public void schedulesAnEventRetryOnException() { Event event = prepareCREvent(); TestCustomResource customResource = testCustomResource(); - ExecutionScope executionScope = new ExecutionScope(Arrays.asList(event), customResource, null); + ExecutionScope executionScope = new ExecutionScope(List.of(event), customResource, null); PostExecutionControl postExecutionControl = PostExecutionControl.exceptionDuringExecution(new RuntimeException("test")); @@ -155,8 +141,7 @@ public void schedulesAnEventRetryOnException() { public void executesTheControllerInstantlyAfterErrorIfEventsBuffered() { Event event = prepareCREvent(); TestCustomResource customResource = testCustomResource(); - customResource.getMetadata().setUid(event.getRelatedCustomResourceUid()); - ExecutionScope executionScope = new ExecutionScope(Arrays.asList(event), customResource, null); + overrideData(event.getRelatedCustomResourceID(), customResource); PostExecutionControl postExecutionControl = PostExecutionControl.exceptionDuringExecution(new RuntimeException("test")); @@ -186,7 +171,7 @@ public void successfulExecutionResetsTheRetry() { Event event = prepareCREvent(); TestCustomResource customResource = testCustomResource(); - customResource.getMetadata().setUid(event.getRelatedCustomResourceUid()); + overrideData(event.getRelatedCustomResourceID(), customResource); PostExecutionControl postExecutionControlWithException = PostExecutionControl.exceptionDuringExecution(new RuntimeException("test")); PostExecutionControl defaultDispatchControl = PostExecutionControl.defaultDispatch(); @@ -222,7 +207,7 @@ public void successfulExecutionResetsTheRetry() { @Test public void scheduleTimedEventIfInstructedByPostExecutionControl() { - var testDelay = 10000l; + var testDelay = 10000L; when(eventDispatcherMock.handleExecution(any())) .thenReturn(PostExecutionControl.defaultDispatch().withReSchedule(testDelay)); @@ -234,7 +219,7 @@ public void scheduleTimedEventIfInstructedByPostExecutionControl() { @Test public void reScheduleOnlyIfNotExecutedBufferedEvents() { - var testDelay = 10000l; + var testDelay = 10000L; when(eventDispatcherMock.handleExecution(any())) .thenReturn(PostExecutionControl.defaultDispatch().withReSchedule(testDelay)); @@ -261,7 +246,7 @@ private void waitMinimalTime() { } } - private String eventAlreadyUnderProcessing() { + private CustomResourceID eventAlreadyUnderProcessing() { when(eventDispatcherMock.handleExecution(any())) .then( (Answer) invocationOnMock -> { @@ -270,21 +255,26 @@ private String eventAlreadyUnderProcessing() { }); Event event = prepareCREvent(); defaultEventHandler.handleEvent(event); - return event.getRelatedCustomResourceUid(); + return event.getRelatedCustomResourceID(); } private CustomResourceEvent prepareCREvent() { - return prepareCREvent(UUID.randomUUID().toString()); + return prepareCREvent(new CustomResourceID(UUID.randomUUID().toString(), TEST_NAMESPACE)); } - private CustomResourceEvent prepareCREvent(String uid) { + private CustomResourceEvent prepareCREvent(CustomResourceID uid) { TestCustomResource customResource = testCustomResource(uid); - customResourceCache.cacheResource(customResource); - return new CustomResourceEvent(Watcher.Action.MODIFIED, customResource, null); + when(resourceCache.getCustomResource(eq(uid))).thenReturn(Optional.of(customResource)); + return new CustomResourceEvent(ResourceAction.UPDATED, customResource); } - private Event nonCREvent(String relatedCustomResourceUid) { - TimerEvent timerEvent = new TimerEvent(relatedCustomResourceUid, null); - return timerEvent; + private Event nonCREvent(CustomResourceID relatedCustomResourceUid) { + return new TimerEvent(relatedCustomResourceUid); } + + private void overrideData(CustomResourceID id, CustomResource applyTo) { + applyTo.getMetadata().setName(id.getName()); + applyTo.getMetadata().setNamespace(id.getNamespace().orElse(null)); + } + } diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/EventBufferTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/EventBufferTest.java index c9b414f52b..6dc16f8a69 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/EventBufferTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/EventBufferTest.java @@ -5,6 +5,7 @@ import org.junit.jupiter.api.Test; +import io.javaoperatorsdk.operator.processing.event.CustomResourceID; import io.javaoperatorsdk.operator.processing.event.Event; import io.javaoperatorsdk.operator.processing.event.internal.TimerEvent; @@ -14,17 +15,18 @@ class EventBufferTest { private EventBuffer eventBuffer = new EventBuffer(); - String uid = UUID.randomUUID().toString(); - Event testEvent1 = new TimerEvent(uid, null); - Event testEvent2 = new TimerEvent(uid, null); + String name = UUID.randomUUID().toString(); + CustomResourceID customResourceID = new CustomResourceID(name); + Event testEvent1 = new TimerEvent(customResourceID); + Event testEvent2 = new TimerEvent(customResourceID); @Test public void storesEvents() { eventBuffer.addEvent(testEvent1); eventBuffer.addEvent(testEvent2); - assertThat(eventBuffer.containsEvents(testEvent1.getRelatedCustomResourceUid())).isTrue(); - List events = eventBuffer.getAndRemoveEventsForExecution(uid); + assertThat(eventBuffer.containsEvents(testEvent1.getRelatedCustomResourceID())).isTrue(); + List events = eventBuffer.getAndRemoveEventsForExecution(customResourceID); assertThat(events).hasSize(2); } @@ -33,7 +35,7 @@ public void getsAndRemovesEvents() { eventBuffer.addEvent(testEvent1); eventBuffer.addEvent(testEvent2); - List events = eventBuffer.getAndRemoveEventsForExecution(uid); + List events = eventBuffer.getAndRemoveEventsForExecution(new CustomResourceID(name)); assertThat(events).hasSize(2); assertThat(events).contains(testEvent1, testEvent2); } @@ -43,7 +45,7 @@ public void checksIfThereAreStoredEvents() { eventBuffer.addEvent(testEvent1); eventBuffer.addEvent(testEvent2); - assertThat(eventBuffer.containsEvents(testEvent1.getRelatedCustomResourceUid())).isTrue(); + assertThat(eventBuffer.containsEvents(testEvent1.getRelatedCustomResourceID())).isTrue(); } @Test @@ -51,8 +53,8 @@ public void canClearEvents() { eventBuffer.addEvent(testEvent1); eventBuffer.addEvent(testEvent2); - eventBuffer.cleanup(uid); + eventBuffer.cleanup(customResourceID); - assertThat(eventBuffer.containsEvents(testEvent1.getRelatedCustomResourceUid())).isFalse(); + assertThat(eventBuffer.containsEvents(testEvent1.getRelatedCustomResourceID())).isFalse(); } } 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 2f9de4de82..2a8ad4b0b9 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 @@ -10,7 +10,6 @@ import org.mockito.ArgumentMatchers; import io.fabric8.kubernetes.client.CustomResource; -import io.fabric8.kubernetes.client.Watcher; import io.javaoperatorsdk.operator.Metrics; import io.javaoperatorsdk.operator.TestUtils; import io.javaoperatorsdk.operator.api.Context; @@ -22,7 +21,10 @@ import io.javaoperatorsdk.operator.api.config.ControllerConfiguration; import io.javaoperatorsdk.operator.processing.event.Event; import io.javaoperatorsdk.operator.processing.event.internal.CustomResourceEvent; +import io.javaoperatorsdk.operator.processing.event.internal.ResourceAction; +import static io.javaoperatorsdk.operator.processing.event.internal.ResourceAction.ADDED; +import static io.javaoperatorsdk.operator.processing.event.internal.ResourceAction.UPDATED; import static org.assertj.core.api.Assertions.assertThat; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; @@ -72,7 +74,7 @@ void setup() { void addFinalizerOnNewResource() { assertFalse(testCustomResource.hasFinalizer(DEFAULT_FINALIZER)); eventDispatcher.handleExecution( - executionScopeWithCREvent(Watcher.Action.ADDED, testCustomResource)); + executionScopeWithCREvent(ADDED, testCustomResource)); verify(controller, never()) .createOrUpdateResource(ArgumentMatchers.eq(testCustomResource), any()); verify(customResourceFacade, times(1)) @@ -85,7 +87,7 @@ void addFinalizerOnNewResource() { void callCreateOrUpdateOnNewResourceIfFinalizerSet() { testCustomResource.addFinalizer(DEFAULT_FINALIZER); eventDispatcher.handleExecution( - executionScopeWithCREvent(Watcher.Action.ADDED, testCustomResource)); + executionScopeWithCREvent(ADDED, testCustomResource)); verify(controller, times(1)) .createOrUpdateResource(ArgumentMatchers.eq(testCustomResource), any()); } @@ -98,7 +100,7 @@ void updatesOnlyStatusSubResourceIfFinalizerSet() { .thenReturn(UpdateControl.updateStatusSubResource(testCustomResource)); eventDispatcher.handleExecution( - executionScopeWithCREvent(Watcher.Action.ADDED, testCustomResource)); + executionScopeWithCREvent(ADDED, testCustomResource)); verify(customResourceFacade, times(1)).updateStatus(testCustomResource); verify(customResourceFacade, never()).replaceWithLock(any()); @@ -113,7 +115,7 @@ void updatesBothResourceAndStatusIfFinalizerSet() { when(customResourceFacade.replaceWithLock(testCustomResource)).thenReturn(testCustomResource); eventDispatcher.handleExecution( - executionScopeWithCREvent(Watcher.Action.MODIFIED, testCustomResource)); + executionScopeWithCREvent(UPDATED, testCustomResource)); verify(customResourceFacade, times(1)).replaceWithLock(testCustomResource); verify(customResourceFacade, times(1)).updateStatus(testCustomResource); @@ -124,7 +126,7 @@ void callCreateOrUpdateOnModifiedResourceIfFinalizerSet() { testCustomResource.addFinalizer(DEFAULT_FINALIZER); eventDispatcher.handleExecution( - executionScopeWithCREvent(Watcher.Action.MODIFIED, testCustomResource)); + executionScopeWithCREvent(UPDATED, testCustomResource)); verify(controller, times(1)) .createOrUpdateResource(ArgumentMatchers.eq(testCustomResource), any()); } @@ -136,7 +138,7 @@ void callsDeleteIfObjectHasFinalizerAndMarkedForDelete() { markForDeletion(testCustomResource); eventDispatcher.handleExecution( - executionScopeWithCREvent(Watcher.Action.MODIFIED, testCustomResource)); + executionScopeWithCREvent(UPDATED, testCustomResource)); verify(controller, times(1)).deleteResource(eq(testCustomResource), any()); } @@ -148,7 +150,7 @@ void callDeleteOnControllerIfMarkedForDeletionWhenNoFinalizerIsConfigured() { markForDeletion(testCustomResource); eventDispatcher.handleExecution( - executionScopeWithCREvent(Watcher.Action.MODIFIED, testCustomResource)); + executionScopeWithCREvent(UPDATED, testCustomResource)); verify(controller).deleteResource(eq(testCustomResource), any()); } @@ -158,7 +160,7 @@ void doNotCallDeleteIfMarkedForDeletionWhenFinalizerHasAlreadyBeenRemoved() { markForDeletion(testCustomResource); eventDispatcher.handleExecution( - executionScopeWithCREvent(Watcher.Action.MODIFIED, testCustomResource)); + executionScopeWithCREvent(UPDATED, testCustomResource)); verify(controller, never()).deleteResource(eq(testCustomResource), any()); } @@ -178,7 +180,7 @@ void doesNotAddFinalizerIfConfiguredNotTo() { configureToNotUseFinalizer(); eventDispatcher.handleExecution( - executionScopeWithCREvent(Watcher.Action.MODIFIED, testCustomResource)); + executionScopeWithCREvent(UPDATED, testCustomResource)); assertEquals(0, testCustomResource.getMetadata().getFinalizers().size()); } @@ -189,7 +191,7 @@ void removesDefaultFinalizerOnDeleteIfSet() { markForDeletion(testCustomResource); eventDispatcher.handleExecution( - executionScopeWithCREvent(Watcher.Action.MODIFIED, testCustomResource)); + executionScopeWithCREvent(UPDATED, testCustomResource)); assertEquals(0, testCustomResource.getMetadata().getFinalizers().size()); verify(customResourceFacade, times(1)).replaceWithLock(any()); @@ -204,7 +206,7 @@ void doesNotRemovesTheSetFinalizerIfTheDeleteNotMethodInstructsIt() { markForDeletion(testCustomResource); eventDispatcher.handleExecution( - executionScopeWithCREvent(Watcher.Action.MODIFIED, testCustomResource)); + executionScopeWithCREvent(UPDATED, testCustomResource)); assertEquals(1, testCustomResource.getMetadata().getFinalizers().size()); verify(customResourceFacade, never()).replaceWithLock(any()); @@ -218,7 +220,7 @@ void doesNotUpdateTheResourceIfNoUpdateUpdateControlIfFinalizerSet() { .thenReturn(UpdateControl.noUpdate()); eventDispatcher.handleExecution( - executionScopeWithCREvent(Watcher.Action.MODIFIED, testCustomResource)); + executionScopeWithCREvent(UPDATED, testCustomResource)); verify(customResourceFacade, never()).replaceWithLock(any()); verify(customResourceFacade, never()).updateStatus(testCustomResource); } @@ -230,7 +232,7 @@ void addsFinalizerIfNotMarkedForDeletionAndEmptyCustomResourceReturned() { .thenReturn(UpdateControl.noUpdate()); eventDispatcher.handleExecution( - executionScopeWithCREvent(Watcher.Action.MODIFIED, testCustomResource)); + executionScopeWithCREvent(UPDATED, testCustomResource)); assertEquals(1, testCustomResource.getMetadata().getFinalizers().size()); verify(customResourceFacade, times(1)).replaceWithLock(any()); @@ -242,7 +244,7 @@ void doesNotCallDeleteIfMarkedForDeletionButNotOurFinalizer() { markForDeletion(testCustomResource); eventDispatcher.handleExecution( - executionScopeWithCREvent(Watcher.Action.MODIFIED, testCustomResource)); + executionScopeWithCREvent(UPDATED, testCustomResource)); verify(customResourceFacade, never()).replaceWithLock(any()); verify(controller, never()).deleteResource(eq(testCustomResource), any()); @@ -252,9 +254,9 @@ void doesNotCallDeleteIfMarkedForDeletionButNotOurFinalizer() { void executeControllerRegardlessGenerationInNonGenerationAwareModeIfFinalizerSet() { testCustomResource.addFinalizer(DEFAULT_FINALIZER); eventDispatcher.handleExecution( - executionScopeWithCREvent(Watcher.Action.MODIFIED, testCustomResource)); + executionScopeWithCREvent(UPDATED, testCustomResource)); eventDispatcher.handleExecution( - executionScopeWithCREvent(Watcher.Action.MODIFIED, testCustomResource)); + executionScopeWithCREvent(UPDATED, testCustomResource)); verify(controller, times(2)).createOrUpdateResource(eq(testCustomResource), any()); } @@ -265,7 +267,7 @@ void propagatesRetryInfoToContextIfFinalizerSet() { eventDispatcher.handleExecution( new ExecutionScope( - Arrays.asList(), + List.of(), testCustomResource, new RetryInfo() { @Override @@ -295,12 +297,12 @@ void setReScheduleToPostExecutionControlFromUpdateControl() { when(controller.createOrUpdateResource(eq(testCustomResource), any())) .thenReturn( - UpdateControl.updateStatusSubResource(testCustomResource).withReSchedule(1000l)); + UpdateControl.updateStatusSubResource(testCustomResource).withReSchedule(1000L)); PostExecutionControl control = eventDispatcher.handleExecution( - executionScopeWithCREvent(Watcher.Action.ADDED, testCustomResource)); + executionScopeWithCREvent(ADDED, testCustomResource)); - assertThat(control.getReScheduleDelay().get()).isEqualTo(1000l); + assertThat(control.getReScheduleDelay().get()).isEqualTo(1000L); } private void markForDeletion(CustomResource customResource) { @@ -312,8 +314,8 @@ private void removeFinalizers(CustomResource customResource) { } public ExecutionScope executionScopeWithCREvent( - Watcher.Action action, CustomResource resource, Event... otherEvents) { - CustomResourceEvent event = new CustomResourceEvent(action, resource, null); + ResourceAction action, CustomResource resource, Event... otherEvents) { + CustomResourceEvent event = new CustomResourceEvent(action, resource); List eventList = new ArrayList<>(1 + otherEvents.length); eventList.add(event); eventList.addAll(Arrays.asList(otherEvents)); diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/DefaultEventSourceManagerTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/DefaultEventSourceManagerTest.java index 8862450d06..089215486e 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/DefaultEventSourceManagerTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/DefaultEventSourceManagerTest.java @@ -8,9 +8,7 @@ import io.fabric8.kubernetes.client.CustomResource; import io.javaoperatorsdk.operator.TestUtils; import io.javaoperatorsdk.operator.processing.DefaultEventHandler; -import io.javaoperatorsdk.operator.processing.KubernetesResourceUtils; -import static io.javaoperatorsdk.operator.processing.KubernetesResourceUtils.getUID; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatExceptionOfType; import static org.mockito.Mockito.eq; @@ -24,7 +22,7 @@ class DefaultEventSourceManagerTest { private DefaultEventHandler defaultEventHandlerMock = mock(DefaultEventHandler.class); private DefaultEventSourceManager defaultEventSourceManager = - new DefaultEventSourceManager(defaultEventHandlerMock, false); + new DefaultEventSourceManager(defaultEventHandlerMock); @Test public void registersEventSource() { @@ -34,7 +32,7 @@ public void registersEventSource() { Map registeredSources = defaultEventSourceManager.getRegisteredEventSources(); - assertThat(registeredSources.entrySet()).hasSize(1); + assertThat(registeredSources.entrySet()).hasSize(2); assertThat(registeredSources.get(CUSTOM_EVENT_SOURCE_NAME)).isEqualTo(eventSource); verify(eventSource, times(1)).setEventHandler(eq(defaultEventHandlerMock)); verify(eventSource, times(1)).start(); @@ -61,9 +59,8 @@ public void throwExceptionIfRegisteringEventSourceWithSameName() { defaultEventSourceManager.registerEventSource(CUSTOM_EVENT_SOURCE_NAME, eventSource); assertThatExceptionOfType(IllegalStateException.class) .isThrownBy( - () -> { - defaultEventSourceManager.registerEventSource(CUSTOM_EVENT_SOURCE_NAME, eventSource2); - }); + () -> defaultEventSourceManager.registerEventSource(CUSTOM_EVENT_SOURCE_NAME, + eventSource2)); } @Test @@ -73,9 +70,9 @@ public void deRegistersEventSources() { defaultEventSourceManager.registerEventSource(CUSTOM_EVENT_SOURCE_NAME, eventSource); defaultEventSourceManager.deRegisterCustomResourceFromEventSource( - CUSTOM_EVENT_SOURCE_NAME, getUID(customResource)); + CUSTOM_EVENT_SOURCE_NAME, CustomResourceID.fromResource(customResource)); verify(eventSource, times(1)) - .eventSourceDeRegisteredForResource(eq(KubernetesResourceUtils.getUID(customResource))); + .eventSourceDeRegisteredForResource(eq(CustomResourceID.fromResource(customResource))); } } diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/EventListTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/EventListTest.java index 2f3dba65a5..dc65981cff 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/EventListTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/EventListTest.java @@ -13,10 +13,11 @@ class EventListTest { @Test public void returnsLatestOfEventType() { - TimerEvent event2 = new TimerEvent("1", null); + TimerEvent event2 = new TimerEvent(new CustomResourceID("name1")); EventList eventList = new EventList( - Arrays.asList(mock(Event.class), new TimerEvent("2", null), event2, mock(Event.class))); + Arrays.asList(mock(Event.class), new TimerEvent(new CustomResourceID("name2")), event2, + mock(Event.class))); assertThat(eventList.getLatestOfType(TimerEvent.class).get()).isEqualTo(event2); } 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 ab41b833ff..bd0d2ed72d 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 @@ -7,7 +7,6 @@ import org.junit.jupiter.api.Test; import io.fabric8.kubernetes.api.model.KubernetesResourceList; -import io.fabric8.kubernetes.client.Watcher; import io.fabric8.kubernetes.client.dsl.MixedOperation; import io.fabric8.kubernetes.client.dsl.Resource; import io.javaoperatorsdk.operator.TestUtils; @@ -52,13 +51,13 @@ public void eventFilteredByCustomPredicate() { cr.getMetadata().setGeneration(1L); cr.getStatus().setConfigMapStatus("1"); - eventSource.eventReceived(Watcher.Action.MODIFIED, cr); + eventSource.eventReceived(ResourceAction.UPDATED, cr, null); verify(eventHandler, times(1)).handleEvent(any()); cr.getMetadata().setGeneration(1L); cr.getStatus().setConfigMapStatus("1"); - eventSource.eventReceived(Watcher.Action.MODIFIED, cr); + eventSource.eventReceived(ResourceAction.UPDATED, cr, cr); verify(eventHandler, times(1)).handleEvent(any()); } @@ -80,13 +79,18 @@ public void eventFilteredByCustomPredicateAndGenerationAware() { cr.getMetadata().setGeneration(1L); cr.getStatus().setConfigMapStatus("1"); - eventSource.eventReceived(Watcher.Action.MODIFIED, cr); + TestCustomResource cr2 = TestUtils.testCustomResource(); + cr.getMetadata().setFinalizers(List.of(FINALIZER)); + cr.getMetadata().setGeneration(2L); + cr.getStatus().setConfigMapStatus("1"); + + eventSource.eventReceived(ResourceAction.UPDATED, cr, cr2); verify(eventHandler, times(1)).handleEvent(any()); cr.getMetadata().setGeneration(1L); cr.getStatus().setConfigMapStatus("2"); - eventSource.eventReceived(Watcher.Action.MODIFIED, cr); + eventSource.eventReceived(ResourceAction.UPDATED, cr, cr); verify(eventHandler, times(1)).handleEvent(any()); } @@ -95,11 +99,9 @@ public void eventNotFilteredByCustomPredicateIfFinalizerIsRequired() { var config = new TestControllerConfig( FINALIZER, false, - (configuration, oldResource, newResource) -> { - return !Objects.equals( - oldResource.getStatus().getConfigMapStatus(), - newResource.getStatus().getConfigMapStatus()); - }); + (configuration, oldResource, newResource) -> !Objects.equals( + oldResource.getStatus().getConfigMapStatus(), + newResource.getStatus().getConfigMapStatus())); when(config.getConfigurationService().getObjectMapper()) .thenReturn(ConfigurationService.OBJECT_MAPPER); @@ -112,13 +114,13 @@ public void eventNotFilteredByCustomPredicateIfFinalizerIsRequired() { cr.getMetadata().setGeneration(1L); cr.getStatus().setConfigMapStatus("1"); - eventSource.eventReceived(Watcher.Action.MODIFIED, cr); + eventSource.eventReceived(ResourceAction.UPDATED, cr, cr); verify(eventHandler, times(1)).handleEvent(any()); cr.getMetadata().setGeneration(1L); cr.getStatus().setConfigMapStatus("1"); - eventSource.eventReceived(Watcher.Action.MODIFIED, cr); + eventSource.eventReceived(ResourceAction.UPDATED, cr, cr); verify(eventHandler, times(2)).handleEvent(any()); } diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/internal/CustomResourceEventSourceTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/internal/CustomResourceEventSourceTest.java index 727caf3be1..4bf7d1afd7 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/internal/CustomResourceEventSourceTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/internal/CustomResourceEventSourceTest.java @@ -7,7 +7,6 @@ import org.junit.jupiter.api.Test; import io.fabric8.kubernetes.api.model.KubernetesResourceList; -import io.fabric8.kubernetes.client.Watcher; import io.fabric8.kubernetes.client.dsl.MixedOperation; import io.fabric8.kubernetes.client.dsl.Resource; import io.javaoperatorsdk.operator.Metrics; @@ -41,13 +40,19 @@ public void setup() { @Test public void skipsEventHandlingIfGenerationNotIncreased() { - TestCustomResource customResource1 = TestUtils.testCustomResource(); - customResource1.getMetadata().setFinalizers(List.of(FINALIZER)); + TestCustomResource customResource = TestUtils.testCustomResource(); + customResource.getMetadata().setFinalizers(List.of(FINALIZER)); + customResource.getMetadata().setGeneration(2L); + + TestCustomResource oldCustomResource = TestUtils.testCustomResource(); + oldCustomResource.getMetadata().setFinalizers(List.of(FINALIZER)); - customResourceEventSource.eventReceived(Watcher.Action.MODIFIED, customResource1); + customResourceEventSource.eventReceived(ResourceAction.UPDATED, customResource, + oldCustomResource); verify(eventHandler, times(1)).handleEvent(any()); - customResourceEventSource.eventReceived(Watcher.Action.MODIFIED, customResource1); + customResourceEventSource.eventReceived(ResourceAction.UPDATED, customResource, + customResource); verify(eventHandler, times(1)).handleEvent(any()); } @@ -55,12 +60,14 @@ public void skipsEventHandlingIfGenerationNotIncreased() { public void dontSkipEventHandlingIfMarkedForDeletion() { TestCustomResource customResource1 = TestUtils.testCustomResource(); - customResourceEventSource.eventReceived(Watcher.Action.MODIFIED, customResource1); + customResourceEventSource.eventReceived(ResourceAction.UPDATED, customResource1, + customResource1); verify(eventHandler, times(1)).handleEvent(any()); // mark for deletion customResource1.getMetadata().setDeletionTimestamp(LocalDateTime.now().toString()); - customResourceEventSource.eventReceived(Watcher.Action.MODIFIED, customResource1); + customResourceEventSource.eventReceived(ResourceAction.UPDATED, customResource1, + customResource1); verify(eventHandler, times(2)).handleEvent(any()); } @@ -68,11 +75,13 @@ public void dontSkipEventHandlingIfMarkedForDeletion() { public void normalExecutionIfGenerationChanges() { TestCustomResource customResource1 = TestUtils.testCustomResource(); - customResourceEventSource.eventReceived(Watcher.Action.MODIFIED, customResource1); + customResourceEventSource.eventReceived(ResourceAction.UPDATED, customResource1, + customResource1); verify(eventHandler, times(1)).handleEvent(any()); customResource1.getMetadata().setGeneration(2L); - customResourceEventSource.eventReceived(Watcher.Action.MODIFIED, customResource1); + customResourceEventSource.eventReceived(ResourceAction.UPDATED, customResource1, + customResource1); verify(eventHandler, times(2)).handleEvent(any()); } @@ -84,10 +93,12 @@ public void handlesAllEventIfNotGenerationAware() { TestCustomResource customResource1 = TestUtils.testCustomResource(); - customResourceEventSource.eventReceived(Watcher.Action.MODIFIED, customResource1); + customResourceEventSource.eventReceived(ResourceAction.UPDATED, customResource1, + customResource1); verify(eventHandler, times(1)).handleEvent(any()); - customResourceEventSource.eventReceived(Watcher.Action.MODIFIED, customResource1); + customResourceEventSource.eventReceived(ResourceAction.UPDATED, customResource1, + customResource1); verify(eventHandler, times(2)).handleEvent(any()); } @@ -95,10 +106,12 @@ public void handlesAllEventIfNotGenerationAware() { public void eventNotMarkedForLastGenerationIfNoFinalizer() { TestCustomResource customResource1 = TestUtils.testCustomResource(); - customResourceEventSource.eventReceived(Watcher.Action.MODIFIED, customResource1); + customResourceEventSource.eventReceived(ResourceAction.UPDATED, customResource1, + customResource1); verify(eventHandler, times(1)).handleEvent(any()); - customResourceEventSource.eventReceived(Watcher.Action.MODIFIED, customResource1); + customResourceEventSource.eventReceived(ResourceAction.UPDATED, customResource1, + customResource1); verify(eventHandler, times(2)).handleEvent(any()); } diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/internal/CustomResourceSelectorTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/internal/CustomResourceSelectorTest.java index b6094a0ffe..7556d2412a 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/internal/CustomResourceSelectorTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/internal/CustomResourceSelectorTest.java @@ -4,6 +4,7 @@ import java.time.LocalDateTime; import java.time.format.DateTimeFormatter; import java.util.Date; +import java.util.Set; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicInteger; import java.util.function.Consumer; @@ -11,6 +12,7 @@ import org.awaitility.core.ConditionTimeoutException; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.mockito.internal.util.collections.Sets; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -42,6 +44,7 @@ public class CustomResourceSelectorTest { private static final Logger LOGGER = LoggerFactory.getLogger(CustomResourceSelectorTest.class); + public static final String NAMESPACE = "test"; KubernetesMockServer server; KubernetesClient client; @@ -112,11 +115,13 @@ void resourceWatchedByLabel() { new MyConfiguration(configurationService, "app=bar")); o2.start(); - client.resources(TestCustomResource.class).inNamespace("test").create(newMyResource("foo")); - client.resources(TestCustomResource.class).inNamespace("test").create(newMyResource("bar")); + client.resources(TestCustomResource.class).inNamespace(NAMESPACE).create(newMyResource("foo", + NAMESPACE)); + client.resources(TestCustomResource.class).inNamespace(NAMESPACE).create(newMyResource("bar", + NAMESPACE)); await() - .atMost(5, TimeUnit.SECONDS) + .atMost(325, TimeUnit.SECONDS) .pollInterval(100, TimeUnit.MILLISECONDS) .until(() -> c1.get() == 1 && c1err.get() == 0); await() @@ -133,9 +138,10 @@ void resourceWatchedByLabel() { } } - public TestCustomResource newMyResource(String app) { + public TestCustomResource newMyResource(String app, String namespace) { TestCustomResource resource = new TestCustomResource(); resource.setMetadata(new ObjectMetaBuilder().withName(app).addToLabels("app", app).build()); + resource.getMetadata().setNamespace(namespace); return resource; } @@ -159,13 +165,18 @@ public String getAssociatedControllerClassName() { return MyController.class.getCanonicalName(); } + @Override + public Set getNamespaces() { + return Sets.newSet(NAMESPACE); + } + @Override public ConfigurationService getConfigurationService() { return service; } } - @Controller + @Controller(namespaces = NAMESPACE) public static class MyController implements ResourceController { private final Consumer consumer; diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/internal/LabelSelectorParserTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/internal/LabelSelectorParserTest.java new file mode 100644 index 0000000000..3a619bbc19 --- /dev/null +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/internal/LabelSelectorParserTest.java @@ -0,0 +1,29 @@ +package io.javaoperatorsdk.operator.processing.event.internal; + +import org.junit.jupiter.api.Test; + +import static org.assertj.core.api.Assertions.assertThat; + +class LabelSelectorParserTest { + + @Test + public void nullParamReturnsEmptyMap() { + var res = LabelSelectorParser.parseSimpleLabelSelector(null); + assertThat(res).hasSize(0); + } + + @Test + public void emptyLabelSelectorReturnsEmptyMap() { + var res = LabelSelectorParser.parseSimpleLabelSelector(" "); + assertThat(res).hasSize(0); + } + + @Test + public void parseSimpleLabelSelector() { + var res = LabelSelectorParser.parseSimpleLabelSelector("app=foo"); + assertThat(res).hasSize(1).containsEntry("app", "foo"); + + res = LabelSelectorParser.parseSimpleLabelSelector("app=foo,owner=me"); + assertThat(res).hasSize(2).containsEntry("app", "foo").containsEntry("owner", "me"); + } +} diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/internal/TimerEventSourceTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/internal/TimerEventSourceTest.java index 892c422fc1..f7f2814255 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/internal/TimerEventSourceTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/internal/TimerEventSourceTest.java @@ -12,12 +12,11 @@ import org.junit.jupiter.api.Test; import io.javaoperatorsdk.operator.TestUtils; -import io.javaoperatorsdk.operator.processing.KubernetesResourceUtils; +import io.javaoperatorsdk.operator.processing.event.CustomResourceID; import io.javaoperatorsdk.operator.processing.event.Event; import io.javaoperatorsdk.operator.processing.event.EventHandler; import io.javaoperatorsdk.operator.sample.simple.TestCustomResource; -import static io.javaoperatorsdk.operator.processing.KubernetesResourceUtils.getUID; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatExceptionOfType; @@ -48,9 +47,9 @@ public void producesEventsPeriodically() { assertThat(eventHandlerMock.events) .hasSizeGreaterThan(2); assertThat(eventHandlerMock.events) - .allMatch(e -> e.getRelatedCustomResourceUid().equals(getUID(customResource))); - assertThat(eventHandlerMock.events) - .allMatch(e -> e.getEventSource().equals(timerEventSource)); + .allMatch(e -> e.getRelatedCustomResourceID() + .equals(CustomResourceID.fromResource(customResource))); + }); } @@ -61,7 +60,8 @@ public void deRegistersPeriodicalEventSources() { timerEventSource.schedule(customResource, INITIAL_DELAY, PERIOD); untilAsserted(() -> assertThat(eventHandlerMock.events).hasSizeGreaterThan(1)); - timerEventSource.eventSourceDeRegisteredForResource(getUID(customResource)); + timerEventSource + .eventSourceDeRegisteredForResource(CustomResourceID.fromResource(customResource)); int size = eventHandlerMock.events.size(); untilAsserted(() -> assertThat(eventHandlerMock.events).hasSize(size)); @@ -82,7 +82,7 @@ public void canCancelOnce() { TestCustomResource customResource = TestUtils.testCustomResource(); timerEventSource.scheduleOnce(customResource, PERIOD); - timerEventSource.cancelOnceSchedule(KubernetesResourceUtils.getUID(customResource)); + timerEventSource.cancelOnceSchedule(CustomResourceID.fromResource(customResource)); untilAsserted(() -> assertThat(eventHandlerMock.events).isEmpty()); } @@ -102,7 +102,8 @@ public void deRegistersOnceEventSources() { TestCustomResource customResource = TestUtils.testCustomResource(); timerEventSource.scheduleOnce(customResource, PERIOD); - timerEventSource.eventSourceDeRegisteredForResource(getUID(customResource)); + timerEventSource + .eventSourceDeRegisteredForResource(CustomResourceID.fromResource(customResource)); untilAsserted(() -> assertThat(eventHandlerMock.events).isEmpty()); } diff --git a/operator-framework/src/main/java/io/javaoperatorsdk/operator/config/runtime/AccumulativeMappingWriter.java b/operator-framework/src/main/java/io/javaoperatorsdk/operator/config/runtime/AccumulativeMappingWriter.java index 279167fc22..67cc71ac5a 100644 --- a/operator-framework/src/main/java/io/javaoperatorsdk/operator/config/runtime/AccumulativeMappingWriter.java +++ b/operator-framework/src/main/java/io/javaoperatorsdk/operator/config/runtime/AccumulativeMappingWriter.java @@ -18,7 +18,7 @@ */ class AccumulativeMappingWriter { - private Map mappings = new ConcurrentHashMap<>(); + private final Map mappings = new ConcurrentHashMap<>(); private final String resourcePath; private final ProcessingEnvironment processingEnvironment; diff --git a/operator-framework/src/main/java/io/javaoperatorsdk/operator/config/runtime/AnnotationConfiguration.java b/operator-framework/src/main/java/io/javaoperatorsdk/operator/config/runtime/AnnotationConfiguration.java index 04502d217f..ba4cf40250 100644 --- a/operator-framework/src/main/java/io/javaoperatorsdk/operator/config/runtime/AnnotationConfiguration.java +++ b/operator-framework/src/main/java/io/javaoperatorsdk/operator/config/runtime/AnnotationConfiguration.java @@ -1,8 +1,7 @@ package io.javaoperatorsdk.operator.config.runtime; -import java.util.Optional; import java.util.Set; -import java.util.function.Predicate; +import java.util.function.Function; import io.fabric8.kubernetes.client.CustomResource; import io.javaoperatorsdk.operator.ControllerUtils; @@ -17,12 +16,12 @@ public class AnnotationConfiguration implements ControllerConfiguration { private final ResourceController controller; - private final Optional annotation; + private final Controller annotation; private ConfigurationService service; public AnnotationConfiguration(ResourceController controller) { this.controller = controller; - this.annotation = Optional.ofNullable(controller.getClass().getAnnotation(Controller.class)); + this.annotation = controller.getClass().getAnnotation(Controller.class); } @Override @@ -32,15 +31,16 @@ public String getName() { @Override public String getFinalizer() { - return annotation - .map(Controller::finalizerName) - .filter(Predicate.not(String::isBlank)) - .orElse(ControllerUtils.getDefaultFinalizerName(getCRDName())); + if (annotation == null || annotation.finalizerName().isBlank()) { + return ControllerUtils.getDefaultFinalizerName(getCRDName()); + } else { + return annotation.finalizerName(); + } } @Override public boolean isGenerationAware() { - return annotation.map(Controller::generationAwareEventProcessing).orElse(true); + return valueOrDefault(annotation, Controller::generationAwareEventProcessing, true); } @Override @@ -50,12 +50,12 @@ public Class getCustomResourceClass() { @Override public Set getNamespaces() { - return Set.of(annotation.map(Controller::namespaces).orElse(new String[] {})); + return Set.of(valueOrDefault(annotation, Controller::namespaces, new String[] {})); } @Override public String getLabelSelector() { - return annotation.map(Controller::labelSelector).orElse(""); + return valueOrDefault(annotation, Controller::labelSelector, ""); } @Override @@ -78,9 +78,11 @@ public String getAssociatedControllerClassName() { public CustomResourceEventFilter getEventFilter() { CustomResourceEventFilter answer = null; - var filterTypes = annotation.map(Controller::eventFilters); - if (filterTypes.isPresent()) { - for (var filterType : filterTypes.get()) { + Class>[] filterTypes = + (Class>[]) valueOrDefault(annotation, Controller::eventFilters, + new Object[] {}); + if (filterTypes.length > 0) { + for (var filterType : filterTypes) { try { CustomResourceEventFilter filter = filterType.getConstructor().newInstance(); @@ -94,10 +96,18 @@ public CustomResourceEventFilter getEventFilter() { } } } - return answer != null ? answer : CustomResourceEventFilters.passthrough(); } + + public static T valueOrDefault(Controller controller, Function mapper, + T defaultValue) { + if (controller == null) { + return defaultValue; + } else { + return mapper.apply(controller); + } + } } diff --git a/operator-framework/src/main/java/io/javaoperatorsdk/operator/config/runtime/ClassMappingProvider.java b/operator-framework/src/main/java/io/javaoperatorsdk/operator/config/runtime/ClassMappingProvider.java index 1258904394..d76db8fbcd 100644 --- a/operator-framework/src/main/java/io/javaoperatorsdk/operator/config/runtime/ClassMappingProvider.java +++ b/operator-framework/src/main/java/io/javaoperatorsdk/operator/config/runtime/ClassMappingProvider.java @@ -36,7 +36,7 @@ static Map provide(final String resourcePath, T key, V value) { throw new IllegalStateException( String.format( "%s is not valid Mapping metadata, defined in %s", - clazzPair, url.toString())); + clazzPair, url)); } result.put( diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/EventSourceIT.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/EventSourceIT.java index d2954b86ee..eb556535a1 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/EventSourceIT.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/EventSourceIT.java @@ -35,10 +35,8 @@ public void receivingPeriodicEvents() { .pollInterval( EventSourceTestCustomResourceController.TIMER_PERIOD / 2, TimeUnit.MILLISECONDS) .untilAsserted( - () -> { - assertThat(TestUtils.getNumberOfExecutions(operator)) - .isGreaterThanOrEqualTo(4); - }); + () -> assertThat(TestUtils.getNumberOfExecutions(operator)) + .isGreaterThanOrEqualTo(4)); } public EventSourceTestCustomResource createTestCustomResource(String id) { diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/informereventsource/InformerEventSourceTestCustomResourceController.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/informereventsource/InformerEventSourceTestCustomResourceController.java index e5c44198b4..e5c34e1610 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/informereventsource/InformerEventSourceTestCustomResourceController.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/informereventsource/InformerEventSourceTestCustomResourceController.java @@ -27,7 +27,7 @@ public class InformerEventSourceTestCustomResourceController implements private static final Logger LOGGER = LoggerFactory.getLogger(InformerEventSourceTestCustomResourceController.class); - public static final String RELATED_RESOURCE_UID = "relatedResourceUID"; + public static final String RELATED_RESOURCE_UID = "relatedResourceName"; public static final String TARGET_CONFIG_MAP_KEY = "targetStatus"; private KubernetesClient kubernetesClient; diff --git a/pom.xml b/pom.xml index ac2324c699..7686a9591f 100644 --- a/pom.xml +++ b/pom.xml @@ -177,7 +177,6 @@ https://oss.sonatype.org/content/repositories/snapshots - @@ -363,6 +362,7 @@ release + org.apache.maven.plugins maven-surefire-plugin @@ -431,5 +431,4 @@ - From 00fdb361a99effbdee3e926add608716c1cb4de4 Mon Sep 17 00:00:00 2001 From: csviri Date: Tue, 12 Oct 2021 16:56:39 +0200 Subject: [PATCH 10/11] feat: delete reschedule --- .../operator/api/ControlBase.java | 22 ++++++++++++++ .../operator/api/DeleteControl.java | 29 +++++++++++++++++-- .../operator/api/ResourceController.java | 8 ++--- .../operator/api/UpdateControl.java | 19 +----------- .../processing/ConfiguredController.java | 11 ++----- .../operator/processing/EventDispatcher.java | 22 +++++++++----- .../operator/api/DeleteControlTest.java | 17 +++++++++++ .../processing/EventDispatcherTest.java | 19 ++++++++++-- .../simple/TestCustomResourceController.java | 2 +- .../simple/TestCustomResourceController.java | 2 +- .../ControllerImplemented2Interfaces.java | 2 +- ...rImplementedIntermediateAbstractClass.java | 2 +- .../MultilevelController.java | 2 +- .../sample/CustomServiceController.java | 2 +- 14 files changed, 110 insertions(+), 49 deletions(-) create mode 100644 operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/ControlBase.java create mode 100644 operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/DeleteControlTest.java diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/ControlBase.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/ControlBase.java new file mode 100644 index 0000000000..9e93edd6f8 --- /dev/null +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/ControlBase.java @@ -0,0 +1,22 @@ +package io.javaoperatorsdk.operator.api; + +import java.util.Optional; +import java.util.concurrent.TimeUnit; + +public abstract class ControlBase { + + private Long reScheduleDelay = null; + + public T withReSchedule(long delay) { + this.reScheduleDelay = delay; + return (T) this; + } + + public T withReSchedule(long delay, TimeUnit timeUnit) { + return withReSchedule(timeUnit.toMillis(delay)); + } + + public Optional getReScheduleDelay() { + return Optional.ofNullable(reScheduleDelay); + } +} diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/DeleteControl.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/DeleteControl.java index 2c721442cd..8a73be7f8f 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/DeleteControl.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/DeleteControl.java @@ -1,5 +1,30 @@ package io.javaoperatorsdk.operator.api; -public enum DeleteControl { - DEFAULT_DELETE, NO_FINALIZER_REMOVAL +public class DeleteControl extends ControlBase { + + private final boolean removeFinalizer; + + private DeleteControl(boolean removeFinalizer) { + this.removeFinalizer = removeFinalizer; + } + + public static DeleteControl defaultDelete() { + return new DeleteControl(true); + } + + public static DeleteControl noFinalizerRemoval() { + return new DeleteControl(false); + } + + public boolean isRemoveFinalizer() { + return removeFinalizer; + } + + @Override + public DeleteControl withReSchedule(long delay) { + if (removeFinalizer == true) { + throw new IllegalStateException("Cannot reschedule deleteResource if removing finalizer"); + } + return super.withReSchedule(delay); + } } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/ResourceController.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/ResourceController.java index 6d1adb84b7..edad82be27 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/ResourceController.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/ResourceController.java @@ -9,7 +9,7 @@ public interface ResourceController { * The implementation should delete the associated component(s). Note that this is method is * called when an object is marked for deletion. After it's executed the custom resource finalizer * is automatically removed by the framework; unless the return value is - * {@link DeleteControl#NO_FINALIZER_REMOVAL}, which indicates that the controller has determined + * {@link DeleteControl#noFinalizerRemoval()}, which indicates that the controller has determined * that the resource should not be deleted yet, in which case it is up to the controller to * restore the resource's status so that it's not marked for deletion anymore. * @@ -21,13 +21,13 @@ public interface ResourceController { * * @param resource the resource that is marked for deletion * @param context the context with which the operation is executed - * @return {@link DeleteControl#DEFAULT_DELETE} - so the finalizer is automatically removed after - * the call. {@link DeleteControl#NO_FINALIZER_REMOVAL} if you don't want to remove the + * @return {@link DeleteControl#defaultDelete()} - so the finalizer is automatically removed after + * the call. {@link DeleteControl#noFinalizerRemoval()} if you don't want to remove the * finalizer to indicate that the resource should not be deleted after all, in which case * the controller should restore the resource's state appropriately. */ default DeleteControl deleteResource(R resource, Context context) { - return DeleteControl.DEFAULT_DELETE; + return DeleteControl.defaultDelete(); } /** diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/UpdateControl.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/UpdateControl.java index d9e14a6c5a..d0dace98a3 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/UpdateControl.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/UpdateControl.java @@ -1,16 +1,12 @@ package io.javaoperatorsdk.operator.api; -import java.util.Optional; -import java.util.concurrent.TimeUnit; - import io.fabric8.kubernetes.client.CustomResource; -public class UpdateControl { +public class UpdateControl extends ControlBase> { private final T customResource; private final boolean updateStatusSubResource; private final boolean updateCustomResource; - private Long reScheduleDelay = null; private UpdateControl( T customResource, boolean updateStatusSubResource, boolean updateCustomResource) { @@ -47,19 +43,6 @@ public static UpdateControl noUpdate() { return new UpdateControl<>(null, false, false); } - public UpdateControl withReSchedule(long delay, TimeUnit timeUnit) { - return withReSchedule(timeUnit.toMillis(delay)); - } - - public UpdateControl withReSchedule(long delay) { - this.reScheduleDelay = delay; - return this; - } - - public Optional getReScheduleDelay() { - return Optional.ofNullable(reScheduleDelay); - } - public T getCustomResource() { return customResource; } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/ConfiguredController.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/ConfiguredController.java index 120a05bb65..3ce90ebb67 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/ConfiguredController.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/ConfiguredController.java @@ -52,15 +52,8 @@ public String controllerName() { } @Override - public String successTypeName(DeleteControl result) { - switch (result) { - case DEFAULT_DELETE: - return "delete"; - case NO_FINALIZER_REMOVAL: - return "finalizerNotRemoved"; - default: - return "unknown"; - } + public String successTypeName(DeleteControl deleteControl) { + return deleteControl.isRemoveFinalizer() ? "delete" : "finalizerNotRemoved"; } @Override 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 92347d5460..603c085c4d 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 @@ -8,11 +8,7 @@ import io.fabric8.kubernetes.client.KubernetesClientException; import io.fabric8.kubernetes.client.dsl.MixedOperation; import io.fabric8.kubernetes.client.dsl.Resource; -import io.javaoperatorsdk.operator.api.Context; -import io.javaoperatorsdk.operator.api.DefaultContext; -import io.javaoperatorsdk.operator.api.DeleteControl; -import io.javaoperatorsdk.operator.api.ResourceController; -import io.javaoperatorsdk.operator.api.UpdateControl; +import io.javaoperatorsdk.operator.api.*; import io.javaoperatorsdk.operator.api.config.ControllerConfiguration; import io.javaoperatorsdk.operator.processing.event.EventList; @@ -152,10 +148,16 @@ private PostExecutionControl createPostExecutionControl(R updatedCustomResour } else { postExecutionControl = PostExecutionControl.defaultDispatch(); } - updateControl.getReScheduleDelay().ifPresent(postExecutionControl::withReSchedule); + updatePostExecutionControlWithReschedule(postExecutionControl, updateControl); return postExecutionControl; } + private void updatePostExecutionControlWithReschedule( + PostExecutionControl postExecutionControl, + ControlBase controlBase) { + controlBase.getReScheduleDelay().ifPresent(postExecutionControl::withReSchedule); + } + private PostExecutionControl handleDelete(R resource, Context context) { log.debug( "Executing delete for resource: {} with version: {}", @@ -165,7 +167,9 @@ private PostExecutionControl handleDelete(R resource, Context context) { DeleteControl deleteControl = controller.deleteResource(resource, context); final var useFinalizer = configuration().useFinalizer(); if (useFinalizer) { - if (deleteControl == DeleteControl.DEFAULT_DELETE + // note that we don't reschedule here even if instructed. Removing finalizer means that + // cleanup is finished, nothing left to done + if (deleteControl.isRemoveFinalizer() && resource.hasFinalizer(configuration().getFinalizer())) { R customResource = removeFinalizer(resource); return PostExecutionControl.customResourceUpdated(customResource); @@ -177,7 +181,9 @@ private PostExecutionControl handleDelete(R resource, Context context) { getVersion(resource), deleteControl, useFinalizer); - return PostExecutionControl.defaultDispatch(); + PostExecutionControl postExecutionControl = PostExecutionControl.defaultDispatch(); + updatePostExecutionControlWithReschedule(postExecutionControl, deleteControl); + return postExecutionControl; } private void updateCustomResourceWithFinalizer(R resource) { diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/DeleteControlTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/DeleteControlTest.java new file mode 100644 index 0000000000..a5b613df8c --- /dev/null +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/DeleteControlTest.java @@ -0,0 +1,17 @@ +package io.javaoperatorsdk.operator.api; + +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.*; + +class DeleteControlTest { + + @Test + void cannotReScheduleForDefaultDelete() { + Assertions.assertThrows(IllegalStateException.class, () -> { + DeleteControl.defaultDelete().withReSchedule(1000L); + }); + } + +} 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 2a8ad4b0b9..002c532016 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 @@ -66,7 +66,7 @@ void setup() { when(controller.createOrUpdateResource(eq(testCustomResource), any())) .thenReturn(UpdateControl.updateCustomResource(testCustomResource)); when(controller.deleteResource(eq(testCustomResource), any())) - .thenReturn(DeleteControl.DEFAULT_DELETE); + .thenReturn(DeleteControl.defaultDelete()); when(customResourceFacade.replaceWithLock(any())).thenReturn(null); } @@ -202,7 +202,7 @@ void doesNotRemovesTheSetFinalizerIfTheDeleteNotMethodInstructsIt() { testCustomResource.addFinalizer(DEFAULT_FINALIZER); when(controller.deleteResource(eq(testCustomResource), any())) - .thenReturn(DeleteControl.NO_FINALIZER_REMOVAL); + .thenReturn(DeleteControl.noFinalizerRemoval()); markForDeletion(testCustomResource); eventDispatcher.handleExecution( @@ -305,6 +305,21 @@ void setReScheduleToPostExecutionControlFromUpdateControl() { assertThat(control.getReScheduleDelay().get()).isEqualTo(1000L); } + @Test + void reScheduleOnDeleteWithoutFinalizerRemoval() { + testCustomResource.addFinalizer(DEFAULT_FINALIZER); + markForDeletion(testCustomResource); + + when(controller.deleteResource(eq(testCustomResource), any())) + .thenReturn( + DeleteControl.noFinalizerRemoval().withReSchedule(1000L)); + + PostExecutionControl control = eventDispatcher.handleExecution( + executionScopeWithCREvent(UPDATED, testCustomResource)); + + assertThat(control.getReScheduleDelay().get()).isEqualTo(1000L); + } + private void markForDeletion(CustomResource customResource) { customResource.getMetadata().setDeletionTimestamp("2019-8-10"); } diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/sample/simple/TestCustomResourceController.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/sample/simple/TestCustomResourceController.java index 0af533d0c3..2cef04a09a 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/sample/simple/TestCustomResourceController.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/sample/simple/TestCustomResourceController.java @@ -57,7 +57,7 @@ public DeleteControl deleteResource( resource.getSpec().getConfigMapName(), resource.getMetadata().getName()); } - return DeleteControl.DEFAULT_DELETE; + return DeleteControl.defaultDelete(); } @Override diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/simple/TestCustomResourceController.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/simple/TestCustomResourceController.java index 4726a0fdfa..b6aabd1e67 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/simple/TestCustomResourceController.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/simple/TestCustomResourceController.java @@ -81,7 +81,7 @@ public DeleteControl deleteResource( resource.getSpec().getConfigMapName(), resource.getMetadata().getName()); } - return DeleteControl.DEFAULT_DELETE; + return DeleteControl.defaultDelete(); } @Override diff --git a/operator-framework/src/test/resources/compile-fixtures/ControllerImplemented2Interfaces.java b/operator-framework/src/test/resources/compile-fixtures/ControllerImplemented2Interfaces.java index 406d314091..e56943fa30 100644 --- a/operator-framework/src/test/resources/compile-fixtures/ControllerImplemented2Interfaces.java +++ b/operator-framework/src/test/resources/compile-fixtures/ControllerImplemented2Interfaces.java @@ -21,6 +21,6 @@ public UpdateControl createOrUpdateResource(MyCustomResource c @Override public DeleteControl deleteResource(MyCustomResource customResource, Context context) { - return DeleteControl.DEFAULT_DELETE; + return DeleteControl.defaultDelete(); } } diff --git a/operator-framework/src/test/resources/compile-fixtures/ControllerImplementedIntermediateAbstractClass.java b/operator-framework/src/test/resources/compile-fixtures/ControllerImplementedIntermediateAbstractClass.java index f048fb25f0..4f5619b4a4 100644 --- a/operator-framework/src/test/resources/compile-fixtures/ControllerImplementedIntermediateAbstractClass.java +++ b/operator-framework/src/test/resources/compile-fixtures/ControllerImplementedIntermediateAbstractClass.java @@ -18,6 +18,6 @@ public UpdateControl createOrUpdateResource public DeleteControl deleteResource(AbstractController.MyCustomResource customResource, Context context) { - return DeleteControl.DEFAULT_DELETE; + return DeleteControl.defaultDelete(); } } diff --git a/operator-framework/src/test/resources/compile-fixtures/MultilevelController.java b/operator-framework/src/test/resources/compile-fixtures/MultilevelController.java index 8c1b6a7ffb..d941a16d9d 100644 --- a/operator-framework/src/test/resources/compile-fixtures/MultilevelController.java +++ b/operator-framework/src/test/resources/compile-fixtures/MultilevelController.java @@ -22,7 +22,7 @@ public UpdateControl createOrUpdateResour public DeleteControl deleteResource(MultilevelController.MyCustomResource customResource, Context context) { - return DeleteControl.DEFAULT_DELETE; + return DeleteControl.defaultDelete(); } } diff --git a/samples/common/src/main/java/io/javaoperatorsdk/operator/sample/CustomServiceController.java b/samples/common/src/main/java/io/javaoperatorsdk/operator/sample/CustomServiceController.java index aacff140c9..67fb81dbd7 100644 --- a/samples/common/src/main/java/io/javaoperatorsdk/operator/sample/CustomServiceController.java +++ b/samples/common/src/main/java/io/javaoperatorsdk/operator/sample/CustomServiceController.java @@ -36,7 +36,7 @@ public CustomServiceController(KubernetesClient kubernetesClient) { @Override public DeleteControl deleteResource(CustomService resource, Context context) { log.info("Execution deleteResource for: {}", resource.getMetadata().getName()); - return DeleteControl.DEFAULT_DELETE; + return DeleteControl.defaultDelete(); } @Override From 8b398b66e671307f541592565b3533baa538479b Mon Sep 17 00:00:00 2001 From: csviri Date: Wed, 13 Oct 2021 09:10:55 +0200 Subject: [PATCH 11/11] fix: naming after review --- .../operator/api/BaseControl.java | 22 +++++++++++++++++++ .../operator/api/ControlBase.java | 22 ------------------- .../operator/api/DeleteControl.java | 6 ++--- .../operator/api/UpdateControl.java | 2 +- .../operator/processing/EventDispatcher.java | 4 ++-- .../operator/api/DeleteControlTest.java | 4 +--- .../processing/EventDispatcherTest.java | 4 ++-- 7 files changed, 31 insertions(+), 33 deletions(-) create mode 100644 operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/BaseControl.java delete mode 100644 operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/ControlBase.java diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/BaseControl.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/BaseControl.java new file mode 100644 index 0000000000..327bc34791 --- /dev/null +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/BaseControl.java @@ -0,0 +1,22 @@ +package io.javaoperatorsdk.operator.api; + +import java.util.Optional; +import java.util.concurrent.TimeUnit; + +public abstract class BaseControl { + + private Long scheduleDelay = null; + + public T rescheduleAfter(long delay) { + this.scheduleDelay = delay; + return (T) this; + } + + public T rescheduleAfter(long delay, TimeUnit timeUnit) { + return rescheduleAfter(timeUnit.toMillis(delay)); + } + + public Optional getScheduleDelay() { + return Optional.ofNullable(scheduleDelay); + } +} diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/ControlBase.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/ControlBase.java deleted file mode 100644 index 9e93edd6f8..0000000000 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/ControlBase.java +++ /dev/null @@ -1,22 +0,0 @@ -package io.javaoperatorsdk.operator.api; - -import java.util.Optional; -import java.util.concurrent.TimeUnit; - -public abstract class ControlBase { - - private Long reScheduleDelay = null; - - public T withReSchedule(long delay) { - this.reScheduleDelay = delay; - return (T) this; - } - - public T withReSchedule(long delay, TimeUnit timeUnit) { - return withReSchedule(timeUnit.toMillis(delay)); - } - - public Optional getReScheduleDelay() { - return Optional.ofNullable(reScheduleDelay); - } -} diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/DeleteControl.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/DeleteControl.java index 8a73be7f8f..b1ddac3df3 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/DeleteControl.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/DeleteControl.java @@ -1,6 +1,6 @@ package io.javaoperatorsdk.operator.api; -public class DeleteControl extends ControlBase { +public class DeleteControl extends BaseControl { private final boolean removeFinalizer; @@ -21,10 +21,10 @@ public boolean isRemoveFinalizer() { } @Override - public DeleteControl withReSchedule(long delay) { + public DeleteControl rescheduleAfter(long delay) { if (removeFinalizer == true) { throw new IllegalStateException("Cannot reschedule deleteResource if removing finalizer"); } - return super.withReSchedule(delay); + return super.rescheduleAfter(delay); } } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/UpdateControl.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/UpdateControl.java index d0dace98a3..18cc2ca4ee 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/UpdateControl.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/UpdateControl.java @@ -2,7 +2,7 @@ import io.fabric8.kubernetes.client.CustomResource; -public class UpdateControl extends ControlBase> { +public class UpdateControl extends BaseControl> { private final T customResource; private final boolean updateStatusSubResource; 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 603c085c4d..0e385dc553 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 @@ -154,8 +154,8 @@ private PostExecutionControl createPostExecutionControl(R updatedCustomResour private void updatePostExecutionControlWithReschedule( PostExecutionControl postExecutionControl, - ControlBase controlBase) { - controlBase.getReScheduleDelay().ifPresent(postExecutionControl::withReSchedule); + BaseControl baseControl) { + baseControl.getScheduleDelay().ifPresent(postExecutionControl::withReSchedule); } private PostExecutionControl handleDelete(R resource, Context context) { diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/DeleteControlTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/DeleteControlTest.java index a5b613df8c..645c997285 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/DeleteControlTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/DeleteControlTest.java @@ -3,14 +3,12 @@ import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; -import static org.junit.jupiter.api.Assertions.*; - class DeleteControlTest { @Test void cannotReScheduleForDefaultDelete() { Assertions.assertThrows(IllegalStateException.class, () -> { - DeleteControl.defaultDelete().withReSchedule(1000L); + DeleteControl.defaultDelete().rescheduleAfter(1000L); }); } 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 002c532016..00693e2f34 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 @@ -297,7 +297,7 @@ void setReScheduleToPostExecutionControlFromUpdateControl() { when(controller.createOrUpdateResource(eq(testCustomResource), any())) .thenReturn( - UpdateControl.updateStatusSubResource(testCustomResource).withReSchedule(1000L)); + UpdateControl.updateStatusSubResource(testCustomResource).rescheduleAfter(1000L)); PostExecutionControl control = eventDispatcher.handleExecution( executionScopeWithCREvent(ADDED, testCustomResource)); @@ -312,7 +312,7 @@ void reScheduleOnDeleteWithoutFinalizerRemoval() { when(controller.deleteResource(eq(testCustomResource), any())) .thenReturn( - DeleteControl.noFinalizerRemoval().withReSchedule(1000L)); + DeleteControl.noFinalizerRemoval().rescheduleAfter(1000L)); PostExecutionControl control = eventDispatcher.handleExecution( executionScopeWithCREvent(UPDATED, testCustomResource));