Skip to content

Commit 4e42612

Browse files
csviricsviri
committed
feat: move name is directly to dependent resource
- use this name when throwing aggregate exception Signed-off-by: Attila Mészáros <[email protected]>
1 parent e636956 commit 4e42612

File tree

16 files changed

+105
-45
lines changed

16 files changed

+105
-45
lines changed

Diff for: ‎operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/dependent/DependentResource.java

+4
Original file line numberDiff line numberDiff line change
@@ -68,4 +68,8 @@ static String defaultNameFor(Class<? extends DependentResource> dependentResourc
6868
default boolean isDeletable() {
6969
return this instanceof Deleter;
7070
}
71+
72+
String getName();
73+
74+
void setName(String name);
7175
}

Diff for: ‎operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/AbstractDependentResource.java

+11
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,8 @@ public abstract class AbstractDependentResource<R, P extends HasMetadata>
2929
private ResourceDiscriminator<R, P> resourceDiscriminator;
3030
private final DependentResourceReconciler<R, P> dependentResourceReconciler;
3131

32+
protected String name = DependentResource.defaultNameFor(this.getClass());
33+
3234
@SuppressWarnings({"unchecked"})
3335
protected AbstractDependentResource() {
3436
creator = creatable ? (Creator<R, P>) this : null;
@@ -176,4 +178,13 @@ protected boolean isUpdatable() {
176178
public boolean isDeletable() {
177179
return deletable;
178180
}
181+
182+
@Override
183+
public String getName() {
184+
return name;
185+
}
186+
187+
public void setName(String name) {
188+
this.name = name;
189+
}
179190
}

Diff for: ‎operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/AbstractWorkflowExecutor.java

+4-2
Original file line numberDiff line numberDiff line change
@@ -139,11 +139,13 @@ protected <R> void registerOrDeregisterEventSourceBasedOnActivation(
139139
.eventSourceContextForDynamicRegistration());
140140
var es = eventSource.orElseThrow();
141141
context.eventSourceRetriever()
142-
.dynamicallyRegisterEventSource(dependentResourceNode.getName(), es);
142+
.dynamicallyRegisterEventSource(dependentResourceNode.getDependentResource().getName(),
143+
es);
143144

144145
} else {
145146
context.eventSourceRetriever()
146-
.dynamicallyDeRegisterEventSource(dependentResourceNode.getName());
147+
.dynamicallyDeRegisterEventSource(
148+
dependentResourceNode.getDependentResource().getName());
147149
}
148150
}
149151
}

Diff for: ‎operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/DefaultManagedWorkflow.java

+7-2
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
import io.javaoperatorsdk.operator.api.reconciler.dependent.EventSourceReferencer;
1515
import io.javaoperatorsdk.operator.api.reconciler.dependent.managed.KubernetesClientAware;
1616

17+
import static io.javaoperatorsdk.operator.api.reconciler.Constants.NO_VALUE_SET;
1718
import static io.javaoperatorsdk.operator.processing.dependent.workflow.Workflow.THROW_EXCEPTION_AUTOMATICALLY_DEFAULT;
1819

