Skip to content

Commit d730ed0

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 30d82f3 commit d730ed0

19 files changed

+116
-98
lines changed

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

Lines changed: 5 additions & 0 deletions
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
}

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

Lines changed: 7 additions & 0 deletions
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+
}

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

Lines changed: 18 additions & 1 deletion
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
/**
@@ -176,4 +184,13 @@ protected boolean isUpdatable() {
176184
public boolean isDeletable() {
177185
return deletable;
178186
}
187+
188+
@Override
189+
public String name() {
190+
return name;
191+
}
192+
193+
public void setName(String name) {
194+
this.name = name;
195+
}
179196
}

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

Lines changed: 5 additions & 0 deletions
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

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

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,12 @@ public abstract class KubernetesDependentResource<R extends HasMetadata, P exten
4343

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

4853
usingCustomResourceUpdateMatcher = this instanceof ResourceUpdaterMatcher;
4954
updaterMatcher = usingCustomResourceUpdateMatcher

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

Lines changed: 6 additions & 7 deletions
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
}

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

Lines changed: 11 additions & 3 deletions
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
}

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@
2020
* @param <P> primary resource
2121
*/
2222
@SuppressWarnings("rawtypes")
23-
public class DefaultWorkflow<P extends HasMetadata> implements Workflow<P> {
23+
class DefaultWorkflow<P extends HasMetadata> implements Workflow<P> {
2424

2525
private final Map<String, DependentResourceNode> dependentResourceNodes;
2626
private final Set<DependentResourceNode> topLevelResources;
@@ -77,9 +77,9 @@ 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().name(), node);
8181
}
82-
if (topLevelResources.size() == 0) {
82+
if (topLevelResources.isEmpty()) {
8383
throw new IllegalStateException(
8484
"No top-level dependent resources found. This might indicate a cyclic Set of DependentResourceNode has been provided.");
8585
}

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

Lines changed: 6 additions & 23 deletions
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

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

Lines changed: 3 additions & 9 deletions
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.name();
3227
dependentResourceNodes.put(actualName, currentNode);
3328
return this;
3429
}
@@ -70,8 +65,7 @@ public WorkflowBuilder<P> withActivationCondition(Condition activationCondition)
7065

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

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

Lines changed: 4 additions & 16 deletions
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
}

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

Lines changed: 2 additions & 7 deletions
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

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

Lines changed: 1 addition & 4 deletions
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

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

Lines changed: 11 additions & 0 deletions
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)