Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
cbd1211
fix: retrieve DependentResource based on name instead of class
metacosm Mar 21, 2022
3ec87e8
feat: record association of name and DependentResource in configuration
metacosm Mar 23, 2022
f49eace
fix: make it possible to override dependent specs
metacosm Mar 23, 2022
5b0801f
chore: add more tests
metacosm Mar 23, 2022
8ecf536
refactor: rename replaceNamedDependentResourceConfig to fit other met…
metacosm Mar 23, 2022
684289e
feat: make it possible to retrieve actual dependent resource type
metacosm Mar 23, 2022
b7b3593
docs: add javadoc
metacosm Mar 23, 2022
81cc3f8
fix: use proper name to retrieve SchemaDependentResource
metacosm Mar 23, 2022
3168db7
fix: change logging level since these are low-level logs
metacosm Mar 23, 2022
5f5ca2d
feat: add simplified logging for reconcile logic
metacosm Mar 23, 2022
bc58211
feat: EventSource are now registered with a name
metacosm Mar 24, 2022
c837a9e
feat: ReconcileResult can now report errors as well
metacosm Mar 24, 2022
c8b6dbc
feat: log dependent reconciliation result
metacosm Mar 24, 2022
dbaaa2b
refactor: use context instead of retrieving the dependent resource
metacosm Mar 24, 2022
3e321eb
feat: record ReconcileResult per DependentResource in context
metacosm Mar 24, 2022
ecac6c4
refactor: remove DependentResource access from managed context
metacosm Mar 25, 2022
3cbab93
feat: aggregate dependent exceptions instead of using error result
metacosm Mar 25, 2022
156a51a
refactor: extract inner classes from EventSourceManager
metacosm Mar 25, 2022
6eb1890
refactor: remove ResourceTypeAware interface
metacosm Mar 25, 2022
5cefd44
refactor: improve error reporting, remove redundant logging
metacosm Mar 26, 2022
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
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
package io.javaoperatorsdk.operator;

import java.util.Arrays;
import java.util.Collections;
import java.util.List;

public class AggregatedOperatorException extends OperatorException {
private final List<Exception> causes;

public AggregatedOperatorException(String message, Exception... exceptions) {
super(message);
this.causes = exceptions != null ? Arrays.asList(exceptions) : Collections.emptyList();
}

public AggregatedOperatorException(String message, List<Exception> exceptions) {
super(message);
this.causes = exceptions != null ? exceptions : Collections.emptyList();
}

public List<Exception> getAggregatedExceptions() {
return causes;
}
}
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
package io.javaoperatorsdk.operator.api.config;

import java.time.Duration;
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
import java.util.HashMap;
import java.util.Map;
import java.util.Optional;
import java.util.Set;
import java.util.function.Function;
Expand All @@ -26,7 +26,7 @@ public class AnnotationControllerConfiguration<R extends HasMetadata>

protected final Reconciler<R> reconciler;
private final ControllerConfiguration annotation;
private List<DependentResourceSpec<?, ?>> specs;
private Map<String, DependentResourceSpec<?, ?>> specs;

