From 100b4dab01dd098b06bbea6cd16af4bfea26b932 Mon Sep 17 00:00:00 2001 From: Chris Laprun Date: Wed, 4 Jan 2023 22:03:15 +0100 Subject: [PATCH 1/3] feat: provide more exception context on workflow errors --- .../operator/AggregatedOperatorException.java | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/AggregatedOperatorException.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/AggregatedOperatorException.java index db27ce50a3..2fdc80d606 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/AggregatedOperatorException.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/AggregatedOperatorException.java @@ -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 { @@ -22,7 +25,15 @@ public Map 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 entry) { + final var exception = entry.getValue(); + final var out = new StringWriter(2000); + final var stringWriter = new PrintWriter(out); + exception.printStackTrace(stringWriter); + return out.toString(); } } From 1c60fd0880ba70f313be133c0dfa28ee03f0d205 Mon Sep 17 00:00:00 2001 From: Chris Laprun Date: Wed, 4 Jan 2023 22:04:48 +0100 Subject: [PATCH 2/3] refactor: simplify --- .../AbstractExternalDependentResource.java | 17 +++++++---------- .../processing/event/EventSourceManager.java | 7 +------ .../processing/event/EventSourceRetriever.java | 8 ++++---- .../BulkDependentResourceExternalWithState.java | 14 ++++++++++---- 4 files changed, 22 insertions(+), 24 deletions(-) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/AbstractExternalDependentResource.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/AbstractExternalDependentResource.java index e3dd9c630a..18e6f41a92 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/AbstractExternalDependentResource.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/AbstractExternalDependentResource.java @@ -33,14 +33,11 @@ protected AbstractExternalDependentResource(Class resourceType) { public void resolveEventSource(EventSourceRetriever

eventSourceRetriever) { super.resolveEventSource(eventSourceRetriever); if (isDependentResourceWithExplicitState) { - externalStateEventSource = (InformerEventSource) dependentResourceWithExplicitState - .eventSourceName() - .map(n -> eventSourceRetriever - .getResourceEventSourceFor(dependentResourceWithExplicitState.stateResourceClass(), - (String) n)) - .orElseGet(() -> eventSourceRetriever - .getResourceEventSourceFor( - (Class) dependentResourceWithExplicitState.stateResourceClass())); + final var eventSourceName = (String) dependentResourceWithExplicitState + .eventSourceName().orElse(null); + externalStateEventSource = (InformerEventSource) eventSourceRetriever + .getResourceEventSourceFor(dependentResourceWithExplicitState.stateResourceClass(), + eventSourceName); } } @@ -65,13 +62,13 @@ public void delete(P primary, Context

context) { } } - @SuppressWarnings("unchecked") + @SuppressWarnings({"unchecked", "unused"}) private void handleExplicitStateDelete(P primary, R secondary, Context

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

context) { var resource = dependentResourceWithExplicitState.stateResource(primary, created); var stateResource = diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventSourceManager.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventSourceManager.java index 6a6aae471a..97ff150126 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventSourceManager.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventSourceManager.java @@ -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; @@ -213,11 +213,6 @@ public ControllerResourceEventSource

getControllerResourceEventSource() { return eventSources.controllerResourceEventSource(); } - @Override - public ResourceEventSource getResourceEventSourceFor(Class dependentType) { - return getResourceEventSourceFor(dependentType, null); - } - public List> getResourceEventSourcesFor(Class dependentType) { return eventSources.getEventSources(dependentType); } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventSourceRetriever.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventSourceRetriever.java index 421092fb6a..a37f35531f 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventSourceRetriever.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventSourceRetriever.java @@ -7,11 +7,11 @@ public interface EventSourceRetriever

{ - ResourceEventSource getResourceEventSourceFor( - Class dependentType); + default ResourceEventSource getResourceEventSourceFor(Class dependentType) { + return getResourceEventSourceFor(dependentType, null); + } - ResourceEventSource getResourceEventSourceFor( - Class dependentType, String name); + ResourceEventSource getResourceEventSourceFor(Class dependentType, String name); List> getResourceEventSourcesFor(Class dependentType); diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/externalstate/externalstatebulkdependent/BulkDependentResourceExternalWithState.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/externalstate/externalstatebulkdependent/BulkDependentResourceExternalWithState.java index 5344810609..37684dd39f 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/externalstate/externalstatebulkdependent/BulkDependentResourceExternalWithState.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/externalstate/externalstatebulkdependent/BulkDependentResourceExternalWithState.java @@ -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; @@ -36,10 +42,10 @@ public Set fetchResources( getExternalStateEventSource().getSecondaryResources(primaryResource); Set 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; } From 9517068e3d837fe7eab9df2b22a8840b3cfffc86 Mon Sep 17 00:00:00 2001 From: Chris Laprun Date: Wed, 4 Jan 2023 22:05:12 +0100 Subject: [PATCH 3/3] docs: improve wording --- docs/documentation/patterns-best-practices.md | 2 +- .../operator/api/reconciler/EventSourceInitializer.java | 3 +-- .../api/reconciler/dependent/DependentResource.java | 8 ++++---- 3 files changed, 6 insertions(+), 7 deletions(-) diff --git a/docs/documentation/patterns-best-practices.md b/docs/documentation/patterns-best-practices.md index 6a5f729fcd..06ea101581 100644 --- a/docs/documentation/patterns-best-practices.md +++ b/docs/documentation/patterns-best-practices.md @@ -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 diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/EventSourceInitializer.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/EventSourceInitializer.java index 017418ea35..2c94840d38 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/EventSourceInitializer.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/EventSourceInitializer.java @@ -60,8 +60,7 @@ static Map 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 diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/dependent/DependentResource.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/dependent/DependentResource.java index d522a30151..affeb96bbf 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/dependent/DependentResource.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/dependent/DependentResource.java @@ -32,12 +32,12 @@ public interface DependentResource { Class 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: *

    *
  • 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.
    • - *
  • 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.
    • + *
  • Some special implementation of an event source that just executes some action might not * provide one.
    • *
*