Skip to content

Commit 7fa7fe7

Browse files
author
attila.meszaros
committed
- remove generation awareness
- tests execute
1 parent 19c8977 commit 7fa7fe7

File tree

10 files changed

+15
-164
lines changed

10 files changed

+15
-164
lines changed

‎operator-framework/src/main/java/com/github/containersolutions/operator/ControllerUtils.java

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -28,10 +28,6 @@ static String getDefaultFinalizer(ResourceController controller) {
2828
return getAnnotation(controller).finalizerName();
2929
}
3030

31-
static boolean getGenerationEventProcessing(ResourceController controller) {
32-
return getAnnotation(controller).generationAwareEventProcessing();
33-
}
34-
3531
static <R extends CustomResource> Class<R> getCustomResourceClass(ResourceController controller) {
3632
return (Class<R>) getAnnotation(controller).customResourceClass();
3733
}

‎operator-framework/src/main/java/com/github/containersolutions/operator/Operator.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ private <R extends CustomResource> void registerController(ResourceController<R>
6060
MixedOperation client = k8sClient.customResources(crd, resClass, CustomResourceList.class, getCustomResourceDoneableClass(controller));
6161
EventDispatcher eventDispatcher = new EventDispatcher(controller,
6262
finalizer, new EventDispatcher.CustomResourceFacade(client));
63-
EventScheduler eventScheduler = new EventScheduler(eventDispatcher, retry, ControllerUtils.getGenerationEventProcessing(controller), finalizer);
63+
EventScheduler eventScheduler = new EventScheduler(eventDispatcher, retry, finalizer);
6464
registerWatches(controller, client, resClass, watchAllNamespaces, targetNamespaces, eventScheduler);
6565
}
6666

‎operator-framework/src/main/java/com/github/containersolutions/operator/api/Controller.java

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -19,11 +19,4 @@
1919

2020
String finalizerName() default DEFAULT_FINALIZER;
2121

22-
/**
23-
* If true, will process new event only if generation increased since the last processing, otherwise will
24-
* process all events.
25-
* See generation meta attribute
26-
* <a href="https://kubernetes.io/docs/tasks/access-kubernetes-api/custom-resources/custom-resource-definitions/#status-subresource">here</a>
27-
*/
28-
boolean generationAwareEventProcessing() default true;
2922
}

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

Lines changed: 3 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
package com.github.containersolutions.operator.processing;
22

33

