Skip to content

Commit 2371b4f

Browse files
csviricsviri
authored
Event filter fix (#1129)
1 parent d02c2fe commit 2371b4f

File tree

11 files changed

+202
-59
lines changed

11 files changed

+202
-59
lines changed

‎operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/dependent/RecentOperationEventFilter.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,6 @@ public interface RecentOperationEventFilter<R> extends RecentOperationCacheFille
66

77
void prepareForCreateOrUpdateEventFiltering(ResourceID resourceID, R resource);
88

9-
void cleanupOnCreateOrUpdateEventFiltering(ResourceID resourceID, R resource);
9+
void cleanupOnCreateOrUpdateEventFiltering(ResourceID resourceID);
1010

1111
}

‎operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/AbstractEventSourceHolderDependentResource.java

Lines changed: 0 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,9 @@
11
package io.javaoperatorsdk.operator.processing.dependent;
22

33
import io.fabric8.kubernetes.api.model.HasMetadata;
4-
import io.javaoperatorsdk.operator.api.reconciler.Context;
54
import io.javaoperatorsdk.operator.api.reconciler.EventSourceContext;
65
import io.javaoperatorsdk.operator.api.reconciler.dependent.EventSourceProvider;
76
import io.javaoperatorsdk.operator.api.reconciler.dependent.RecentOperationCacheFiller;
8-
import io.javaoperatorsdk.operator.api.reconciler.dependent.RecentOperationEventFilter;
97
import io.javaoperatorsdk.operator.processing.event.ResourceID;
108
import io.javaoperatorsdk.operator.processing.event.source.EventSource;
119
import io.javaoperatorsdk.operator.processing.event.source.ResourceEventSource;
@@ -14,7 +12,6 @@ public abstract class AbstractEventSourceHolderDependentResource<R, P extends Ha
1412
extends AbstractDependentResource<R, P>
1513
implements EventSourceProvider<P> {
1614
private T eventSource;
17-
private boolean isFilteringEventSource;
1815
private boolean isCacheFillerEventSource;
1916

2017
public EventSource initEventSource(EventSourceContext<P> context) {
@@ -25,9 +22,6 @@ public EventSource initEventSource(EventSourceContext<P> context) {
2522
eventSource = createEventSource(context);
2623
}
2724

28-
// but we still need to record which interfaces the event source implements even if it has
29-
// already been set before this method is called
30-
isFilteringEventSource = eventSource instanceof RecentOperationEventFilter;
3125
isCacheFillerEventSource = eventSource instanceof RecentOperationCacheFiller;
3226
return eventSource;
3327
}
@@ -42,33 +36,6 @@ protected T eventSource() {
4236
return eventSource;
4337
}
4438

45-
protected R handleCreate(R desired, P primary, Context<P> context) {
46-
ResourceID resourceID = ResourceID.fromResource(primary);
47-
R created = null;
48-
try {
49-
prepareEventFiltering(desired, resourceID);
50-
created = super.handleCreate(desired, primary, context);
51-
return created;
52-
} catch (RuntimeException e) {
53-
cleanupAfterEventFiltering(desired, resourceID, created);
54-
throw e;
55-
}
56-
}
57-
58-
protected R handleUpdate(R actual, R desired, P primary, Context<P> context) {
59-
ResourceID resourceID = ResourceID.fromResource(primary);
60-
R updated = null;
61-
try {
62-
prepareEventFiltering(desired, resourceID);
63-
updated = super.handleUpdate(actual, desired, primary, context);
64-
return updated;
65-
} catch (RuntimeException e) {
66-
cleanupAfterEventFiltering(desired, resourceID, updated);
67-
throw e;
68-
}
69-
}
70-
71-
7239
protected void onCreated(ResourceID primaryResourceId, R created) {
7340
if (isCacheFillerEventSource) {
7441
recentOperationCacheFiller().handleRecentResourceCreate(primaryResourceId, created);
@@ -81,22 +48,6 @@ protected void onUpdated(ResourceID primaryResourceId, R updated, R actual) {
8148
}
8249
}
8350

84-
private void prepareEventFiltering(R desired, ResourceID resourceID) {
85-
if (isFilteringEventSource) {
86-
recentOperationEventFilter().prepareForCreateOrUpdateEventFiltering(resourceID, desired);
87-
}
88-
}
89-
90-
private void cleanupAfterEventFiltering(R desired, ResourceID resourceID, R created) {
91-
if (isFilteringEventSource) {
92-
recentOperationEventFilter().cleanupOnCreateOrUpdateEventFiltering(resourceID, created);
93-
}
94-
}
95-
96-
@SuppressWarnings("unchecked")
97-
private RecentOperationEventFilter<R> recentOperationEventFilter() {
98-
return (RecentOperationEventFilter<R>) eventSource;
99-
}
10051

10152
@SuppressWarnings("unchecked")
10253
private RecentOperationCacheFiller<R> recentOperationCacheFiller() {

‎operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/KubernetesDependentResource.java

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,33 @@ public void configureWith(InformerEventSource<R, P> informerEventSource) {
8181
setEventSource(informerEventSource);
8282
}
8383

84+
85+
protected R handleCreate(R desired, P primary, Context<P> context) {
86+
ResourceID resourceID = ResourceID.fromResource(desired);
87+
R created = null;
88+
try {
89+
prepareEventFiltering(desired, resourceID);
90+
created = super.handleCreate(desired, primary, context);
91+
return created;
92+
} catch (RuntimeException e) {
93+
cleanupAfterEventFiltering(resourceID);
94+
throw e;
95+
}
96+
}
97+
98+
protected R handleUpdate(R actual, R desired, P primary, Context<P> context) {
99+
ResourceID resourceID = ResourceID.fromResource(desired);
100+
R updated = null;
101+
try {
102+
prepareEventFiltering(desired, resourceID);
103+
updated = super.handleUpdate(actual, desired, primary, context);
104+
return updated;
105+
} catch (RuntimeException e) {
106+
cleanupAfterEventFiltering(resourceID);
107+
throw e;
108+
}
109+
}
110+
84111
@SuppressWarnings("unused")
85112
public R create(R target, P primary, Context<P> context) {
86113
return prepare(target, primary, "Creating").create(target);
@@ -152,4 +179,13 @@ public KubernetesClient getKubernetesClient() {
152179
protected R desired(P primary, Context<P> context) {
153180
return super.desired(primary, context);
154181
}
182+
183+
private void prepareEventFiltering(R desired, ResourceID resourceID) {
184+
eventSource().prepareForCreateOrUpdateEventFiltering(resourceID, desired);
185+
}
186+
187+
private void cleanupAfterEventFiltering(ResourceID resourceID) {
188+
eventSource().cleanupOnCreateOrUpdateEventFiltering(resourceID);
189+
}
190+
155191
}

‎operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerEventSource.java

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -231,11 +231,10 @@ public synchronized void prepareForCreateOrUpdateEventFiltering(ResourceID resou
231231
* Mean to be called to clean up in case of an exception from the client. Usually in a catch
232232
* block.
233233
*
234-
* @param resource handled by the informer
234+
* @param resourceID to cleanup
235235
*/
236236
@Override
237-
public synchronized void cleanupOnCreateOrUpdateEventFiltering(ResourceID resourceID,
238-
R resource) {
237+
public synchronized void cleanupOnCreateOrUpdateEventFiltering(ResourceID resourceID) {
239238
log.debug("Stopping event recording for: {}", resourceID);
240239
eventRecorder.stopEventRecording(resourceID);
241240
}

‎operator-framework/src/test/java/io/javaoperatorsdk/operator/OperationEventFilterIT.java

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
package io.javaoperatorsdk.operator;
2+
3+
import java.time.Duration;
4+
5+
import org.junit.jupiter.api.Test;
6+
import org.junit.jupiter.api.extension.RegisterExtension;
7+
8+
import io.fabric8.kubernetes.api.model.ConfigMap;
9+
import io.fabric8.kubernetes.api.model.ObjectMeta;
10+
import io.javaoperatorsdk.operator.junit.OperatorExtension;
11+
import io.javaoperatorsdk.operator.sample.operationeventfiltering.ConfigMapDependentResource;
12+
import io.javaoperatorsdk.operator.sample.operationeventfiltering.OperationEventFilterCustomResource;
13+
import io.javaoperatorsdk.operator.sample.operationeventfiltering.OperationEventFilterCustomResourceSpec;
14+
import io.javaoperatorsdk.operator.sample.operationeventfiltering.OperationEventFilterCustomResourceTestReconciler;
15+
16+
import static org.assertj.core.api.Assertions.assertThat;
17+
import static org.awaitility.Awaitility.await;
18+
19+
class OperationEventFilterIT {
20+
21+
public static final String TEST = "test";
22+
public static final String SPEC_VAL_1 = "val1";
23+
public static final String SPEC_VAL_2 = "val2";
24+
25+
@RegisterExtension
26+
OperatorExtension operator =
27+
OperatorExtension.builder()
28+
.withReconciler(new OperationEventFilterCustomResourceTestReconciler())
29+
.build();
30+
31+
@Test
32+
void reconcileNotTriggeredWithDependentResourceCreateOrUpdate() {
33+
var resource = operator.create(OperationEventFilterCustomResource.class, createTestResource());
34+
35+
await().pollDelay(Duration.ofSeconds(1)).atMost(Duration.ofSeconds(3))
36+
.until(
37+
() -> ((OperationEventFilterCustomResourceTestReconciler) operator.getFirstReconciler())
38+
.getNumberOfExecutions() == 1);
39+
assertThat(operator.get(ConfigMap.class, TEST).getData())
40+
.containsEntry(ConfigMapDependentResource.KEY, SPEC_VAL_1);
41+
42+
resource.getSpec().setValue(SPEC_VAL_2);
43+
operator.replace(OperationEventFilterCustomResource.class, resource);
44+
45+
await().pollDelay(Duration.ofSeconds(1)).atMost(Duration.ofSeconds(3))
46+
.until(
47+
() -> ((OperationEventFilterCustomResourceTestReconciler) operator.getFirstReconciler())
48+
.getNumberOfExecutions() == 2);
49+
assertThat(operator.get(ConfigMap.class, TEST).getData())
50+
.containsEntry(ConfigMapDependentResource.KEY, SPEC_VAL_2);
51+
}
52+
53+
54+
private OperationEventFilterCustomResource createTestResource() {
55+
OperationEventFilterCustomResource cr = new OperationEventFilterCustomResource();
56+
cr.setMetadata(new ObjectMeta());
57+
cr.getMetadata().setName(TEST);
58+
cr.setSpec(new OperationEventFilterCustomResourceSpec());
59+
cr.getSpec().setValue(SPEC_VAL_1);
60+
return cr;
61+
}
62+
63+
}

‎operator-framework/src/test/java/io/javaoperatorsdk/operator/OrderedManagedDependentIT.java

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -28,14 +28,12 @@ void managedDependentsAreReconciledInOrder() {
2828

2929
await().atMost(Duration.ofSeconds(5))
3030
.until(() -> ((OrderedManagedDependentTestReconciler) operator.getFirstReconciler())
31-
.getNumberOfExecutions() >= 1);
32-
// todo change to more precise values when event filtering is fixed
33-
// assertThat(OrderedManagedDependentTestReconciler.dependentExecution).hasSize(4);
31+
.getNumberOfExecutions() == 1);
32+
3433
assertThat(OrderedManagedDependentTestReconciler.dependentExecution.get(0))
3534
.isEqualTo(ConfigMapDependentResource1.class);
3635
assertThat(OrderedManagedDependentTestReconciler.dependentExecution.get(1))
3736
.isEqualTo(ConfigMapDependentResource2.class);
38-
3937
}
4038

4139

‎operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/createupdateeventfilter/CreateUpdateEventFilterTestReconciler.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ public UpdateControl<CreateUpdateEventFilterTestCustomResource> reconcile(
5656
informerEventSource.handleRecentResourceCreate(resourceID, configMap);
5757
} catch (RuntimeException e) {
5858
informerEventSource
59-
.cleanupOnCreateOrUpdateEventFiltering(resourceID, configMapToCreate);
59+
.cleanupOnCreateOrUpdateEventFiltering(resourceID);
6060
throw e;
6161
}
6262
} else {
@@ -76,7 +76,7 @@ public UpdateControl<CreateUpdateEventFilterTestCustomResource> reconcile(
7676
newConfigMap, configMap);
7777
} catch (RuntimeException e) {
7878
informerEventSource
79-
.cleanupOnCreateOrUpdateEventFiltering(resourceID, configMap);
79+
.cleanupOnCreateOrUpdateEventFiltering(resourceID);
8080
throw e;
8181
}
8282
}

‎operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/operationeventfiltering/ConfigMapDependentResource.java

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
package io.javaoperatorsdk.operator.sample.operationeventfiltering;
2+
3+
import java.util.HashMap;
4+
5+
import io.fabric8.kubernetes.api.model.ConfigMap;
6+
import io.fabric8.kubernetes.api.model.ObjectMeta;
7+
import io.javaoperatorsdk.operator.api.reconciler.Context;
8+
import io.javaoperatorsdk.operator.processing.dependent.kubernetes.CRUKubernetesDependentResource;
9+
10+
public class ConfigMapDependentResource extends
11+
CRUKubernetesDependentResource<ConfigMap, OperationEventFilterCustomResource> {
12+
13+
public static final String KEY = "key1";
14+
15+
public ConfigMapDependentResource() {
16+
super(ConfigMap.class);
17+
}
18+
19+
@Override
20+
protected ConfigMap desired(OperationEventFilterCustomResource primary,
21+
Context<OperationEventFilterCustomResource> context) {
22+
23+
ConfigMap configMap = new ConfigMap();
24+
configMap.setMetadata(new ObjectMeta());
25+
configMap.getMetadata().setName(primary.getMetadata().getName());
26+
configMap.getMetadata().setNamespace(primary.getMetadata().getNamespace());
27+
HashMap<String, String> data = new HashMap<>();
28+
data.put(KEY, primary.getSpec().getValue());
29+
configMap.setData(data);
30+
return configMap;
31+
}
32+
}

‎operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/operationeventfiltering/OperationEventFilterCustomResource.java

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
package io.javaoperatorsdk.operator.sample.operationeventfiltering;
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.Kind;
7+
import io.fabric8.kubernetes.model.annotation.ShortNames;
8+
import io.fabric8.kubernetes.model.annotation.Version;
9+
10+
@Group("sample.javaoperatorsdk")
11+
@Version("v1")
12+
@Kind("OperationEventFilterCustomResource")
13+
@ShortNames("oef")
14+
public class OperationEventFilterCustomResource
15+
extends CustomResource<OperationEventFilterCustomResourceSpec, String>
16+
implements Namespaced {
17+
}

‎operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/operationeventfiltering/OperationEventFilterCustomResourceSpec.java

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
package io.javaoperatorsdk.operator.sample.operationeventfiltering;
2+
3+
public class OperationEventFilterCustomResourceSpec {
4+
5+
private String value;
6+
7+
public String getValue() {
8+
return value;
9+
}
10+
11+
public OperationEventFilterCustomResourceSpec setValue(String value) {
12+
this.value = value;
13+
return this;
14+
}
15+
}

‎operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/operationeventfiltering/OperationEventFilterCustomResourceTestReconciler.java

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
package io.javaoperatorsdk.operator.sample.operationeventfiltering;
2+
3+
import java.util.concurrent.atomic.AtomicInteger;
4+
5+
import io.javaoperatorsdk.operator.api.reconciler.*;
6+
import io.javaoperatorsdk.operator.api.reconciler.dependent.Dependent;
7+
import io.javaoperatorsdk.operator.support.TestExecutionInfoProvider;
8+
9+
@ControllerConfiguration(
10+
namespaces = Constants.WATCH_CURRENT_NAMESPACE,
11+
dependents = {
12+
@Dependent(type = ConfigMapDependentResource.class),
13+
})
14+
public class OperationEventFilterCustomResourceTestReconciler
15+
implements Reconciler<OperationEventFilterCustomResource>,
16+
TestExecutionInfoProvider {
17+
18+
private final AtomicInteger numberOfExecutions = new AtomicInteger(0);
19+
20+
@Override
21+
public UpdateControl<OperationEventFilterCustomResource> reconcile(
22+
OperationEventFilterCustomResource resource,
23+
Context<OperationEventFilterCustomResource> context) {
24+
numberOfExecutions.addAndGet(1);
25+
return UpdateControl.noUpdate();
26+
}
27+
28+
public int getNumberOfExecutions() {
29+
return numberOfExecutions.get();
30+
}
31+
32+
}

0 commit comments

Comments
 (0)