From 6cbc509cf797973b82de7261517d0af1c23a3be2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Attila=20M=C3=A9sz=C3=A1ros?= Date: Mon, 26 Feb 2024 15:31:54 +0100 Subject: [PATCH 1/3] feat: move name is directly to dependent resource MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - use this name when throwing aggregate exception Signed-off-by: Attila Mészáros --- .../dependent/DependentResource.java | 4 +++ .../dependent/AbstractDependentResource.java | 11 ++++++++ .../workflow/AbstractWorkflowExecutor.java | 6 ++-- .../workflow/DefaultManagedWorkflow.java | 9 ++++-- .../dependent/workflow/DefaultWorkflow.java | 2 +- .../workflow/DependentResourceNode.java | 20 ++++--------- .../dependent/workflow/WorkflowBuilder.java | 11 ++------ .../dependent/workflow/WorkflowResult.java | 2 +- .../ControllerConfigurationOverriderTest.java | 8 ++++++ ...dentResourceConfigurationResolverTest.java | 8 ++++++ .../dependent/EmptyTestDependentResource.java | 12 ++++++++ .../AbstractWorkflowExecutorTest.java | 17 ++++++++--- .../workflow/WorkflowBuilderTest.java | 2 ++ .../WorkflowReconcileExecutorTest.java | 2 +- .../dependent/workflow/WorkflowTest.java | 28 +++++++++++-------- .../config/BaseConfigurationServiceTest.java | 8 ++++++ 16 files changed, 105 insertions(+), 45 deletions(-) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/dependent/DependentResource.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/dependent/DependentResource.java index 8230dc4cf9..083e1d5954 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/dependent/DependentResource.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/dependent/DependentResource.java @@ -68,4 +68,8 @@ static String defaultNameFor(Class dependentResourc default boolean isDeletable() { return this instanceof Deleter; } + + String getName(); + + void setName(String name); } 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 ea1e020bfb..321b3fd7dc 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 @@ -29,6 +29,8 @@ public abstract class AbstractDependentResource private ResourceDiscriminator resourceDiscriminator; private final DependentResourceReconciler dependentResourceReconciler; + protected String name = DependentResource.defaultNameFor(this.getClass()); + @SuppressWarnings({"unchecked"}) protected AbstractDependentResource() { creator = creatable ? (Creator) this : null; @@ -176,4 +178,13 @@ protected boolean isUpdatable() { public boolean isDeletable() { return deletable; } + + @Override + public String getName() { + return name; + } + + public void setName(String name) { + this.name = name; + } } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/AbstractWorkflowExecutor.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/AbstractWorkflowExecutor.java index 56e5e17d07..ba59a3b066 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/AbstractWorkflowExecutor.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/AbstractWorkflowExecutor.java @@ -139,11 +139,13 @@ protected void registerOrDeregisterEventSourceBasedOnActivation( .eventSourceContextForDynamicRegistration()); var es = eventSource.orElseThrow(); context.eventSourceRetriever() - .dynamicallyRegisterEventSource(dependentResourceNode.getName(), es); + .dynamicallyRegisterEventSource(dependentResourceNode.getDependentResource().getName(), + es); } else { context.eventSourceRetriever() - .dynamicallyDeRegisterEventSource(dependentResourceNode.getName()); + .dynamicallyDeRegisterEventSource( + dependentResourceNode.getDependentResource().getName()); } } } 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 27400230df..5e69f7fe12 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 @@ -14,6 +14,7 @@ import io.javaoperatorsdk.operator.api.reconciler.dependent.EventSourceReferencer; import io.javaoperatorsdk.operator.api.reconciler.dependent.managed.KubernetesClientAware; +import static io.javaoperatorsdk.operator.api.reconciler.Constants.NO_VALUE_SET; import static io.javaoperatorsdk.operator.processing.dependent.workflow.Workflow.THROW_EXCEPTION_AUTOMATICALLY_DEFAULT; @SuppressWarnings("rawtypes") @@ -77,13 +78,13 @@ public Workflow

