Skip to content

feat: workflow Integration with API (dependent annotations, context) #1257

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 36 commits into from
Jun 9, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
36 commits
Select commit Hold shift + click to select a range
2d3c111
feat: annotation config for workflow
csviri May 26, 2022
7c40e1b
wip
csviri May 30, 2022
5c62702
impl skeleton
csviri May 31, 2022
e344e77
wip
csviri May 31, 2022
7ca2a9f
untit test for workflow utils
csviri Jun 1, 2022
6a91a5d
workflow api integration
csviri Jun 1, 2022
6916bb4
fixes
csviri Jun 1, 2022
b45a0e3
it fix
csviri Jun 1, 2022
6af09ff
IT fix
csviri Jun 1, 2022
88d81d0
renaming
csviri Jun 2, 2022
9fb2ffb
refactors, tests
csviri Jun 2, 2022
6e6d47a
wip
csviri Jun 2, 2022
8e08769
additional unit tests
csviri Jun 2, 2022
0e3693b
format
csviri Jun 2, 2022
01b6ffd
updated samples
csviri Jun 2, 2022
8d26095
IT wip
csviri Jun 2, 2022
c8fe351
IT wip
csviri Jun 3, 2022
29c0062
test, and fixis with the consig service provider in tests
csviri Jun 6, 2022
a3c203d
renaming conditions
csviri Jun 6, 2022
83aa6a5
sonar fixes
csviri Jun 7, 2022
36b2501
sonar issue
csviri Jun 7, 2022
639c414
refactor: avoid unneeded object creation
metacosm Jun 8, 2022
682072c
refactor: improve readability
metacosm Jun 8, 2022
a654a74
refactor: checkForNameDuplication can now report all duplicates
metacosm Jun 8, 2022
4b41176
refactor: rename more appropriately
metacosm Jun 8, 2022
e3771f2
refactor: no need for mock, makes things faster & easier to debug
metacosm Jun 8, 2022
b3dbfbd
chore(tests): add more tests
metacosm Jun 8, 2022
ff71f3b
refactor: simplify and make orderAndDetectCycles more readable
metacosm Jun 8, 2022
18bdddf
refactor: avoid creating a ManagedWorkflow instance when not needed
metacosm Jun 8, 2022
4b01cb8
fix: rename test to reflect use case
metacosm Jun 8, 2022
ad988b1
chore: format
metacosm Jun 8, 2022
3f12362
ˆ
metacosm Jun 9, 2022
f8d3c16
fix: possible NPE
metacosm Jun 9, 2022
9156441
moved javadoc
csviri Jun 9, 2022
0b7dc97
Merge branch 'annotation-workflow' of github.com:java-operator-sdk/ja…
csviri Jun 9, 2022
e951058
comment in test
csviri Jun 9, 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
@@ -1,5 +1,6 @@
package io.javaoperatorsdk.operator.api.config;

import java.lang.reflect.InvocationTargetException;
import java.time.Duration;
import java.util.Arrays;
import java.util.Collections;
Expand All @@ -19,9 +20,11 @@
import io.javaoperatorsdk.operator.api.reconciler.Reconciler;
import io.javaoperatorsdk.operator.api.reconciler.dependent.Dependent;
import io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResource;
import io.javaoperatorsdk.operator.api.reconciler.dependent.VoidCondition;
import io.javaoperatorsdk.operator.processing.dependent.kubernetes.KubernetesDependent;
import io.javaoperatorsdk.operator.processing.dependent.kubernetes.KubernetesDependentResource;
import io.javaoperatorsdk.operator.processing.dependent.kubernetes.KubernetesDependentResourceConfig;
import io.javaoperatorsdk.operator.processing.dependent.workflow.Condition;
import io.javaoperatorsdk.operator.processing.event.source.controller.ResourceEventFilter;
import io.javaoperatorsdk.operator.processing.event.source.controller.ResourceEventFilters;

Expand Down Expand Up @@ -166,19 +169,46 @@ public List<DependentResourceSpec> getDependentResources() {
}

