Skip to content

feat: provide more exception context on workflow errors #1699

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Jan 5, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion docs/documentation/patterns-best-practices.md
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ done automatically, the whole process can be completely transparent.
## Managing State

Thanks to the declarative nature of Kubernetes resources, operators that deal only with
Kubernetes resources can operator in a stateless fashion, i.e. they do not need to maintain
Kubernetes resources can operate in a stateless fashion, i.e. they do not need to maintain
information about the state of these resources, as it should be possible to completely rebuild
the resource state from its representation (that's what declarative means, after all).
However, this usually doesn't hold true anymore when dealing with external resources, and it
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
package io.javaoperatorsdk.operator;

import java.io.PrintWriter;
import java.io.StringWriter;
import java.util.Collections;
import java.util.Map;
import java.util.Map.Entry;
import java.util.stream.Collectors;

public class AggregatedOperatorException extends OperatorException {
Expand All @@ -22,7 +25,15 @@ public Map<String, Exception> getAggregatedExceptions() {
@Override
public String getMessage() {
return super.getMessage() + " " + causes.entrySet().stream()
.map(entry -> entry.getKey() + " -> " + entry.getValue())
.collect(Collectors.joining("\n - ", "Details:\n", ""));
.map(entry -> entry.getKey() + " -> " + exceptionDescription(entry))
.collect(Collectors.joining("\n - ", "Details:\n - ", ""));
}

private static String exceptionDescription(Entry<String, Exception> entry) {
final var exception = entry.getValue();
final var out = new StringWriter(2000);
final var stringWriter = new PrintWriter(out);
exception.printStackTrace(stringWriter);
return out.toString();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -60,8 +60,7 @@ static <K extends HasMetadata> Map<String, EventSource> nameEventSourcesFromDepe
}

/**
* This is for the use case when the event sources are not access explicitly by name in the
* reconciler.
* Used when event sources are not explicitly named when created/registered.
*
* @param eventSource EventSource
* @return generated name
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,12 +32,12 @@ public interface DependentResource<R, P extends HasMetadata> {
Class<R> resourceType();

/**
* Dependent resources are designed to by default provide event sources. There are cases where it
* might not:
* Dependent resources are designed to by default provide event sources. There are cases where
* they might not:
* <ul>
* <li>If an event source is shared between multiple dependent resources. In this case only one or
* none of the dependent resources sharing the event source should provide one.</li>
* <li>Some special implementation of an event source. That just execute some action might not
* none of the dependent resources sharing the event source should provide one, if any.</li>
* <li>Some special implementation of an event source that just executes some action might not
* provide one.</li>
* </ul>
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,14 +33,11 @@ protected AbstractExternalDependentResource(Class<R> resourceType) {
public void resolveEventSource(EventSourceRetriever<P> eventSourceRetriever) {
super.resolveEventSource(eventSourceRetriever);
if (isDependentResourceWithExplicitState) {
externalStateEventSource = (InformerEventSource<?, P>) dependentResourceWithExplicitState
.eventSourceName()
.map(n -> eventSourceRetriever
.getResourceEventSourceFor(dependentResourceWithExplicitState.stateResourceClass(),
(String) n))
.orElseGet(() -> eventSourceRetriever
.getResourceEventSourceFor(
(Class<R>) dependentResourceWithExplicitState.stateResourceClass()));
final var eventSourceName = (String) dependentResourceWithExplicitState
.eventSourceName().orElse(null);
externalStateEventSource = (InformerEventSource<?, P>) eventSourceRetriever
.getResourceEventSourceFor(dependentResourceWithExplicitState.stateResourceClass(),
eventSourceName);
}

}
Expand All @@ -65,13 +62,13 @@ public void delete(P primary, Context<P> context) {
}
}

@SuppressWarnings("unchecked")
@SuppressWarnings({"unchecked", "unused"})
private void handleExplicitStateDelete(P primary, R secondary, Context<P> context) {
var res = dependentResourceWithExplicitState.stateResource(primary, secondary);
dependentResourceWithExplicitState.getKubernetesClient().resource(res).delete();
}

@SuppressWarnings({"rawtypes", "unchecked"})
@SuppressWarnings({"rawtypes", "unchecked", "unused"})
protected void handleExplicitStateCreation(P primary, R created, Context<P> context) {
var resource = dependentResourceWithExplicitState.stateResource(primary, created);
var stateResource =
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
package io.javaoperatorsdk.operator.processing.event;

import java.util.*;
import java.util.LinkedHashSet;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Set;
import java.util.function.Function;
Expand Down Expand Up @@ -213,11 +213,6 @@ public ControllerResourceEventSource<P> getControllerResourceEventSource() {
return eventSources.controllerResourceEventSource();
}

@Override
public <R> ResourceEventSource<R, P> getResourceEventSourceFor(Class<R> dependentType) {
return getResourceEventSourceFor(dependentType, null);
}

public <R> List<ResourceEventSource<R, P>> getResourceEventSourcesFor(Class<R> dependentType) {
return eventSources.getEventSources(dependentType);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,11 @@

public interface EventSourceRetriever<P extends HasMetadata> {

<R> ResourceEventSource<R, P> getResourceEventSourceFor(
Class<R> dependentType);
default <R> ResourceEventSource<R, P> getResourceEventSourceFor(Class<R> dependentType) {
return getResourceEventSourceFor(dependentType, null);
}

<R> ResourceEventSource<R, P> getResourceEventSourceFor(
Class<R> dependentType, String name);
<R> ResourceEventSource<R, P> getResourceEventSourceFor(Class<R> dependentType, String name);

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

Expand Down
Original file line number Diff line number Diff line change
@@ -1,13 +1,19 @@
package io.javaoperatorsdk.operator.sample.externalstate.externalstatebulkdependent;

import java.util.*;
import java.util.HashMap;
import java.util.HashSet;
import java.util.Map;
import java.util.Set;
import java.util.stream.Collectors;

import io.fabric8.kubernetes.api.model.ConfigMap;
import io.fabric8.kubernetes.api.model.ConfigMapBuilder;
import io.fabric8.kubernetes.api.model.ObjectMetaBuilder;
import io.javaoperatorsdk.operator.api.reconciler.Context;
import io.javaoperatorsdk.operator.processing.dependent.*;
import io.javaoperatorsdk.operator.processing.dependent.BulkDependentResource;
import io.javaoperatorsdk.operator.processing.dependent.BulkUpdater;
import io.javaoperatorsdk.operator.processing.dependent.DependentResourceWithExplicitState;
import io.javaoperatorsdk.operator.processing.dependent.Matcher;
import io.javaoperatorsdk.operator.processing.dependent.external.PerResourcePollingDependentResource;
import io.javaoperatorsdk.operator.support.ExternalIDGenServiceMock;
import io.javaoperatorsdk.operator.support.ExternalResource;
Expand Down Expand Up @@ -36,10 +42,10 @@ public Set<ExternalResource> fetchResources(
getExternalStateEventSource().getSecondaryResources(primaryResource);
Set<ExternalResource> res = new HashSet<>();

configMaps.stream().forEach(cm -> {
configMaps.forEach(cm -> {
var id = cm.getData().get(ID_KEY);
var externalResource = externalService.read(id);
externalResource.ifPresent(er -> res.add(er));
externalResource.ifPresent(res::add);
});
return res;
}
Expand Down