4-
import com.github.containersolutions.operator.ControllerUtils;
54
import com.github.containersolutions.operator.processing.retry.Retry;
65
import io.fabric8.kubernetes.client.CustomResource;
76
import io.fabric8.kubernetes.client.KubernetesClientException;
@@ -41,15 +40,13 @@ public class EventScheduler implements Watcher<CustomResource> {
4140
private final ScheduledThreadPoolExecutor executor;
4241
private final EventStore eventStore = new EventStore();
4342
private final Retry retry;
44-
private final boolean generationAware;
4543
private final String finalizer;
4644

4745
private ReentrantLock lock = new ReentrantLock();
4846

49-
public EventScheduler(EventDispatcher eventDispatcher, Retry retry, boolean generationAware, String finalizer) {
47+
public EventScheduler(EventDispatcher eventDispatcher, Retry retry, String finalizer) {
5048
this.eventDispatcher = eventDispatcher;
5149
this.retry = retry;
52-
this.generationAware = generationAware;
5350
this.finalizer = finalizer;
5451
executor = new ScheduledThreadPoolExecutor(1);
5552
executor.setRemoveOnCancelPolicy(true);
@@ -73,26 +70,12 @@ void scheduleEventFromApi(CustomResourceEvent event) {
7370
log.debug("Skipping delete event");
7471
return;
7572
}
76-
if (generationAware) {
77-
// we have to store the last event for generation aware retries, since if we received new events since
78-
// the execution, which did not have increased generation we will fail automatically on a conflict
79-
// on a retry.
80-
eventStore.addLastEventForGenerationAwareRetry(event);
81-
}
82-
// In case of generation aware processing, we want to replace this even if generation not increased,
83-
// to have the most recent copy of the event.
8473
if (eventStore.containsNotScheduledEvent(event.resourceUid())) {
8574
log.debug("Replacing not scheduled event with actual event." +
8675
" New event: {}", event);
8776
eventStore.addOrReplaceEventAsNotScheduledAndUpdateLastGeneration(event);
8877
return;
8978
}
90-
if (generationAware && !eventStore.hasLargerGenerationThanLastStored(event)
91-
&& eventStore.successfullyProcessedWithFinalizer(event)) {
92-
log.debug("Skipping event, has not larger generation than last stored, actual generation: {}, last stored: {} ",
93-
event.getResource().getMetadata().getGeneration(), eventStore.getLastStoredGeneration(event));
94-
return;
95-
}
9679
if (eventStore.containsEventUnderProcessing(event.resourceUid())) {
9780
log.debug("Scheduling event for later processing since there is an event under processing for same kind." +
9881
" New event: {}", event);
@@ -116,8 +99,7 @@ private void scheduleEventForExecution(CustomResourceEvent event) {
11699
log.warn("Event max retry limit reached. Will be discarded. {}", event);
117100
return;
118101
}
119-
eventStore.addEventUnderProcessingAndUpdateLastGeneration(event);
120-
markStartedProcessingWithFinalizer(event);
102+
eventStore.addEventUnderProcessing(event);
121103
executor.schedule(new EventConsumer(event, eventDispatcher, this),
122104
nextBackOff.get(), TimeUnit.MILLISECONDS);
123105
log.trace("Scheduled task for event: {}", event);
@@ -130,7 +112,6 @@ void eventProcessingFinishedSuccessfully(CustomResourceEvent event) {
130112
try {
131113
lock.lock();
132114
eventStore.removeEventUnderProcessing(event.resourceUid());
133-
markSuccessfullyProcessedWithFinalizer(event);
134115
if (eventStore.containsNotScheduledEvent(event.resourceUid())) {
135116
log.debug("Scheduling recent event for processing: {}", event);
136117
scheduleNotYetScheduledEventForExecution(event.resourceUid());
@@ -149,40 +130,13 @@ void eventProcessingFailed(CustomResourceEvent event) {
149130
scheduleNotYetScheduledEventForExecution(event.resourceUid());
150131
} else {
151132
log.debug("Event processing failed. Attempting to re-schedule the event: {}", event);
152-
if (generationAware) {
153-
CustomResourceEvent eventToRetry = selectEventToRetry(event);
154-
scheduleEventForExecution(eventToRetry);
155-
} else {
156-
scheduleEventForExecution(event);
157-
}
133+
scheduleEventForExecution(event);
158134
}
159135
} finally {
160136
lock.unlock();
161137
}
162138
}
163139

164-
private void markStartedProcessingWithFinalizer(CustomResourceEvent event) {
165-
if (ControllerUtils.hasDefaultFinalizer(event.getResource(),finalizer) && !eventStore.successfullyProcessedWithFinalizer(event)) {
166-
eventStore.markStartedProcessingWithFinalizerOnResource(event);
167-
}
168-
}
169-
170-
private void markSuccessfullyProcessedWithFinalizer (CustomResourceEvent event) {
171-
if (eventStore.startedProcessingWithFinalizerOnResource(event)) {
172-
eventStore.markSuccessfullyProcessedWitFinalizer(event.resourceUid());
173-
}
174-
}
175-
176-
private CustomResourceEvent selectEventToRetry(CustomResourceEvent event) {
177-
CustomResourceEvent lastEvent = eventStore.getReceivedLastEventForGenerationAwareRetry(event.resourceUid());
178-
if (!event.getResource().getMetadata().getResourceVersion()
179-
.equals(lastEvent.getResource().getMetadata().getResourceVersion())) {
180-
return lastEvent;
181-
} else {
182-
return event;
183-
}
184-
}
185-
186140
private void scheduleNotYetScheduledEventForExecution(String uuid) {
187141
CustomResourceEvent notScheduledEvent = eventStore.removeEventNotScheduled(uuid);
188142
scheduleEventForExecution(notScheduledEvent);

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

Lines changed: 2 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -8,10 +8,6 @@ public class EventStore {
88
private final Map<String, CustomResourceEvent> eventsNotScheduled = new HashMap<>();
99
private final Map<String, CustomResourceEvent> eventsUnderProcessing = new HashMap<>();
1010
private final Map<String, Long> lastGeneration = new HashMap<>();
11-
private final Map<String, CustomResourceEvent> receivedLastEventForGenerationAwareRetry = new HashMap<>();
12-
13-
private final Map<String, Boolean> startedProcessingWithFinalizer = new HashMap<>();
14-
private final Map<String, Boolean> successFullyProcessedWithFinalizer = new HashMap<>();
1511

1612
public boolean containsNotScheduledEvent(String uuid) {
1713
return eventsNotScheduled.containsKey(uuid);
@@ -23,69 +19,21 @@ public CustomResourceEvent removeEventNotScheduled(String uid) {
2319

2420
public void addOrReplaceEventAsNotScheduledAndUpdateLastGeneration(CustomResourceEvent event) {
2521
eventsNotScheduled.put(event.resourceUid(), event);
26-
updateLastGeneration(event);
22+
2723
}
2824

2925
public boolean containsEventUnderProcessing(String uuid) {
3026
return eventsUnderProcessing.containsKey(uuid);
3127
}
3228

33-
public void addEventUnderProcessingAndUpdateLastGeneration(CustomResourceEvent event) {
29+
public void addEventUnderProcessing(CustomResourceEvent event) {
3430
eventsUnderProcessing.put(event.resourceUid(), event);
35-
updateLastGeneration(event);
3631
}
37-
3832
public CustomResourceEvent removeEventUnderProcessing(String uid) {
3933
return eventsUnderProcessing.remove(uid);
4034
}
4135

42-
private void updateLastGeneration(CustomResourceEvent event) {
43-
Long generation = event.getResource().getMetadata().getGeneration();
44-
Long storedGeneration = lastGeneration.get(event.getResource().getMetadata().getUid());
45-
if (storedGeneration == null || generation > storedGeneration) {
46-
lastGeneration.put(event.getResource().getMetadata().getUid(), generation);
47-
}
48-
}
49-
50-
public boolean hasLargerGenerationThanLastStored(CustomResourceEvent event) {
51-
return getLastStoredGeneration(event) == null || getLastStoredGeneration(event) <
52-
event.getResource().getMetadata().getGeneration();
53-
}
54-
55-
public Long getLastStoredGeneration(CustomResourceEvent event) {
56-
return lastGeneration.get(event.getResource().getMetadata().getUid());
57-
}
58-
59-
public void addLastEventForGenerationAwareRetry(CustomResourceEvent event) {
60-
receivedLastEventForGenerationAwareRetry.put(event.resourceUid(), event);
61-
}
62-
63-
public CustomResourceEvent getReceivedLastEventForGenerationAwareRetry(String uuid) {
64-
return receivedLastEventForGenerationAwareRetry.get(uuid);
65-
}
66-
67-
public void markStartedProcessingWithFinalizerOnResource(CustomResourceEvent event) {
68-
startedProcessingWithFinalizer.put(event.resourceUid(),true);
69-
}
70-
71-
public boolean startedProcessingWithFinalizerOnResource(CustomResourceEvent event) {
72-
Boolean res = startedProcessingWithFinalizer.get(event.resourceUid());
73-
return res == null ? false : res;
74-
}
75-
76-
public boolean successfullyProcessedWithFinalizer(CustomResourceEvent event) {
77-
Boolean res = successFullyProcessedWithFinalizer.get(event.resourceUid());
78-
return res == null ? false : res;
79-
}
80-
81-
public void markSuccessfullyProcessedWitFinalizer(String uuid) {
82-
successFullyProcessedWithFinalizer.put(uuid, true);
83-
}
84-
8536
public void cleanup(String uuid) {
8637
lastGeneration.remove(uuid);
87-
receivedLastEventForGenerationAwareRetry.remove(uuid);
88-
successFullyProcessedWithFinalizer.remove(uuid);
89-
startedProcessingWithFinalizer.remove(uuid);
9038
}
9139
}

‎operator-framework/src/test/java/com/github/containersolutions/operator/ControllerExecutionIT.java

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -59,11 +59,8 @@ public void eventIsSkippedChangedOnMetadataOnlyUpdate() {
5959
});
6060
}
6161

62-
// We test the scenario when we receive 2 events, while the generation is not increased by the other.
63-
// This will cause a conflict, and on retry the new version of the resource needs to be scheduled
64-
// to avoid repeating conflicts
6562
@Test
66-
public void generationAwareRetryConflict() {
63+
public void retryConflict() {
6764
initAndCleanup(true);
6865
integrationTestSupport.teardownIfSuccess(() -> {
6966
TestCustomResource resource = testCustomResource();
@@ -74,7 +71,7 @@ public void generationAwareRetryConflict() {
7471
integrationTestSupport.getCrOperations().inNamespace(TEST_NAMESPACE).createOrReplace(resource2);
7572

7673
awaitResourcesCreatedOrUpdated();
77-
awaitStatusUpdated(20);
74+
awaitStatusUpdated(5);
7875
});
7976
}
8077

‎operator-framework/src/test/java/com/github/containersolutions/operator/ControllerUtilsTest.java

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@ public void returnsValuesFromControllerAnnotationFinalizer() {
1717
assertEquals(DEFAULT_FINALIZER, ControllerUtils.getDefaultFinalizer(new TestCustomResourceController(null)));
1818
assertEquals(TestCustomResource.class, ControllerUtils.getCustomResourceClass(new TestCustomResourceController(null)));
1919
assertEquals(CRD_NAME, ControllerUtils.getCrdName(new TestCustomResourceController(null)));
20-
assertEquals(true, ControllerUtils.getGenerationEventProcessing(new TestCustomResourceController(null)));
2120
assertTrue(CustomResourceDoneable.class.isAssignableFrom(ControllerUtils.getCustomResourceDoneableClass(new TestCustomResourceController(null))));
2221
}
2322
}

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

Lines changed: 5 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ class EventSchedulerTest {
3737
@SuppressWarnings("unchecked")
3838
private EventDispatcher eventDispatcher = mock(EventDispatcher.class);
3939

40-
private EventScheduler eventScheduler = initScheduler(true);
40+
private EventScheduler eventScheduler = initScheduler();
4141

4242
private List<EventProcessingDetail> eventProcessingList = Collections.synchronizedList(new ArrayList<>());
4343

@@ -77,33 +77,7 @@ public void eventsAreNotExecutedConcurrentlyForSameResource() throws Interrupted
7777
}
7878

7979
@Test
80-
public void generationAwareSchedulingSkipsEventsWithoutIncreasedGeneration() {
81-
normalDispatcherExecution();
82-
CustomResource resource1 = sampleResource();
83-
addFinalizer(resource1);
84-
CustomResource resource2 = sampleResource();
85-
addFinalizer(resource2);
86-
resource2.getMetadata().setResourceVersion("2");
87-
88-
eventScheduler.eventReceived(Watcher.Action.MODIFIED, resource1);
89-
eventScheduler.eventReceived(Watcher.Action.MODIFIED, resource2);
90-
91-
waitTimeForExecution(2);
92-
assertThat(eventProcessingList).hasSize(1)
93-
.matches(list ->
94-
eventProcessingList.get(0).getCustomResource().getMetadata().getResourceVersion().equals("1"));
95-
96-
}
97-
98-
private void addFinalizer(CustomResource resource) {
99-
if (resource.getMetadata().getFinalizers() == null) {
100-
resource.getMetadata().setFinalizers(new ArrayList<>());
101-
}
102-
resource.getMetadata().getFinalizers().add(Controller.DEFAULT_FINALIZER);
103-
}
104-
105-
@Test
106-
public void notGenerationAwareSchedulingProcessesAllEventsRegardlessOfGeneration() {
80+
public void processesAllEventsRegardlessOfGeneration() {
10781
generationUnAwareScheduler();
10882
normalDispatcherExecution();
10983
CustomResource resource1 = sampleResource();
@@ -237,12 +211,11 @@ private Object normalExecution(InvocationOnMock invocation) {
237211
}
238212

239213
private void generationUnAwareScheduler() {
240-
eventScheduler = initScheduler(false);
214+
eventScheduler = initScheduler();
241215
}
242216

243-
private EventScheduler initScheduler(boolean generationAware) {
244-
return new EventScheduler(eventDispatcher,
245-
new GenericRetry().setMaxAttempts(MAX_RETRY_ATTEMPTS).withLinearRetry(), generationAware, finalizer);
217+
private EventScheduler initScheduler() {
218+
return new EventScheduler(eventDispatcher, new GenericRetry().setMaxAttempts(MAX_RETRY_ATTEMPTS).withLinearRetry(), finalizer);
246219
}
247220

248221
private Object exceptionInExecution(InvocationOnMock invocation) {

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ public void updatesSubResourceStatus() {
4040
// wait for sure, there are no more events
4141
waitXms(200);
4242
// there is no event on status update processed
43-
assertThat(integrationTestSupport.numberOfControllerExecutions()).isEqualTo(1);
43+
assertThat(integrationTestSupport.numberOfControllerExecutions()).isEqualTo(2);
4444
});
4545
}
4646

@@ -59,7 +59,7 @@ public void ifNoFinalizerPresentFirstAddsTheFinalizerThenExecutesControllerAgain
5959
// wait for sure, there are no more events
6060
waitXms(200);
6161
// there is no event on status update processed
62-
assertThat(integrationTestSupport.numberOfControllerExecutions()).isEqualTo(1);
62+
assertThat(integrationTestSupport.numberOfControllerExecutions()).isEqualTo(2);
6363
});
6464
}
6565

‎operator-framework/src/test/java/com/github/containersolutions/operator/sample/TestCustomResourceController.java

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -83,18 +83,9 @@ public UpdateControl<TestCustomResource> createOrUpdateResource(TestCustomResour
8383
}
8484
resource.getStatus().setConfigMapStatus("ConfigMap Ready");
8585
}
86-
addOrUpdateAnnotation(resource);
8786
return UpdateControl.updateCustomResource(resource);
8887
}
8988

90-
// for testing purposes we change metadata
91-
private void addOrUpdateAnnotation(TestCustomResource resource) {
92-
if (resource.getMetadata().getAnnotations() == null) {
93-
resource.getMetadata().setAnnotations(new HashMap<>());
94-
}
95-
resource.getMetadata().getAnnotations().put("testAnnotation", LocalDateTime.now().toString());
96-
}
97-
9889
private Map<String, String> configMapData(TestCustomResource resource) {
9990
Map<String, String> data = new HashMap<>();
10091
data.put(resource.getSpec().getKey(), resource.getSpec().getValue());

0 commit comments

Comments
 (0)