public AnnotationControllerConfiguration(Reconciler<R> reconciler) {
this.reconciler = reconciler;
Expand Down Expand Up @@ -135,15 +135,15 @@ public static <T> T valueOrDefault(

@SuppressWarnings({"rawtypes", "unchecked"})
@Override
public List<DependentResourceSpec<?, ?>> getDependentResources() {
public Map<String, DependentResourceSpec<?, ?>> getDependentResources() {
if (specs == null) {
final var dependents =
valueOrDefault(annotation, ControllerConfiguration::dependents, new Dependent[] {});
if (dependents.length == 0) {
return Collections.emptyList();
return Collections.emptyMap();
}

specs = new ArrayList<>(dependents.length);
specs = new HashMap<>(dependents.length);
for (Dependent dependent : dependents) {
Object config = null;
final Class<? extends DependentResource> dependentType = dependent.type();
Expand All @@ -159,9 +159,18 @@ public static <T> T valueOrDefault(
config =
new KubernetesDependentResourceConfig(namespaces, labelSelector);
}
specs.add(new DependentResourceSpec(dependentType, config));
var name = dependent.name();
if (name.isBlank()) {
name = DependentResource.defaultNameFor(dependentType);
}
final DependentResourceSpec<?, ?> spec = specs.get(name);
if (spec != null) {
throw new IllegalArgumentException(
"A DependentResource named: " + name + " already exists: " + spec);
}
specs.put(name, new DependentResourceSpec(dependentType, config, name));
}
}
return specs;
return Collections.unmodifiableMap(specs);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

import java.time.Duration;
import java.util.Collections;
import java.util.List;
import java.util.Map;
import java.util.Optional;

import io.fabric8.kubernetes.api.model.HasMetadata;
Expand Down Expand Up @@ -46,8 +46,8 @@ default ResourceEventFilter<R> getEventFilter() {
return ResourceEventFilters.passthrough();
}

default List<DependentResourceSpec<?, ?>> getDependentResources() {
return Collections.emptyList();
default Map<String, DependentResourceSpec<?, ?>> getDependentResources() {
return Collections.emptyMap();
}

default Optional<Duration> reconciliationMaxInterval() {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
package io.javaoperatorsdk.operator.api.config;

import java.time.Duration;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
import java.util.Optional;
import java.util.Map;
import java.util.Set;

import io.fabric8.kubernetes.api.model.HasMetadata;
import io.javaoperatorsdk.operator.api.config.dependent.DependentResourceSpec;
import io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResource;
import io.javaoperatorsdk.operator.processing.event.source.controller.ResourceEventFilter;

@SuppressWarnings({"rawtypes", "unchecked", "unused"})
Expand All @@ -22,7 +22,7 @@ public class ControllerConfigurationOverrider<R extends HasMetadata> {
private ResourceEventFilter<R> customResourcePredicate;
private final ControllerConfiguration<R> original;
private Duration reconciliationMaxInterval;
private final List<DependentResourceSpec<?, ?>> dependentResourceSpecs;
private final Map<String, DependentResourceSpec<?, ?>> dependentResourceSpecs;

private ControllerConfigurationOverrider(ControllerConfiguration<R> original) {
finalizer = original.getFinalizerName();
Expand All @@ -32,7 +32,8 @@ private ControllerConfigurationOverrider(ControllerConfiguration<R> original) {
labelSelector = original.getLabelSelector();
customResourcePredicate = original.getEventFilter();
reconciliationMaxInterval = original.reconciliationMaxInterval().orElse(null);
dependentResourceSpecs = original.getDependentResources();
// make the original specs modifiable
dependentResourceSpecs = new HashMap<>(original.getDependentResources());
this.original = original;
}

Expand Down Expand Up @@ -89,37 +90,17 @@ public ControllerConfigurationOverrider<R> withReconciliationMaxInterval(
return this;
}

public void replaceDependentResourceConfig(
Class<? extends DependentResource<?, R>> dependentResourceClass,
public ControllerConfigurationOverrider<R> replacingNamedDependentResourceConfig(String name,
Object dependentResourceConfig) {

var currentConfig =
findConfigForDependentResourceClass(dependentResourceClass);
if (currentConfig.isEmpty()) {
throw new IllegalStateException("Cannot find DependentResource config for class: "
+ dependentResourceClass);
}
dependentResourceSpecs.remove(currentConfig.get());
dependentResourceSpecs
.add(new DependentResourceSpec(dependentResourceClass, dependentResourceConfig));
}

public void addNewDependentResourceConfig(DependentResourceSpec dependentResourceSpec) {
var currentConfig =
findConfigForDependentResourceClass(dependentResourceSpec.getDependentResourceClass());
if (currentConfig.isPresent()) {
throw new IllegalStateException(
"Config already present for class: "
+ dependentResourceSpec.getDependentResourceClass());
final var currentConfig = dependentResourceSpecs.get(name);
if (currentConfig == null) {
throw new IllegalArgumentException("Cannot find a DependentResource named: " + name);
}
dependentResourceSpecs.add(dependentResourceSpec);
}

private Optional<DependentResourceSpec<?, ?>> findConfigForDependentResourceClass(
Class<? extends DependentResource> dependentResourceClass) {
return dependentResourceSpecs.stream()
.filter(dc -> dc.getDependentResourceClass().equals(dependentResourceClass))
.findFirst();
dependentResourceSpecs.remove(name);
dependentResourceSpecs.put(name,
new DependentResourceSpec(currentConfig.getDependentResourceClass(),
dependentResourceConfig, name));
return this;
}

public ControllerConfiguration<R> build() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,14 @@

import java.time.Duration;
import java.util.Collections;
import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.Set;

import io.fabric8.kubernetes.api.model.HasMetadata;
import io.javaoperatorsdk.operator.api.config.dependent.DependentResourceSpec;
import io.javaoperatorsdk.operator.processing.event.source.controller.ResourceEventFilter;

@SuppressWarnings("rawtypes")
public class DefaultControllerConfiguration<R extends HasMetadata>
extends DefaultResourceConfiguration<R>
implements ControllerConfiguration<R> {
Expand All @@ -22,7 +21,7 @@ public class DefaultControllerConfiguration<R extends HasMetadata>
private final boolean generationAware;
private final RetryConfiguration retryConfiguration;
private final ResourceEventFilter<R> resourceEventFilter;
private final List<DependentResourceSpec<?, ?>> dependents;
private final Map<String, DependentResourceSpec<?, ?>> dependents;
private final Duration reconciliationMaxInterval;

// NOSONAR constructor is meant to provide all information
Expand All @@ -38,7 +37,7 @@ public DefaultControllerConfiguration(
ResourceEventFilter<R> resourceEventFilter,
Class<R> resourceClass,
Duration reconciliationMaxInterval,
List<DependentResourceSpec<?, ?>> dependents) {
Map<String, DependentResourceSpec<?, ?>> dependents) {
super(labelSelector, resourceClass, namespaces);
this.associatedControllerClassName = associatedControllerClassName;
this.name = name;
Expand All @@ -52,7 +51,7 @@ public DefaultControllerConfiguration(
: retryConfiguration;
this.resourceEventFilter = resourceEventFilter;

this.dependents = dependents != null ? dependents : Collections.emptyList();
this.dependents = dependents != null ? dependents : Collections.emptyMap();
}

@Override
Expand Down Expand Up @@ -91,7 +90,7 @@ public ResourceEventFilter<R> getEventFilter() {
}

@Override
public List<DependentResourceSpec<?, ?>> getDependentResources() {
public Map<String, DependentResourceSpec<?, ?>> getDependentResources() {
return dependents;
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package io.javaoperatorsdk.operator.api.config.dependent;

import java.util.Objects;
import java.util.Optional;

import io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResource;
Expand All @@ -10,9 +11,13 @@ public class DependentResourceSpec<T extends DependentResource<?, ?>, C> {

private final C dependentResourceConfig;

public DependentResourceSpec(Class<T> dependentResourceClass, C dependentResourceConfig) {
private final String name;

public DependentResourceSpec(Class<T> dependentResourceClass, C dependentResourceConfig,
String name) {
this.dependentResourceClass = dependentResourceClass;
this.dependentResourceConfig = dependentResourceConfig;
this.name = name;
}

public Class<T> getDependentResourceClass() {
Expand All @@ -22,4 +27,32 @@ public Class<T> getDependentResourceClass() {
public Optional<C> getDependentResourceConfiguration() {
return Optional.ofNullable(dependentResourceConfig);
}

public String getName() {
return name;
}

@Override
public String toString() {
return "DependentResourceSpec{ name='" + name +
"', type=" + dependentResourceClass.getCanonicalName() +
", config=" + dependentResourceConfig + '}';
}

@Override
public boolean equals(Object o) {
if (this == o) {
return true;
}
if (o == null || getClass() != o.getClass()) {
return false;
}
DependentResourceSpec<?, ?> that = (DependentResourceSpec<?, ?>) o;
return name.equals(that.name);
}

@Override
public int hashCode() {
return Objects.hash(name);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,7 @@ public DefaultContext(RetryInfo retryInfo, Controller<P> controller, P primaryRe
this.controller = controller;
this.primaryResource = primaryResource;
this.controllerConfiguration = controller.getConfiguration();
this.managedDependentResourceContext = new ManagedDependentResourceContext(
controller.getDependents());
this.managedDependentResourceContext = new ManagedDependentResourceContext();
}

@Override
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
package io.javaoperatorsdk.operator.api.reconciler;

import java.util.List;
import java.util.Map;

import io.fabric8.kubernetes.api.model.HasMetadata;
import io.javaoperatorsdk.operator.processing.event.source.EventSource;
Expand All @@ -20,6 +20,6 @@ public interface EventSourceInitializer<P extends HasMetadata> {
* sources
* @return list of event sources to register
*/
List<EventSource> prepareEventSources(EventSourceContext<P> context);
Map<String, EventSource> prepareEventSources(EventSourceContext<P> context);

}
Original file line number Diff line number Diff line change
@@ -1,6 +1,11 @@
package io.javaoperatorsdk.operator.api.reconciler.dependent;

import static io.javaoperatorsdk.operator.api.reconciler.Constants.EMPTY_STRING;

public @interface Dependent {

@SuppressWarnings("rawtypes")
Class<? extends DependentResource> type();

String name() default EMPTY_STRING;
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,49 @@
import io.fabric8.kubernetes.api.model.HasMetadata;
import io.javaoperatorsdk.operator.api.reconciler.Context;

/**
* An interface to implement and provide dependent resource support.
*
* @param <R> the dependent resource type
* @param <P> the associated primary resource type
*/
public interface DependentResource<R, P extends HasMetadata> {

/**
* Reconciles the dependent resource given the desired primary state
*
* @param primary the primary resource for which we want to reconcile the dependent state
* @param context {@link Context} providing useful contextual information
* @return a {@link ReconcileResult} providing information about the reconciliation result
*/
ReconcileResult<R> reconcile(P primary, Context<P> context);

/**
* Retrieves the dependent resource associated with the specified primary one
*
* @param primaryResource the primary resource for which we want to retrieve the secondary
* resource
* @return an {@link Optional} containing the secondary resource or {@link Optional#empty()} if it
* doesn't exist
*/
Optional<R> getResource(P primaryResource);

/**
* Retrieves the resource type associated with this DependentResource
*
* @return the resource type associated with this DependentResource
*/
Class<R> resourceType();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why we need this at this level?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To make things explicit so that we don't need to rely on a default implementation that will not work in all situations.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mean are we sure that this will be needed in all cases?
I mean I can imaging a dependent resource implementation that does not need resource type.

I thought it is something that is required for quarkus.

I guess we can then delete ResourceTypeAware.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I'm conflicted on this… but the problem is that I haven't had time to spend completely focused on the extension so I'm not 100% sure about it.


/**
* Computes a default name for the specified DependentResource class
*
* @param dependentResourceClass the DependentResource class for which we want to compute a
* default name
* @return the default name for the specified DependentResource class
*/
@SuppressWarnings("rawtypes")
static String defaultNameFor(Class<? extends DependentResource> dependentResourceClass) {
return dependentResourceClass.getCanonicalName();
}
}
Loading