From 5ac06fe71f496c475f7b769d26ebba3b3c82bf63 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Attila=20M=C3=A9sz=C3=A1ros?= Date: Thu, 7 Mar 2024 14:31:36 +0100 Subject: [PATCH 1/5] feat: API to check if next reconciliatio is imminent MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Attila Mészáros --- .../javaoperatorsdk/operator/api/reconciler/Context.java | 9 +++++++++ .../operator/api/reconciler/DefaultContext.java | 7 +++++++ .../operator/processing/event/EventProcessor.java | 4 ++++ 3 files changed, 20 insertions(+) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/Context.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/Context.java index e157ed5fd7..682db6b6a8 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/Context.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/Context.java @@ -52,4 +52,13 @@ Optional getSecondaryResource(Class expectedType, * this context */ IndexedResourceCache

getPrimaryCache(); + + /** + * Returns true if a new reconciliation will be triggered rights after the current reconciliation is finished. + * This allows to optimize certain situations, for example like reconciler might not want to even do a status update if + * there is already a next reconciliation scheduled since that would do an additional status update, thus an additional + * API call (what is ideally reduced to minimal). + **/ + boolean isNextReconciliationImminent(); + } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/DefaultContext.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/DefaultContext.java index 2b0f20ef33..633daea6aa 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/DefaultContext.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/DefaultContext.java @@ -13,6 +13,7 @@ import io.javaoperatorsdk.operator.api.reconciler.dependent.managed.ManagedDependentResourceContext; import io.javaoperatorsdk.operator.processing.Controller; import io.javaoperatorsdk.operator.processing.event.EventSourceRetriever; +import io.javaoperatorsdk.operator.processing.event.ResourceID; public class DefaultContext

implements Context

