Skip to content

Commit e289c98

Browse files
csviricsviri
committed
feat: context getSecondary resource is activation condition aware (#2532)
 Signed-off-by: Attila Mészáros <[email protected]>
1 parent 82c7a29 commit e289c98

File tree

12 files changed

+252
-8
lines changed

12 files changed

+252
-8
lines changed

Diff for: ‎operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/DefaultContext.java

+21-4
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
import io.javaoperatorsdk.operator.api.reconciler.dependent.managed.ManagedWorkflowAndDependentResourceContext;
1414
import io.javaoperatorsdk.operator.processing.Controller;
1515
import io.javaoperatorsdk.operator.processing.event.EventSourceRetriever;
16+
import io.javaoperatorsdk.operator.processing.event.NoEventSourceForClassException;
1617
import io.javaoperatorsdk.operator.processing.event.ResourceID;
1718

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

6364
@Override
6465
public <T> Optional<T> getSecondaryResource(Class<T> expectedType, String eventSourceName) {
65-
return controller
66-
.getEventSourceManager()
67-
.getEventSourceFor(expectedType, eventSourceName)
68-
.getSecondaryResource(primaryResource);
66+
try {
67+
return controller
68+
.getEventSourceManager()
69+
.getEventSourceFor(expectedType, eventSourceName)
70+
.getSecondaryResource(primaryResource);
71+
} catch (NoEventSourceForClassException e) {
72+
/*
73+
* If a workflow has an activation condition there can be event sources which are only
74+
* registered if the activation condition holds, but to provide a consistent API we return an
75+
* Optional instead of throwing an exception.
76+
*
77+
* Note that not only the resource which has an activation condition might not be registered
78+
* but dependents which depend on it.
79+
*/
80+
if (eventSourceName == null && controller.workflowContainsDependentForType(expectedType)) {
81+
return Optional.empty();
82+
} else {
83+
throw e;
84+
}
85+
}
6986
}
7087

7188
@Override

Diff for: ‎operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/Controller.java

+6
Original file line numberDiff line numberDiff line change
@@ -473,4 +473,10 @@ public WorkflowCleanupResult cleanupManagedWorkflow(P resource, Context<P> conte
473473
public boolean isWorkflowExplicitInvocation() {
474474
return explicitWorkflowInvocation;
475475
}
476+
477+
public boolean workflowContainsDependentForType(Class<?> clazz) {
478+
return managedWorkflow.getDependentResourcesByName().values().stream()
479+
.anyMatch(d -> d.resourceType().equals(clazz));
480+
}
481+
476482
}

Diff for: ‎operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventSources.java

+1-2
Original file line numberDiff line numberDiff line change
@@ -99,8 +99,7 @@ public <S> EventSource<S, P> get(Class<S> dependentType, String name) {
9999

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

106105
final var size = sourcesForType.size();

Diff for: ‎operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/NoEventSourceForClassException.java

+16
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
package io.javaoperatorsdk.operator.processing.event;
2+
3+
import io.javaoperatorsdk.operator.OperatorException;
4+
5+
public class NoEventSourceForClassException extends OperatorException {
6+
7+
private Class<?> clazz;
8+
9+
public NoEventSourceForClassException(Class<?> clazz) {
10+
this.clazz = clazz;
11+
}
12+
13+
public Class<?> getClazz() {
14+
return clazz;
15+
}
16+
}

Diff for: ‎operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/reconciler/DefaultContextTest.java

+37
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
package io.javaoperatorsdk.operator.api.reconciler;
2+
3+
import org.junit.jupiter.api.Test;
4+
5+
import io.fabric8.kubernetes.api.model.ConfigMap;
6+
import io.fabric8.kubernetes.api.model.Secret;
7+
import io.javaoperatorsdk.operator.processing.Controller;
8+
import io.javaoperatorsdk.operator.processing.event.EventSourceManager;
9+
import io.javaoperatorsdk.operator.processing.event.NoEventSourceForClassException;
10+
11+
import static org.assertj.core.api.Assertions.assertThat;
12+
import static org.junit.jupiter.api.Assertions.*;
13+
import static org.mockito.ArgumentMatchers.any;
14+
import static org.mockito.Mockito.mock;
15+
import static org.mockito.Mockito.when;
16+
17+
class DefaultContextTest {
18+
19+
Secret primary = new Secret();
20+
Controller<Secret> mockController = mock(Controller.class);
21+
22+
DefaultContext<?> context = new DefaultContext<>(null, mockController, primary);
23+
24+
@Test
25+
void getSecondaryResourceReturnsEmptyOptionalOnNonActivatedDRType() {
26+
var mockManager = mock(EventSourceManager.class);
27+
when(mockController.getEventSourceManager()).thenReturn(mockManager);
28+
when(mockController.workflowContainsDependentForType(ConfigMap.class)).thenReturn(true);
29+
when(mockManager.getEventSourceFor(any(), any()))
30+
.thenThrow(new NoEventSourceForClassException(ConfigMap.class));
31+
32+
var res = context.getSecondaryResource(ConfigMap.class);
33+
34+
assertThat(res).isEmpty();
35+
}
36+
37+
}

Diff for: ‎operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/EventSourceManagerTest.java

+2-2
Original file line numberDiff line numberDiff line change
@@ -85,12 +85,12 @@ public void startCascadesToEventSources() {
8585

8686
@Test
8787
void retrievingEventSourceForClassShouldWork() {
88-
assertThatExceptionOfType(IllegalArgumentException.class)
88+
assertThatExceptionOfType(NoEventSourceForClassException.class)
8989
.isThrownBy(() -> eventSourceManager.getEventSourceFor(Class.class));
9090

9191
// manager is initialized with a controller configured to handle HasMetadata
9292
EventSourceManager manager = initManager();
93-
assertThatExceptionOfType(IllegalArgumentException.class)
93+
assertThatExceptionOfType(NoEventSourceForClassException.class)
9494
.isThrownBy(() -> manager.getEventSourceFor(HasMetadata.class, "unknown_name"));
9595

9696
ManagedInformerEventSource eventSource = mock(ManagedInformerEventSource.class);

Diff for: ‎operator-framework/src/test/java/io/javaoperatorsdk/operator/workflow/getnonactivesecondary/ConfigMapDependentResource.java

+28
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
package io.javaoperatorsdk.operator.workflow.getnonactivesecondary;
2+
3+
4+
import io.fabric8.kubernetes.api.model.ConfigMap;
5+
import io.fabric8.kubernetes.api.model.ObjectMetaBuilder;
6+
import io.javaoperatorsdk.operator.api.reconciler.Context;
7+
import io.javaoperatorsdk.operator.processing.dependent.kubernetes.CRUDKubernetesDependentResource;
8+
9+
public class ConfigMapDependentResource
10+
extends CRUDKubernetesDependentResource<ConfigMap, GetNonActiveSecondaryCustomResource> {
11+
12+
public static final String DATA_KEY = "data";
13+
14+
public ConfigMapDependentResource() {
15+
super(ConfigMap.class);
16+
}
17+
18+
@Override
19+
protected ConfigMap desired(GetNonActiveSecondaryCustomResource primary,
20+
Context<GetNonActiveSecondaryCustomResource> context) {
21+
ConfigMap configMap = new ConfigMap();
22+
configMap.setMetadata(new ObjectMetaBuilder()
23+
.withName(primary.getMetadata().getName())
24+
.withNamespace(primary.getMetadata().getNamespace())
25+
.build());
26+
return configMap;
27+
}
28+
}

Diff for: ‎operator-framework/src/test/java/io/javaoperatorsdk/operator/workflow/getnonactivesecondary/FalseActivationCondition.java

+17
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
package io.javaoperatorsdk.operator.workflow.getnonactivesecondary;
2+
3+
import io.fabric8.openshift.api.model.Route;
4+
import io.javaoperatorsdk.operator.api.reconciler.Context;
5+
import io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResource;
6+
import io.javaoperatorsdk.operator.processing.dependent.workflow.Condition;
7+
8+
public class FalseActivationCondition
9+
implements Condition<Route, GetNonActiveSecondaryCustomResource> {
10+
@Override
11+
public boolean isMet(
12+
DependentResource<Route, GetNonActiveSecondaryCustomResource> dependentResource,
13+
GetNonActiveSecondaryCustomResource primary,
14+
Context<GetNonActiveSecondaryCustomResource> context) {
15+
return false;
16+
}
17+
}

Diff for: ‎operator-framework/src/test/java/io/javaoperatorsdk/operator/workflow/getnonactivesecondary/GetNonActiveSecondaryCustomResource.java

+17
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
package io.javaoperatorsdk.operator.workflow.getnonactivesecondary;
2+
3+
import io.fabric8.kubernetes.api.model.Namespaced;
4+
import io.fabric8.kubernetes.client.CustomResource;
5+
import io.fabric8.kubernetes.model.annotation.Group;
6+
import io.fabric8.kubernetes.model.annotation.ShortNames;
7+
import io.fabric8.kubernetes.model.annotation.Version;
8+
9+
@Group("sample.javaoperatorsdk")
10+
@Version("v1")
11+
@ShortNames("gnas")
12+
public class GetNonActiveSecondaryCustomResource
13+
extends CustomResource<Void, Void>
14+
implements Namespaced {
15+
16+
17+
}

Diff for: ‎operator-framework/src/test/java/io/javaoperatorsdk/operator/workflow/getnonactivesecondary/RouteDependentResource.java

+27
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
package io.javaoperatorsdk.operator.workflow.getnonactivesecondary;
2+
3+
import io.fabric8.kubernetes.api.model.ObjectMetaBuilder;
4+
import io.fabric8.openshift.api.model.Route;
5+
import io.javaoperatorsdk.operator.api.reconciler.Context;
6+
import io.javaoperatorsdk.operator.processing.dependent.kubernetes.CRUDKubernetesDependentResource;
7+
8+
public class RouteDependentResource
9+
extends CRUDKubernetesDependentResource<Route, GetNonActiveSecondaryCustomResource> {
10+
11+
public RouteDependentResource() {
12+
super(Route.class);
13+
}
14+
15+
@Override
16+
protected Route desired(GetNonActiveSecondaryCustomResource primary,
17+
Context<GetNonActiveSecondaryCustomResource> context) {
18+
// basically does not matter since this should not be called
19+
Route route = new Route();
20+
route.setMetadata(new ObjectMetaBuilder()
21+
.withName(primary.getMetadata().getName())
22+
.withNamespace(primary.getMetadata().getNamespace())
23+
.build());
24+
25+
return route;
26+
}
27+
}

Diff for: ‎operator-framework/src/test/java/io/javaoperatorsdk/operator/workflow/getnonactivesecondary/WorkflowActivationConditionIT.java

+40
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
package io.javaoperatorsdk.operator.workflow.getnonactivesecondary;
2+
3+
import org.junit.jupiter.api.Test;
4+
import org.junit.jupiter.api.extension.RegisterExtension;
5+
6+
import io.fabric8.kubernetes.api.model.ObjectMetaBuilder;
7+
import io.javaoperatorsdk.operator.junit.LocallyRunOperatorExtension;
8+
9+
import static org.assertj.core.api.Assertions.assertThat;
10+
import static org.awaitility.Awaitility.await;
11+
12+
public class WorkflowActivationConditionIT {
13+
14+
public static final String TEST_RESOURCE_NAME = "test1";
15+
16+
@RegisterExtension
17+
LocallyRunOperatorExtension extension =
18+
LocallyRunOperatorExtension.builder()
19+
.withReconciler(WorkflowActivationConditionReconciler.class)
20+
.build();
21+
22+
@Test
23+
void reconciledOnVanillaKubernetesDespiteRouteInWorkflow() {
24+
extension.create(testResource());
25+
26+
await().untilAsserted(() -> {
27+
assertThat(extension.getReconcilerOfType(WorkflowActivationConditionReconciler.class)
28+
.getNumberOfReconciliationExecution()).isEqualTo(1);
29+
});
30+
}
31+
32+
private GetNonActiveSecondaryCustomResource testResource() {
33+
var res = new GetNonActiveSecondaryCustomResource();
34+
res.setMetadata(new ObjectMetaBuilder()
35+
.withName(TEST_RESOURCE_NAME)
36+
.build());
37+
return res;
38+
}
39+
40+
}

Diff for: ‎operator-framework/src/test/java/io/javaoperatorsdk/operator/workflow/getnonactivesecondary/WorkflowActivationConditionReconciler.java

+40
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
package io.javaoperatorsdk.operator.workflow.getnonactivesecondary;
2+
3+
import java.util.concurrent.atomic.AtomicInteger;
4+
5+
import io.fabric8.openshift.api.model.Route;
6+
import io.javaoperatorsdk.operator.api.reconciler.Context;
7+
import io.javaoperatorsdk.operator.api.reconciler.ControllerConfiguration;
8+
import io.javaoperatorsdk.operator.api.reconciler.Reconciler;
9+
import io.javaoperatorsdk.operator.api.reconciler.UpdateControl;
10+
import io.javaoperatorsdk.operator.api.reconciler.Workflow;
11+
import io.javaoperatorsdk.operator.api.reconciler.dependent.Dependent;
12+
13+
@Workflow(dependents = {
14+
@Dependent(type = ConfigMapDependentResource.class),
15+
@Dependent(type = RouteDependentResource.class,
16+
activationCondition = FalseActivationCondition.class)
17+
})
18+
@ControllerConfiguration
19+
public class WorkflowActivationConditionReconciler
20+
implements Reconciler<GetNonActiveSecondaryCustomResource> {
21+
22+
private final AtomicInteger numberOfReconciliationExecution = new AtomicInteger(0);
23+
24+
@Override
25+
public UpdateControl<GetNonActiveSecondaryCustomResource> reconcile(
26+
GetNonActiveSecondaryCustomResource resource,
27+
Context<GetNonActiveSecondaryCustomResource> context) {
28+
29+
// should not throw an exception even if the condition is false
30+
var route = context.getSecondaryResource(Route.class);
31+
32+
numberOfReconciliationExecution.incrementAndGet();
33+
34+
return UpdateControl.noUpdate();
35+
}
36+
37+
public int getNumberOfReconciliationExecution() {
38+
return numberOfReconciliationExecution.get();
39+
}
40+
}

0 commit comments

Comments
 (0)