Skip to content
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

feat: context getSecondary resource is activation condition aware #2532

Merged
merged 3 commits into from
Sep 23, 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 @@ -13,6 +13,7 @@
import io.javaoperatorsdk.operator.api.reconciler.dependent.managed.ManagedWorkflowAndDependentResourceContext;
import io.javaoperatorsdk.operator.processing.Controller;
import io.javaoperatorsdk.operator.processing.event.EventSourceRetriever;
import io.javaoperatorsdk.operator.processing.event.NoEventSourceForClassException;
import io.javaoperatorsdk.operator.processing.event.ResourceID;

public class DefaultContext<P extends HasMetadata> implements Context<P> {
Expand Down Expand Up @@ -62,10 +63,26 @@ public <R> Stream<R> getSecondaryResourcesAsStream(Class<R> expectedType) {

@Override
public <T> Optional<T> getSecondaryResource(Class<T> expectedType, String eventSourceName) {
return controller
.getEventSourceManager()
.getEventSourceFor(expectedType, eventSourceName)
.getSecondaryResource(primaryResource);
try {
return controller
.getEventSourceManager()
.getEventSourceFor(expectedType, eventSourceName)
.getSecondaryResource(primaryResource);
} catch (NoEventSourceForClassException e) {
/*
* If a workflow has an activation condition there can be event sources which are only
* registered if the activation condition holds, but to provide a consistent API we return an
* Optional instead of throwing an exception.
*
* Note that not only the resource which has an activation condition might not be registered
* but dependents which depend on it.
*/
if (eventSourceName == null && controller.workflowContainsDependentForType(expectedType)) {
return Optional.empty();
} else {
throw e;
}
}
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -475,4 +475,10 @@ public WorkflowCleanupResult cleanupManagedWorkflow(P resource, Context<P> conte
public boolean isWorkflowExplicitInvocation() {
return explicitWorkflowInvocation;
}

public boolean workflowContainsDependentForType(Class<?> clazz) {
return managedWorkflow.getDependentResourcesByName().values().stream()
.anyMatch(d -> d.resourceType().equals(clazz));
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -99,8 +99,7 @@ public <S> EventSource<S, P> get(Class<S> dependentType, String name) {

final var sourcesForType = sources.get(keyFor(dependentType));
if (sourcesForType == null || sourcesForType.isEmpty()) {
throw new IllegalArgumentException(
"There is no event source found for class:" + dependentType.getName());
throw new NoEventSourceForClassException(dependentType);
}

final var size = sourcesForType.size();
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
package io.javaoperatorsdk.operator.processing.event;

import io.javaoperatorsdk.operator.OperatorException;

public class NoEventSourceForClassException extends OperatorException {

private Class<?> clazz;

public NoEventSourceForClassException(Class<?> clazz) {
this.clazz = clazz;
}

public Class<?> getClazz() {
return clazz;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
package io.javaoperatorsdk.operator.api.reconciler;

import org.junit.jupiter.api.Test;

import io.fabric8.kubernetes.api.model.ConfigMap;
import io.fabric8.kubernetes.api.model.Secret;
import io.javaoperatorsdk.operator.processing.Controller;
import io.javaoperatorsdk.operator.processing.event.EventSourceManager;
import io.javaoperatorsdk.operator.processing.event.NoEventSourceForClassException;

import static org.assertj.core.api.Assertions.assertThat;
import static org.junit.jupiter.api.Assertions.*;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;

class DefaultContextTest {

Secret primary = new Secret();
Controller<Secret> mockController = mock(Controller.class);

DefaultContext<?> context = new DefaultContext<>(null, mockController, primary);

@Test
void getSecondaryResourceReturnsEmptyOptionalOnNonActivatedDRType() {
var mockManager = mock(EventSourceManager.class);
when(mockController.getEventSourceManager()).thenReturn(mockManager);
when(mockController.workflowContainsDependentForType(ConfigMap.class)).thenReturn(true);
when(mockManager.getEventSourceFor(any(), any()))
.thenThrow(new NoEventSourceForClassException(ConfigMap.class));

var res = context.getSecondaryResource(ConfigMap.class);

assertThat(res).isEmpty();
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -85,12 +85,12 @@ public void startCascadesToEventSources() {

@Test
void retrievingEventSourceForClassShouldWork() {
assertThatExceptionOfType(IllegalArgumentException.class)
assertThatExceptionOfType(NoEventSourceForClassException.class)
.isThrownBy(() -> eventSourceManager.getEventSourceFor(Class.class));

// manager is initialized with a controller configured to handle HasMetadata
EventSourceManager manager = initManager();
assertThatExceptionOfType(IllegalArgumentException.class)
assertThatExceptionOfType(NoEventSourceForClassException.class)
csviri marked this conversation as resolved.
Show resolved Hide resolved
.isThrownBy(() -> manager.getEventSourceFor(HasMetadata.class, "unknown_name"));

ManagedInformerEventSource eventSource = mock(ManagedInformerEventSource.class);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
package io.javaoperatorsdk.operator.workflow.getnonactivesecondary;


import io.fabric8.kubernetes.api.model.ConfigMap;
import io.fabric8.kubernetes.api.model.ObjectMetaBuilder;
import io.javaoperatorsdk.operator.api.reconciler.Context;
import io.javaoperatorsdk.operator.processing.dependent.kubernetes.CRUDKubernetesDependentResource;

public class ConfigMapDependentResource
extends CRUDKubernetesDependentResource<ConfigMap, GetNonActiveSecondaryCustomResource> {

public static final String DATA_KEY = "data";

public ConfigMapDependentResource() {
super(ConfigMap.class);
}

@Override
protected ConfigMap desired(GetNonActiveSecondaryCustomResource primary,
Context<GetNonActiveSecondaryCustomResource> context) {
ConfigMap configMap = new ConfigMap();
configMap.setMetadata(new ObjectMetaBuilder()
.withName(primary.getMetadata().getName())
.withNamespace(primary.getMetadata().getNamespace())
.build());
return configMap;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
package io.javaoperatorsdk.operator.workflow.getnonactivesecondary;

import io.fabric8.openshift.api.model.Route;
import io.javaoperatorsdk.operator.api.reconciler.Context;
import io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResource;
import io.javaoperatorsdk.operator.processing.dependent.workflow.Condition;

public class FalseActivationCondition
implements Condition<Route, GetNonActiveSecondaryCustomResource> {
@Override
public boolean isMet(
DependentResource<Route, GetNonActiveSecondaryCustomResource> dependentResource,
GetNonActiveSecondaryCustomResource primary,
Context<GetNonActiveSecondaryCustomResource> context) {
return false;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
package io.javaoperatorsdk.operator.workflow.getnonactivesecondary;

import io.fabric8.kubernetes.api.model.Namespaced;
import io.fabric8.kubernetes.client.CustomResource;
import io.fabric8.kubernetes.model.annotation.Group;
import io.fabric8.kubernetes.model.annotation.ShortNames;
import io.fabric8.kubernetes.model.annotation.Version;

@Group("sample.javaoperatorsdk")
@Version("v1")
@ShortNames("gnas")
public class GetNonActiveSecondaryCustomResource
extends CustomResource<Void, Void>
implements Namespaced {


}
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
package io.javaoperatorsdk.operator.workflow.getnonactivesecondary;

import io.fabric8.kubernetes.api.model.ObjectMetaBuilder;
import io.fabric8.openshift.api.model.Route;
import io.javaoperatorsdk.operator.api.reconciler.Context;
import io.javaoperatorsdk.operator.processing.dependent.kubernetes.CRUDKubernetesDependentResource;

public class RouteDependentResource
extends CRUDKubernetesDependentResource<Route, GetNonActiveSecondaryCustomResource> {

public RouteDependentResource() {
super(Route.class);
}

@Override
protected Route desired(GetNonActiveSecondaryCustomResource primary,
Context<GetNonActiveSecondaryCustomResource> context) {
// basically does not matter since this should not be called
Route route = new Route();
route.setMetadata(new ObjectMetaBuilder()
.withName(primary.getMetadata().getName())
.withNamespace(primary.getMetadata().getNamespace())
.build());

return route;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
package io.javaoperatorsdk.operator.workflow.getnonactivesecondary;

import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.RegisterExtension;

import io.fabric8.kubernetes.api.model.ObjectMetaBuilder;
import io.javaoperatorsdk.operator.junit.LocallyRunOperatorExtension;

import static org.assertj.core.api.Assertions.assertThat;
import static org.awaitility.Awaitility.await;

public class WorkflowActivationConditionIT {

public static final String TEST_RESOURCE_NAME = "test1";

@RegisterExtension
LocallyRunOperatorExtension extension =
LocallyRunOperatorExtension.builder()
.withReconciler(WorkflowActivationConditionReconciler.class)
.build();

@Test
void reconciledOnVanillaKubernetesDespiteRouteInWorkflow() {
extension.create(testResource());

await().untilAsserted(() -> {
assertThat(extension.getReconcilerOfType(WorkflowActivationConditionReconciler.class)
.getNumberOfReconciliationExecution()).isEqualTo(1);
});
}

private GetNonActiveSecondaryCustomResource testResource() {
var res = new GetNonActiveSecondaryCustomResource();
res.setMetadata(new ObjectMetaBuilder()
.withName(TEST_RESOURCE_NAME)
.build());
return res;
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
package io.javaoperatorsdk.operator.workflow.getnonactivesecondary;

import java.util.concurrent.atomic.AtomicInteger;

import io.fabric8.openshift.api.model.Route;
import io.javaoperatorsdk.operator.api.reconciler.Context;
import io.javaoperatorsdk.operator.api.reconciler.ControllerConfiguration;
import io.javaoperatorsdk.operator.api.reconciler.Reconciler;
import io.javaoperatorsdk.operator.api.reconciler.UpdateControl;
import io.javaoperatorsdk.operator.api.reconciler.Workflow;
import io.javaoperatorsdk.operator.api.reconciler.dependent.Dependent;

@Workflow(dependents = {
@Dependent(type = ConfigMapDependentResource.class),
@Dependent(type = RouteDependentResource.class,
activationCondition = FalseActivationCondition.class)
})
@ControllerConfiguration
public class WorkflowActivationConditionReconciler
implements Reconciler<GetNonActiveSecondaryCustomResource> {

private final AtomicInteger numberOfReconciliationExecution = new AtomicInteger(0);

@Override
public UpdateControl<GetNonActiveSecondaryCustomResource> reconcile(
GetNonActiveSecondaryCustomResource resource,
Context<GetNonActiveSecondaryCustomResource> context) {

// should not throw an exception even if the condition is false
var route = context.getSecondaryResource(Route.class);

numberOfReconciliationExecution.incrementAndGet();

return UpdateControl.noUpdate();
}

public int getNumberOfReconciliationExecution() {
return numberOfReconciliationExecution.get();
}
}
Loading