From cbb30c05f2278cdd397312ba3503e4c2ede9c44d Mon Sep 17 00:00:00 2001 From: csviri Date: Mon, 2 Jan 2023 15:00:39 +0100 Subject: [PATCH 1/3] feat: dependent resource in the condition instead of resource --- .../workflow/AbstractWorkflowExecutor.java | 11 +---------- .../dependent/workflow/Condition.java | 7 +++---- .../ControllerConfigurationOverriderTest.java | 4 +++- .../dependent/StatefulSetReadyCondition.java | 12 +++++++++--- .../ConfigMapDeletePostCondition.java | 7 +++++-- .../ConfigMapReconcileCondition.java | 6 +++++- .../DeploymentReadyCondition.java | 17 ++++++++++------- .../sample/ExposedIngressCondition.java | 5 ++++- 8 files changed, 40 insertions(+), 29 deletions(-) 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 8fa9a6fd12..ba38131db9 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

{ @@ -102,15 +101,7 @@ protected synchronized void handleNodeExecutionFinish( 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/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 9b6259b0c8f1a5505177a76084a5e7a075fe1c98 Mon Sep 17 00:00:00 2001 From: csviri Date: Mon, 2 Jan 2023 16:25:19 +0100 Subject: [PATCH 2/3] migration guide --- docs/documentation/v4-3-migration.md | 36 ++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) 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)`. From 828c2453d83353ec220d17aa9d374417889482cb Mon Sep 17 00:00:00 2001 From: csviri Date: Tue, 10 Jan 2023 09:21:59 +0100 Subject: [PATCH 3/3] sample fix --- .../sample/bulkdependent/SampleBulkCondition.java | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) 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 82b6a6d2d1..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,23 +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().allMatch(cm -> !cm.getData().isEmpty()); - + var resources = ((CRUDConfigMapBulkDependentResource) dependentResource) + .getSecondaryResources(primary, context); + return resources.values().stream().noneMatch(cm -> cm.getData().isEmpty()); } }