Removing events from context #596

csviri · 2021-10-11T08:01:01Z

No description provided.

…Cache WIP

# Conflicts: # operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/Event.java

# Conflicts: # operator-framework-core/src/main/java/io/javaoperatorsdk/operator/Operator.java # operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/DefaultEventHandler.java

…nformer

…/java-operator-sdk into informer-creventsource

# Conflicts: # operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/DefaultEventHandler.java

# Conflicts: # operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/internal/InformerEventSource.java

csviri · 2021-10-11T13:04:33Z

pls review this first: #581

@metacosm @lburgazzoli

# Conflicts: # operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/DefaultEventHandler.java # operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/EventBuffer.java # operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/ExecutionScope.java # operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/DefaultEvent.java # operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/DefaultEventSourceManager.java # operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/internal/CustomResourceEvent.java # operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/internal/CustomResourceEventSource.java # operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/internal/InformerEvent.java # operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/internal/InformerEventSource.java # operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/internal/TimerEvent.java # operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/internal/TimerEventSource.java # operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/DefaultEventHandlerTest.java # operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/EventBufferTest.java # operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/EventDispatcherTest.java # operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/EventListTest.java # operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/internal/CustomResourceEventFilterTest.java # operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/internal/CustomResourceEventSourceTest.java # operator-framework/src/main/java/io/javaoperatorsdk/operator/config/runtime/AnnotationConfiguration.java

metacosm · 2021-10-12T15:35:36Z

...framework-core/src/main/java/io/javaoperatorsdk/operator/processing/DefaultEventHandler.java

      }
      return false;
    }
  }

