From 4d511e7624a128840beb4a1ca3e305e9995ab830 Mon Sep 17 00:00:00 2001 From: Chris Laprun Date: Mon, 9 Jan 2023 17:09:02 +0100 Subject: [PATCH 01/25] feat: provide Kubernetes client version (#1706) Generate a Versions file to be consumed at runtime Co-authored-by: Andrea Peruffo --- operator-framework-core/pom.xml | 15 ++++++++++++++- .../operator/api/config/Versions.java | 10 ++++++++++ .../operator/api/config/Utils.java | 1 - .../operator/api/config/Version.java | 19 +++++++++++++------ .../operator/api/config/VersionTest.java | 17 +++++++++++++++++ 5 files changed, 54 insertions(+), 8 deletions(-) create mode 100644 operator-framework-core/src/main/java-templates/io/javaoperatorsdk/operator/api/config/Versions.java create mode 100644 operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/config/VersionTest.java diff --git a/operator-framework-core/pom.xml b/operator-framework-core/pom.xml index c8a4d099ec..0efb080c20 100644 --- a/operator-framework-core/pom.xml +++ b/operator-framework-core/pom.xml @@ -40,13 +40,26 @@ ${project.build.outputDirectory}/version.properties - ^git.build.(time|version)$ + ^git.build.time$ ^git.commit.id.(abbrev|full)$ git.branch full + + org.codehaus.mojo + templating-maven-plugin + 1.0.0 + + + filtering-java-templates + + filter-sources + + + + diff --git a/operator-framework-core/src/main/java-templates/io/javaoperatorsdk/operator/api/config/Versions.java b/operator-framework-core/src/main/java-templates/io/javaoperatorsdk/operator/api/config/Versions.java new file mode 100644 index 0000000000..8d67199510 --- /dev/null +++ b/operator-framework-core/src/main/java-templates/io/javaoperatorsdk/operator/api/config/Versions.java @@ -0,0 +1,10 @@ +package io.javaoperatorsdk.operator.api.config; + +public final class Versions { + + private Versions() {} + + protected static final String JOSDK = "${project.version}"; + protected static final String KUBERNETES_CLIENT = "${fabric8-client.version}"; + +} diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/Utils.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/Utils.java index c98ab895f7..bee5b96cbe 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/Utils.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/Utils.java @@ -63,7 +63,6 @@ public static Version loadFromProperties() { builtTime = Date.from(Instant.EPOCH); } return new Version( - properties.getProperty("git.build.version", "unknown"), properties.getProperty("git.commit.id.abbrev", "unknown"), builtTime); } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/Version.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/Version.java index 6bfb5bb2e5..d43d8aa1cf 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/Version.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/Version.java @@ -6,14 +6,11 @@ /** A class encapsulating the version information associated with this SDK instance. */ public class Version { - public static final Version UNKNOWN = new Version("unknown", "unknown", Date.from(Instant.EPOCH)); - - private final String sdk; + public static final Version UNKNOWN = new Version("unknown", Date.from(Instant.EPOCH)); private final String commit; private final Date builtTime; - public Version(String sdkVersion, String commit, Date builtTime) { - this.sdk = sdkVersion; + public Version(String commit, Date builtTime) { this.commit = commit; this.builtTime = builtTime; } @@ -24,7 +21,7 @@ public Version(String sdkVersion, String commit, Date builtTime) { * @return the SDK project version */ public String getSdkVersion() { - return sdk; + return Versions.JOSDK; } /** @@ -45,4 +42,14 @@ public String getCommit() { public Date getBuiltTime() { return builtTime; } + + /** + * Returns the version of the Fabric8 Kubernetes Client being used by this version of the SDK + * + * @return the Fabric8 Kubernetes Client version + */ + @SuppressWarnings("unused") + public String getKubernetesClientVersion() { + return Versions.KUBERNETES_CLIENT; + } } diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/config/VersionTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/config/VersionTest.java new file mode 100644 index 0000000000..2a6b8002e3 --- /dev/null +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/config/VersionTest.java @@ -0,0 +1,17 @@ +package io.javaoperatorsdk.operator.api.config; + +import org.junit.jupiter.api.Test; + +import static org.junit.Assert.assertEquals; + +public class VersionTest { + + @Test + void versionShouldReturnTheSameResultFromMavenAndProperties() { + String versionFromProperties = Utils.loadFromProperties().getSdkVersion(); + String versionFromMaven = Version.UNKNOWN.getSdkVersion(); + + assertEquals(versionFromProperties, versionFromMaven); + } + +} From 5e45fee514659600f2b2120319038de072433ab3 Mon Sep 17 00:00:00 2001 From: Chris Laprun Date: Mon, 9 Jan 2023 17:46:33 +0100 Subject: [PATCH 02/25] chore: update version to 4.3.0-SNAPSHOT (#1709) --- micrometer-support/pom.xml | 2 +- operator-framework-bom/pom.xml | 2 +- operator-framework-core/pom.xml | 2 +- operator-framework-junit5/pom.xml | 2 +- operator-framework/pom.xml | 2 +- pom.xml | 2 +- sample-operators/leader-election/pom.xml | 2 +- sample-operators/mysql-schema/pom.xml | 2 +- sample-operators/pom.xml | 2 +- sample-operators/tomcat-operator/pom.xml | 2 +- sample-operators/webpage/pom.xml | 2 +- 11 files changed, 11 insertions(+), 11 deletions(-) diff --git a/micrometer-support/pom.xml b/micrometer-support/pom.xml index 133435c6b2..ae1be1816f 100644 --- a/micrometer-support/pom.xml +++ b/micrometer-support/pom.xml @@ -5,7 +5,7 @@ java-operator-sdk io.javaoperatorsdk - 4.2.9-SNAPSHOT + 4.3.0-SNAPSHOT 4.0.0 diff --git a/operator-framework-bom/pom.xml b/operator-framework-bom/pom.xml index 134c57d456..de2b095675 100644 --- a/operator-framework-bom/pom.xml +++ b/operator-framework-bom/pom.xml @@ -5,7 +5,7 @@ io.javaoperatorsdk operator-framework-bom - 4.2.9-SNAPSHOT + 4.3.0-SNAPSHOT Operator SDK - Bill of Materials pom Java SDK for implementing Kubernetes operators diff --git a/operator-framework-core/pom.xml b/operator-framework-core/pom.xml index 0efb080c20..7c6e42da69 100644 --- a/operator-framework-core/pom.xml +++ b/operator-framework-core/pom.xml @@ -6,7 +6,7 @@ io.javaoperatorsdk java-operator-sdk - 4.2.9-SNAPSHOT + 4.3.0-SNAPSHOT ../pom.xml diff --git a/operator-framework-junit5/pom.xml b/operator-framework-junit5/pom.xml index 24b6378421..b6694c0e7d 100644 --- a/operator-framework-junit5/pom.xml +++ b/operator-framework-junit5/pom.xml @@ -5,7 +5,7 @@ java-operator-sdk io.javaoperatorsdk - 4.2.9-SNAPSHOT + 4.3.0-SNAPSHOT 4.0.0 diff --git a/operator-framework/pom.xml b/operator-framework/pom.xml index 1d0f8307e0..a3404648df 100644 --- a/operator-framework/pom.xml +++ b/operator-framework/pom.xml @@ -5,7 +5,7 @@ java-operator-sdk io.javaoperatorsdk - 4.2.9-SNAPSHOT + 4.3.0-SNAPSHOT 4.0.0 diff --git a/pom.xml b/pom.xml index 6bccd6bf96..ac62939e7e 100644 --- a/pom.xml +++ b/pom.xml @@ -5,7 +5,7 @@ io.javaoperatorsdk java-operator-sdk - 4.2.9-SNAPSHOT + 4.3.0-SNAPSHOT Operator SDK for Java Java SDK for implementing Kubernetes operators pom diff --git a/sample-operators/leader-election/pom.xml b/sample-operators/leader-election/pom.xml index 56093477c9..596136cf2d 100644 --- a/sample-operators/leader-election/pom.xml +++ b/sample-operators/leader-election/pom.xml @@ -7,7 +7,7 @@ io.javaoperatorsdk sample-operators - 4.2.9-SNAPSHOT + 4.3.0-SNAPSHOT sample-leader-election diff --git a/sample-operators/mysql-schema/pom.xml b/sample-operators/mysql-schema/pom.xml index cdcdf2a2bd..8ef3387779 100644 --- a/sample-operators/mysql-schema/pom.xml +++ b/sample-operators/mysql-schema/pom.xml @@ -7,7 +7,7 @@ io.javaoperatorsdk sample-operators - 4.2.9-SNAPSHOT + 4.3.0-SNAPSHOT sample-mysql-schema-operator diff --git a/sample-operators/pom.xml b/sample-operators/pom.xml index 3a1092e5bd..cc5b370995 100644 --- a/sample-operators/pom.xml +++ b/sample-operators/pom.xml @@ -7,7 +7,7 @@ io.javaoperatorsdk java-operator-sdk - 4.2.9-SNAPSHOT + 4.3.0-SNAPSHOT sample-operators diff --git a/sample-operators/tomcat-operator/pom.xml b/sample-operators/tomcat-operator/pom.xml index 22f8295867..6a38e2d93b 100644 --- a/sample-operators/tomcat-operator/pom.xml +++ b/sample-operators/tomcat-operator/pom.xml @@ -7,7 +7,7 @@ io.javaoperatorsdk sample-operators - 4.2.9-SNAPSHOT + 4.3.0-SNAPSHOT sample-tomcat-operator diff --git a/sample-operators/webpage/pom.xml b/sample-operators/webpage/pom.xml index fd43acc1b3..76485ad707 100644 --- a/sample-operators/webpage/pom.xml +++ b/sample-operators/webpage/pom.xml @@ -7,7 +7,7 @@ io.javaoperatorsdk sample-operators - 4.2.9-SNAPSHOT + 4.3.0-SNAPSHOT sample-webpage-operator From ad0267b0e2529b0b863d90ba29d4beb9591d873a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Attila=20M=C3=A9sz=C3=A1ros?= Date: Tue, 10 Jan 2023 09:52:56 +0100 Subject: [PATCH 03/25] feat: dependent resource in the condition instead of resource (#1690) --- docs/documentation/v4-3-migration.md | 36 +++++++++++++++++++ .../workflow/AbstractWorkflowExecutor.java | 11 +----- .../dependent/workflow/Condition.java | 7 ++-- .../ControllerConfigurationOverriderTest.java | 4 ++- .../bulkdependent/SampleBulkCondition.java | 14 +++++--- .../dependent/StatefulSetReadyCondition.java | 12 +++++-- .../ConfigMapDeletePostCondition.java | 7 ++-- .../ConfigMapReconcileCondition.java | 6 +++- .../DeploymentReadyCondition.java | 17 +++++---- .../sample/ExposedIngressCondition.java | 5 ++- 10 files changed, 85 insertions(+), 34 deletions(-) create mode 100644 docs/documentation/v4-3-migration.md diff --git a/docs/documentation/v4-3-migration.md b/docs/documentation/v4-3-migration.md new file mode 100644 index 0000000000..77979fe3ba --- /dev/null +++ b/docs/documentation/v4-3-migration.md @@ -0,0 +1,36 @@ +--- +title: Migrating from v4.2 to v4.3 +description: Migrating from v4.2 to v4.3 +layout: docs +permalink: /docs/v4-3-migration +--- + +# Migrating from v4.2 to v4.3 + +## Condition API Change + +In Workflows the target of the condition was the managed resource itself, not a dependent resource. This changed, from +not the API contains the dependent resource. + +New API: + +```java +public interface Condition { + + boolean isMet(DependentResource dependentResource, P primary, Context

context); + +} +``` + +Former API: + +```java +public interface Condition { + + boolean isMet(P primary, R secondary, Context

context); + +} +``` + +Migration is trivial. Since the secondary resource can be accessed from the dependent resource. So to access the secondary +resource just use `dependentResource.getSecondaryResource(primary,context)`. diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/AbstractWorkflowExecutor.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/AbstractWorkflowExecutor.java index 04576098b2..510edf87af 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/AbstractWorkflowExecutor.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/AbstractWorkflowExecutor.java @@ -14,7 +14,6 @@ import io.fabric8.kubernetes.api.model.HasMetadata; import io.javaoperatorsdk.operator.api.reconciler.Context; import io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResource; -import io.javaoperatorsdk.operator.processing.dependent.BulkDependentResource; @SuppressWarnings("rawtypes") public abstract class AbstractWorkflowExecutor

{ @@ -103,15 +102,7 @@ protected synchronized void handleNodeExecutionFinish( @SuppressWarnings("unchecked") protected boolean isConditionMet(Optional> condition, DependentResource dependentResource) { - if (condition.isEmpty()) { - return true; - } - var resources = dependentResource instanceof BulkDependentResource - ? ((BulkDependentResource) dependentResource).getSecondaryResources(primary, context) - : dependentResource.getSecondaryResource(primary, context).orElse(null); - - return condition.map(c -> c.isMet(primary, - (R) resources, context)) + return condition.map(c -> c.isMet(dependentResource, primary, context)) .orElse(true); } } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/Condition.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/Condition.java index 68ed5530ec..87690ed69b 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/Condition.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/Condition.java @@ -2,6 +2,7 @@ import io.fabric8.kubernetes.api.model.HasMetadata; import io.javaoperatorsdk.operator.api.reconciler.Context; +import io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResource; public interface Condition { @@ -10,12 +11,10 @@ public interface Condition { * {@link io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResource} based on the * observed cluster state. * + * @param dependentResource for which the condition applies to * @param primary the primary resource being considered - * @param secondary the secondary resource associated with the specified primary resource or - * {@code null} if no such secondary resource exists (for example because it's been - * deleted) * @param context the current reconciliation {@link Context} * @return {@code true} if the condition holds, {@code false} otherwise */ - boolean isMet(P primary, R secondary, Context

context); + boolean isMet(DependentResource dependentResource, P primary, Context

context); } diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/config/ControllerConfigurationOverriderTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/config/ControllerConfigurationOverriderTest.java index 7626d8bfd0..71be6a2cd4 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/config/ControllerConfigurationOverriderTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/config/ControllerConfigurationOverriderTest.java @@ -362,7 +362,9 @@ public UpdateControl reconcile(ConfigMap resource, Context private static class TestCondition implements Condition { @Override - public boolean isMet(ConfigMap primary, ConfigMap secondary, Context context) { + public boolean isMet(DependentResource dependentResource, + ConfigMap primary, + Context context) { return true; } } diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/bulkdependent/SampleBulkCondition.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/bulkdependent/SampleBulkCondition.java index cff0bd10d1..74048e7b54 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/bulkdependent/SampleBulkCondition.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/bulkdependent/SampleBulkCondition.java @@ -1,21 +1,25 @@ package io.javaoperatorsdk.operator.sample.bulkdependent; -import java.util.Map; - import io.fabric8.kubernetes.api.model.ConfigMap; import io.javaoperatorsdk.operator.api.reconciler.Context; +import io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResource; import io.javaoperatorsdk.operator.processing.dependent.workflow.Condition; public class SampleBulkCondition - implements Condition, BulkDependentTestCustomResource> { + implements Condition { // We use ConfigMaps here just to show how to check some properties of resources managed by a // BulkDependentResource. In real life example this would be rather based on some status of those // resources, like Pods. @Override - public boolean isMet(BulkDependentTestCustomResource primary, Map secondary, + public boolean isMet( + DependentResource dependentResource, + BulkDependentTestCustomResource primary, Context context) { - return secondary.values().stream().noneMatch(cm -> cm.getData().isEmpty()); + + var resources = ((CRUDConfigMapBulkDependentResource) dependentResource) + .getSecondaryResources(primary, context); + return resources.values().stream().noneMatch(cm -> cm.getData().isEmpty()); } } diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/complexdependent/dependent/StatefulSetReadyCondition.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/complexdependent/dependent/StatefulSetReadyCondition.java index 9025a5a24b..894cab310a 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/complexdependent/dependent/StatefulSetReadyCondition.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/complexdependent/dependent/StatefulSetReadyCondition.java @@ -2,6 +2,7 @@ import io.fabric8.kubernetes.api.model.apps.StatefulSet; import io.javaoperatorsdk.operator.api.reconciler.Context; +import io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResource; import io.javaoperatorsdk.operator.processing.dependent.workflow.Condition; import io.javaoperatorsdk.operator.sample.complexdependent.ComplexDependentCustomResource; @@ -9,10 +10,15 @@ public class StatefulSetReadyCondition implements Condition { @Override - public boolean isMet(ComplexDependentCustomResource primary, StatefulSet secondary, + public boolean isMet( + DependentResource dependentResource, + ComplexDependentCustomResource primary, Context context) { - var readyReplicas = secondary.getStatus().getReadyReplicas(); - return readyReplicas != null && readyReplicas > 0; + return dependentResource.getSecondaryResource(primary, context).map(secondary -> { + var readyReplicas = secondary.getStatus().getReadyReplicas(); + return readyReplicas != null && readyReplicas > 0; + }) + .orElse(false); } } diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowallfeature/ConfigMapDeletePostCondition.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowallfeature/ConfigMapDeletePostCondition.java index e7d09a5e95..da6a693fe4 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowallfeature/ConfigMapDeletePostCondition.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowallfeature/ConfigMapDeletePostCondition.java @@ -5,6 +5,7 @@ import io.fabric8.kubernetes.api.model.ConfigMap; import io.javaoperatorsdk.operator.api.reconciler.Context; +import io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResource; import io.javaoperatorsdk.operator.processing.dependent.workflow.Condition; public class ConfigMapDeletePostCondition @@ -14,9 +15,11 @@ public class ConfigMapDeletePostCondition @Override public boolean isMet( - WorkflowAllFeatureCustomResource primary, ConfigMap secondary, + DependentResource dependentResource, + WorkflowAllFeatureCustomResource primary, Context context) { - var configMapDeleted = secondary == null; + + var configMapDeleted = dependentResource.getSecondaryResource(primary, context).isEmpty(); log.debug("Config Map Deleted: {}", configMapDeleted); return configMapDeleted; } diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowallfeature/ConfigMapReconcileCondition.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowallfeature/ConfigMapReconcileCondition.java index c413b5f03c..b3d9d7a541 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowallfeature/ConfigMapReconcileCondition.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowallfeature/ConfigMapReconcileCondition.java @@ -2,14 +2,18 @@ import io.fabric8.kubernetes.api.model.ConfigMap; import io.javaoperatorsdk.operator.api.reconciler.Context; +import io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResource; import io.javaoperatorsdk.operator.processing.dependent.workflow.Condition; public class ConfigMapReconcileCondition implements Condition { @Override - public boolean isMet(WorkflowAllFeatureCustomResource primary, ConfigMap secondary, + public boolean isMet( + DependentResource dependentResource, + WorkflowAllFeatureCustomResource primary, Context context) { + return primary.getSpec().isCreateConfigMap(); } } diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowallfeature/DeploymentReadyCondition.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowallfeature/DeploymentReadyCondition.java index 8681097962..0e6f5d8580 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowallfeature/DeploymentReadyCondition.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowallfeature/DeploymentReadyCondition.java @@ -2,18 +2,21 @@ import io.fabric8.kubernetes.api.model.apps.Deployment; import io.javaoperatorsdk.operator.api.reconciler.Context; +import io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResource; import io.javaoperatorsdk.operator.processing.dependent.workflow.Condition; public class DeploymentReadyCondition implements Condition { @Override - public boolean isMet(WorkflowAllFeatureCustomResource primary, Deployment deployment, + public boolean isMet( + DependentResource dependentResource, + WorkflowAllFeatureCustomResource primary, Context context) { - if (deployment == null) { - return false; - } - var readyReplicas = deployment.getStatus().getReadyReplicas(); - - return readyReplicas != null && deployment.getSpec().getReplicas().equals(readyReplicas); + return dependentResource.getSecondaryResource(primary, context) + .map(deployment -> { + var readyReplicas = deployment.getStatus().getReadyReplicas(); + return readyReplicas != null && deployment.getSpec().getReplicas().equals(readyReplicas); + }) + .orElse(false); } } diff --git a/sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/ExposedIngressCondition.java b/sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/ExposedIngressCondition.java index 5eb0135586..c2c0d0b423 100644 --- a/sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/ExposedIngressCondition.java +++ b/sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/ExposedIngressCondition.java @@ -2,11 +2,14 @@ import io.fabric8.kubernetes.api.model.networking.v1.Ingress; import io.javaoperatorsdk.operator.api.reconciler.Context; +import io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResource; import io.javaoperatorsdk.operator.processing.dependent.workflow.Condition; public class ExposedIngressCondition implements Condition { + @Override - public boolean isMet(WebPage primary, Ingress secondary, Context context) { + public boolean isMet(DependentResource dependentResource, + WebPage primary, Context context) { return primary.getSpec().getExposed(); } } From e0f6ae65d0e9e8f9c9bb40733cc34a5fcb1fb61e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Attila=20M=C3=A9sz=C3=A1ros?= Date: Fri, 20 Jan 2023 16:26:56 +0100 Subject: [PATCH 04/25] chore: fabric8 client v6.3.1 (#1669) --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index ac62939e7e..dfc9aa1a67 100644 --- a/pom.xml +++ b/pom.xml @@ -43,7 +43,7 @@ https://sonarcloud.io 5.9.1 - 6.2.0 + 6.3.1 1.7.36 2.19.0 5.2.0 From ec85153bea9e5416c05579f8a4b4840c691299f8 Mon Sep 17 00:00:00 2001 From: Chris Laprun Date: Mon, 23 Jan 2023 11:11:02 +0100 Subject: [PATCH 05/25] feat: expose setting namespaces directly to subclasses (#1728) --- .../kubernetes/KubernetesDependentResourceConfig.java | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/KubernetesDependentResourceConfig.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/KubernetesDependentResourceConfig.java index 02a3c8dd78..44a3b0a7bd 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/KubernetesDependentResourceConfig.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/KubernetesDependentResourceConfig.java @@ -93,6 +93,12 @@ public ResourceDiscriminator getResourceDiscriminator() { public void changeNamespaces(Set namespaces) { if (!wereNamespacesConfigured()) { this.namespacesWereConfigured = true; + setNamespaces(namespaces); + } + } + + protected void setNamespaces(Set namespaces) { + if (namespaces != null && !namespaces.isEmpty()) { this.namespaces = namespaces; } } From 79728bbe5338885e10778f725d0f6dd0267467a5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Attila=20M=C3=A9sz=C3=A1ros?= Date: Tue, 7 Feb 2023 14:50:17 +0100 Subject: [PATCH 06/25] feat: graceful shutdown (#1735) --- .../io/javaoperatorsdk/operator/Operator.java | 37 +++++++-- .../api/config/ConfigurationService.java | 4 + .../api/config/ExecutorServiceManager.java | 50 +++++++----- .../processing/event/EventProcessor.java | 7 ++ .../operator/api/config/VersionTest.java | 2 +- .../processing/event/EventProcessorTest.java | 27 +++++++ .../event/ReconciliationDispatcherTest.java | 1 + .../operator/GracefulStopIT.java | 79 +++++++++++++++++++ .../GracefulStopTestCustomResource.java | 16 ++++ .../GracefulStopTestCustomResourceSpec.java | 14 ++++ .../GracefulStopTestCustomResourceStatus.java | 7 ++ .../GracefulStopTestReconciler.java | 34 ++++++++ 12 files changed, 252 insertions(+), 26 deletions(-) create mode 100644 operator-framework/src/test/java/io/javaoperatorsdk/operator/GracefulStopIT.java create mode 100644 operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/gracefulstop/GracefulStopTestCustomResource.java create mode 100644 operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/gracefulstop/GracefulStopTestCustomResourceSpec.java create mode 100644 operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/gracefulstop/GracefulStopTestCustomResourceStatus.java create mode 100644 operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/gracefulstop/GracefulStopTestReconciler.java diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/Operator.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/Operator.java index 613ea622b5..d18b340f36 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/Operator.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/Operator.java @@ -1,5 +1,6 @@ package io.javaoperatorsdk.operator; +import java.time.Duration; import java.util.HashSet; import java.util.Optional; import java.util.Set; @@ -77,14 +78,31 @@ public Operator(KubernetesClient kubernetesClient, ConfigurationService configur } /** - * Adds a shutdown hook that automatically calls {@link #stop()} when the app shuts down. + * Uses {@link ConfigurationService#getTerminationTimeoutSeconds()} for graceful shutdown timeout + * + * @deprecated use the overloaded version with graceful shutdown timeout parameter. * - * @deprecated This feature should not be used anymore */ @Deprecated(forRemoval = true) public void installShutdownHook() { + installShutdownHook( + Duration.ofSeconds(ConfigurationServiceProvider.instance().getTerminationTimeoutSeconds())); + } + + /** + * Adds a shutdown hook that automatically calls {@link #stop()} when the app shuts down. Note + * that graceful shutdown is usually not needed, but your {@link Reconciler} implementations might + * require it. + *

+ * Note that you might want to tune "terminationGracePeriodSeconds" for the Pod running the + * controller. + * + * @param gracefulShutdownTimeout timeout to wait for executor threads to complete actual + * reconciliations + */ + public void installShutdownHook(Duration gracefulShutdownTimeout) { if (!leaderElectionManager.isLeaderElectionEnabled()) { - Runtime.getRuntime().addShutdownHook(new Thread(this::stop)); + Runtime.getRuntime().addShutdownHook(new Thread(() -> stop(gracefulShutdownTimeout))); } else { log.warn("Leader election is on, shutdown hook will not be installed."); } @@ -126,14 +144,16 @@ public synchronized void start() { } } - @Override - public void stop() throws OperatorException { + public void stop(Duration gracefulShutdownTimeout) throws OperatorException { + if (!started) { + return; + } final var configurationService = ConfigurationServiceProvider.instance(); log.info( "Operator SDK {} is shutting down...", configurationService.getVersion().getSdkVersion()); controllerManager.stop(); - ExecutorServiceManager.stop(); + ExecutorServiceManager.stop(gracefulShutdownTimeout); leaderElectionManager.stop(); if (configurationService.closeClientOnStop()) { kubernetesClient.close(); @@ -142,6 +162,11 @@ public void stop() throws OperatorException { started = false; } + @Override + public void stop() throws OperatorException { + stop(Duration.ZERO); + } + /** * Add a registration requests for the specified reconciler with this operator. The effective * registration of the reconciler is delayed till the operator is started. diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationService.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationService.java index d7dccd5bb4..53f2ae4176 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationService.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationService.java @@ -121,8 +121,12 @@ public HasMetadata clone(HasMetadata object) { * Retrieves the number of seconds the SDK waits for reconciliation threads to terminate before * shutting down. * + * @deprecated use {@link io.javaoperatorsdk.operator.Operator#stop(Duration)} instead. Where the + * parameter can be passed to specify graceful timeout. + * * @return the number of seconds to wait before terminating reconciliation threads */ + @Deprecated(forRemoval = true) default int getTerminationTimeoutSeconds() { return DEFAULT_TERMINATION_TIMEOUT_SECONDS; } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ExecutorServiceManager.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ExecutorServiceManager.java index 3261651f3d..54e4586aa0 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ExecutorServiceManager.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ExecutorServiceManager.java @@ -1,5 +1,6 @@ package io.javaoperatorsdk.operator.api.config; +import java.time.Duration; import java.util.Collection; import java.util.List; import java.util.concurrent.Callable; @@ -24,23 +25,20 @@ public class ExecutorServiceManager { private final ExecutorService executor; private final ExecutorService workflowExecutor; private final ExecutorService cachingExecutorService; - private final int terminationTimeoutSeconds; - private ExecutorServiceManager(ExecutorService executor, ExecutorService workflowExecutor, - int terminationTimeoutSeconds) { + private ExecutorServiceManager(ExecutorService executor, ExecutorService workflowExecutor) { this.cachingExecutorService = Executors.newCachedThreadPool(); this.executor = new InstrumentedExecutorService(executor); this.workflowExecutor = new InstrumentedExecutorService(workflowExecutor); - this.terminationTimeoutSeconds = terminationTimeoutSeconds; + } - public static void init() { + public static synchronized void init() { if (instance == null) { final var configuration = ConfigurationServiceProvider.instance(); final var executorService = configuration.getExecutorService(); final var workflowExecutorService = configuration.getWorkflowExecutorService(); - instance = new ExecutorServiceManager(executorService, workflowExecutorService, - configuration.getTerminationTimeoutSeconds()); + instance = new ExecutorServiceManager(executorService, workflowExecutorService); log.debug( "Initialized ExecutorServiceManager executor: {}, workflow executor: {}, timeout: {}", executorService.getClass(), @@ -51,16 +49,23 @@ public static void init() { } } - public static synchronized void stop() { + /** For testing purposes only */ + public static synchronized void reset() { + instance().doStop(Duration.ZERO); + instance = null; + init(); + } + + public static synchronized void stop(Duration gracefulShutdownTimeout) { if (instance != null) { - instance.doStop(); + instance.doStop(gracefulShutdownTimeout); } // make sure that we remove the singleton so that the thread pool is re-created on next call to // start instance = null; } - public synchronized static ExecutorServiceManager instance() { + public static synchronized ExecutorServiceManager instance() { if (instance == null) { // provide a default configuration if none has been provided by init init(); @@ -128,23 +133,30 @@ public ExecutorService cachingExecutorService() { return cachingExecutorService; } - private void doStop() { + private void doStop(Duration gracefulShutdownTimeout) { try { + var parallelExec = Executors.newFixedThreadPool(3); log.debug("Closing executor"); - shutdown(executor); - shutdown(workflowExecutor); - shutdown(cachingExecutorService); + parallelExec.invokeAll(List.of(shutdown(executor, gracefulShutdownTimeout), + shutdown(workflowExecutor, gracefulShutdownTimeout), + shutdown(cachingExecutorService, gracefulShutdownTimeout))); + parallelExec.shutdownNow(); } catch (InterruptedException e) { log.debug("Exception closing executor: {}", e.getLocalizedMessage()); Thread.currentThread().interrupt(); } } - private static void shutdown(ExecutorService executorService) throws InterruptedException { - executorService.shutdown(); - if (!executorService.awaitTermination(instance().terminationTimeoutSeconds, TimeUnit.SECONDS)) { - executorService.shutdownNow(); // if we timed out, waiting, cancel everything - } + private static Callable shutdown(ExecutorService executorService, + Duration gracefulShutdownTimeout) { + return () -> { + executorService.shutdown(); + if (!executorService.awaitTermination(gracefulShutdownTimeout.toMillis(), + TimeUnit.MILLISECONDS)) { + executorService.shutdownNow(); // if we timed out, waiting, cancel everything + } + return null; + }; } private static class InstrumentedExecutorService implements ExecutorService { 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 8709589c66..f973a7d354 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 @@ -388,6 +388,13 @@ private ReconcilerExecutor(ResourceID resourceID, ExecutionScope

executionSco @Override public void run() { + if (!running) { + // this is needed for the case when controller stopped, but there is a graceful shutdown + // timeout. that should finish the currently executing reconciliations but not the ones + // which where submitted but not started yet + log.debug("Event processor not running skipping resource processing: {}", resourceID); + return; + } // change thread name for easier debugging final var thread = Thread.currentThread(); final var name = thread.getName(); diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/config/VersionTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/config/VersionTest.java index 2a6b8002e3..5df0738022 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/config/VersionTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/config/VersionTest.java @@ -4,7 +4,7 @@ import static org.junit.Assert.assertEquals; -public class VersionTest { +class VersionTest { @Test void versionShouldReturnTheSameResultFromMavenAndProperties() { diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/EventProcessorTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/EventProcessorTest.java index 583e7a7289..c3880982e6 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/EventProcessorTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/EventProcessorTest.java @@ -15,7 +15,9 @@ import org.slf4j.LoggerFactory; import io.fabric8.kubernetes.api.model.HasMetadata; +import io.javaoperatorsdk.operator.api.config.ConfigurationServiceProvider; import io.javaoperatorsdk.operator.api.config.ControllerConfiguration; +import io.javaoperatorsdk.operator.api.config.ExecutorServiceManager; import io.javaoperatorsdk.operator.api.config.RetryConfiguration; import io.javaoperatorsdk.operator.api.monitoring.Metrics; import io.javaoperatorsdk.operator.processing.event.rate.LinearRateLimiter; @@ -39,6 +41,7 @@ import static org.mockito.Mockito.after; import static org.mockito.Mockito.any; import static org.mockito.Mockito.anyLong; +import static org.mockito.Mockito.atMostOnce; import static org.mockito.Mockito.doAnswer; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; @@ -56,6 +59,8 @@ class EventProcessorTest { 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"; + public static final int TIME_TO_WAIT_AFTER_SUBMISSION_BEFORE_EXECUTION = 150; + public static final int DISPATCHING_DELAY = 250; private final ReconciliationDispatcher reconciliationDispatcherMock = mock(ReconciliationDispatcher.class); @@ -417,6 +422,28 @@ void schedulesRetryForMarReconciliationIntervalIfRetryExhausted() { verify(retryTimerEventSourceMock, times(1)).scheduleOnce((ResourceID) any(), anyLong()); } + @Test + void executionOfReconciliationShouldNotStartIfProcessorStopped() throws InterruptedException { + when(reconciliationDispatcherMock.handleExecution(any())) + .then((Answer) invocationOnMock -> { + Thread.sleep(DISPATCHING_DELAY); + return PostExecutionControl.defaultDispatch(); + }); + // one event will lock the thread / executor + ConfigurationServiceProvider.overrideCurrent(o -> o.withConcurrentReconciliationThreads(1)); + ExecutorServiceManager.reset(); + eventProcessor.start(); + + eventProcessor.handleEvent(prepareCREvent()); + eventProcessor.handleEvent(prepareCREvent()); + eventProcessor.stop(); + + // wait until both event should be handled + Thread.sleep(TIME_TO_WAIT_AFTER_SUBMISSION_BEFORE_EXECUTION + 2 * DISPATCHING_DELAY); + verify(reconciliationDispatcherMock, atMostOnce()) + .handleExecution(any()); + } + private ResourceID eventAlreadyUnderProcessing() { when(reconciliationDispatcherMock.handleExecution(any())) .then( diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/ReconciliationDispatcherTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/ReconciliationDispatcherTest.java index c154e8a910..e7f9331d5c 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/ReconciliationDispatcherTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/ReconciliationDispatcherTest.java @@ -75,6 +75,7 @@ static void classSetup() { * equals will fail on the two equal but NOT identical TestCustomResources because equals is not * implemented on TestCustomResourceSpec or TestCustomResourceStatus */ + ConfigurationServiceProvider.reset(); ConfigurationServiceProvider.overrideCurrent(overrider -> overrider .checkingCRDAndValidateLocalModel(false).withResourceCloner(new Cloner() { @Override diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/GracefulStopIT.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/GracefulStopIT.java new file mode 100644 index 0000000000..715921ecbb --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/GracefulStopIT.java @@ -0,0 +1,79 @@ +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.gracefulstop.GracefulStopTestCustomResource; +import io.javaoperatorsdk.operator.sample.gracefulstop.GracefulStopTestCustomResourceSpec; +import io.javaoperatorsdk.operator.sample.gracefulstop.GracefulStopTestReconciler; + +import static io.javaoperatorsdk.operator.sample.gracefulstop.GracefulStopTestReconciler.RECONCILER_SLEEP; +import static org.assertj.core.api.Assertions.assertThat; +import static org.awaitility.Awaitility.await; + +public class GracefulStopIT { + + public static final String TEST_1 = "test1"; + public static final String TEST_2 = "test2"; + + @RegisterExtension + LocallyRunOperatorExtension operator = + LocallyRunOperatorExtension.builder() + .withConfigurationService(o -> o.withCloseClientOnStop(false)) + .withReconciler(new GracefulStopTestReconciler()) + .build(); + + @Test + void stopsGracefullyWIthTimeout() { + testGracefulStop(TEST_1, RECONCILER_SLEEP, 2); + } + + @Test + void stopsGracefullyWithExpiredTimeout() { + testGracefulStop(TEST_2, RECONCILER_SLEEP / 5, 1); + } + + private void testGracefulStop(String resourceName, int stopTimeout, int expectedFinalGeneration) { + var testRes = operator.create(testResource(resourceName)); + await().untilAsserted(() -> { + var r = operator.get(GracefulStopTestCustomResource.class, resourceName); + assertThat(r.getStatus()).isNotNull(); + assertThat(r.getStatus().getObservedGeneration()).isEqualTo(1); + assertThat(operator.getReconcilerOfType(GracefulStopTestReconciler.class) + .getNumberOfExecutions()).isEqualTo(1); + }); + + testRes.getSpec().setValue(2); + operator.replace(testRes); + + await().pollDelay(Duration.ofMillis(50)).untilAsserted( + () -> assertThat(operator.getReconcilerOfType(GracefulStopTestReconciler.class) + .getNumberOfExecutions()).isEqualTo(2)); + + operator.getOperator().stop(Duration.ofMillis(stopTimeout)); + + await().untilAsserted(() -> { + var r = operator.get(GracefulStopTestCustomResource.class, resourceName); + assertThat(r.getStatus()).isNotNull(); + assertThat(r.getStatus().getObservedGeneration()).isEqualTo(expectedFinalGeneration); + }); + } + + public GracefulStopTestCustomResource testResource(String name) { + GracefulStopTestCustomResource resource = + new GracefulStopTestCustomResource(); + resource.setMetadata( + new ObjectMetaBuilder() + .withName(name) + .withNamespace(operator.getNamespace()) + .build()); + resource.setSpec(new GracefulStopTestCustomResourceSpec()); + resource.getSpec().setValue(1); + return resource; + } + +} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/gracefulstop/GracefulStopTestCustomResource.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/gracefulstop/GracefulStopTestCustomResource.java new file mode 100644 index 0000000000..529c5ff480 --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/gracefulstop/GracefulStopTestCustomResource.java @@ -0,0 +1,16 @@ +package io.javaoperatorsdk.operator.sample.gracefulstop; + +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("gst") +public class GracefulStopTestCustomResource + extends CustomResource + implements Namespaced { + +} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/gracefulstop/GracefulStopTestCustomResourceSpec.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/gracefulstop/GracefulStopTestCustomResourceSpec.java new file mode 100644 index 0000000000..4d1f45e646 --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/gracefulstop/GracefulStopTestCustomResourceSpec.java @@ -0,0 +1,14 @@ +package io.javaoperatorsdk.operator.sample.gracefulstop; + +public class GracefulStopTestCustomResourceSpec { + + private int value; + + public int getValue() { + return value; + } + + public void setValue(int value) { + this.value = value; + } +} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/gracefulstop/GracefulStopTestCustomResourceStatus.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/gracefulstop/GracefulStopTestCustomResourceStatus.java new file mode 100644 index 0000000000..f59f5b1163 --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/gracefulstop/GracefulStopTestCustomResourceStatus.java @@ -0,0 +1,7 @@ +package io.javaoperatorsdk.operator.sample.gracefulstop; + +import io.javaoperatorsdk.operator.api.ObservedGenerationAwareStatus; + +public class GracefulStopTestCustomResourceStatus extends ObservedGenerationAwareStatus { + +} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/gracefulstop/GracefulStopTestReconciler.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/gracefulstop/GracefulStopTestReconciler.java new file mode 100644 index 0000000000..cf266c0b48 --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/gracefulstop/GracefulStopTestReconciler.java @@ -0,0 +1,34 @@ +package io.javaoperatorsdk.operator.sample.gracefulstop; + +import java.util.concurrent.atomic.AtomicInteger; + +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 +public class GracefulStopTestReconciler + implements Reconciler { + + public static final int RECONCILER_SLEEP = 1000; + + private final AtomicInteger numberOfExecutions = new AtomicInteger(0); + + @Override + public UpdateControl reconcile( + GracefulStopTestCustomResource resource, + Context context) throws InterruptedException { + + numberOfExecutions.addAndGet(1); + resource.setStatus(new GracefulStopTestCustomResourceStatus()); + Thread.sleep(RECONCILER_SLEEP); + + return UpdateControl.patchStatus(resource); + } + + public int getNumberOfExecutions() { + return numberOfExecutions.get(); + } + +} From 67e9655db1f1b5c90e655c59a30e72d0cb639e23 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Attila=20M=C3=A9sz=C3=A1ros?= Date: Wed, 8 Feb 2023 14:27:06 +0100 Subject: [PATCH 07/25] Primary to seconday dr informer sample (#1744) --- docs/documentation/v4-3-migration.md | 4 +- .../PrimaryToSecondaryDependentIT.java | 76 +++++++++++++++ .../ConfigMapDependent.java | 12 +++ .../ConfigMapReconcilePrecondition.java | 26 ++++++ ...aryToSecondaryDependentCustomResource.java | 15 +++ ...PrimaryToSecondaryDependentReconciler.java | 92 +++++++++++++++++++ .../PrimaryToSecondaryDependentSpec.java | 15 +++ .../SecretDependent.java | 32 +++++++ 8 files changed, 270 insertions(+), 2 deletions(-) create mode 100644 operator-framework/src/test/java/io/javaoperatorsdk/operator/PrimaryToSecondaryDependentIT.java create mode 100644 operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/primarytosecondaydependent/ConfigMapDependent.java create mode 100644 operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/primarytosecondaydependent/ConfigMapReconcilePrecondition.java create mode 100644 operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/primarytosecondaydependent/PrimaryToSecondaryDependentCustomResource.java create mode 100644 operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/primarytosecondaydependent/PrimaryToSecondaryDependentReconciler.java create mode 100644 operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/primarytosecondaydependent/PrimaryToSecondaryDependentSpec.java create mode 100644 operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/primarytosecondaydependent/SecretDependent.java diff --git a/docs/documentation/v4-3-migration.md b/docs/documentation/v4-3-migration.md index 77979fe3ba..f5022fa856 100644 --- a/docs/documentation/v4-3-migration.md +++ b/docs/documentation/v4-3-migration.md @@ -9,8 +9,8 @@ permalink: /docs/v4-3-migration ## Condition API Change -In Workflows the target of the condition was the managed resource itself, not a dependent resource. This changed, from -not the API contains the dependent resource. +In Workflows the target of the condition was the managed resource itself, not the target dependent resource. +This changed, now the API contains the dependent resource. New API: diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/PrimaryToSecondaryDependentIT.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/PrimaryToSecondaryDependentIT.java new file mode 100644 index 0000000000..eaa7e4410f --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/PrimaryToSecondaryDependentIT.java @@ -0,0 +1,76 @@ +package io.javaoperatorsdk.operator; + +import java.time.Duration; +import java.util.Map; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; + +import io.fabric8.kubernetes.api.model.ConfigMap; +import io.fabric8.kubernetes.api.model.ObjectMetaBuilder; +import io.fabric8.kubernetes.api.model.Secret; +import io.javaoperatorsdk.operator.junit.LocallyRunOperatorExtension; +import io.javaoperatorsdk.operator.sample.primarytosecondaydependent.PrimaryToSecondaryDependentCustomResource; +import io.javaoperatorsdk.operator.sample.primarytosecondaydependent.PrimaryToSecondaryDependentReconciler; +import io.javaoperatorsdk.operator.sample.primarytosecondaydependent.PrimaryToSecondaryDependentSpec; + +import static io.javaoperatorsdk.operator.sample.primarytosecondaydependent.ConfigMapReconcilePrecondition.DO_NOT_RECONCILE; +import static io.javaoperatorsdk.operator.sample.primarytosecondaydependent.PrimaryToSecondaryDependentReconciler.DATA_KEY; +import static org.assertj.core.api.Assertions.assertThat; +import static org.awaitility.Awaitility.await; + +class PrimaryToSecondaryDependentIT { + + public static final String TEST_CONFIG_MAP_NAME = "testconfigmap"; + public static final String TEST_CR_NAME = "test1"; + public static final String TEST_DATA = "testData"; + public + + @RegisterExtension LocallyRunOperatorExtension operator = + LocallyRunOperatorExtension.builder() + .withReconciler(new PrimaryToSecondaryDependentReconciler()) + .build(); + + @Test + void testPrimaryToSecondaryInDependentResources() { + var reconciler = operator.getReconcilerOfType(PrimaryToSecondaryDependentReconciler.class); + var cm = operator.create(configMap(DO_NOT_RECONCILE)); + operator.create(testCustomResource()); + + await().pollDelay(Duration.ofMillis(250)).untilAsserted(() -> { + assertThat(reconciler.getNumberOfExecutions()).isPositive(); + assertThat(operator.get(Secret.class, TEST_CR_NAME)).isNull(); + }); + + cm.setData(Map.of(DATA_KEY, TEST_DATA)); + var executions = reconciler.getNumberOfExecutions(); + operator.replace(cm); + + await().pollDelay(Duration.ofMillis(250)).untilAsserted(() -> { + assertThat(reconciler.getNumberOfExecutions()).isGreaterThan(executions); + var secret = operator.get(Secret.class, TEST_CR_NAME); + assertThat(secret).isNotNull(); + assertThat(secret.getData().get(DATA_KEY)).isEqualTo(TEST_DATA); + }); + } + + PrimaryToSecondaryDependentCustomResource testCustomResource() { + var res = new PrimaryToSecondaryDependentCustomResource(); + res.setMetadata(new ObjectMetaBuilder() + .withName(TEST_CR_NAME) + .build()); + res.setSpec(new PrimaryToSecondaryDependentSpec()); + res.getSpec().setConfigMapName(TEST_CONFIG_MAP_NAME); + return res; + } + + ConfigMap configMap(String data) { + var cm = new ConfigMap(); + cm.setMetadata(new ObjectMetaBuilder() + .withName(TEST_CONFIG_MAP_NAME) + .build()); + cm.setData(Map.of(DATA_KEY, data)); + return cm; + } + +} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/primarytosecondaydependent/ConfigMapDependent.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/primarytosecondaydependent/ConfigMapDependent.java new file mode 100644 index 0000000000..d08bc2131f --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/primarytosecondaydependent/ConfigMapDependent.java @@ -0,0 +1,12 @@ +package io.javaoperatorsdk.operator.sample.primarytosecondaydependent; + +import io.fabric8.kubernetes.api.model.ConfigMap; +import io.javaoperatorsdk.operator.processing.dependent.kubernetes.KubernetesDependentResource; + +public class ConfigMapDependent extends + KubernetesDependentResource { + + public ConfigMapDependent() { + super(ConfigMap.class); + } +} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/primarytosecondaydependent/ConfigMapReconcilePrecondition.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/primarytosecondaydependent/ConfigMapReconcilePrecondition.java new file mode 100644 index 0000000000..b8aa65585d --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/primarytosecondaydependent/ConfigMapReconcilePrecondition.java @@ -0,0 +1,26 @@ +package io.javaoperatorsdk.operator.sample.primarytosecondaydependent; + +import io.fabric8.kubernetes.api.model.ConfigMap; +import io.javaoperatorsdk.operator.api.reconciler.Context; +import io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResource; +import io.javaoperatorsdk.operator.processing.dependent.workflow.Condition; + +import static io.javaoperatorsdk.operator.sample.primarytosecondaydependent.PrimaryToSecondaryDependentReconciler.DATA_KEY; + +public class ConfigMapReconcilePrecondition + implements Condition { + + public static final String DO_NOT_RECONCILE = "doNotReconcile"; + + @Override + public boolean isMet( + DependentResource dependentResource, + PrimaryToSecondaryDependentCustomResource primary, + Context context) { + return dependentResource.getSecondaryResource(primary, context).map(cm -> { + var data = cm.getData().get(DATA_KEY); + return data != null && !data.equals(DO_NOT_RECONCILE); + }) + .orElse(false); + } +} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/primarytosecondaydependent/PrimaryToSecondaryDependentCustomResource.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/primarytosecondaydependent/PrimaryToSecondaryDependentCustomResource.java new file mode 100644 index 0000000000..4ec7065f2e --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/primarytosecondaydependent/PrimaryToSecondaryDependentCustomResource.java @@ -0,0 +1,15 @@ +package io.javaoperatorsdk.operator.sample.primarytosecondaydependent; + +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("ptsd") +public class PrimaryToSecondaryDependentCustomResource + extends CustomResource + implements Namespaced { +} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/primarytosecondaydependent/PrimaryToSecondaryDependentReconciler.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/primarytosecondaydependent/PrimaryToSecondaryDependentReconciler.java new file mode 100644 index 0000000000..c51111b206 --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/primarytosecondaydependent/PrimaryToSecondaryDependentReconciler.java @@ -0,0 +1,92 @@ +package io.javaoperatorsdk.operator.sample.primarytosecondaydependent; + +import java.util.List; +import java.util.Map; +import java.util.Set; +import java.util.concurrent.atomic.AtomicInteger; +import java.util.stream.Collectors; + +import io.fabric8.kubernetes.api.model.ConfigMap; +import io.javaoperatorsdk.operator.api.config.informer.InformerConfiguration; +import io.javaoperatorsdk.operator.api.reconciler.*; +import io.javaoperatorsdk.operator.api.reconciler.dependent.Dependent; +import io.javaoperatorsdk.operator.processing.event.ResourceID; +import io.javaoperatorsdk.operator.processing.event.source.EventSource; +import io.javaoperatorsdk.operator.processing.event.source.PrimaryToSecondaryMapper; +import io.javaoperatorsdk.operator.processing.event.source.informer.InformerEventSource; +import io.javaoperatorsdk.operator.support.TestExecutionInfoProvider; + +import static io.javaoperatorsdk.operator.sample.primarytosecondaydependent.PrimaryToSecondaryDependentReconciler.CONFIG_MAP; +import static io.javaoperatorsdk.operator.sample.primarytosecondaydependent.PrimaryToSecondaryDependentReconciler.CONFIG_MAP_EVENT_SOURCE; + +/** + * Sample showcases how it is possible to do a primary to secondary mapper for a dependent resource. + * Note that this is usually just used with read only resources. So it has limited usage, one reason + * to use it is to have nice condition on that resource within a workflow. + */ +@ControllerConfiguration(dependents = {@Dependent(type = ConfigMapDependent.class, + name = CONFIG_MAP, + reconcilePrecondition = ConfigMapReconcilePrecondition.class, + useEventSourceWithName = CONFIG_MAP_EVENT_SOURCE), + @Dependent(type = SecretDependent.class, dependsOn = CONFIG_MAP)}) +public class PrimaryToSecondaryDependentReconciler + implements Reconciler, TestExecutionInfoProvider, + EventSourceInitializer { + + public static final String DATA_KEY = "data"; + public static final String CONFIG_MAP = "ConfigMap"; + public static final String CONFIG_MAP_INDEX = "ConfigMapIndex"; + public static final String CONFIG_MAP_EVENT_SOURCE = "ConfigMapEventSource"; + + private final AtomicInteger numberOfExecutions = new AtomicInteger(0); + + @Override + public UpdateControl reconcile( + PrimaryToSecondaryDependentCustomResource resource, + Context context) { + numberOfExecutions.addAndGet(1); + return UpdateControl.noUpdate(); + } + + public int getNumberOfExecutions() { + return numberOfExecutions.get(); + } + + /** + * Creating an Event Source and setting it for the Dependent Resource. Since it is not possible to + * do this setup elegantly within the bounds of the KubernetesDependentResource API. However, this + * is quite a corner case; might be covered more out of the box in the future if there will be + * demand for it. + **/ + @Override + public Map prepareEventSources( + EventSourceContext context) { + // there is no owner reference in the config map, but we still want to trigger reconciliation if + // the config map changes. So first we add an index which custom resource references the config + // map. + context.getPrimaryCache().addIndexer(CONFIG_MAP_INDEX, (primary -> List + .of(indexKey(primary.getSpec().getConfigMapName(), primary.getMetadata().getNamespace())))); + + var cmES = new InformerEventSource<>(InformerConfiguration + .from(ConfigMap.class, context) + // if there is a many-to-many relationship (thus no direct owner reference) + // PrimaryToSecondaryMapper needs to be added + .withPrimaryToSecondaryMapper( + (PrimaryToSecondaryMapper) p -> Set + .of(new ResourceID(p.getSpec().getConfigMapName(), p.getMetadata().getNamespace()))) + // the index is used to trigger reconciliation of related custom resources if config map + // changes + .withSecondaryToPrimaryMapper(cm -> context.getPrimaryCache() + .byIndex(CONFIG_MAP_INDEX, indexKey(cm.getMetadata().getName(), + cm.getMetadata().getNamespace())) + .stream().map(ResourceID::fromResource).collect(Collectors.toSet())) + .build(), + context); + + return Map.of(CONFIG_MAP_EVENT_SOURCE, cmES); + } + + private String indexKey(String configMapName, String namespace) { + return configMapName + "#" + namespace; + } +} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/primarytosecondaydependent/PrimaryToSecondaryDependentSpec.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/primarytosecondaydependent/PrimaryToSecondaryDependentSpec.java new file mode 100644 index 0000000000..966a5c242b --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/primarytosecondaydependent/PrimaryToSecondaryDependentSpec.java @@ -0,0 +1,15 @@ +package io.javaoperatorsdk.operator.sample.primarytosecondaydependent; + +public class PrimaryToSecondaryDependentSpec { + + private String configMapName; + + public String getConfigMapName() { + return configMapName; + } + + public PrimaryToSecondaryDependentSpec setConfigMapName(String configMapName) { + this.configMapName = configMapName; + return this; + } +} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/primarytosecondaydependent/SecretDependent.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/primarytosecondaydependent/SecretDependent.java new file mode 100644 index 0000000000..c78f3c470f --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/primarytosecondaydependent/SecretDependent.java @@ -0,0 +1,32 @@ +package io.javaoperatorsdk.operator.sample.primarytosecondaydependent; + +import java.util.Map; + +import io.fabric8.kubernetes.api.model.ConfigMap; +import io.fabric8.kubernetes.api.model.ObjectMetaBuilder; +import io.fabric8.kubernetes.api.model.Secret; +import io.javaoperatorsdk.operator.api.reconciler.Context; +import io.javaoperatorsdk.operator.processing.dependent.kubernetes.CRUDKubernetesDependentResource; + +import static io.javaoperatorsdk.operator.sample.primarytosecondaydependent.PrimaryToSecondaryDependentReconciler.DATA_KEY; + +public class SecretDependent + extends CRUDKubernetesDependentResource { + + public SecretDependent() { + super(Secret.class); + } + + @Override + protected Secret desired(PrimaryToSecondaryDependentCustomResource primary, + Context context) { + Secret secret = new Secret(); + secret.setMetadata(new ObjectMetaBuilder() + .withName(primary.getMetadata().getName()) + .withNamespace(primary.getMetadata().getNamespace()) + .build()); + secret.setData(Map.of(DATA_KEY, context.getSecondaryResource(ConfigMap.class) + .orElseThrow().getData().get(DATA_KEY))); + return secret; + } +} From 6e12546dcd3f1cafaa679c40f302a66ab4d12fbc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Attila=20M=C3=A9sz=C3=A1ros?= Date: Thu, 9 Feb 2023 08:46:40 +0100 Subject: [PATCH 08/25] imrpove: replacing confusing UpdateControl method (#1762) --- .../api/reconciler/UpdateControl.java | 44 ++++++++++++++++--- .../event/ReconciliationDispatcher.java | 19 ++++---- 2 files changed, 49 insertions(+), 14 deletions(-) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/UpdateControl.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/UpdateControl.java index 12f392901d..7eb7d2ae84 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/UpdateControl.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/UpdateControl.java @@ -8,17 +8,17 @@ public class UpdateControl

extends BaseControl custom resource type * @param customResource customResource to use for update * @return initialized update control @@ -73,6 +76,9 @@ public static UpdateControl updateStatus(T customReso * As a results of this there will be two call to K8S API. First the custom resource will be * updates then the status sub-resource. * + * Using this update makes sure that the resource in the next reconciliation is the updated one - + * this is not guaranteed by default if you do an update on a resource by the Kubernetes client. + * * @param resource type * @param customResource - custom resource to use in both API calls * @return UpdateControl instance @@ -82,11 +88,36 @@ public static UpdateControl updateResourceAndStatus( return new UpdateControl<>(customResource, true, true, false); } - public static UpdateControl patchResourceAndStatus( + /** + * Updates the resource - with optimistic locking - and patches the status without optimistic + * locking in place. + * + * Note that using this method, it is not guaranteed that the most recent updated resource will be + * in case for next reconciliation. + * + * @param customResource to update + * @return UpdateControl instance + * @param resource type + */ + public static UpdateControl updateResourceAndPatchStatus( T customResource) { return new UpdateControl<>(customResource, true, true, true); } + /** + * Marked for removal, because of confusing name. It does not patch the resource but rather + * updates it. + * + * @deprecated use {@link UpdateControl#updateResourceAndPatchStatus(HasMetadata)} + * + * @param customResource to update + * @return UpdateControl instance + * @param resource type + */ + @Deprecated(forRemoval = true) + public static UpdateControl patchResourceAndStatus(T customResource) { + return updateResourceAndStatus(customResource); + } public static UpdateControl noUpdate() { return new UpdateControl<>(null, false, false, false); @@ -104,8 +135,8 @@ public boolean isUpdateResource() { return updateResource; } - public boolean isPatch() { - return patch; + public boolean isPatchStatus() { + return patchStatus; } public boolean isNoUpdate() { @@ -115,4 +146,5 @@ public boolean isNoUpdate() { public boolean isUpdateResourceAndStatus() { return updateResource && updateStatus; } + } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/ReconciliationDispatcher.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/ReconciliationDispatcher.java index 3018ed30f1..6c7e7bc9e7 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/ReconciliationDispatcher.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/ReconciliationDispatcher.java @@ -84,7 +84,7 @@ private PostExecutionControl

handleDispatch(ExecutionScope

executionScope) Context

context = new DefaultContext<>(executionScope.getRetryInfo(), controller, originalResource); if (markedForDeletion) { - return handleCleanup(originalResource, resourceForExecution, context); + return handleCleanup(resourceForExecution, context); } else { return handleReconcile(executionScope, resourceForExecution, originalResource, context); } @@ -147,23 +147,24 @@ private PostExecutionControl

reconcileExecution(ExecutionScope

executionSc .setResourceVersion(updatedCustomResource.getMetadata().getResourceVersion()); updatedCustomResource = updateStatusGenerationAware(updateControl.getResource(), originalResource, - updateControl.isPatch()); + updateControl.isPatchStatus()); } else if (updateControl.isUpdateStatus()) { updatedCustomResource = updateStatusGenerationAware(updateControl.getResource(), originalResource, - updateControl.isPatch()); + updateControl.isPatchStatus()); } else if (updateControl.isUpdateResource()) { updatedCustomResource = updateCustomResource(updateControl.getResource()); if (shouldUpdateObservedGenerationAutomatically(updatedCustomResource)) { updatedCustomResource = updateStatusGenerationAware(updateControl.getResource(), originalResource, - updateControl.isPatch()); + updateControl.isPatchStatus()); } } else if (updateControl.isNoUpdate() && shouldUpdateObservedGenerationAutomatically(resourceForExecution)) { updatedCustomResource = - updateStatusGenerationAware(originalResource, originalResource, updateControl.isPatch()); + updateStatusGenerationAware(originalResource, originalResource, + updateControl.isPatchStatus()); } return createPostExecutionControl(updatedCustomResource, updateControl); } @@ -259,7 +260,7 @@ private PostExecutionControl

createPostExecutionControl(P updatedCustomResour UpdateControl

updateControl) { PostExecutionControl

postExecutionControl; if (updatedCustomResource != null) { - if (updateControl.isUpdateStatus() && updateControl.isPatch()) { + if (updateControl.isUpdateStatus() && updateControl.isPatchStatus()) { postExecutionControl = PostExecutionControl.customResourceStatusPatched(updatedCustomResource); } else { @@ -279,7 +280,7 @@ private void updatePostExecutionControlWithReschedule( } - private PostExecutionControl

handleCleanup(P originalResource, P resource, + private PostExecutionControl

handleCleanup(P resource, Context

context) { log.debug( "Executing delete for resource: {} with version: {}", @@ -317,8 +318,10 @@ private P updateCustomResourceWithFinalizer(P resourceForExecution, P originalRe } private P updateCustomResource(P resource) { - log.debug("Updating resource: {} with version: {}", getUID(resource), getVersion(resource)); + log.debug("Updating resource: {} with version: {}", getUID(resource), + getVersion(resource)); log.trace("Resource before update: {}", resource); + return customResourceFacade.updateResource(resource); } From dfb0f6fdb0f4c24366f26e733cdf71751ccfa9c3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Attila=20M=C3=A9sz=C3=A1ros?= Date: Tue, 21 Feb 2023 09:44:17 +0100 Subject: [PATCH 09/25] refactor: Webpage sample better readability (#1771) --- .../io/javaoperatorsdk/operator/Operator.java | 2 +- .../operator/sample/Utils.java | 20 ++++++++++--------- .../WebPageDependentsWorkflowReconciler.java | 2 ++ .../WebPageManagedDependentsReconciler.java | 2 ++ .../operator/sample/WebPageOperator.java | 6 ++++++ .../operator/sample/WebPageReconciler.java | 1 + ...WebPageStandaloneDependentsReconciler.java | 5 +++++ .../sample/{ => customresource}/WebPage.java | 2 +- .../{ => customresource}/WebPageSpec.java | 2 +- .../{ => customresource}/WebPageStatus.java | 2 +- .../ConfigMapDependentResource.java | 3 ++- .../DeploymentDependentResource.java | 6 ++++-- .../ExposedIngressCondition.java | 3 ++- .../IngressDependentResource.java | 4 +++- .../ServiceDependentResource.java | 6 ++++-- .../sample/{ => probes}/LivenessHandler.java | 4 ++-- .../sample/{ => probes}/StartupHandler.java | 2 +- .../operator/sample/html-configmap.yaml | 6 ------ .../sample/WebPageOperatorAbstractTest.java | 2 ++ .../operator/sample/WebPageOperatorE2E.java | 1 + 20 files changed, 52 insertions(+), 29 deletions(-) rename sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/{ => customresource}/WebPage.java (89%) rename sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/{ => customresource}/WebPageSpec.java (89%) rename sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/{ => customresource}/WebPageStatus.java (94%) rename sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/{ => dependentresource}/ConfigMapDependentResource.java (95%) rename sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/{ => dependentresource}/DeploymentDependentResource.java (87%) rename sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/{ => dependentresource}/ExposedIngressCondition.java (81%) rename sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/{ => dependentresource}/IngressDependentResource.java (80%) rename sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/{ => dependentresource}/ServiceDependentResource.java (85%) rename sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/{ => probes}/LivenessHandler.java (83%) rename sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/{ => probes}/StartupHandler.java (95%) delete mode 100644 sample-operators/webpage/src/main/resources/io/javaoperatorsdk/operator/sample/html-configmap.yaml diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/Operator.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/Operator.java index d18b340f36..0cb82c5b30 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/Operator.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/Operator.java @@ -91,7 +91,7 @@ public void installShutdownHook() { /** * Adds a shutdown hook that automatically calls {@link #stop()} when the app shuts down. Note - * that graceful shutdown is usually not needed, but your {@link Reconciler} implementations might + * that graceful shutdown is usually not needed, but some {@link Reconciler} implementations might * require it. *

* Note that you might want to tune "terminationGracePeriodSeconds" for the Pod running the diff --git a/sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/Utils.java b/sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/Utils.java index 38d9e68828..b37b98aa52 100644 --- a/sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/Utils.java +++ b/sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/Utils.java @@ -2,6 +2,8 @@ import io.fabric8.kubernetes.api.model.networking.v1.Ingress; import io.javaoperatorsdk.operator.api.reconciler.ErrorStatusUpdateControl; +import io.javaoperatorsdk.operator.sample.customresource.WebPage; +import io.javaoperatorsdk.operator.sample.customresource.WebPageStatus; import static io.javaoperatorsdk.operator.ReconcilerUtils.loadYaml; @@ -9,7 +11,7 @@ public class Utils { private Utils() {} - static WebPageStatus createStatus(String configMapName) { + public static WebPageStatus createStatus(String configMapName) { WebPageStatus status = new WebPageStatus(); status.setHtmlConfigMap(configMapName); status.setAreWeGood(true); @@ -17,37 +19,37 @@ static WebPageStatus createStatus(String configMapName) { return status; } - static String configMapName(WebPage nginx) { + public static String configMapName(WebPage nginx) { return nginx.getMetadata().getName() + "-html"; } - static String deploymentName(WebPage nginx) { + public static String deploymentName(WebPage nginx) { return nginx.getMetadata().getName(); } - static String serviceName(WebPage webPage) { + public static String serviceName(WebPage webPage) { return webPage.getMetadata().getName(); } - static ErrorStatusUpdateControl handleError(WebPage resource, Exception e) { + public static ErrorStatusUpdateControl handleError(WebPage resource, Exception e) { resource.getStatus().setErrorMessage("Error: " + e.getMessage()); return ErrorStatusUpdateControl.updateStatus(resource); } - static void simulateErrorIfRequested(WebPage webPage) throws ErrorSimulationException { + public static void simulateErrorIfRequested(WebPage webPage) throws ErrorSimulationException { if (webPage.getSpec().getHtml().contains("error")) { // special case just to showcase error if doing a demo throw new ErrorSimulationException("Simulating error"); } } - static boolean isValidHtml(WebPage webPage) { + public static boolean isValidHtml(WebPage webPage) { // very dummy html validation var lowerCaseHtml = webPage.getSpec().getHtml().toLowerCase(); return lowerCaseHtml.contains("") && lowerCaseHtml.contains(""); } - static WebPage setInvalidHtmlErrorMessage(WebPage webPage) { + public static WebPage setInvalidHtmlErrorMessage(WebPage webPage) { if (webPage.getStatus() == null) { webPage.setStatus(new WebPageStatus()); } @@ -55,7 +57,7 @@ static WebPage setInvalidHtmlErrorMessage(WebPage webPage) { return webPage; } - static Ingress makeDesiredIngress(WebPage webPage) { + public static Ingress makeDesiredIngress(WebPage webPage) { Ingress ingress = loadYaml(Ingress.class, Utils.class, "ingress.yaml"); ingress.getMetadata().setName(webPage.getMetadata().getName()); ingress.getMetadata().setNamespace(webPage.getMetadata().getNamespace()); diff --git a/sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/WebPageDependentsWorkflowReconciler.java b/sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/WebPageDependentsWorkflowReconciler.java index 287d27a5c3..1aaf6d19db 100644 --- a/sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/WebPageDependentsWorkflowReconciler.java +++ b/sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/WebPageDependentsWorkflowReconciler.java @@ -21,6 +21,8 @@ import io.javaoperatorsdk.operator.processing.dependent.workflow.Workflow; import io.javaoperatorsdk.operator.processing.dependent.workflow.WorkflowBuilder; import io.javaoperatorsdk.operator.processing.event.source.EventSource; +import io.javaoperatorsdk.operator.sample.customresource.WebPage; +import io.javaoperatorsdk.operator.sample.dependentresource.*; import static io.javaoperatorsdk.operator.sample.Utils.createStatus; import static io.javaoperatorsdk.operator.sample.Utils.handleError; diff --git a/sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/WebPageManagedDependentsReconciler.java b/sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/WebPageManagedDependentsReconciler.java index fa41e2ff13..d370cd3315 100644 --- a/sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/WebPageManagedDependentsReconciler.java +++ b/sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/WebPageManagedDependentsReconciler.java @@ -3,6 +3,8 @@ import io.fabric8.kubernetes.api.model.ConfigMap; import io.javaoperatorsdk.operator.api.reconciler.*; import io.javaoperatorsdk.operator.api.reconciler.dependent.Dependent; +import io.javaoperatorsdk.operator.sample.customresource.WebPage; +import io.javaoperatorsdk.operator.sample.dependentresource.*; import static io.javaoperatorsdk.operator.sample.Utils.createStatus; import static io.javaoperatorsdk.operator.sample.Utils.handleError; diff --git a/sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/WebPageOperator.java b/sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/WebPageOperator.java index 864f97ccfe..28a0b4e84f 100644 --- a/sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/WebPageOperator.java +++ b/sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/WebPageOperator.java @@ -9,6 +9,8 @@ import io.fabric8.kubernetes.client.KubernetesClient; import io.fabric8.kubernetes.client.KubernetesClientBuilder; import io.javaoperatorsdk.operator.Operator; +import io.javaoperatorsdk.operator.sample.probes.LivenessHandler; +import io.javaoperatorsdk.operator.sample.probes.StartupHandler; import com.sun.net.httpserver.HttpServer; @@ -19,6 +21,10 @@ public class WebPageOperator { private static final Logger log = LoggerFactory.getLogger(WebPageOperator.class); + /** + * Based on env variables a different flavor of Reconciler is used, showcasing how the same logic + * can be implemented using the low level and higher level APIs. + */ public static void main(String[] args) throws IOException { log.info("WebServer Operator starting!"); diff --git a/sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/WebPageReconciler.java b/sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/WebPageReconciler.java index ff63cbcac7..2680e18010 100644 --- a/sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/WebPageReconciler.java +++ b/sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/WebPageReconciler.java @@ -29,6 +29,7 @@ import io.javaoperatorsdk.operator.processing.event.rate.RateLimited; import io.javaoperatorsdk.operator.processing.event.source.EventSource; import io.javaoperatorsdk.operator.processing.event.source.informer.InformerEventSource; +import io.javaoperatorsdk.operator.sample.customresource.WebPage; import static io.javaoperatorsdk.operator.sample.Utils.configMapName; import static io.javaoperatorsdk.operator.sample.Utils.createStatus; diff --git a/sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/WebPageStandaloneDependentsReconciler.java b/sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/WebPageStandaloneDependentsReconciler.java index 12e61d19a1..3230259c1d 100644 --- a/sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/WebPageStandaloneDependentsReconciler.java +++ b/sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/WebPageStandaloneDependentsReconciler.java @@ -15,6 +15,11 @@ import io.javaoperatorsdk.operator.processing.dependent.kubernetes.KubernetesDependentResource; import io.javaoperatorsdk.operator.processing.dependent.kubernetes.KubernetesDependentResourceConfig; import io.javaoperatorsdk.operator.processing.event.source.EventSource; +import io.javaoperatorsdk.operator.sample.customresource.WebPage; +import io.javaoperatorsdk.operator.sample.dependentresource.ConfigMapDependentResource; +import io.javaoperatorsdk.operator.sample.dependentresource.DeploymentDependentResource; +import io.javaoperatorsdk.operator.sample.dependentresource.IngressDependentResource; +import io.javaoperatorsdk.operator.sample.dependentresource.ServiceDependentResource; import static io.javaoperatorsdk.operator.sample.Utils.*; import static io.javaoperatorsdk.operator.sample.WebPageManagedDependentsReconciler.SELECTOR; diff --git a/sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/WebPage.java b/sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/customresource/WebPage.java similarity index 89% rename from sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/WebPage.java rename to sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/customresource/WebPage.java index f1e4a69e3b..c468fa212a 100644 --- a/sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/WebPage.java +++ b/sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/customresource/WebPage.java @@ -1,4 +1,4 @@ -package io.javaoperatorsdk.operator.sample; +package io.javaoperatorsdk.operator.sample.customresource; import io.fabric8.kubernetes.api.model.Namespaced; import io.fabric8.kubernetes.client.CustomResource; diff --git a/sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/WebPageSpec.java b/sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/customresource/WebPageSpec.java similarity index 89% rename from sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/WebPageSpec.java rename to sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/customresource/WebPageSpec.java index 1303495dc3..56fd7dda40 100644 --- a/sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/WebPageSpec.java +++ b/sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/customresource/WebPageSpec.java @@ -1,4 +1,4 @@ -package io.javaoperatorsdk.operator.sample; +package io.javaoperatorsdk.operator.sample.customresource; public class WebPageSpec { diff --git a/sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/WebPageStatus.java b/sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/customresource/WebPageStatus.java similarity index 94% rename from sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/WebPageStatus.java rename to sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/customresource/WebPageStatus.java index 5f66646b92..7ab20c76be 100644 --- a/sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/WebPageStatus.java +++ b/sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/customresource/WebPageStatus.java @@ -1,4 +1,4 @@ -package io.javaoperatorsdk.operator.sample; +package io.javaoperatorsdk.operator.sample.customresource; import io.javaoperatorsdk.operator.api.ObservedGenerationAwareStatus; diff --git a/sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/ConfigMapDependentResource.java b/sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/dependentresource/ConfigMapDependentResource.java similarity index 95% rename from sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/ConfigMapDependentResource.java rename to sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/dependentresource/ConfigMapDependentResource.java index 4eddc97be3..cc7b15146b 100644 --- a/sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/ConfigMapDependentResource.java +++ b/sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/dependentresource/ConfigMapDependentResource.java @@ -1,4 +1,4 @@ -package io.javaoperatorsdk.operator.sample; +package io.javaoperatorsdk.operator.sample.dependentresource; import java.util.HashMap; import java.util.Map; @@ -12,6 +12,7 @@ import io.javaoperatorsdk.operator.api.reconciler.Context; import io.javaoperatorsdk.operator.processing.dependent.kubernetes.CRUDKubernetesDependentResource; import io.javaoperatorsdk.operator.processing.dependent.kubernetes.KubernetesDependent; +import io.javaoperatorsdk.operator.sample.customresource.WebPage; import static io.javaoperatorsdk.operator.sample.Utils.configMapName; import static io.javaoperatorsdk.operator.sample.Utils.deploymentName; diff --git a/sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/DeploymentDependentResource.java b/sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/dependentresource/DeploymentDependentResource.java similarity index 87% rename from sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/DeploymentDependentResource.java rename to sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/dependentresource/DeploymentDependentResource.java index 8986660bdf..4b2e373260 100644 --- a/sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/DeploymentDependentResource.java +++ b/sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/dependentresource/DeploymentDependentResource.java @@ -1,4 +1,4 @@ -package io.javaoperatorsdk.operator.sample; +package io.javaoperatorsdk.operator.sample.dependentresource; import java.util.HashMap; import java.util.Map; @@ -8,6 +8,8 @@ import io.javaoperatorsdk.operator.api.reconciler.Context; import io.javaoperatorsdk.operator.processing.dependent.kubernetes.CRUDKubernetesDependentResource; import io.javaoperatorsdk.operator.processing.dependent.kubernetes.KubernetesDependent; +import io.javaoperatorsdk.operator.sample.Utils; +import io.javaoperatorsdk.operator.sample.customresource.WebPage; import static io.javaoperatorsdk.operator.ReconcilerUtils.loadYaml; import static io.javaoperatorsdk.operator.sample.Utils.configMapName; @@ -28,7 +30,7 @@ protected Deployment desired(WebPage webPage, Context context) { Map labels = new HashMap<>(); labels.put(SELECTOR, "true"); var deploymentName = deploymentName(webPage); - Deployment deployment = loadYaml(Deployment.class, getClass(), "deployment.yaml"); + Deployment deployment = loadYaml(Deployment.class, Utils.class, "deployment.yaml"); deployment.getMetadata().setName(deploymentName); deployment.getMetadata().setNamespace(webPage.getMetadata().getNamespace()); deployment.getMetadata().setLabels(labels); diff --git a/sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/ExposedIngressCondition.java b/sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/dependentresource/ExposedIngressCondition.java similarity index 81% rename from sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/ExposedIngressCondition.java rename to sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/dependentresource/ExposedIngressCondition.java index c2c0d0b423..80691de96e 100644 --- a/sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/ExposedIngressCondition.java +++ b/sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/dependentresource/ExposedIngressCondition.java @@ -1,9 +1,10 @@ -package io.javaoperatorsdk.operator.sample; +package io.javaoperatorsdk.operator.sample.dependentresource; import io.fabric8.kubernetes.api.model.networking.v1.Ingress; import io.javaoperatorsdk.operator.api.reconciler.Context; import io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResource; import io.javaoperatorsdk.operator.processing.dependent.workflow.Condition; +import io.javaoperatorsdk.operator.sample.customresource.WebPage; public class ExposedIngressCondition implements Condition { diff --git a/sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/IngressDependentResource.java b/sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/dependentresource/IngressDependentResource.java similarity index 80% rename from sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/IngressDependentResource.java rename to sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/dependentresource/IngressDependentResource.java index 703d3aceb1..f3a3aa65ba 100644 --- a/sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/IngressDependentResource.java +++ b/sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/dependentresource/IngressDependentResource.java @@ -1,9 +1,11 @@ -package io.javaoperatorsdk.operator.sample; +package io.javaoperatorsdk.operator.sample.dependentresource; import io.fabric8.kubernetes.api.model.networking.v1.Ingress; import io.javaoperatorsdk.operator.api.reconciler.Context; import io.javaoperatorsdk.operator.processing.dependent.kubernetes.CRUDKubernetesDependentResource; import io.javaoperatorsdk.operator.processing.dependent.kubernetes.KubernetesDependent; +import io.javaoperatorsdk.operator.sample.WebPageManagedDependentsReconciler; +import io.javaoperatorsdk.operator.sample.customresource.WebPage; import static io.javaoperatorsdk.operator.sample.Utils.makeDesiredIngress; diff --git a/sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/ServiceDependentResource.java b/sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/dependentresource/ServiceDependentResource.java similarity index 85% rename from sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/ServiceDependentResource.java rename to sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/dependentresource/ServiceDependentResource.java index 1080b1b461..80d5073a6e 100644 --- a/sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/ServiceDependentResource.java +++ b/sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/dependentresource/ServiceDependentResource.java @@ -1,4 +1,4 @@ -package io.javaoperatorsdk.operator.sample; +package io.javaoperatorsdk.operator.sample.dependentresource; import java.util.HashMap; import java.util.Map; @@ -6,6 +6,8 @@ import io.fabric8.kubernetes.api.model.Service; import io.javaoperatorsdk.operator.api.reconciler.Context; import io.javaoperatorsdk.operator.processing.dependent.kubernetes.KubernetesDependent; +import io.javaoperatorsdk.operator.sample.Utils; +import io.javaoperatorsdk.operator.sample.customresource.WebPage; import static io.javaoperatorsdk.operator.ReconcilerUtils.loadYaml; import static io.javaoperatorsdk.operator.sample.Utils.deploymentName; @@ -25,7 +27,7 @@ public ServiceDependentResource() { protected Service desired(WebPage webPage, Context context) { Map serviceLabels = new HashMap<>(); serviceLabels.put(SELECTOR, "true"); - Service service = loadYaml(Service.class, getClass(), "service.yaml"); + Service service = loadYaml(Service.class, Utils.class, "service.yaml"); service.getMetadata().setName(serviceName(webPage)); service.getMetadata().setNamespace(webPage.getMetadata().getNamespace()); service.getMetadata().setLabels(serviceLabels); diff --git a/sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/LivenessHandler.java b/sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/probes/LivenessHandler.java similarity index 83% rename from sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/LivenessHandler.java rename to sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/probes/LivenessHandler.java index 155fc13fec..e3259e9adf 100644 --- a/sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/LivenessHandler.java +++ b/sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/probes/LivenessHandler.java @@ -1,4 +1,4 @@ -package io.javaoperatorsdk.operator.sample; +package io.javaoperatorsdk.operator.sample.probes; import java.io.IOException; @@ -7,7 +7,7 @@ import com.sun.net.httpserver.HttpExchange; import com.sun.net.httpserver.HttpHandler; -import static io.javaoperatorsdk.operator.sample.StartupHandler.sendMessage; +import static io.javaoperatorsdk.operator.sample.probes.StartupHandler.sendMessage; public class LivenessHandler implements HttpHandler { diff --git a/sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/StartupHandler.java b/sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/probes/StartupHandler.java similarity index 95% rename from sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/StartupHandler.java rename to sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/probes/StartupHandler.java index 0cbc313273..3e7bdd4673 100644 --- a/sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/StartupHandler.java +++ b/sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/probes/StartupHandler.java @@ -1,4 +1,4 @@ -package io.javaoperatorsdk.operator.sample; +package io.javaoperatorsdk.operator.sample.probes; import java.io.IOException; import java.nio.charset.StandardCharsets; diff --git a/sample-operators/webpage/src/main/resources/io/javaoperatorsdk/operator/sample/html-configmap.yaml b/sample-operators/webpage/src/main/resources/io/javaoperatorsdk/operator/sample/html-configmap.yaml deleted file mode 100644 index 8314c5b927..0000000000 --- a/sample-operators/webpage/src/main/resources/io/javaoperatorsdk/operator/sample/html-configmap.yaml +++ /dev/null @@ -1,6 +0,0 @@ -kind: ConfigMap -apiVersion: v1 -metadata: - name: "" -data: - html: "" \ No newline at end of file diff --git a/sample-operators/webpage/src/test/java/io/javaoperatorsdk/operator/sample/WebPageOperatorAbstractTest.java b/sample-operators/webpage/src/test/java/io/javaoperatorsdk/operator/sample/WebPageOperatorAbstractTest.java index 4db22853c6..24945a58b3 100644 --- a/sample-operators/webpage/src/test/java/io/javaoperatorsdk/operator/sample/WebPageOperatorAbstractTest.java +++ b/sample-operators/webpage/src/test/java/io/javaoperatorsdk/operator/sample/WebPageOperatorAbstractTest.java @@ -18,6 +18,8 @@ import io.fabric8.kubernetes.client.KubernetesClientBuilder; import io.fabric8.kubernetes.client.LocalPortForward; import io.javaoperatorsdk.operator.junit.AbstractOperatorExtension; +import io.javaoperatorsdk.operator.sample.customresource.WebPage; +import io.javaoperatorsdk.operator.sample.customresource.WebPageSpec; import static io.javaoperatorsdk.operator.sample.Utils.deploymentName; import static io.javaoperatorsdk.operator.sample.Utils.serviceName; diff --git a/sample-operators/webpage/src/test/java/io/javaoperatorsdk/operator/sample/WebPageOperatorE2E.java b/sample-operators/webpage/src/test/java/io/javaoperatorsdk/operator/sample/WebPageOperatorE2E.java index a39d1cc054..81746769c1 100644 --- a/sample-operators/webpage/src/test/java/io/javaoperatorsdk/operator/sample/WebPageOperatorE2E.java +++ b/sample-operators/webpage/src/test/java/io/javaoperatorsdk/operator/sample/WebPageOperatorE2E.java @@ -12,6 +12,7 @@ import io.javaoperatorsdk.operator.junit.AbstractOperatorExtension; import io.javaoperatorsdk.operator.junit.ClusterDeployedOperatorExtension; import io.javaoperatorsdk.operator.junit.LocallyRunOperatorExtension; +import io.javaoperatorsdk.operator.sample.customresource.WebPage; import static io.javaoperatorsdk.operator.sample.WebPageOperator.WEBPAGE_CLASSIC_RECONCILER_ENV_VALUE; import static io.javaoperatorsdk.operator.sample.WebPageOperator.WEBPAGE_RECONCILER_ENV; From 79d479a8b01cb110b6e6ece7859d37133f43a416 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Attila=20M=C3=A9sz=C3=A1ros?= Date: Tue, 21 Feb 2023 15:07:27 +0100 Subject: [PATCH 10/25] fix: flaky test `informerStoppedHandlerShouldBeCalledWhenInformerStops` (#1777) --- .../event/source/informer/InformerEventSourceTest.java | 1 + 1 file changed, 1 insertion(+) diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerEventSourceTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerEventSourceTest.java index a33658415e..548f14e4f8 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerEventSourceTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerEventSourceTest.java @@ -248,6 +248,7 @@ void filtersOnDeleteEvents() { @Test void informerStoppedHandlerShouldBeCalledWhenInformerStops() { try { + ConfigurationServiceProvider.reset(); final var exception = new RuntimeException("Informer stopped exceptionally!"); final var informerStoppedHandler = mock(InformerStoppedHandler.class); ConfigurationServiceProvider From c6e10aab036c82c4ab9610c1cbf6ef1a1b01020e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Attila=20M=C3=A9sz=C3=A1ros?= Date: Tue, 21 Feb 2023 16:31:13 +0100 Subject: [PATCH 11/25] feat: bounded cache for informers (#1718) --- caffeine-bounded-cache-support/pom.xml | 84 ++++++++++ .../source/cache/CaffeineBoundedCache.java | 32 ++++ .../cache/CaffeineBoundedItemStores.java | 47 ++++++ .../source/cache/BoundedCacheTestBase.java | 95 +++++++++++ .../CaffeineBoundedCacheClusterScopeIT.java | 52 ++++++ .../CaffeineBoundedCacheNamespacedIT.java | 50 ++++++ .../cache/sample/AbstractTestReconciler.java | 117 ++++++++++++++ ...edCacheClusterScopeTestCustomResource.java | 15 ++ ...oundedCacheClusterScopeTestReconciler.java | 10 ++ .../BoundedCacheTestCustomResource.java | 14 ++ .../BoundedCacheTestReconciler.java | 10 ++ .../namespacescope/BoundedCacheTestSpec.java | 25 +++ .../BoundedCacheTestStatus.java | 6 + .../src/test/resources/log4j2.xml | 13 ++ docs/documentation/features.md | 26 +++ .../api/config/BaseConfigurationService.java | 19 ++- .../ControllerConfigurationOverrider.java | 9 +- .../config/DefaultResourceConfiguration.java | 10 +- .../ResolvedControllerConfiguration.java | 21 ++- .../api/config/ResourceConfiguration.java | 20 +++ .../operator/api/config/Utils.java | 24 ++- .../informer/InformerConfiguration.java | 15 +- .../reconciler/ControllerConfiguration.java | 3 + .../event/source/cache/BoundedCache.java | 11 ++ .../event/source/cache/BoundedItemStore.java | 148 ++++++++++++++++++ .../cache/KubernetesResourceFetcher.java | 47 ++++++ .../event/source/cache/ResourceFetcher.java | 7 + .../source/informer/InformerManager.java | 1 + .../source/cache/BoundedItemStoreTest.java | 111 +++++++++++++ .../cache/KubernetesResourceFetcherTest.java | 48 ++++++ pom.xml | 7 + 31 files changed, 1071 insertions(+), 26 deletions(-) create mode 100644 caffeine-bounded-cache-support/pom.xml create mode 100644 caffeine-bounded-cache-support/src/main/java/io/javaoperatorsdk/operator/processing/event/source/cache/CaffeineBoundedCache.java create mode 100644 caffeine-bounded-cache-support/src/main/java/io/javaoperatorsdk/operator/processing/event/source/cache/CaffeineBoundedItemStores.java create mode 100644 caffeine-bounded-cache-support/src/test/java/io/javaoperatorsdk/operator/processing/event/source/cache/BoundedCacheTestBase.java create mode 100644 caffeine-bounded-cache-support/src/test/java/io/javaoperatorsdk/operator/processing/event/source/cache/CaffeineBoundedCacheClusterScopeIT.java create mode 100644 caffeine-bounded-cache-support/src/test/java/io/javaoperatorsdk/operator/processing/event/source/cache/CaffeineBoundedCacheNamespacedIT.java create mode 100644 caffeine-bounded-cache-support/src/test/java/io/javaoperatorsdk/operator/processing/event/source/cache/sample/AbstractTestReconciler.java create mode 100644 caffeine-bounded-cache-support/src/test/java/io/javaoperatorsdk/operator/processing/event/source/cache/sample/clusterscope/BoundedCacheClusterScopeTestCustomResource.java create mode 100644 caffeine-bounded-cache-support/src/test/java/io/javaoperatorsdk/operator/processing/event/source/cache/sample/clusterscope/BoundedCacheClusterScopeTestReconciler.java create mode 100644 caffeine-bounded-cache-support/src/test/java/io/javaoperatorsdk/operator/processing/event/source/cache/sample/namespacescope/BoundedCacheTestCustomResource.java create mode 100644 caffeine-bounded-cache-support/src/test/java/io/javaoperatorsdk/operator/processing/event/source/cache/sample/namespacescope/BoundedCacheTestReconciler.java create mode 100644 caffeine-bounded-cache-support/src/test/java/io/javaoperatorsdk/operator/processing/event/source/cache/sample/namespacescope/BoundedCacheTestSpec.java create mode 100644 caffeine-bounded-cache-support/src/test/java/io/javaoperatorsdk/operator/processing/event/source/cache/sample/namespacescope/BoundedCacheTestStatus.java create mode 100644 caffeine-bounded-cache-support/src/test/resources/log4j2.xml create mode 100644 operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/cache/BoundedCache.java create mode 100644 operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/cache/BoundedItemStore.java create mode 100644 operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/cache/KubernetesResourceFetcher.java create mode 100644 operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/cache/ResourceFetcher.java create mode 100644 operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/cache/BoundedItemStoreTest.java create mode 100644 operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/cache/KubernetesResourceFetcherTest.java diff --git a/caffeine-bounded-cache-support/pom.xml b/caffeine-bounded-cache-support/pom.xml new file mode 100644 index 0000000000..23b5334287 --- /dev/null +++ b/caffeine-bounded-cache-support/pom.xml @@ -0,0 +1,84 @@ + + + + java-operator-sdk + io.javaoperatorsdk + 4.3.0-SNAPSHOT + + 4.0.0 + + caffeine-bounded-cache-support + Operator SDK - Caffeine Bounded Cache Support + + + 11 + 11 + + + + + io.javaoperatorsdk + operator-framework-core + + + com.github.ben-manes.caffeine + caffeine + + + io.javaoperatorsdk + operator-framework + test + + + io.javaoperatorsdk + operator-framework-junit-5 + ${project.version} + test + + + io.fabric8 + crd-generator-apt + test + + + org.apache.logging.log4j + log4j-slf4j-impl + test + + + org.apache.logging.log4j + log4j-core + ${log4j.version} + test-jar + test + + + + + + + maven-compiler-plugin + ${maven-compiler-plugin.version} + + + + default-compile + compile + + compile + + + + -proc:none + + + + + + + + + \ No newline at end of file diff --git a/caffeine-bounded-cache-support/src/main/java/io/javaoperatorsdk/operator/processing/event/source/cache/CaffeineBoundedCache.java b/caffeine-bounded-cache-support/src/main/java/io/javaoperatorsdk/operator/processing/event/source/cache/CaffeineBoundedCache.java new file mode 100644 index 0000000000..eb70fe04d3 --- /dev/null +++ b/caffeine-bounded-cache-support/src/main/java/io/javaoperatorsdk/operator/processing/event/source/cache/CaffeineBoundedCache.java @@ -0,0 +1,32 @@ +package io.javaoperatorsdk.operator.processing.event.source.cache; + +import com.github.benmanes.caffeine.cache.Cache; + +/** + * Caffein cache wrapper to be used in a {@link BoundedItemStore} + */ +public class CaffeineBoundedCache implements BoundedCache { + + private Cache cache; + + public CaffeineBoundedCache(Cache cache) { + this.cache = cache; + } + + @Override + public R get(K key) { + return cache.getIfPresent(key); + } + + @Override + public R remove(K key) { + var value = cache.getIfPresent(key); + cache.invalidate(key); + return value; + } + + @Override + public void put(K key, R object) { + cache.put(key, object); + } +} diff --git a/caffeine-bounded-cache-support/src/main/java/io/javaoperatorsdk/operator/processing/event/source/cache/CaffeineBoundedItemStores.java b/caffeine-bounded-cache-support/src/main/java/io/javaoperatorsdk/operator/processing/event/source/cache/CaffeineBoundedItemStores.java new file mode 100644 index 0000000000..aa4db53030 --- /dev/null +++ b/caffeine-bounded-cache-support/src/main/java/io/javaoperatorsdk/operator/processing/event/source/cache/CaffeineBoundedItemStores.java @@ -0,0 +1,47 @@ +package io.javaoperatorsdk.operator.processing.event.source.cache; + +import java.time.Duration; + +import io.fabric8.kubernetes.api.model.HasMetadata; +import io.fabric8.kubernetes.client.KubernetesClient; + +import com.github.benmanes.caffeine.cache.Cache; +import com.github.benmanes.caffeine.cache.Caffeine; + +/** + * The idea about CaffeinBoundedItemStore-s is that, caffeine will cache the resources which were + * recently used, and will evict resource, which are not used for a while. This is ideal from the + * perspective that on startup controllers reconcile all resources (this is why a maxSize not ideal) + * but after a while it can happen (well depending on the controller and domain) that only some + * resources are actually active, thus related events happen. So in case large amount of custom + * resources only the active once will remain in the cache. Note that if a resource is reconciled + * all the secondary resources are usually reconciled too, in that case all those resources are + * fetched and populated to the cache, and will remain there for some time, for a subsequent + * reconciliations. + */ +public class CaffeineBoundedItemStores { + + private CaffeineBoundedItemStores() {} + + /** + * @param client Kubernetes Client + * @param rClass resource class + * @param accessExpireDuration the duration after resources is evicted from cache if not accessed. + * @return the ItemStore implementation + * @param resource type + */ + public static BoundedItemStore boundedItemStore( + KubernetesClient client, Class rClass, + Duration accessExpireDuration) { + Cache cache = Caffeine.newBuilder() + .expireAfterAccess(accessExpireDuration) + .build(); + return boundedItemStore(client, rClass, cache); + } + + public static BoundedItemStore boundedItemStore( + KubernetesClient client, Class rClass, Cache cache) { + return new BoundedItemStore<>(new CaffeineBoundedCache<>(cache), rClass, client); + } + +} diff --git a/caffeine-bounded-cache-support/src/test/java/io/javaoperatorsdk/operator/processing/event/source/cache/BoundedCacheTestBase.java b/caffeine-bounded-cache-support/src/test/java/io/javaoperatorsdk/operator/processing/event/source/cache/BoundedCacheTestBase.java new file mode 100644 index 0000000000..21adf81cc0 --- /dev/null +++ b/caffeine-bounded-cache-support/src/test/java/io/javaoperatorsdk/operator/processing/event/source/cache/BoundedCacheTestBase.java @@ -0,0 +1,95 @@ +package io.javaoperatorsdk.operator.processing.event.source.cache; + +import java.time.Duration; +import java.util.stream.IntStream; + +import org.junit.jupiter.api.Test; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import io.fabric8.kubernetes.api.model.ConfigMap; +import io.fabric8.kubernetes.client.CustomResource; +import io.javaoperatorsdk.operator.junit.LocallyRunOperatorExtension; +import io.javaoperatorsdk.operator.processing.event.source.cache.sample.namespacescope.BoundedCacheTestSpec; +import io.javaoperatorsdk.operator.processing.event.source.cache.sample.namespacescope.BoundedCacheTestStatus; + +import static io.javaoperatorsdk.operator.processing.event.source.cache.sample.AbstractTestReconciler.DATA_KEY; +import static org.assertj.core.api.Assertions.assertThat; +import static org.awaitility.Awaitility.await; + +public abstract class BoundedCacheTestBase

> { + + private static final Logger log = LoggerFactory.getLogger(BoundedCacheTestBase.class); + + public static final int NUMBER_OF_RESOURCE_TO_TEST = 3; + public static final String RESOURCE_NAME_PREFIX = "test-"; + public static final String INITIAL_DATA_PREFIX = "data-"; + public static final String UPDATED_PREFIX = "updatedPrefix"; + + @Test + void reconciliationWorksWithLimitedCache() { + createTestResources(); + + assertConfigMapData(INITIAL_DATA_PREFIX); + + updateTestResources(); + + assertConfigMapData(UPDATED_PREFIX); + + deleteTestResources(); + + assertConfigMapsDeleted(); + } + + private void assertConfigMapsDeleted() { + await().atMost(Duration.ofSeconds(30)) + .untilAsserted(() -> IntStream.range(0, NUMBER_OF_RESOURCE_TO_TEST).forEach(i -> { + var cm = extension().get(ConfigMap.class, RESOURCE_NAME_PREFIX + i); + assertThat(cm).isNull(); + })); + } + + private void deleteTestResources() { + IntStream.range(0, NUMBER_OF_RESOURCE_TO_TEST).forEach(i -> { + var cm = extension().get(customResourceClass(), RESOURCE_NAME_PREFIX + i); + var deleted = extension().delete(cm); + if (!deleted) { + log.warn("Custom resource might not be deleted: {}", cm); + } + }); + } + + private void updateTestResources() { + IntStream.range(0, NUMBER_OF_RESOURCE_TO_TEST).forEach(i -> { + var cm = extension().get(ConfigMap.class, RESOURCE_NAME_PREFIX + i); + cm.getData().put(DATA_KEY, UPDATED_PREFIX + i); + extension().replace(cm); + }); + } + + void assertConfigMapData(String dataPrefix) { + await().untilAsserted(() -> IntStream.range(0, NUMBER_OF_RESOURCE_TO_TEST) + .forEach(i -> assertConfigMap(i, dataPrefix))); + } + + private void assertConfigMap(int i, String prefix) { + var cm = extension().get(ConfigMap.class, RESOURCE_NAME_PREFIX + i); + assertThat(cm).isNotNull(); + assertThat(cm.getData().get(DATA_KEY)).isEqualTo(prefix + i); + } + + private void createTestResources() { + IntStream.range(0, NUMBER_OF_RESOURCE_TO_TEST).forEach(i -> { + extension().create(createTestResource(i)); + }); + } + + abstract P createTestResource(int index); + + abstract Class

customResourceClass(); + + abstract LocallyRunOperatorExtension extension(); + + + +} diff --git a/caffeine-bounded-cache-support/src/test/java/io/javaoperatorsdk/operator/processing/event/source/cache/CaffeineBoundedCacheClusterScopeIT.java b/caffeine-bounded-cache-support/src/test/java/io/javaoperatorsdk/operator/processing/event/source/cache/CaffeineBoundedCacheClusterScopeIT.java new file mode 100644 index 0000000000..252b20f4a4 --- /dev/null +++ b/caffeine-bounded-cache-support/src/test/java/io/javaoperatorsdk/operator/processing/event/source/cache/CaffeineBoundedCacheClusterScopeIT.java @@ -0,0 +1,52 @@ +package io.javaoperatorsdk.operator.processing.event.source.cache; + +import java.time.Duration; + +import org.junit.jupiter.api.extension.RegisterExtension; + +import io.fabric8.kubernetes.api.model.ObjectMetaBuilder; +import io.fabric8.kubernetes.client.KubernetesClientBuilder; +import io.javaoperatorsdk.operator.junit.LocallyRunOperatorExtension; +import io.javaoperatorsdk.operator.processing.event.source.cache.sample.clusterscope.BoundedCacheClusterScopeTestCustomResource; +import io.javaoperatorsdk.operator.processing.event.source.cache.sample.clusterscope.BoundedCacheClusterScopeTestReconciler; +import io.javaoperatorsdk.operator.processing.event.source.cache.sample.namespacescope.BoundedCacheTestSpec; + +import static io.javaoperatorsdk.operator.processing.event.source.cache.sample.AbstractTestReconciler.boundedItemStore; + +public class CaffeineBoundedCacheClusterScopeIT + extends BoundedCacheTestBase { + + @RegisterExtension + LocallyRunOperatorExtension extension = + LocallyRunOperatorExtension.builder() + .withReconciler(new BoundedCacheClusterScopeTestReconciler(), o -> { + o.withItemStore(boundedItemStore( + new KubernetesClientBuilder().build(), + BoundedCacheClusterScopeTestCustomResource.class, + Duration.ofMinutes(1), + 1)); + }) + .build(); + + @Override + BoundedCacheClusterScopeTestCustomResource createTestResource(int index) { + var res = new BoundedCacheClusterScopeTestCustomResource(); + res.setMetadata(new ObjectMetaBuilder() + .withName(RESOURCE_NAME_PREFIX + index) + .build()); + res.setSpec(new BoundedCacheTestSpec()); + res.getSpec().setData(INITIAL_DATA_PREFIX + index); + res.getSpec().setTargetNamespace(extension.getNamespace()); + return res; + } + + @Override + Class customResourceClass() { + return BoundedCacheClusterScopeTestCustomResource.class; + } + + @Override + LocallyRunOperatorExtension extension() { + return extension; + } +} diff --git a/caffeine-bounded-cache-support/src/test/java/io/javaoperatorsdk/operator/processing/event/source/cache/CaffeineBoundedCacheNamespacedIT.java b/caffeine-bounded-cache-support/src/test/java/io/javaoperatorsdk/operator/processing/event/source/cache/CaffeineBoundedCacheNamespacedIT.java new file mode 100644 index 0000000000..ae7f8f5873 --- /dev/null +++ b/caffeine-bounded-cache-support/src/test/java/io/javaoperatorsdk/operator/processing/event/source/cache/CaffeineBoundedCacheNamespacedIT.java @@ -0,0 +1,50 @@ +package io.javaoperatorsdk.operator.processing.event.source.cache; + +import java.time.Duration; + +import org.junit.jupiter.api.extension.RegisterExtension; + +import io.fabric8.kubernetes.api.model.ObjectMetaBuilder; +import io.fabric8.kubernetes.client.KubernetesClientBuilder; +import io.javaoperatorsdk.operator.junit.LocallyRunOperatorExtension; +import io.javaoperatorsdk.operator.processing.event.source.cache.sample.namespacescope.BoundedCacheTestCustomResource; +import io.javaoperatorsdk.operator.processing.event.source.cache.sample.namespacescope.BoundedCacheTestReconciler; +import io.javaoperatorsdk.operator.processing.event.source.cache.sample.namespacescope.BoundedCacheTestSpec; + +import static io.javaoperatorsdk.operator.processing.event.source.cache.sample.AbstractTestReconciler.boundedItemStore; + +class CaffeineBoundedCacheNamespacedIT + extends BoundedCacheTestBase { + + @RegisterExtension + LocallyRunOperatorExtension extension = + LocallyRunOperatorExtension.builder().withReconciler(new BoundedCacheTestReconciler(), o -> { + o.withItemStore(boundedItemStore( + new KubernetesClientBuilder().build(), BoundedCacheTestCustomResource.class, + Duration.ofMinutes(1), + 1)); + }) + .build(); + + BoundedCacheTestCustomResource createTestResource(int index) { + var res = new BoundedCacheTestCustomResource(); + res.setMetadata(new ObjectMetaBuilder() + .withName(RESOURCE_NAME_PREFIX + index) + .build()); + res.setSpec(new BoundedCacheTestSpec()); + res.getSpec().setData(INITIAL_DATA_PREFIX + index); + res.getSpec().setTargetNamespace(extension.getNamespace()); + return res; + } + + @Override + Class customResourceClass() { + return BoundedCacheTestCustomResource.class; + } + + @Override + LocallyRunOperatorExtension extension() { + return extension; + } + +} diff --git a/caffeine-bounded-cache-support/src/test/java/io/javaoperatorsdk/operator/processing/event/source/cache/sample/AbstractTestReconciler.java b/caffeine-bounded-cache-support/src/test/java/io/javaoperatorsdk/operator/processing/event/source/cache/sample/AbstractTestReconciler.java new file mode 100644 index 0000000000..835fcef91a --- /dev/null +++ b/caffeine-bounded-cache-support/src/test/java/io/javaoperatorsdk/operator/processing/event/source/cache/sample/AbstractTestReconciler.java @@ -0,0 +1,117 @@ +package io.javaoperatorsdk.operator.processing.event.source.cache.sample; + +import java.time.Duration; +import java.util.Map; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import io.fabric8.kubernetes.api.model.*; +import io.fabric8.kubernetes.client.CustomResource; +import io.fabric8.kubernetes.client.KubernetesClient; +import io.fabric8.kubernetes.client.KubernetesClientBuilder; +import io.javaoperatorsdk.operator.api.config.informer.InformerConfiguration; +import io.javaoperatorsdk.operator.api.reconciler.*; +import io.javaoperatorsdk.operator.api.reconciler.Context; +import io.javaoperatorsdk.operator.junit.KubernetesClientAware; +import io.javaoperatorsdk.operator.processing.event.source.EventSource; +import io.javaoperatorsdk.operator.processing.event.source.cache.BoundedItemStore; +import io.javaoperatorsdk.operator.processing.event.source.cache.CaffeineBoundedItemStores; +import io.javaoperatorsdk.operator.processing.event.source.cache.sample.clusterscope.BoundedCacheClusterScopeTestReconciler; +import io.javaoperatorsdk.operator.processing.event.source.cache.sample.namespacescope.BoundedCacheTestSpec; +import io.javaoperatorsdk.operator.processing.event.source.cache.sample.namespacescope.BoundedCacheTestStatus; +import io.javaoperatorsdk.operator.processing.event.source.informer.InformerEventSource; +import io.javaoperatorsdk.operator.processing.event.source.informer.Mappers; + +import com.github.benmanes.caffeine.cache.Cache; +import com.github.benmanes.caffeine.cache.Caffeine; + +public abstract class AbstractTestReconciler

> + implements KubernetesClientAware, Reconciler

, + EventSourceInitializer

{ + + private static final Logger log = + LoggerFactory.getLogger(BoundedCacheClusterScopeTestReconciler.class); + + public static final String DATA_KEY = "dataKey"; + + protected KubernetesClient client; + + @Override + public UpdateControl

reconcile( + P resource, + Context

context) { + var maybeConfigMap = context.getSecondaryResource(ConfigMap.class); + maybeConfigMap.ifPresentOrElse( + cm -> updateConfigMapIfNeeded(cm, resource), + () -> createConfigMap(resource)); + ensureStatus(resource); + log.info("Reconciled: {}", resource.getMetadata().getName()); + return UpdateControl.patchStatus(resource); + } + + protected void updateConfigMapIfNeeded(ConfigMap cm, P resource) { + var data = cm.getData().get(DATA_KEY); + if (data == null || data.equals(resource.getSpec().getData())) { + cm.setData(Map.of(DATA_KEY, resource.getSpec().getData())); + client.configMaps().resource(cm).replace(); + } + } + + protected void createConfigMap(P resource) { + var cm = new ConfigMapBuilder() + .withMetadata(new ObjectMetaBuilder() + .withName(resource.getMetadata().getName()) + .withNamespace(resource.getSpec().getTargetNamespace()) + .build()) + .withData(Map.of(DATA_KEY, resource.getSpec().getData())) + .build(); + cm.addOwnerReference(resource); + client.configMaps().resource(cm).create(); + } + + @Override + public KubernetesClient getKubernetesClient() { + return client; + } + + @Override + public void setKubernetesClient(KubernetesClient kubernetesClient) { + this.client = kubernetesClient; + } + + @Override + public Map prepareEventSources( + EventSourceContext

context) { + + var boundedItemStore = + boundedItemStore(new KubernetesClientBuilder().build(), + ConfigMap.class, Duration.ofMinutes(1), 1); // setting max size for testing purposes + + var es = new InformerEventSource<>(InformerConfiguration.from(ConfigMap.class, context) + .withItemStore(boundedItemStore) + .withSecondaryToPrimaryMapper( + Mappers.fromOwnerReference(this instanceof BoundedCacheClusterScopeTestReconciler)) + .build(), context); + + return EventSourceInitializer.nameEventSources(es); + } + + private void ensureStatus(P resource) { + if (resource.getStatus() == null) { + resource.setStatus(new BoundedCacheTestStatus()); + } + } + + public static BoundedItemStore boundedItemStore( + KubernetesClient client, Class rClass, + Duration accessExpireDuration, + // max size is only for testing purposes + long cacheMaxSize) { + Cache cache = Caffeine.newBuilder() + .expireAfterAccess(accessExpireDuration) + .maximumSize(cacheMaxSize) + .build(); + return CaffeineBoundedItemStores.boundedItemStore(client, rClass, cache); + } +} diff --git a/caffeine-bounded-cache-support/src/test/java/io/javaoperatorsdk/operator/processing/event/source/cache/sample/clusterscope/BoundedCacheClusterScopeTestCustomResource.java b/caffeine-bounded-cache-support/src/test/java/io/javaoperatorsdk/operator/processing/event/source/cache/sample/clusterscope/BoundedCacheClusterScopeTestCustomResource.java new file mode 100644 index 0000000000..a77416715e --- /dev/null +++ b/caffeine-bounded-cache-support/src/test/java/io/javaoperatorsdk/operator/processing/event/source/cache/sample/clusterscope/BoundedCacheClusterScopeTestCustomResource.java @@ -0,0 +1,15 @@ +package io.javaoperatorsdk.operator.processing.event.source.cache.sample.clusterscope; + +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; +import io.javaoperatorsdk.operator.processing.event.source.cache.sample.namespacescope.BoundedCacheTestSpec; +import io.javaoperatorsdk.operator.processing.event.source.cache.sample.namespacescope.BoundedCacheTestStatus; + +@Group("sample.javaoperatorsdk") +@Version("v1") +@ShortNames("bccs") +public class BoundedCacheClusterScopeTestCustomResource + extends CustomResource { +} diff --git a/caffeine-bounded-cache-support/src/test/java/io/javaoperatorsdk/operator/processing/event/source/cache/sample/clusterscope/BoundedCacheClusterScopeTestReconciler.java b/caffeine-bounded-cache-support/src/test/java/io/javaoperatorsdk/operator/processing/event/source/cache/sample/clusterscope/BoundedCacheClusterScopeTestReconciler.java new file mode 100644 index 0000000000..a154659164 --- /dev/null +++ b/caffeine-bounded-cache-support/src/test/java/io/javaoperatorsdk/operator/processing/event/source/cache/sample/clusterscope/BoundedCacheClusterScopeTestReconciler.java @@ -0,0 +1,10 @@ +package io.javaoperatorsdk.operator.processing.event.source.cache.sample.clusterscope; + +import io.javaoperatorsdk.operator.api.reconciler.*; +import io.javaoperatorsdk.operator.processing.event.source.cache.sample.AbstractTestReconciler; + +@ControllerConfiguration +public class BoundedCacheClusterScopeTestReconciler extends + AbstractTestReconciler { + +} diff --git a/caffeine-bounded-cache-support/src/test/java/io/javaoperatorsdk/operator/processing/event/source/cache/sample/namespacescope/BoundedCacheTestCustomResource.java b/caffeine-bounded-cache-support/src/test/java/io/javaoperatorsdk/operator/processing/event/source/cache/sample/namespacescope/BoundedCacheTestCustomResource.java new file mode 100644 index 0000000000..a5e37917ba --- /dev/null +++ b/caffeine-bounded-cache-support/src/test/java/io/javaoperatorsdk/operator/processing/event/source/cache/sample/namespacescope/BoundedCacheTestCustomResource.java @@ -0,0 +1,14 @@ +package io.javaoperatorsdk.operator.processing.event.source.cache.sample.namespacescope; + +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("bct") +public class BoundedCacheTestCustomResource + extends CustomResource implements Namespaced { +} diff --git a/caffeine-bounded-cache-support/src/test/java/io/javaoperatorsdk/operator/processing/event/source/cache/sample/namespacescope/BoundedCacheTestReconciler.java b/caffeine-bounded-cache-support/src/test/java/io/javaoperatorsdk/operator/processing/event/source/cache/sample/namespacescope/BoundedCacheTestReconciler.java new file mode 100644 index 0000000000..211877b361 --- /dev/null +++ b/caffeine-bounded-cache-support/src/test/java/io/javaoperatorsdk/operator/processing/event/source/cache/sample/namespacescope/BoundedCacheTestReconciler.java @@ -0,0 +1,10 @@ +package io.javaoperatorsdk.operator.processing.event.source.cache.sample.namespacescope; + +import io.javaoperatorsdk.operator.api.reconciler.ControllerConfiguration; +import io.javaoperatorsdk.operator.processing.event.source.cache.sample.AbstractTestReconciler; + +@ControllerConfiguration +public class BoundedCacheTestReconciler + extends AbstractTestReconciler { + +} diff --git a/caffeine-bounded-cache-support/src/test/java/io/javaoperatorsdk/operator/processing/event/source/cache/sample/namespacescope/BoundedCacheTestSpec.java b/caffeine-bounded-cache-support/src/test/java/io/javaoperatorsdk/operator/processing/event/source/cache/sample/namespacescope/BoundedCacheTestSpec.java new file mode 100644 index 0000000000..63e5876267 --- /dev/null +++ b/caffeine-bounded-cache-support/src/test/java/io/javaoperatorsdk/operator/processing/event/source/cache/sample/namespacescope/BoundedCacheTestSpec.java @@ -0,0 +1,25 @@ +package io.javaoperatorsdk.operator.processing.event.source.cache.sample.namespacescope; + +public class BoundedCacheTestSpec { + + private String data; + private String targetNamespace; + + public String getData() { + return data; + } + + public BoundedCacheTestSpec setData(String data) { + this.data = data; + return this; + } + + public String getTargetNamespace() { + return targetNamespace; + } + + public BoundedCacheTestSpec setTargetNamespace(String targetNamespace) { + this.targetNamespace = targetNamespace; + return this; + } +} diff --git a/caffeine-bounded-cache-support/src/test/java/io/javaoperatorsdk/operator/processing/event/source/cache/sample/namespacescope/BoundedCacheTestStatus.java b/caffeine-bounded-cache-support/src/test/java/io/javaoperatorsdk/operator/processing/event/source/cache/sample/namespacescope/BoundedCacheTestStatus.java new file mode 100644 index 0000000000..03a311529e --- /dev/null +++ b/caffeine-bounded-cache-support/src/test/java/io/javaoperatorsdk/operator/processing/event/source/cache/sample/namespacescope/BoundedCacheTestStatus.java @@ -0,0 +1,6 @@ +package io.javaoperatorsdk.operator.processing.event.source.cache.sample.namespacescope; + +import io.javaoperatorsdk.operator.api.ObservedGenerationAwareStatus; + +public class BoundedCacheTestStatus extends ObservedGenerationAwareStatus { +} diff --git a/caffeine-bounded-cache-support/src/test/resources/log4j2.xml b/caffeine-bounded-cache-support/src/test/resources/log4j2.xml new file mode 100644 index 0000000000..f23cf772dd --- /dev/null +++ b/caffeine-bounded-cache-support/src/test/resources/log4j2.xml @@ -0,0 +1,13 @@ + + + + + + + + + + + + + diff --git a/docs/documentation/features.md b/docs/documentation/features.md index ac4f8dfc69..4dc1a92b4c 100644 --- a/docs/documentation/features.md +++ b/docs/documentation/features.md @@ -756,3 +756,29 @@ with a `mycrs` plural form will result in 2 files: **NOTE:** > Quarkus users using the `quarkus-operator-sdk` extension do not need to add any extra dependency > to get their CRD generated as this is handled by the extension itself. + +## Optimizing Caches + +One of the ideas around operator pattern, is that all the relevant resources are cached, thus reconciliation is usually +very fast (especially if it does not need to update resources) since it's mostly working with in memory state. +However or large clusters, caching huge amount of primary and secondary resources might consume lots of memory. +There are some semi-experimental (experimental in terms that it works, but we need feedback from real production usage) +features to optimize memory usage of controllers. + +### Bounded Caches for Informers + +Limiting caches for informers - thus for Kubernetes resources, both controllers primary resource - is supported for now. +The idea with the implementation that is provided, is that resources are in the cache for a limited time. +So for use cases, when a resource is only frequently reconciled when it is created, and later no or +occasionally reconciled, will be evicted from the cache, since the resources are not accessed. +If a resource accessed in the future but not in the cache, the bounded cache implementation will fetch it from +the service if needed. +Note that on start of a controller all the resources are reconciled, for this reason explicitly setting a maximal +size of a cache might not be ideal. In other words it is desired to have all the resources in the cache at startup, +but not later if not accessed. + +See usage of the related implementation using Caffein cache in integration tests for [primary resource](https://github.com/java-operator-sdk/java-operator-sdk/blob/10e11e587447667ef0da1ddb29e0ba15fcd24ada/caffein-bounded-cache-support/src/test/java/io/javaoperatorsdk/operator/processing/event/source/cache/CaffeinBoundedCacheNamespacedIT.java#L19-L19) and for an [informer](https://github.com/java-operator-sdk/java-operator-sdk/blob/10e11e587447667ef0da1ddb29e0ba15fcd24ada/caffein-bounded-cache-support/src/test/java/io/javaoperatorsdk/operator/processing/event/source/cache/sample/AbstractTestReconciler.java#L84-L93). + +See also [CaffeinBoundedItemStores](https://github.com/java-operator-sdk/java-operator-sdk/blob/10e11e587447667ef0da1ddb29e0ba15fcd24ada/caffein-bounded-cache-support/src/main/java/io/javaoperatorsdk/operator/processing/event/source/cache/CaffeinBoundedItemStores.java#L22-L22) + + diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/BaseConfigurationService.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/BaseConfigurationService.java index e294e0babf..772a16fe71 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/BaseConfigurationService.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/BaseConfigurationService.java @@ -13,6 +13,7 @@ import org.slf4j.LoggerFactory; import io.fabric8.kubernetes.api.model.HasMetadata; +import io.fabric8.kubernetes.client.informers.cache.ItemStore; import io.javaoperatorsdk.operator.OperatorException; import io.javaoperatorsdk.operator.ReconcilerUtils; import io.javaoperatorsdk.operator.api.config.Utils.Configurator; @@ -117,14 +118,14 @@ protected

ControllerConfiguration

configFor(Reconcile final var associatedReconcilerClass = ResolvedControllerConfiguration.getAssociatedReconcilerClassName(reconciler.getClass()); + final var context = Utils.contextFor(name); final Class retryClass = annotation.retry(); final var retry = Utils.instantiateAndConfigureIfNeeded(retryClass, Retry.class, - Utils.contextFor(name, null, null), configuratorFor(Retry.class, reconciler)); + context, configuratorFor(Retry.class, reconciler)); final Class rateLimiterClass = annotation.rateLimiter(); final var rateLimiter = Utils.instantiateAndConfigureIfNeeded(rateLimiterClass, - RateLimiter.class, - Utils.contextFor(name, null, null), configuratorFor(RateLimiter.class, reconciler)); + RateLimiter.class, context, configuratorFor(RateLimiter.class, reconciler)); final var reconciliationInterval = annotation.maxReconciliationInterval(); long interval = -1; @@ -138,12 +139,9 @@ protected

ControllerConfiguration

configFor(Reconcile resourceClass, name, generationAware, associatedReconcilerClass, retry, rateLimiter, ResolvedControllerConfiguration.getMaxReconciliationInterval(interval, timeUnit), - Utils.instantiate(annotation.onAddFilter(), OnAddFilter.class, - Utils.contextFor(name, null, null)), - Utils.instantiate(annotation.onUpdateFilter(), OnUpdateFilter.class, - Utils.contextFor(name, null, null)), - Utils.instantiate(annotation.genericFilter(), GenericFilter.class, - Utils.contextFor(name, null, null)), + Utils.instantiate(annotation.onAddFilter(), OnAddFilter.class, context), + Utils.instantiate(annotation.onUpdateFilter(), OnUpdateFilter.class, context), + Utils.instantiate(annotation.genericFilter(), GenericFilter.class, context), Set.of(valueOrDefault(annotation, io.javaoperatorsdk.operator.api.reconciler.ControllerConfiguration::namespaces, DEFAULT_NAMESPACES_SET.toArray(String[]::new))), @@ -153,7 +151,8 @@ protected

ControllerConfiguration

configFor(Reconcile valueOrDefault(annotation, io.javaoperatorsdk.operator.api.reconciler.ControllerConfiguration::labelSelector, Constants.NO_VALUE_SET), - null); + null, + Utils.instantiate(annotation.itemStore(), ItemStore.class, context)); ResourceEventFilter

answer = deprecatedEventFilter(annotation); config.setEventFilter(answer != null ? answer : ResourceEventFilters.passthrough()); diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ControllerConfigurationOverrider.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ControllerConfigurationOverrider.java index 74a1d78a07..7f9bf45b58 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ControllerConfigurationOverrider.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ControllerConfigurationOverrider.java @@ -8,6 +8,7 @@ import java.util.Set; import io.fabric8.kubernetes.api.model.HasMetadata; +import io.fabric8.kubernetes.client.informers.cache.ItemStore; import io.javaoperatorsdk.operator.api.config.dependent.DependentResourceSpec; import io.javaoperatorsdk.operator.processing.event.rate.RateLimiter; import io.javaoperatorsdk.operator.processing.event.source.controller.ResourceEventFilter; @@ -36,6 +37,7 @@ public class ControllerConfigurationOverrider { private GenericFilter genericFilter; private RateLimiter rateLimiter; private Map configurations; + private ItemStore itemStore; private String name; private ControllerConfigurationOverrider(ControllerConfiguration original) { @@ -154,6 +156,11 @@ public ControllerConfigurationOverrider withGenericFilter(GenericFilter ge return this; } + public ControllerConfigurationOverrider withItemStore(ItemStore itemStore) { + this.itemStore = itemStore; + return this; + } + public ControllerConfigurationOverrider withName(String name) { this.name = name; return this; @@ -181,7 +188,7 @@ public ControllerConfiguration build() { generationAware, original.getAssociatedReconcilerClassName(), retry, rateLimiter, reconciliationMaxInterval, onAddFilter, onUpdateFilter, genericFilter, original.getDependentResources(), - namespaces, finalizer, labelSelector, configurations); + namespaces, finalizer, labelSelector, configurations, itemStore); overridden.setEventFilter(customResourcePredicate); return overridden; } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/DefaultResourceConfiguration.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/DefaultResourceConfiguration.java index b85bd76e9a..a612f2d136 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/DefaultResourceConfiguration.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/DefaultResourceConfiguration.java @@ -4,6 +4,7 @@ import java.util.Set; import io.fabric8.kubernetes.api.model.HasMetadata; +import io.fabric8.kubernetes.client.informers.cache.ItemStore; import io.javaoperatorsdk.operator.ReconcilerUtils; import io.javaoperatorsdk.operator.processing.event.source.filter.GenericFilter; import io.javaoperatorsdk.operator.processing.event.source.filter.OnAddFilter; @@ -19,10 +20,11 @@ public class DefaultResourceConfiguration private final GenericFilter genericFilter; private final String labelSelector; private final Set namespaces; + private final ItemStore itemStore; protected DefaultResourceConfiguration(Class resourceClass, Set namespaces, String labelSelector, OnAddFilter onAddFilter, - OnUpdateFilter onUpdateFilter, GenericFilter genericFilter) { + OnUpdateFilter onUpdateFilter, GenericFilter genericFilter, ItemStore itemStore) { this.resourceClass = resourceClass; this.resourceTypeName = ReconcilerUtils.getResourceTypeName(resourceClass); this.onAddFilter = onAddFilter; @@ -31,6 +33,7 @@ protected DefaultResourceConfiguration(Class resourceClass, this.namespaces = ResourceConfiguration.ensureValidNamespaces(namespaces); this.labelSelector = ResourceConfiguration.ensureValidLabelSelector(labelSelector); + this.itemStore = itemStore; } @Override @@ -66,4 +69,9 @@ public Optional> onUpdateFilter() { public Optional> genericFilter() { return Optional.ofNullable(genericFilter); } + + @Override + public Optional> getItemStore() { + return Optional.ofNullable(itemStore); + } } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ResolvedControllerConfiguration.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ResolvedControllerConfiguration.java index 5513b43060..844724fa48 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ResolvedControllerConfiguration.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ResolvedControllerConfiguration.java @@ -5,6 +5,7 @@ import java.util.concurrent.TimeUnit; import io.fabric8.kubernetes.api.model.HasMetadata; +import io.fabric8.kubernetes.client.informers.cache.ItemStore; import io.javaoperatorsdk.operator.api.config.dependent.DependentResourceConfigurationProvider; import io.javaoperatorsdk.operator.api.config.dependent.DependentResourceSpec; import io.javaoperatorsdk.operator.api.reconciler.Reconciler; @@ -29,6 +30,7 @@ public class ResolvedControllerConfiguration

private final Duration maxReconciliationInterval; private final String finalizer; private final Map configurations; + private final ItemStore

itemStore; private ResourceEventFilter

eventFilter; private List dependentResources; @@ -40,7 +42,8 @@ public ResolvedControllerConfiguration(Class

resourceClass, ControllerConfigu other.onAddFilter().orElse(null), other.onUpdateFilter().orElse(null), other.genericFilter().orElse(null), other.getDependentResources(), other.getNamespaces(), - other.getFinalizerName(), other.getLabelSelector(), Collections.emptyMap()); + other.getFinalizerName(), other.getLabelSelector(), Collections.emptyMap(), + other.getItemStore().orElse(null)); } public static Duration getMaxReconciliationInterval(long interval, TimeUnit timeUnit) { @@ -67,10 +70,10 @@ public ResolvedControllerConfiguration(Class

resourceClass, String name, GenericFilter

genericFilter, List dependentResources, Set namespaces, String finalizer, String labelSelector, - Map configurations) { + Map configurations, ItemStore

itemStore) { this(resourceClass, name, generationAware, associatedReconcilerClassName, retry, rateLimiter, maxReconciliationInterval, onAddFilter, onUpdateFilter, genericFilter, - namespaces, finalizer, labelSelector, configurations); + namespaces, finalizer, labelSelector, configurations, itemStore); setDependentResources(dependentResources); } @@ -79,8 +82,9 @@ protected ResolvedControllerConfiguration(Class

resourceClass, String name, RateLimiter rateLimiter, Duration maxReconciliationInterval, OnAddFilter

onAddFilter, OnUpdateFilter

onUpdateFilter, GenericFilter

genericFilter, Set namespaces, String finalizer, String labelSelector, - Map configurations) { - super(resourceClass, namespaces, labelSelector, onAddFilter, onUpdateFilter, genericFilter); + Map configurations, ItemStore

itemStore) { + super(resourceClass, namespaces, labelSelector, onAddFilter, onUpdateFilter, genericFilter, + itemStore); this.name = ControllerConfiguration.ensureValidName(name, associatedReconcilerClassName); this.generationAware = generationAware; this.associatedReconcilerClassName = associatedReconcilerClassName; @@ -88,7 +92,7 @@ protected ResolvedControllerConfiguration(Class

resourceClass, String name, this.rateLimiter = ensureRateLimiter(rateLimiter); this.maxReconciliationInterval = maxReconciliationInterval; this.configurations = configurations != null ? configurations : Collections.emptyMap(); - + this.itemStore = itemStore; this.finalizer = ControllerConfiguration.ensureValidFinalizerName(finalizer, getResourceTypeName()); } @@ -162,4 +166,9 @@ protected void setEventFilter(ResourceEventFilter

eventFilter) { public Object getConfigurationFor(DependentResourceSpec spec) { return configurations.get(spec); } + + @Override + public Optional> getItemStore() { + return Optional.ofNullable(itemStore); + } } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ResourceConfiguration.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ResourceConfiguration.java index 81b7fdf4a7..d3a8379d46 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ResourceConfiguration.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ResourceConfiguration.java @@ -6,6 +6,7 @@ import java.util.Set; import io.fabric8.kubernetes.api.model.HasMetadata; +import io.fabric8.kubernetes.client.informers.cache.ItemStore; import io.javaoperatorsdk.operator.OperatorException; import io.javaoperatorsdk.operator.ReconcilerUtils; import io.javaoperatorsdk.operator.api.reconciler.Constants; @@ -122,4 +123,23 @@ default Set getEffectiveNamespaces() { } return targetNamespaces; } + + /** + * Replaces the item store in informer. See underling + * method in fabric8 client informer implementation. + * + * The main goal, is to be able to use limited caches. + * + * See {@link io.javaoperatorsdk.operator.processing.event.source.cache.BoundedItemStore} and + * + * CaffeinBoundedCache + * + * @return Optional ItemStore implementation. If present this item store will be used inside the + * informers. + */ + default Optional> getItemStore() { + return Optional.empty(); + } } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/Utils.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/Utils.java index bee5b96cbe..ec244f224c 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/Utils.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/Utils.java @@ -203,9 +203,7 @@ public static T instantiateAndConfigureIfNeeded(Class targetCla } try { - final Constructor constructor = targetClass.getDeclaredConstructor(); - constructor.setAccessible(true); - final var instance = constructor.newInstance(); + final var instance = getConstructor(targetClass).newInstance(); if (configurator != null) { configurator.configure(instance); @@ -213,13 +211,25 @@ public static T instantiateAndConfigureIfNeeded(Class targetCla return instance; } catch (InstantiationException | IllegalAccessException | InvocationTargetException - | NoSuchMethodException e) { + | IllegalStateException e) { throw new OperatorException("Couldn't instantiate " + expectedType.getSimpleName() + " '" - + targetClass.getName() + "': you need to provide an accessible no-arg constructor." + + targetClass.getName() + "'." + (context != null ? " Context: " + context : ""), e); } } + public static Constructor getConstructor(Class targetClass) { + final Constructor constructor; + try { + constructor = targetClass.getDeclaredConstructor(); + } catch (NoSuchMethodException e) { + throw new IllegalStateException( + "Couldn't find a no-arg constructor for " + targetClass.getName(), e); + } + constructor.setAccessible(true); + return constructor; + } + public static T instantiate(Class toInstantiate, Class expectedType, String context) { return instantiateAndConfigureIfNeeded(toInstantiate, expectedType, context, null); @@ -237,6 +247,10 @@ public static String contextFor(ControllerConfiguration controllerConfigurati return contextFor(controllerConfiguration.getName(), dependentType, configurationAnnotation); } + public static String contextFor(String reconcilerName) { + return contextFor(reconcilerName, null, null); + } + @SuppressWarnings("rawtypes") public static String contextFor(String reconcilerName, Class dependentType, diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/informer/InformerConfiguration.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/informer/InformerConfiguration.java index c6ee847cb0..cafce36213 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/informer/InformerConfiguration.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/informer/InformerConfiguration.java @@ -5,6 +5,7 @@ import java.util.Set; import io.fabric8.kubernetes.api.model.HasMetadata; +import io.fabric8.kubernetes.client.informers.cache.ItemStore; import io.javaoperatorsdk.operator.api.config.DefaultResourceConfiguration; import io.javaoperatorsdk.operator.api.config.ResourceConfiguration; import io.javaoperatorsdk.operator.api.config.Utils; @@ -38,8 +39,10 @@ protected DefaultInformerConfiguration(String labelSelector, OnAddFilter onAddFilter, OnUpdateFilter onUpdateFilter, OnDeleteFilter onDeleteFilter, - GenericFilter genericFilter) { - super(resourceClass, namespaces, labelSelector, onAddFilter, onUpdateFilter, genericFilter); + GenericFilter genericFilter, + ItemStore itemStore) { + super(resourceClass, namespaces, labelSelector, onAddFilter, onUpdateFilter, genericFilter, + itemStore); this.followControllerNamespaceChanges = followControllerNamespaceChanges; this.primaryToSecondaryMapper = primaryToSecondaryMapper; @@ -103,6 +106,7 @@ class InformerConfigurationBuilder { private OnDeleteFilter onDeleteFilter; private GenericFilter genericFilter; private boolean inheritControllerNamespacesOnChange = false; + private ItemStore itemStore; private InformerConfigurationBuilder(Class resourceClass) { this.resourceClass = resourceClass; @@ -203,12 +207,17 @@ public InformerConfigurationBuilder withGenericFilter(GenericFilter generi return this; } + public InformerConfigurationBuilder withItemStore(ItemStore itemStore) { + this.itemStore = itemStore; + return this; + } + public InformerConfiguration build() { return new DefaultInformerConfiguration<>(labelSelector, resourceClass, primaryToSecondaryMapper, secondaryToPrimaryMapper, namespaces, inheritControllerNamespacesOnChange, onAddFilter, onUpdateFilter, - onDeleteFilter, genericFilter); + onDeleteFilter, genericFilter, itemStore); } } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/ControllerConfiguration.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/ControllerConfiguration.java index ec76adf89d..a4a4a78797 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/ControllerConfiguration.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/ControllerConfiguration.java @@ -6,6 +6,7 @@ import java.lang.annotation.RetentionPolicy; import java.lang.annotation.Target; +import io.fabric8.kubernetes.client.informers.cache.ItemStore; import io.javaoperatorsdk.operator.api.reconciler.dependent.Dependent; import io.javaoperatorsdk.operator.processing.event.rate.LinearRateLimiter; import io.javaoperatorsdk.operator.processing.event.rate.RateLimiter; @@ -118,4 +119,6 @@ MaxReconciliationInterval maxReconciliationInterval() default @MaxReconciliation * accessible no-arg constructor. */ Class rateLimiter() default LinearRateLimiter.class; + + Class itemStore() default ItemStore.class; } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/cache/BoundedCache.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/cache/BoundedCache.java new file mode 100644 index 0000000000..1651d44dc7 --- /dev/null +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/cache/BoundedCache.java @@ -0,0 +1,11 @@ +package io.javaoperatorsdk.operator.processing.event.source.cache; + +public interface BoundedCache { + + R get(K key); + + R remove(K key); + + void put(K key, R object); + +} diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/cache/BoundedItemStore.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/cache/BoundedItemStore.java new file mode 100644 index 0000000000..4f0fcad280 --- /dev/null +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/cache/BoundedItemStore.java @@ -0,0 +1,148 @@ +package io.javaoperatorsdk.operator.processing.event.source.cache; + +import java.lang.reflect.Constructor; +import java.lang.reflect.InvocationTargetException; +import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; +import java.util.function.Function; +import java.util.stream.Stream; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import io.fabric8.kubernetes.api.model.HasMetadata; +import io.fabric8.kubernetes.api.model.ObjectMetaBuilder; +import io.fabric8.kubernetes.client.KubernetesClient; +import io.fabric8.kubernetes.client.informers.cache.Cache; +import io.fabric8.kubernetes.client.informers.cache.ItemStore; +import io.javaoperatorsdk.operator.api.config.Utils; + +public class BoundedItemStore + implements ItemStore { + + private static final Logger log = LoggerFactory.getLogger(BoundedItemStore.class); + + private final ResourceFetcher resourceFetcher; + private final BoundedCache cache; + private final Function keyFunction; + private final Map existingMinimalResources = new ConcurrentHashMap<>(); + private final Constructor resourceConstructor; + + public BoundedItemStore(BoundedCache cache, Class resourceClass, + KubernetesClient client) { + this(cache, resourceClass, namespaceKeyFunc(), + new KubernetesResourceFetcher<>(resourceClass, client)); + } + + public BoundedItemStore(BoundedCache cache, + Class resourceClass, + Function keyFunction, + ResourceFetcher resourceFetcher) { + this.resourceFetcher = resourceFetcher; + this.cache = cache; + this.keyFunction = keyFunction; + this.resourceConstructor = Utils.getConstructor(resourceClass); + } + + @Override + public String getKey(R obj) { + return keyFunction.apply(obj); + } + + @Override + public synchronized R put(String key, R obj) { + var result = existingMinimalResources.get(key); + cache.put(key, obj); + existingMinimalResources.put(key, createMinimalResource(obj)); + return result; + } + + private R createMinimalResource(R obj) { + try { + R minimal = resourceConstructor.newInstance(); + final var metadata = obj.getMetadata(); + minimal.setMetadata(new ObjectMetaBuilder() + .withName(metadata.getName()) + .withNamespace(metadata.getNamespace()) + .withResourceVersion(metadata.getResourceVersion()) + .build()); + return minimal; + } catch (InstantiationException | IllegalAccessException | InvocationTargetException e) { + throw new IllegalStateException(e); + } + } + + @Override + public synchronized R remove(String key) { + var fullValue = cache.remove(key); + var minimalValue = existingMinimalResources.remove(key); + return fullValue != null ? fullValue : minimalValue; + } + + @Override + public Stream keySet() { + return existingMinimalResources.keySet().stream(); + } + + @Override + public Stream values() { + return existingMinimalResources.values().stream(); + } + + @Override + public int size() { + return existingMinimalResources.size(); + } + + @Override + public R get(String key) { + var res = cache.get(key); + if (res != null) { + return res; + } + if (!existingMinimalResources.containsKey(key)) { + return null; + } else { + return refreshMissingStateFromServer(key); + } + } + + @Override + public boolean isFullState() { + return false; + } + + public static Function namespaceKeyFunc() { + return r -> Cache.namespaceKeyFunc(r.getMetadata().getNamespace(), r.getMetadata().getName()); + } + + protected R refreshMissingStateFromServer(String key) { + log.debug("Fetching resource from server for key: {}", key); + var newRes = resourceFetcher.fetchResource(key); + synchronized (this) { + log.debug("Fetched resource: {}", newRes); + var actual = cache.get(key); + if (newRes == null) { + // double-checking if actual, not received since. + // If received we just return. Since the resource from informer should be always leading, + // even if the fetched resource is null, this will be eventually received as an event. + if (actual == null) { + existingMinimalResources.remove(key); + return null; + } else { + return actual; + } + } + // Just want to put the fetched resource if there is still no resource published from + // different source. In case of informers actually multiple events might arrive, therefore non + // fetched resource should take always precedence. + if (actual == null) { + cache.put(key, newRes); + existingMinimalResources.put(key, createMinimalResource(newRes)); + return newRes; + } else { + return actual; + } + } + } +} diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/cache/KubernetesResourceFetcher.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/cache/KubernetesResourceFetcher.java new file mode 100644 index 0000000000..ad996afc36 --- /dev/null +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/cache/KubernetesResourceFetcher.java @@ -0,0 +1,47 @@ +package io.javaoperatorsdk.operator.processing.event.source.cache; + +import java.util.function.Function; + +import io.fabric8.kubernetes.api.model.HasMetadata; +import io.fabric8.kubernetes.client.KubernetesClient; +import io.javaoperatorsdk.operator.processing.event.ResourceID; + +public class KubernetesResourceFetcher + implements ResourceFetcher { + + private final Class rClass; + private final KubernetesClient client; + private final Function resourceIDFunction; + + public KubernetesResourceFetcher(Class rClass, KubernetesClient client) { + this(rClass, client, inverseNamespaceKeyFunction()); + } + + public KubernetesResourceFetcher(Class rClass, + KubernetesClient client, + Function resourceIDFunction) { + this.rClass = rClass; + this.client = client; + this.resourceIDFunction = resourceIDFunction; + } + + @Override + public R fetchResource(String key) { + var resourceId = resourceIDFunction.apply(key); + return resourceId.getNamespace().map(ns -> client.resources(rClass).inNamespace(ns) + .withName(resourceId.getName()).get()) + .orElse(client.resources(rClass).withName(resourceId.getName()).get()); + } + + public static Function inverseNamespaceKeyFunction() { + return s -> { + int delimiterIndex = s.indexOf("/"); + if (delimiterIndex == -1) { + return new ResourceID(s); + } else { + return new ResourceID(s.substring(delimiterIndex + 1), s.substring(0, delimiterIndex)); + } + }; + } + +} diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/cache/ResourceFetcher.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/cache/ResourceFetcher.java new file mode 100644 index 0000000000..9cc4fe7a35 --- /dev/null +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/cache/ResourceFetcher.java @@ -0,0 +1,7 @@ +package io.javaoperatorsdk.operator.processing.event.source.cache; + +public interface ResourceFetcher { + + R fetchResource(K key); + +} diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerManager.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerManager.java index 0a6b8327c5..db4ce49076 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerManager.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerManager.java @@ -127,6 +127,7 @@ private InformerWrapper createEventSource( FilterWatchListDeletable, Resource> filteredBySelectorClient, ResourceEventHandler eventHandler, String namespaceIdentifier) { var informer = filteredBySelectorClient.runnableInformer(0); + configuration.getItemStore().ifPresent(informer::itemStore); var source = new InformerWrapper<>(informer, namespaceIdentifier); source.addEventHandler(eventHandler); diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/cache/BoundedItemStoreTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/cache/BoundedItemStoreTest.java new file mode 100644 index 0000000000..9381aedd0d --- /dev/null +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/cache/BoundedItemStoreTest.java @@ -0,0 +1,111 @@ +package io.javaoperatorsdk.operator.processing.event.source.cache; + +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +import io.fabric8.kubernetes.api.model.HasMetadata; +import io.fabric8.kubernetes.client.informers.cache.Cache; +import io.javaoperatorsdk.operator.TestUtils; +import io.javaoperatorsdk.operator.sample.simple.TestCustomResource; + +import static io.javaoperatorsdk.operator.processing.event.source.cache.BoundedItemStore.namespaceKeyFunc; +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +class BoundedItemStoreTest { + + private BoundedItemStore boundedItemStore; + @SuppressWarnings("unchecked") + private final BoundedCache boundedCache = mock(BoundedCache.class); + @SuppressWarnings("unchecked") + private final ResourceFetcher resourceFetcher = + mock(ResourceFetcher.class); + + @BeforeEach + void setup() { + boundedItemStore = new BoundedItemStore<>(boundedCache, + TestCustomResource.class, + namespaceKeyFunc(), + resourceFetcher); + } + + @Test + void shouldNotFetchResourcesFromServerIfNotKnown() { + var res = boundedItemStore.get(testRes1Key()); + + assertThat(res).isNull(); + verify(resourceFetcher, never()).fetchResource(any()); + } + + @Test + void getsResourceFromServerIfNotInCache() { + boundedItemStore.put(testRes1Key(), + TestUtils.testCustomResource1()); + when(resourceFetcher.fetchResource(testRes1Key())) + .thenReturn(TestUtils.testCustomResource1()); + + var res = boundedItemStore.get(testRes1Key()); + + assertThat(res).isNotNull(); + verify(resourceFetcher, times(1)).fetchResource(any()); + } + + @Test + void removesResourcesNotFoundOnServerFromStore() { + boundedItemStore.put(testRes1Key(), + TestUtils.testCustomResource1()); + when(resourceFetcher.fetchResource(testRes1Key())) + .thenReturn(null); + + var res = boundedItemStore.get(testRes1Key()); + + assertThat(res).isNull(); + assertThat(boundedItemStore.keySet()).isEmpty(); + } + + @Test + void removesResourceFromCache() { + boundedItemStore.put(testRes1Key(), + TestUtils.testCustomResource1()); + + boundedItemStore.remove(testRes1Key()); + + var res = boundedItemStore.get(testRes1Key()); + verify(resourceFetcher, never()).fetchResource(any()); + assertThat(res).isNull(); + assertThat(boundedItemStore.keySet()).isEmpty(); + } + + @Test + void readingKeySetDoesNotReadFromBoundedCache() { + boundedItemStore.put(testRes1Key(), + TestUtils.testCustomResource1()); + + boundedItemStore.keySet(); + + verify(boundedCache, never()).get(any()); + } + + @Test + void readingValuesDoesNotReadFromBoundedCache() { + boundedItemStore.put(testRes1Key(), + TestUtils.testCustomResource1()); + + boundedItemStore.values(); + + verify(boundedCache, never()).get(any()); + } + + String key(HasMetadata r) { + return Cache.namespaceKeyFunc(r.getMetadata().getNamespace(), r.getMetadata().getName()); + } + + String testRes1Key() { + return key(TestUtils.testCustomResource1()); + } +} diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/cache/KubernetesResourceFetcherTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/cache/KubernetesResourceFetcherTest.java new file mode 100644 index 0000000000..1158e00295 --- /dev/null +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/cache/KubernetesResourceFetcherTest.java @@ -0,0 +1,48 @@ +package io.javaoperatorsdk.operator.processing.event.source.cache; + +import org.junit.jupiter.api.Test; + +import io.fabric8.kubernetes.api.model.ConfigMap; +import io.fabric8.kubernetes.api.model.HasMetadata; +import io.fabric8.kubernetes.api.model.ObjectMetaBuilder; +import io.fabric8.kubernetes.api.model.apiextensions.v1.CustomResourceDefinition; + +import static org.assertj.core.api.Assertions.assertThat; + +class KubernetesResourceFetcherTest { + + public static final String DEFAULT_NAMESPACE = "default"; + public static final String TEST_RESOURCE_NAME = "test1"; + + @Test + void inverseKeyFunction() { + String key = BoundedItemStore.namespaceKeyFunc().apply(namespacedResource()); + var resourceID = KubernetesResourceFetcher.inverseNamespaceKeyFunction().apply(key); + + assertThat(resourceID.getNamespace()).isPresent().get().isEqualTo(DEFAULT_NAMESPACE); + assertThat(resourceID.getName()).isEqualTo(TEST_RESOURCE_NAME); + + key = BoundedItemStore.namespaceKeyFunc().apply(clusterScopedResource()); + resourceID = KubernetesResourceFetcher.inverseNamespaceKeyFunction().apply(key); + + assertThat(resourceID.getNamespace()).isEmpty(); + assertThat(resourceID.getName()).isEqualTo(TEST_RESOURCE_NAME); + } + + private HasMetadata namespacedResource() { + var cm = new ConfigMap(); + cm.setMetadata(new ObjectMetaBuilder() + .withName(TEST_RESOURCE_NAME) + .withNamespace(DEFAULT_NAMESPACE) + .build()); + return cm; + } + + private HasMetadata clusterScopedResource() { + var cm = new CustomResourceDefinition(); + cm.setMetadata(new ObjectMetaBuilder() + .withName(TEST_RESOURCE_NAME) + .build()); + return cm; + } +} diff --git a/pom.xml b/pom.xml index dfc9aa1a67..5c4af4f2f4 100644 --- a/pom.xml +++ b/pom.xml @@ -72,6 +72,7 @@ 1.0 1.8.0 4.10.0 + 3.1.3 0.4.0 @@ -82,6 +83,7 @@ operator-framework micrometer-support sample-operators + caffeine-bounded-cache-support @@ -194,6 +196,11 @@ mockwebserver ${okhttp.version} + + com.github.ben-manes.caffeine + caffeine + ${caffein.version} + io.javaoperatorsdk jenvtest From 715433c0d184b3d7faac3c2a87af45883b43c67e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Attila=20M=C3=A9sz=C3=A1ros?= Date: Wed, 22 Feb 2023 18:34:39 +0100 Subject: [PATCH 12/25] chore: update to fabric8 client 6.4.1 (#1724) Co-authored-by: Chris Laprun --- pom.xml | 2 +- .../javaoperatorsdk/operator/sample/LeaderElectionE2E.java | 2 +- .../operator/sample/MySQLSchemaOperatorE2E.java | 6 +++--- .../javaoperatorsdk/operator/sample/TomcatOperatorE2E.java | 2 +- .../javaoperatorsdk/operator/sample/WebPageOperatorE2E.java | 2 +- .../sample/WebPageOperatorManagedDependentResourcesE2E.java | 2 +- .../WebPageOperatorStandaloneDependentResourcesE2E.java | 2 +- 7 files changed, 9 insertions(+), 9 deletions(-) diff --git a/pom.xml b/pom.xml index 5c4af4f2f4..9c56de08b5 100644 --- a/pom.xml +++ b/pom.xml @@ -43,7 +43,7 @@ https://sonarcloud.io 5.9.1 - 6.3.1 + 6.4.1 1.7.36 2.19.0 5.2.0 diff --git a/sample-operators/leader-election/src/test/java/io/javaoperatorsdk/operator/sample/LeaderElectionE2E.java b/sample-operators/leader-election/src/test/java/io/javaoperatorsdk/operator/sample/LeaderElectionE2E.java index 5512f54fc1..7932472aab 100644 --- a/sample-operators/leader-election/src/test/java/io/javaoperatorsdk/operator/sample/LeaderElectionE2E.java +++ b/sample-operators/leader-election/src/test/java/io/javaoperatorsdk/operator/sample/LeaderElectionE2E.java @@ -165,7 +165,7 @@ void applyCRD() { void applyResources(String path) { try { - List resources = client.load(new FileInputStream(path)).get(); + List resources = client.load(new FileInputStream(path)).items(); resources.forEach(hm -> { hm.getMetadata().setNamespace(namespace); if (hm.getKind().toLowerCase(Locale.ROOT).equals("clusterrolebinding")) { diff --git a/sample-operators/mysql-schema/src/test/java/io/javaoperatorsdk/operator/sample/MySQLSchemaOperatorE2E.java b/sample-operators/mysql-schema/src/test/java/io/javaoperatorsdk/operator/sample/MySQLSchemaOperatorE2E.java index e6aa796656..d65ab98647 100644 --- a/sample-operators/mysql-schema/src/test/java/io/javaoperatorsdk/operator/sample/MySQLSchemaOperatorE2E.java +++ b/sample-operators/mysql-schema/src/test/java/io/javaoperatorsdk/operator/sample/MySQLSchemaOperatorE2E.java @@ -43,8 +43,8 @@ class MySQLSchemaOperatorE2E { infrastructure.add( new NamespaceBuilder().withNewMetadata().withName(MY_SQL_NS).endMetadata().build()); try { - infrastructure.addAll(client.load(new FileInputStream("k8s/mysql-deployment.yaml")).get()); - infrastructure.addAll(client.load(new FileInputStream("k8s/mysql-service.yaml")).get()); + infrastructure.addAll(client.load(new FileInputStream("k8s/mysql-deployment.yaml")).items()); + infrastructure.addAll(client.load(new FileInputStream("k8s/mysql-service.yaml")).items()); } catch (FileNotFoundException e) { e.printStackTrace(); } @@ -67,7 +67,7 @@ boolean isLocal() { .withPortForward(MY_SQL_NS, "app", "mysql", 3306, SchemaDependentResource.LOCAL_PORT) .build() : ClusterDeployedOperatorExtension.builder() - .withOperatorDeployment(client.load(new FileInputStream("k8s/operator.yaml")).get()) + .withOperatorDeployment(client.load(new FileInputStream("k8s/operator.yaml")).items()) .withInfrastructure(infrastructure) .build(); diff --git a/sample-operators/tomcat-operator/src/test/java/io/javaoperatorsdk/operator/sample/TomcatOperatorE2E.java b/sample-operators/tomcat-operator/src/test/java/io/javaoperatorsdk/operator/sample/TomcatOperatorE2E.java index 377d6c540f..929af47e5d 100644 --- a/sample-operators/tomcat-operator/src/test/java/io/javaoperatorsdk/operator/sample/TomcatOperatorE2E.java +++ b/sample-operators/tomcat-operator/src/test/java/io/javaoperatorsdk/operator/sample/TomcatOperatorE2E.java @@ -50,7 +50,7 @@ boolean isLocal() { : ClusterDeployedOperatorExtension.builder() .waitForNamespaceDeletion(false) .withOperatorDeployment( - client.load(new FileInputStream("k8s/operator.yaml")).get()) + client.load(new FileInputStream("k8s/operator.yaml")).items()) .build(); Tomcat getTomcat() { diff --git a/sample-operators/webpage/src/test/java/io/javaoperatorsdk/operator/sample/WebPageOperatorE2E.java b/sample-operators/webpage/src/test/java/io/javaoperatorsdk/operator/sample/WebPageOperatorE2E.java index 81746769c1..03c3d2c2e0 100644 --- a/sample-operators/webpage/src/test/java/io/javaoperatorsdk/operator/sample/WebPageOperatorE2E.java +++ b/sample-operators/webpage/src/test/java/io/javaoperatorsdk/operator/sample/WebPageOperatorE2E.java @@ -31,7 +31,7 @@ public WebPageOperatorE2E() throws FileNotFoundException {} .build() : ClusterDeployedOperatorExtension.builder() .waitForNamespaceDeletion(false) - .withOperatorDeployment(client.load(new FileInputStream("k8s/operator.yaml")).get(), + .withOperatorDeployment(client.load(new FileInputStream("k8s/operator.yaml")).items(), resources -> { Deployment deployment = (Deployment) resources.stream() .filter(r -> r instanceof Deployment).findFirst().orElseThrow(); diff --git a/sample-operators/webpage/src/test/java/io/javaoperatorsdk/operator/sample/WebPageOperatorManagedDependentResourcesE2E.java b/sample-operators/webpage/src/test/java/io/javaoperatorsdk/operator/sample/WebPageOperatorManagedDependentResourcesE2E.java index 3fbf877d64..92b0ce0a41 100644 --- a/sample-operators/webpage/src/test/java/io/javaoperatorsdk/operator/sample/WebPageOperatorManagedDependentResourcesE2E.java +++ b/sample-operators/webpage/src/test/java/io/javaoperatorsdk/operator/sample/WebPageOperatorManagedDependentResourcesE2E.java @@ -29,7 +29,7 @@ public WebPageOperatorManagedDependentResourcesE2E() throws FileNotFoundExceptio .build() : ClusterDeployedOperatorExtension.builder() .waitForNamespaceDeletion(false) - .withOperatorDeployment(client.load(new FileInputStream("k8s/operator.yaml")).get(), + .withOperatorDeployment(client.load(new FileInputStream("k8s/operator.yaml")).items(), resources -> { Deployment deployment = (Deployment) resources.stream() .filter(r -> r instanceof Deployment).findFirst().orElseThrow(); diff --git a/sample-operators/webpage/src/test/java/io/javaoperatorsdk/operator/sample/WebPageOperatorStandaloneDependentResourcesE2E.java b/sample-operators/webpage/src/test/java/io/javaoperatorsdk/operator/sample/WebPageOperatorStandaloneDependentResourcesE2E.java index 1b7a08d71b..e1e9e65f96 100644 --- a/sample-operators/webpage/src/test/java/io/javaoperatorsdk/operator/sample/WebPageOperatorStandaloneDependentResourcesE2E.java +++ b/sample-operators/webpage/src/test/java/io/javaoperatorsdk/operator/sample/WebPageOperatorStandaloneDependentResourcesE2E.java @@ -22,7 +22,7 @@ public WebPageOperatorStandaloneDependentResourcesE2E() throws FileNotFoundExcep .build() : ClusterDeployedOperatorExtension.builder() .waitForNamespaceDeletion(false) - .withOperatorDeployment(client.load(new FileInputStream("k8s/operator.yaml")).get()) + .withOperatorDeployment(client.load(new FileInputStream("k8s/operator.yaml")).items()) .build(); @Override From 34ad8ae14f14e3984f3b2a4ecbd67d21cf2a76ad Mon Sep 17 00:00:00 2001 From: Chris Laprun Date: Mon, 27 Feb 2023 15:16:16 +0100 Subject: [PATCH 13/25] fix: do not change namespaces if they were manually set (#1734) Also remove unneeded implementation of NamespaceChangeable --- .../KubernetesDependentResourceConfig.java | 14 +++----------- 1 file changed, 3 insertions(+), 11 deletions(-) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/KubernetesDependentResourceConfig.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/KubernetesDependentResourceConfig.java index 44a3b0a7bd..4047b25a13 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/KubernetesDependentResourceConfig.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/KubernetesDependentResourceConfig.java @@ -2,7 +2,6 @@ import java.util.Set; -import io.javaoperatorsdk.operator.api.config.NamespaceChangeable; import io.javaoperatorsdk.operator.api.reconciler.Constants; import io.javaoperatorsdk.operator.api.reconciler.ResourceDiscriminator; import io.javaoperatorsdk.operator.processing.event.source.filter.GenericFilter; @@ -12,7 +11,7 @@ import static io.javaoperatorsdk.operator.api.reconciler.Constants.NO_VALUE_SET; -public class KubernetesDependentResourceConfig implements NamespaceChangeable { +public class KubernetesDependentResourceConfig { private Set namespaces = Constants.SAME_AS_CONTROLLER_NAMESPACES_SET; private String labelSelector = NO_VALUE_SET; @@ -89,16 +88,9 @@ public ResourceDiscriminator getResourceDiscriminator() { return resourceDiscriminator; } - @Override - public void changeNamespaces(Set namespaces) { - if (!wereNamespacesConfigured()) { - this.namespacesWereConfigured = true; - setNamespaces(namespaces); - } - } - + @SuppressWarnings("unused") protected void setNamespaces(Set namespaces) { - if (namespaces != null && !namespaces.isEmpty()) { + if (!wereNamespacesConfigured() && namespaces != null && !namespaces.isEmpty()) { this.namespaces = namespaces; } } From 22ee9963f175473dbea9ec9ac2639c0c98c71a3e Mon Sep 17 00:00:00 2001 From: Chris Laprun Date: Fri, 3 Mar 2023 13:20:19 +0100 Subject: [PATCH 14/25] fix: typo caffein -> caffeine (#1795) --- pom.xml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pom.xml b/pom.xml index 9c56de08b5..2cc8f8e888 100644 --- a/pom.xml +++ b/pom.xml @@ -72,7 +72,7 @@ 1.0 1.8.0 4.10.0 - 3.1.3 + 3.1.3 0.4.0 @@ -199,7 +199,7 @@ com.github.ben-manes.caffeine caffeine - ${caffein.version} + ${caffeine.version} io.javaoperatorsdk From 021ca59f8f40e5b931ae82acaeb35efd5eb8cc35 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Attila=20M=C3=A9sz=C3=A1ros?= Date: Wed, 8 Mar 2023 12:12:40 +0100 Subject: [PATCH 15/25] feat: access client from context (#1800) --- .../io/javaoperatorsdk/operator/api/reconciler/Context.java | 3 +++ .../operator/api/reconciler/DefaultContext.java | 6 ++++++ 2 files changed, 9 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 2e4fb98e6f..031b930775 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 @@ -4,6 +4,7 @@ import java.util.Set; import io.fabric8.kubernetes.api.model.HasMetadata; +import io.fabric8.kubernetes.client.KubernetesClient; import io.javaoperatorsdk.operator.api.config.ControllerConfiguration; import io.javaoperatorsdk.operator.api.reconciler.dependent.managed.ManagedDependentResourceContext; import io.javaoperatorsdk.operator.processing.event.EventSourceRetriever; @@ -29,4 +30,6 @@ Optional getSecondaryResource(Class expectedType, ManagedDependentResourceContext managedDependentResourceContext(); EventSourceRetriever

eventSourceRetriever(); + + KubernetesClient getClient(); } 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 c95d5e3d03..8e17b09fc6 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 @@ -5,6 +5,7 @@ import java.util.stream.Collectors; import io.fabric8.kubernetes.api.model.HasMetadata; +import io.fabric8.kubernetes.client.KubernetesClient; import io.javaoperatorsdk.operator.api.config.ControllerConfiguration; import io.javaoperatorsdk.operator.api.reconciler.dependent.managed.DefaultManagedDependentResourceContext; import io.javaoperatorsdk.operator.api.reconciler.dependent.managed.ManagedDependentResourceContext; @@ -69,6 +70,11 @@ public EventSourceRetriever

eventSourceRetriever() { return controller.getEventSourceManager(); } + @Override + public KubernetesClient getClient() { + return controller.getClient(); + } + public DefaultContext

setRetryInfo(RetryInfo retryInfo) { this.retryInfo = retryInfo; return this; From cdc283d45fcd780bf8f6328087480f71ca5f93bd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Attila=20M=C3=A9sz=C3=A1ros?= Date: Fri, 10 Mar 2023 16:06:09 +0100 Subject: [PATCH 16/25] feat: using dynamic ThreadPoolExecutor for reconcile and workflow executor services (#1804) --- .../api/config/ConfigurationService.java | 42 +++++++++++++++---- .../config/ConfigurationServiceOverrider.java | 24 +++++++++++ 2 files changed, 59 insertions(+), 7 deletions(-) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationService.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationService.java index 53f2ae4176..d55984fae1 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationService.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationService.java @@ -3,8 +3,7 @@ import java.time.Duration; import java.util.Optional; import java.util.Set; -import java.util.concurrent.ExecutorService; -import java.util.concurrent.Executors; +import java.util.concurrent.*; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -75,11 +74,12 @@ default boolean checkCRDAndValidateLocalModel() { return false; } - int DEFAULT_RECONCILIATION_THREADS_NUMBER = 10; + int DEFAULT_RECONCILIATION_THREADS_NUMBER = 200; + int MIN_DEFAULT_RECONCILIATION_THREADS_NUMBER = 10; /** - * Retrieves the maximum number of threads the operator can spin out to dispatch reconciliation - * requests to reconcilers + * The maximum number of threads the operator can spin out to dispatch reconciliation requests to + * reconcilers * * @return the maximum number of concurrent reconciliation threads */ @@ -87,12 +87,36 @@ default int concurrentReconciliationThreads() { return DEFAULT_RECONCILIATION_THREADS_NUMBER; } + /** + * The minimum number of threads the operator starts in the thread pool for reconciliations. + * + * @return the minimum number of concurrent reconciliation threads + */ + default int minConcurrentReconciliationThreads() { + return MIN_DEFAULT_RECONCILIATION_THREADS_NUMBER; + } + int DEFAULT_WORKFLOW_EXECUTOR_THREAD_NUMBER = DEFAULT_RECONCILIATION_THREADS_NUMBER; + int MIN_DEFAULT_WORKFLOW_EXECUTOR_THREAD_NUMBER = MIN_DEFAULT_RECONCILIATION_THREADS_NUMBER; + /** + * Retrieves the maximum number of threads the operator can spin out to be used in the workflows. + * + * @return the maximum number of concurrent workflow threads + */ default int concurrentWorkflowExecutorThreads() { return DEFAULT_WORKFLOW_EXECUTOR_THREAD_NUMBER; } + /** + * The minimum number of threads the operator starts in the thread pool for workflows. + * + * @return the minimum number of concurrent workflow threads + */ + default int minConcurrentWorkflowExecutorThreads() { + return MIN_DEFAULT_WORKFLOW_EXECUTOR_THREAD_NUMBER; + } + /** * Used to clone custom resources. It is strongly suggested that implementors override this method * since the default implementation creates a new {@link Cloner} instance each time this method is @@ -136,11 +160,15 @@ default Metrics getMetrics() { } default ExecutorService getExecutorService() { - return Executors.newFixedThreadPool(concurrentReconciliationThreads()); + return new ThreadPoolExecutor(minConcurrentReconciliationThreads(), + concurrentReconciliationThreads(), + 1, TimeUnit.MINUTES, new LinkedBlockingDeque<>()); } default ExecutorService getWorkflowExecutorService() { - return Executors.newFixedThreadPool(concurrentWorkflowExecutorThreads()); + return new ThreadPoolExecutor(minConcurrentWorkflowExecutorThreads(), + concurrentWorkflowExecutorThreads(), + 1, TimeUnit.MINUTES, new LinkedBlockingDeque<>()); } default boolean closeClientOnStop() { diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationServiceOverrider.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationServiceOverrider.java index 22a4b6e6bd..3cc8dd6078 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationServiceOverrider.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationServiceOverrider.java @@ -19,7 +19,9 @@ public class ConfigurationServiceOverrider { private Config clientConfig; private Boolean checkCR; private Integer concurrentReconciliationThreads; + private Integer minConcurrentReconciliationThreads; private Integer concurrentWorkflowExecutorThreads; + private Integer minConcurrentWorkflowExecutorThreads; private Cloner cloner; private Integer timeoutSeconds; private Boolean closeClientOnStop; @@ -56,6 +58,16 @@ public ConfigurationServiceOverrider withConcurrentWorkflowExecutorThreads(int t return this; } + public ConfigurationServiceOverrider withMinConcurrentReconciliationThreads(int threadNumber) { + this.minConcurrentReconciliationThreads = threadNumber; + return this; + } + + public ConfigurationServiceOverrider withMinConcurrentWorkflowExecutorThreads(int threadNumber) { + this.minConcurrentWorkflowExecutorThreads = threadNumber; + return this; + } + public ConfigurationServiceOverrider withResourceCloner(Cloner cloner) { this.cloner = cloner; return this; @@ -149,6 +161,18 @@ public int concurrentWorkflowExecutorThreads() { : original.concurrentWorkflowExecutorThreads(); } + @Override + public int minConcurrentReconciliationThreads() { + return minConcurrentReconciliationThreads != null ? minConcurrentReconciliationThreads + : original.minConcurrentReconciliationThreads(); + } + + @Override + public int minConcurrentWorkflowExecutorThreads() { + return minConcurrentWorkflowExecutorThreads != null ? minConcurrentWorkflowExecutorThreads + : original.minConcurrentWorkflowExecutorThreads(); + } + @Override public int getTerminationTimeoutSeconds() { return timeoutSeconds != null ? timeoutSeconds : original.getTerminationTimeoutSeconds(); From 85f0e0aaee3c23bfaa8553effb1c8232abbdb618 Mon Sep 17 00:00:00 2001 From: Chris Laprun Date: Fri, 10 Mar 2023 16:20:12 +0100 Subject: [PATCH 17/25] feat: make it possible to change the httpclient implementation (#1793) Also document how to use a different implementation instead of the okhttp one which is the default one and currently, the one we recommend to use. Vert.X implementation is also coming along nicely. JDK implementation should work if you're using Java 17 and up, and will work with 11 when some bug fixes land in 11. --- caffeine-bounded-cache-support/pom.xml | 5 +++++ docs/documentation/v4-3-migration.md | 28 ++++++++++++++++++------ operator-framework-core/pom.xml | 6 +++++ operator-framework/pom.xml | 5 ++++- pom.xml | 24 +++++++++++++++++++- sample-operators/tomcat-operator/pom.xml | 10 +++++++++ 6 files changed, 69 insertions(+), 9 deletions(-) diff --git a/caffeine-bounded-cache-support/pom.xml b/caffeine-bounded-cache-support/pom.xml index 23b5334287..7b5223c339 100644 --- a/caffeine-bounded-cache-support/pom.xml +++ b/caffeine-bounded-cache-support/pom.xml @@ -54,6 +54,11 @@ test-jar test + + io.fabric8 + kubernetes-httpclient-okhttp + test + diff --git a/docs/documentation/v4-3-migration.md b/docs/documentation/v4-3-migration.md index f5022fa856..17d3be70b4 100644 --- a/docs/documentation/v4-3-migration.md +++ b/docs/documentation/v4-3-migration.md @@ -9,16 +9,16 @@ permalink: /docs/v4-3-migration ## Condition API Change -In Workflows the target of the condition was the managed resource itself, not the target dependent resource. +In Workflows the target of the condition was the managed resource itself, not the target dependent resource. This changed, now the API contains the dependent resource. New API: ```java public interface Condition { - - boolean isMet(DependentResource dependentResource, P primary, Context

context); - + + boolean isMet(DependentResource dependentResource, P primary, Context

context); + } ``` @@ -27,10 +27,24 @@ Former API: ```java public interface Condition { - boolean isMet(P primary, R secondary, Context

context); - + boolean isMet(P primary, R secondary, Context

context); + } ``` -Migration is trivial. Since the secondary resource can be accessed from the dependent resource. So to access the secondary +Migration is trivial. Since the secondary resource can be accessed from the dependent resource. So to access the +secondary resource just use `dependentResource.getSecondaryResource(primary,context)`. + +## HTTP client choice + +It is now possible to change the HTTP client used by the Fabric8 client to communicate with the Kubernetes API server. +By default, the SDK uses the historical default HTTP client which relies on Okhttp and there shouldn't be anything +needed to keep using this implementation. The `tomcat-operator` sample has been migrated to use the Vert.X based +implementation. You can see how to change the client by looking at +that [sample POM file](https://github.com/java-operator-sdk/java-operator-sdk/blob/d259fcd084f7e22032dfd0df3c7e64fe68850c1b/sample-operators/tomcat-operator/pom.xml#L37-L50): + +- You need to exclude the default implementation (in this case okhttp) from the `operator-framework` dependency +- You need to add the appropriate implementation dependency, `kubernetes-httpclient-vertx` in this case, HTTP client + implementations provided as part of the Fabric8 client all following the `kubernetes-httpclient-` + pattern for their artifact identifier. \ No newline at end of file diff --git a/operator-framework-core/pom.xml b/operator-framework-core/pom.xml index 7c6e42da69..4d09220023 100644 --- a/operator-framework-core/pom.xml +++ b/operator-framework-core/pom.xml @@ -68,6 +68,12 @@ io.fabric8 kubernetes-client + + + io.fabric8 + kubernetes-httpclient-okhttp + + org.slf4j diff --git a/operator-framework/pom.xml b/operator-framework/pom.xml index a3404648df..ade6c88d96 100644 --- a/operator-framework/pom.xml +++ b/operator-framework/pom.xml @@ -17,7 +17,10 @@ io.javaoperatorsdk operator-framework-core - + + io.fabric8 + kubernetes-httpclient-okhttp + org.apache.commons commons-lang3 diff --git a/pom.xml b/pom.xml index 2cc8f8e888..a0e56f1a55 100644 --- a/pom.xml +++ b/pom.xml @@ -111,7 +111,7 @@ io.fabric8 - kubernetes-client + kubernetes-client-api ${fabric8-client.version} @@ -207,6 +207,28 @@ ${jenvtest.version} test + + + io.fabric8 + kubernetes-httpclient-okhttp + ${fabric8-client.version} + + + io.fabric8 + kubernetes-httpclient-vertx + ${fabric8-client.version} + + + + + + + + + + + + diff --git a/sample-operators/tomcat-operator/pom.xml b/sample-operators/tomcat-operator/pom.xml index 6a38e2d93b..9ce4dd8e1b 100644 --- a/sample-operators/tomcat-operator/pom.xml +++ b/sample-operators/tomcat-operator/pom.xml @@ -37,6 +37,16 @@ io.javaoperatorsdk operator-framework + + + io.fabric8 + kubernetes-httpclient-okhttp + + + + + io.fabric8 + kubernetes-httpclient-vertx io.fabric8 From 211a4193eeef99f586160517f311555f5b16f688 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Attila=20M=C3=A9sz=C3=A1ros?= Date: Tue, 14 Mar 2023 10:33:31 +0100 Subject: [PATCH 18/25] docs: bounded cache link fixes (#1816) Co-authored-by: Chris Laprun --- .../source/cache/CaffeineBoundedCache.java | 4 +- .../cache/CaffeineBoundedItemStores.java | 26 +++++++---- docs/documentation/features.md | 46 +++++++++++-------- 3 files changed, 45 insertions(+), 31 deletions(-) diff --git a/caffeine-bounded-cache-support/src/main/java/io/javaoperatorsdk/operator/processing/event/source/cache/CaffeineBoundedCache.java b/caffeine-bounded-cache-support/src/main/java/io/javaoperatorsdk/operator/processing/event/source/cache/CaffeineBoundedCache.java index eb70fe04d3..af4a17e2c2 100644 --- a/caffeine-bounded-cache-support/src/main/java/io/javaoperatorsdk/operator/processing/event/source/cache/CaffeineBoundedCache.java +++ b/caffeine-bounded-cache-support/src/main/java/io/javaoperatorsdk/operator/processing/event/source/cache/CaffeineBoundedCache.java @@ -3,11 +3,11 @@ import com.github.benmanes.caffeine.cache.Cache; /** - * Caffein cache wrapper to be used in a {@link BoundedItemStore} + * Caffeine cache wrapper to be used in a {@link BoundedItemStore} */ public class CaffeineBoundedCache implements BoundedCache { - private Cache cache; + private final Cache cache; public CaffeineBoundedCache(Cache cache) { this.cache = cache; diff --git a/caffeine-bounded-cache-support/src/main/java/io/javaoperatorsdk/operator/processing/event/source/cache/CaffeineBoundedItemStores.java b/caffeine-bounded-cache-support/src/main/java/io/javaoperatorsdk/operator/processing/event/source/cache/CaffeineBoundedItemStores.java index aa4db53030..a58d58bd2a 100644 --- a/caffeine-bounded-cache-support/src/main/java/io/javaoperatorsdk/operator/processing/event/source/cache/CaffeineBoundedItemStores.java +++ b/caffeine-bounded-cache-support/src/main/java/io/javaoperatorsdk/operator/processing/event/source/cache/CaffeineBoundedItemStores.java @@ -9,15 +9,22 @@ import com.github.benmanes.caffeine.cache.Caffeine; /** - * The idea about CaffeinBoundedItemStore-s is that, caffeine will cache the resources which were - * recently used, and will evict resource, which are not used for a while. This is ideal from the - * perspective that on startup controllers reconcile all resources (this is why a maxSize not ideal) - * but after a while it can happen (well depending on the controller and domain) that only some - * resources are actually active, thus related events happen. So in case large amount of custom - * resources only the active once will remain in the cache. Note that if a resource is reconciled - * all the secondary resources are usually reconciled too, in that case all those resources are - * fetched and populated to the cache, and will remain there for some time, for a subsequent - * reconciliations. + * A factory for Caffeine-backed + * {@link BoundedItemStore}. The implementation uses a {@link CaffeineBoundedCache} to store + * resources and progressively evict them if they haven't been used in a while. The idea about + * CaffeinBoundedItemStore-s is that, caffeine will cache the resources which were recently used, + * and will evict resource, which are not used for a while. This is ideal for startup performance + * and efficiency when all resources should be cached to avoid undue load on the API server. This is + * why setting a maximal cache size is not practical and the approach of evicting least recently + * used resources was chosen. However, depending on controller implementations and domains, it could + * happen that some / many of these resources are then seldom or even reconciled anymore. In that + * situation, large amounts of memory might be consumed to cache resources that are never used + * again. + *

+ * Note that if a resource is reconciled and is not present anymore in cache, it will transparently + * be fetched again from the API server. Similarly, since associated secondary resources are usually + * reconciled too, they might need to be fetched and populated to the cache, and will remain there + * for some time, for subsequent reconciliations. */ public class CaffeineBoundedItemStores { @@ -30,6 +37,7 @@ private CaffeineBoundedItemStores() {} * @return the ItemStore implementation * @param resource type */ + @SuppressWarnings("unused") public static BoundedItemStore boundedItemStore( KubernetesClient client, Class rClass, Duration accessExpireDuration) { diff --git a/docs/documentation/features.md b/docs/documentation/features.md index 4dc1a92b4c..d69dc4da09 100644 --- a/docs/documentation/features.md +++ b/docs/documentation/features.md @@ -739,9 +739,9 @@ to add the following dependencies to your project: ```xml - io.fabric8 - crd-generator-apt - provided + io.fabric8 + crd-generator-apt + provided ``` @@ -759,26 +759,32 @@ with a `mycrs` plural form will result in 2 files: ## Optimizing Caches -One of the ideas around operator pattern, is that all the relevant resources are cached, thus reconciliation is usually -very fast (especially if it does not need to update resources) since it's mostly working with in memory state. -However or large clusters, caching huge amount of primary and secondary resources might consume lots of memory. -There are some semi-experimental (experimental in terms that it works, but we need feedback from real production usage) -features to optimize memory usage of controllers. +One of the ideas around the operator pattern is that all the relevant resources are cached, thus reconciliation is +usually very fast (especially if no resources are updated in the process) since the operator is then mostly working with +in-memory state. However for large clusters, caching huge amount of primary and secondary resources might consume lots +of memory. JOSDK provides ways to mitigate this issue and optimize the memory usage of controllers. While these features +are working and tested, we need feedback from real production usage. -### Bounded Caches for Informers +### Bounded Caches for Informers -Limiting caches for informers - thus for Kubernetes resources, both controllers primary resource - is supported for now. -The idea with the implementation that is provided, is that resources are in the cache for a limited time. -So for use cases, when a resource is only frequently reconciled when it is created, and later no or -occasionally reconciled, will be evicted from the cache, since the resources are not accessed. -If a resource accessed in the future but not in the cache, the bounded cache implementation will fetch it from -the service if needed. -Note that on start of a controller all the resources are reconciled, for this reason explicitly setting a maximal -size of a cache might not be ideal. In other words it is desired to have all the resources in the cache at startup, -but not later if not accessed. +Limiting caches for informers - thus for Kubernetes resources - is supported by ensuring that resources are in the cache +for a limited time, via a cache eviction of least recently used resources. This means that when resources are created +and frequently reconciled, they stay "hot" in the cache. However, if, over time, a given resource "cools" down, i.e. it +becomes less and less used to the point that it might not be reconciled anymore, it will eventually get evicted from the +cache to free up memory. If such an evicted resource were to become reconciled again, the bounded cache implementation +would then fetch it from the API server and the "hot/cold" cycle would start anew. -See usage of the related implementation using Caffein cache in integration tests for [primary resource](https://github.com/java-operator-sdk/java-operator-sdk/blob/10e11e587447667ef0da1ddb29e0ba15fcd24ada/caffein-bounded-cache-support/src/test/java/io/javaoperatorsdk/operator/processing/event/source/cache/CaffeinBoundedCacheNamespacedIT.java#L19-L19) and for an [informer](https://github.com/java-operator-sdk/java-operator-sdk/blob/10e11e587447667ef0da1ddb29e0ba15fcd24ada/caffein-bounded-cache-support/src/test/java/io/javaoperatorsdk/operator/processing/event/source/cache/sample/AbstractTestReconciler.java#L84-L93). +Since all resources need to be reconciled when a controller start, it is not practical to set a maximal cache size as +it's desirable that all resources be cached as soon as possible to make the initial reconciliation process on start as +fast and efficient as possible, avoiding undue load on the API server. It's therefore more interesting to gradually +evict cold resources than try to limit cache sizes. -See also [CaffeinBoundedItemStores](https://github.com/java-operator-sdk/java-operator-sdk/blob/10e11e587447667ef0da1ddb29e0ba15fcd24ada/caffein-bounded-cache-support/src/main/java/io/javaoperatorsdk/operator/processing/event/source/cache/CaffeinBoundedItemStores.java#L22-L22) +See usage of the related implementation using [Caffeine](https://github.com/ben-manes/caffeine) cache in integration +tests +for [primary resources](https://github.com/java-operator-sdk/java-operator-sdk/blob/902c8a562dfd7f8993a52e03473a7ad4b00f378b/caffeine-bounded-cache-support/src/test/java/io/javaoperatorsdk/operator/processing/event/source/cache/sample/AbstractTestReconciler.java#L29-L29). + +See +also [CaffeineBoundedItemStores](https://github.com/java-operator-sdk/java-operator-sdk/blob/902c8a562dfd7f8993a52e03473a7ad4b00f378b/caffeine-bounded-cache-support/src/main/java/io/javaoperatorsdk/operator/processing/event/source/cache/CaffeineBoundedItemStores.java) +for more details. From 0a569fe95b86b711cb6ad51eae618ea487767dc1 Mon Sep 17 00:00:00 2001 From: Chris Laprun Date: Wed, 15 Mar 2023 12:58:21 +0100 Subject: [PATCH 19/25] feat: remove CR meters when they are deleted (after a delay) (#1805) * feat: remove CR meters when they are deleted (after a delay) Fixes #1803. Also added documentation for the Metrics feature. --- docs/documentation/features.md | 45 +++++++ micrometer-support/pom.xml | 31 +++++ .../micrometer/MicrometerMetrics.java | 125 ++++++++++++++++-- .../micrometer/MetricsCleaningOnDeleteIT.java | 97 ++++++++++++++ 4 files changed, 285 insertions(+), 13 deletions(-) create mode 100644 micrometer-support/src/test/java/io/javaoperatorsdk/operator/monitoring/micrometer/MetricsCleaningOnDeleteIT.java diff --git a/docs/documentation/features.md b/docs/documentation/features.md index d69dc4da09..2a27250559 100644 --- a/docs/documentation/features.md +++ b/docs/documentation/features.md @@ -757,6 +757,51 @@ with a `mycrs` plural form will result in 2 files: > Quarkus users using the `quarkus-operator-sdk` extension do not need to add any extra dependency > to get their CRD generated as this is handled by the extension itself. +## Metrics + +JOSDK provides built-in support for metrics reporting on what is happening with your reconcilers in the form of +the `Metrics` interface which can be implemented to connect to your metrics provider of choice, JOSDK calling the +methods as it goes about reconciling resources. By default, a no-operation implementation is provided thus providing a +no-cost sane default. A [micrometer](https://micrometer.io)-based implementation is also provided. + +You can use a different implementation by overriding the default one provided by the default `ConfigurationService`, as +follows: + +```java +Metrics metrics= …; +ConfigurationServiceProvider.overrideCurrent(overrider->overrider.withMetrics(metrics)); +``` + +### Micrometer implementation + +The micrometer implementation records a lot of metrics associated to each resource handled by the operator by default. +In order to be efficient, the implementation removes meters associated with resources when they are deleted. Since it +might be useful to keep these metrics around for a bit before they are deleted, it is possible to configure a delay +before their removal. As this is done asynchronously, it is also possible to configure how many threads you want to +devote to these operations. Both aspects are controlled by the `MicrometerMetrics` constructor so changing the defaults +is a matter of instantiating `MicrometerMetrics` with the desired values and tell `ConfigurationServiceProvider` about +it as shown above. + +The micrometer implementation records the following metrics: + +| Meter name | Type | Tags | Description | +|-----------------------------------------------------------|----------------|------------------------------------------------------------------------------------------------------------|--------------------------------------------------------------------------------------------------------| +| operator.sdk.reconciliations.executions. | gauge | group, version, kind | Number of executions of the named reconciler | +| operator.sdk.reconciliations.queue.size. | gauge | group, version, kind | How many resources are queued to get reconciled by named reconciler | +| operator.sdk..size | gauge map size | | Gauge tracking the size of a specified map (currently unused but could be used to monitor caches size) | +| operator.sdk.events.received | counter | group, version, kind, name, namespace, scope, event, action | Number of received Kubernetes events | +| operator.sdk.events.delete | counter | group, version, kind, name, namespace, scope | Number of received Kubernetes delete events | +| operator.sdk.reconciliations.started | counter | group, version, kind, name, namespace, scope, reconciliations.retries.last, reconciliations.retries.number | Number of started reconciliations per resource type | +| operator.sdk.reconciliations.failed | counter | group, version, kind, name, namespace, scope, exception | Number of failed reconciliations per resource type | +| operator.sdk.reconciliations.success | counter | group, version, kind, name, namespace, scope | Number of successful reconciliations per resource type | +| operator.sdk.controllers.execution.reconcile.success | counter | controller, type | Number of successful reconciliations per controller | +| operator.sdk.controllers.execution.reconcile.failure | counter | controller, exception | Number of failed reconciliations per controller | +| operator.sdk.controllers.execution.cleanup.success | counter | controller, type | Number of successful cleanups per controller | +| operator.sdk.controllers.execution.cleanup.failure | counter | controller, exception | Number of failed cleanups per controller | + +As you can see all the recorded metrics start with the `operator.sdk` prefix. + + ## Optimizing Caches One of the ideas around the operator pattern is that all the relevant resources are cached, thus reconciliation is diff --git a/micrometer-support/pom.xml b/micrometer-support/pom.xml index ae1be1816f..92c2a6fc18 100644 --- a/micrometer-support/pom.xml +++ b/micrometer-support/pom.xml @@ -26,6 +26,37 @@ io.javaoperatorsdk operator-framework-core + + org.junit.jupiter + junit-jupiter-api + test + + + org.junit.jupiter + junit-jupiter-engine + test + + + org.assertj + assertj-core + test + + + org.awaitility + awaitility + test + + + io.javaoperatorsdk + operator-framework-junit-5 + ${project.version} + test + + + io.fabric8 + kubernetes-httpclient-vertx + test + \ No newline at end of file diff --git a/micrometer-support/src/main/java/io/javaoperatorsdk/operator/monitoring/micrometer/MicrometerMetrics.java b/micrometer-support/src/main/java/io/javaoperatorsdk/operator/monitoring/micrometer/MicrometerMetrics.java index 651b012a02..5b3a83a1c6 100644 --- a/micrometer-support/src/main/java/io/javaoperatorsdk/operator/monitoring/micrometer/MicrometerMetrics.java +++ b/micrometer-support/src/main/java/io/javaoperatorsdk/operator/monitoring/micrometer/MicrometerMetrics.java @@ -1,11 +1,10 @@ package io.javaoperatorsdk.operator.monitoring.micrometer; -import java.util.ArrayList; -import java.util.Collections; -import java.util.List; -import java.util.Map; -import java.util.Optional; +import java.util.*; import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.Executors; +import java.util.concurrent.ScheduledExecutorService; +import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicInteger; import io.fabric8.kubernetes.api.model.HasMetadata; @@ -17,6 +16,8 @@ import io.javaoperatorsdk.operator.processing.GroupVersionKind; import io.javaoperatorsdk.operator.processing.event.Event; import io.javaoperatorsdk.operator.processing.event.ResourceID; +import io.javaoperatorsdk.operator.processing.event.source.controller.ResourceEvent; +import io.micrometer.core.instrument.Meter; import io.micrometer.core.instrument.MeterRegistry; import io.micrometer.core.instrument.Tag; import io.micrometer.core.instrument.Timer; @@ -31,9 +32,53 @@ public class MicrometerMetrics implements Metrics { private static final String RECONCILIATIONS_QUEUE_SIZE = PREFIX + RECONCILIATIONS + "queue.size."; private final MeterRegistry registry; private final Map gauges = new ConcurrentHashMap<>(); + private final Map> metersPerResource = new ConcurrentHashMap<>(); + private final Cleaner cleaner; + /** + * Creates a non-delayed, micrometer-based Metrics implementation. The non-delayed part refers to + * the cleaning of meters associated with deleted resources. + * + * @param registry the {@link MeterRegistry} instance to use for metrics recording + */ public MicrometerMetrics(MeterRegistry registry) { + this(registry, 0); + } + + /** + * Creates a micrometer-based Metrics implementation that delays cleaning up {@link Meter}s + * associated with deleted resources by the specified amount of seconds, using a single thread for + * that process. + * + * @param registry the {@link MeterRegistry} instance to use for metrics recording + * @param cleanUpDelayInSeconds the number of seconds to wait before meters are removed for + * deleted resources + */ + public MicrometerMetrics(MeterRegistry registry, int cleanUpDelayInSeconds) { + this(registry, cleanUpDelayInSeconds, 1); + } + + /** + * Creates a micrometer-based Metrics implementation that delays cleaning up {@link Meter}s + * associated with deleted resources by the specified amount of seconds, using the specified + * (maximally) number of threads for that process. + * + * @param registry the {@link MeterRegistry} instance to use for metrics recording + * @param cleanUpDelayInSeconds the number of seconds to wait before meters are removed for + * deleted resources + * @param cleaningThreadsNumber the number of threads to use for the cleaning process + */ + public MicrometerMetrics(MeterRegistry registry, int cleanUpDelayInSeconds, + int cleaningThreadsNumber) { this.registry = registry; + if (cleanUpDelayInSeconds < 0) { + cleaner = new NoDelayCleaner(); + } else { + cleaningThreadsNumber = + cleaningThreadsNumber <= 0 ? Runtime.getRuntime().availableProcessors() + : cleaningThreadsNumber; + cleaner = new DelayedCleaner(cleanUpDelayInSeconds, cleaningThreadsNumber); + } } @Override @@ -108,14 +153,24 @@ private static String getScope(ResourceID resourceID) { @Override public void receivedEvent(Event event, Map metadata) { + final String[] tags; + if (event instanceof ResourceEvent) { + tags = new String[] {"event", event.getClass().getSimpleName(), "action", + ((ResourceEvent) event).getAction().toString()}; + } else { + tags = new String[] {"event", event.getClass().getSimpleName()}; + } + incrementCounter(event.getRelatedCustomResourceID(), "events.received", metadata, - "event", event.getClass().getSimpleName()); + tags); } @Override public void cleanupDoneFor(ResourceID resourceID, Map metadata) { incrementCounter(resourceID, "events.delete", metadata); + + cleaner.removeMetersFor(resourceID); } @Override @@ -125,11 +180,11 @@ public void reconcileCustomResource(HasMetadata resource, RetryInfo retryInfoNul incrementCounter(ResourceID.fromResource(resource), RECONCILIATIONS + "started", metadata, RECONCILIATIONS + "retries.number", - "" + retryInfo.map(RetryInfo::getAttemptCount).orElse(0), + String.valueOf(retryInfo.map(RetryInfo::getAttemptCount).orElse(0)), RECONCILIATIONS + "retries.last", - "" + retryInfo.map(RetryInfo::isLastAttempt).orElse(true)); + String.valueOf(retryInfo.map(RetryInfo::isLastAttempt).orElse(true))); - AtomicInteger controllerQueueSize = + var controllerQueueSize = gauges.get(RECONCILIATIONS_QUEUE_SIZE + metadata.get(CONTROLLER_NAME)); controllerQueueSize.incrementAndGet(); } @@ -141,18 +196,18 @@ public void finishedReconciliation(HasMetadata resource, Map met @Override public void reconciliationExecutionStarted(HasMetadata resource, Map metadata) { - AtomicInteger reconcilerExecutions = + var reconcilerExecutions = gauges.get(RECONCILIATIONS_EXECUTIONS + metadata.get(CONTROLLER_NAME)); reconcilerExecutions.incrementAndGet(); } @Override public void reconciliationExecutionFinished(HasMetadata resource, Map metadata) { - AtomicInteger reconcilerExecutions = + var reconcilerExecutions = gauges.get(RECONCILIATIONS_EXECUTIONS + metadata.get(CONTROLLER_NAME)); reconcilerExecutions.decrementAndGet(); - AtomicInteger controllerQueueSize = + var controllerQueueSize = gauges.get(RECONCILIATIONS_QUEUE_SIZE + metadata.get(CONTROLLER_NAME)); controllerQueueSize.decrementAndGet(); } @@ -202,6 +257,50 @@ private void incrementCounter(ResourceID id, String counterName, Map new HashSet<>()).add(counter.getId()); + counter.increment(); + } + + protected Set recordedMeterIdsFor(ResourceID resourceID) { + return metersPerResource.get(resourceID); + } + + private interface Cleaner { + void removeMetersFor(ResourceID resourceID); + } + + private void removeMetersFor(ResourceID resourceID) { + // remove each meter + final var toClean = metersPerResource.get(resourceID); + if (toClean != null) { + toClean.forEach(registry::remove); + } + // then clean-up local recording of associations + metersPerResource.remove(resourceID); + } + + private class NoDelayCleaner implements Cleaner { + @Override + public void removeMetersFor(ResourceID resourceID) { + MicrometerMetrics.this.removeMetersFor(resourceID); + } + } + + private class DelayedCleaner implements Cleaner { + private final ScheduledExecutorService metersCleaner; + private final int cleanUpDelayInSeconds; + + private DelayedCleaner(int cleanUpDelayInSeconds, int cleaningThreadsNumber) { + this.cleanUpDelayInSeconds = cleanUpDelayInSeconds; + this.metersCleaner = Executors.newScheduledThreadPool(cleaningThreadsNumber); + } + + @Override + public void removeMetersFor(ResourceID resourceID) { + // schedule deletion of meters associated with ResourceID + metersCleaner.schedule(() -> MicrometerMetrics.this.removeMetersFor(resourceID), + cleanUpDelayInSeconds, TimeUnit.SECONDS); + } } } diff --git a/micrometer-support/src/test/java/io/javaoperatorsdk/operator/monitoring/micrometer/MetricsCleaningOnDeleteIT.java b/micrometer-support/src/test/java/io/javaoperatorsdk/operator/monitoring/micrometer/MetricsCleaningOnDeleteIT.java new file mode 100644 index 0000000000..717f3a11f9 --- /dev/null +++ b/micrometer-support/src/test/java/io/javaoperatorsdk/operator/monitoring/micrometer/MetricsCleaningOnDeleteIT.java @@ -0,0 +1,97 @@ +package io.javaoperatorsdk.operator.monitoring.micrometer; + +import java.time.Duration; +import java.util.HashSet; +import java.util.Set; + +import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; + +import io.fabric8.kubernetes.api.model.ConfigMap; +import io.fabric8.kubernetes.api.model.ConfigMapBuilder; +import io.javaoperatorsdk.operator.api.config.ConfigurationServiceProvider; +import io.javaoperatorsdk.operator.api.reconciler.*; +import io.javaoperatorsdk.operator.junit.LocallyRunOperatorExtension; +import io.javaoperatorsdk.operator.processing.event.ResourceID; +import io.micrometer.core.instrument.Meter; +import io.micrometer.core.instrument.simple.SimpleMeterRegistry; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.awaitility.Awaitility.await; + +public class MetricsCleaningOnDeleteIT { + @RegisterExtension + static LocallyRunOperatorExtension operator = + LocallyRunOperatorExtension.builder().withReconciler(new MetricsCleaningTestReconciler()) + .build(); + + private static final TestSimpleMeterRegistry registry = new TestSimpleMeterRegistry(); + private static final int testDelay = 1; + private static final MicrometerMetrics metrics = new MicrometerMetrics(registry, testDelay, 2); + private static final String testResourceName = "cleaning-metrics-cr"; + + @BeforeAll + static void setup() { + ConfigurationServiceProvider.overrideCurrent(overrider -> overrider.withMetrics(metrics)); + } + + @AfterAll + static void reset() { + ConfigurationServiceProvider.reset(); + } + + @Test + void removesMetersAssociatedWithResourceAfterItsDeletion() throws InterruptedException { + var testResource = new ConfigMapBuilder() + .withNewMetadata() + .withName(testResourceName) + .endMetadata() + .build(); + final var created = operator.create(testResource); + + // make sure the resource is created + await().until(() -> !operator.get(ConfigMap.class, testResourceName) + .getMetadata().getFinalizers().isEmpty()); + + // check that we properly recorded meters associated with the resource + final var meters = metrics.recordedMeterIdsFor(ResourceID.fromResource(created)); + assertThat(meters).isNotNull(); + assertThat(meters).isNotEmpty(); + + // delete the resource and wait for it to be deleted + operator.delete(testResource); + await().until(() -> operator.get(ConfigMap.class, testResourceName) == null); + + // check that the meters are properly removed after the specified delay + Thread.sleep(Duration.ofSeconds(testDelay).toMillis()); + assertThat(registry.removed).isEqualTo(meters); + assertThat(metrics.recordedMeterIdsFor(ResourceID.fromResource(created))).isNull(); + } + + @ControllerConfiguration + private static class MetricsCleaningTestReconciler + implements Reconciler, Cleaner { + @Override + public UpdateControl reconcile(ConfigMap resource, Context context) { + return UpdateControl.noUpdate(); + } + + @Override + public DeleteControl cleanup(ConfigMap resource, Context context) { + return DeleteControl.defaultDelete(); + } + } + + private static class TestSimpleMeterRegistry extends SimpleMeterRegistry { + private final Set removed = new HashSet<>(); + + @Override + public Meter remove(Meter.Id mappedId) { + final var removed = super.remove(mappedId); + this.removed.add(removed.getId()); + return removed; + } + } +} From 3ec133504b7da5c9c29823c51c6d2030e6e3ad32 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Attila=20M=C3=A9sz=C3=A1ros?= Date: Wed, 15 Mar 2023 17:32:03 +0100 Subject: [PATCH 20/25] fix: ControllerTest flake (#1822) --- .../io/javaoperatorsdk/operator/processing/ControllerTest.java | 1 + 1 file changed, 1 insertion(+) diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/ControllerTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/ControllerTest.java index 67e9112b27..a9031ffcf6 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/ControllerTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/ControllerTest.java @@ -30,6 +30,7 @@ void crdShouldNotBeCheckedForNativeResources() { @Test void crdShouldNotBeCheckedForCustomResourcesIfDisabled() { + ConfigurationServiceProvider.reset(); final var client = MockKubernetesClient.client(TestCustomResource.class); final var configuration = MockControllerConfiguration.forResource(TestCustomResource.class); From 3751a68472e12318c9db4d425951922a4cf380c2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Attila=20M=C3=A9sz=C3=A1ros?= Date: Mon, 27 Mar 2023 12:40:41 +0200 Subject: [PATCH 21/25] feat: improved per resource polling (#1826) --- .../PerResourcePollingEventSource.java | 109 +++++++++++++----- .../PerResourcePollingEventSourceTest.java | 105 ++++++++++++----- 2 files changed, 157 insertions(+), 57 deletions(-) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/polling/PerResourcePollingEventSource.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/polling/PerResourcePollingEventSource.java index 8bf5c50fd4..ad688599db 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/polling/PerResourcePollingEventSource.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/polling/PerResourcePollingEventSource.java @@ -1,7 +1,8 @@ package io.javaoperatorsdk.operator.processing.event.source.polling; +import java.time.Duration; import java.util.*; -import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.*; import java.util.function.Predicate; import org.slf4j.Logger; @@ -32,8 +33,10 @@ public class PerResourcePollingEventSource private static final Logger log = LoggerFactory.getLogger(PerResourcePollingEventSource.class); - private final Timer timer = new Timer(); - private final Map timerTasks = new ConcurrentHashMap<>(); + public static final int DEFAULT_EXECUTOR_THREAD_NUMBER = 1; + + private final ScheduledExecutorService executorService; + private final Map> scheduledFutures = new ConcurrentHashMap<>(); private final ResourceFetcher resourceFetcher; private final Cache

resourceCache; private final Predicate

registerPredicate; @@ -57,11 +60,20 @@ public PerResourcePollingEventSource(ResourceFetcher resourceFetcher, Cache

resourceCache, long period, Predicate

registerPredicate, Class resourceClass, CacheKeyMapper cacheKeyMapper) { + this(resourceFetcher, resourceCache, period, registerPredicate, resourceClass, cacheKeyMapper, + new ScheduledThreadPoolExecutor(DEFAULT_EXECUTOR_THREAD_NUMBER)); + } + + public PerResourcePollingEventSource(ResourceFetcher resourceFetcher, + Cache

resourceCache, long period, + Predicate

registerPredicate, Class resourceClass, + CacheKeyMapper cacheKeyMapper, ScheduledExecutorService executorService) { super(resourceClass, cacheKeyMapper); this.resourceFetcher = resourceFetcher; this.resourceCache = resourceCache; this.period = period; this.registerPredicate = registerPredicate; + this.executorService = executorService; } private Set getAndCacheResource(P primary, boolean fromGetter) { @@ -71,6 +83,17 @@ private Set getAndCacheResource(P primary, boolean fromGetter) { return values; } + @SuppressWarnings("unchecked") + private void scheduleNextExecution(P primary, Set actualResources) { + var primaryID = ResourceID.fromResource(primary); + var fetchDelay = resourceFetcher.fetchDelay(actualResources, primary); + var fetchDuration = fetchDelay.orElse(Duration.ofMillis(period)); + + ScheduledFuture scheduledFuture = (ScheduledFuture) executorService + .schedule(new FetchingExecutor(primaryID), fetchDuration.toMillis(), TimeUnit.MILLISECONDS); + scheduledFutures.put(primaryID, scheduledFuture); + } + @Override public void onResourceCreated(P resource) { checkAndRegisterTask(resource); @@ -84,10 +107,10 @@ public void onResourceUpdated(P newResource, P oldResource) { @Override public void onResourceDeleted(P resource) { var resourceID = ResourceID.fromResource(resource); - TimerTask task = timerTasks.remove(resourceID); - if (task != null) { - log.debug("Canceling task for resource: {}", resource); - task.cancel(); + var scheduledFuture = scheduledFutures.remove(resourceID); + if (scheduledFuture != null) { + log.debug("Canceling scheduledFuture for resource: {}", resource); + scheduledFuture.cancel(true); } handleDelete(resourceID); fetchedForPrimaries.remove(resourceID); @@ -95,30 +118,42 @@ public void onResourceDeleted(P resource) { // This method is always called from the same Thread for the same resource, // since events from ResourceEventAware are propagated from the thread of the informer. This is - // important - // because otherwise there will be a race condition related to the timerTasks. + // important because otherwise there will be a race condition related to the timerTasks. private void checkAndRegisterTask(P resource) { var primaryID = ResourceID.fromResource(resource); - if (timerTasks.get(primaryID) == null && (registerPredicate == null + if (scheduledFutures.get(primaryID) == null && (registerPredicate == null || registerPredicate.test(resource))) { - var task = - new TimerTask() { - @Override - public void run() { - if (!isRunning()) { - log.debug("Event source not yet started. Will not run for: {}", primaryID); - return; - } - // always use up-to-date resource from cache - var res = resourceCache.get(primaryID); - res.ifPresentOrElse(p -> getAndCacheResource(p, false), - () -> log.warn("No resource in cache for resource ID: {}", primaryID)); - } - }; - timerTasks.put(primaryID, task); - // there is a delay, to not do two fetches when the resources first appeared + var cachedResources = cache.get(primaryID); + var actualResources = + cachedResources == null ? null : new HashSet<>(cachedResources.values()); + // note that there is a delay, to not do two fetches when the resources first appeared // and getSecondaryResource is called on reconciliation. - timer.schedule(task, period, period); + scheduleNextExecution(resource, actualResources); + } + } + + private class FetchingExecutor implements Runnable { + private final ResourceID primaryID; + + public FetchingExecutor(ResourceID primaryID) { + this.primaryID = primaryID; + } + + @Override + public void run() { + if (!isRunning()) { + log.debug("Event source not yet started. Will not run for: {}", primaryID); + return; + } + // always use up-to-date resource from cache + var primary = resourceCache.get(primaryID); + if (primary.isEmpty()) { + log.warn("No resource in cache for resource ID: {}", primaryID); + // no new execution is scheduled in this case, a on delete event should be received shortly + } else { + var actualResources = primary.map(p -> getAndCacheResource(p, false)); + scheduleNextExecution(primary.get(), actualResources.orElse(null)); + } } } @@ -146,12 +181,28 @@ public Set getSecondaryResources(P primary) { public interface ResourceFetcher { Set fetchResources(P primaryResource); + + /** + * By implementing this method it is possible to specify dynamic durations to wait between the + * polls of the resources. This is especially handy if a resources "stabilized" so it is not + * expected to change its state frequently. For example an AWS RDS instance is up and running, + * it is expected to run and be stable for a very long time. In this case it is enough to poll + * with a lower frequency, compared to the phase when it is being initialized. + * + * @param lastFetchedResource might be null, in case no fetch happened before. Empty set if + * fetch happened but no resources were found. + * @param primary related primary resource + * @return an Optional containing the Duration to wait until the next fetch. If an empty + * Optional is returned, the default polling period will be used. + */ + default Optional fetchDelay(Set lastFetchedResource, P primary) { + return Optional.empty(); + } } @Override public void stop() throws OperatorException { super.stop(); - timer.cancel(); + executorService.shutdownNow(); } - } diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/polling/PerResourcePollingEventSourceTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/polling/PerResourcePollingEventSourceTest.java index fa4a624d59..727b60ed9b 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/polling/PerResourcePollingEventSourceTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/polling/PerResourcePollingEventSourceTest.java @@ -1,5 +1,6 @@ package io.javaoperatorsdk.operator.processing.event.source.polling; +import java.time.Duration; import java.util.Collections; import java.util.Optional; import java.util.Set; @@ -16,6 +17,7 @@ import io.javaoperatorsdk.operator.sample.simple.TestCustomResource; import static org.assertj.core.api.Assertions.assertThat; +import static org.awaitility.Awaitility.await; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.atLeast; @@ -47,45 +49,50 @@ public void setup() { } @Test - void pollsTheResourceAfterAwareOfIt() throws InterruptedException { + void pollsTheResourceAfterAwareOfIt() { source.onResourceCreated(testCustomResource); - Thread.sleep(3 * PERIOD); - verify(supplier, atLeast(2)).fetchResources(eq(testCustomResource)); - verify(eventHandler, times(1)).handleEvent(any()); + await().pollDelay(Duration.ofMillis(3 * PERIOD)).untilAsserted(() -> { + verify(supplier, atLeast(2)).fetchResources(eq(testCustomResource)); + verify(supplier, atLeast(2)).fetchDelay(any(), eq(testCustomResource)); + verify(eventHandler, times(1)).handleEvent(any()); + }); } @Test - void registeringTaskOnAPredicate() throws InterruptedException { + void registeringTaskOnAPredicate() { setUpSource(new PerResourcePollingEventSource<>(supplier, resourceCache, PERIOD, testCustomResource -> testCustomResource.getMetadata().getGeneration() > 1, SampleExternalResource.class, CacheKeyMapper.singleResourceCacheKeyMapper())); source.onResourceCreated(testCustomResource); - Thread.sleep(2 * PERIOD); - verify(supplier, times(0)).fetchResources(eq(testCustomResource)); + + await().pollDelay(Duration.ofMillis(2 * PERIOD)) + .untilAsserted(() -> verify(supplier, times(0)).fetchResources(eq(testCustomResource))); + testCustomResource.getMetadata().setGeneration(2L); source.onResourceUpdated(testCustomResource, testCustomResource); - Thread.sleep(2 * PERIOD); - verify(supplier, atLeast(1)).fetchResources(eq(testCustomResource)); + await().pollDelay(Duration.ofMillis(2 * PERIOD)) + .untilAsserted(() -> verify(supplier, atLeast(1)).fetchResources(eq(testCustomResource))); } @Test - void propagateEventOnDeletedResource() throws InterruptedException { + void propagateEventOnDeletedResource() { source.onResourceCreated(testCustomResource); when(supplier.fetchResources(any())) .thenReturn(Set.of(SampleExternalResource.testResource1())) .thenReturn(Collections.emptySet()); - Thread.sleep(3 * PERIOD); - verify(supplier, atLeast(2)).fetchResources(eq(testCustomResource)); - verify(eventHandler, times(2)).handleEvent(any()); + await().pollDelay(Duration.ofMillis(3 * PERIOD)).untilAsserted(() -> { + verify(supplier, atLeast(2)).fetchResources(eq(testCustomResource)); + verify(eventHandler, times(2)).handleEvent(any()); + }); } @Test - void getSecondaryResourceInitiatesFetchJustForFirstTime() throws InterruptedException { + void getSecondaryResourceInitiatesFetchJustForFirstTime() { source.onResourceCreated(testCustomResource); when(supplier.fetchResources(any())) .thenReturn(Set.of(SampleExternalResource.testResource1())) @@ -104,31 +111,73 @@ void getSecondaryResourceInitiatesFetchJustForFirstTime() throws InterruptedExce verify(supplier, times(1)).fetchResources(eq(testCustomResource)); verify(eventHandler, never()).handleEvent(any()); - Thread.sleep(PERIOD * 2); - - verify(supplier, atLeast(2)).fetchResources(eq(testCustomResource)); - value = source.getSecondaryResources(testCustomResource); - assertThat(value).hasSize(2); + await().pollDelay(Duration.ofMillis(PERIOD * 2)).untilAsserted(() -> { + verify(supplier, atLeast(2)).fetchResources(eq(testCustomResource)); + var val = source.getSecondaryResources(testCustomResource); + assertThat(val).hasSize(2); + }); } @Test - void getsValueFromCacheOrSupplier() throws InterruptedException { + void getsValueFromCacheOrSupplier() { source.onResourceCreated(testCustomResource); when(supplier.fetchResources(any())) .thenReturn(Collections.emptySet()) .thenReturn(Set.of(SampleExternalResource.testResource1())); - Thread.sleep(PERIOD / 3); + await().pollDelay(Duration.ofMillis(PERIOD / 3)).untilAsserted(() -> { + var value = source.getSecondaryResources(testCustomResource); + verify(eventHandler, times(0)).handleEvent(any()); + assertThat(value).isEmpty(); + }); + + await().pollDelay(Duration.ofMillis(PERIOD * 2)).untilAsserted(() -> { + var value2 = source.getSecondaryResources(testCustomResource); + assertThat(value2).hasSize(1); + verify(eventHandler, times(1)).handleEvent(any()); + }); + } - var value = source.getSecondaryResources(testCustomResource); - verify(eventHandler, times(0)).handleEvent(any()); - assertThat(value).isEmpty(); + @Test + void supportsDynamicPollingDelay() { + when(supplier.fetchResources(any())) + .thenReturn(Set.of(SampleExternalResource.testResource1())); + when(supplier.fetchDelay(any(),any())) + .thenReturn(Optional.of(Duration.ofMillis(PERIOD))) + .thenReturn(Optional.of(Duration.ofMillis(PERIOD*2))); - Thread.sleep(PERIOD * 2); + source.onResourceCreated(testCustomResource); - value = source.getSecondaryResources(testCustomResource); - assertThat(value).hasSize(1); - verify(eventHandler, times(1)).handleEvent(any()); + await().pollDelay(Duration.ofMillis(PERIOD)).atMost(Duration.ofMillis((long) (1.5 * PERIOD))) + .pollInterval(Duration.ofMillis(20)) + .untilAsserted(() -> verify(supplier,times(1)).fetchResources(any())); + // verifying that it is not called as with normal interval + await().pollDelay(Duration.ofMillis(PERIOD)).atMost(Duration.ofMillis((long) (1.5*PERIOD))) + .pollInterval(Duration.ofMillis(20)) + .untilAsserted(() -> verify(supplier,times(1)).fetchResources(any())); + await().pollDelay(Duration.ofMillis(PERIOD)).atMost(Duration.ofMillis(2 * PERIOD)) + .pollInterval(Duration.ofMillis(20)) + .untilAsserted(() -> verify(supplier,times(2)).fetchResources(any())); + } + + @Test + void deleteEventCancelsTheScheduling() { + when(supplier.fetchResources(any())) + .thenReturn(Set.of(SampleExternalResource.testResource1())); + + source.onResourceCreated(testCustomResource); + + await().pollDelay(Duration.ofMillis(PERIOD)) + .atMost(Duration.ofMillis((2* PERIOD))) + .pollInterval(Duration.ofMillis(20)) + .untilAsserted(() -> verify(supplier,times(1)).fetchResources(any())); + + source.onResourceDeleted(testCustomResource); + + // check if not called again. + await().pollDelay(Duration.ofMillis(PERIOD)) + .atMost(Duration.ofMillis((2* PERIOD))) + .untilAsserted(() -> verify(supplier,times(1)).fetchResources(any())); } } From 2b0280f6ae8f5762ae5ae00bbf04ad9d1936edcf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Attila=20M=C3=A9sz=C3=A1ros?= Date: Mon, 27 Mar 2023 15:27:55 +0200 Subject: [PATCH 22/25] fix: flaky special integration test (#1834) * fix: flaky special integration test * fix format --- .../operator/InformerRelatedBehaviorITS.java | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/InformerRelatedBehaviorITS.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/InformerRelatedBehaviorITS.java index 8fa0186bba..26f544bd90 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/InformerRelatedBehaviorITS.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/InformerRelatedBehaviorITS.java @@ -4,6 +4,8 @@ import org.junit.jupiter.api.*; +import io.fabric8.kubernetes.api.model.ConfigMap; +import io.fabric8.kubernetes.api.model.ConfigMapBuilder; import io.fabric8.kubernetes.api.model.Namespace; import io.fabric8.kubernetes.api.model.ObjectMetaBuilder; import io.fabric8.kubernetes.api.model.rbac.ClusterRole; @@ -71,6 +73,7 @@ void beforeEach(TestInfo testInfo) { @AfterEach void cleanup() { adminClient.resource(testCustomResource()).delete(); + adminClient.resource(dependentConfigMap()).delete(); } @Test @@ -149,7 +152,7 @@ private void assertInformerNotWatchingForAdditionalNamespace(Operator operator) } @Test - void resilientForLoosingPermissionForCustomResource() throws InterruptedException { + void resilientForLoosingPermissionForCustomResource() { setFullResourcesAccess(); startOperator(true); setNoCustomResourceAccess(); @@ -229,6 +232,15 @@ InformerRelatedBehaviorTestCustomResource testCustomResource() { return testCustomResource; } + private ConfigMap dependentConfigMap() { + return new ConfigMapBuilder() + .withMetadata(new ObjectMetaBuilder() + .withName(TEST_RESOURCE_NAME) + .withNamespace(actualNamespace) + .build()) + .build(); + } + private void assertReconciled() { await().untilAsserted(() -> { assertThat(reconciler.getNumberOfExecutions()).isGreaterThan(0); From 0b208a25757984ef4019f856428ccb494900d940 Mon Sep 17 00:00:00 2001 From: Chris Laprun Date: Mon, 27 Mar 2023 15:47:12 +0200 Subject: [PATCH 23/25] feat: now possible to only output non-resource related metrics (#1823) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat: bounded cache for informers (#1718) * fix: typo caffein -> caffeine (#1795) * feat: now possible to only output non-resource related metrics Fixes #1812. * refactor: extract abstract test fixture to add tests with variations * fix: add missing annotation * tests: add more test variations * fix: make operator non-static so it's registered once per test subclass * feat: introduce builder for MicrometerMetrics, fix test * fix: exclude more tags when not collecting per resource * fix: registry should be per-instance to ensure test independence * fix: make sure we wait a little to ensure event is properly processed * fix: make things work on Java 11, format * fix: also clean metrics on finalizer removal This is needed because the finalizer will trigger a reconciliation that adds a resource-specific metric. * fix: format * refactor: extract common tags Co-authored-by: Sébastien CROCQUESEL <88554524+scrocquesel@users.noreply.github.com> * feat: make per-resource collecting finer-grained We now still collect GVK information when per-resource collection is switched off. * fix: do not create tag for group if not present * fix: remove unreliable no-delay implementation, defaulting to 1s delay * refactor: renamed & documented factory methods to make things clearer * docs: updated metrics section for code changes * feat: avoid emitting tag on empty value * docs: update * fix: format [skip ci] * refactor: use Tag more directly, avoid unneeded work, use constants * fix: change will happen instead of might * docs: add missing timer Co-authored-by: Sébastien CROCQUESEL <88554524+scrocquesel@users.noreply.github.com> * docs: fix wrong & missing information * refactor: add constants * fix: wording [skip ci] Co-authored-by: Attila Mészáros --------- Co-authored-by: Attila Mészáros Co-authored-by: Sébastien CROCQUESEL <88554524+scrocquesel@users.noreply.github.com> --- docs/documentation/features.md | 77 ++-- .../micrometer/MicrometerMetrics.java | 371 ++++++++++++------ ...AbstractMicrometerMetricsTestFixture.java} | 52 ++- .../micrometer/DefaultBehaviorIT.java | 31 ++ .../DelayedMetricsCleaningOnDeleteIT.java | 29 ++ .../micrometer/NoPerResourceCollectionIT.java | 23 ++ .../operator/api/monitoring/Metrics.java | 5 +- .../operator/processing/Controller.java | 37 +- .../processing/event/EventProcessor.java | 1 + .../operator/sample/MySQLSchemaOperator.java | 6 +- 10 files changed, 452 insertions(+), 180 deletions(-) rename micrometer-support/src/test/java/io/javaoperatorsdk/operator/monitoring/micrometer/{MetricsCleaningOnDeleteIT.java => AbstractMicrometerMetricsTestFixture.java} (67%) create mode 100644 micrometer-support/src/test/java/io/javaoperatorsdk/operator/monitoring/micrometer/DefaultBehaviorIT.java create mode 100644 micrometer-support/src/test/java/io/javaoperatorsdk/operator/monitoring/micrometer/DelayedMetricsCleaningOnDeleteIT.java create mode 100644 micrometer-support/src/test/java/io/javaoperatorsdk/operator/monitoring/micrometer/NoPerResourceCollectionIT.java diff --git a/docs/documentation/features.md b/docs/documentation/features.md index 2a27250559..9a62271e6c 100644 --- a/docs/documentation/features.md +++ b/docs/documentation/features.md @@ -774,33 +774,62 @@ ConfigurationServiceProvider.overrideCurrent(overrider->overrider.withMetrics(me ### Micrometer implementation -The micrometer implementation records a lot of metrics associated to each resource handled by the operator by default. -In order to be efficient, the implementation removes meters associated with resources when they are deleted. Since it -might be useful to keep these metrics around for a bit before they are deleted, it is possible to configure a delay -before their removal. As this is done asynchronously, it is also possible to configure how many threads you want to -devote to these operations. Both aspects are controlled by the `MicrometerMetrics` constructor so changing the defaults -is a matter of instantiating `MicrometerMetrics` with the desired values and tell `ConfigurationServiceProvider` about -it as shown above. +The micrometer implementation is typically created using one of the provided factory methods which, depending on which +is used, will return either a ready to use instance or a builder allowing users to customized how the implementation +behaves, in particular when it comes to the granularity of collected metrics. It is, for example, possible to collect +metrics on a per-resource basis via tags that are associated with meters. This is the default, historical behavior but +this will change in a future version of JOSDK because this dramatically increases the cardinality of metrics, which +could lead to performance issues. -The micrometer implementation records the following metrics: +To create a `MicrometerMetrics` implementation that behaves how it has historically behaved, you can just create an +instance via: + +```java +MeterRegistry registry= …; +Metrics metrics=new MicrometerMetrics(registry) +``` + +Note, however, that this constructor is deprecated and we encourage you to use the factory methods instead, which either +return a fully pre-configured instance or a builder object that will allow you to configure more easily how the instance +will behave. You can, for example, configure whether or not the implementation should collect metrics on a per-resource +basis, whether or not associated meters should be removed when a resource is deleted and how the clean-up is performed. +See the relevant classes documentation for more details. -| Meter name | Type | Tags | Description | -|-----------------------------------------------------------|----------------|------------------------------------------------------------------------------------------------------------|--------------------------------------------------------------------------------------------------------| -| operator.sdk.reconciliations.executions. | gauge | group, version, kind | Number of executions of the named reconciler | -| operator.sdk.reconciliations.queue.size. | gauge | group, version, kind | How many resources are queued to get reconciled by named reconciler | -| operator.sdk..size | gauge map size | | Gauge tracking the size of a specified map (currently unused but could be used to monitor caches size) | -| operator.sdk.events.received | counter | group, version, kind, name, namespace, scope, event, action | Number of received Kubernetes events | -| operator.sdk.events.delete | counter | group, version, kind, name, namespace, scope | Number of received Kubernetes delete events | -| operator.sdk.reconciliations.started | counter | group, version, kind, name, namespace, scope, reconciliations.retries.last, reconciliations.retries.number | Number of started reconciliations per resource type | -| operator.sdk.reconciliations.failed | counter | group, version, kind, name, namespace, scope, exception | Number of failed reconciliations per resource type | -| operator.sdk.reconciliations.success | counter | group, version, kind, name, namespace, scope | Number of successful reconciliations per resource type | -| operator.sdk.controllers.execution.reconcile.success | counter | controller, type | Number of successful reconciliations per controller | -| operator.sdk.controllers.execution.reconcile.failure | counter | controller, exception | Number of failed reconciliations per controller | -| operator.sdk.controllers.execution.cleanup.success | counter | controller, type | Number of successful cleanups per controller | -| operator.sdk.controllers.execution.cleanup.failure | counter | controller, exception | Number of failed cleanups per controller | - -As you can see all the recorded metrics start with the `operator.sdk` prefix. +For example, the following will create a `MicrometerMetrics` instance configured to collect metrics on a per-resource +basis, deleting the associated meters after 5 seconds when a resource is deleted, using up to 2 threads to do so. + +```java +MicrometerMetrics.newPerResourceCollectingMicrometerMetricsBuilder(registry) + .withCleanUpDelayInSeconds(5) + .withCleaningThreadNumber(2) + .build() +``` + +The micrometer implementation records the following metrics: +| Meter name | Type | Tag names | Description | +|-----------------------------------------------------------|----------------|-----------------------------------------------------------------------------------|--------------------------------------------------------------------------------------------------------| +| operator.sdk.reconciliations.executions. | gauge | group, version, kind | Number of executions of the named reconciler | +| operator.sdk.reconciliations.queue.size. | gauge | group, version, kind | How many resources are queued to get reconciled by named reconciler | +| operator.sdk..size | gauge map size | | Gauge tracking the size of a specified map (currently unused but could be used to monitor caches size) | +| operator.sdk.events.received | counter | , event, action | Number of received Kubernetes events | +| operator.sdk.events.delete | counter | | Number of received Kubernetes delete events | +| operator.sdk.reconciliations.started | counter | , reconciliations.retries.last, reconciliations.retries.number | Number of started reconciliations per resource type | +| operator.sdk.reconciliations.failed | counter | , exception | Number of failed reconciliations per resource type | +| operator.sdk.reconciliations.success | counter | | Number of successful reconciliations per resource type | +| operator.sdk.controllers.execution.reconcile | timer | , controller | Time taken for reconciliations per controller | +| operator.sdk.controllers.execution.cleanup | timer | , controller | Time taken for cleanups per controller | +| operator.sdk.controllers.execution.reconcile.success | counter | controller, type | Number of successful reconciliations per controller | +| operator.sdk.controllers.execution.reconcile.failure | counter | controller, exception | Number of failed reconciliations per controller | +| operator.sdk.controllers.execution.cleanup.success | counter | controller, type | Number of successful cleanups per controller | +| operator.sdk.controllers.execution.cleanup.failure | counter | controller, exception | Number of failed cleanups per controller | + +As you can see all the recorded metrics start with the `operator.sdk` prefix. ``, in the table above, +refers to resource-specific metadata and depends on the considered metric and how the implementation is configured and +could be summed up as follows: `group?, version, kind, [name, namespace?], scope` where the tags in square +brackets (`[]`) won't be present when per-resource collection is disabled and tags followed by a question mark are +omitted if the associated value is empty. Of note, when in the context of controllers' execution metrics, these tag +names are prefixed with `resource.`. This prefix might be removed in a future version for greater consistency. ## Optimizing Caches diff --git a/micrometer-support/src/main/java/io/javaoperatorsdk/operator/monitoring/micrometer/MicrometerMetrics.java b/micrometer-support/src/main/java/io/javaoperatorsdk/operator/monitoring/micrometer/MicrometerMetrics.java index 5b3a83a1c6..61ff1562ee 100644 --- a/micrometer-support/src/main/java/io/javaoperatorsdk/operator/monitoring/micrometer/MicrometerMetrics.java +++ b/micrometer-support/src/main/java/io/javaoperatorsdk/operator/monitoring/micrometer/MicrometerMetrics.java @@ -28,100 +28,132 @@ public class MicrometerMetrics implements Metrics { private static final String PREFIX = "operator.sdk."; private static final String RECONCILIATIONS = "reconciliations."; + private static final String RECONCILIATIONS_FAILED = RECONCILIATIONS + "failed"; + private static final String RECONCILIATIONS_SUCCESS = RECONCILIATIONS + "success"; + private static final String RECONCILIATIONS_RETRIES_LAST = RECONCILIATIONS + "retries.last"; + private static final String RECONCILIATIONS_RETRIES_NUMBER = RECONCILIATIONS + "retries.number"; + private static final String RECONCILIATIONS_STARTED = RECONCILIATIONS + "started"; private static final String RECONCILIATIONS_EXECUTIONS = PREFIX + RECONCILIATIONS + "executions."; private static final String RECONCILIATIONS_QUEUE_SIZE = PREFIX + RECONCILIATIONS + "queue.size."; + private static final String NAME = "name"; + private static final String NAMESPACE = "namespace"; + private static final String GROUP = "group"; + private static final String VERSION = "version"; + private static final String KIND = "kind"; + private static final String SCOPE = "scope"; + private static final String METADATA_PREFIX = "resource."; + private static final String CONTROLLERS_EXECUTION = "controllers.execution."; + private static final String CONTROLLER = "controller"; + private static final String SUCCESS_SUFFIX = ".success"; + private static final String FAILURE_SUFFIX = ".failure"; + private static final String TYPE = "type"; + private static final String EXCEPTION = "exception"; + private static final String EVENT = "event"; + private static final String ACTION = "action"; + private static final String EVENTS_RECEIVED = "events.received"; + private static final String EVENTS_DELETE = "events.delete"; + private static final String CLUSTER = "cluster"; + private static final String SIZE_SUFFIX = ".size"; + private final boolean collectPerResourceMetrics; private final MeterRegistry registry; private final Map gauges = new ConcurrentHashMap<>(); - private final Map> metersPerResource = new ConcurrentHashMap<>(); private final Cleaner cleaner; /** - * Creates a non-delayed, micrometer-based Metrics implementation. The non-delayed part refers to - * the cleaning of meters associated with deleted resources. + * Creates a default micrometer-based Metrics implementation, collecting metrics on a per resource + * basis and not dealing with cleaning these after these resources are deleted. Note that this + * probably will change in a future release. If you want more control over what the implementation + * actually does, please use the static factory methods instead. * * @param registry the {@link MeterRegistry} instance to use for metrics recording + * @deprecated Use the factory methods / builders instead */ + @Deprecated public MicrometerMetrics(MeterRegistry registry) { - this(registry, 0); + this(registry, Cleaner.NOOP, true); } /** - * Creates a micrometer-based Metrics implementation that delays cleaning up {@link Meter}s - * associated with deleted resources by the specified amount of seconds, using a single thread for - * that process. + * Creates a MicrometerMetrics instance configured to not collect per-resource metrics, just + * aggregates per resource **type** * * @param registry the {@link MeterRegistry} instance to use for metrics recording - * @param cleanUpDelayInSeconds the number of seconds to wait before meters are removed for - * deleted resources + * @return a MicrometerMetrics instance configured to not collect per-resource metrics */ - public MicrometerMetrics(MeterRegistry registry, int cleanUpDelayInSeconds) { - this(registry, cleanUpDelayInSeconds, 1); + public static MicrometerMetrics withoutPerResourceMetrics(MeterRegistry registry) { + return new MicrometerMetrics(registry, Cleaner.NOOP, false); } /** - * Creates a micrometer-based Metrics implementation that delays cleaning up {@link Meter}s - * associated with deleted resources by the specified amount of seconds, using the specified - * (maximally) number of threads for that process. + * Creates a new builder to configure how the eventual MicrometerMetrics instance will behave. * * @param registry the {@link MeterRegistry} instance to use for metrics recording - * @param cleanUpDelayInSeconds the number of seconds to wait before meters are removed for - * deleted resources - * @param cleaningThreadsNumber the number of threads to use for the cleaning process + * @return a MicrometerMetrics instance configured to not collect per-resource metrics + * @see MicrometerMetricsBuilder */ - public MicrometerMetrics(MeterRegistry registry, int cleanUpDelayInSeconds, - int cleaningThreadsNumber) { + public static MicrometerMetricsBuilder newMicrometerMetricsBuilder(MeterRegistry registry) { + return new MicrometerMetricsBuilder(registry); + } + + /** + * Creates a new builder to configure how the eventual MicrometerMetrics instance will behave, + * pre-configuring it to collect metrics per resource. + * + * @param registry the {@link MeterRegistry} instance to use for metrics recording + * @return a MicrometerMetrics instance configured to not collect per-resource metrics + * @see PerResourceCollectingMicrometerMetricsBuilder + */ + public static PerResourceCollectingMicrometerMetricsBuilder newPerResourceCollectingMicrometerMetricsBuilder( + MeterRegistry registry) { + return new PerResourceCollectingMicrometerMetricsBuilder(registry); + } + + /** + * Creates a micrometer-based Metrics implementation that cleans up {@link Meter}s associated with + * deleted resources as specified by the (possibly {@code null}) provided {@link Cleaner} + * instance. + * + * @param registry the {@link MeterRegistry} instance to use for metrics recording + * @param cleaner the {@link Cleaner} to use + * @param collectingPerResourceMetrics whether to collect per resource metrics + */ + private MicrometerMetrics(MeterRegistry registry, Cleaner cleaner, + boolean collectingPerResourceMetrics) { this.registry = registry; - if (cleanUpDelayInSeconds < 0) { - cleaner = new NoDelayCleaner(); - } else { - cleaningThreadsNumber = - cleaningThreadsNumber <= 0 ? Runtime.getRuntime().availableProcessors() - : cleaningThreadsNumber; - cleaner = new DelayedCleaner(cleanUpDelayInSeconds, cleaningThreadsNumber); - } + this.cleaner = cleaner; + this.collectPerResourceMetrics = collectingPerResourceMetrics; } @Override - public void controllerRegistered(Controller controller) { - String executingThreadsName = - RECONCILIATIONS_EXECUTIONS + controller.getConfiguration().getName(); + public void controllerRegistered(Controller controller) { + final var configuration = controller.getConfiguration(); + final var name = configuration.getName(); + final var executingThreadsName = RECONCILIATIONS_EXECUTIONS + name; + final var resourceClass = configuration.getResourceClass(); + final var tags = new ArrayList(3); + addGVKTags(GroupVersionKind.gvkFor(resourceClass), tags, false); AtomicInteger executingThreads = - registry.gauge(executingThreadsName, - gvkTags(controller.getConfiguration().getResourceClass()), - new AtomicInteger(0)); + registry.gauge(executingThreadsName, tags, new AtomicInteger(0)); gauges.put(executingThreadsName, executingThreads); - String controllerQueueName = - RECONCILIATIONS_QUEUE_SIZE + controller.getConfiguration().getName(); + final var controllerQueueName = RECONCILIATIONS_QUEUE_SIZE + name; AtomicInteger controllerQueueSize = - registry.gauge(controllerQueueName, - gvkTags(controller.getConfiguration().getResourceClass()), - new AtomicInteger(0)); + registry.gauge(controllerQueueName, tags, new AtomicInteger(0)); gauges.put(controllerQueueName, controllerQueueSize); } @Override public T timeControllerExecution(ControllerExecution execution) { final var name = execution.controllerName(); - final var execName = PREFIX + "controllers.execution." + execution.name(); + final var execName = PREFIX + CONTROLLERS_EXECUTION + execution.name(); final var resourceID = execution.resourceID(); final var metadata = execution.metadata(); - final var tags = new ArrayList(metadata.size() + 4); - tags.addAll(List.of( - "controller", name, - "resource.name", resourceID.getName(), - "resource.namespace", resourceID.getNamespace().orElse(""), - "resource.scope", getScope(resourceID))); - final var gvk = (GroupVersionKind) metadata.get(Constants.RESOURCE_GVK_KEY); - if (gvk != null) { - tags.addAll(List.of( - "resource.group", gvk.group, - "resource.version", gvk.version, - "resource.kind", gvk.kind)); - } + final var tags = new ArrayList(16); + tags.add(Tag.of(CONTROLLER, name)); + addMetadataTags(resourceID, metadata, tags, true); final var timer = Timer.builder(execName) - .tags(tags.toArray(new String[0])) + .tags(tags) .publishPercentiles(0.3, 0.5, 0.95) .publishPercentileHistogram() .register(registry); @@ -135,40 +167,35 @@ public T timeControllerExecution(ControllerExecution execution) { }); final var successType = execution.successTypeName(result); registry - .counter(execName + ".success", "controller", name, "type", successType) + .counter(execName + SUCCESS_SUFFIX, CONTROLLER, name, TYPE, successType) .increment(); return result; } catch (Exception e) { final var exception = e.getClass().getSimpleName(); registry - .counter(execName + ".failure", "controller", name, "exception", exception) + .counter(execName + FAILURE_SUFFIX, CONTROLLER, name, EXCEPTION, exception) .increment(); throw e; } } - private static String getScope(ResourceID resourceID) { - return resourceID.getNamespace().isPresent() ? "namespace" : "cluster"; - } - @Override public void receivedEvent(Event event, Map metadata) { - final String[] tags; if (event instanceof ResourceEvent) { - tags = new String[] {"event", event.getClass().getSimpleName(), "action", - ((ResourceEvent) event).getAction().toString()}; + incrementCounter(event.getRelatedCustomResourceID(), EVENTS_RECEIVED, + metadata, + Tag.of(EVENT, event.getClass().getSimpleName()), + Tag.of(ACTION, ((ResourceEvent) event).getAction().toString())); } else { - tags = new String[] {"event", event.getClass().getSimpleName()}; + incrementCounter(event.getRelatedCustomResourceID(), EVENTS_RECEIVED, + metadata, + Tag.of(EVENT, event.getClass().getSimpleName())); } - - incrementCounter(event.getRelatedCustomResourceID(), "events.received", - metadata, - tags); } @Override public void cleanupDoneFor(ResourceID resourceID, Map metadata) { - incrementCounter(resourceID, "events.delete", metadata); + incrementCounter(resourceID, EVENTS_DELETE, metadata); cleaner.removeMetersFor(resourceID); } @@ -177,12 +204,12 @@ public void cleanupDoneFor(ResourceID resourceID, Map metadata) public void reconcileCustomResource(HasMetadata resource, RetryInfo retryInfoNullable, Map metadata) { Optional retryInfo = Optional.ofNullable(retryInfoNullable); - incrementCounter(ResourceID.fromResource(resource), RECONCILIATIONS + "started", + incrementCounter(ResourceID.fromResource(resource), RECONCILIATIONS_STARTED, metadata, - RECONCILIATIONS + "retries.number", - String.valueOf(retryInfo.map(RetryInfo::getAttemptCount).orElse(0)), - RECONCILIATIONS + "retries.last", - String.valueOf(retryInfo.map(RetryInfo::isLastAttempt).orElse(true))); + Tag.of(RECONCILIATIONS_RETRIES_NUMBER, + String.valueOf(retryInfo.map(RetryInfo::getAttemptCount).orElse(0))), + Tag.of(RECONCILIATIONS_RETRIES_LAST, + String.valueOf(retryInfo.map(RetryInfo::isLastAttempt).orElse(true)))); var controllerQueueSize = gauges.get(RECONCILIATIONS_QUEUE_SIZE + metadata.get(CONTROLLER_NAME)); @@ -191,7 +218,7 @@ public void reconcileCustomResource(HasMetadata resource, RetryInfo retryInfoNul @Override public void finishedReconciliation(HasMetadata resource, Map metadata) { - incrementCounter(ResourceID.fromResource(resource), RECONCILIATIONS + "success", metadata); + incrementCounter(ResourceID.fromResource(resource), RECONCILIATIONS_SUCCESS, metadata); } @Override @@ -221,77 +248,197 @@ public void failedReconciliation(HasMetadata resource, Exception exception, } else if (cause instanceof RuntimeException) { cause = cause.getCause() != null ? cause.getCause() : cause; } - incrementCounter(ResourceID.fromResource(resource), RECONCILIATIONS + "failed", metadata, - "exception", - cause.getClass().getSimpleName()); + incrementCounter(ResourceID.fromResource(resource), RECONCILIATIONS_FAILED, metadata, + Tag.of(EXCEPTION, cause.getClass().getSimpleName())); } @Override public > T monitorSizeOf(T map, String name) { - return registry.gaugeMapSize(PREFIX + name + ".size", Collections.emptyList(), map); + return registry.gaugeMapSize(PREFIX + name + SIZE_SUFFIX, Collections.emptyList(), map); + } + + + private void addMetadataTags(ResourceID resourceID, Map metadata, + List tags, boolean prefixed) { + if (collectPerResourceMetrics) { + addTag(NAME, resourceID.getName(), tags, prefixed); + addTagOmittingOnEmptyValue(NAMESPACE, resourceID.getNamespace().orElse(null), tags, prefixed); + } + addTag(SCOPE, getScope(resourceID), tags, prefixed); + final var gvk = (GroupVersionKind) metadata.get(Constants.RESOURCE_GVK_KEY); + if (gvk != null) { + addGVKTags(gvk, tags, prefixed); + } + } + + private static void addTag(String name, String value, List tags, boolean prefixed) { + tags.add(Tag.of(getPrefixedMetadataTag(name, prefixed), value)); + } + + private static void addTagOmittingOnEmptyValue(String name, String value, List tags, + boolean prefixed) { + if (value != null && !value.isBlank()) { + addTag(name, value, tags, prefixed); + } + } + + private static String getPrefixedMetadataTag(String tagName, boolean prefixed) { + return prefixed ? METADATA_PREFIX + tagName : tagName; } - public static List gvkTags(Class resourceClass) { - final var gvk = GroupVersionKind.gvkFor(resourceClass); - return List.of(Tag.of("group", gvk.group), Tag.of("version", gvk.version), - Tag.of("kind", gvk.kind)); + private static String getScope(ResourceID resourceID) { + return resourceID.getNamespace().isPresent() ? NAMESPACE : CLUSTER; + } + + private static void addGVKTags(GroupVersionKind gvk, List tags, boolean prefixed) { + addTagOmittingOnEmptyValue(GROUP, gvk.group, tags, prefixed); + addTag(VERSION, gvk.version, tags, prefixed); + addTag(KIND, gvk.kind, tags, prefixed); } private void incrementCounter(ResourceID id, String counterName, Map metadata, - String... additionalTags) { + Tag... additionalTags) { final var additionalTagsNb = additionalTags != null && additionalTags.length > 0 ? additionalTags.length : 0; final var metadataNb = metadata != null ? metadata.size() : 0; - final var tags = new ArrayList(6 + additionalTagsNb + metadataNb); - tags.addAll(List.of( - "name", id.getName(), - "namespace", id.getNamespace().orElse(""), - "scope", getScope(id))); + final var tags = new ArrayList(6 + additionalTagsNb + metadataNb); + addMetadataTags(id, metadata, tags, false); if (additionalTagsNb > 0) { tags.addAll(List.of(additionalTags)); } - if (metadataNb > 0) { - final var gvk = (GroupVersionKind) metadata.get(Constants.RESOURCE_GVK_KEY); - tags.addAll(List.of( - "group", gvk.group, - "version", gvk.version, - "kind", gvk.kind)); - } - final var counter = registry.counter(PREFIX + counterName, tags.toArray(new String[0])); - metersPerResource.computeIfAbsent(id, resourceID -> new HashSet<>()).add(counter.getId()); + + final var counter = registry.counter(PREFIX + counterName, tags); + cleaner.recordAssociation(id, counter); counter.increment(); } protected Set recordedMeterIdsFor(ResourceID resourceID) { - return metersPerResource.get(resourceID); + return cleaner.recordedMeterIdsFor(resourceID); + } + + public static class PerResourceCollectingMicrometerMetricsBuilder + extends MicrometerMetricsBuilder { + + private int cleaningThreadsNumber; + private int cleanUpDelayInSeconds; + + private PerResourceCollectingMicrometerMetricsBuilder(MeterRegistry registry) { + super(registry); + } + + /** + * @param cleaningThreadsNumber the maximal number of threads that can be assigned to the + * removal of {@link Meter}s associated with deleted resources, defaults to 1 if not + * specified or if the provided number is lesser or equal to 0 + */ + public PerResourceCollectingMicrometerMetricsBuilder withCleaningThreadNumber( + int cleaningThreadsNumber) { + this.cleaningThreadsNumber = cleaningThreadsNumber <= 0 ? 1 : cleaningThreadsNumber; + return this; + } + + /** + * @param cleanUpDelayInSeconds the number of seconds to wait before {@link Meter}s are removed + * for deleted resources, defaults to 1 (meaning meters will be removed one second after + * the associated resource is deleted) if not specified or if the provided number is + * lesser than 0. Threading and the general interaction model of interacting with the API + * server means that it's not possible to ensure that meters are immediately deleted in + * all cases so a minimal delay of one second is always enforced + */ + public PerResourceCollectingMicrometerMetricsBuilder withCleanUpDelayInSeconds( + int cleanUpDelayInSeconds) { + this.cleanUpDelayInSeconds = Math.max(cleanUpDelayInSeconds, 1); + return this; + } + + @Override + public MicrometerMetrics build() { + final var cleaner = + new DelayedCleaner(registry, cleanUpDelayInSeconds, cleaningThreadsNumber); + return new MicrometerMetrics(registry, cleaner, true); + } } + public static class MicrometerMetricsBuilder { + protected final MeterRegistry registry; + private boolean collectingPerResourceMetrics = true; + + private MicrometerMetricsBuilder(MeterRegistry registry) { + this.registry = registry; + } + + /** + * Configures the instance to collect metrics on a per-resource basis. + */ + @SuppressWarnings("unused") + public PerResourceCollectingMicrometerMetricsBuilder collectingMetricsPerResource() { + collectingPerResourceMetrics = true; + return new PerResourceCollectingMicrometerMetricsBuilder(registry); + } + + /** + * Configures the instance to only collect metrics per resource **type**, in an aggregate + * fashion, instead of per resource instance. + */ + @SuppressWarnings("unused") + public MicrometerMetricsBuilder notCollectingMetricsPerResource() { + collectingPerResourceMetrics = false; + return this; + } - private interface Cleaner { - void removeMetersFor(ResourceID resourceID); + public MicrometerMetrics build() { + return new MicrometerMetrics(registry, Cleaner.NOOP, collectingPerResourceMetrics); + } } - private void removeMetersFor(ResourceID resourceID) { - // remove each meter - final var toClean = metersPerResource.get(resourceID); - if (toClean != null) { - toClean.forEach(registry::remove); + interface Cleaner { + Cleaner NOOP = new Cleaner() {}; + + default void removeMetersFor(ResourceID resourceID) {} + + default void recordAssociation(ResourceID resourceID, Meter meter) {} + + default Set recordedMeterIdsFor(ResourceID resourceID) { + return Collections.emptySet(); } - // then clean-up local recording of associations - metersPerResource.remove(resourceID); } - private class NoDelayCleaner implements Cleaner { + static class DefaultCleaner implements Cleaner { + private final Map> metersPerResource = new ConcurrentHashMap<>(); + private final MeterRegistry registry; + + private DefaultCleaner(MeterRegistry registry) { + this.registry = registry; + } + @Override public void removeMetersFor(ResourceID resourceID) { - MicrometerMetrics.this.removeMetersFor(resourceID); + // remove each meter + final var toClean = metersPerResource.get(resourceID); + if (toClean != null) { + toClean.forEach(registry::remove); + } + // then clean-up local recording of associations + metersPerResource.remove(resourceID); + } + + @Override + public void recordAssociation(ResourceID resourceID, Meter meter) { + metersPerResource.computeIfAbsent(resourceID, id -> new HashSet<>()).add(meter.getId()); + } + + @Override + public Set recordedMeterIdsFor(ResourceID resourceID) { + return metersPerResource.get(resourceID); } } - private class DelayedCleaner implements Cleaner { + static class DelayedCleaner extends MicrometerMetrics.DefaultCleaner { private final ScheduledExecutorService metersCleaner; private final int cleanUpDelayInSeconds; - private DelayedCleaner(int cleanUpDelayInSeconds, int cleaningThreadsNumber) { + private DelayedCleaner(MeterRegistry registry, int cleanUpDelayInSeconds, + int cleaningThreadsNumber) { + super(registry); this.cleanUpDelayInSeconds = cleanUpDelayInSeconds; this.metersCleaner = Executors.newScheduledThreadPool(cleaningThreadsNumber); } @@ -299,7 +446,7 @@ private DelayedCleaner(int cleanUpDelayInSeconds, int cleaningThreadsNumber) { @Override public void removeMetersFor(ResourceID resourceID) { // schedule deletion of meters associated with ResourceID - metersCleaner.schedule(() -> MicrometerMetrics.this.removeMetersFor(resourceID), + metersCleaner.schedule(() -> super.removeMetersFor(resourceID), cleanUpDelayInSeconds, TimeUnit.SECONDS); } } diff --git a/micrometer-support/src/test/java/io/javaoperatorsdk/operator/monitoring/micrometer/MetricsCleaningOnDeleteIT.java b/micrometer-support/src/test/java/io/javaoperatorsdk/operator/monitoring/micrometer/AbstractMicrometerMetricsTestFixture.java similarity index 67% rename from micrometer-support/src/test/java/io/javaoperatorsdk/operator/monitoring/micrometer/MetricsCleaningOnDeleteIT.java rename to micrometer-support/src/test/java/io/javaoperatorsdk/operator/monitoring/micrometer/AbstractMicrometerMetricsTestFixture.java index 717f3a11f9..aa67c75f76 100644 --- a/micrometer-support/src/test/java/io/javaoperatorsdk/operator/monitoring/micrometer/MetricsCleaningOnDeleteIT.java +++ b/micrometer-support/src/test/java/io/javaoperatorsdk/operator/monitoring/micrometer/AbstractMicrometerMetricsTestFixture.java @@ -1,12 +1,12 @@ package io.javaoperatorsdk.operator.monitoring.micrometer; -import java.time.Duration; import java.util.HashSet; import java.util.Set; import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.TestInstance; import org.junit.jupiter.api.extension.RegisterExtension; import io.fabric8.kubernetes.api.model.ConfigMap; @@ -21,29 +21,31 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.awaitility.Awaitility.await; -public class MetricsCleaningOnDeleteIT { +@TestInstance(TestInstance.Lifecycle.PER_CLASS) +public abstract class AbstractMicrometerMetricsTestFixture { @RegisterExtension - static LocallyRunOperatorExtension operator = + LocallyRunOperatorExtension operator = LocallyRunOperatorExtension.builder().withReconciler(new MetricsCleaningTestReconciler()) .build(); - private static final TestSimpleMeterRegistry registry = new TestSimpleMeterRegistry(); - private static final int testDelay = 1; - private static final MicrometerMetrics metrics = new MicrometerMetrics(registry, testDelay, 2); - private static final String testResourceName = "cleaning-metrics-cr"; + protected final TestSimpleMeterRegistry registry = new TestSimpleMeterRegistry(); + protected final MicrometerMetrics metrics = getMetrics(); + protected static final String testResourceName = "micrometer-metrics-cr"; + + protected abstract MicrometerMetrics getMetrics(); @BeforeAll - static void setup() { + void setup() { ConfigurationServiceProvider.overrideCurrent(overrider -> overrider.withMetrics(metrics)); } @AfterAll - static void reset() { + void reset() { ConfigurationServiceProvider.reset(); } @Test - void removesMetersAssociatedWithResourceAfterItsDeletion() throws InterruptedException { + void properlyHandlesResourceDeletion() throws Exception { var testResource = new ConfigMapBuilder() .withNewMetadata() .withName(testResourceName) @@ -55,21 +57,29 @@ void removesMetersAssociatedWithResourceAfterItsDeletion() throws InterruptedExc await().until(() -> !operator.get(ConfigMap.class, testResourceName) .getMetadata().getFinalizers().isEmpty()); - // check that we properly recorded meters associated with the resource - final var meters = metrics.recordedMeterIdsFor(ResourceID.fromResource(created)); - assertThat(meters).isNotNull(); - assertThat(meters).isNotEmpty(); + final var resourceID = ResourceID.fromResource(created); + final var meters = preDeleteChecks(resourceID); // delete the resource and wait for it to be deleted operator.delete(testResource); await().until(() -> operator.get(ConfigMap.class, testResourceName) == null); - // check that the meters are properly removed after the specified delay - Thread.sleep(Duration.ofSeconds(testDelay).toMillis()); - assertThat(registry.removed).isEqualTo(meters); - assertThat(metrics.recordedMeterIdsFor(ResourceID.fromResource(created))).isNull(); + postDeleteChecks(resourceID, meters); } + protected Set preDeleteChecks(ResourceID resourceID) { + // check that we properly recorded meters associated with the resource + final var meters = metrics.recordedMeterIdsFor(resourceID); + // metrics are collected per resource + assertThat(registry.getMetersAsString()).contains(resourceID.getName()); + assertThat(meters).isNotNull(); + assertThat(meters).isNotEmpty(); + return meters; + } + + protected void postDeleteChecks(ResourceID resourceID, Set recordedMeters) + throws Exception {} + @ControllerConfiguration private static class MetricsCleaningTestReconciler implements Reconciler, Cleaner { @@ -84,7 +94,7 @@ public DeleteControl cleanup(ConfigMap resource, Context context) { } } - private static class TestSimpleMeterRegistry extends SimpleMeterRegistry { + static class TestSimpleMeterRegistry extends SimpleMeterRegistry { private final Set removed = new HashSet<>(); @Override @@ -93,5 +103,9 @@ public Meter remove(Meter.Id mappedId) { this.removed.add(removed.getId()); return removed; } + + public Set getRemoved() { + return removed; + } } } diff --git a/micrometer-support/src/test/java/io/javaoperatorsdk/operator/monitoring/micrometer/DefaultBehaviorIT.java b/micrometer-support/src/test/java/io/javaoperatorsdk/operator/monitoring/micrometer/DefaultBehaviorIT.java new file mode 100644 index 0000000000..6f0388c49b --- /dev/null +++ b/micrometer-support/src/test/java/io/javaoperatorsdk/operator/monitoring/micrometer/DefaultBehaviorIT.java @@ -0,0 +1,31 @@ +package io.javaoperatorsdk.operator.monitoring.micrometer; + +import java.util.Collections; +import java.util.Set; + +import io.javaoperatorsdk.operator.processing.event.ResourceID; +import io.micrometer.core.instrument.Meter; + +import static org.assertj.core.api.Assertions.assertThat; + +public class DefaultBehaviorIT extends AbstractMicrometerMetricsTestFixture { + @Override + protected MicrometerMetrics getMetrics() { + return MicrometerMetrics.newMicrometerMetricsBuilder(registry).build(); + } + + @Override + protected Set preDeleteChecks(ResourceID resourceID) { + // no meter should be recorded because we're not tracking anything to be deleted later + assertThat(metrics.recordedMeterIdsFor(resourceID)).isEmpty(); + // metrics are collected per resource by default for now, this will change in a future release + assertThat(registry.getMetersAsString()).contains(resourceID.getName()); + return Collections.emptySet(); + } + + @Override + protected void postDeleteChecks(ResourceID resourceID, Set recordedMeters) { + // meters should be neither recorded, nor removed by default + assertThat(registry.getRemoved()).isEmpty(); + } +} diff --git a/micrometer-support/src/test/java/io/javaoperatorsdk/operator/monitoring/micrometer/DelayedMetricsCleaningOnDeleteIT.java b/micrometer-support/src/test/java/io/javaoperatorsdk/operator/monitoring/micrometer/DelayedMetricsCleaningOnDeleteIT.java new file mode 100644 index 0000000000..26dfe59f84 --- /dev/null +++ b/micrometer-support/src/test/java/io/javaoperatorsdk/operator/monitoring/micrometer/DelayedMetricsCleaningOnDeleteIT.java @@ -0,0 +1,29 @@ +package io.javaoperatorsdk.operator.monitoring.micrometer; + +import java.time.Duration; +import java.util.Set; + +import io.javaoperatorsdk.operator.processing.event.ResourceID; +import io.micrometer.core.instrument.Meter; + +import static org.assertj.core.api.Assertions.assertThat; + +public class DelayedMetricsCleaningOnDeleteIT extends AbstractMicrometerMetricsTestFixture { + + private static final int testDelay = 1; + + @Override + protected MicrometerMetrics getMetrics() { + return MicrometerMetrics.newPerResourceCollectingMicrometerMetricsBuilder(registry) + .withCleanUpDelayInSeconds(testDelay).withCleaningThreadNumber(2).build(); + } + + @Override + protected void postDeleteChecks(ResourceID resourceID, Set recordedMeters) + throws Exception { + // check that the meters are properly removed after the specified delay + Thread.sleep(Duration.ofSeconds(testDelay).toMillis()); + assertThat(registry.getRemoved()).isEqualTo(recordedMeters); + assertThat(metrics.recordedMeterIdsFor(resourceID)).isNull(); + } +} diff --git a/micrometer-support/src/test/java/io/javaoperatorsdk/operator/monitoring/micrometer/NoPerResourceCollectionIT.java b/micrometer-support/src/test/java/io/javaoperatorsdk/operator/monitoring/micrometer/NoPerResourceCollectionIT.java new file mode 100644 index 0000000000..ac35347697 --- /dev/null +++ b/micrometer-support/src/test/java/io/javaoperatorsdk/operator/monitoring/micrometer/NoPerResourceCollectionIT.java @@ -0,0 +1,23 @@ +package io.javaoperatorsdk.operator.monitoring.micrometer; + +import java.util.Collections; +import java.util.Set; + +import io.javaoperatorsdk.operator.processing.event.ResourceID; +import io.micrometer.core.instrument.Meter; + +import static org.assertj.core.api.Assertions.assertThat; + +public class NoPerResourceCollectionIT extends AbstractMicrometerMetricsTestFixture { + @Override + protected MicrometerMetrics getMetrics() { + return MicrometerMetrics.withoutPerResourceMetrics(registry); + } + + @Override + protected Set preDeleteChecks(ResourceID resourceID) { + assertThat(metrics.recordedMeterIdsFor(resourceID)).isEmpty(); + assertThat(registry.getMetersAsString()).doesNotContain(resourceID.getName()); + return Collections.emptySet(); + } +} diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/monitoring/Metrics.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/monitoring/Metrics.java index 3f73e6c9a4..1ce27d29df 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/monitoring/Metrics.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/monitoring/Metrics.java @@ -24,7 +24,7 @@ public interface Metrics { /** * Do initialization if necessary; */ - default void controllerRegistered(Controller controller) {} + default void controllerRegistered(Controller controller) {} /** * Called when an event has been accepted by the SDK from an event source, which would result in @@ -39,6 +39,7 @@ default void receivedEvent(Event event, Map metadata) {} * @deprecated Use {@link #reconcileCustomResource(HasMetadata, RetryInfo, Map)} instead */ @Deprecated(forRemoval = true) + @SuppressWarnings("unused") default void reconcileCustomResource(ResourceID resourceID, RetryInfo retryInfo, Map metadata) {} @@ -58,6 +59,7 @@ default void reconcileCustomResource(HasMetadata resource, RetryInfo retryInfo, * @deprecated Use {@link #failedReconciliation(HasMetadata, Exception, Map)} instead */ @Deprecated(forRemoval = true) + @SuppressWarnings("unused") default void failedReconciliation(ResourceID resourceID, Exception exception, Map metadata) {} @@ -112,6 +114,7 @@ default void finishedReconciliation(ResourceID resourceID) { * @deprecated Use {@link #finishedReconciliation(HasMetadata, Map)} instead */ @Deprecated(forRemoval = true) + @SuppressWarnings("unused") default void finishedReconciliation(ResourceID resourceID, Map metadata) {} /** diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/Controller.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/Controller.java index f547fee778..e59d2a789b 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/Controller.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/Controller.java @@ -1,11 +1,6 @@ package io.javaoperatorsdk.operator.processing; -import java.util.ArrayList; -import java.util.HashMap; -import java.util.List; -import java.util.Map; -import java.util.Optional; -import java.util.Set; +import java.util.*; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -25,16 +20,7 @@ import io.javaoperatorsdk.operator.api.config.ControllerConfiguration; import io.javaoperatorsdk.operator.api.monitoring.Metrics; import io.javaoperatorsdk.operator.api.monitoring.Metrics.ControllerExecution; -import io.javaoperatorsdk.operator.api.reconciler.Cleaner; -import io.javaoperatorsdk.operator.api.reconciler.Constants; -import io.javaoperatorsdk.operator.api.reconciler.Context; -import io.javaoperatorsdk.operator.api.reconciler.ContextInitializer; -import io.javaoperatorsdk.operator.api.reconciler.DeleteControl; -import io.javaoperatorsdk.operator.api.reconciler.EventSourceContext; -import io.javaoperatorsdk.operator.api.reconciler.EventSourceInitializer; -import io.javaoperatorsdk.operator.api.reconciler.Ignore; -import io.javaoperatorsdk.operator.api.reconciler.Reconciler; -import io.javaoperatorsdk.operator.api.reconciler.UpdateControl; +import io.javaoperatorsdk.operator.api.reconciler.*; import io.javaoperatorsdk.operator.api.reconciler.dependent.EventSourceNotFoundException; import io.javaoperatorsdk.operator.api.reconciler.dependent.EventSourceProvider; import io.javaoperatorsdk.operator.api.reconciler.dependent.EventSourceReferencer; @@ -56,6 +42,13 @@ public class Controller

RegisteredController

{ private static final Logger log = LoggerFactory.getLogger(Controller.class); + private static final String CLEANUP = "cleanup"; + private static final String DELETE = "delete"; + private static final String FINALIZER_NOT_REMOVED = "finalizerNotRemoved"; + private static final String RECONCILE = "reconcile"; + private static final String RESOURCE = "resource"; + private static final String STATUS = "status"; + private static final String BOTH = "both"; private final Reconciler

reconciler; private final ControllerConfiguration

configuration; @@ -103,7 +96,7 @@ public UpdateControl

reconcile(P resource, Context

context) throws Excepti new ControllerExecution<>() { @Override public String name() { - return "reconcile"; + return RECONCILE; } @Override @@ -113,12 +106,12 @@ public String controllerName() { @Override public String successTypeName(UpdateControl

result) { - String successType = "resource"; + String successType = RESOURCE; if (result.isUpdateStatus()) { - successType = "status"; + successType = STATUS; } if (result.isUpdateResourceAndStatus()) { - successType = "both"; + successType = BOTH; } return successType; } @@ -154,7 +147,7 @@ public DeleteControl cleanup(P resource, Context

context) { new ControllerExecution<>() { @Override public String name() { - return "cleanup"; + return CLEANUP; } @Override @@ -164,7 +157,7 @@ public String controllerName() { @Override public String successTypeName(DeleteControl deleteControl) { - return deleteControl.isRemoveFinalizer() ? "delete" : "finalizerNotRemoved"; + return deleteControl.isRemoveFinalizer() ? DELETE : FINALIZER_NOT_REMOVED; } @Override 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 f973a7d354..7bf499c1c3 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 @@ -238,6 +238,7 @@ synchronized void eventProcessingFinished( cleanupForDeletedEvent(executionScope.getResourceID()); } else if (postExecutionControl.isFinalizerRemoved()) { state.markProcessedMarkForDeletion(); + metrics.cleanupDoneFor(resourceID, metricsMetadata); } else { postExecutionControl .getUpdatedCustomResource() diff --git a/sample-operators/mysql-schema/src/main/java/io/javaoperatorsdk/operator/sample/MySQLSchemaOperator.java b/sample-operators/mysql-schema/src/main/java/io/javaoperatorsdk/operator/sample/MySQLSchemaOperator.java index 4128dd0ea8..f312716f44 100644 --- a/sample-operators/mysql-schema/src/main/java/io/javaoperatorsdk/operator/sample/MySQLSchemaOperator.java +++ b/sample-operators/mysql-schema/src/main/java/io/javaoperatorsdk/operator/sample/MySQLSchemaOperator.java @@ -9,7 +9,8 @@ import org.takes.http.Exit; import org.takes.http.FtBasic; -import io.fabric8.kubernetes.client.*; +import io.fabric8.kubernetes.client.KubernetesClient; +import io.fabric8.kubernetes.client.KubernetesClientBuilder; import io.javaoperatorsdk.operator.Operator; import io.javaoperatorsdk.operator.monitoring.micrometer.MicrometerMetrics; import io.javaoperatorsdk.operator.sample.dependent.ResourcePollerConfig; @@ -25,7 +26,8 @@ public static void main(String[] args) throws IOException { KubernetesClient client = new KubernetesClientBuilder().build(); Operator operator = new Operator(client, - overrider -> overrider.withMetrics(new MicrometerMetrics(new LoggingMeterRegistry()))); + overrider -> overrider + .withMetrics(MicrometerMetrics.withoutPerResourceMetrics(new LoggingMeterRegistry()))); MySQLSchemaReconciler schemaReconciler = new MySQLSchemaReconciler(); From 7239c5a36975c7119d8af1a0501620466ac83bd6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Attila=20M=C3=A9sz=C3=A1ros?= Date: Tue, 28 Mar 2023 13:49:19 +0200 Subject: [PATCH 24/25] fix: special IT failing test case (#1836) --- .../operator/InformerRelatedBehaviorITS.java | 27 ++++++++++++------- ...InformerRelatedBehaviorTestReconciler.java | 8 ++++++ 2 files changed, 26 insertions(+), 9 deletions(-) diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/InformerRelatedBehaviorITS.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/InformerRelatedBehaviorITS.java index 26f544bd90..3f19c27e53 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/InformerRelatedBehaviorITS.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/InformerRelatedBehaviorITS.java @@ -3,6 +3,8 @@ import java.time.Duration; import org.junit.jupiter.api.*; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import io.fabric8.kubernetes.api.model.ConfigMap; import io.fabric8.kubernetes.api.model.ConfigMapBuilder; @@ -34,7 +36,7 @@ /** * The test relies on a special api server configuration: "min-request-timeout" to have a very low * value (in case want to try with minikube use: "minikube start - * --extra-config=apiserver.min-request-timeout=3") + * --extra-config=apiserver.min-request-timeout=1") * *

* This is important when tests are affected by permission changes, since the watch permissions are @@ -45,9 +47,11 @@ * The test ends with "ITS" (Special) since it needs to run separately from other ITs *

*/ -@EnableKubeAPIServer(apiServerFlags = {"--min-request-timeout", "3"}) +@EnableKubeAPIServer(apiServerFlags = {"--min-request-timeout", "1"}) class InformerRelatedBehaviorITS { + private static final Logger log = LoggerFactory.getLogger(InformerRelatedBehaviorITS.class); + public static final String TEST_RESOURCE_NAME = "test1"; public static final String ADDITIONAL_NAMESPACE_SUFFIX = "-additional"; @@ -55,6 +59,7 @@ class InformerRelatedBehaviorITS { InformerRelatedBehaviorTestReconciler reconciler; String actualNamespace; String additionalNamespace; + Operator operator; volatile boolean replacementStopHandlerCalled = false; @BeforeEach @@ -72,8 +77,11 @@ void beforeEach(TestInfo testInfo) { @AfterEach void cleanup() { - adminClient.resource(testCustomResource()).delete(); + if (operator != null) { + operator.stop(Duration.ofSeconds(1)); + } adminClient.resource(dependentConfigMap()).delete(); + adminClient.resource(testCustomResource()).delete(); } @Test @@ -90,7 +98,7 @@ void startsUpWhenNoPermissionToCustomResource() { adminClient.resource(testCustomResource()).createOrReplace(); setNoCustomResourceAccess(); - var operator = startOperator(false); + operator = startOperator(false); assertNotReconciled(); assertRuntimeInfoNoCRPermission(operator); @@ -106,7 +114,7 @@ void startsUpWhenNoPermissionToSecondaryResource() { adminClient.resource(testCustomResource()).createOrReplace(); setNoConfigMapAccess(); - var operator = startOperator(false); + operator = startOperator(false); assertNotReconciled(); assertRuntimeInfoForSecondaryPermission(operator); @@ -120,7 +128,7 @@ void startsUpIfNoPermissionToOneOfTwoNamespaces() { adminClient.resource(namespace(additionalNamespace)).createOrReplace(); addRoleBindingsToTestNamespaces(); - var operator = startOperator(false, false, actualNamespace, additionalNamespace); + operator = startOperator(false, false, actualNamespace, additionalNamespace); assertInformerNotWatchingForAdditionalNamespace(operator); adminClient.resource(testCustomResource()).createOrReplace(); @@ -151,17 +159,18 @@ private void assertInformerNotWatchingForAdditionalNamespace(Operator operator) assertThat(configMapHealthIndicator.isWatching()).isFalse(); } + @Test void resilientForLoosingPermissionForCustomResource() { setFullResourcesAccess(); - startOperator(true); + operator = startOperator(true); setNoCustomResourceAccess(); waitForWatchReconnect(); + adminClient.resource(testCustomResource()).createOrReplace(); assertNotReconciled(); - setFullResourcesAccess(); assertReconciled(); } @@ -210,7 +219,7 @@ void notExitingWithDefaultStopHandlerIfErrorHappensOnStartup() { private static void waitForWatchReconnect() { try { - Thread.sleep(6000); + Thread.sleep(5000); } catch (InterruptedException e) { throw new IllegalStateException(e); } diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/informerrelatedbehavior/InformerRelatedBehaviorTestReconciler.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/informerrelatedbehavior/InformerRelatedBehaviorTestReconciler.java index 13057d547a..f71f243c79 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/informerrelatedbehavior/InformerRelatedBehaviorTestReconciler.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/informerrelatedbehavior/InformerRelatedBehaviorTestReconciler.java @@ -2,12 +2,16 @@ import java.util.concurrent.atomic.AtomicInteger; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + import io.fabric8.kubernetes.client.KubernetesClient; 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; import io.javaoperatorsdk.operator.api.reconciler.dependent.Dependent; +import io.javaoperatorsdk.operator.processing.event.ResourceID; import io.javaoperatorsdk.operator.support.TestExecutionInfoProvider; @ControllerConfiguration( @@ -18,6 +22,9 @@ public class InformerRelatedBehaviorTestReconciler implements Reconciler, TestExecutionInfoProvider { + private static final Logger log = + LoggerFactory.getLogger(InformerRelatedBehaviorTestReconciler.class); + public static final String INFORMER_RELATED_BEHAVIOR_TEST_RECONCILER = "InformerRelatedBehaviorTestReconciler"; public static final String CONFIG_MAP_DEPENDENT_RESOURCE = "ConfigMapDependentResource"; @@ -30,6 +37,7 @@ public UpdateControl reconcile( InformerRelatedBehaviorTestCustomResource resource, Context context) { numberOfExecutions.addAndGet(1); + log.info("Reconciled for: {}", ResourceID.fromResource(resource)); return UpdateControl.noUpdate(); } From b410c65dd63a24deed9c2726a568ab0cebd715ac Mon Sep 17 00:00:00 2001 From: Chris Laprun Date: Thu, 30 Mar 2023 18:52:10 +0200 Subject: [PATCH 25/25] chore: update to Fabric8 client 6.5.1 (#1813) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Attila Mészáros --- .../operator/InformerRelatedBehaviorITS.java | 5 +++++ pom.xml | 8 ++++---- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/InformerRelatedBehaviorITS.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/InformerRelatedBehaviorITS.java index 3f19c27e53..d214d9ec49 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/InformerRelatedBehaviorITS.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/InformerRelatedBehaviorITS.java @@ -160,6 +160,11 @@ private void assertInformerNotWatchingForAdditionalNamespace(Operator operator) } + // this will be investigated separately under the issue below, it's not crucial functional wise, + // it is rather "something working why it should", not other way around; but it's not a + // showstopper + // https://github.com/java-operator-sdk/java-operator-sdk/issues/1835 + @Disabled @Test void resilientForLoosingPermissionForCustomResource() { setFullResourcesAccess(); diff --git a/pom.xml b/pom.xml index a0e56f1a55..d21b69c3b7 100644 --- a/pom.xml +++ b/pom.xml @@ -43,7 +43,7 @@ https://sonarcloud.io 5.9.1 - 6.4.1 + 6.5.1 1.7.36 2.19.0 5.2.0 @@ -54,6 +54,9 @@ 4.2.0 2.7.3 1.10.5 + 4.10.0 + 3.1.3 + 0.4.3 2.11 3.11.0 @@ -71,9 +74,6 @@ 2.22.0 1.0 1.8.0 - 4.10.0 - 3.1.3 - 0.4.0