Skip to content

Commit 718698d

Browse files
csviricsviri
committed
fix for event handling if no finalizer
1 parent 2e4a30d commit 718698d

File tree

3 files changed

+41
-20
lines changed

3 files changed

+41
-20
lines changed

‎operator-framework/src/main/java/io/javaoperatorsdk/operator/Operator.java

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ private <R extends CustomResource> void registerController(ResourceController<R>
6868
controller.init(eventSourceManager);
6969
CustomResourceEventSource customResourceEventSource
7070
= createCustomResourceEventSource(client, customResourceCache, watchAllNamespaces, targetNamespaces,
71-
defaultEventHandler, ControllerUtils.getGenerationEventProcessing(controller));
71+
defaultEventHandler, ControllerUtils.getGenerationEventProcessing(controller), finalizer);
7272
eventSourceManager.registerCustomResourceEventSource(customResourceEventSource);
7373

7474

@@ -81,10 +81,11 @@ private CustomResourceEventSource createCustomResourceEventSource(MixedOperation
8181
boolean watchAllNamespaces,
8282
String[] targetNamespaces,
8383
DefaultEventHandler defaultEventHandler,
84-
boolean generationAware) {
84+
boolean generationAware,
85+
String finalizer) {
8586
CustomResourceEventSource customResourceEventSource = watchAllNamespaces ?
86-
CustomResourceEventSource.customResourceEventSourceForAllNamespaces(customResourceCache, client, generationAware) :
87-
CustomResourceEventSource.customResourceEventSourceForTargetNamespaces(customResourceCache, client, targetNamespaces, generationAware);
87+
CustomResourceEventSource.customResourceEventSourceForAllNamespaces(customResourceCache, client, generationAware, finalizer) :
88+
CustomResourceEventSource.customResourceEventSourceForTargetNamespaces(customResourceCache, client, targetNamespaces, generationAware, finalizer);
8889

8990
customResourceEventSource.setEventHandler(defaultEventHandler);
9091

‎operator-framework/src/main/java/io/javaoperatorsdk/operator/processing/event/internal/CustomResourceEventSource.java

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
import io.fabric8.kubernetes.client.Watcher;
66
import io.fabric8.kubernetes.client.dsl.MixedOperation;
77
import io.fabric8.kubernetes.client.dsl.internal.CustomResourceOperationsImpl;
8+
import io.javaoperatorsdk.operator.ControllerUtils;
89
import io.javaoperatorsdk.operator.processing.CustomResourceCache;
910
import io.javaoperatorsdk.operator.processing.KubernetesResourceUtils;
1011
import io.javaoperatorsdk.operator.processing.event.AbstractEventSource;
@@ -28,24 +29,28 @@ public class CustomResourceEventSource extends AbstractEventSource implements Wa
2829
private MixedOperation client;
2930
private final String[] targetNamespaces;
3031
private final boolean generationAware;
32+
private final String resourceFinalizer;
3133
private final Map<String, Long> lastGenerationProcessedSuccessfully = new ConcurrentHashMap<>();
3234

3335
public static CustomResourceEventSource customResourceEventSourceForAllNamespaces(CustomResourceCache customResourceCache,
34-
MixedOperation client, boolean generationAware) {
35-
return new CustomResourceEventSource(customResourceCache, client, null, generationAware);
36+
MixedOperation client, boolean generationAware,
37+
String resourceFinalizer) {
38+
return new CustomResourceEventSource(customResourceCache, client, null, generationAware, resourceFinalizer);
3639
}
3740

3841
public static CustomResourceEventSource customResourceEventSourceForTargetNamespaces(CustomResourceCache customResourceCache,
3942
MixedOperation client,
40-
String[] namespaces, boolean generationAware) {
41-
return new CustomResourceEventSource(customResourceCache, client, namespaces, generationAware);
43+
String[] namespaces, boolean generationAware,
44+
String resourceFinalizer) {
45+
return new CustomResourceEventSource(customResourceCache, client, namespaces, generationAware, resourceFinalizer);
4246
}
4347

44-
private CustomResourceEventSource(CustomResourceCache customResourceCache, MixedOperation client, String[] targetNamespaces, boolean generationAware) {
48+
private CustomResourceEventSource(CustomResourceCache customResourceCache, MixedOperation client, String[] targetNamespaces, boolean generationAware, String resourceFinalizer) {
4549
this.resourceCache = customResourceCache;
4650
this.client = client;
4751
this.targetNamespaces = targetNamespaces;
4852
this.generationAware = generationAware;
53+
this.resourceFinalizer = resourceFinalizer;
4954
}
5055

5156
private boolean isWatchAllNamespaces() {
@@ -91,7 +96,7 @@ public void eventReceived(Watcher.Action action, CustomResource customResource)
9196
}
9297

9398
private void markLastGenerationProcessed(CustomResource resource) {
94-
if (generationAware) {
99+
if (generationAware && ControllerUtils.hasGivenFinalizer(resource, resourceFinalizer)) {
95100
lastGenerationProcessedSuccessfully.put(KubernetesResourceUtils.getUID(resource), resource.getMetadata().getGeneration());
96101
}
97102
}

‎operator-framework/src/test/java/io/javaoperatorsdk/operator/processing/event/internal/CustomResourceEventSourceTest.java

Lines changed: 25 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -11,18 +11,20 @@
1111

1212
import java.time.LocalDate;
1313
import java.time.LocalDateTime;
14+
import java.util.Arrays;
1415

1516
import static org.junit.jupiter.api.Assertions.*;
1617
import static org.mockito.Mockito.*;
1718

1819
class CustomResourceEventSourceTest {
1920

21+
public static final String FINALIZER = "finalizer";
2022
CustomResourceCache customResourceCache = new CustomResourceCache();
2123
MixedOperation mixedOperation = mock(MixedOperation.class);
2224
EventHandler eventHandler = mock(EventHandler.class);
2325

2426
private CustomResourceEventSource customResourceEventSource =
25-
CustomResourceEventSource.customResourceEventSourceForAllNamespaces(customResourceCache, mixedOperation, true);
27+
CustomResourceEventSource.customResourceEventSourceForAllNamespaces(customResourceCache, mixedOperation, true, FINALIZER);
2628

2729
@BeforeEach
2830
public void setup() {
@@ -32,25 +34,26 @@ public void setup() {
3234
@Test
3335
public void skipsEventHandlingIfGenerationNotIncreased() {
3436
TestCustomResource customResource1 = TestUtils.testCustomResource();
37+
customResource1.getMetadata().setFinalizers(Arrays.asList(FINALIZER));
3538

3639
customResourceEventSource.eventReceived(Watcher.Action.MODIFIED, customResource1);
37-
verify(eventHandler,times(1)).handleEvent(any());
40+
verify(eventHandler, times(1)).handleEvent(any());
3841

3942
customResourceEventSource.eventReceived(Watcher.Action.MODIFIED, customResource1);
40-
verify(eventHandler,times(1)).handleEvent(any());
43+
verify(eventHandler, times(1)).handleEvent(any());
4144
}
4245

4346
@Test
4447
public void dontSkipEventHandlingIfMarkedForDeletion() {
4548
TestCustomResource customResource1 = TestUtils.testCustomResource();
4649

4750
customResourceEventSource.eventReceived(Watcher.Action.MODIFIED, customResource1);
48-
verify(eventHandler,times(1)).handleEvent(any());
51+
verify(eventHandler, times(1)).handleEvent(any());
4952

5053
// mark for deletion
5154
customResource1.getMetadata().setDeletionTimestamp(LocalDateTime.now().toString());
5255
customResourceEventSource.eventReceived(Watcher.Action.MODIFIED, customResource1);
53-
verify(eventHandler,times(2)).handleEvent(any());
56+
verify(eventHandler, times(2)).handleEvent(any());
5457
}
5558

5659

@@ -59,26 +62,38 @@ public void normalExecutionIfGenerationChanges() {
5962
TestCustomResource customResource1 = TestUtils.testCustomResource();
6063

6164
customResourceEventSource.eventReceived(Watcher.Action.MODIFIED, customResource1);
62-
verify(eventHandler,times(1)).handleEvent(any());
65+
verify(eventHandler, times(1)).handleEvent(any());
6366

6467
customResource1.getMetadata().setGeneration(2L);
6568
customResourceEventSource.eventReceived(Watcher.Action.MODIFIED, customResource1);
66-
verify(eventHandler,times(2)).handleEvent(any());
69+
verify(eventHandler, times(2)).handleEvent(any());
6770
}
6871

6972
@Test
7073
public void handlesAllEventIfNotGenerationAware() {
7174
customResourceEventSource =
72-
CustomResourceEventSource.customResourceEventSourceForAllNamespaces(customResourceCache, mixedOperation, false);
75+
CustomResourceEventSource.customResourceEventSourceForAllNamespaces(customResourceCache,
76+
mixedOperation, false, FINALIZER);
7377
setup();
7478

7579
TestCustomResource customResource1 = TestUtils.testCustomResource();
7680

7781
customResourceEventSource.eventReceived(Watcher.Action.MODIFIED, customResource1);
78-
verify(eventHandler,times(1)).handleEvent(any());
82+
verify(eventHandler, times(1)).handleEvent(any());
7983

8084
customResourceEventSource.eventReceived(Watcher.Action.MODIFIED, customResource1);
81-
verify(eventHandler,times(2)).handleEvent(any());
85+
verify(eventHandler, times(2)).handleEvent(any());
86+
}
87+
88+
@Test
89+
public void eventNotMarkedForLastGenerationIfNoFinalizer() {
90+
TestCustomResource customResource1 = TestUtils.testCustomResource();
91+
92+
customResourceEventSource.eventReceived(Watcher.Action.MODIFIED, customResource1);
93+
verify(eventHandler, times(1)).handleEvent(any());
94+
95+
customResourceEventSource.eventReceived(Watcher.Action.MODIFIED, customResource1);
96+
verify(eventHandler, times(2)).handleEvent(any());
8297
}
8398

8499
}

0 commit comments

Comments
 (0)