Skip to content

Commit 19c8977

Browse files
author
attila.meszaros
committed
- improve test
- improve event handling
1 parent 5d2b773 commit 19c8977

File tree

4 files changed

+39
-18
lines changed

4 files changed

+39
-18
lines changed

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

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -79,9 +79,7 @@ private void updateCustomResource(CustomResource updatedResource) {
7979
log.debug("Updating resource: {} with version: {}", updatedResource.getMetadata().getName(),
8080
updatedResource.getMetadata().getResourceVersion());
8181
log.trace("Resource before update: {}", updatedResource);
82-
addFinalizerIfNotPresent(updatedResource);
8382
replace(updatedResource);
84-
log.trace("Resource after update: {}", updatedResource);
8583
}
8684

8785

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

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -88,16 +88,11 @@ void scheduleEventFromApi(CustomResourceEvent event) {
8888
return;
8989
}
9090
if (generationAware && !eventStore.hasLargerGenerationThanLastStored(event)
91-
&& eventStore.lastProcessingHadFinalizer(event)) {
91+
&& eventStore.successfullyProcessedWithFinalizer(event)) {
9292
log.debug("Skipping event, has not larger generation than last stored, actual generation: {}, last stored: {} ",
9393
event.getResource().getMetadata().getGeneration(), eventStore.getLastStoredGeneration(event));
9494
return;
9595
}
96-
if (generationAware && !eventStore.lastProcessingHadFinalizer(event)
97-
&& ControllerUtils.hasDefaultFinalizer(event.getResource(), finalizer)) {
98-
eventStore.markProcessedWitFinalizer(event.resourceUid(), true);
99-
}
100-
10196
if (eventStore.containsEventUnderProcessing(event.resourceUid())) {
10297
log.debug("Scheduling event for later processing since there is an event under processing for same kind." +
10398
" New event: {}", event);
@@ -122,6 +117,7 @@ private void scheduleEventForExecution(CustomResourceEvent event) {
122117
return;
123118
}
124119
eventStore.addEventUnderProcessingAndUpdateLastGeneration(event);
120+
markStartedProcessingWithFinalizer(event);
125121
executor.schedule(new EventConsumer(event, eventDispatcher, this),
126122
nextBackOff.get(), TimeUnit.MILLISECONDS);
127123
log.trace("Scheduled task for event: {}", event);
@@ -134,6 +130,7 @@ void eventProcessingFinishedSuccessfully(CustomResourceEvent event) {
134130
try {
135131
lock.lock();
136132
eventStore.removeEventUnderProcessing(event.resourceUid());
133+
markSuccessfullyProcessedWithFinalizer(event);
137134
if (eventStore.containsNotScheduledEvent(event.resourceUid())) {
138135
log.debug("Scheduling recent event for processing: {}", event);
139136
scheduleNotYetScheduledEventForExecution(event.resourceUid());
@@ -164,6 +161,18 @@ void eventProcessingFailed(CustomResourceEvent event) {
164161
}
165162
}
166163

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+
167176
private CustomResourceEvent selectEventToRetry(CustomResourceEvent event) {
168177
CustomResourceEvent lastEvent = eventStore.getReceivedLastEventForGenerationAwareRetry(event.resourceUid());
169178
if (!event.getResource().getMetadata().getResourceVersion()

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

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,9 @@ public class EventStore {
99
private final Map<String, CustomResourceEvent> eventsUnderProcessing = new HashMap<>();
1010
private final Map<String, Long> lastGeneration = new HashMap<>();
1111
private final Map<String, CustomResourceEvent> receivedLastEventForGenerationAwareRetry = new HashMap<>();
12-
private final Map<String, Boolean> lastProcessingWithFinalizer = new HashMap<>();
12+
13+
private final Map<String, Boolean> startedProcessingWithFinalizer = new HashMap<>();
14+
private final Map<String, Boolean> successFullyProcessedWithFinalizer = new HashMap<>();
1315

1416
public boolean containsNotScheduledEvent(String uuid) {
1517
return eventsNotScheduled.containsKey(uuid);
@@ -62,18 +64,28 @@ public CustomResourceEvent getReceivedLastEventForGenerationAwareRetry(String uu
6264
return receivedLastEventForGenerationAwareRetry.get(uuid);
6365
}
6466

65-
public boolean lastProcessingHadFinalizer(CustomResourceEvent event) {
66-
Boolean res = lastProcessingWithFinalizer.get(event.resourceUid());
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());
6778
return res == null ? false : res;
6879
}
6980

70-
public void markProcessedWitFinalizer(String uuid, boolean hadFinalizer) {
71-
lastProcessingWithFinalizer.put(uuid, hadFinalizer);
81+
public void markSuccessfullyProcessedWitFinalizer(String uuid) {
82+
successFullyProcessedWithFinalizer.put(uuid, true);
7283
}
7384

7485
public void cleanup(String uuid) {
7586
lastGeneration.remove(uuid);
7687
receivedLastEventForGenerationAwareRetry.remove(uuid);
77-
lastProcessingWithFinalizer.remove(uuid);
88+
successFullyProcessedWithFinalizer.remove(uuid);
89+
startedProcessingWithFinalizer.remove(uuid);
7890
}
7991
}

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

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -19,10 +19,11 @@
1919
import static org.assertj.core.api.Assertions.assertThat;
2020
import static org.awaitility.Awaitility.await;
2121

22-
@TestInstance(TestInstance.Lifecycle.PER_CLASS)
22+
@TestInstance(TestInstance.Lifecycle.PER_METHOD)
2323
public class ControllerExecutionIT {
2424

2525
private final static Logger log = LoggerFactory.getLogger(ControllerExecutionIT.class);
26+
public static final String TEST_CUSTOM_RESOURCE_NAME = "test-custom-resource";
2627
private IntegrationTestSupport integrationTestSupport = new IntegrationTestSupport();
2728

2829
public void initAndCleanup(boolean controllerStatusUpdate) {
@@ -73,7 +74,7 @@ public void generationAwareRetryConflict() {
7374
integrationTestSupport.getCrOperations().inNamespace(TEST_NAMESPACE).createOrReplace(resource2);
7475

7576
awaitResourcesCreatedOrUpdated();
76-
awaitStatusUpdated(10);
77+
awaitStatusUpdated(20);
7778
});
7879
}
7980

@@ -95,7 +96,8 @@ void awaitStatusUpdated() {
9596
void awaitStatusUpdated(int timeout) {
9697
await("cr status updated").atMost(timeout, TimeUnit.SECONDS)
9798
.untilAsserted(() -> {
98-
TestCustomResource cr = (TestCustomResource) integrationTestSupport.getCrOperations().inNamespace(TEST_NAMESPACE).withName("test-custom-resource").get();
99+
TestCustomResource cr = (TestCustomResource) integrationTestSupport.getCrOperations()
100+
.inNamespace(TEST_NAMESPACE).withName(TEST_CUSTOM_RESOURCE_NAME).get();
99101
assertThat(cr).isNotNull();
100102
assertThat(cr.getStatus()).isNotNull();
101103
assertThat(cr.getStatus().getConfigMapStatus()).isEqualTo("ConfigMap Ready");
@@ -105,7 +107,7 @@ void awaitStatusUpdated(int timeout) {
105107
private TestCustomResource testCustomResource() {
106108
TestCustomResource resource = new TestCustomResource();
107109
resource.setMetadata(new ObjectMetaBuilder()
108-
.withName("test-custom-resource")
110+
.withName(TEST_CUSTOM_RESOURCE_NAME)
109111
.withNamespace(TEST_NAMESPACE)
110112
.build());
111113
resource.getMetadata().setAnnotations(new HashMap<>());

0 commit comments

Comments
 (0)