Skip to content

Commit 15c779f

Browse files
csvirimetacosm
csviri
and
metacosm
committed
feat: move name is directly to dependent resource (#2250)
* feat: move name is directly to dependent resource - use this name when throwing aggregate exception Signed-off-by: Attila Mészáros <[email protected]> * refactor to use a dedicated interface for setting the name Signed-off-by: Attila Mészáros <[email protected]> * refactor: add default implementation for name() (#2255) Signed-off-by: Chris Laprun <[email protected]> Signed-off-by: Attila Mészáros <[email protected]> --------- Signed-off-by: Attila Mészáros <[email protected]> Signed-off-by: Chris Laprun <[email protected]> Co-authored-by: Chris Laprun <[email protected]> Signed-off-by: Attila Mészáros <[email protected]>
1 parent b0a8597 commit 15c779f

19 files changed

+118
-98
lines changed

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

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

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

+7
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
package io.javaoperatorsdk.operator.api.reconciler.dependent;
2+
3+
public interface NameSetter {
4+
5+
void setName(String name);
6+
7+
}

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

+18-1
Original file line numberDiff line numberDiff line change
@@ -11,13 +11,14 @@
1111
import io.javaoperatorsdk.operator.api.reconciler.ResourceDiscriminator;
1212
import io.javaoperatorsdk.operator.api.reconciler.dependent.Deleter;
1313
import io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResource;
14+
import io.javaoperatorsdk.operator.api.reconciler.dependent.NameSetter;
1415
import io.javaoperatorsdk.operator.api.reconciler.dependent.ReconcileResult;
1516
import io.javaoperatorsdk.operator.processing.dependent.Matcher.Result;
1617
import io.javaoperatorsdk.operator.processing.event.ResourceID;
1718

1819
@Ignore
1920
public abstract class AbstractDependentResource<R, P extends HasMetadata>
20-
implements DependentResource<R, P> {
21+
implements DependentResource<R, P>, NameSetter {
2122
private static final Logger log = LoggerFactory.getLogger(AbstractDependentResource.class);
2223

2324
private final boolean creatable = this instanceof Creator;
@@ -29,14 +30,21 @@ public abstract class AbstractDependentResource<R, P extends HasMetadata>
2930
private ResourceDiscriminator<R, P> resourceDiscriminator;
3031
private final DependentResourceReconciler<R, P> dependentResourceReconciler;
3132

33+
protected String name;
34+
3235
@SuppressWarnings({"unchecked"})
3336
protected AbstractDependentResource() {
37+
this(null);
38+
}
39+
40+
protected AbstractDependentResource(String name) {
3441
creator = creatable ? (Creator<R, P>) this : null;
3542
updater = updatable ? (Updater<R, P>) this : null;
3643

3744
dependentResourceReconciler = this instanceof BulkDependentResource
3845
? new BulkDependentResourceReconciler<>((BulkDependentResource<R, P>) this)
3946
: new SingleDependentResourceReconciler<>(this);
47+
this.name = name == null ? DependentResource.defaultNameFor(this.getClass()) : name;
4048
}
4149

4250
/**
@@ -183,4 +191,13 @@ protected boolean isUpdatable() {
183191
public boolean isDeletable() {
184192
return deletable;
185193
}
194+
195+
@Override
196+
public String name() {
197+
return name;
198+
}
199+
200+
public void setName(String name) {
201+
this.name = name;
202+
}
186203
}

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

+5
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,11 @@ public abstract class AbstractEventSourceHolderDependentResource<R, P extends Ha
3131
protected String eventSourceNameToUse;
3232

3333
protected AbstractEventSourceHolderDependentResource(Class<R> resourceType) {
34+
this(resourceType, null);
35+
}
36+
37+
protected AbstractEventSourceHolderDependentResource(Class<R> resourceType, String name) {
38+
super(name);
3439
this.resourceType = resourceType;
3540
}
3641

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

+6-1
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,12 @@ public abstract class KubernetesDependentResource<R extends HasMetadata, P exten
4242

4343
@SuppressWarnings("unchecked")
4444
public KubernetesDependentResource(Class<R> resourceType) {
45-
super(resourceType);
45+
this(resourceType, null);
46+
}
47+
48+
@SuppressWarnings("unchecked")
49+
public KubernetesDependentResource(Class<R> resourceType, String name) {
50+
super(resourceType, name);
4651

4752
updaterMatcher = this instanceof ResourceUpdaterMatcher
4853
? (ResourceUpdaterMatcher<R>) this

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

+6-7
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@
1818
import io.javaoperatorsdk.operator.processing.event.ResourceID;
1919

2020
@SuppressWarnings("rawtypes")
21-
public abstract class AbstractWorkflowExecutor<P extends HasMetadata> {
21+
abstract class AbstractWorkflowExecutor<P extends HasMetadata> {
2222

2323
protected final Workflow<P> workflow;
2424
protected final P primary;
@@ -133,17 +133,16 @@ protected <R> void registerOrDeregisterEventSourceBasedOnActivation(
133133
boolean activationConditionMet,
134134
DependentResourceNode<R, P> dependentResourceNode) {
135135
if (dependentResourceNode.getActivationCondition().isPresent()) {
136+
final var dr = dependentResourceNode.getDependentResource();
137+
final var eventSourceRetriever = context.eventSourceRetriever();
136138
if (activationConditionMet) {
137139
var eventSource =
138-
dependentResourceNode.getDependentResource().eventSource(context.eventSourceRetriever()
139-
.eventSourceContextForDynamicRegistration());
140+
dr.eventSource(eventSourceRetriever.eventSourceContextForDynamicRegistration());
140141
var es = eventSource.orElseThrow();
141-
context.eventSourceRetriever()
142-
.dynamicallyRegisterEventSource(dependentResourceNode.getName(), es);
142+
eventSourceRetriever.dynamicallyRegisterEventSource(dr.name(), es);
143143

144144
} else {
145-
context.eventSourceRetriever()
146-
.dynamicallyDeRegisterEventSource(dependentResourceNode.getName());
145+
eventSourceRetriever.dynamicallyDeRegisterEventSource(dr.name());
147146
}
148147
}
149148
}

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

+11-3
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,10 @@
1212
import io.javaoperatorsdk.operator.api.config.dependent.DependentResourceSpec;
1313
import io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResource;
1414
import io.javaoperatorsdk.operator.api.reconciler.dependent.EventSourceReferencer;
15+
import io.javaoperatorsdk.operator.api.reconciler.dependent.NameSetter;
1516
import io.javaoperatorsdk.operator.api.reconciler.dependent.managed.KubernetesClientAware;
1617

18+
import static io.javaoperatorsdk.operator.api.reconciler.Constants.NO_VALUE_SET;
1719
import static io.javaoperatorsdk.operator.processing.dependent.workflow.Workflow.THROW_EXCEPTION_AUTOMATICALLY_DEFAULT;
1820

1921
@SuppressWarnings("rawtypes")
@@ -77,13 +79,14 @@ public Workflow<P> resolve(KubernetesClient client,
7779
ControllerConfiguration<P> configuration) {
7880
final var alreadyResolved = new HashMap<String, DependentResourceNode>(orderedSpecs.size());
7981
for (DependentResourceSpec spec : orderedSpecs) {
80-
final var node = new DependentResourceNode(spec.getName(),
82+
final var dependentResource = resolve(spec, client, configuration);
83+
final var node = new DependentResourceNode(
8184
spec.getReconcileCondition(),
8285
spec.getDeletePostCondition(),
8386
spec.getReadyCondition(),
8487
spec.getActivationCondition(),
85-
resolve(spec, client, configuration));
86-
alreadyResolved.put(node.getName(), node);
88+
dependentResource);
89+
alreadyResolved.put(dependentResource.name(), node);
8790
spec.getDependsOn()
8891
.forEach(depend -> node.addDependsOnRelation(alreadyResolved.get(depend)));
8992
}
@@ -104,6 +107,11 @@ private <R> DependentResource<R, P> resolve(DependentResourceSpec<R, P> spec,
104107
configuration.getConfigurationService().dependentResourceFactory()
105108
.createFrom(spec, configuration);
106109

110+
final var name = spec.getName();
111+
if (name != null && !NO_VALUE_SET.equals(name) && dependentResource instanceof NameSetter) {
112+
((NameSetter) dependentResource).setName(name);
113+
}
114+
107115
if (dependentResource instanceof KubernetesClientAware) {
108116
((KubernetesClientAware) dependentResource).setKubernetesClient(client);
109117
}

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

+2-2
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@
2121
* @param <P> primary resource
2222
*/
2323
@SuppressWarnings("rawtypes")
24-
public class DefaultWorkflow<P extends HasMetadata> implements Workflow<P> {
24+
class DefaultWorkflow<P extends HasMetadata> implements Workflow<P> {
2525

2626
private final Map<String, DependentResourceNode> dependentResourceNodes;
2727
private final Set<DependentResourceNode> topLevelResources;
@@ -78,7 +78,7 @@ private Map<String, DependentResourceNode> toMap(Set<DependentResourceNode> node
7878
bottomLevelResource.remove(dependsOn);
7979
}
8080
}
81-
map.put(node.getName(), node);
81+
map.put(node.getDependentResource().name(), node);
8282
}
8383
if (topLevelResources.isEmpty()) {
8484
throw new IllegalStateException(

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

+6-23
Original file line numberDiff line numberDiff line change
@@ -8,29 +8,24 @@
88
import io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResource;
99

1010
@SuppressWarnings("rawtypes")
11-
public class DependentResourceNode<R, P extends HasMetadata> {
11+
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,16 +50,10 @@ 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
}
6656

67-
6857
public Optional<Condition<R, P>> getDeletePostcondition() {
6958
return Optional.ofNullable(deletePostcondition);
7059
}
@@ -106,18 +95,12 @@ public boolean equals(Object o) {
10695
return false;
10796
}
10897
DependentResourceNode<?, ?> that = (DependentResourceNode<?, ?>) o;
109-
return name.equals(that.name);
98+
return this.getDependentResource().name().equals(that.getDependentResource().name());
11099
}
111100

112101
@Override
113102
public int hashCode() {
114-
return name.hashCode();
115-
}
116-
117-
@SuppressWarnings("rawtypes")
118-
static String getNameFor(DependentResource dependentResource) {
119-
return DependentResource.defaultNameFor(dependentResource.getClass()) + "#"
120-
+ dependentResource.hashCode();
103+
return this.getDependentResource().name().hashCode();
121104
}
122105

123106
@Override

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

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

2222
public WorkflowBuilder<P> addDependentResource(DependentResource dependentResource) {
23-
return addDependentResource(dependentResource, null);
24-
}
25-
26-
public WorkflowBuilder<P> addDependentResource(DependentResource dependentResource, String name) {
27-
currentNode = name == null ? new DependentResourceNode<>(dependentResource)
28-
: new DependentResourceNode<>(name, dependentResource);
23+
currentNode = new DependentResourceNode<>(dependentResource);
2924
isCleaner = isCleaner || dependentResource.isDeletable();
30-
final var actualName = currentNode.getName();
25+
final var actualName = dependentResource.name();
3126
dependentResourceNodes.put(actualName, currentNode);
3227
return this;
3328
}
@@ -69,8 +64,7 @@ public WorkflowBuilder<P> withActivationCondition(Condition activationCondition)
6964

7065
DependentResourceNode getNodeByDependentResource(DependentResource<?, ?> dependentResource) {
7166
// first check by name
72-
final var node =
73-
dependentResourceNodes.get(DependentResourceNode.getNameFor(dependentResource));
67+
final var node = dependentResourceNodes.get(dependentResource.name());
7468
if (node != null) {
7569
return node;
7670
} else {

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

+4-16
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
import java.util.HashMap;
44
import java.util.Map;
55
import java.util.Map.Entry;
6+
import java.util.stream.Collectors;
67

78
import io.javaoperatorsdk.operator.AggregatedOperatorException;
89
import io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResource;
@@ -37,22 +38,9 @@ public boolean erroredDependentsExist() {
3738

3839
public void throwAggregateExceptionIfErrorsPresent() {
3940
if (erroredDependentsExist()) {
40-
Map<String, Exception> exceptionMap = new HashMap<>();
41-
Map<String, Integer> numberOfClasses = new HashMap<>();
42-
43-
for (Entry<DependentResource, Exception> entry : erroredDependents.entrySet()) {
44-
String name = entry.getKey().getClass().getName();
45-
var num = numberOfClasses.getOrDefault(name, 0);
46-
if (num > 0) {
47-
exceptionMap.put(name + NUMBER_DELIMITER + num, entry.getValue());
48-
} else {
49-
exceptionMap.put(name, entry.getValue());
50-
}
51-
numberOfClasses.put(name, num + 1);
52-
}
53-
54-
throw new AggregatedOperatorException("Exception(s) during workflow execution.",
55-
exceptionMap);
41+
throw new AggregatedOperatorException("Exception(s) during workflow execution.",
42+
erroredDependents.entrySet().stream()
43+
.collect(Collectors.toMap(e -> e.getKey().name(), Entry::getValue)));
5644
}
5745
}
5846
}

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

+2-7
Original file line numberDiff line numberDiff line change
@@ -26,11 +26,7 @@
2626
import io.javaoperatorsdk.operator.processing.dependent.kubernetes.KubernetesDependentResourceConfigBuilder;
2727
import io.javaoperatorsdk.operator.processing.dependent.workflow.Condition;
2828

29-
import static org.junit.jupiter.api.Assertions.assertEquals;
30-
import static org.junit.jupiter.api.Assertions.assertFalse;
31-
import static org.junit.jupiter.api.Assertions.assertNotNull;
32-
import static org.junit.jupiter.api.Assertions.assertNull;
33-
import static org.junit.jupiter.api.Assertions.assertTrue;
29+
import static org.junit.jupiter.api.Assertions.*;
3430

3531
class ControllerConfigurationOverriderTest {
3632
private final BaseConfigurationService configurationService = new BaseConfigurationService();
@@ -75,8 +71,7 @@ void overridingNSShouldPreserveUntouchedDependents() {
7571
private static class NamedDependentReconciler implements Reconciler<ConfigMap> {
7672

7773
@Override
78-
public UpdateControl<ConfigMap> reconcile(ConfigMap resource, Context<ConfigMap> context)
79-
throws Exception {
74+
public UpdateControl<ConfigMap> reconcile(ConfigMap resource, Context<ConfigMap> context) {
8075
return null;
8176
}
8277

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

+1-4
Original file line numberDiff line numberDiff line change
@@ -24,10 +24,7 @@
2424
import io.javaoperatorsdk.operator.processing.dependent.kubernetes.KubernetesDependentResource;
2525

2626
import static io.javaoperatorsdk.operator.api.config.dependent.DependentResourceConfigurationResolverTest.CustomAnnotationReconciler.DR_NAME;
27-
import static org.junit.jupiter.api.Assertions.assertEquals;
28-
import static org.junit.jupiter.api.Assertions.assertNotNull;
29-
import static org.junit.jupiter.api.Assertions.assertNull;
30-
import static org.junit.jupiter.api.Assertions.assertTrue;
27+
import static org.junit.jupiter.api.Assertions.*;
3128

3229
class DependentResourceConfigurationResolverTest {
3330

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

+11
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,14 @@ public ReconcileResult<Deployment> reconcile(TestCustomResource primary,
1921
public Class<Deployment> resourceType() {
2022
return Deployment.class;
2123
}
24+
25+
@Override
26+
public String name() {
27+
return name;
28+
}
29+
30+
public void setName(String name) {
31+
this.name = name;
32+
}
2233
}
2334

0 commit comments

Comments
 (0)