Skip to content

feat: move name is directly to dependent resource #2250

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 4, 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
Original file line number Diff line number Diff line change
Expand Up @@ -68,4 +68,9 @@ static String defaultNameFor(Class<? extends DependentResource> dependentResourc
default boolean isDeletable() {
return this instanceof Deleter;
}

default String name() {
return defaultNameFor(getClass());
}

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

public interface NameSetter {

void setName(String name);

}
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,14 @@
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;
import io.javaoperatorsdk.operator.api.reconciler.dependent.ReconcileResult;
import io.javaoperatorsdk.operator.processing.dependent.Matcher.Result;
import io.javaoperatorsdk.operator.processing.event.ResourceID;

@Ignore
public abstract class AbstractDependentResource<R, P extends HasMetadata>
implements DependentResource<R, P> {
implements DependentResource<R, P>, NameSetter {
private static final Logger log = LoggerFactory.getLogger(AbstractDependentResource.class);

private final boolean creatable = this instanceof Creator;
Expand All @@ -29,14 +30,21 @@ public abstract class AbstractDependentResource<R, P extends HasMetadata>
private ResourceDiscriminator<R, P> resourceDiscriminator;
private final DependentResourceReconciler<R, P> dependentResourceReconciler;

protected String name;

@SuppressWarnings({"unchecked"})
protected AbstractDependentResource() {
this(null);
}

protected AbstractDependentResource(String name) {
creator = creatable ? (Creator<R, P>) this : null;
updater = updatable ? (Updater<R, P>) this : null;

dependentResourceReconciler = this instanceof BulkDependentResource
? new BulkDependentResourceReconciler<>((BulkDependentResource<R, P>) this)
: new SingleDependentResourceReconciler<>(this);
this.name = name == null ? DependentResource.defaultNameFor(this.getClass()) : name;
}

/**
Expand Down Expand Up @@ -176,4 +184,13 @@ protected boolean isUpdatable() {
public boolean isDeletable() {
return deletable;
}

@Override
public String name() {
return name;
}

public void setName(String name) {
this.name = name;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,11 @@ public abstract class AbstractEventSourceHolderDependentResource<R, P extends Ha
protected String eventSourceNameToUse;

protected AbstractEventSourceHolderDependentResource(Class<R> resourceType) {
this(resourceType, null);
}

protected AbstractEventSourceHolderDependentResource(Class<R> resourceType, String name) {
super(name);
this.resourceType = resourceType;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,12 @@ public abstract class KubernetesDependentResource<R extends HasMetadata, P exten

@SuppressWarnings("unchecked")
public KubernetesDependentResource(Class<R> resourceType) {
super(resourceType);
this(resourceType, null);
}

@SuppressWarnings("unchecked")
public KubernetesDependentResource(Class<R> resourceType, String name) {
super(resourceType, name);

usingCustomResourceUpdateMatcher = this instanceof ResourceUpdaterMatcher;
updaterMatcher = usingCustomResourceUpdateMatcher
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
import io.javaoperatorsdk.operator.processing.event.ResourceID;

@SuppressWarnings("rawtypes")
public abstract class AbstractWorkflowExecutor<P extends HasMetadata> {
abstract class AbstractWorkflowExecutor<P extends HasMetadata> {

protected final Workflow<P> workflow;
protected final P primary;
Expand Down Expand Up @@ -133,17 +133,16 @@ protected <R> void registerOrDeregisterEventSourceBasedOnActivation(
boolean activationConditionMet,
DependentResourceNode<R, P> dependentResourceNode) {
if (dependentResourceNode.getActivationCondition().isPresent()) {
final var dr = dependentResourceNode.getDependentResource();
final var eventSourceRetriever = context.eventSourceRetriever();
if (activationConditionMet) {
var eventSource =
dependentResourceNode.getDependentResource().eventSource(context.eventSourceRetriever()
.eventSourceContextForDynamicRegistration());
dr.eventSource(eventSourceRetriever.eventSourceContextForDynamicRegistration());
var es = eventSource.orElseThrow();
context.eventSourceRetriever()
.dynamicallyRegisterEventSource(dependentResourceNode.getName(), es);
eventSourceRetriever.dynamicallyRegisterEventSource(dr.name(), es);

} else {
context.eventSourceRetriever()
.dynamicallyDeRegisterEventSource(dependentResourceNode.getName());
eventSourceRetriever.dynamicallyDeRegisterEventSource(dr.name());
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,10 @@
import io.javaoperatorsdk.operator.api.config.dependent.DependentResourceSpec;
import io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResource;
import io.javaoperatorsdk.operator.api.reconciler.dependent.EventSourceReferencer;
import io.javaoperatorsdk.operator.api.reconciler.dependent.NameSetter;
import io.javaoperatorsdk.operator.api.reconciler.dependent.managed.KubernetesClientAware;

import static io.javaoperatorsdk.operator.api.reconciler.Constants.NO_VALUE_SET;
import static io.javaoperatorsdk.operator.processing.dependent.workflow.Workflow.THROW_EXCEPTION_AUTOMATICALLY_DEFAULT;

@SuppressWarnings("rawtypes")
Expand Down Expand Up @@ -77,13 +79,14 @@ public Workflow<P> resolve(KubernetesClient client,
ControllerConfiguration<P> configuration) {
final var alreadyResolved = new HashMap<String, DependentResourceNode>(orderedSpecs.size());
for (DependentResourceSpec spec : orderedSpecs) {
final var node = new DependentResourceNode(spec.getName(),
final var dependentResource = resolve(spec, client, configuration);
final var node = new DependentResourceNode(
spec.getReconcileCondition(),
spec.getDeletePostCondition(),
spec.getReadyCondition(),
spec.getActivationCondition(),
resolve(spec, client, configuration));
alreadyResolved.put(node.getName(), node);
dependentResource);
alreadyResolved.put(dependentResource.name(), node);
spec.getDependsOn()
.forEach(depend -> node.addDependsOnRelation(alreadyResolved.get(depend)));
}
Expand All @@ -104,6 +107,11 @@ private <R> DependentResource<R, P> resolve(DependentResourceSpec<R, P> spec,
configuration.getConfigurationService().dependentResourceFactory()
.createFrom(spec, configuration);

final var name = spec.getName();
if (name != null && !NO_VALUE_SET.equals(name) && dependentResource instanceof NameSetter) {
((NameSetter) dependentResource).setName(name);
}

if (dependentResource instanceof KubernetesClientAware) {
((KubernetesClientAware) dependentResource).setKubernetesClient(client);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
* @param <P> primary resource
*/
@SuppressWarnings("rawtypes")
public class DefaultWorkflow<P extends HasMetadata> implements Workflow<P> {
class DefaultWorkflow<P extends HasMetadata> implements Workflow<P> {

private final Map<String, DependentResourceNode> dependentResourceNodes;
private final Set<DependentResourceNode> topLevelResources;
Expand Down Expand Up @@ -77,9 +77,9 @@ private Map<String, DependentResourceNode> toMap(Set<DependentResourceNode> node
bottomLevelResource.remove(dependsOn);
}
}
map.put(node.getName(), node);
map.put(node.getDependentResource().name(), node);
}
if (topLevelResources.size() == 0) {
if (topLevelResources.isEmpty()) {
throw new IllegalStateException(
"No top-level dependent resources found. This might indicate a cyclic Set of DependentResourceNode has been provided.");
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,29 +8,24 @@
import io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResource;

@SuppressWarnings("rawtypes")
public class DependentResourceNode<R, P extends HasMetadata> {
class DependentResourceNode<R, P extends HasMetadata> {

private final List<DependentResourceNode> dependsOn = new LinkedList<>();
private final List<DependentResourceNode> parents = new LinkedList<>();
private final String name;

private Condition<R, P> reconcilePrecondition;
private Condition<R, P> deletePostcondition;
private Condition<R, P> readyPostcondition;
private Condition<R, P> activationCondition;
private final DependentResource<R, P> dependentResource;

DependentResourceNode(String name, DependentResource<R, P> dependentResource) {
this(name, null, null, null, null, dependentResource);
}

DependentResourceNode(DependentResource<R, P> dependentResource) {
this(getNameFor(dependentResource), null, null, null, null, dependentResource);
this(null, null, null, null, dependentResource);
}

public DependentResourceNode(String name, Condition<R, P> reconcilePrecondition,
public DependentResourceNode(Condition<R, P> reconcilePrecondition,
Condition<R, P> deletePostcondition, Condition<R, P> readyPostcondition,
Condition<R, P> activationCondition, DependentResource<R, P> dependentResource) {
this.name = name;
this.reconcilePrecondition = reconcilePrecondition;
this.deletePostcondition = deletePostcondition;
this.readyPostcondition = readyPostcondition;
Expand All @@ -55,16 +50,10 @@ public List<DependentResourceNode> getParents() {
return parents;
}

public String getName() {
return name;
}


public Optional<Condition<R, P>> getReconcilePrecondition() {
return Optional.ofNullable(reconcilePrecondition);
}


public Optional<Condition<R, P>> getDeletePostcondition() {
return Optional.ofNullable(deletePostcondition);
}
Expand Down Expand Up @@ -106,18 +95,12 @@ public boolean equals(Object o) {
return false;
}
DependentResourceNode<?, ?> that = (DependentResourceNode<?, ?>) o;
return name.equals(that.name);
return this.getDependentResource().name().equals(that.getDependentResource().name());
}

@Override
public int hashCode() {
return name.hashCode();
}

@SuppressWarnings("rawtypes")
static String getNameFor(DependentResource dependentResource) {
return DependentResource.defaultNameFor(dependentResource.getClass()) + "#"
+ dependentResource.hashCode();
return this.getDependentResource().name().hashCode();
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,14 +21,9 @@ public class WorkflowBuilder<P extends HasMetadata> {
private boolean isCleaner = false;

public WorkflowBuilder<P> addDependentResource(DependentResource dependentResource) {
return addDependentResource(dependentResource, null);
}

public WorkflowBuilder<P> addDependentResource(DependentResource dependentResource, String name) {
currentNode = name == null ? new DependentResourceNode<>(dependentResource)
: new DependentResourceNode<>(name, dependentResource);
currentNode = new DependentResourceNode<>(dependentResource);
isCleaner = isCleaner || dependentResource.isDeletable();
final var actualName = currentNode.getName();
final var actualName = dependentResource.name();
dependentResourceNodes.put(actualName, currentNode);
return this;
}
Expand Down Expand Up @@ -70,8 +65,7 @@ public WorkflowBuilder<P> withActivationCondition(Condition activationCondition)

DependentResourceNode getNodeByDependentResource(DependentResource<?, ?> dependentResource) {
// first check by name
final var node =
dependentResourceNodes.get(DependentResourceNode.getNameFor(dependentResource));
final var node = dependentResourceNodes.get(dependentResource.name());
if (node != null) {
return node;
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ public void throwAggregateExceptionIfErrorsPresent() {
if (erroredDependentsExist()) {
throw new AggregatedOperatorException("Exception(s) during workflow execution.",
erroredDependents.entrySet().stream()
.collect(Collectors.toMap(e -> e.getKey().getClass().getName(), Entry::getValue)));
.collect(Collectors.toMap(e -> e.getKey().name(), Entry::getValue)));
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,7 @@
import io.javaoperatorsdk.operator.processing.dependent.kubernetes.KubernetesDependentResourceConfigBuilder;
import io.javaoperatorsdk.operator.processing.dependent.workflow.Condition;

import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertNull;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.junit.jupiter.api.Assertions.*;

class ControllerConfigurationOverriderTest {
private final BaseConfigurationService configurationService = new BaseConfigurationService();
Expand Down Expand Up @@ -75,8 +71,7 @@ void overridingNSShouldPreserveUntouchedDependents() {
private static class NamedDependentReconciler implements Reconciler<ConfigMap> {

@Override
public UpdateControl<ConfigMap> reconcile(ConfigMap resource, Context<ConfigMap> context)
throws Exception {
public UpdateControl<ConfigMap> reconcile(ConfigMap resource, Context<ConfigMap> context) {
return null;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,7 @@
import io.javaoperatorsdk.operator.processing.dependent.kubernetes.KubernetesDependentResource;

import static io.javaoperatorsdk.operator.api.config.dependent.DependentResourceConfigurationResolverTest.CustomAnnotationReconciler.DR_NAME;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertNull;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.junit.jupiter.api.Assertions.*;

class DependentResourceConfigurationResolverTest {

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@
public class EmptyTestDependentResource
implements DependentResource<Deployment, TestCustomResource> {

private String name;

@Override
public ReconcileResult<Deployment> reconcile(TestCustomResource primary,
Context<TestCustomResource> context) {
Expand All @@ -19,5 +21,14 @@ public ReconcileResult<Deployment> reconcile(TestCustomResource primary,
public Class<Deployment> resourceType() {
return Deployment.class;
}

@Override
public String name() {
return name;
}

public void setName(String name) {
this.name = name;
}
}

Original file line number Diff line number Diff line change
Expand Up @@ -35,11 +35,8 @@ public class AbstractWorkflowExecutorTest {

public class TestDependent extends KubernetesDependentResource<ConfigMap, TestCustomResource> {

private final String name;

public TestDependent(String name) {
super(ConfigMap.class);
this.name = name;
super(ConfigMap.class, name);
}

@Override
Expand All @@ -52,7 +49,7 @@ public ReconcileResult<ConfigMap> reconcile(TestCustomResource primary,

@Override
public String toString() {
return name;
return name();
}
}

Expand Down Expand Up @@ -110,6 +107,11 @@ public Class<String> resourceType() {
return String.class;
}

@Override
public String name() {
return name;
}

@Override
public String toString() {
return name;
Expand Down
Loading