final var name = getName(dependent, dependentType);
final var spec = specsMap.get(name);
var spec = specsMap.get(name);
if (spec != null) {
throw new IllegalArgumentException(
"A DependentResource named: " + name + " already exists: " + spec);
}
specsMap.put(name, new DependentResourceSpec(dependentType, config, name));
spec = new DependentResourceSpec(dependentType, config, name);
spec.setDependsOn(Set.of(dependent.dependsOn()));
addConditions(spec, dependent);
specsMap.put(name, spec);
}

specs = specsMap.values().stream().collect(Collectors.toUnmodifiableList());
}
return specs;
}

@SuppressWarnings("unchecked")
private void addConditions(DependentResourceSpec spec, Dependent dependent) {
if (dependent.deletePostcondition() != VoidCondition.class) {
spec.setDeletePostCondition(instantiateCondition(dependent.deletePostcondition()));
}
if (dependent.readyPostcondition() != VoidCondition.class) {
spec.setReadyPostcondition(instantiateCondition(dependent.readyPostcondition()));
}
if (dependent.reconcilePrecondition() != VoidCondition.class) {
spec.setReconcilePrecondition(instantiateCondition(dependent.reconcilePrecondition()));
}
}

private Condition<?, ?> instantiateCondition(Class<? extends Condition> condition) {
try {
return condition.getDeclaredConstructor().newInstance();
} catch (InstantiationException
| IllegalAccessException
| InvocationTargetException
| NoSuchMethodException e) {
throw new OperatorException(e);
}
}