{ @@ -45,6 +46,12 @@ public IndexedResourceCache

getPrimaryCache() { return controller.getEventSourceManager().getControllerResourceEventSource(); } + @Override + public boolean isNextReconciliationImminent() { + return controller.getEventProcessor() + .isNextReconciliationImminent(ResourceID.fromResource(primaryResource)); + } + @Override public Stream getSecondaryResourcesAsStream(Class expectedType) { return controller.getEventSourceManager().getResourceEventSourcesFor(expectedType).stream() diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventProcessor.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventProcessor.java index e94f5f3a7e..b0bf48802a 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventProcessor.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventProcessor.java @@ -408,6 +408,10 @@ public void start() throws OperatorException { handleAlreadyMarkedEvents(); } + public boolean isNextReconciliationImminent(ResourceID resourceID) { + return resourceStateManager.getOrCreate(resourceID).eventPresent(); + } + private void handleAlreadyMarkedEvents() { for (var state : resourceStateManager.resourcesWithEventPresent()) { handleMarkedEventForResource(state); From a3ebc93c1294e9f1995113be4b2dbd6843ada6e1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Attila=20M=C3=A9sz=C3=A1ros?= Date: Thu, 7 Mar 2024 15:20:13 +0100 Subject: [PATCH 2/5] Integration Test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Attila Mészáros --- .../operator/api/reconciler/Context.java | 9 +-- .../NextReconciliationImminentIT.java | 57 +++++++++++++++++++ ...tReconciliationImminentCustomResource.java | 18 ++++++ .../NextReconciliationImminentReconciler.java | 57 +++++++++++++++++++ .../NextReconciliationImminentStatus.java | 14 +++++ 5 files changed, 151 insertions(+), 4 deletions(-) create mode 100644 operator-framework/src/test/java/io/javaoperatorsdk/operator/NextReconciliationImminentIT.java create mode 100644 operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/nextreconciliationimminent/NextReconciliationImminentCustomResource.java create mode 100644 operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/nextreconciliationimminent/NextReconciliationImminentReconciler.java create mode 100644 operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/nextreconciliationimminent/NextReconciliationImminentStatus.java diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/Context.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/Context.java index 682db6b6a8..19f7c9ba8e 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/Context.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/Context.java @@ -54,10 +54,11 @@ Optional getSecondaryResource(Class expectedType, IndexedResourceCache

getPrimaryCache(); /** - * Returns true if a new reconciliation will be triggered rights after the current reconciliation is finished. - * This allows to optimize certain situations, for example like reconciler might not want to even do a status update if - * there is already a next reconciliation scheduled since that would do an additional status update, thus an additional - * API call (what is ideally reduced to minimal). + * Returns true if a new reconciliation will be triggered rights after the current reconciliation + * is finished. This allows to optimize certain situations, for example like reconciler might not + * want to even do a status update if there is already a next reconciliation scheduled since that + * would do an additional status update, thus an additional API call (what is ideally reduced to + * minimal). **/ boolean isNextReconciliationImminent(); diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/NextReconciliationImminentIT.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/NextReconciliationImminentIT.java new file mode 100644 index 0000000000..5d681929df --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/NextReconciliationImminentIT.java @@ -0,0 +1,57 @@ +package io.javaoperatorsdk.operator; + +import java.time.Duration; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; + +import io.fabric8.kubernetes.api.model.ObjectMetaBuilder; +import io.javaoperatorsdk.operator.junit.LocallyRunOperatorExtension; +import io.javaoperatorsdk.operator.sample.nextreconciliationimminent.NextReconciliationImminentCustomResource; +import io.javaoperatorsdk.operator.sample.nextreconciliationimminent.NextReconciliationImminentReconciler; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.awaitility.Awaitility.await; + +public class NextReconciliationImminentIT { + + public static final int WAIT_FOR_EVENT = 300; + public static final String TEST_RESOURCE_NAME = "test1"; + + @RegisterExtension + LocallyRunOperatorExtension extension = + LocallyRunOperatorExtension.builder() + .withReconciler(new NextReconciliationImminentReconciler()) + .build(); + + @Test + void skippingStatusUpdateWithNextReconciliationImminent() throws InterruptedException { + var resource = extension.create(testResource()); + + var reconciler = extension.getReconcilerOfType(NextReconciliationImminentReconciler.class); + await().untilAsserted(() -> assertThat(reconciler.isReconciliationWaiting()).isTrue()); + resource.getMetadata().getAnnotations().put("trigger", "" + System.currentTimeMillis()); + Thread.sleep(WAIT_FOR_EVENT); + extension.replace(resource); + reconciler.allowReconciliationToProceed(); + Thread.sleep(WAIT_FOR_EVENT); + // second event arrived + await().untilAsserted(() -> assertThat(reconciler.isReconciliationWaiting()).isTrue()); + reconciler.allowReconciliationToProceed(); + + await().pollDelay(Duration.ofMillis(300)).untilAsserted(() -> { + assertThat(extension.get(NextReconciliationImminentCustomResource.class, TEST_RESOURCE_NAME) + .getStatus().getUpdateNumber()).isEqualTo(1); + }); + } + + + NextReconciliationImminentCustomResource testResource() { + var res = new NextReconciliationImminentCustomResource(); + res.setMetadata(new ObjectMetaBuilder() + .withName(TEST_RESOURCE_NAME) + .build()); + return res; + } + +} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/nextreconciliationimminent/NextReconciliationImminentCustomResource.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/nextreconciliationimminent/NextReconciliationImminentCustomResource.java new file mode 100644 index 0000000000..fba4242925 --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/nextreconciliationimminent/NextReconciliationImminentCustomResource.java @@ -0,0 +1,18 @@ +package io.javaoperatorsdk.operator.sample.nextreconciliationimminent; + +import io.fabric8.kubernetes.api.model.Namespaced; +import io.fabric8.kubernetes.client.CustomResource; +import io.fabric8.kubernetes.model.annotation.Group; +import io.fabric8.kubernetes.model.annotation.ShortNames; +import io.fabric8.kubernetes.model.annotation.Version; + +@Group("sample.javaoperatorsdk") +@Version("v1") +@ShortNames("nri") +public class NextReconciliationImminentCustomResource + extends CustomResource + implements Namespaced { + + + +} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/nextreconciliationimminent/NextReconciliationImminentReconciler.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/nextreconciliationimminent/NextReconciliationImminentReconciler.java new file mode 100644 index 0000000000..1035e61476 --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/nextreconciliationimminent/NextReconciliationImminentReconciler.java @@ -0,0 +1,57 @@ +package io.javaoperatorsdk.operator.sample.nextreconciliationimminent; + +import java.util.concurrent.SynchronousQueue; +import java.util.concurrent.TimeUnit; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import io.javaoperatorsdk.operator.api.reconciler.Context; +import io.javaoperatorsdk.operator.api.reconciler.ControllerConfiguration; +import io.javaoperatorsdk.operator.api.reconciler.Reconciler; +import io.javaoperatorsdk.operator.api.reconciler.UpdateControl; + +@ControllerConfiguration(generationAwareEventProcessing = false) +public class NextReconciliationImminentReconciler + implements Reconciler { + + private static final Logger log = + LoggerFactory.getLogger(NextReconciliationImminentReconciler.class); + + private final SynchronousQueue queue = new SynchronousQueue<>(); + private volatile boolean reconciliationWaiting = false; + + @Override + public UpdateControl reconcile( + NextReconciliationImminentCustomResource resource, + Context context) throws InterruptedException { + + reconciliationWaiting = true; + // wait long enough to get manually allowed + queue.poll(30, TimeUnit.SECONDS); + log.info("Continue after wait"); + reconciliationWaiting = false; + + if (context.isNextReconciliationImminent()) { + return UpdateControl.noUpdate(); + } else { + if (resource.getStatus() == null) { + resource.setStatus(new NextReconciliationImminentStatus()); + } + resource.getStatus().setUpdateNumber(resource.getStatus().getUpdateNumber() + 1); + return UpdateControl.patchStatus(resource); + } + } + + public void allowReconciliationToProceed() { + try { + queue.put(true); + } catch (InterruptedException e) { + throw new RuntimeException(e); + } + } + + public boolean isReconciliationWaiting() { + return reconciliationWaiting; + } +} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/nextreconciliationimminent/NextReconciliationImminentStatus.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/nextreconciliationimminent/NextReconciliationImminentStatus.java new file mode 100644 index 0000000000..ee4528af7a --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/nextreconciliationimminent/NextReconciliationImminentStatus.java @@ -0,0 +1,14 @@ +package io.javaoperatorsdk.operator.sample.nextreconciliationimminent; + +public class NextReconciliationImminentStatus { + + private int updateNumber; + + public int getUpdateNumber() { + return updateNumber; + } + + public void setUpdateNumber(int updateNumber) { + this.updateNumber = updateNumber; + } +} From 9d317b077703ad9d2a6f4b25ed4db12956408876 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Attila=20M=C3=A9sz=C3=A1ros?= Date: Fri, 8 Mar 2024 09:26:57 +0100 Subject: [PATCH 3/5] Integration Test improvement MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Attila Mészáros --- .../operator/NextReconciliationImminentIT.java | 6 ++++-- .../NextReconciliationImminentReconciler.java | 2 +- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/NextReconciliationImminentIT.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/NextReconciliationImminentIT.java index 5d681929df..2dc530ff43 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/NextReconciliationImminentIT.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/NextReconciliationImminentIT.java @@ -30,16 +30,18 @@ void skippingStatusUpdateWithNextReconciliationImminent() throws InterruptedExce var reconciler = extension.getReconcilerOfType(NextReconciliationImminentReconciler.class); await().untilAsserted(() -> assertThat(reconciler.isReconciliationWaiting()).isTrue()); - resource.getMetadata().getAnnotations().put("trigger", "" + System.currentTimeMillis()); Thread.sleep(WAIT_FOR_EVENT); + + resource.getMetadata().getAnnotations().put("trigger", "" + System.currentTimeMillis()); extension.replace(resource); + reconciler.allowReconciliationToProceed(); Thread.sleep(WAIT_FOR_EVENT); // second event arrived await().untilAsserted(() -> assertThat(reconciler.isReconciliationWaiting()).isTrue()); reconciler.allowReconciliationToProceed(); - await().pollDelay(Duration.ofMillis(300)).untilAsserted(() -> { + await().pollDelay(Duration.ofMillis(WAIT_FOR_EVENT)).untilAsserted(() -> { assertThat(extension.get(NextReconciliationImminentCustomResource.class, TEST_RESOURCE_NAME) .getStatus().getUpdateNumber()).isEqualTo(1); }); diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/nextreconciliationimminent/NextReconciliationImminentReconciler.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/nextreconciliationimminent/NextReconciliationImminentReconciler.java index 1035e61476..960741e041 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/nextreconciliationimminent/NextReconciliationImminentReconciler.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/nextreconciliationimminent/NextReconciliationImminentReconciler.java @@ -28,7 +28,7 @@ public UpdateControl reconcile( reconciliationWaiting = true; // wait long enough to get manually allowed - queue.poll(30, TimeUnit.SECONDS); + queue.poll(120, TimeUnit.SECONDS); log.info("Continue after wait"); reconciliationWaiting = false; From 7f02bcc5cbc9a6b7be6a5dfc02a94555ea93daf6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Attila=20M=C3=A9sz=C3=A1ros?= Date: Fri, 8 Mar 2024 09:35:08 +0100 Subject: [PATCH 4/5] IT stability MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Attila Mészáros --- .../operator/NextReconciliationImminentIT.java | 7 +++++++ .../NextReconciliationImminentReconciler.java | 3 ++- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/NextReconciliationImminentIT.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/NextReconciliationImminentIT.java index 2dc530ff43..9f9b464a83 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/NextReconciliationImminentIT.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/NextReconciliationImminentIT.java @@ -4,6 +4,8 @@ import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.RegisterExtension; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import io.fabric8.kubernetes.api.model.ObjectMetaBuilder; import io.javaoperatorsdk.operator.junit.LocallyRunOperatorExtension; @@ -15,6 +17,9 @@ public class NextReconciliationImminentIT { + private static final Logger log = + LoggerFactory.getLogger(NextReconciliationImminentIT.class); + public static final int WAIT_FOR_EVENT = 300; public static final String TEST_RESOURCE_NAME = "test1"; @@ -34,6 +39,8 @@ void skippingStatusUpdateWithNextReconciliationImminent() throws InterruptedExce resource.getMetadata().getAnnotations().put("trigger", "" + System.currentTimeMillis()); extension.replace(resource); + Thread.sleep(WAIT_FOR_EVENT); + log.info("Made change to trigger event"); reconciler.allowReconciliationToProceed(); Thread.sleep(WAIT_FOR_EVENT); diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/nextreconciliationimminent/NextReconciliationImminentReconciler.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/nextreconciliationimminent/NextReconciliationImminentReconciler.java index 960741e041..be3ad70ee8 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/nextreconciliationimminent/NextReconciliationImminentReconciler.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/nextreconciliationimminent/NextReconciliationImminentReconciler.java @@ -25,7 +25,7 @@ public class NextReconciliationImminentReconciler public UpdateControl reconcile( NextReconciliationImminentCustomResource resource, Context context) throws InterruptedException { - + log.info("started reconciliation"); reconciliationWaiting = true; // wait long enough to get manually allowed queue.poll(120, TimeUnit.SECONDS); @@ -39,6 +39,7 @@ public UpdateControl reconcile( resource.setStatus(new NextReconciliationImminentStatus()); } resource.getStatus().setUpdateNumber(resource.getStatus().getUpdateNumber() + 1); + log.info("Patching status"); return UpdateControl.patchStatus(resource); } } From 6de9c8c8ce5a17c52dba47f87d3f0f70d1942d03 Mon Sep 17 00:00:00 2001 From: Chris Laprun Date: Mon, 11 Mar 2024 16:36:29 +0100 Subject: [PATCH 5/5] docs: improve Signed-off-by: Chris Laprun --- .../operator/api/reconciler/Context.java | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/Context.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/Context.java index 19f7c9ba8e..78592495ad 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/Context.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/Context.java @@ -51,14 +51,17 @@ Optional getSecondaryResource(Class expectedType, * @return the {@link IndexerResourceCache} associated with the associated {@link Reconciler} for * this context */ + @SuppressWarnings("unused") IndexedResourceCache

getPrimaryCache(); /** - * Returns true if a new reconciliation will be triggered rights after the current reconciliation - * is finished. This allows to optimize certain situations, for example like reconciler might not - * want to even do a status update if there is already a next reconciliation scheduled since that - * would do an additional status update, thus an additional API call (what is ideally reduced to - * minimal). + * Determines whether a new reconciliation will be triggered right after the current + * reconciliation is finished. This allows to optimize certain situations, helping avoid unneeded + * API calls. A reconciler might, for example, skip updating the status when it's known another + * reconciliation is already scheduled, which would in turn trigger another status update, thus + * rendering the current one moot. + * + * @return {@code true} is another reconciliation is already scheduled, {@code false} otherwise **/ boolean isNextReconciliationImminent();