From 6035b85d3358a55ebe953cdd7772da95e594da0e Mon Sep 17 00:00:00 2001 From: Chris Laprun Date: Mon, 8 Aug 2022 17:15:08 +0200 Subject: [PATCH 1/7] fix: grammar --- .../dependent/workflow/WorkflowReconcileResult.java | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowReconcileResult.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowReconcileResult.java index 3c75873920..1e57509fe2 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowReconcileResult.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowReconcileResult.java @@ -71,9 +71,15 @@ public boolean allDependentResourcesReady() { return notReadyDependents.isEmpty(); } + /** + * @deprecated Use {@link #erroredDependentsExist()} instead + */ + @Deprecated(forRemoval = true) public boolean erroredDependentsExists() { return !erroredDependents.isEmpty(); } - + public boolean erroredDependentsExist() { + return !erroredDependents.isEmpty(); + } } From 62994bece7373a556aa0222dd275930f4c408746 Mon Sep 17 00:00:00 2001 From: Chris Laprun Date: Mon, 8 Aug 2022 18:30:47 +0200 Subject: [PATCH 2/7] refactor: clean up WorkflowReconcileResult --- .../workflow/WorkflowReconcileExecutor.java | 21 +++++---- .../workflow/WorkflowReconcileResult.java | 44 +++++++------------ 2 files changed, 26 insertions(+), 39 deletions(-) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowReconcileExecutor.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowReconcileExecutor.java index 0412e1c912..a0336e860e 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowReconcileExecutor.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowReconcileExecutor.java @@ -282,17 +282,16 @@ private boolean hasErroredParent( } private WorkflowReconcileResult createReconcileResult() { - WorkflowReconcileResult workflowReconcileResult = new WorkflowReconcileResult(); - workflowReconcileResult.setErroredDependents(exceptionsDuringExecution - .entrySet().stream() - .collect(Collectors.toMap(e -> e.getKey().getDependentResource(), Map.Entry::getValue))); - workflowReconcileResult.setNotReadyDependents(notReady.stream() - .map(DependentResourceNode::getDependentResource) - .collect(Collectors.toList())); - workflowReconcileResult.setReconciledDependents(reconciled.stream() - .map(DependentResourceNode::getDependentResource).collect(Collectors.toList())); - workflowReconcileResult.setReconcileResults(reconcileResults); - return workflowReconcileResult; + return new WorkflowReconcileResult( + reconciled.stream() + .map(DependentResourceNode::getDependentResource).collect(Collectors.toList()), + notReady.stream() + .map(DependentResourceNode::getDependentResource) + .collect(Collectors.toList()), + exceptionsDuringExecution + .entrySet().stream() + .collect(Collectors.toMap(e -> e.getKey().getDependentResource(), Map.Entry::getValue)), + reconcileResults); } } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowReconcileResult.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowReconcileResult.java index 1e57509fe2..d2cdc7c85f 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowReconcileResult.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowReconcileResult.java @@ -11,51 +11,38 @@ @SuppressWarnings("rawtypes") public class WorkflowReconcileResult { - private List reconciledDependents; - private List notReadyDependents; - private Map erroredDependents; - private Map reconcileResults; + private final List reconciledDependents; + private final List notReadyDependents; + private final Map erroredDependents; + private final Map reconcileResults; + + public WorkflowReconcileResult(List reconciledDependents, + List notReadyDependents, + Map erroredDependents, + Map reconcileResults) { + this.reconciledDependents = reconciledDependents; + this.notReadyDependents = notReadyDependents; + this.erroredDependents = erroredDependents; + this.reconcileResults = reconcileResults; + } public Map getErroredDependents() { return erroredDependents; } - public WorkflowReconcileResult setErroredDependents( - Map erroredDependents) { - this.erroredDependents = erroredDependents; - return this; - } - public List getReconciledDependents() { return reconciledDependents; } - public WorkflowReconcileResult setReconciledDependents( - List reconciledDependents) { - this.reconciledDependents = reconciledDependents; - return this; - } - public List getNotReadyDependents() { return notReadyDependents; } - public WorkflowReconcileResult setNotReadyDependents( - List notReadyDependents) { - this.notReadyDependents = notReadyDependents; - return this; - } - + @SuppressWarnings("unused") public Map getReconcileResults() { return reconcileResults; } - public WorkflowReconcileResult setReconcileResults( - Map reconcileResults) { - this.reconcileResults = reconcileResults; - return this; - } - public void throwAggregateExceptionIfErrorsPresent() { if (!erroredDependents.isEmpty()) { throw createFinalException(); @@ -79,6 +66,7 @@ public boolean erroredDependentsExists() { return !erroredDependents.isEmpty(); } + @SuppressWarnings("unused") public boolean erroredDependentsExist() { return !erroredDependents.isEmpty(); } From 29189739cdd291b74e737f0d462c5cf067ba3803 Mon Sep 17 00:00:00 2001 From: Chris Laprun Date: Mon, 8 Aug 2022 18:41:08 +0200 Subject: [PATCH 3/7] refactor: make it easier to create Conditions --- .../AnnotationControllerConfiguration.java | 3 +- .../api/reconciler/dependent/Dependent.java | 9 ++---- .../reconciler/dependent/VoidCondition.java | 14 --------- .../dependent/workflow/Condition.java | 5 ++-- .../workflow/WorkflowCleanupExecutor.java | 30 ++++++++++--------- .../workflow/WorkflowReconcileExecutor.java | 23 ++++++++------ .../AbstractWorkflowExecutorTest.java | 12 ++++---- .../WorkflowReconcileExecutorTest.java | 10 +++---- .../ConfigMapDeletePostCondition.java | 9 +++--- .../ConfigMapReconcileCondition.java | 8 ++--- .../DeploymentReadyCondition.java | 11 ++++--- .../sample/ExposedIngressCondition.java | 6 ++-- 12 files changed, 64 insertions(+), 76 deletions(-) delete mode 100644 operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/dependent/VoidCondition.java diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/AnnotationControllerConfiguration.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/AnnotationControllerConfiguration.java index 7588a9996d..56e7806890 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/AnnotationControllerConfiguration.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/AnnotationControllerConfiguration.java @@ -22,7 +22,6 @@ import io.javaoperatorsdk.operator.api.reconciler.Reconciler; import io.javaoperatorsdk.operator.api.reconciler.dependent.Dependent; import io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResource; -import io.javaoperatorsdk.operator.api.reconciler.dependent.VoidCondition; import io.javaoperatorsdk.operator.processing.dependent.kubernetes.KubernetesDependent; import io.javaoperatorsdk.operator.processing.dependent.kubernetes.KubernetesDependentResource; import io.javaoperatorsdk.operator.processing.dependent.kubernetes.KubernetesDependentResourceConfig; @@ -276,7 +275,7 @@ public List getDependentResources() { } private Condition instantiateConditionIfNotVoid(Class condition) { - if (condition != VoidCondition.class) { + if (condition != Condition.class) { return instantiateAndConfigureIfNeeded(condition, Condition.class); } return null; diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/dependent/Dependent.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/dependent/Dependent.java index 0eba3fb598..ed52d61632 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/dependent/Dependent.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/dependent/Dependent.java @@ -11,14 +11,11 @@ String name() default NO_VALUE_SET; - @SuppressWarnings("rawtypes") - Class readyPostcondition() default VoidCondition.class; + Class readyPostcondition() default Condition.class; - @SuppressWarnings("rawtypes") - Class reconcilePrecondition() default VoidCondition.class; + Class reconcilePrecondition() default Condition.class; - @SuppressWarnings("rawtypes") - Class deletePostcondition() default VoidCondition.class; + Class deletePostcondition() default Condition.class; String[] dependsOn() default {}; } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/dependent/VoidCondition.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/dependent/VoidCondition.java deleted file mode 100644 index 7f4faf0ed1..0000000000 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/dependent/VoidCondition.java +++ /dev/null @@ -1,14 +0,0 @@ -package io.javaoperatorsdk.operator.api.reconciler.dependent; - -import io.fabric8.kubernetes.api.model.HasMetadata; -import io.javaoperatorsdk.operator.api.reconciler.Context; -import io.javaoperatorsdk.operator.processing.dependent.workflow.Condition; - -/** Used as default value for Condition in annotations */ -@SuppressWarnings("rawtypes") -public class VoidCondition implements Condition { - @Override - public boolean isMet(DependentResource dependentResource, HasMetadata primary, Context context) { - throw new IllegalStateException("This is a placeholder class, should not be called"); - } -} 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 222433118e..427a735274 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 @@ -1,10 +1,11 @@ package io.javaoperatorsdk.operator.processing.dependent.workflow; +import java.util.Optional; + 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 { - boolean isMet(DependentResource dependentResource, P primary, Context

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

context); } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowCleanupExecutor.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowCleanupExecutor.java index 27ff266ad6..05df5a7334 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowCleanupExecutor.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowCleanupExecutor.java @@ -1,6 +1,8 @@ package io.javaoperatorsdk.operator.processing.dependent.workflow; +import java.util.List; import java.util.Map; +import java.util.Optional; import java.util.Set; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.Future; @@ -75,17 +77,16 @@ private synchronized void handleCleanup(DependentResourceNode dependentResourceN } Future nodeFuture = - workflow.getExecutorService().submit( - new NodeExecutor(dependentResourceNode)); + workflow.getExecutorService().submit(new NodeExecutor(dependentResourceNode)); actualExecutions.put(dependentResourceNode, nodeFuture); log.debug("Submitted for cleanup: {}", dependentResourceNode); } private class NodeExecutor implements Runnable { - private final DependentResourceNode dependentResourceNode; + private final DependentResourceNode dependentResourceNode; - private NodeExecutor(DependentResourceNode dependentResourceNode) { + private NodeExecutor(DependentResourceNode dependentResourceNode) { this.dependentResourceNode = dependentResourceNode; } @@ -94,7 +95,7 @@ private NodeExecutor(DependentResourceNode dependentResourceNode) { public void run() { try { var dependentResource = dependentResourceNode.getDependentResource(); - var deletePostCondition = dependentResourceNode.getDeletePostcondition(); + Optional deletePostCondition = dependentResourceNode.getDeletePostcondition(); if (dependentResource instanceof Deleter && !(dependentResource instanceof GarbageCollected)) { @@ -103,7 +104,9 @@ public void run() { } alreadyVisited.add(dependentResourceNode); boolean deletePostConditionMet = - deletePostCondition.map(c -> c.isMet(dependentResource, primary, context)).orElse(true); + deletePostCondition.map(c -> c.isMet(primary, + dependentResourceNode.getDependentResource().getSecondaryResource(primary), + context)).orElse(true); if (deletePostConditionMet) { handleDependentCleaned(dependentResourceNode); } else { @@ -147,22 +150,21 @@ private boolean isCleaningNow(DependentResourceNode dependentResourceNode) return actualExecutions.containsKey(dependentResourceNode); } - private boolean alreadyVisited( - DependentResourceNode dependentResourceNode) { + private boolean alreadyVisited(DependentResourceNode dependentResourceNode) { return alreadyVisited.contains(dependentResourceNode); } - private boolean allDependentsCleaned( - DependentResourceNode dependentResourceNode) { - var parents = dependentResourceNode.getParents(); + @SuppressWarnings("unchecked") + private boolean allDependentsCleaned(DependentResourceNode dependentResourceNode) { + List parents = dependentResourceNode.getParents(); return parents.isEmpty() || parents.stream() .allMatch(d -> alreadyVisited(d) && !postDeleteConditionNotMet.contains(d)); } - private boolean hasErroredDependent( - DependentResourceNode dependentResourceNode) { - var parents = dependentResourceNode.getParents(); + @SuppressWarnings("unchecked") + private boolean hasErroredDependent(DependentResourceNode dependentResourceNode) { + List parents = dependentResourceNode.getParents(); return !parents.isEmpty() && parents.stream().anyMatch(exceptionsDuringExecution::containsKey); } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowReconcileExecutor.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowReconcileExecutor.java index a0336e860e..bf617d85b5 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowReconcileExecutor.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowReconcileExecutor.java @@ -3,6 +3,7 @@ import java.util.HashMap; import java.util.HashSet; import java.util.Map; +import java.util.Optional; import java.util.Set; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.Future; @@ -71,8 +72,7 @@ public synchronized WorkflowReconcileResult reconcile() { return createReconcileResult(); } - private synchronized void handleReconcile( - DependentResourceNode dependentResourceNode) { + private synchronized void handleReconcile(DependentResourceNode dependentResourceNode) { log.debug("Submitting for reconcile: {}", dependentResourceNode); if (alreadyVisited(dependentResourceNode) @@ -84,8 +84,9 @@ private synchronized void handleReconcile( return; } - boolean reconcileConditionMet = dependentResourceNode.getReconcilePrecondition().map( - rc -> rc.isMet(dependentResourceNode.getDependentResource(), primary, context)) + boolean reconcileConditionMet = dependentResourceNode.getReconcilePrecondition() + .map(rc -> rc.isMet(primary, + dependentResourceNode.getDependentResource().getSecondaryResource(primary), context)) .orElse(true); if (!reconcileConditionMet) { @@ -145,9 +146,9 @@ private synchronized void setAlreadyReconciledButNotReady( private class NodeReconcileExecutor implements Runnable { - private final DependentResourceNode dependentResourceNode; + private final DependentResourceNode dependentResourceNode; - private NodeReconcileExecutor(DependentResourceNode dependentResourceNode) { + private NodeReconcileExecutor(DependentResourceNode dependentResourceNode) { this.dependentResourceNode = dependentResourceNode; } @@ -165,8 +166,10 @@ public void run() { ReconcileResult reconcileResult = dependentResource.reconcile(primary, context); reconcileResults.put(dependentResource, reconcileResult); reconciled.add(dependentResourceNode); - boolean ready = dependentResourceNode.getReadyPostcondition() - .map(rc -> rc.isMet(dependentResource, primary, context)) + boolean ready = ((Optional) dependentResourceNode.getReadyPostcondition()) + .map(rc -> rc.isMet(primary, + dependentResourceNode.getDependentResource().getSecondaryResource(primary), + context)) .orElse(true); if (ready) { @@ -205,7 +208,9 @@ public void run() { } alreadyVisited.add(dependentResourceNode); boolean deletePostConditionMet = - deletePostCondition.map(c -> c.isMet(dependentResource, primary, context)).orElse(true); + deletePostCondition.map(c -> c.isMet(primary, + dependentResourceNode.getDependentResource().getSecondaryResource(primary), + context)).orElse(true); if (deletePostConditionMet) { handleDependentDeleted(dependentResourceNode); } else { diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/workflow/AbstractWorkflowExecutorTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/workflow/AbstractWorkflowExecutorTest.java index 0ad559b7ca..8dfab3fa20 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/workflow/AbstractWorkflowExecutorTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/workflow/AbstractWorkflowExecutorTest.java @@ -21,17 +21,17 @@ public class AbstractWorkflowExecutorTest { protected TestErrorDependent drError = new TestErrorDependent("ERROR_1"); protected TestErrorDeleterDependent errorDD = new TestErrorDeleterDependent("ERROR_DELETER"); - protected final Condition noMetDeletePostCondition = - (dependentResource, primary, context) -> false; - protected final Condition metDeletePostCondition = - (dependentResource, primary, context) -> true; + @SuppressWarnings("rawtypes") + protected final Condition noMetDeletePostCondition = (primary, secondary, context) -> false; + @SuppressWarnings("rawtypes") + protected final Condition metDeletePostCondition = (primary, secondary, context) -> true; protected List executionHistory = Collections.synchronizedList(new ArrayList<>()); public class TestDependent implements DependentResource { - private String name; + private final String name; public TestDependent(String name) { this.name = name; @@ -95,7 +95,7 @@ public void delete(TestCustomResource primary, Context conte } public class TestErrorDependent implements DependentResource { - private String name; + private final String name; public TestErrorDependent(String name) { this.name = name; diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowReconcileExecutorTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowReconcileExecutorTest.java index c0c2e9899c..fa9d757b67 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowReconcileExecutorTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowReconcileExecutorTest.java @@ -13,15 +13,13 @@ @SuppressWarnings("rawtypes") class WorkflowReconcileExecutorTest extends AbstractWorkflowExecutorTest { - private final Condition met_reconcile_condition = - (dependentResource, primary, context) -> true; - private final Condition not_met_reconcile_condition = - (dependentResource, primary, context) -> false; + private final Condition met_reconcile_condition = (primary, secondary, context) -> true; + private final Condition not_met_reconcile_condition = (primary, secondary, context) -> false; private final Condition metReadyCondition = - (dependentResource, primary, context) -> true; + (primary, secondary, context) -> true; private final Condition notMetReadyCondition = - (dependentResource, primary, context) -> false; + (primary, secondary, context) -> false; @Test void reconcileTopLevelResources() { 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 c5c908dfe6..a88f23fffe 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 @@ -1,11 +1,12 @@ package io.javaoperatorsdk.operator.sample.workflowallfeature; +import java.util.Optional; + import org.slf4j.Logger; import org.slf4j.LoggerFactory; 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 @@ -15,9 +16,9 @@ public class ConfigMapDeletePostCondition @Override public boolean isMet( - DependentResource dependentResource, - WorkflowAllFeatureCustomResource primary, Context context) { - var configMapDeleted = dependentResource.getSecondaryResource(primary).isEmpty(); + WorkflowAllFeatureCustomResource primary, Optional secondary, + Context context) { + var configMapDeleted = secondary.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 65409efc36..42085235c6 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 @@ -1,17 +1,17 @@ package io.javaoperatorsdk.operator.sample.workflowallfeature; +import java.util.Optional; + 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( - DependentResource dependentResource, - WorkflowAllFeatureCustomResource primary, Context context) { + public boolean isMet(WorkflowAllFeatureCustomResource primary, Optional secondary, + 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 c8646f6ea5..b12514fc70 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 @@ -1,18 +1,17 @@ package io.javaoperatorsdk.operator.sample.workflowallfeature; +import java.util.Optional; + 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( - DependentResource dependentResource, - WorkflowAllFeatureCustomResource primary, Context context) { - - var deployment = dependentResource.getSecondaryResource(primary).orElseThrow(); + public boolean isMet(WorkflowAllFeatureCustomResource primary, Optional secondary, + Context context) { + var deployment = secondary.orElseThrow(); var readyReplicas = deployment.getStatus().getReadyReplicas(); return readyReplicas != null && deployment.getSpec().getReplicas().equals(readyReplicas); 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 218d1f8ca2..9791f426a9 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 @@ -1,14 +1,14 @@ package io.javaoperatorsdk.operator.sample; +import java.util.Optional; + 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(DependentResource dependentResource, WebPage primary, - Context context) { + public boolean isMet(WebPage primary, Optional secondary, Context context) { return primary.getSpec().getExposed(); } } From c5cead44c6a25f5399ed7ee689770b575e8e2e20 Mon Sep 17 00:00:00 2001 From: Chris Laprun Date: Wed, 10 Aug 2022 17:00:27 +0200 Subject: [PATCH 4/7] fix: properly implement Condition --- .../api/config/ControllerConfigurationOverriderTest.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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 80156aaccc..0450af13a5 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 @@ -342,8 +342,8 @@ public UpdateControl reconcile(ConfigMap resource, Context private static class TestCondition implements Condition { @Override - public boolean isMet(DependentResource dependentResource, - ConfigMap primary, Context context) { + public boolean isMet(ConfigMap primary, Optional secondary, + Context context) { return true; } } From 31eaaf2877cb8859ffad7c0ba36458876e5427b0 Mon Sep 17 00:00:00 2001 From: Chris Laprun Date: Thu, 11 Aug 2022 09:47:40 +0200 Subject: [PATCH 5/7] refactor: simplify handling of filters --- .../AnnotationControllerConfiguration.java | 81 ++++++++----------- .../reconciler/ControllerConfiguration.java | 10 +-- .../kubernetes/KubernetesDependent.java | 14 ++-- .../source/filter/VoidGenericFilter.java | 10 --- .../event/source/filter/VoidOnAddFilter.java | 10 --- .../source/filter/VoidOnDeleteFilter.java | 10 --- .../source/filter/VoidOnUpdateFilter.java | 10 --- 7 files changed, 45 insertions(+), 100 deletions(-) delete mode 100644 operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/filter/VoidGenericFilter.java delete mode 100644 operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/filter/VoidOnAddFilter.java delete mode 100644 operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/filter/VoidOnDeleteFilter.java delete mode 100644 operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/filter/VoidOnUpdateFilter.java diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/AnnotationControllerConfiguration.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/AnnotationControllerConfiguration.java index 56e7806890..b5e3fffcf0 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/AnnotationControllerConfiguration.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/AnnotationControllerConfiguration.java @@ -33,10 +33,6 @@ import io.javaoperatorsdk.operator.processing.event.source.filter.OnAddFilter; import io.javaoperatorsdk.operator.processing.event.source.filter.OnDeleteFilter; import io.javaoperatorsdk.operator.processing.event.source.filter.OnUpdateFilter; -import io.javaoperatorsdk.operator.processing.event.source.filter.VoidGenericFilter; -import io.javaoperatorsdk.operator.processing.event.source.filter.VoidOnAddFilter; -import io.javaoperatorsdk.operator.processing.event.source.filter.VoidOnDeleteFilter; -import io.javaoperatorsdk.operator.processing.event.source.filter.VoidOnUpdateFilter; import io.javaoperatorsdk.operator.processing.retry.Retry; import static io.javaoperatorsdk.operator.api.reconciler.Constants.DEFAULT_NAMESPACES_SET; @@ -45,6 +41,10 @@ public class AnnotationControllerConfiguration

implements io.javaoperatorsdk.operator.api.config.ControllerConfiguration

{ + private static final String CONTROLLER_CONFIG_ANNOTATION = + ControllerConfiguration.class.getSimpleName(); + private static final String KUBE_DEPENDENT_NAME = KubernetesDependent.class.getSimpleName(); + protected final Reconciler

reconciler; private final ControllerConfiguration annotation; private List specs; @@ -152,18 +152,19 @@ public Optional maxReconciliationInterval() { @Override public RateLimiter getRateLimiter() { final Class rateLimiterClass = annotation.rateLimiter(); - return instantiateAndConfigureIfNeeded(rateLimiterClass, RateLimiter.class); + return instantiateAndConfigureIfNeeded(rateLimiterClass, RateLimiter.class, + CONTROLLER_CONFIG_ANNOTATION); } @Override public Retry getRetry() { final Class retryClass = annotation.retry(); - return instantiateAndConfigureIfNeeded(retryClass, Retry.class); + return instantiateAndConfigureIfNeeded(retryClass, Retry.class, CONTROLLER_CONFIG_ANNOTATION); } @SuppressWarnings("unchecked") - private T instantiateAndConfigureIfNeeded(Class targetClass, - Class expectedType) { + protected T instantiateAndConfigureIfNeeded(Class targetClass, + Class expectedType, String context) { try { final Constructor constructor = targetClass.getDeclaredConstructor(); constructor.setAccessible(true); @@ -183,42 +184,24 @@ private T instantiateAndConfigureIfNeeded(Class targetClass, | NoSuchMethodException e) { throw new OperatorException("Couldn't instantiate " + expectedType.getSimpleName() + " '" + targetClass.getName() + "' for '" + getName() - + "' reconciler. You need to provide an accessible no-arg constructor.", e); + + "' reconciler in " + context + + ". You need to provide an accessible no-arg constructor.", e); } } @Override @SuppressWarnings("unchecked") public Optional> onAddFilter() { - return (Optional>) createFilter(annotation.onAddFilter(), FilterType.onAdd, - annotation.getClass().getSimpleName()); - } - - private enum FilterType { - onAdd(VoidOnAddFilter.class), onUpdate(VoidOnUpdateFilter.class), onDelete( - VoidOnDeleteFilter.class), generic(VoidGenericFilter.class); - - final Class defaultValue; - - FilterType(Class defaultValue) { - this.defaultValue = defaultValue; - } + return (Optional>) createFilter(annotation.onAddFilter(), OnAddFilter.class, + CONTROLLER_CONFIG_ANNOTATION); } - private Optional createFilter(Class filter, FilterType filterType, String origin) { - if (filterType.defaultValue.equals(filter)) { + protected Optional createFilter(Class filter, Class defaultValue, + String origin) { + if (defaultValue.equals(filter)) { return Optional.empty(); } else { - try { - var instance = (T) filter.getDeclaredConstructor().newInstance(); - return Optional.of(instance); - } catch (InstantiationException | IllegalAccessException | InvocationTargetException - | NoSuchMethodException e) { - throw new OperatorException( - "Couldn't create " + filterType + " filter from " + filter.getName() + " class in " - + origin + " for reconciler " + getName(), - e); - } + return Optional.of(instantiateAndConfigureIfNeeded(filter, defaultValue, origin)); } } @@ -226,14 +209,14 @@ private Optional createFilter(Class filter, FilterType filterType, Str @Override public Optional> onUpdateFilter() { return (Optional>) createFilter(annotation.onUpdateFilter(), - FilterType.onUpdate, annotation.getClass().getSimpleName()); + OnUpdateFilter.class, CONTROLLER_CONFIG_ANNOTATION); } @SuppressWarnings("unchecked") @Override public Optional> genericFilter() { return (Optional>) createFilter(annotation.genericFilter(), - FilterType.generic, annotation.getClass().getSimpleName()); + GenericFilter.class, CONTROLLER_CONFIG_ANNOTATION); } @SuppressWarnings({"rawtypes", "unchecked"}) @@ -259,13 +242,14 @@ public List getDependentResources() { var spec = specsMap.get(name); if (spec != null) { throw new IllegalArgumentException( - "A DependentResource named: " + name + " already exists: " + spec); + "A DependentResource named '" + name + "' already exists: " + spec); } + final var context = "DependentResource of type '" + dependentType.getName() + "'"; spec = new DependentResourceSpec(dependentType, config, name, Set.of(dependent.dependsOn()), - instantiateConditionIfNotVoid(dependent.readyPostcondition()), - instantiateConditionIfNotVoid(dependent.reconcilePrecondition()), - instantiateConditionIfNotVoid(dependent.deletePostcondition())); + instantiateConditionIfNotDefault(dependent.readyPostcondition(), context), + instantiateConditionIfNotDefault(dependent.reconcilePrecondition(), context), + instantiateConditionIfNotDefault(dependent.deletePostcondition(), context)); specsMap.put(name, spec); } @@ -274,9 +258,10 @@ public List getDependentResources() { return specs; } - private Condition instantiateConditionIfNotVoid(Class condition) { + protected Condition instantiateConditionIfNotDefault(Class condition, + String context) { if (condition != Condition.class) { - return instantiateAndConfigureIfNeeded(condition, Condition.class); + return instantiateAndConfigureIfNeeded(condition, Condition.class, context); } return null; } @@ -312,17 +297,19 @@ private Object createKubernetesResourceConfig(Class final var fromAnnotation = kubeDependent.labelSelector(); labelSelector = Constants.NO_VALUE_SET.equals(fromAnnotation) ? null : fromAnnotation; - final var kubeDependentName = KubernetesDependent.class.getSimpleName(); - onAddFilter = createFilter(kubeDependent.onAddFilter(), FilterType.onAdd, kubeDependentName) + + final var context = + KUBE_DEPENDENT_NAME + " annotation on " + dependentType.getName() + " DependentResource"; + onAddFilter = createFilter(kubeDependent.onAddFilter(), OnAddFilter.class, context) .orElse(null); onUpdateFilter = - createFilter(kubeDependent.onUpdateFilter(), FilterType.onUpdate, kubeDependentName) + createFilter(kubeDependent.onUpdateFilter(), OnUpdateFilter.class, context) .orElse(null); onDeleteFilter = - createFilter(kubeDependent.onDeleteFilter(), FilterType.onDelete, kubeDependentName) + createFilter(kubeDependent.onDeleteFilter(), OnDeleteFilter.class, context) .orElse(null); genericFilter = - createFilter(kubeDependent.genericFilter(), FilterType.generic, kubeDependentName) + createFilter(kubeDependent.genericFilter(), GenericFilter.class, context) .orElse(null); } 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 4a35cfd205..ec76adf89d 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,7 +6,6 @@ import java.lang.annotation.RetentionPolicy; import java.lang.annotation.Target; -import io.fabric8.kubernetes.api.model.HasMetadata; import io.javaoperatorsdk.operator.api.reconciler.dependent.Dependent; import io.javaoperatorsdk.operator.processing.event.rate.LinearRateLimiter; import io.javaoperatorsdk.operator.processing.event.rate.RateLimiter; @@ -14,9 +13,6 @@ import io.javaoperatorsdk.operator.processing.event.source.filter.GenericFilter; import io.javaoperatorsdk.operator.processing.event.source.filter.OnAddFilter; import io.javaoperatorsdk.operator.processing.event.source.filter.OnUpdateFilter; -import io.javaoperatorsdk.operator.processing.event.source.filter.VoidGenericFilter; -import io.javaoperatorsdk.operator.processing.event.source.filter.VoidOnAddFilter; -import io.javaoperatorsdk.operator.processing.event.source.filter.VoidOnUpdateFilter; import io.javaoperatorsdk.operator.processing.retry.GenericRetry; import io.javaoperatorsdk.operator.processing.retry.Retry; @@ -79,15 +75,15 @@ /** * Filter of onAdd events of resources. **/ - Class> onAddFilter() default VoidOnAddFilter.class; + Class onAddFilter() default OnAddFilter.class; /** Filter of onUpdate events of resources. */ - Class> onUpdateFilter() default VoidOnUpdateFilter.class; + Class onUpdateFilter() default OnUpdateFilter.class; /** * Filter applied to all operations (add, update, delete). Used to ignore some resources. **/ - Class> genericFilter() default VoidGenericFilter.class; + Class genericFilter() default GenericFilter.class; /** * Optional configuration of the maximal interval the SDK will wait for a reconciliation request diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/KubernetesDependent.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/KubernetesDependent.java index ef9d595fd6..53360ef394 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/KubernetesDependent.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/KubernetesDependent.java @@ -5,9 +5,11 @@ import java.lang.annotation.RetentionPolicy; import java.lang.annotation.Target; -import io.fabric8.kubernetes.api.model.HasMetadata; import io.javaoperatorsdk.operator.api.reconciler.Constants; -import io.javaoperatorsdk.operator.processing.event.source.filter.*; +import io.javaoperatorsdk.operator.processing.event.source.filter.GenericFilter; +import io.javaoperatorsdk.operator.processing.event.source.filter.OnAddFilter; +import io.javaoperatorsdk.operator.processing.event.source.filter.OnDeleteFilter; +import io.javaoperatorsdk.operator.processing.event.source.filter.OnUpdateFilter; import static io.javaoperatorsdk.operator.api.reconciler.Constants.NO_VALUE_SET; @@ -35,11 +37,11 @@ */ String labelSelector() default NO_VALUE_SET; - Class> onAddFilter() default VoidOnAddFilter.class; + Class onAddFilter() default OnAddFilter.class; - Class> onUpdateFilter() default VoidOnUpdateFilter.class; + Class onUpdateFilter() default OnUpdateFilter.class; - Class> onDeleteFilter() default VoidOnDeleteFilter.class; + Class onDeleteFilter() default OnDeleteFilter.class; - Class> genericFilter() default VoidGenericFilter.class; + Class genericFilter() default GenericFilter.class; } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/filter/VoidGenericFilter.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/filter/VoidGenericFilter.java deleted file mode 100644 index ef6a095492..0000000000 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/filter/VoidGenericFilter.java +++ /dev/null @@ -1,10 +0,0 @@ -package io.javaoperatorsdk.operator.processing.event.source.filter; - -import io.fabric8.kubernetes.api.model.HasMetadata; - -public class VoidGenericFilter implements GenericFilter { - @Override - public boolean accept(HasMetadata hasMetadata) { - return true; - } -} diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/filter/VoidOnAddFilter.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/filter/VoidOnAddFilter.java deleted file mode 100644 index e0a67d58c6..0000000000 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/filter/VoidOnAddFilter.java +++ /dev/null @@ -1,10 +0,0 @@ -package io.javaoperatorsdk.operator.processing.event.source.filter; - -import io.fabric8.kubernetes.api.model.HasMetadata; - -public class VoidOnAddFilter implements OnAddFilter { - @Override - public boolean accept(HasMetadata hasMetadata) { - return true; - } -} diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/filter/VoidOnDeleteFilter.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/filter/VoidOnDeleteFilter.java deleted file mode 100644 index 6f21a53c0d..0000000000 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/filter/VoidOnDeleteFilter.java +++ /dev/null @@ -1,10 +0,0 @@ -package io.javaoperatorsdk.operator.processing.event.source.filter; - -import io.fabric8.kubernetes.api.model.HasMetadata; - -public class VoidOnDeleteFilter implements OnDeleteFilter { - @Override - public boolean accept(HasMetadata hasMetadata, Boolean aBoolean) { - return true; - } -} diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/filter/VoidOnUpdateFilter.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/filter/VoidOnUpdateFilter.java deleted file mode 100644 index 6b09572710..0000000000 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/filter/VoidOnUpdateFilter.java +++ /dev/null @@ -1,10 +0,0 @@ -package io.javaoperatorsdk.operator.processing.event.source.filter; - -import io.fabric8.kubernetes.api.model.HasMetadata; - -public class VoidOnUpdateFilter implements OnUpdateFilter { - @Override - public boolean accept(HasMetadata hasMetadata, HasMetadata hasMetadata2) { - return true; - } -} From c7689f11b4f38c80972442d3c120e69babc6ba2a Mon Sep 17 00:00:00 2001 From: Chris Laprun Date: Wed, 24 Aug 2022 15:59:55 +0200 Subject: [PATCH 6/7] fix: do not use Optional in Condition, fix generics --- .../dependent/workflow/Condition.java | 16 ++++++++++--- .../workflow/DependentResourceNode.java | 20 ++++++++-------- .../workflow/WorkflowCleanupExecutor.java | 13 ++++++---- .../workflow/WorkflowReconcileExecutor.java | 24 ++++++++++--------- .../ControllerConfigurationOverriderTest.java | 3 +-- .../ConfigMapDeletePostCondition.java | 6 ++--- .../ConfigMapReconcileCondition.java | 4 +--- .../DeploymentReadyCondition.java | 8 +++---- .../sample/ExposedIngressCondition.java | 4 +--- 9 files changed, 53 insertions(+), 45 deletions(-) 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 427a735274..68ed5530ec 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 @@ -1,11 +1,21 @@ package io.javaoperatorsdk.operator.processing.dependent.workflow; -import java.util.Optional; - import io.fabric8.kubernetes.api.model.HasMetadata; import io.javaoperatorsdk.operator.api.reconciler.Context; public interface Condition { - boolean isMet(P primary, Optional secondary, Context

context); + /** + * Checks whether a condition holds true for a given + * {@link io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResource} based on the + * observed cluster state. + * + * @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); } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/DependentResourceNode.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/DependentResourceNode.java index d7af13bc43..e317145916 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/DependentResourceNode.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/DependentResourceNode.java @@ -11,9 +11,9 @@ public class DependentResourceNode { private final DependentResource dependentResource; - private Condition reconcilePrecondition; - private Condition deletePostcondition; - private Condition readyPostcondition; + private Condition reconcilePrecondition; + private Condition deletePostcondition; + private Condition readyPostcondition; private final List dependsOn = new LinkedList<>(); private final List parents = new LinkedList<>(); @@ -22,12 +22,12 @@ public DependentResourceNode(DependentResource dependentResource) { } public DependentResourceNode(DependentResource dependentResource, - Condition reconcilePrecondition) { + Condition reconcilePrecondition) { this(dependentResource, reconcilePrecondition, null); } public DependentResourceNode(DependentResource dependentResource, - Condition reconcilePrecondition, Condition deletePostcondition) { + Condition reconcilePrecondition, Condition deletePostcondition) { this.dependentResource = dependentResource; this.reconcilePrecondition = reconcilePrecondition; this.deletePostcondition = deletePostcondition; @@ -37,11 +37,11 @@ public DependentResource getDependentResource() { return dependentResource; } - public Optional getReconcilePrecondition() { + public Optional> getReconcilePrecondition() { return Optional.ofNullable(reconcilePrecondition); } - public Optional getDeletePostcondition() { + public Optional> getDeletePostcondition() { return Optional.ofNullable(deletePostcondition); } @@ -63,12 +63,12 @@ public String toString() { } public DependentResourceNode setReconcilePrecondition( - Condition reconcilePrecondition) { + Condition reconcilePrecondition) { this.reconcilePrecondition = reconcilePrecondition; return this; } - public DependentResourceNode setDeletePostcondition(Condition cleanupCondition) { + public DependentResourceNode setDeletePostcondition(Condition cleanupCondition) { this.deletePostcondition = cleanupCondition; return this; } @@ -77,7 +77,7 @@ public Optional> getReadyPostcondition() { return Optional.ofNullable(readyPostcondition); } - public DependentResourceNode setReadyPostcondition(Condition readyPostcondition) { + public DependentResourceNode setReadyPostcondition(Condition readyPostcondition) { this.readyPostcondition = readyPostcondition; return this; } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowCleanupExecutor.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowCleanupExecutor.java index 05df5a7334..f32192e839 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowCleanupExecutor.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowCleanupExecutor.java @@ -65,6 +65,7 @@ private synchronized boolean noMoreExecutionsScheduled() { return actualExecutions.isEmpty(); } + @SuppressWarnings({"rawtypes", "unchecked"}) private synchronized void handleCleanup(DependentResourceNode dependentResourceNode) { log.debug("Submitting for cleanup: {}", dependentResourceNode); @@ -82,11 +83,11 @@ private synchronized void handleCleanup(DependentResourceNode dependentResourceN log.debug("Submitted for cleanup: {}", dependentResourceNode); } - private class NodeExecutor implements Runnable { + private class NodeExecutor implements Runnable { - private final DependentResourceNode dependentResourceNode; + private final DependentResourceNode dependentResourceNode; - private NodeExecutor(DependentResourceNode dependentResourceNode) { + private NodeExecutor(DependentResourceNode dependentResourceNode) { this.dependentResourceNode = dependentResourceNode; } @@ -95,7 +96,8 @@ private NodeExecutor(DependentResourceNode dependentResourceNode) { public void run() { try { var dependentResource = dependentResourceNode.getDependentResource(); - Optional deletePostCondition = dependentResourceNode.getDeletePostcondition(); + Optional> deletePostCondition = + dependentResourceNode.getDeletePostcondition(); if (dependentResource instanceof Deleter && !(dependentResource instanceof GarbageCollected)) { @@ -105,7 +107,8 @@ public void run() { alreadyVisited.add(dependentResourceNode); boolean deletePostConditionMet = deletePostCondition.map(c -> c.isMet(primary, - dependentResourceNode.getDependentResource().getSecondaryResource(primary), + dependentResourceNode.getDependentResource().getSecondaryResource(primary) + .orElse(null), context)).orElse(true); if (deletePostConditionMet) { handleDependentCleaned(dependentResourceNode); diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowReconcileExecutor.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowReconcileExecutor.java index bf617d85b5..567f999ba9 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowReconcileExecutor.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowReconcileExecutor.java @@ -3,7 +3,6 @@ import java.util.HashMap; import java.util.HashSet; import java.util.Map; -import java.util.Optional; import java.util.Set; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.Future; @@ -72,7 +71,7 @@ public synchronized WorkflowReconcileResult reconcile() { return createReconcileResult(); } - private synchronized void handleReconcile(DependentResourceNode dependentResourceNode) { + private synchronized void handleReconcile(DependentResourceNode dependentResourceNode) { log.debug("Submitting for reconcile: {}", dependentResourceNode); if (alreadyVisited(dependentResourceNode) @@ -86,7 +85,8 @@ private synchronized void handleReconcile(DependentResourceNode dependentR boolean reconcileConditionMet = dependentResourceNode.getReconcilePrecondition() .map(rc -> rc.isMet(primary, - dependentResourceNode.getDependentResource().getSecondaryResource(primary), context)) + dependentResourceNode.getDependentResource().getSecondaryResource(primary).orElse(null), + context)) .orElse(true); if (!reconcileConditionMet) { @@ -144,9 +144,9 @@ private synchronized void setAlreadyReconciledButNotReady( notReady.add(dependentResourceNode); } - private class NodeReconcileExecutor implements Runnable { + private class NodeReconcileExecutor implements Runnable { - private final DependentResourceNode dependentResourceNode; + private final DependentResourceNode dependentResourceNode; private NodeReconcileExecutor(DependentResourceNode dependentResourceNode) { this.dependentResourceNode = dependentResourceNode; @@ -166,9 +166,10 @@ public void run() { ReconcileResult reconcileResult = dependentResource.reconcile(primary, context); reconcileResults.put(dependentResource, reconcileResult); reconciled.add(dependentResourceNode); - boolean ready = ((Optional) dependentResourceNode.getReadyPostcondition()) + boolean ready = dependentResourceNode.getReadyPostcondition() .map(rc -> rc.isMet(primary, - dependentResourceNode.getDependentResource().getSecondaryResource(primary), + dependentResourceNode.getDependentResource().getSecondaryResource(primary) + .orElse(null), context)) .orElse(true); @@ -187,11 +188,11 @@ public void run() { } } - private class NodeDeleteExecutor implements Runnable { + private class NodeDeleteExecutor implements Runnable { - private final DependentResourceNode dependentResourceNode; + private final DependentResourceNode dependentResourceNode; - private NodeDeleteExecutor(DependentResourceNode dependentResourceNode) { + private NodeDeleteExecutor(DependentResourceNode dependentResourceNode) { this.dependentResourceNode = dependentResourceNode; } @@ -209,7 +210,8 @@ public void run() { alreadyVisited.add(dependentResourceNode); boolean deletePostConditionMet = deletePostCondition.map(c -> c.isMet(primary, - dependentResourceNode.getDependentResource().getSecondaryResource(primary), + dependentResourceNode.getDependentResource().getSecondaryResource(primary) + .orElse(null), context)).orElse(true); if (deletePostConditionMet) { handleDependentDeleted(dependentResourceNode); 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 0450af13a5..127cd535b1 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 @@ -342,8 +342,7 @@ public UpdateControl reconcile(ConfigMap resource, Context private static class TestCondition implements Condition { @Override - public boolean isMet(ConfigMap primary, Optional secondary, - Context context) { + public boolean isMet(ConfigMap primary, ConfigMap secondary, Context context) { return true; } } 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 a88f23fffe..e7d09a5e95 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 @@ -1,7 +1,5 @@ package io.javaoperatorsdk.operator.sample.workflowallfeature; -import java.util.Optional; - import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -16,9 +14,9 @@ public class ConfigMapDeletePostCondition @Override public boolean isMet( - WorkflowAllFeatureCustomResource primary, Optional secondary, + WorkflowAllFeatureCustomResource primary, ConfigMap secondary, Context context) { - var configMapDeleted = secondary.isEmpty(); + var configMapDeleted = secondary == null; 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 42085235c6..c413b5f03c 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 @@ -1,7 +1,5 @@ package io.javaoperatorsdk.operator.sample.workflowallfeature; -import java.util.Optional; - import io.fabric8.kubernetes.api.model.ConfigMap; import io.javaoperatorsdk.operator.api.reconciler.Context; import io.javaoperatorsdk.operator.processing.dependent.workflow.Condition; @@ -10,7 +8,7 @@ public class ConfigMapReconcileCondition implements Condition { @Override - public boolean isMet(WorkflowAllFeatureCustomResource primary, Optional secondary, + public boolean isMet(WorkflowAllFeatureCustomResource primary, ConfigMap secondary, 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 b12514fc70..8681097962 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 @@ -1,7 +1,5 @@ package io.javaoperatorsdk.operator.sample.workflowallfeature; -import java.util.Optional; - import io.fabric8.kubernetes.api.model.apps.Deployment; import io.javaoperatorsdk.operator.api.reconciler.Context; import io.javaoperatorsdk.operator.processing.dependent.workflow.Condition; @@ -9,9 +7,11 @@ public class DeploymentReadyCondition implements Condition { @Override - public boolean isMet(WorkflowAllFeatureCustomResource primary, Optional secondary, + public boolean isMet(WorkflowAllFeatureCustomResource primary, Deployment deployment, Context context) { - var deployment = secondary.orElseThrow(); + if (deployment == null) { + return false; + } var readyReplicas = deployment.getStatus().getReadyReplicas(); return readyReplicas != null && deployment.getSpec().getReplicas().equals(readyReplicas); 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 9791f426a9..5eb0135586 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 @@ -1,14 +1,12 @@ package io.javaoperatorsdk.operator.sample; -import java.util.Optional; - import io.fabric8.kubernetes.api.model.networking.v1.Ingress; import io.javaoperatorsdk.operator.api.reconciler.Context; import io.javaoperatorsdk.operator.processing.dependent.workflow.Condition; public class ExposedIngressCondition implements Condition { @Override - public boolean isMet(WebPage primary, Optional secondary, Context context) { + public boolean isMet(WebPage primary, Ingress secondary, Context context) { return primary.getSpec().getExposed(); } } From 6ea3dded0e02a91fdd317ba043e2e9290edfef0e Mon Sep 17 00:00:00 2001 From: Chris Laprun Date: Mon, 29 Aug 2022 12:07:42 +0200 Subject: [PATCH 7/7] docs: add javadoc skip-ci --- .../api/reconciler/dependent/Dependent.java | 39 +++++++++++++++++++ .../kubernetes/KubernetesDependent.java | 24 ++++++++++++ 2 files changed, 63 insertions(+) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/dependent/Dependent.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/dependent/Dependent.java index ed52d61632..90ba701a6a 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/dependent/Dependent.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/dependent/Dependent.java @@ -4,18 +4,57 @@ import static io.javaoperatorsdk.operator.api.reconciler.Constants.NO_VALUE_SET; +/** + * The annotation used to create managed {@link DependentResource} associated with a given + * {@link io.javaoperatorsdk.operator.api.reconciler.Reconciler} + */ public @interface Dependent { @SuppressWarnings("rawtypes") Class type(); + /** + * The name of this dependent. This is needed to be able to refer to it when creating dependencies + * between dependent resources. + * + * @return the name if it has been set, + * {@link io.javaoperatorsdk.operator.api.reconciler.Constants#NO_VALUE_SET} otherwise + */ String name() default NO_VALUE_SET; + /** + * The condition (if it exists) that needs to become true before the workflow can further proceed. + * + * @return a {@link Condition} implementation, defaulting to the interface itself if no value is + * set + */ Class readyPostcondition() default Condition.class; + /** + * The condition (if it exists) that needs to become true before the associated + * {@link DependentResource} is reconciled. Note that if this condition is set and the condition + * doesn't hold true, the associated secondary will be deleted. + * + * @return a {@link Condition} implementation, defaulting to the interface itself if no value is + * set + */ Class reconcilePrecondition() default Condition.class; + /** + * The condition (if it exists) that needs to become true before further reconciliation of + * dependents can proceed after the secondary resource associated with this dependent is supposed + * to have been deleted. + * + * @return a {@link Condition} implementation, defaulting to the interface itself if no value is + * set + */ Class deletePostcondition() default Condition.class; + /** + * The list of named dependents that need to be reconciled before this one can be. + * + * @return the list (possibly empty) of named dependents that need to be reconciled before this + * one can be + */ String[] dependsOn() default {}; } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/KubernetesDependent.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/KubernetesDependent.java index 53360ef394..f66ff95373 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/KubernetesDependent.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/KubernetesDependent.java @@ -37,11 +37,35 @@ */ String labelSelector() default NO_VALUE_SET; + /** + * Optional {@link OnAddFilter} to filter events sent to this KubernetesDependent + * + * @return the {@link OnAddFilter} filter implementation to use, defaulting to the interface + * itself if no value is set + */ Class onAddFilter() default OnAddFilter.class; + /** + * Optional {@link OnUpdateFilter} to filter events sent to this KubernetesDependent + * + * @return the {@link OnUpdateFilter} filter implementation to use, defaulting to the interface + * itself if no value is set + */ Class onUpdateFilter() default OnUpdateFilter.class; + /** + * Optional {@link OnDeleteFilter} to filter events sent to this KubernetesDependent + * + * @return the {@link OnDeleteFilter} filter implementation to use, defaulting to the interface + * itself if no value is set + */ Class onDeleteFilter() default OnDeleteFilter.class; + /** + * Optional {@link GenericFilter} to filter events sent to this KubernetesDependent + * + * @return the {@link GenericFilter} filter implementation to use, defaulting to the interface + * itself if no value is set + */ Class genericFilter() default GenericFilter.class; }