+  private void markEvent(Event event) {


Shouldn't this be a method on EventMarker?

Did not want to put it there, since Event marker should not know that there is special CustomResourceEvent . Will renamet his method so it's more descriptive.

Ah yes, good point!

...framework-core/src/main/java/io/javaoperatorsdk/operator/processing/DefaultEventHandler.java

operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/EventMarker.java

...framework-core/src/main/java/io/javaoperatorsdk/operator/processing/DefaultEventHandler.java

metacosm · 2021-10-12T15:53:54Z

...tor-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/EventMarkerTest.java

+    eventMarker.markEventReceived(sampleCustomResourceID);
+
+    assertThat(eventMarker.getEventingState(sampleCustomResourceID))
+        .isEqualTo(DELETE_AND_NON_DELETE_EVENT_PRESENT);


Is this situation supposed to happen? When would it?

Good point. So if we state that delete events are retried only when if finalizer is used. Or even in more general the deleteResource method is called in case we use finalizers. This actually might not make sense. This is the case now. So we receive a delete event we don't call the deleteResource method.

Will document this on deleteResource method for now. And remove this test, also change the marker.

Pushed changes, event marker changed quite a bit

...tor-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/EventMarkerTest.java

metacosm · 2021-10-13T12:03:33Z

operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/ResourceController.java

@@ -6,12 +6,15 @@
 public interface ResourceController<R extends CustomResource> {

  /**
+   * Note that this method is used in combination of finalizers. If automatic finalizer handling is


Hmm, that's a significant change. So if you don't have a finalizer, you don't get a chance to clean up associated components? That seems wrong to me, the SDK should always give the users the opportunity to clean things up, regardless of finalizers.

The thing is, that users either use finalizers or all dependent resources are cleaned up using owner reference by kubernetes. Otherwise it's not correct. Typically if the operator is down, the delete event will be missed, and cleanup would not happen anyways. So I think it is actually good to force this pattern.

metacosm · 2021-10-13T12:58:28Z

...framework-core/src/main/java/io/javaoperatorsdk/operator/processing/DefaultEventHandler.java

@@ -188,23 +195,29 @@ void eventProcessingFinished(
          postExecutionControl);
      unsetUnderExecution(executionScope.getCustomResourceID());

-      if (retry != null && postExecutionControl.exceptionDuringExecution()) {
+      // If a delete event present it was received during reconciliation.


Not sure I understand this comment… :(

tried to rephrase it.

...framework-core/src/main/java/io/javaoperatorsdk/operator/processing/DefaultEventHandler.java

operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/EventMarker.java

…tor/processing/EventMarker.java Co-authored-by: Chris Laprun <metacosm@users.noreply.github.com>

# Conflicts: # operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/ResourceController.java # operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/DefaultEventHandler.java # operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/DefaultEventSourceManager.java # operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/DefaultEventHandlerTest.java

* fix: WIP * fix: Addressing Custom Resource by Name and Namespace refactor + Informer Cache WIP * fix: Build is fixed, (test failing) * fix: Test fixes * fix: minor update * fix: EventSourceManager small fix * fix: Unit tests fixed * fix: DefaultEventHandler init from EventSourceManager * fix: custom resource selector test improvement * fix: wip test imrpovements * fix: test improvements * fix: further improvements * fix: build * feature: add mvn jar to gitignore * Exposing CustomResourceEventSource and informers * fix: cleanup * fix: remove caching optimization since it not possible anymore with informer * fix: formatting * refactor: make name/namespace final * feature: Simple label selector support * fix: formatting * fix: code inspection reports * fix: merge from v2 * fix: removed most deprecated apis * wip: started to remove events from variouse layers * fix: progress with implementation and tests * fix: Updated informer mapping to CustomResourceID * fix: imports * fix: decorational changes * fix: event marker unit test * Default Event Handler Unit tests * fix: fixes after merge * fix: changes from code review * fix: method naming * Update operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/EventMarker.java Co-authored-by: Chris Laprun <metacosm@users.noreply.github.com> * Update operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/EventMarker.java Co-authored-by: Chris Laprun <metacosm@users.noreply.github.com> * fix: comment * fix: fixes from merge * fix: remove not used method * fix: formatting Co-authored-by: Chris Laprun <metacosm@gmail.com> Co-authored-by: Chris Laprun <metacosm@users.noreply.github.com>

csviri and others added 30 commits September 30, 2021 17:02

WIP

898e2c2

Addressing Custom Resource by Name and Namespace refactor + Informer …

be4e5cf

…Cache WIP

Build is fixed, (test failing)

b2ab4b9

Test fixes

9145b52

Merge branch 'master' into informer-creventsource

4ef27bf

# Conflicts: # operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/Event.java

minor update

34cc2a1

EventSourceManager small fix

4b15974

Merge branch 'access-event-source-manager' into informer-creventsource

77033e6

Unit tests fixed

e1b5926

fix: DefaultEventHandler init from EventSourceManager

e7d1b99

fix: custom resource selector test improvement

a1f92c6

fix: wip test imrpovements

e928a3e

fix: test improvements

f35a340

fix: further improvements

5d5817e

Merge branch 'v2' into informer-creventsource

544ce35

# Conflicts: # operator-framework-core/src/main/java/io/javaoperatorsdk/operator/Operator.java # operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/DefaultEventHandler.java

fix: build

75ad7d2

feature: add mvn jar to gitignore

7012fb3

Exposing CustomResourceEventSource and informers

3746122

fix: cleanup

f0f2e91

fix: remove caching optimization since it not possible anymore with i…

6638a48

…nformer

fix: formatting

1d786ef

refactor: make name/namespace final

a5343b7

feature: Simple label selector support

9e0430a

Merge branch 'informer-creventsource' of github.com:java-operator-sdk…

7f48b9a

…/java-operator-sdk into informer-creventsource

fix: formatting

0aa29a1

fix: code inspection reports

3ad2fc5

Merge branch 'v2' into informer-creventsource

86b8185

# Conflicts: # operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/DefaultEventHandler.java

fix: merge from v2

92d3ed3

fix: removed most deprecated apis

d194d25

wip: started to remove events from variouse layers

7ee0b7d

csviri added 5 commits October 8, 2021 15:22

fix: progress with implementation and tests

150e875

fix: Updated informer mapping to CustomResourceID

a03cfb9

Merge branch 'informer-creventsource' into removing-events-from-context

6b1d7fe

# Conflicts: # operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/internal/InformerEventSource.java

fix: imports

f97ced5

fix: decorational changes

109b7bc

csviri self-assigned this Oct 11, 2021

csviri marked this pull request as draft October 11, 2021 08:01

fix: event marker unit test

874c25e

snowdrop-bot mentioned this pull request Oct 11, 2021

Removing events from context snowdrop-zen/java-operator-sdk#49

Closed

Default Event Handler Unit tests

ff2b32a

csviri marked this pull request as ready for review October 11, 2021 13:04

csviri added 2 commits October 12, 2021 17:17

fix: fixes after merge

b6c87f0

csviri requested a review from metacosm October 12, 2021 15:27

metacosm reviewed Oct 12, 2021

View reviewed changes

csviri added 2 commits October 13, 2021 13:08

fix: changes from code review

af50089

fix: method naming

58c3f5c

metacosm reviewed Oct 13, 2021

View reviewed changes

csviri and others added 7 commits October 13, 2021 15:07

Update operator-framework-core/src/main/java/io/javaoperatorsdk/opera…

6635b54

…tor/processing/EventMarker.java Co-authored-by: Chris Laprun <metacosm@users.noreply.github.com>

Update operator-framework-core/src/main/java/io/javaoperatorsdk/opera…

f40ee4e

…tor/processing/EventMarker.java Co-authored-by: Chris Laprun <metacosm@users.noreply.github.com>

fix: comment

73c32a8

fix: fixes from merge

e75cad4

fix: remove not used method

b9fc1fc

fix: formatting

0d409ee

csviri merged commit 1dc1993 into v2 Oct 13, 2021

csviri deleted the removing-events-from-context branch October 13, 2021 14:54

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Removing events from context #596

Removing events from context #596

csviri commented Oct 11, 2021

csviri commented Oct 11, 2021

metacosm Oct 12, 2021

csviri Oct 13, 2021

metacosm Oct 13, 2021

metacosm Oct 12, 2021

csviri Oct 13, 2021

csviri Oct 13, 2021

metacosm Oct 13, 2021

csviri Oct 13, 2021

metacosm Oct 13, 2021

csviri Oct 13, 2021

Removing events from context #596

Removing events from context #596

Conversation

csviri commented Oct 11, 2021

csviri commented Oct 11, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment