- remove generation awareness

- tests execute
- tests, comments

Author: attila.meszaros

operator-framework/src/main/java/com/github/containersolutions/operator/processing/EventScheduler.java

@@ -65,21 +65,19 @@ void scheduleEventFromApi(CustomResourceEvent event) {
             lock.lock();
             log.debug("Scheduling event from Api: {}", event);
             if (event.getAction() == Action.DELETED) {
-                // This removes data from memory for deleted resource (prevent memory leak).
-                eventStore.cleanup(event.resourceUid());
                 log.debug("Skipping delete event");
                 return;
             }
             if (eventStore.containsNotScheduledEvent(event.resourceUid())) {
                 log.debug("Replacing not scheduled event with actual event." +
                         " New event: {}", event);
-                eventStore.addOrReplaceEventAsNotScheduledAndUpdateLastGeneration(event);
+                eventStore.addOrReplaceEventAsNotScheduled(event);
                 return;
             }
             if (eventStore.containsEventUnderProcessing(event.resourceUid())) {
                 log.debug("Scheduling event for later processing since there is an event under processing for same kind." +
                         " New event: {}", event);
-                eventStore.addOrReplaceEventAsNotScheduledAndUpdateLastGeneration(event);
+                eventStore.addOrReplaceEventAsNotScheduled(event);
                 return;
             }
             scheduleEventForExecution(event);

operator-framework/src/main/java/com/github/containersolutions/operator/processing/EventStore.java

@@ -7,7 +7,6 @@ public class EventStore {
 
     private final Map<String, CustomResourceEvent> eventsNotScheduled = new HashMap<>();
     private final Map<String, CustomResourceEvent> eventsUnderProcessing = new HashMap<>();
-    private final Map<String, Long> lastGeneration = new HashMap<>();
 
     public boolean containsNotScheduledEvent(String uuid) {
         return eventsNotScheduled.containsKey(uuid);
@@ -17,9 +16,8 @@ public CustomResourceEvent removeEventNotScheduled(String uid) {
         return eventsNotScheduled.remove(uid);
     }
 
-    public void addOrReplaceEventAsNotScheduledAndUpdateLastGeneration(CustomResourceEvent event) {
+    public void addOrReplaceEventAsNotScheduled(CustomResourceEvent event) {
         eventsNotScheduled.put(event.resourceUid(), event);
-
     }
 
     public boolean containsEventUnderProcessing(String uuid) {
@@ -33,7 +31,4 @@ public CustomResourceEvent removeEventUnderProcessing(String uid) {
         return eventsUnderProcessing.remove(uid);
     }
 
-    public void cleanup(String uuid) {
-        lastGeneration.remove(uuid);
-    }
 }

operator-framework/src/test/java/com/github/containersolutions/operator/EventSchedulerTest.java

@@ -62,7 +62,6 @@ public void eventsAreNotExecutedConcurrentlyForSameResource() throws Interrupted
         CustomResource resource1 = sampleResource();
         CustomResource resource2 = sampleResource();
         resource2.getMetadata().setResourceVersion("2");
-        resource2.getMetadata().setGeneration(2l);
 
         eventScheduler.eventReceived(Watcher.Action.MODIFIED, resource1);
         eventScheduler.eventReceived(Watcher.Action.MODIFIED, resource2);
@@ -78,7 +77,6 @@ public void eventsAreNotExecutedConcurrentlyForSameResource() throws Interrupted
 
     @Test
     public void processesAllEventsRegardlessOfGeneration() {
-        generationUnAwareScheduler();
         normalDispatcherExecution();
         CustomResource resource1 = sampleResource();
         CustomResource resource2 = sampleResource();
@@ -104,10 +102,8 @@ public void onlyLastEventIsScheduledIfMoreReceivedDuringAndExecution() {
         CustomResource resource1 = sampleResource();
         CustomResource resource2 = sampleResource();
         resource2.getMetadata().setResourceVersion("2");
-        resource2.getMetadata().setGeneration(2l);
         CustomResource resource3 = sampleResource();
         resource3.getMetadata().setResourceVersion("3");
-        resource3.getMetadata().setGeneration(3l);
 
         eventScheduler.eventReceived(Watcher.Action.MODIFIED, resource1);
         eventScheduler.eventReceived(Watcher.Action.MODIFIED, resource2);
@@ -147,7 +143,6 @@ public void processesNewEventIfItIsReceivedAfterExecutionInError() {
         CustomResource resource1 = sampleResource();
         CustomResource resource2 = sampleResource();
         resource2.getMetadata().setResourceVersion("2");
-        resource2.getMetadata().setGeneration(2l);
 
         doAnswer(this::exceptionInExecution).when(eventDispatcher).handleEvent(ArgumentMatchers.argThat(new ArgumentMatcher<CustomResourceEvent>() {
             @Override
@@ -210,9 +205,9 @@ private Object normalExecution(InvocationOnMock invocation) {
         }
     }
 
-    private void generationUnAwareScheduler() {
-        eventScheduler = initScheduler();
-    }
+
+
+
 
     private EventScheduler initScheduler() {
         return new EventScheduler(eventDispatcher, new GenericRetry().setMaxAttempts(MAX_RETRY_ATTEMPTS).withLinearRetry(), finalizer);

operator-framework/src/test/java/com/github/containersolutions/operator/SubResourceUpdateIT.java

@@ -46,7 +46,7 @@ public void updatesSubResourceStatus() {
 
     /**
      * Note that we check on controller impl if there is finalizer on execution.
-     * */
+     */
     @Test
     public void ifNoFinalizerPresentFirstAddsTheFinalizerThenExecutesControllerAgain() {
         initAndCleanup();
@@ -63,10 +63,37 @@ public void ifNoFinalizerPresentFirstAddsTheFinalizerThenExecutesControllerAgain
         });
     }
 
+    /**
+     * Not that here status sub-resource update will fail on optimistic locking. This solves a tricky situation:
+     * If this would not happen (no optimistic locking on status sub-resource)  we could receive and store an event
+     * while processing the controller method. But this event would always fail since its resource version is outdated
+     * already.
+     * */
+    @Test
+    public void updateCustomResourceAfterSubResourceChange() {
+        initAndCleanup();
+        integrationTestSupport.teardownIfSuccess(() -> {
+            SubResourceTestCustomResource resource = createTestCustomResource("1");
+            integrationTestSupport.getCrOperations().inNamespace(TEST_NAMESPACE).create(resource);
+
+            resource.getSpec().setValue("new value");
+            integrationTestSupport.getCrOperations().inNamespace(TEST_NAMESPACE).createOrReplace(resource);
+
+            awaitStatusUpdated(resource.getMetadata().getName());
+
+            // wait for sure, there are no more events
+            waitXms(200);
+            // there is no event on status update processed
+            assertThat(integrationTestSupport.numberOfControllerExecutions()).isEqualTo(3);
+        });
+
+    }
+
     void awaitStatusUpdated(String name) {
         await("cr status updated").atMost(5, TimeUnit.SECONDS)
                 .untilAsserted(() -> {
                     SubResourceTestCustomResource cr = (SubResourceTestCustomResource) integrationTestSupport.getCrOperations().inNamespace(TEST_NAMESPACE).withName(name).get();
+                    assertThat(cr.getMetadata().getFinalizers()).hasSize(1);
                     assertThat(cr).isNotNull();
                     assertThat(cr.getStatus()).isNotNull();
                     assertThat(cr.getStatus().getState()).isEqualTo(SUCCESS);