diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/dependent/NameSetter.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/dependent/NameSetter.java deleted file mode 100644 index 952bf14490..0000000000 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/dependent/NameSetter.java +++ /dev/null @@ -1,7 +0,0 @@ -package io.javaoperatorsdk.operator.api.reconciler.dependent; - -public interface NameSetter { - - void setName(String name); - -} 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 dbb97e1e6b..e53d584949 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 @@ -94,9 +94,9 @@ public Controller(Reconciler

reconciler, final var managed = configurationService.getWorkflowFactory().workflowFor(configuration); managedWorkflow = managed.resolve(kubernetesClient, configuration); - explicitWorkflowInvocation = - configuration.getWorkflowSpec().map(WorkflowSpec::isExplicitInvocation) - .orElse(false); + explicitWorkflowInvocation = configuration.getWorkflowSpec() + .map(WorkflowSpec::isExplicitInvocation) + .orElse(false); eventSourceManager = new EventSourceManager<>(this); eventProcessor = new EventProcessor<>(eventSourceManager, configurationService); diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/AbstractDependentResource.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/AbstractDependentResource.java index db69d8134b..f3d1050055 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/AbstractDependentResource.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/AbstractDependentResource.java @@ -11,7 +11,6 @@ import io.javaoperatorsdk.operator.api.reconciler.Ignore; import io.javaoperatorsdk.operator.api.reconciler.dependent.Deleter; import io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResource; -import io.javaoperatorsdk.operator.api.reconciler.dependent.NameSetter; import io.javaoperatorsdk.operator.api.reconciler.dependent.ReconcileResult; import io.javaoperatorsdk.operator.processing.dependent.Matcher.Result; import io.javaoperatorsdk.operator.processing.event.ResourceID; @@ -26,16 +25,16 @@ */ @Ignore public abstract class AbstractDependentResource - implements DependentResource, NameSetter { + implements DependentResource { private static final Logger log = LoggerFactory.getLogger(AbstractDependentResource.class); private final boolean creatable = this instanceof Creator; private final boolean updatable = this instanceof Updater; private final boolean deletable = this instanceof Deleter; + private final String name; private final DependentResourceReconciler dependentResourceReconciler; protected Creator creator; protected Updater updater; - protected String name; protected AbstractDependentResource() { this(null); @@ -227,10 +226,6 @@ public String name() { return name; } - public void setName(String name) { - this.name = name; - } - protected boolean creatable() { return creatable; } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/DefaultManagedWorkflow.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/DefaultManagedWorkflow.java index 64f936c70d..f2fd317def 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/DefaultManagedWorkflow.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/DefaultManagedWorkflow.java @@ -1,7 +1,6 @@ package io.javaoperatorsdk.operator.processing.dependent.workflow; import java.util.HashMap; -import java.util.HashSet; import java.util.List; import java.util.Set; import java.util.stream.Collectors; @@ -10,37 +9,25 @@ import io.fabric8.kubernetes.client.KubernetesClient; import io.javaoperatorsdk.operator.api.config.ControllerConfiguration; import io.javaoperatorsdk.operator.api.config.dependent.DependentResourceSpec; -import io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResource; -import io.javaoperatorsdk.operator.api.reconciler.dependent.EventSourceReferencer; -import io.javaoperatorsdk.operator.api.reconciler.dependent.NameSetter; - -import static io.javaoperatorsdk.operator.api.reconciler.Constants.NO_VALUE_SET; @SuppressWarnings("rawtypes") public class DefaultManagedWorkflow

implements ManagedWorkflow

{ + private final DefaultWorkflow

workflow; + private boolean resolved; + private final List orderedSpecs; // todo: remove - private final Set topLevelResources; - private final Set bottomLevelResources; - private final List orderedSpecs; - private final boolean hasCleaner; - + @SuppressWarnings("unchecked") protected DefaultManagedWorkflow(List orderedSpecs, boolean hasCleaner) { - this.hasCleaner = hasCleaner; - topLevelResources = new HashSet<>(orderedSpecs.size()); - bottomLevelResources = orderedSpecs.stream() - .map(DependentResourceSpec::getName) - .collect(Collectors.toSet()); - this.orderedSpecs = orderedSpecs; - for (DependentResourceSpec spec : orderedSpecs) { - // add cycle detection? - if (spec.getDependsOn().isEmpty()) { - topLevelResources.add(spec.getName()); - } else { - for (String dependsOn : spec.getDependsOn()) { - bottomLevelResources.remove(dependsOn); - } - } + final var alreadyResolved = new HashMap(orderedSpecs.size()); + for (DependentResourceSpec spec : orderedSpecs) { + final var node = new UnresolvedDependentResourceNode(spec); + alreadyResolved.put(spec.getName(), node); + spec.getDependsOn() + .forEach(depend -> node.addDependsOnRelation(alreadyResolved.get(depend))); } + + this.workflow = new DefaultWorkflow<>(alreadyResolved.values(), false, hasCleaner); + this.orderedSpecs = orderedSpecs; // todo: remove } @Override @@ -50,79 +37,41 @@ public List getOrderedSpecs() { } protected Set getTopLevelResources() { - return topLevelResources; + return workflow.getTopLevelDependentResources().stream().map(DependentResourceNode::name) + .collect(Collectors.toSet()); } protected Set getBottomLevelResources() { - return bottomLevelResources; + return workflow.getBottomLevelDependentResources().stream().map(DependentResourceNode::name) + .collect(Collectors.toSet()); } List nodeNames() { - return orderedSpecs.stream().map(DependentResourceSpec::getName).collect(Collectors.toList()); + return workflow.getDependentResourceNodes().keySet().stream().toList(); } @Override public boolean hasCleaner() { - return hasCleaner; + return workflow.hasCleaner(); } @Override public boolean isEmpty() { - return orderedSpecs.isEmpty(); + return workflow.isEmpty(); } @Override @SuppressWarnings("unchecked") public Workflow

resolve(KubernetesClient client, ControllerConfiguration

configuration) { - final var alreadyResolved = new HashMap(orderedSpecs.size()); - for (DependentResourceSpec spec : orderedSpecs) { - final var dependentResource = resolve(spec, client, configuration); - final var node = new DependentResourceNode( - spec.getReconcileCondition(), - spec.getDeletePostCondition(), - spec.getReadyCondition(), - spec.getActivationCondition(), - dependentResource); - alreadyResolved.put(dependentResource.name(), node); - spec.getDependsOn() - .forEach(depend -> node.addDependsOnRelation(alreadyResolved.get(depend))); + if (!resolved) { + workflow.getDependentResourceNodes().values() + .parallelStream() + .filter(UnresolvedDependentResourceNode.class::isInstance) + .map(UnresolvedDependentResourceNode.class::cast) + .forEach(node -> node.resolve(configuration)); + resolved = true; } - - final var bottom = - bottomLevelResources.stream().map(alreadyResolved::get).collect(Collectors.toSet()); - final var top = - topLevelResources.stream().map(alreadyResolved::get).collect(Collectors.toSet()); - return new DefaultWorkflow<>(alreadyResolved, bottom, top, - configuration.getWorkflowSpec().map(w -> !w.handleExceptionsInReconciler()).orElseThrow(), - hasCleaner); - } - - @SuppressWarnings({"rawtypes", "unchecked"}) - private DependentResource resolve(DependentResourceSpec spec, - KubernetesClient client, - ControllerConfiguration

configuration) { - final DependentResource dependentResource = - configuration.getConfigurationService().dependentResourceFactory() - .createFrom(spec, configuration); - - final var name = spec.getName(); - if (name != null && !NO_VALUE_SET.equals(name) && dependentResource instanceof NameSetter) { - ((NameSetter) dependentResource).setName(name); - } - - spec.getUseEventSourceWithName() - .ifPresent(esName -> { - if (dependentResource instanceof EventSourceReferencer) { - ((EventSourceReferencer) dependentResource).useEventSourceWithName(esName); - } else { - throw new IllegalStateException( - "DependentResource " + spec + " wants to use EventSource named " + esName - + " but doesn't implement support for this feature by implementing " - + EventSourceReferencer.class.getSimpleName()); - } - }); - - return dependentResource; + return workflow; } } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/DefaultWorkflow.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/DefaultWorkflow.java index 52efb312a9..447b86d342 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/DefaultWorkflow.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/DefaultWorkflow.java @@ -1,5 +1,6 @@ package io.javaoperatorsdk.operator.processing.dependent.workflow; +import java.util.Collection; import java.util.Collections; import java.util.HashMap; import java.util.HashSet; @@ -29,11 +30,11 @@ class DefaultWorkflow

implements Workflow

{ private final boolean throwExceptionAutomatically; private final boolean hasCleaner; - DefaultWorkflow(Set dependentResourceNodes) { + DefaultWorkflow(Collection dependentResourceNodes) { this(dependentResourceNodes, THROW_EXCEPTION_AUTOMATICALLY_DEFAULT, false); } - DefaultWorkflow(Set dependentResourceNodes, + DefaultWorkflow(Collection dependentResourceNodes, boolean throwExceptionAutomatically, boolean hasCleaner) { this.throwExceptionAutomatically = throwExceptionAutomatically; @@ -62,7 +63,8 @@ protected DefaultWorkflow(Map dependentResourceNo } @SuppressWarnings("unchecked") - private Map toMap(Set nodes) { + private Map toMap( + Collection nodes) { if (nodes == null || nodes.isEmpty()) { return Collections.emptyMap(); } @@ -77,7 +79,7 @@ private Map toMap(Set node bottomLevelResource.remove(dependsOn); } } - map.put(node.getDependentResource().name(), node); + map.put(node.name(), node); } if (topLevelResources.isEmpty()) { throw new IllegalStateException( @@ -157,4 +159,8 @@ public List getDependentResourcesWithoutActivationCondition() .map(DependentResourceNode::getDependentResource) .toList(); } + + Map getDependentResourceNodes() { + return dependentResourceNodes; + } } 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 aa8bb31c66..b0abf24873 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 @@ -10,27 +10,51 @@ @SuppressWarnings("rawtypes") class DependentResourceNode { - private final List dependsOn = new LinkedList<>(); - private final List parents = new LinkedList<>(); + private final List dependsOn; + private final List parents; private ConditionWithType reconcilePrecondition; private ConditionWithType deletePostcondition; private ConditionWithType readyPostcondition; private ConditionWithType activationCondition; - private final DependentResource dependentResource; + protected DependentResource dependentResource; + + protected DependentResourceNode(Condition reconcilePrecondition, + Condition deletePostcondition, Condition readyPostcondition, + Condition activationCondition) { + this(null, reconcilePrecondition, deletePostcondition, readyPostcondition, activationCondition); + } DependentResourceNode(DependentResource dependentResource) { - this(null, null, null, null, dependentResource); + this(dependentResource, null, null, null, null); } - public DependentResourceNode(Condition reconcilePrecondition, + private DependentResourceNode(DependentResource dependentResource, + Condition reconcilePrecondition, Condition deletePostcondition, Condition readyPostcondition, - Condition activationCondition, DependentResource dependentResource) { + Condition activationCondition) { setReconcilePrecondition(reconcilePrecondition); setDeletePostcondition(deletePostcondition); setReadyPostcondition(readyPostcondition); setActivationCondition(activationCondition); this.dependentResource = dependentResource; + dependsOn = new LinkedList<>(); + parents = new LinkedList<>(); + } + + protected DependentResourceNode(DependentResourceNode other) { + this.dependentResource = other.dependentResource; + this.parents = other.parents; + this.dependsOn = other.dependsOn; + this.reconcilePrecondition = other.reconcilePrecondition; + this.deletePostcondition = other.deletePostcondition; + this.readyPostcondition = other.readyPostcondition; + this.activationCondition = other.activationCondition; + } + + @SuppressWarnings("unchecked") + Class> getDependentResourceClass() { + return (Class>) dependentResource.getClass(); } public List getDependsOn() { @@ -54,33 +78,33 @@ public List getParents() { return Optional.ofNullable(reconcilePrecondition); } - public Optional> getDeletePostcondition() { - return Optional.ofNullable(deletePostcondition); - } - - public Optional> getActivationCondition() { - return Optional.ofNullable(activationCondition); - } - - public Optional> getReadyPostcondition() { - return Optional.ofNullable(readyPostcondition); - } - void setReconcilePrecondition(Condition reconcilePrecondition) { this.reconcilePrecondition = reconcilePrecondition == null ? null : new ConditionWithType<>(reconcilePrecondition, Condition.Type.RECONCILE); } + public Optional> getDeletePostcondition() { + return Optional.ofNullable(deletePostcondition); + } + void setDeletePostcondition(Condition deletePostcondition) { this.deletePostcondition = deletePostcondition == null ? null : new ConditionWithType<>(deletePostcondition, Condition.Type.DELETE); } + public Optional> getActivationCondition() { + return Optional.ofNullable(activationCondition); + } + void setActivationCondition(Condition activationCondition) { this.activationCondition = activationCondition == null ? null : new ConditionWithType<>(activationCondition, Condition.Type.ACTIVATION); } + public Optional> getReadyPostcondition() { + return Optional.ofNullable(readyPostcondition); + } + void setReadyPostcondition(Condition readyPostcondition) { this.readyPostcondition = readyPostcondition == null ? null : new ConditionWithType<>(readyPostcondition, Condition.Type.READY); @@ -90,6 +114,10 @@ public DependentResource getDependentResource() { return dependentResource; } + public String name() { + return getDependentResource().name(); + } + @Override public boolean equals(Object o) { if (this == o) { @@ -99,12 +127,12 @@ public boolean equals(Object o) { return false; } DependentResourceNode that = (DependentResourceNode) o; - return this.getDependentResource().name().equals(that.getDependentResource().name()); + return name().equals(that.name()); } @Override public int hashCode() { - return this.getDependentResource().name().hashCode(); + return name().hashCode(); } @Override diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/DependentResourceNodePrecursor.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/DependentResourceNodePrecursor.java new file mode 100644 index 0000000000..50a7381d3d --- /dev/null +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/DependentResourceNodePrecursor.java @@ -0,0 +1,29 @@ +package io.javaoperatorsdk.operator.processing.dependent.workflow; + +import java.util.HashSet; +import java.util.Set; + +import io.fabric8.kubernetes.api.model.HasMetadata; +import io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResource; + +class DependentResourceNodePrecursor extends WorkflowNodePrecursor { + private final Set dependsOn = new HashSet<>(); + + DependentResourceNodePrecursor(DependentResourceNode dependentResource) { + super(dependentResource); + } + + @Override + public Class> getDependentResourceClass() { + return super.getDependentResourceClass(); + } + + @Override + public Set dependsOnAsNames() { + return dependsOn; + } + + void addDependsOn(String dependsOn) { + this.dependsOn.add(dependsOn); + } +} diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/UnresolvedDependentResourceNode.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/UnresolvedDependentResourceNode.java new file mode 100644 index 0000000000..c65093a52a --- /dev/null +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/UnresolvedDependentResourceNode.java @@ -0,0 +1,59 @@ +package io.javaoperatorsdk.operator.processing.dependent.workflow; + + +import java.util.Set; + +import io.fabric8.kubernetes.api.model.HasMetadata; +import io.javaoperatorsdk.operator.api.config.ControllerConfiguration; +import io.javaoperatorsdk.operator.api.config.dependent.DependentResourceSpec; +import io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResource; +import io.javaoperatorsdk.operator.api.reconciler.dependent.EventSourceReferencer; + +@SuppressWarnings({"rawtypes", "unchecked"}) +class UnresolvedDependentResourceNode + extends WorkflowNodePrecursor { + private final DependentResourceSpec spec; + + UnresolvedDependentResourceNode(DependentResourceSpec spec) { + super(spec.getReconcileCondition(), spec.getDeletePostCondition(), spec.getReadyCondition(), + spec.getActivationCondition()); + this.spec = spec; + } + + void resolve(ControllerConfiguration

configuration) { + if (dependentResource == null) { + dependentResource = configuration.getConfigurationService().dependentResourceFactory() + .createFrom(spec, configuration); + + spec.getUseEventSourceWithName() + .ifPresent(esName -> { + if (dependentResource instanceof EventSourceReferencer esReferencer) { + esReferencer.useEventSourceWithName(esName); + } else { + throw new IllegalStateException( + "DependentResource " + spec + " wants to use EventSource named " + esName + + " but doesn't implement support for this feature by implementing " + + EventSourceReferencer.class.getSimpleName()); + } + }); + } + } + + @Override + public DependentResource getDependentResource() { + if (dependentResource == null) { + throw new IllegalStateException( + name() + " dependent resource node should be resolved first"); + } + return super.getDependentResource(); + } + + public String name() { + return dependentResource != null ? super.name() : spec.getName(); + } + + @Override + public Set dependsOnAsNames() { + return spec.getDependsOn(); + } +} diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowBuilder.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowBuilder.java index 2a534892a8..5e79611332 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowBuilder.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowBuilder.java @@ -1,12 +1,18 @@ package io.javaoperatorsdk.operator.processing.dependent.workflow; -import java.util.Arrays; +import java.util.Collection; import java.util.HashMap; import java.util.HashSet; +import java.util.LinkedHashMap; +import java.util.LinkedList; +import java.util.List; import java.util.Map; import java.util.Set; +import java.util.function.Function; +import java.util.stream.Collectors; import io.fabric8.kubernetes.api.model.HasMetadata; +import io.javaoperatorsdk.operator.OperatorException; import io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResource; import static io.javaoperatorsdk.operator.processing.dependent.workflow.Workflow.THROW_EXCEPTION_AUTOMATICALLY_DEFAULT; @@ -14,10 +20,9 @@ @SuppressWarnings({"rawtypes", "unchecked"}) public class WorkflowBuilder

{ - private final Map> dependentResourceNodes = + private final Map dependentResourceNodes = new HashMap<>(); private boolean throwExceptionAutomatically = THROW_EXCEPTION_AUTOMATICALLY_DEFAULT; - private boolean isCleaner = false; public WorkflowNodeConfigurationBuilder addDependentResourceAndConfigure( DependentResource dependentResource) { @@ -30,27 +35,15 @@ public WorkflowBuilder

addDependentResource(DependentResource dependentResour return this; } - private DependentResourceNode doAddDependentResource(DependentResource dependentResource) { - final var currentNode = new DependentResourceNode<>(dependentResource); - isCleaner = isCleaner || dependentResource.isDeletable(); + private DependentResourceNodePrecursor doAddDependentResource( + DependentResource dependentResource) { + final var currentNode = + new DependentResourceNodePrecursor(new DependentResourceNode(dependentResource)); final var actualName = dependentResource.name(); dependentResourceNodes.put(actualName, currentNode); return currentNode; } - DependentResourceNode getNodeByDependentResource(DependentResource dependentResource) { - // first check by name - final var node = dependentResourceNodes.get(dependentResource.name()); - if (node != null) { - return node; - } else { - return dependentResourceNodes.values().stream() - .filter(dr -> dr.getDependentResource() == dependentResource) - .findFirst() - .orElseThrow(); - } - } - public WorkflowBuilder

withThrowExceptionFurther(boolean throwExceptionFurther) { this.throwExceptionAutomatically = throwExceptionFurther; return this; @@ -61,14 +54,118 @@ public Workflow

build() { } DefaultWorkflow

buildAsDefaultWorkflow() { - return new DefaultWorkflow(new HashSet<>(dependentResourceNodes.values()), - throwExceptionAutomatically, isCleaner); + final boolean[] cleanerHolder = {false}; + final List> nodes = + orderAndDetectCycles(dependentResourceNodes.values(), cleanerHolder); + return new DefaultWorkflow(nodes, throwExceptionAutomatically, cleanerHolder[0]); + } + + private static class DRInfo { + + private final WorkflowNodePrecursor spec; + private final List waitingForCompletion; + + private DRInfo(WorkflowNodePrecursor spec) { + this.spec = spec; + this.waitingForCompletion = new LinkedList<>(); + } + + void add(WorkflowNodePrecursor spec) { + waitingForCompletion.add(spec); + } + + String name() { + return spec.name(); + } + } + + private boolean isReadyForVisit(WorkflowNodePrecursor dr, Set alreadyVisited, + String alreadyPresentName) { + for (var name : dr.dependsOnAsNames()) { + if (name.equals(alreadyPresentName)) { + continue; + } + if (!alreadyVisited.contains(name)) { + return false; + } + } + return true; + } + + private Set getTopDependentResources( + Collection dependentResourceSpecs) { + return dependentResourceSpecs.stream() + .filter(r -> r.dependsOnAsNames().isEmpty()) + .collect(Collectors.toSet()); + } + + private Map createDRInfos( + Collection dependentResourceSpecs) { + // first create mappings + final var infos = dependentResourceSpecs.stream() + .map(DRInfo::new) + .collect(Collectors.toMap(DRInfo::name, Function.identity())); + + // then populate the reverse depends on information + dependentResourceSpecs.forEach(spec -> spec.dependsOnAsNames().forEach(name -> { + final var drInfo = infos.get(name); + drInfo.add(spec); + })); + + return infos; + } + + private List> orderAndDetectCycles( + Collection dependentResourceSpecs, boolean[] cleanerHolder) { + + final var drInfosByName = createDRInfos(dependentResourceSpecs); + final var orderedSpecs = + new LinkedHashMap>(dependentResourceSpecs.size()); + final var alreadyVisited = new HashSet(); + var toVisit = getTopDependentResources(dependentResourceSpecs); + + while (!toVisit.isEmpty()) { + final var toVisitNext = new HashSet(); + toVisit.forEach(dr -> { + if (cleanerHolder != null) { + cleanerHolder[0] = + cleanerHolder[0] || DefaultWorkflow.isDeletable(dr.getDependentResourceClass()); + } + final var name = dr.name(); + var drInfo = drInfosByName.get(name); + if (drInfo != null) { + drInfo.waitingForCompletion.forEach(spec -> { + if (isReadyForVisit(spec, alreadyVisited, name)) { + toVisitNext.add(spec); + } + }); + final var node = convertToNodeAndResolveRelations(dr, orderedSpecs); + orderedSpecs.put(name, node); + } + alreadyVisited.add(name); + }); + + toVisit = toVisitNext; + } + + if (orderedSpecs.size() != dependentResourceSpecs.size()) { + // could provide improved message where the exact cycles are made visible + throw new OperatorException("Cycle(s) between dependent resources."); + } + return orderedSpecs.values().stream().toList(); + } + + private DependentResourceNode convertToNodeAndResolveRelations( + WorkflowNodePrecursor node, LinkedHashMap> known) { + final var drn = new DependentResourceNode(node); + node.dependsOnAsNames().forEach(name -> drn.addDependsOnRelation(known.get(name))); + return drn; } public class WorkflowNodeConfigurationBuilder { - private final DependentResourceNode currentNode; + private final DependentResourceNodePrecursor currentNode; - private WorkflowNodeConfigurationBuilder(DependentResourceNode currentNode) { + private WorkflowNodeConfigurationBuilder(DependentResourceNodePrecursor currentNode) { this.currentNode = currentNode; } @@ -94,17 +191,11 @@ public WorkflowBuilder

withThrowExceptionFurther(boolean throwExceptionFurthe return WorkflowBuilder.this.withThrowExceptionFurther(throwExceptionFurther); } - public WorkflowNodeConfigurationBuilder toDependOn(Set dependentResources) { - for (var dependentResource : dependentResources) { - var dependsOn = getNodeByDependentResource(dependentResource); - currentNode.addDependsOnRelation(dependsOn); - } - return this; - } - public WorkflowNodeConfigurationBuilder toDependOn(DependentResource... dependentResources) { if (dependentResources != null) { - return toDependOn(new HashSet<>(Arrays.asList(dependentResources))); + for (var dependentResource : dependentResources) { + currentNode.addDependsOn(dependentResource.name()); + } } return this; } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowNodePrecursor.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowNodePrecursor.java new file mode 100644 index 0000000000..74a6774687 --- /dev/null +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowNodePrecursor.java @@ -0,0 +1,19 @@ +package io.javaoperatorsdk.operator.processing.dependent.workflow; + +import java.util.Set; + +import io.fabric8.kubernetes.api.model.HasMetadata; + +abstract class WorkflowNodePrecursor extends DependentResourceNode { + protected WorkflowNodePrecursor(DependentResourceNode other) { + super(other); + } + + protected WorkflowNodePrecursor(Condition reconcilePrecondition, + Condition deletePostcondition, Condition readyPostcondition, + Condition activationCondition) { + super(reconcilePrecondition, deletePostcondition, readyPostcondition, activationCondition); + } + + abstract Set dependsOnAsNames(); +} 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 970e40eff6..7625de7231 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 @@ -53,11 +53,12 @@ public ReconcileResult reconcile(TestCustomResource primary, .resourceCreated(new ConfigMapBuilder().addToBinaryData("key", VALUE).build()); } + @SuppressWarnings("unchecked") @Override public synchronized Optional> eventSource( EventSourceContext context) { var mockIES = mock(InformerEventSource.class); - when(mockIES.name()).thenReturn(name); + when(mockIES.name()).thenReturn(name()); return Optional.of(mockIES); } diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowBuilderTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowBuilderTest.java index b41ee430f7..028cdf8f71 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowBuilderTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowBuilderTest.java @@ -2,7 +2,10 @@ import org.junit.jupiter.api.Test; +import io.fabric8.kubernetes.api.model.ConfigMap; +import io.javaoperatorsdk.operator.api.reconciler.dependent.Deleter; import io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResource; +import io.javaoperatorsdk.operator.processing.dependent.kubernetes.KubernetesDependentResource; import io.javaoperatorsdk.operator.sample.simple.TestCustomResource; import static org.assertj.core.api.Assertions.assertThat; @@ -14,16 +17,21 @@ class WorkflowBuilderTest { void workflowIsCleanerIfAtLeastOneDRIsCleaner() { var dr = mock(DependentResource.class); when(dr.name()).thenReturn("dr"); - var deleter = mock(DependentResource.class); - when(deleter.isDeletable()).thenReturn(true); - when(deleter.name()).thenReturn("deleter"); var workflow = new WorkflowBuilder() - .addDependentResource(deleter) + .addDependentResource(new DeleterDependentResource()) .addDependentResource(dr) .build(); assertThat(workflow.hasCleaner()).isTrue(); } + static class DeleterDependentResource + extends KubernetesDependentResource + implements Deleter { + public DeleterDependentResource() { + super(ConfigMap.class); + } + } + } diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowTest.java index 415218b587..99ad2fe390 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowTest.java @@ -5,6 +5,7 @@ import org.junit.jupiter.api.Test; +import io.javaoperatorsdk.operator.OperatorException; import io.javaoperatorsdk.operator.api.reconciler.dependent.Deleter; import io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResource; import io.javaoperatorsdk.operator.api.reconciler.dependent.GarbageCollected; @@ -27,12 +28,12 @@ void zeroTopLevelDRShouldThrowException() { var dr3 = mockDependent("dr3"); var cyclicWorkflowBuilderSetup = new WorkflowBuilder() - .addDependentResourceAndConfigure(dr1).toDependOn() + .addDependentResource(dr1) .addDependentResourceAndConfigure(dr2).toDependOn(dr1) .addDependentResourceAndConfigure(dr3).toDependOn(dr2) .addDependentResourceAndConfigure(dr1).toDependOn(dr2); - assertThrows(IllegalStateException.class, + assertThrows(OperatorException.class, cyclicWorkflowBuilderSetup::build); }