Skip to content

feat: introduce @Workflow annotation #2209

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.

Sign up for GitHub

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

Closed
wants to merge 11 commits into from
Closed

feat: introduce @Workflow annotation #2209

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
dd9dbfd
chore: set version to 5.0.0-SNAPSHOT (#2200)
csviri Jan 18, 2024
4887d6c
improve: java version minimal 11, tested on 21 (#2207)
csviri Jan 18, 2024
acdc0c8
improve: remove deprecated EventFilter (#2208)
csviri Jan 18, 2024
a68f96c
docs: 5.0 migration guide skeleton (#2210)
csviri Jan 19, 2024
b9e55bd
improve: managed dependent reconciliation results not optional (#2212)
csviri Jan 22, 2024
c85b9e0
improve: remove deprecated RetryConfiguration (#2211)
csviri Jan 22, 2024
a264b2a
feat: JDK client is not the default (#2235)
csviri Feb 6, 2024
e636956
fix: update SCM information (#2237)
metacosm Feb 7, 2024
51526e0
feat: introduce @Workflow annoation
csviri Jan 18, 2024
76cde32
docs
csviri Jan 23, 2024
0bc24ec
wip
csviri Feb 5, 2024
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 .github/workflows/integration-tests.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ on:
http-client:
type: string
required: false
default: 'okhttp'
default: 'jdk'
experimental:
type: boolean
required: false
Expand Down
4 changes: 2 additions & 2 deletions .github/workflows/pr.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ jobs:
integration_tests:
strategy:
matrix:
java: [ 11, 17 ]
java: [ 17, 21 ]
kubernetes: [ 'v1.26.13', 'v1.27.10', 'v1.28.6', 'v1.29.1' ]
uses: ./.github/workflows/integration-tests.yml
with:
Expand All @@ -56,7 +56,7 @@ jobs:
runs-on: ubuntu-latest
strategy:
matrix:
java: [ 11, 17 ]
java: [ 17, 21 ]
steps:
- uses: actions/checkout@v4
- name: Set up Java and Maven
Expand Down
4 changes: 2 additions & 2 deletions .github/workflows/release-project-in-dir.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ jobs:
- name: Set up Java and Maven
uses: actions/setup-java@v4
with:
java-version: 11
java-version: 17
distribution: temurin
cache: 'maven'

Expand Down Expand Up @@ -61,7 +61,7 @@ jobs:
- name: Set up Java and Maven
uses: actions/setup-java@v4
with:
java-version: 11
java-version: 17
distribution: temurin
cache: 'maven'

Expand Down
4 changes: 2 additions & 2 deletions .github/workflows/snapshot-releases.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ jobs:
uses: actions/setup-java@v4
with:
distribution: temurin
java-version: 11
java-version: 17
cache: 'maven'
- name: Build and test project
run: ./mvnw ${MAVEN_ARGS} clean install --file pom.xml
Expand All @@ -34,7 +34,7 @@ jobs:
uses: actions/setup-java@v4
with:
distribution: temurin
java-version: 11
java-version: 17
cache: 'maven'
- name: Release Maven package
uses: samuelmeuli/action-maven-publish@v1
Expand Down
2 changes: 1 addition & 1 deletion bootstrapper-maven-plugin/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
<parent>
<artifactId>java-operator-sdk</artifactId>
<groupId>io.javaoperatorsdk</groupId>
<version>4.7.2-SNAPSHOT</version>
<version>5.0.0-SNAPSHOT</version>
</parent>

<artifactId>bootstrapper</artifactId>
Expand Down
6 changes: 5 additions & 1 deletion bootstrapper-maven-plugin/src/main/resources/templates/Reconciler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,17 @@
import io.javaoperatorsdk.operator.api.reconciler.EventSourceContext;
import io.javaoperatorsdk.operator.api.reconciler.ControllerConfiguration;
import io.javaoperatorsdk.operator.api.reconciler.dependent.Dependent;
import io.javaoperatorsdk.operator.api.reconciler.workflow.Workflow;
import io.javaoperatorsdk.operator.processing.event.source.EventSource;
import io.javaoperatorsdk.operator.processing.event.source.informer.InformerEventSource;

import java.util.Map;
import java.util.Optional;

@ControllerConfiguration(dependents = {@Dependent(type = ConfigMapDependentResource.class)})
@ControllerConfiguration(
workflow = @Workflow(dependents = {
Copy link
Collaborator

Choose a reason for hiding this comment

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

TBH, I'm not convinced that this improves anything. This is more verbose and will require quite a bit of change in lots of places for non-obvious benefits.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is required also for this issue:
#1898

That is why it is not just a simple rename.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't follow how this is required for explicit invocation of workflows. There's an implicit workflow being created already so adding an annotation doesn't bring much unless we would like to have data specifically associated with the workflow. That said, even workflow data could be added to the ControllerConfiguration annotation. Adding a new annotation would only make sense if we envisioned to have lots of data associated with workflows and/or if we wanted to have several workflows with different data set, which I don't think we should in either cases.

Copy link
Collaborator Author

@csviri csviri Feb 13, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

There are multiple aspects of this, why I think this is really beneficial:

  1. what is probably less important is just better domain language, when talking to devs usually they don't get that in the background there is this Workflow that is executes the Dependent resources, so would be nice to have both in the domain language.

  2. Features like this:

@ControllerConfiguration(
    workflow = @Workflow( 
          explicitInvocation = true,
          dependents = {
              @Dependent(...),
              @Dependent(...)}
    )

In the mentioned link, this is closely related to the workflow, and how it is invoked. It is much nicer to have such configs / feature options put there since its is coupled with the workflow not the controller configuration directly.
I expect that more of such will come in the future, so it is kinda future compatibility also.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I don't follow how this is required for explicit invocation of workflows.

So if there is no flag like mentioned how can the controller know if the workflow should be executed before reconcile method is invoked or it will be invoked manually / explicitly.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't follow how this is required for explicit invocation of workflows.

So if there is no flag like mentioned how can the controller know if the workflow should be executed before reconcile method is invoked or it will be invoked manually / explicitly.

By simply having an explicitWorkflowInvocation attribute on the ControllerConfiguration annotation without the need for an intermediate annotation.

Copy link
Collaborator

Choose a reason for hiding this comment

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

There are multiple aspects of this, why I think this is really beneficial:

1. what is probably less important is just better domain language, when talking to devs usually they don't get that in the background there is this Workflow that is executes the Dependent resources, so would be nice to have both in the domain language.

Sure but this could be addressed by better documentation 😄

2. Features like [this](https://github.com/operator-framework/java-operator-sdk/issues/1898):

@ControllerConfiguration(
    workflow = @Workflow( 
          explicitInvocation = true,
          dependents = {
              @Dependent(...),
              @Dependent(...)}
    )

In the mentioned link, this is closely related to the workflow, and how it is invoked. It is much nicer to have such configs / feature options put there since its is coupled with the workflow not the controller configuration directly. I expect that more of such will come in the future, so it is kinda future compatibility also.

What other attributes would we be adding to the workflow configuration? I don't think that we should be adding complexity just for the sake of potential future features which might never materialize.

Copy link
Collaborator Author

@csviri csviri Feb 13, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

see also: #2238 (comment)

It is much nicer and descriptive to have these coupled.

Not sure what is wrong with an intermediate annotation. It is much nicer domain modeling, if we have hierachical structure.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sure but this could be addressed by better documentation 😄

I think we all agree if it is much better if the user does not have to dive into docs, just understands the code based how it is structured.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

What other attributes would we be adding to the workflow configuration? I don't think that we should be adding complexity just for the sake of potential future features which might never materialize.

So the explicit domain objects reduce cognitive burden imo, as mentioned, users will create a @Workflow and will see what are the related properties and config options. So they don't have to search for (and potintially miss it) in @ControllerConfiguratio

@Dependent(type = ConfigMapDependentResource.class)
}))
public class {{artifactClassId}}Reconciler implements Reconciler<{{artifactClassId}}CustomResource> {

public UpdateControl<{{artifactClassId}}CustomResource> reconcile({{artifactClassId}}CustomResource primary,
Expand Down
2 changes: 1 addition & 1 deletion caffeine-bounded-cache-support/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
<parent>
<artifactId>java-operator-sdk</artifactId>
<groupId>io.javaoperatorsdk</groupId>
<version>4.7.2-SNAPSHOT</version>
<version>5.0.0-SNAPSHOT</version>
</parent>
<modelVersion>4.0.0</modelVersion>

Expand Down
4 changes: 3 additions & 1 deletion docs/_data/sidebar.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,4 +32,6 @@
- title: Migrating from v4.3 to v4.4
url: /docs/v4-4-migration
- title: Migrating from v4.4 to v4.5
url: /docs/v4-5-migration
url: /docs/v4-5-migration
- title: Migrating from v4.7 to v5.0
url: /docs/v5-0-migration
45 changes: 45 additions & 0 deletions docs/documentation/v5-0-migration.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
---
title: Migrating from v4.7 to v5.0
description: Migrating from v4.7 to v5.0
layout: docs
permalink: /docs/v5-0-migration
---

# Migrating from v4.7 to v5.0

## API Tweaks

1. [Result of managed dependent resources](https://github.com/operator-framework/java-operator-sdk/blob/main/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/dependent/managed/ManagedDependentResourceContext.java#L55-L57)
is not `Optional` anymore. In case you use this result, simply use the result
objects directly.

2. Workflow is now explicit for managed dependent resources:
So instead (from WebPage sample):

```java

@ControllerConfiguration(
dependents = {
@Dependent(type = ConfigMapDependentResource.class),
@Dependent(type = DeploymentDependentResource.class),
@Dependent(type = ServiceDependentResource.class),
@Dependent(type = IngressDependentResource.class,
reconcilePrecondition = ExposedIngressCondition.class)
}))
// Omitted code
```


Now the following structure is used:

```java
@ControllerConfiguration(
workflow = @Workflow(dependents = {
@Dependent(type = ConfigMapDependentResource.class),
@Dependent(type = DeploymentDependentResource.class),
@Dependent(type = ServiceDependentResource.class),
@Dependent(type = IngressDependentResource.class,
reconcilePrecondition = ExposedIngressCondition.class)
}))
// Omitted code
```
2 changes: 1 addition & 1 deletion micrometer-support/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
<parent>
<artifactId>java-operator-sdk</artifactId>
<groupId>io.javaoperatorsdk</groupId>
<version>4.7.2-SNAPSHOT</version>
<version>5.0.0-SNAPSHOT</version>
</parent>
<modelVersion>4.0.0</modelVersion>

Expand Down
2 changes: 1 addition & 1 deletion operator-framework-bom/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

<groupId>io.javaoperatorsdk</groupId>
<artifactId>operator-framework-bom</artifactId>
<version>4.7.2-SNAPSHOT</version>
<version>5.0.0-SNAPSHOT</version>
<name>Operator SDK - Bill of Materials</name>
<packaging>pom</packaging>
<description>Java SDK for implementing Kubernetes operators</description>
Expand Down
2 changes: 1 addition & 1 deletion operator-framework-core/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
<parent>
<groupId>io.javaoperatorsdk</groupId>
<artifactId>java-operator-sdk</artifactId>
<version>4.7.2-SNAPSHOT</version>
<version>5.0.0-SNAPSHOT</version>
<relativePath>../pom.xml</relativePath>
</parent>

Expand Down
32 changes: 1 addition & 31 deletions ...k-core/src/main/java/io/javaoperatorsdk/operator/api/config/BaseConfigurationService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,6 @@
import io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResource;
import io.javaoperatorsdk.operator.processing.dependent.workflow.Condition;
import io.javaoperatorsdk.operator.processing.event.rate.RateLimiter;
import io.javaoperatorsdk.operator.processing.event.source.controller.ResourceEventFilter;
import io.javaoperatorsdk.operator.processing.event.source.controller.ResourceEventFilters;
import io.javaoperatorsdk.operator.processing.event.source.filter.GenericFilter;
import io.javaoperatorsdk.operator.processing.event.source.filter.OnAddFilter;
import io.javaoperatorsdk.operator.processing.event.source.filter.OnUpdateFilter;
Expand Down Expand Up @@ -163,47 +161,19 @@ protected <P extends HasMetadata> ControllerConfiguration<P> configFor(Reconcile
Utils.instantiate(annotation.itemStore(), ItemStore.class, context), dependentFieldManager,
this, informerListLimit);

ResourceEventFilter<P> answer = deprecatedEventFilter(annotation);
config.setEventFilter(answer != null ? answer : ResourceEventFilters.passthrough());

List<DependentResourceSpec> specs = dependentResources(annotation, config);
config.setDependentResources(specs);

return config;
}

@SuppressWarnings("unchecked")
private static <P extends HasMetadata> ResourceEventFilter<P> deprecatedEventFilter(
io.javaoperatorsdk.operator.api.reconciler.ControllerConfiguration annotation) {
ResourceEventFilter<P> answer = null;

Class<ResourceEventFilter<P>>[] filterTypes =
(Class<ResourceEventFilter<P>>[]) valueOrDefault(annotation,
io.javaoperatorsdk.operator.api.reconciler.ControllerConfiguration::eventFilters,
new Object[] {});
for (var filterType : filterTypes) {
try {
ResourceEventFilter<P> filter = filterType.getConstructor().newInstance();

if (answer == null) {
answer = filter;
} else {
answer = answer.and(filter);
}
} catch (Exception e) {
throw new IllegalArgumentException(e);
}
}
return answer;
}

@SuppressWarnings({"unchecked", "rawtypes"})
private static List<DependentResourceSpec> dependentResources(
io.javaoperatorsdk.operator.api.reconciler.ControllerConfiguration annotation,
ControllerConfiguration<?> parent) {
final var dependents =
valueOrDefault(annotation,
io.javaoperatorsdk.operator.api.reconciler.ControllerConfiguration::dependents,
c -> c.workflow().dependents(),
Copy link
Collaborator

Choose a reason for hiding this comment

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

This will crash if there is no workflow annotation.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

There is a default workflow annotation, with empty dependents. To my understanding that never happens.

new Dependent[] {});
if (dependents.length == 0) {
return Collections.emptyList();
Expand Down
39 changes: 1 addition & 38 deletions ...rk-core/src/main/java/io/javaoperatorsdk/operator/api/config/ControllerConfiguration.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,7 @@
import io.javaoperatorsdk.operator.api.reconciler.MaxReconciliationInterval;
import io.javaoperatorsdk.operator.processing.event.rate.LinearRateLimiter;
import io.javaoperatorsdk.operator.processing.event.rate.RateLimiter;
import io.javaoperatorsdk.operator.processing.event.source.controller.ResourceEventFilter;
import io.javaoperatorsdk.operator.processing.event.source.controller.ResourceEventFilters;
import io.javaoperatorsdk.operator.processing.retry.GenericRetry;
import io.javaoperatorsdk.operator.processing.retry.GradualRetry;
import io.javaoperatorsdk.operator.processing.retry.Retry;

public interface ControllerConfiguration<P extends HasMetadata> extends ResourceConfiguration<P> {
Expand Down Expand Up @@ -60,48 +57,14 @@ default boolean isGenerationAware() {
String getAssociatedReconcilerClassName();

default Retry getRetry() {
final var configuration = getRetryConfiguration();
return !RetryConfiguration.DEFAULT.equals(configuration)
? GenericRetry.fromConfiguration(configuration)
: GenericRetry.DEFAULT; // NOSONAR
}

/**
* Use {@link #getRetry()} instead.
*
* @return configuration for retry.
* @deprecated provide your own {@link Retry} implementation or use the {@link GradualRetry}
* annotation instead
*/
@Deprecated(forRemoval = true)
default RetryConfiguration getRetryConfiguration() {
return RetryConfiguration.DEFAULT;
return GenericRetry.DEFAULT;
}

@SuppressWarnings("rawtypes")
default RateLimiter getRateLimiter() {
return DEFAULT_RATE_LIMITER;
}

/**
* Allow controllers to filter events before they are passed to the
* {@link io.javaoperatorsdk.operator.processing.event.EventHandler}.
*
* <p>
* Resource event filters only applies on events of the main custom resource. Not on events from
* other event sources nor the periodic events.
* </p>
*
* @return filter
* @deprecated use {@link ResourceConfiguration#onAddFilter()},
* {@link ResourceConfiguration#onUpdateFilter()} or
* {@link ResourceConfiguration#genericFilter()} instead
*/
@Deprecated(forRemoval = true)
default ResourceEventFilter<P> getEventFilter() {
return ResourceEventFilters.passthrough();
}

@SuppressWarnings("rawtypes")
default List<DependentResourceSpec> getDependentResources() {
return Collections.emptyList();
Expand Down
25 changes: 1 addition & 24 deletions ...rc/main/java/io/javaoperatorsdk/operator/api/config/ControllerConfigurationOverrider.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,9 @@
import io.fabric8.kubernetes.client.informers.cache.ItemStore;
import io.javaoperatorsdk.operator.api.config.dependent.DependentResourceSpec;
import io.javaoperatorsdk.operator.processing.event.rate.RateLimiter;
import io.javaoperatorsdk.operator.processing.event.source.controller.ResourceEventFilter;
import io.javaoperatorsdk.operator.processing.event.source.filter.GenericFilter;
import io.javaoperatorsdk.operator.processing.event.source.filter.OnAddFilter;
import io.javaoperatorsdk.operator.processing.event.source.filter.OnUpdateFilter;
import io.javaoperatorsdk.operator.processing.retry.GenericRetry;
import io.javaoperatorsdk.operator.processing.retry.Retry;

import static io.javaoperatorsdk.operator.api.reconciler.Constants.DEFAULT_NAMESPACES_SET;
Expand All @@ -29,7 +27,6 @@ public class ControllerConfigurationOverrider<R extends HasMetadata> {
private Set<String> namespaces;
private Retry retry;
private String labelSelector;
private ResourceEventFilter<R> customResourcePredicate;
private final ControllerConfiguration<R> original;
private Duration reconciliationMaxInterval;
private OnAddFilter<? super R> onAddFilter;
Expand All @@ -48,7 +45,6 @@ private ControllerConfigurationOverrider(ControllerConfiguration<R> original) {
this.namespaces = new HashSet<>(original.getNamespaces());
this.retry = original.getRetry();
this.labelSelector = original.getLabelSelector();
this.customResourcePredicate = original.getEventFilter();
this.reconciliationMaxInterval = original.maxReconciliationInterval().orElse(null);
this.onAddFilter = original.onAddFilter().orElse(null);
this.onUpdateFilter = original.onUpdateFilter().orElse(null);
Expand Down Expand Up @@ -110,17 +106,6 @@ public ControllerConfigurationOverrider<R> watchingAllNamespaces() {
return this;
}

/**
* @param retry configuration
* @return current instance of overrider
* @deprecated Use {@link #withRetry(Retry)} instead
*/
@Deprecated(forRemoval = true)
public ControllerConfigurationOverrider<R> withRetry(RetryConfiguration retry) {
this.retry = GenericRetry.fromConfiguration(retry);
return this;
}

public ControllerConfigurationOverrider<R> withRetry(Retry retry) {
this.retry = retry;
return this;
Expand All @@ -136,12 +121,6 @@ public ControllerConfigurationOverrider<R> withLabelSelector(String labelSelecto
return this;
}

public ControllerConfigurationOverrider<R> withCustomResourcePredicate(
ResourceEventFilter<R> customResourcePredicate) {
this.customResourcePredicate = customResourcePredicate;
return this;
}

public ControllerConfigurationOverrider<R> withReconciliationMaxInterval(
Duration reconciliationMaxInterval) {
this.reconciliationMaxInterval = reconciliationMaxInterval;
Expand Down Expand Up @@ -210,15 +189,13 @@ public ControllerConfigurationOverrider<R> replacingNamedDependentResourceConfig
}

public ControllerConfiguration<R> build() {
final var overridden = new ResolvedControllerConfiguration<>(original.getResourceClass(),
return new ResolvedControllerConfiguration<>(original.getResourceClass(),
name,
generationAware, original.getAssociatedReconcilerClassName(), retry, rateLimiter,
reconciliationMaxInterval, onAddFilter, onUpdateFilter, genericFilter,
original.getDependentResources(),
namespaces, finalizer, labelSelector, configurations, itemStore, fieldManager,
original.getConfigurationService(), informerListLimit);
overridden.setEventFilter(customResourcePredicate);
return overridden;
}

public static <R extends HasMetadata> ControllerConfigurationOverrider<R> override(
Expand Down
5 changes: 0 additions & 5 deletions ...-core/src/main/java/io/javaoperatorsdk/operator/api/config/DefaultRetryConfiguration.java
View file
Open in desktop

This file was deleted.

Loading