resolve(KubernetesClient client, ControllerConfiguration

configuration) { final var alreadyResolved = new HashMap(orderedSpecs.size()); for (DependentResourceSpec spec : orderedSpecs) { - final var node = new DependentResourceNode(spec.getName(), + final var node = new DependentResourceNode( spec.getReconcileCondition(), spec.getDeletePostCondition(), spec.getReadyCondition(), spec.getActivationCondition(), resolve(spec, client, configuration)); - alreadyResolved.put(node.getName(), node); + alreadyResolved.put(node.getDependentResource().getName(), node); spec.getDependsOn() .forEach(depend -> node.addDependsOnRelation(alreadyResolved.get(depend))); } @@ -104,6 +105,10 @@ private DependentResource resolve(DependentResourceSpec spec, configuration.getConfigurationService().dependentResourceFactory() .createFrom(spec, configuration); + if (spec.getName() != null && !spec.getName().equals(NO_VALUE_SET)) { + dependentResource.setName(spec.getName()); + } + if (dependentResource instanceof KubernetesClientAware) { ((KubernetesClientAware) dependentResource).setKubernetesClient(client); } 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 d31f42419d..d1daa13db2 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 @@ -77,7 +77,7 @@ private Map toMap(Set node bottomLevelResource.remove(dependsOn); } } - map.put(node.getName(), node); + map.put(node.getDependentResource().getName(), node); } if (topLevelResources.size() == 0) { throw new IllegalStateException( 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 4c82ee19f3..35b1559948 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 @@ -12,25 +12,20 @@ public class DependentResourceNode { private final List dependsOn = new LinkedList<>(); private final List parents = new LinkedList<>(); - private final String name; + private Condition reconcilePrecondition; private Condition deletePostcondition; private Condition readyPostcondition; private Condition activationCondition; private final DependentResource dependentResource; - DependentResourceNode(String name, DependentResource dependentResource) { - this(name, null, null, null, null, dependentResource); - } - DependentResourceNode(DependentResource dependentResource) { - this(getNameFor(dependentResource), null, null, null, null, dependentResource); + this(null, null, null, null, dependentResource); } - public DependentResourceNode(String name, Condition reconcilePrecondition, + public DependentResourceNode(Condition reconcilePrecondition, Condition deletePostcondition, Condition readyPostcondition, Condition activationCondition, DependentResource dependentResource) { - this.name = name; this.reconcilePrecondition = reconcilePrecondition; this.deletePostcondition = deletePostcondition; this.readyPostcondition = readyPostcondition; @@ -55,11 +50,6 @@ public List getParents() { return parents; } - public String getName() { - return name; - } - - public Optional> getReconcilePrecondition() { return Optional.ofNullable(reconcilePrecondition); } @@ -106,12 +96,12 @@ public boolean equals(Object o) { return false; } DependentResourceNode that = (DependentResourceNode) o; - return name.equals(that.name); + return this.getDependentResource().getName().equals(that.getDependentResource().getName()); } @Override public int hashCode() { - return name.hashCode(); + return this.getDependentResource().getName().hashCode(); } @SuppressWarnings("rawtypes") 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 d65e21659c..cc0655e28b 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 @@ -21,14 +21,9 @@ public class WorkflowBuilder

{ private boolean isCleaner = false; public WorkflowBuilder

addDependentResource(DependentResource dependentResource) { - return addDependentResource(dependentResource, null); - } - - public WorkflowBuilder

addDependentResource(DependentResource dependentResource, String name) { - currentNode = name == null ? new DependentResourceNode<>(dependentResource) - : new DependentResourceNode<>(name, dependentResource); + currentNode = new DependentResourceNode<>(dependentResource); isCleaner = isCleaner || dependentResource.isDeletable(); - final var actualName = currentNode.getName(); + final var actualName = dependentResource.getName(); dependentResourceNodes.put(actualName, currentNode); return this; } @@ -71,7 +66,7 @@ public WorkflowBuilder

withActivationCondition(Condition activationCondition) DependentResourceNode getNodeByDependentResource(DependentResource dependentResource) { // first check by name final var node = - dependentResourceNodes.get(DependentResourceNode.getNameFor(dependentResource)); + dependentResourceNodes.get(dependentResource.getName()); if (node != null) { return node; } else { diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowResult.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowResult.java index ba9e0f5755..901f63dd78 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowResult.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowResult.java @@ -38,7 +38,7 @@ public void throwAggregateExceptionIfErrorsPresent() { if (erroredDependentsExist()) { throw new AggregatedOperatorException("Exception(s) during workflow execution.", erroredDependents.entrySet().stream() - .collect(Collectors.toMap(e -> e.getKey().getClass().getName(), Entry::getValue))); + .collect(Collectors.toMap(e -> e.getKey().getName(), Entry::getValue))); } } } 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 1c86886d89..281bd156cf 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 @@ -103,6 +103,14 @@ public Class resourceType() { return Object.class; } + @Override + public String getName() { + return null; + } + + @Override + public void setName(String name) {} + @Override public void configureWith(String config) { this.config = config; diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/config/dependent/DependentResourceConfigurationResolverTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/config/dependent/DependentResourceConfigurationResolverTest.java index 3187b32645..97b9b0a9c4 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/config/dependent/DependentResourceConfigurationResolverTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/config/dependent/DependentResourceConfigurationResolverTest.java @@ -175,6 +175,14 @@ public Class resourceType() { return ConfigMap.class; } + @Override + public String getName() { + return null; + } + + @Override + public void setName(String name) {} + @Override public void configureWith(CustomConfig config) { this.config = config; diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/EmptyTestDependentResource.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/EmptyTestDependentResource.java index 4fe88296ac..cd2e4defa6 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/EmptyTestDependentResource.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/EmptyTestDependentResource.java @@ -9,6 +9,8 @@ public class EmptyTestDependentResource implements DependentResource { + private String name; + @Override public ReconcileResult reconcile(TestCustomResource primary, Context context) { @@ -19,5 +21,15 @@ public ReconcileResult reconcile(TestCustomResource primary, public Class resourceType() { return Deployment.class; } + + @Override + public String getName() { + return name; + } + + @Override + public void setName(String name) { + this.name = name; + } } 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 941c3fa434..e4e6556e84 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 @@ -35,11 +35,10 @@ public class AbstractWorkflowExecutorTest { public class TestDependent extends KubernetesDependentResource { - private final String name; - public TestDependent(String name) { super(ConfigMap.class); - this.name = name; + setName(name); + } @Override @@ -92,7 +91,7 @@ public void delete(TestCustomResource primary, Context conte } public class TestErrorDependent implements DependentResource { - private final String name; + private String name; public TestErrorDependent(String name) { this.name = name; @@ -110,6 +109,16 @@ public Class resourceType() { return String.class; } + @Override + public String getName() { + return name; + } + + @Override + public void setName(String name) { + this.name = name; + } + @Override public String toString() { return name; 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 9a61931a61..54c7139801 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 @@ -13,8 +13,10 @@ class WorkflowBuilderTest { @Test void workflowIsCleanerIfAtLeastOneDRIsCleaner() { var dr = mock(DependentResource.class); + when(dr.getName()).thenReturn("dr"); var deleter = mock(DependentResource.class); when(deleter.isDeletable()).thenReturn(true); + when(deleter.getName()).thenReturn("deleter"); var workflow = new WorkflowBuilder() .addDependentResource(deleter) 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 099d5fad2b..c66fcb02d6 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 @@ -538,7 +538,7 @@ void reconcilePreconditionNotCheckedOnNonActiveDependent() { @Test void deletesDependentsOfNonActiveDependentButNotTheNonActive() { TestDeleterDependent drDeleter2 = new TestDeleterDependent("DR_DELETER_2"); - TestDeleterDependent drDeleter3 = new TestDeleterDependent("DR_DELETER_2"); + TestDeleterDependent drDeleter3 = new TestDeleterDependent("DR_DELETER_3"); var workflow = new WorkflowBuilder() .addDependentResource(dr1).withActivationCondition(notMetCondition) 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 bc6ab8515f..2d472e3682 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 @@ -17,8 +17,7 @@ import static org.junit.Assert.assertThrows; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertTrue; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.withSettings; +import static org.mockito.Mockito.*; @SuppressWarnings("rawtypes") class WorkflowTest { @@ -27,9 +26,9 @@ class WorkflowTest { @Test void zeroTopLevelDRShouldThrowException() { - var dr1 = mock(DependentResource.class); - var dr2 = mock(DependentResource.class); - var dr3 = mock(DependentResource.class); + var dr1 = mockDependent("dr1"); + var dr2 = mockDependent("dr2"); + var dr3 = mockDependent("dr3"); var cyclicWorkflowBuilderSetup = new WorkflowBuilder() .addDependentResource(dr1).dependsOn() @@ -43,9 +42,9 @@ void zeroTopLevelDRShouldThrowException() { @Test void calculatesTopLevelResources() { - var dr1 = mock(DependentResource.class); - var dr2 = mock(DependentResource.class); - var independentDR = mock(DependentResource.class); + var dr1 = mockDependent("dr1"); + var dr2 = mockDependent("dr2"); + var independentDR = mockDependent("independentDR"); var workflow = new WorkflowBuilder() .addDependentResource(independentDR) @@ -63,9 +62,9 @@ void calculatesTopLevelResources() { @Test void calculatesBottomLevelResources() { - var dr1 = mock(DependentResource.class); - var dr2 = mock(DependentResource.class); - var independentDR = mock(DependentResource.class); + var dr1 = mockDependent("dr1"); + var dr2 = mockDependent("dr2"); + var independentDR = mockDependent("independentDR"); Workflow workflow = new WorkflowBuilder() .addDependentResource(independentDR) @@ -100,4 +99,11 @@ void isDeletableShouldWork() { GarbageCollected.class)); assertFalse(DefaultWorkflow.isDeletable(dr.getClass())); } + + static DependentResource mockDependent(String name) { + var res = mock(DependentResource.class); + when(res.getName()).thenReturn(name); + return res; + } + } diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/config/BaseConfigurationServiceTest.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/config/BaseConfigurationServiceTest.java index e107623347..72cce54ddf 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/config/BaseConfigurationServiceTest.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/config/BaseConfigurationServiceTest.java @@ -470,6 +470,14 @@ public Class resourceType() { return ConfigMap.class; } + @Override + public String getName() { + return null; + } + + @Override + public void setName(String name) {} + @Override public void configureWith(CustomConfig config) { this.config = config; From 4b0503b90998385d02cec8300d06a549438f2802 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Attila=20M=C3=A9sz=C3=A1ros?= Date: Tue, 27 Feb 2024 12:10:57 +0100 Subject: [PATCH 2/3] refactor to use a dedicated interface for setting the name MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Attila Mészáros --- .../api/reconciler/dependent/DependentResource.java | 3 +-- .../operator/api/reconciler/dependent/NameSetter.java | 7 +++++++ .../processing/dependent/AbstractDependentResource.java | 5 +++-- .../dependent/workflow/AbstractWorkflowExecutor.java | 4 ++-- .../dependent/workflow/DefaultManagedWorkflow.java | 8 +++++--- .../processing/dependent/workflow/DefaultWorkflow.java | 2 +- .../dependent/workflow/DependentResourceNode.java | 4 ++-- .../processing/dependent/workflow/WorkflowBuilder.java | 4 ++-- .../processing/dependent/workflow/WorkflowResult.java | 2 +- .../api/config/ControllerConfigurationOverriderTest.java | 8 ++------ .../DependentResourceConfigurationResolverTest.java | 5 +---- .../processing/dependent/EmptyTestDependentResource.java | 3 +-- .../dependent/workflow/AbstractWorkflowExecutorTest.java | 7 +------ .../dependent/workflow/WorkflowBuilderTest.java | 4 ++-- .../processing/dependent/workflow/WorkflowTest.java | 2 +- .../operator/config/BaseConfigurationServiceTest.java | 5 +---- 16 files changed, 33 insertions(+), 40 deletions(-) create mode 100644 operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/dependent/NameSetter.java diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/dependent/DependentResource.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/dependent/DependentResource.java index 083e1d5954..eb51ac3688 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/dependent/DependentResource.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/dependent/DependentResource.java @@ -69,7 +69,6 @@ default boolean isDeletable() { return this instanceof Deleter; } - String getName(); + String name(); - void setName(String name); } 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 new file mode 100644 index 0000000000..952bf14490 --- /dev/null +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/dependent/NameSetter.java @@ -0,0 +1,7 @@ +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/dependent/AbstractDependentResource.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/AbstractDependentResource.java index 321b3fd7dc..21dc66bd98 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,13 +11,14 @@ import io.javaoperatorsdk.operator.api.reconciler.ResourceDiscriminator; 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; @Ignore public abstract class AbstractDependentResource - implements DependentResource { + implements DependentResource, NameSetter { private static final Logger log = LoggerFactory.getLogger(AbstractDependentResource.class); private final boolean creatable = this instanceof Creator; @@ -180,7 +181,7 @@ public boolean isDeletable() { } @Override - public String getName() { + public String name() { return name; } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/AbstractWorkflowExecutor.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/AbstractWorkflowExecutor.java index ba59a3b066..cde85a7af4 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/AbstractWorkflowExecutor.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/AbstractWorkflowExecutor.java @@ -139,13 +139,13 @@ protected void registerOrDeregisterEventSourceBasedOnActivation( .eventSourceContextForDynamicRegistration()); var es = eventSource.orElseThrow(); context.eventSourceRetriever() - .dynamicallyRegisterEventSource(dependentResourceNode.getDependentResource().getName(), + .dynamicallyRegisterEventSource(dependentResourceNode.getDependentResource().name(), es); } else { context.eventSourceRetriever() .dynamicallyDeRegisterEventSource( - dependentResourceNode.getDependentResource().getName()); + dependentResourceNode.getDependentResource().name()); } } } 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 5e69f7fe12..f125fcbe84 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 @@ -12,6 +12,7 @@ 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 io.javaoperatorsdk.operator.api.reconciler.dependent.managed.KubernetesClientAware; import static io.javaoperatorsdk.operator.api.reconciler.Constants.NO_VALUE_SET; @@ -84,7 +85,7 @@ public Workflow

resolve(KubernetesClient client, spec.getReadyCondition(), spec.getActivationCondition(), resolve(spec, client, configuration)); - alreadyResolved.put(node.getDependentResource().getName(), node); + alreadyResolved.put(node.getDependentResource().name(), node); spec.getDependsOn() .forEach(depend -> node.addDependsOnRelation(alreadyResolved.get(depend))); } @@ -105,8 +106,9 @@ private DependentResource resolve(DependentResourceSpec spec, configuration.getConfigurationService().dependentResourceFactory() .createFrom(spec, configuration); - if (spec.getName() != null && !spec.getName().equals(NO_VALUE_SET)) { - dependentResource.setName(spec.getName()); + if (spec.getName() != null && !spec.getName().equals(NO_VALUE_SET) + && dependentResource instanceof NameSetter) { + ((NameSetter) dependentResource).setName(spec.getName()); } if (dependentResource instanceof KubernetesClientAware) { 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 d1daa13db2..9b899c21f7 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 @@ -77,7 +77,7 @@ private Map toMap(Set node bottomLevelResource.remove(dependsOn); } } - map.put(node.getDependentResource().getName(), node); + map.put(node.getDependentResource().name(), node); } if (topLevelResources.size() == 0) { throw new IllegalStateException( 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 35b1559948..0a524d573f 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 @@ -96,12 +96,12 @@ public boolean equals(Object o) { return false; } DependentResourceNode that = (DependentResourceNode) o; - return this.getDependentResource().getName().equals(that.getDependentResource().getName()); + return this.getDependentResource().name().equals(that.getDependentResource().name()); } @Override public int hashCode() { - return this.getDependentResource().getName().hashCode(); + return this.getDependentResource().name().hashCode(); } @SuppressWarnings("rawtypes") 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 cc0655e28b..83e85539c3 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 @@ -23,7 +23,7 @@ public class WorkflowBuilder

{ public WorkflowBuilder

addDependentResource(DependentResource dependentResource) { currentNode = new DependentResourceNode<>(dependentResource); isCleaner = isCleaner || dependentResource.isDeletable(); - final var actualName = dependentResource.getName(); + final var actualName = dependentResource.name(); dependentResourceNodes.put(actualName, currentNode); return this; } @@ -66,7 +66,7 @@ public WorkflowBuilder

withActivationCondition(Condition activationCondition) DependentResourceNode getNodeByDependentResource(DependentResource dependentResource) { // first check by name final var node = - dependentResourceNodes.get(dependentResource.getName()); + dependentResourceNodes.get(dependentResource.name()); if (node != null) { return node; } else { diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowResult.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowResult.java index 901f63dd78..e282c70527 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowResult.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowResult.java @@ -38,7 +38,7 @@ public void throwAggregateExceptionIfErrorsPresent() { if (erroredDependentsExist()) { throw new AggregatedOperatorException("Exception(s) during workflow execution.", erroredDependents.entrySet().stream() - .collect(Collectors.toMap(e -> e.getKey().getName(), Entry::getValue))); + .collect(Collectors.toMap(e -> e.getKey().name(), Entry::getValue))); } } } 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 281bd156cf..2782c860dd 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 @@ -75,8 +75,7 @@ void overridingNSShouldPreserveUntouchedDependents() { private static class NamedDependentReconciler implements Reconciler { @Override - public UpdateControl reconcile(ConfigMap resource, Context context) - throws Exception { + public UpdateControl reconcile(ConfigMap resource, Context context) { return null; } @@ -104,13 +103,10 @@ public Class resourceType() { } @Override - public String getName() { + public String name() { return null; } - @Override - public void setName(String name) {} - @Override public void configureWith(String config) { this.config = config; diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/config/dependent/DependentResourceConfigurationResolverTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/config/dependent/DependentResourceConfigurationResolverTest.java index 97b9b0a9c4..4a91909771 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/config/dependent/DependentResourceConfigurationResolverTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/config/dependent/DependentResourceConfigurationResolverTest.java @@ -176,13 +176,10 @@ public Class resourceType() { } @Override - public String getName() { + public String name() { return null; } - @Override - public void setName(String name) {} - @Override public void configureWith(CustomConfig config) { this.config = config; diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/EmptyTestDependentResource.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/EmptyTestDependentResource.java index cd2e4defa6..b41fc0bac7 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/EmptyTestDependentResource.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/EmptyTestDependentResource.java @@ -23,11 +23,10 @@ public Class resourceType() { } @Override - public String getName() { + public String name() { return name; } - @Override public void setName(String name) { this.name = name; } 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 e4e6556e84..59e6fef55a 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 @@ -110,15 +110,10 @@ public Class resourceType() { } @Override - public String getName() { + public String name() { return name; } - @Override - public void setName(String name) { - this.name = name; - } - @Override public String toString() { return name; 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 54c7139801..b41ee430f7 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 @@ -13,10 +13,10 @@ class WorkflowBuilderTest { @Test void workflowIsCleanerIfAtLeastOneDRIsCleaner() { var dr = mock(DependentResource.class); - when(dr.getName()).thenReturn("dr"); + when(dr.name()).thenReturn("dr"); var deleter = mock(DependentResource.class); when(deleter.isDeletable()).thenReturn(true); - when(deleter.getName()).thenReturn("deleter"); + when(deleter.name()).thenReturn("deleter"); var workflow = new WorkflowBuilder() .addDependentResource(deleter) 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 2d472e3682..ef48fccd6e 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 @@ -102,7 +102,7 @@ void isDeletableShouldWork() { static DependentResource mockDependent(String name) { var res = mock(DependentResource.class); - when(res.getName()).thenReturn(name); + when(res.name()).thenReturn(name); return res; } diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/config/BaseConfigurationServiceTest.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/config/BaseConfigurationServiceTest.java index 72cce54ddf..fb94047aa0 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/config/BaseConfigurationServiceTest.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/config/BaseConfigurationServiceTest.java @@ -471,13 +471,10 @@ public Class resourceType() { } @Override - public String getName() { + public String name() { return null; } - @Override - public void setName(String name) {} - @Override public void configureWith(CustomConfig config) { this.config = config; From a043c49db5844e8240514e41c81f41fca10be812 Mon Sep 17 00:00:00 2001 From: Chris Laprun Date: Mon, 4 Mar 2024 10:12:51 +0100 Subject: [PATCH 3/3] refactor: add default implementation for name() (#2255) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Chris Laprun Signed-off-by: Attila Mészáros --- .../reconciler/dependent/DependentResource.java | 4 +++- .../dependent/AbstractDependentResource.java | 7 ++++++- ...bstractEventSourceHolderDependentResource.java | 5 +++++ .../kubernetes/KubernetesDependentResource.java | 7 ++++++- .../workflow/AbstractWorkflowExecutor.java | 15 ++++++--------- .../workflow/DefaultManagedWorkflow.java | 11 ++++++----- .../dependent/workflow/DefaultWorkflow.java | 4 ++-- .../dependent/workflow/DependentResourceNode.java | 9 +-------- .../dependent/workflow/WorkflowBuilder.java | 3 +-- .../ControllerConfigurationOverriderTest.java | 11 +---------- ...ependentResourceConfigurationResolverTest.java | 10 +--------- .../workflow/AbstractWorkflowExecutorTest.java | 8 +++----- .../config/BaseConfigurationServiceTest.java | 13 +------------ 13 files changed, 42 insertions(+), 65 deletions(-) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/dependent/DependentResource.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/dependent/DependentResource.java index eb51ac3688..98d700324d 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/dependent/DependentResource.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/dependent/DependentResource.java @@ -69,6 +69,8 @@ default boolean isDeletable() { return this instanceof Deleter; } - String name(); + default String name() { + return defaultNameFor(getClass()); + } } 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 21dc66bd98..aa2f0616ba 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 @@ -30,16 +30,21 @@ public abstract class AbstractDependentResource private ResourceDiscriminator resourceDiscriminator; private final DependentResourceReconciler dependentResourceReconciler; - protected String name = DependentResource.defaultNameFor(this.getClass()); + protected String name; @SuppressWarnings({"unchecked"}) protected AbstractDependentResource() { + this(null); + } + + protected AbstractDependentResource(String name) { creator = creatable ? (Creator) this : null; updater = updatable ? (Updater) this : null; dependentResourceReconciler = this instanceof BulkDependentResource ? new BulkDependentResourceReconciler<>((BulkDependentResource) this) : new SingleDependentResourceReconciler<>(this); + this.name = name == null ? DependentResource.defaultNameFor(this.getClass()) : name; } /** diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/AbstractEventSourceHolderDependentResource.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/AbstractEventSourceHolderDependentResource.java index 6562afd09f..0b9f2ae897 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/AbstractEventSourceHolderDependentResource.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/AbstractEventSourceHolderDependentResource.java @@ -31,6 +31,11 @@ public abstract class AbstractEventSourceHolderDependentResource resourceType) { + this(resourceType, null); + } + + protected AbstractEventSourceHolderDependentResource(Class resourceType, String name) { + super(name); this.resourceType = resourceType; } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/KubernetesDependentResource.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/KubernetesDependentResource.java index b804b88a30..a190853b58 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/KubernetesDependentResource.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/KubernetesDependentResource.java @@ -43,7 +43,12 @@ public abstract class KubernetesDependentResource resourceType) { - super(resourceType); + this(resourceType, null); + } + + @SuppressWarnings("unchecked") + public KubernetesDependentResource(Class resourceType, String name) { + super(resourceType, name); usingCustomResourceUpdateMatcher = this instanceof ResourceUpdaterMatcher; updaterMatcher = usingCustomResourceUpdateMatcher diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/AbstractWorkflowExecutor.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/AbstractWorkflowExecutor.java index cde85a7af4..05b546553c 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/AbstractWorkflowExecutor.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/AbstractWorkflowExecutor.java @@ -18,7 +18,7 @@ import io.javaoperatorsdk.operator.processing.event.ResourceID; @SuppressWarnings("rawtypes") -public abstract class AbstractWorkflowExecutor

{ +abstract class AbstractWorkflowExecutor

{ protected final Workflow

workflow; protected final P primary; @@ -133,19 +133,16 @@ protected void registerOrDeregisterEventSourceBasedOnActivation( boolean activationConditionMet, DependentResourceNode dependentResourceNode) { if (dependentResourceNode.getActivationCondition().isPresent()) { + final var dr = dependentResourceNode.getDependentResource(); + final var eventSourceRetriever = context.eventSourceRetriever(); if (activationConditionMet) { var eventSource = - dependentResourceNode.getDependentResource().eventSource(context.eventSourceRetriever() - .eventSourceContextForDynamicRegistration()); + dr.eventSource(eventSourceRetriever.eventSourceContextForDynamicRegistration()); var es = eventSource.orElseThrow(); - context.eventSourceRetriever() - .dynamicallyRegisterEventSource(dependentResourceNode.getDependentResource().name(), - es); + eventSourceRetriever.dynamicallyRegisterEventSource(dr.name(), es); } else { - context.eventSourceRetriever() - .dynamicallyDeRegisterEventSource( - dependentResourceNode.getDependentResource().name()); + eventSourceRetriever.dynamicallyDeRegisterEventSource(dr.name()); } } } 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 f125fcbe84..fb0b733c32 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 @@ -79,13 +79,14 @@ 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(), - resolve(spec, client, configuration)); - alreadyResolved.put(node.getDependentResource().name(), node); + dependentResource); + alreadyResolved.put(dependentResource.name(), node); spec.getDependsOn() .forEach(depend -> node.addDependsOnRelation(alreadyResolved.get(depend))); } @@ -106,9 +107,9 @@ private DependentResource resolve(DependentResourceSpec spec, configuration.getConfigurationService().dependentResourceFactory() .createFrom(spec, configuration); - if (spec.getName() != null && !spec.getName().equals(NO_VALUE_SET) - && dependentResource instanceof NameSetter) { - ((NameSetter) dependentResource).setName(spec.getName()); + final var name = spec.getName(); + if (name != null && !NO_VALUE_SET.equals(name) && dependentResource instanceof NameSetter) { + ((NameSetter) dependentResource).setName(name); } if (dependentResource instanceof KubernetesClientAware) { 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 9b899c21f7..1c241aebbc 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 @@ -20,7 +20,7 @@ * @param

primary resource */ @SuppressWarnings("rawtypes") -public class DefaultWorkflow

implements Workflow

{ +class DefaultWorkflow

implements Workflow

{ private final Map dependentResourceNodes; private final Set topLevelResources; @@ -79,7 +79,7 @@ private Map toMap(Set node } map.put(node.getDependentResource().name(), node); } - if (topLevelResources.size() == 0) { + if (topLevelResources.isEmpty()) { throw new IllegalStateException( "No top-level dependent resources found. This might indicate a cyclic Set of DependentResourceNode has been provided."); } 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 0a524d573f..3e8a762c5e 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 @@ -8,7 +8,7 @@ import io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResource; @SuppressWarnings("rawtypes") -public class DependentResourceNode { +class DependentResourceNode { private final List dependsOn = new LinkedList<>(); private final List parents = new LinkedList<>(); @@ -54,7 +54,6 @@ public Optional> getReconcilePrecondition() { return Optional.ofNullable(reconcilePrecondition); } - public Optional> getDeletePostcondition() { return Optional.ofNullable(deletePostcondition); } @@ -104,12 +103,6 @@ public int hashCode() { return this.getDependentResource().name().hashCode(); } - @SuppressWarnings("rawtypes") - static String getNameFor(DependentResource dependentResource) { - return DependentResource.defaultNameFor(dependentResource.getClass()) + "#" - + dependentResource.hashCode(); - } - @Override public String toString() { return "DependentResourceNode{" + getDependentResource() + '}'; 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 83e85539c3..90ece70d26 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 @@ -65,8 +65,7 @@ public WorkflowBuilder

withActivationCondition(Condition activationCondition) DependentResourceNode getNodeByDependentResource(DependentResource dependentResource) { // first check by name - final var node = - dependentResourceNodes.get(dependentResource.name()); + final var node = dependentResourceNodes.get(dependentResource.name()); if (node != null) { return node; } else { 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 2782c860dd..fa860c917d 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 @@ -26,11 +26,7 @@ import io.javaoperatorsdk.operator.processing.dependent.kubernetes.KubernetesDependentResourceConfigBuilder; import io.javaoperatorsdk.operator.processing.dependent.workflow.Condition; -import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertFalse; -import static org.junit.jupiter.api.Assertions.assertNotNull; -import static org.junit.jupiter.api.Assertions.assertNull; -import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assertions.*; class ControllerConfigurationOverriderTest { private final BaseConfigurationService configurationService = new BaseConfigurationService(); @@ -102,11 +98,6 @@ public Class resourceType() { return Object.class; } - @Override - public String name() { - return null; - } - @Override public void configureWith(String config) { this.config = config; diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/config/dependent/DependentResourceConfigurationResolverTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/config/dependent/DependentResourceConfigurationResolverTest.java index 4a91909771..31f140c558 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/config/dependent/DependentResourceConfigurationResolverTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/config/dependent/DependentResourceConfigurationResolverTest.java @@ -24,10 +24,7 @@ import io.javaoperatorsdk.operator.processing.dependent.kubernetes.KubernetesDependentResource; import static io.javaoperatorsdk.operator.api.config.dependent.DependentResourceConfigurationResolverTest.CustomAnnotationReconciler.DR_NAME; -import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertNotNull; -import static org.junit.jupiter.api.Assertions.assertNull; -import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assertions.*; class DependentResourceConfigurationResolverTest { @@ -175,11 +172,6 @@ public Class resourceType() { return ConfigMap.class; } - @Override - public String name() { - return null; - } - @Override public void configureWith(CustomConfig config) { this.config = config; 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 59e6fef55a..219e9af869 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 @@ -36,9 +36,7 @@ public class AbstractWorkflowExecutorTest { public class TestDependent extends KubernetesDependentResource { public TestDependent(String name) { - super(ConfigMap.class); - setName(name); - + super(ConfigMap.class, name); } @Override @@ -51,7 +49,7 @@ public ReconcileResult reconcile(TestCustomResource primary, @Override public String toString() { - return name; + return name(); } } @@ -91,7 +89,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/src/test/java/io/javaoperatorsdk/operator/config/BaseConfigurationServiceTest.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/config/BaseConfigurationServiceTest.java index fb94047aa0..97c5ec112c 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/config/BaseConfigurationServiceTest.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/config/BaseConfigurationServiceTest.java @@ -42,13 +42,7 @@ import io.javaoperatorsdk.operator.processing.retry.RetryExecution; import io.javaoperatorsdk.operator.sample.readonly.ReadOnlyDependent; -import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertFalse; -import static org.junit.jupiter.api.Assertions.assertInstanceOf; -import static org.junit.jupiter.api.Assertions.assertNotNull; -import static org.junit.jupiter.api.Assertions.assertNull; -import static org.junit.jupiter.api.Assertions.assertThrows; -import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assertions.*; class BaseConfigurationServiceTest { @@ -470,11 +464,6 @@ public Class resourceType() { return ConfigMap.class; } - @Override - public String name() { - return null; - } - @Override public void configureWith(CustomConfig config) { this.config = config;