1920
@SuppressWarnings("rawtypes")
@@ -77,13 +78,13 @@ public Workflow<P> resolve(KubernetesClient client,
7778
ControllerConfiguration<P> configuration) {
7879
final var alreadyResolved = new HashMap<String, DependentResourceNode>(orderedSpecs.size());
7980
for (DependentResourceSpec spec : orderedSpecs) {
80-
final var node = new DependentResourceNode(spec.getName(),
81+
final var node = new DependentResourceNode(
8182
spec.getReconcileCondition(),
8283
spec.getDeletePostCondition(),
8384
spec.getReadyCondition(),
8485
spec.getActivationCondition(),
8586
resolve(spec, client, configuration));
86-
alreadyResolved.put(node.getName(), node);
87+
alreadyResolved.put(node.getDependentResource().getName(), node);
8788
spec.getDependsOn()
8889
.forEach(depend -> node.addDependsOnRelation(alreadyResolved.get(depend)));
8990
}
@@ -104,6 +105,10 @@ private <R> DependentResource<R, P> resolve(DependentResourceSpec<R, P> spec,
104105
configuration.getConfigurationService().dependentResourceFactory()
105106
.createFrom(spec, configuration);
106107

108+
if (spec.getName() != null && !spec.getName().equals(NO_VALUE_SET)) {
109+
dependentResource.setName(spec.getName());
110+
}
111+
107112
if (dependentResource instanceof KubernetesClientAware) {
108113
((KubernetesClientAware) dependentResource).setKubernetesClient(client);
109114
}

Diff for: ‎operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/DefaultWorkflow.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ private Map<String, DependentResourceNode> toMap(Set<DependentResourceNode> node
7777
bottomLevelResource.remove(dependsOn);
7878
}
7979
}
80-
map.put(node.getName(), node);
80+
map.put(node.getDependentResource().getName(), node);
8181
}
8282
if (topLevelResources.size() == 0) {
8383
throw new IllegalStateException(

Diff for: ‎operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/DependentResourceNode.java

+5-15
Original file line numberDiff line numberDiff line change
@@ -12,25 +12,20 @@ public class DependentResourceNode<R, P extends HasMetadata> {
1212

1313
private final List<DependentResourceNode> dependsOn = new LinkedList<>();
1414
private final List<DependentResourceNode> parents = new LinkedList<>();
15-
private final String name;
15+
1616
private Condition<R, P> reconcilePrecondition;
1717
private Condition<R, P> deletePostcondition;
1818
private Condition<R, P> readyPostcondition;
1919
private Condition<R, P> activationCondition;
2020
private final DependentResource<R, P> dependentResource;
2121

22-
DependentResourceNode(String name, DependentResource<R, P> dependentResource) {
23-
this(name, null, null, null, null, dependentResource);
24-
}
25-
2622
DependentResourceNode(DependentResource<R, P> dependentResource) {
27-
this(getNameFor(dependentResource), null, null, null, null, dependentResource);
23+
this(null, null, null, null, dependentResource);
2824
}
2925

30-
public DependentResourceNode(String name, Condition<R, P> reconcilePrecondition,
26+
public DependentResourceNode(Condition<R, P> reconcilePrecondition,
3127
Condition<R, P> deletePostcondition, Condition<R, P> readyPostcondition,
3228
Condition<R, P> activationCondition, DependentResource<R, P> dependentResource) {
33-
this.name = name;
3429
this.reconcilePrecondition = reconcilePrecondition;
3530
this.deletePostcondition = deletePostcondition;
3631
this.readyPostcondition = readyPostcondition;
@@ -55,11 +50,6 @@ public List<DependentResourceNode> getParents() {
5550
return parents;
5651
}
5752

58-
public String getName() {
59-
return name;
60-
}
61-
62-
6353
public Optional<Condition<R, P>> getReconcilePrecondition() {
6454
return Optional.ofNullable(reconcilePrecondition);
6555
}
@@ -106,12 +96,12 @@ public boolean equals(Object o) {
10696
return false;
10797
}
10898
DependentResourceNode<?, ?> that = (DependentResourceNode<?, ?>) o;
109-
return name.equals(that.name);
99+
return this.getDependentResource().getName().equals(that.getDependentResource().getName());
110100
}
111101

112102
@Override
113103
public int hashCode() {
114-
return name.hashCode();
104+
return this.getDependentResource().getName().hashCode();
115105
}
116106

117107
@SuppressWarnings("rawtypes")

Diff for: ‎operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowBuilder.java

+3-8
Original file line numberDiff line numberDiff line change
@@ -21,14 +21,9 @@ public class WorkflowBuilder<P extends HasMetadata> {
2121
private boolean isCleaner = false;
2222

2323
public WorkflowBuilder<P> addDependentResource(DependentResource dependentResource) {
24-
return addDependentResource(dependentResource, null);
25-
}
26-
27-
public WorkflowBuilder<P> addDependentResource(DependentResource dependentResource, String name) {
28-
currentNode = name == null ? new DependentResourceNode<>(dependentResource)
29-
: new DependentResourceNode<>(name, dependentResource);
24+
currentNode = new DependentResourceNode<>(dependentResource);
3025
isCleaner = isCleaner || dependentResource.isDeletable();
31-
final var actualName = currentNode.getName();
26+
final var actualName = dependentResource.getName();
3227
dependentResourceNodes.put(actualName, currentNode);
3328
return this;
3429
}
@@ -71,7 +66,7 @@ public WorkflowBuilder<P> withActivationCondition(Condition activationCondition)
7166
DependentResourceNode getNodeByDependentResource(DependentResource<?, ?> dependentResource) {
7267
// first check by name
7368
final var node =
74-
dependentResourceNodes.get(DependentResourceNode.getNameFor(dependentResource));
69+
dependentResourceNodes.get(dependentResource.getName());
7570
if (node != null) {
7671
return node;
7772
} else {

Diff for: ‎operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowResult.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ public void throwAggregateExceptionIfErrorsPresent() {
3838
if (erroredDependentsExist()) {
3939
throw new AggregatedOperatorException("Exception(s) during workflow execution.",
4040
erroredDependents.entrySet().stream()
41-
.collect(Collectors.toMap(e -> e.getKey().getClass().getName(), Entry::getValue)));
41+
.collect(Collectors.toMap(e -> e.getKey().getName(), Entry::getValue)));
4242
}
4343
}
4444
}

Diff for: ‎operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/config/ControllerConfigurationOverriderTest.java

+8
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,14 @@ public Class<Object> resourceType() {
103103
return Object.class;
104104
}
105105

106+
@Override
107+
public String getName() {
108+
return null;
109+
}
110+
111+
@Override
112+
public void setName(String name) {}
113+
106114
@Override
107115
public void configureWith(String config) {
108116
this.config = config;

Diff for: ‎operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/config/dependent/DependentResourceConfigurationResolverTest.java

+8
Original file line numberDiff line numberDiff line change
@@ -175,6 +175,14 @@ public Class<ConfigMap> resourceType() {
175175
return ConfigMap.class;
176176
}
177177

178+
@Override
179+
public String getName() {
180+
return null;
181+
}
182+
183+
@Override
184+
public void setName(String name) {}
185+
178186
@Override
179187
public void configureWith(CustomConfig config) {
180188
this.config = config;

Diff for: ‎operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/EmptyTestDependentResource.java

+12
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@
99
public class EmptyTestDependentResource
1010
implements DependentResource<Deployment, TestCustomResource> {
1111

12+
private String name;
13+
1214
@Override
1315
public ReconcileResult<Deployment> reconcile(TestCustomResource primary,
1416
Context<TestCustomResource> context) {
@@ -19,5 +21,15 @@ public ReconcileResult<Deployment> reconcile(TestCustomResource primary,
1921
public Class<Deployment> resourceType() {
2022
return Deployment.class;
2123
}
24+
25+
@Override
26+
public String getName() {
27+
return name;
28+
}
29+
30+
@Override
31+
public void setName(String name) {
32+
this.name = name;
33+
}
2234
}
2335

Diff for: ‎operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/workflow/AbstractWorkflowExecutorTest.java

+13-4
Original file line numberDiff line numberDiff line change
@@ -35,11 +35,10 @@ public class AbstractWorkflowExecutorTest {
3535

3636
public class TestDependent extends KubernetesDependentResource<ConfigMap, TestCustomResource> {
3737

38-
private final String name;
39-
4038
public TestDependent(String name) {
4139
super(ConfigMap.class);
42-
this.name = name;
40+
setName(name);
41+
4342
}
4443

4544
@Override
@@ -92,7 +91,7 @@ public void delete(TestCustomResource primary, Context<TestCustomResource> conte
9291
}
9392

9493
public class TestErrorDependent implements DependentResource<String, TestCustomResource> {
95-
private final String name;
94+
private String name;
9695

9796
public TestErrorDependent(String name) {
9897
this.name = name;
@@ -110,6 +109,16 @@ public Class<String> resourceType() {
110109
return String.class;
111110
}
112111

112+
@Override
113+
public String getName() {
114+
return name;
115+
}
116+
117+
@Override
118+
public void setName(String name) {
119+
this.name = name;
120+
}
121+
113122
@Override
114123
public String toString() {
115124
return name;

Diff for: ‎operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowBuilderTest.java

+2
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,10 @@ class WorkflowBuilderTest {
1313
@Test
1414
void workflowIsCleanerIfAtLeastOneDRIsCleaner() {
1515
var dr = mock(DependentResource.class);
16+
when(dr.getName()).thenReturn("dr");
1617
var deleter = mock(DependentResource.class);
1718
when(deleter.isDeletable()).thenReturn(true);
19+
when(deleter.getName()).thenReturn("deleter");
1820

1921
var workflow = new WorkflowBuilder<TestCustomResource>()
2022
.addDependentResource(deleter)

Diff for: ‎operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowReconcileExecutorTest.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -538,7 +538,7 @@ void reconcilePreconditionNotCheckedOnNonActiveDependent() {
538538
@Test
539539
void deletesDependentsOfNonActiveDependentButNotTheNonActive() {
540540
TestDeleterDependent drDeleter2 = new TestDeleterDependent("DR_DELETER_2");
541-
TestDeleterDependent drDeleter3 = new TestDeleterDependent("DR_DELETER_2");
541+
TestDeleterDependent drDeleter3 = new TestDeleterDependent("DR_DELETER_3");
542542

543543
var workflow = new WorkflowBuilder<TestCustomResource>()
544544
.addDependentResource(dr1).withActivationCondition(notMetCondition)

Diff for: ‎operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowTest.java

+17-11
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,7 @@
1717
import static org.junit.Assert.assertThrows;
1818
import static org.junit.jupiter.api.Assertions.assertFalse;
1919
import static org.junit.jupiter.api.Assertions.assertTrue;
20-
import static org.mockito.Mockito.mock;
21-
import static org.mockito.Mockito.withSettings;
20+
import static org.mockito.Mockito.*;
2221

2322
@SuppressWarnings("rawtypes")
2423
class WorkflowTest {
@@ -27,9 +26,9 @@ class WorkflowTest {
2726

2827
@Test
2928
void zeroTopLevelDRShouldThrowException() {
30-
var dr1 = mock(DependentResource.class);
31-
var dr2 = mock(DependentResource.class);
32-
var dr3 = mock(DependentResource.class);
29+
var dr1 = mockDependent("dr1");
30+
var dr2 = mockDependent("dr2");
31+
var dr3 = mockDependent("dr3");
3332

3433
var cyclicWorkflowBuilderSetup = new WorkflowBuilder<TestCustomResource>()
3534
.addDependentResource(dr1).dependsOn()
@@ -43,9 +42,9 @@ void zeroTopLevelDRShouldThrowException() {
4342

4443
@Test
4544
void calculatesTopLevelResources() {
46-
var dr1 = mock(DependentResource.class);
47-
var dr2 = mock(DependentResource.class);
48-
var independentDR = mock(DependentResource.class);
45+
var dr1 = mockDependent("dr1");
46+
var dr2 = mockDependent("dr2");
47+
var independentDR = mockDependent("independentDR");
4948

5049
var workflow = new WorkflowBuilder<TestCustomResource>()
5150
.addDependentResource(independentDR)
@@ -63,9 +62,9 @@ void calculatesTopLevelResources() {
6362

6463
@Test
6564
void calculatesBottomLevelResources() {
66-
var dr1 = mock(DependentResource.class);
67-
var dr2 = mock(DependentResource.class);
68-
var independentDR = mock(DependentResource.class);
65+
var dr1 = mockDependent("dr1");
66+
var dr2 = mockDependent("dr2");
67+
var independentDR = mockDependent("independentDR");
6968

7069
Workflow<TestCustomResource> workflow = new WorkflowBuilder<TestCustomResource>()
7170
.addDependentResource(independentDR)
@@ -100,4 +99,11 @@ void isDeletableShouldWork() {
10099
GarbageCollected.class));
101100
assertFalse(DefaultWorkflow.isDeletable(dr.getClass()));
102101
}
102+
103+
static DependentResource mockDependent(String name) {
104+
var res = mock(DependentResource.class);
105+
when(res.getName()).thenReturn(name);
106+
return res;
107+
}
108+
103109
}

Diff for: ‎operator-framework/src/test/java/io/javaoperatorsdk/operator/config/BaseConfigurationServiceTest.java

+8
Original file line numberDiff line numberDiff line change
@@ -470,6 +470,14 @@ public Class<ConfigMap> resourceType() {
470470
return ConfigMap.class;
471471
}
472472

473+
@Override
474+
public String getName() {
475+
return null;
476+
}
477+
478+
@Override
479+
public void setName(String name) {}
480+
473481
@Override
474482
public void configureWith(CustomConfig config) {
475483
this.config = config;

0 commit comments

Comments
 (0)