private String getName(Dependent dependent, Class<? extends DependentResource> dependentType) {
var name = dependent.name();
if (name.isBlank()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,8 @@
* to the ConfigurationService is via the reconciliation context.
*/
public class ConfigurationServiceProvider {
static final ConfigurationService DEFAULT =
new BaseConfigurationService(Utils.loadFromProperties());
private static ConfigurationService instance;
private static ConfigurationService defaultConfigurationService = DEFAULT;
private static ConfigurationService defaultConfigurationService = createDefault();
Copy link
Collaborator

Choose a reason for hiding this comment

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

What's the point of that change? See also comments on the associated test class…

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, this solves an issue with the integration tests, basically since it is singleton, it needs to be reseted after the integration tests. But if the DEFAULT is not created again (with this method in reset) , the configurations are for dependents stay in there and causes trouble. This is just for tests.

see: https://github.com/java-operator-sdk/java-operator-sdk/blob/0b7dc97de05df069e2db87384a9d1f67b3e79724/operator-framework-junit5/src/main/java/io/javaoperatorsdk/operator/junit/AbstractOperatorExtension.java#L165-L165

Copy link
Collaborator

Choose a reason for hiding this comment

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

🤔 I guess the current reset implementation is indeed wrong…

private static boolean alreadyConfigured = false;

private ConfigurationServiceProvider() {}
Expand Down Expand Up @@ -64,8 +62,12 @@ synchronized static ConfigurationService getDefault() {
}

public synchronized static void reset() {
defaultConfigurationService = DEFAULT;
defaultConfigurationService = createDefault();
instance = null;
alreadyConfigured = false;
}

static ConfigurationService createDefault() {
return new BaseConfigurationService(Utils.loadFromProperties());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -140,11 +140,10 @@ public ControllerConfiguration<R> build() {
// if the spec has a config and it's a KubernetesDependentResourceConfig, update the
// namespaces if needed, otherwise, just return the existing spec
final Optional<?> maybeConfig = spec.getDependentResourceConfiguration();
final Class<?> drClass = drsEntry.getValue().getDependentResourceClass();
return maybeConfig.filter(KubernetesDependentResourceConfig.class::isInstance)
.map(KubernetesDependentResourceConfig.class::cast)
.filter(Predicate.not(KubernetesDependentResourceConfig::wereNamespacesConfigured))
.map(c -> updateSpec(drsEntry.getKey(), drClass, c))
.map(c -> updateSpec(drsEntry.getKey(), spec, c))
.orElse(drsEntry.getValue());
}).collect(Collectors.toUnmodifiableList());

Expand All @@ -164,9 +163,15 @@ public ControllerConfiguration<R> build() {
}

@SuppressWarnings({"rawtypes", "unchecked"})
private DependentResourceSpec<?, ?> updateSpec(String name, Class<?> drClass,
private DependentResourceSpec<?, ?> updateSpec(String name, DependentResourceSpec spec,
KubernetesDependentResourceConfig c) {
return new DependentResourceSpec(drClass, c.setNamespaces(namespaces), name);
var res = new DependentResourceSpec(spec.getDependentResourceClass(),
c.setNamespaces(namespaces), name);
res.setReadyPostcondition(spec.getReadyCondition());
res.setReconcilePrecondition(spec.getReconcileCondition());
res.setDeletePostCondition(spec.getDeletePostCondition());
res.setDependsOn(spec.getDependsOn());
return res;
}

public static <R extends HasMetadata> ControllerConfigurationOverrider<R> override(
Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,12 @@
package io.javaoperatorsdk.operator.api.config.dependent;

import java.util.HashSet;
import java.util.Objects;
import java.util.Optional;
import java.util.Set;

import io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResource;
import io.javaoperatorsdk.operator.processing.dependent.workflow.Condition;

public class DependentResourceSpec<T extends DependentResource<?, ?>, C> {

Expand All @@ -13,6 +16,14 @@ public class DependentResourceSpec<T extends DependentResource<?, ?>, C> {

private final String name;

private Set<String> dependsOn;

private Condition<?, ?> readyCondition;

private Condition<?, ?> reconcileCondition;

private Condition<?, ?> deletePostCondition;

public DependentResourceSpec(Class<T> dependentResourceClass, C dependentResourceConfig,
String name) {
this.dependentResourceClass = dependentResourceClass;
Expand Down Expand Up @@ -55,4 +66,46 @@ public boolean equals(Object o) {
public int hashCode() {
return Objects.hash(name);
}

public Set<String> getDependsOn() {
if (dependsOn == null) {
dependsOn = new HashSet<>(0);
}
return dependsOn;
}

public DependentResourceSpec<T, C> setDependsOn(Set<String> dependsOn) {
this.dependsOn = dependsOn;
return this;
}

@SuppressWarnings("rawtypes")
public Condition getReadyCondition() {
return readyCondition;
}

public DependentResourceSpec<T, C> setReadyPostcondition(Condition<?, ?> readyCondition) {
this.readyCondition = readyCondition;
return this;
}

@SuppressWarnings("rawtypes")
public Condition getReconcileCondition() {
return reconcileCondition;
}

public DependentResourceSpec<T, C> setReconcilePrecondition(Condition<?, ?> reconcileCondition) {
this.reconcileCondition = reconcileCondition;
return this;
}

@SuppressWarnings("rawtypes")
public Condition getDeletePostCondition() {
return deletePostCondition;
}

public DependentResourceSpec<T, C> setDeletePostCondition(Condition<?, ?> deletePostCondition) {
this.deletePostCondition = deletePostCondition;
return this;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
public interface Cleaner<P extends HasMetadata> {

/**
* Note that this method turns on automatic finalizer usage.
* This method turns on automatic finalizer usage.
*
* The implementation should delete the associated component(s). This method is called when an
* object is marked for deletion. After it's executed the custom resource finalizer is
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

import io.fabric8.kubernetes.api.model.HasMetadata;
import io.javaoperatorsdk.operator.api.config.ControllerConfiguration;
import io.javaoperatorsdk.operator.api.reconciler.dependent.managed.DefaultManagedDependentResourceContext;
import io.javaoperatorsdk.operator.api.reconciler.dependent.managed.ManagedDependentResourceContext;
import io.javaoperatorsdk.operator.processing.Controller;

Expand All @@ -15,14 +16,14 @@ public class DefaultContext<P extends HasMetadata> implements Context<P> {
private final Controller<P> controller;
private final P primaryResource;
private final ControllerConfiguration<P> controllerConfiguration;
private final ManagedDependentResourceContext managedDependentResourceContext;
private final DefaultManagedDependentResourceContext defaultManagedDependentResourceContext;

public DefaultContext(RetryInfo retryInfo, Controller<P> controller, P primaryResource) {
this.retryInfo = retryInfo;
this.controller = controller;
this.primaryResource = primaryResource;
this.controllerConfiguration = controller.getConfiguration();
this.managedDependentResourceContext = new ManagedDependentResourceContext();
this.defaultManagedDependentResourceContext = new DefaultManagedDependentResourceContext();
}

@Override
Expand Down Expand Up @@ -52,8 +53,9 @@ public ControllerConfiguration<P> getControllerConfiguration() {
return controllerConfiguration;
}

@Override
public ManagedDependentResourceContext managedDependentResourceContext() {
return managedDependentResourceContext;
return defaultManagedDependentResourceContext;
}

public DefaultContext<P> setRetryInfo(RetryInfo retryInfo) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package io.javaoperatorsdk.operator.api.reconciler.dependent;

import io.javaoperatorsdk.operator.processing.dependent.workflow.Condition;

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

public @interface Dependent {
Expand All @@ -8,4 +10,15 @@
Class<? extends DependentResource> type();

String name() default NO_VALUE_SET;

@SuppressWarnings("rawtypes")
Class<? extends Condition> readyPostcondition() default VoidCondition.class;

@SuppressWarnings("rawtypes")
Class<? extends Condition> reconcilePrecondition() default VoidCondition.class;

@SuppressWarnings("rawtypes")
Class<? extends Condition> deletePostcondition() default VoidCondition.class;

String[] dependsOn() default {};
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
package io.javaoperatorsdk.operator.api.reconciler.dependent;

import io.fabric8.kubernetes.api.model.HasMetadata;
import io.javaoperatorsdk.operator.api.reconciler.Context;
import io.javaoperatorsdk.operator.processing.dependent.workflow.Condition;

/** Used as default value for Condition in annotations */
@SuppressWarnings("rawtypes")
public class VoidCondition implements Condition {
@Override
public boolean isMet(DependentResource dependentResource, HasMetadata primary, Context context) {
throw new IllegalStateException("This is a placeholder class, should not be called");
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
package io.javaoperatorsdk.operator.api.reconciler.dependent.managed;

import java.util.Optional;
import java.util.concurrent.ConcurrentHashMap;

import io.javaoperatorsdk.operator.processing.dependent.workflow.WorkflowCleanupResult;
import io.javaoperatorsdk.operator.processing.dependent.workflow.WorkflowReconcileResult;

@SuppressWarnings("rawtypes")
public class DefaultManagedDependentResourceContext implements ManagedDependentResourceContext {

private WorkflowReconcileResult workflowReconcileResult;
private WorkflowCleanupResult workflowCleanupResult;
private final ConcurrentHashMap attributes = new ConcurrentHashMap();

@Override
public <T> Optional<T> get(Object key, Class<T> expectedType) {
return Optional.ofNullable(attributes.get(key))
.filter(expectedType::isInstance)
.map(expectedType::cast);
}

@Override
@SuppressWarnings("unchecked")
public <T> T put(Object key, T value) {
if (value == null) {
return (T) Optional.ofNullable(attributes.remove(key));
}
return (T) Optional.ofNullable(attributes.put(key, value));
}

@Override
@SuppressWarnings("unused")
public <T> T getMandatory(Object key, Class<T> expectedType) {
return get(key, expectedType).orElseThrow(() -> new IllegalStateException(
"Mandatory attribute (key: " + key + ", type: " + expectedType.getName()
+ ") is missing or not of the expected type"));
}

public DefaultManagedDependentResourceContext setWorkflowExecutionResult(
WorkflowReconcileResult workflowReconcileResult) {
this.workflowReconcileResult = workflowReconcileResult;
return this;
}

public DefaultManagedDependentResourceContext setWorkflowCleanupResult(
WorkflowCleanupResult workflowCleanupResult) {
this.workflowCleanupResult = workflowCleanupResult;
return this;
}

@Override
public Optional<WorkflowReconcileResult> getWorkflowReconcileResult() {
return Optional.ofNullable(workflowReconcileResult);
}

@Override
public Optional<WorkflowCleanupResult> getWorkflowCleanupResult() {
return Optional.ofNullable(workflowCleanupResult);
}
}
Loading