Skip to content

feat: remove resource discriminator #2299

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
Mar 25, 2024
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
6 changes: 0 additions & 6 deletions docs/documentation/dependent-resources.md
Original file line number Diff line number Diff line change
Expand Up @@ -318,12 +318,6 @@ There might be casees, though, where it might be problematic to call the `desire
- Override the `managedSecondaryResourceID` method, if your `DependentResource` extends `KubernetesDependentResource`,
where it's very often possible to easily determine the `ResourceID` of the secondary resource. This would probably be
the easiest solution if you're working with Kubernetes resources.
- Configure
a [`ResourceDiscriminator`](https://github.com/java-operator-sdk/java-operator-sdk/blob/main/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/ResourceDiscriminator.java)
implementation for your `DependentResource`. This was the approach that was used before JOSDK v5 but should not be
needed anymore as it is simpler and more efficient to override one the methods above instead of creating a separate
class. Discriminators can be declaratively set when using managed Kubernetes dependent resources via
the `resourceDiscriminator` field of the `@KubernetesDependent` annotation.

### Sharing an Event Source Between Dependent Resources

Expand Down
2 changes: 2 additions & 0 deletions docs/documentation/v5-0-migration.md
Original file line number Diff line number Diff line change
Expand Up @@ -26,3 +26,5 @@ permalink: /docs/v5-0-migration
Also, the related part of a [workaround](https://github.com/operator-framework/java-operator-sdk/blob/main/operator-framework/src/test/java/io/javaoperatorsdk/operator/StatusPatchSSAMigrationIT.java#L110-L116).
4. `ManagedDependentResourceContext` has been renamed to `ManagedWorkflowAndDependentResourceContext` and is accessed
via the accordingly renamed `managedWorkflowAndDependentResourceContext` method.
5. `ResourceDiscriminator` was removed. In most of the cases you can just delete the discriminator, everything should
work without it by default. To optimize and handle special cases see the relevant section in [Dependent Resource documentation](/docs/dependent-resources#multiple-dependent-resources-of-same-type).
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ public interface Context<P extends HasMetadata> {
Optional<RetryInfo> getRetryInfo();

default <R> Optional<R> getSecondaryResource(Class<R> expectedType) {
return getSecondaryResource(expectedType, (String) null);
return getSecondaryResource(expectedType, null);
}

<R> Set<R> getSecondaryResources(Class<R> expectedType);
Expand All @@ -29,9 +29,6 @@ default <R> Stream<R> getSecondaryResourcesAsStream(Class<R> expectedType) {
@Deprecated(forRemoval = true)
<R> Optional<R> getSecondaryResource(Class<R> expectedType, String eventSourceName);

<R> Optional<R> getSecondaryResource(Class<R> expectedType,
ResourceDiscriminator<R, P> discriminator);

ControllerConfiguration<P> getControllerConfiguration();

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,12 +68,6 @@ public <T> Optional<T> getSecondaryResource(Class<T> expectedType, String eventS
.getSecondaryResource(primaryResource);
}

@Override
public <R> Optional<R> getSecondaryResource(Class<R> expectedType,
ResourceDiscriminator<R, P> discriminator) {
return discriminator.distinguish(expectedType, primaryResource, this);
}

@Override
public ControllerConfiguration<P> getControllerConfiguration() {
return controllerConfiguration;
Expand Down

This file was deleted.

This file was deleted.

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
import io.fabric8.kubernetes.api.model.HasMetadata;
import io.javaoperatorsdk.operator.api.reconciler.Context;
import io.javaoperatorsdk.operator.api.reconciler.Ignore;
import io.javaoperatorsdk.operator.api.reconciler.ResourceDiscriminator;
import io.javaoperatorsdk.operator.api.reconciler.dependent.Deleter;
import io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResource;
import io.javaoperatorsdk.operator.api.reconciler.dependent.NameSetter;
Expand All @@ -28,7 +27,6 @@ public abstract class AbstractDependentResource<R, P extends HasMetadata>

protected Creator<R, P> creator;
protected Updater<R, P> updater;
private ResourceDiscriminator<R, P> resourceDiscriminator;
private final DependentResourceReconciler<R, P> dependentResourceReconciler;

protected String name;
Expand Down Expand Up @@ -98,16 +96,14 @@ protected ReconcileResult<R> reconcile(P primary, R actualResource, Context<P> c

@Override
public Optional<R> getSecondaryResource(P primary, Context<P> context) {
if (resourceDiscriminator != null) {
return resourceDiscriminator.distinguish(resourceType(), primary, context);

var secondaryResources = context.getSecondaryResources(resourceType());
if (secondaryResources.isEmpty()) {
return Optional.empty();
} else {
var secondaryResources = context.getSecondaryResources(resourceType());
if (secondaryResources.isEmpty()) {
return Optional.empty();
} else {
return selectManagedSecondaryResource(secondaryResources, primary, context);
}
return selectManagedSecondaryResource(secondaryResources, primary, context);
}

}

/**
Expand Down Expand Up @@ -198,10 +194,6 @@ protected void handleDelete(P primary, R secondary, Context<P> context) {
"handleDelete method must be implemented if Deleter trait is supported");
}

public void setResourceDiscriminator(ResourceDiscriminator<R, P> resourceDiscriminator) {
this.resourceDiscriminator = resourceDiscriminator;
}

protected boolean isCreatable() {
return creatable;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
import java.lang.annotation.Target;

import io.javaoperatorsdk.operator.api.reconciler.Constants;
import io.javaoperatorsdk.operator.api.reconciler.ResourceDiscriminator;
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.OnDeleteFilter;
Expand Down Expand Up @@ -70,8 +69,6 @@
*/
Class<? extends GenericFilter> genericFilter() default GenericFilter.class;

Class<? extends ResourceDiscriminator> resourceDiscriminator() default ResourceDiscriminator.class;

/**
* Creates the resource only if did not exist before, this applies only if SSA is used.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
import io.javaoperatorsdk.operator.api.config.Utils;
import io.javaoperatorsdk.operator.api.config.dependent.ConfigurationConverter;
import io.javaoperatorsdk.operator.api.reconciler.Constants;
import io.javaoperatorsdk.operator.api.reconciler.ResourceDiscriminator;
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.OnDeleteFilter;
Expand All @@ -33,7 +32,6 @@ public KubernetesDependentResourceConfig<R> configFrom(KubernetesDependent confi
OnUpdateFilter<? extends HasMetadata> onUpdateFilter = null;
OnDeleteFilter<? extends HasMetadata> onDeleteFilter = null;
GenericFilter<? extends HasMetadata> genericFilter = null;
ResourceDiscriminator<?, ?> resourceDiscriminator = null;
Boolean useSSA = null;
if (configAnnotation != null) {
if (!Arrays.equals(KubernetesDependent.DEFAULT_NAMESPACES, configAnnotation.namespaces())) {
Expand All @@ -54,16 +52,13 @@ public KubernetesDependentResourceConfig<R> configFrom(KubernetesDependent confi
genericFilter =
Utils.instantiate(configAnnotation.genericFilter(), GenericFilter.class, context);

resourceDiscriminator =
Utils.instantiate(configAnnotation.resourceDiscriminator(), ResourceDiscriminator.class,
context);
createResourceOnlyIfNotExistingWithSSA =
configAnnotation.createResourceOnlyIfNotExistingWithSSA();
useSSA = configAnnotation.useSSA().asBoolean();
}

return new KubernetesDependentResourceConfig(namespaces, labelSelector, configuredNS,
createResourceOnlyIfNotExistingWithSSA,
resourceDiscriminator, useSSA, onAddFilter, onUpdateFilter, onDeleteFilter, genericFilter);
useSSA, onAddFilter, onUpdateFilter, onDeleteFilter, genericFilter);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -61,10 +61,6 @@ public KubernetesDependentResource(Class<R> resourceType, String name) {
@Override
public void configureWith(KubernetesDependentResourceConfig<R> config) {
this.kubernetesDependentResourceConfig = config;
var discriminator = kubernetesDependentResourceConfig.getResourceDiscriminator();
if (discriminator != null) {
setResourceDiscriminator(discriminator);
}
}

private void configureWith(String labelSelector, Set<String> namespaces,
Expand Down Expand Up @@ -259,10 +255,6 @@ protected InformerEventSource<R, P> createEventSource(EventSourceContext<P> cont
onUpdateFilter = kubernetesDependentResourceConfig.onUpdateFilter();
onDeleteFilter = kubernetesDependentResourceConfig.onDeleteFilter();
genericFilter = kubernetesDependentResourceConfig.genericFilter();
var discriminator = kubernetesDependentResourceConfig.getResourceDiscriminator();
if (discriminator != null) {
setResourceDiscriminator(discriminator);
}
configureWith(kubernetesDependentResourceConfig.labelSelector(),
kubernetesDependentResourceConfig.namespaces(),
!kubernetesDependentResourceConfig.wereNamespacesConfigured(), context);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
import java.util.Set;

import io.javaoperatorsdk.operator.api.reconciler.Constants;
import io.javaoperatorsdk.operator.api.reconciler.ResourceDiscriminator;
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.OnDeleteFilter;
Expand All @@ -20,7 +19,6 @@ public class KubernetesDependentResourceConfig<R> {
private String labelSelector;
private final boolean namespacesWereConfigured;
private final boolean createResourceOnlyIfNotExistingWithSSA;
private final ResourceDiscriminator<R, ?> resourceDiscriminator;
private final Boolean useSSA;

private final OnAddFilter<R> onAddFilter;
Expand All @@ -31,15 +29,14 @@ public class KubernetesDependentResourceConfig<R> {
public KubernetesDependentResourceConfig() {
this(Constants.SAME_AS_CONTROLLER_NAMESPACES_SET, NO_VALUE_SET, true,
DEFAULT_CREATE_RESOURCE_ONLY_IF_NOT_EXISTING_WITH_SSA,
null, null, null,
null, null,
null, null, null);
}

public KubernetesDependentResourceConfig(Set<String> namespaces,
String labelSelector,
boolean configuredNS,
boolean createResourceOnlyIfNotExistingWithSSA,
ResourceDiscriminator<R, ?> resourceDiscriminator,
Boolean useSSA,
OnAddFilter<R> onAddFilter,
OnUpdateFilter<R> onUpdateFilter,
Expand All @@ -52,15 +49,14 @@ public KubernetesDependentResourceConfig(Set<String> namespaces,
this.onUpdateFilter = onUpdateFilter;
this.onDeleteFilter = onDeleteFilter;
this.genericFilter = genericFilter;
this.resourceDiscriminator = resourceDiscriminator;
this.useSSA = useSSA;
}

// use builder instead
@Deprecated(forRemoval = true)
public KubernetesDependentResourceConfig(Set<String> namespaces, String labelSelector) {
this(namespaces, labelSelector, true, DEFAULT_CREATE_RESOURCE_ONLY_IF_NOT_EXISTING_WITH_SSA,
null, null, null,
null, null,
null, null, null);
}

Expand Down Expand Up @@ -104,11 +100,6 @@ public GenericFilter<R> genericFilter() {
return genericFilter;
}

@SuppressWarnings("rawtypes")
public ResourceDiscriminator getResourceDiscriminator() {
return resourceDiscriminator;
}

@SuppressWarnings("unused")
protected void setNamespaces(Set<String> namespaces) {
if (!wereNamespacesConfigured() && namespaces != null && !namespaces.isEmpty()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
import java.util.Set;

import io.javaoperatorsdk.operator.api.reconciler.Constants;
import io.javaoperatorsdk.operator.api.reconciler.ResourceDiscriminator;
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.OnDeleteFilter;
Expand All @@ -14,7 +13,6 @@ public final class KubernetesDependentResourceConfigBuilder<R> {
private Set<String> namespaces = Constants.SAME_AS_CONTROLLER_NAMESPACES_SET;
private String labelSelector;
private boolean createResourceOnlyIfNotExistingWithSSA;
private ResourceDiscriminator<R, ?> resourceDiscriminator;
private Boolean useSSA;
private OnAddFilter<R> onAddFilter;
private OnUpdateFilter<R> onUpdateFilter;
Expand Down Expand Up @@ -43,12 +41,6 @@ public KubernetesDependentResourceConfigBuilder<R> withCreateResourceOnlyIfNotEx
return this;
}

public KubernetesDependentResourceConfigBuilder<R> withResourceDiscriminator(
ResourceDiscriminator<R, ?> resourceDiscriminator) {
this.resourceDiscriminator = resourceDiscriminator;
return this;
}

public KubernetesDependentResourceConfigBuilder<R> withUseSSA(Boolean useSSA) {
this.useSSA = useSSA;
return this;
Expand Down Expand Up @@ -80,7 +72,7 @@ public KubernetesDependentResourceConfigBuilder<R> withGenericFilter(
public KubernetesDependentResourceConfig<R> build() {
return new KubernetesDependentResourceConfig<>(namespaces, labelSelector,
namespaces != Constants.SAME_AS_CONTROLLER_NAMESPACES_SET,
createResourceOnlyIfNotExistingWithSSA, resourceDiscriminator, useSSA, onAddFilter,
createResourceOnlyIfNotExistingWithSSA, useSSA, onAddFilter,
onUpdateFilter, onDeleteFilter, genericFilter);
}
}
Loading
Loading