Skip to content

Commit 8f1a416

Browse files
authored
feat: provide more exception context on workflow errors (#1699)
* feat: provide more exception context on workflow errors * refactor: simplify * docs: improve wording
1 parent c4fc798 commit 8f1a416

File tree

8 files changed

+41
-33
lines changed

8 files changed

+41
-33
lines changed

docs/documentation/patterns-best-practices.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,7 @@ done automatically, the whole process can be completely transparent.
9191
## Managing State
9292

9393
Thanks to the declarative nature of Kubernetes resources, operators that deal only with
94-
Kubernetes resources can operator in a stateless fashion, i.e. they do not need to maintain
94+
Kubernetes resources can operate in a stateless fashion, i.e. they do not need to maintain
9595
information about the state of these resources, as it should be possible to completely rebuild
9696
the resource state from its representation (that's what declarative means, after all).
9797
However, this usually doesn't hold true anymore when dealing with external resources, and it

operator-framework-core/src/main/java/io/javaoperatorsdk/operator/AggregatedOperatorException.java

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,10 @@
11
package io.javaoperatorsdk.operator;
22

3+
import java.io.PrintWriter;
4+
import java.io.StringWriter;
35
import java.util.Collections;
46
import java.util.Map;
7+
import java.util.Map.Entry;
58
import java.util.stream.Collectors;
69

710
public class AggregatedOperatorException extends OperatorException {
@@ -22,7 +25,15 @@ public Map<String, Exception> getAggregatedExceptions() {
2225
@Override
2326
public String getMessage() {
2427
return super.getMessage() + " " + causes.entrySet().stream()
25-
.map(entry -> entry.getKey() + " -> " + entry.getValue())
26-
.collect(Collectors.joining("\n - ", "Details:\n", ""));
28+
.map(entry -> entry.getKey() + " -> " + exceptionDescription(entry))
29+
.collect(Collectors.joining("\n - ", "Details:\n - ", ""));
30+
}
31+
32+
private static String exceptionDescription(Entry<String, Exception> entry) {
33+
final var exception = entry.getValue();
34+
final var out = new StringWriter(2000);
35+
final var stringWriter = new PrintWriter(out);
36+
exception.printStackTrace(stringWriter);
37+
return out.toString();
2738
}
2839
}

operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/EventSourceInitializer.java

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -60,8 +60,7 @@ static <K extends HasMetadata> Map<String, EventSource> nameEventSourcesFromDepe
6060
}
6161

6262
/**
63-
* This is for the use case when the event sources are not access explicitly by name in the
64-
* reconciler.
63+
* Used when event sources are not explicitly named when created/registered.
6564
*
6665
* @param eventSource EventSource
6766
* @return generated name

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -32,12 +32,12 @@ public interface DependentResource<R, P extends HasMetadata> {
3232
Class<R> resourceType();
3333

3434
/**
35-
* Dependent resources are designed to by default provide event sources. There are cases where it
36-
* might not:
35+
* Dependent resources are designed to by default provide event sources. There are cases where
36+
* they might not:
3737
* <ul>
3838
* <li>If an event source is shared between multiple dependent resources. In this case only one or
39-
* none of the dependent resources sharing the event source should provide one.</li>
40-
* <li>Some special implementation of an event source. That just execute some action might not
39+
* none of the dependent resources sharing the event source should provide one, if any.</li>
40+
* <li>Some special implementation of an event source that just executes some action might not
4141
* provide one.</li>
4242
* </ul>
4343
*

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

Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -33,14 +33,11 @@ protected AbstractExternalDependentResource(Class<R> resourceType) {
3333
public void resolveEventSource(EventSourceRetriever<P> eventSourceRetriever) {
3434
super.resolveEventSource(eventSourceRetriever);
3535
if (isDependentResourceWithExplicitState) {
36-
externalStateEventSource = (InformerEventSource<?, P>) dependentResourceWithExplicitState
37-
.eventSourceName()
38-
.map(n -> eventSourceRetriever
39-
.getResourceEventSourceFor(dependentResourceWithExplicitState.stateResourceClass(),
40-
(String) n))
41-
.orElseGet(() -> eventSourceRetriever
42-
.getResourceEventSourceFor(
43-
(Class<R>) dependentResourceWithExplicitState.stateResourceClass()));
36+
final var eventSourceName = (String) dependentResourceWithExplicitState
37+
.eventSourceName().orElse(null);
38+
externalStateEventSource = (InformerEventSource<?, P>) eventSourceRetriever
39+
.getResourceEventSourceFor(dependentResourceWithExplicitState.stateResourceClass(),
40+
eventSourceName);
4441
}
4542

4643
}
@@ -65,13 +62,13 @@ public void delete(P primary, Context<P> context) {
6562
}
6663
}
6764

68-
@SuppressWarnings("unchecked")
65+
@SuppressWarnings({"unchecked", "unused"})
6966
private void handleExplicitStateDelete(P primary, R secondary, Context<P> context) {
7067
var res = dependentResourceWithExplicitState.stateResource(primary, secondary);
7168
dependentResourceWithExplicitState.getKubernetesClient().resource(res).delete();
7269
}
7370

74-
@SuppressWarnings({"rawtypes", "unchecked"})
71+
@SuppressWarnings({"rawtypes", "unchecked", "unused"})
7572
protected void handleExplicitStateCreation(P primary, R created, Context<P> context) {
7673
var resource = dependentResourceWithExplicitState.stateResource(primary, created);
7774
var stateResource =

operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventSourceManager.java

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
package io.javaoperatorsdk.operator.processing.event;
22

3-
import java.util.*;
43
import java.util.LinkedHashSet;
54
import java.util.List;
5+
import java.util.Map;
66
import java.util.Objects;
77
import java.util.Set;
88
import java.util.function.Function;
@@ -213,11 +213,6 @@ public ControllerResourceEventSource<P> getControllerResourceEventSource() {
213213
return eventSources.controllerResourceEventSource();
214214
}
215215

216-
@Override
217-
public <R> ResourceEventSource<R, P> getResourceEventSourceFor(Class<R> dependentType) {
218-
return getResourceEventSourceFor(dependentType, null);
219-
}
220-
221216
public <R> List<ResourceEventSource<R, P>> getResourceEventSourcesFor(Class<R> dependentType) {
222217
return eventSources.getEventSources(dependentType);
223218
}

operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventSourceRetriever.java

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,11 +7,11 @@
77

88
public interface EventSourceRetriever<P extends HasMetadata> {
99

10-
<R> ResourceEventSource<R, P> getResourceEventSourceFor(
11-
Class<R> dependentType);
10+
default <R> ResourceEventSource<R, P> getResourceEventSourceFor(Class<R> dependentType) {
11+
return getResourceEventSourceFor(dependentType, null);
12+
}
1213

13-
<R> ResourceEventSource<R, P> getResourceEventSourceFor(
14-
Class<R> dependentType, String name);
14+
<R> ResourceEventSource<R, P> getResourceEventSourceFor(Class<R> dependentType, String name);
1515

1616
<R> List<ResourceEventSource<R, P>> getResourceEventSourcesFor(Class<R> dependentType);
1717

operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/externalstate/externalstatebulkdependent/BulkDependentResourceExternalWithState.java

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,19 @@
11
package io.javaoperatorsdk.operator.sample.externalstate.externalstatebulkdependent;
22

3-
import java.util.*;
3+
import java.util.HashMap;
4+
import java.util.HashSet;
5+
import java.util.Map;
6+
import java.util.Set;
47
import java.util.stream.Collectors;
58

69
import io.fabric8.kubernetes.api.model.ConfigMap;
710
import io.fabric8.kubernetes.api.model.ConfigMapBuilder;
811
import io.fabric8.kubernetes.api.model.ObjectMetaBuilder;
912
import io.javaoperatorsdk.operator.api.reconciler.Context;
10-
import io.javaoperatorsdk.operator.processing.dependent.*;
13+
import io.javaoperatorsdk.operator.processing.dependent.BulkDependentResource;
14+
import io.javaoperatorsdk.operator.processing.dependent.BulkUpdater;
15+
import io.javaoperatorsdk.operator.processing.dependent.DependentResourceWithExplicitState;
16+
import io.javaoperatorsdk.operator.processing.dependent.Matcher;
1117
import io.javaoperatorsdk.operator.processing.dependent.external.PerResourcePollingDependentResource;
1218
import io.javaoperatorsdk.operator.support.ExternalIDGenServiceMock;
1319
import io.javaoperatorsdk.operator.support.ExternalResource;
@@ -36,10 +42,10 @@ public Set<ExternalResource> fetchResources(
3642
getExternalStateEventSource().getSecondaryResources(primaryResource);
3743
Set<ExternalResource> res = new HashSet<>();
3844

39-
configMaps.stream().forEach(cm -> {
45+
configMaps.forEach(cm -> {
4046
var id = cm.getData().get(ID_KEY);
4147
var externalResource = externalService.read(id);
42-
externalResource.ifPresent(er -> res.add(er));
48+
externalResource.ifPresent(res::add);
4349
});
4450
return res;
4551
}

0 commit comments

Comments
 (0)