Skip to content

Commit 572814f

Browse files
committed
adding informer event source test
and refactoring/creating a couple of methods for readability
1 parent 3d45031 commit 572814f

File tree

3 files changed

+69
-30
lines changed

3 files changed

+69
-30
lines changed

operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/KubernetesDependentResource.java

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -116,8 +116,7 @@ public R create(R target, P primary, Context<P> context) {
116116
target.getMetadata().setResourceVersion("1");
117117
}
118118
}
119-
String id = ((InformerEventSource<R, P>) eventSource().orElseThrow()).getId();
120-
target.getMetadata().getAnnotations().put(PREVIOUS_ANNOTATION_KEY, id);
119+
addPreviousAnnotation(null, target);
121120
final var resource = prepare(target, primary, "Creating");
122121
return useSSA(context)
123122
? resource
@@ -133,9 +132,7 @@ public R update(R actual, R target, P primary, Context<P> context) {
133132
actual.getMetadata().getResourceVersion());
134133
}
135134
R updatedResource;
136-
String id = ((InformerEventSource<R, P>) eventSource().orElseThrow()).getId();
137-
target.getMetadata().getAnnotations().put(PREVIOUS_ANNOTATION_KEY,
138-
id + "," + actual.getMetadata().getResourceVersion());
135+
addPreviousAnnotation(actual.getMetadata().getResourceVersion(), target);
139136
if (useSSA(context)) {
140137
target.getMetadata().setResourceVersion(actual.getMetadata().getResourceVersion());
141138
updatedResource = prepare(target, primary, "Updating")
@@ -150,6 +147,12 @@ public R update(R actual, R target, P primary, Context<P> context) {
150147
return updatedResource;
151148
}
152149

150+
void addPreviousAnnotation(String resourceVersion, HasMetadata target) {
151+
String id = ((InformerEventSource<HasMetadata, HasMetadata>) eventSource().orElseThrow()).getId();
152+
target.getMetadata().getAnnotations().put(PREVIOUS_ANNOTATION_KEY,
153+
id + resourceVersion != null ? ("," + resourceVersion) : "");
154+
}
155+
153156
@Override
154157
public Result<R> match(R actualResource, P primary, Context<P> context) {
155158
final var desired = desired(primary, context);

operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerEventSource.java

Lines changed: 28 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -159,22 +159,7 @@ private synchronized void onAddOrUpdate(Operation operation, R newObject, R oldO
159159
Runnable superOnOp) {
160160
var resourceID = ResourceID.fromResource(newObject);
161161

162-
String previous = newObject.getMetadata().getAnnotations()
163-
.get(KubernetesDependentResource.PREVIOUS_ANNOTATION_KEY);
164-
boolean known = false;
165-
if (previous != null) {
166-
String[] parts = previous.split(",");
167-
if (id.equals(parts[0])) {
168-
if (oldObject == null && parts.length == 1) {
169-
known = true;
170-
} else if (oldObject != null && parts.length == 2
171-
&& oldObject.getMetadata().getResourceVersion().equals(parts[1])) {
172-
known = true;
173-
}
174-
}
175-
}
176-
if (temporaryCacheHasResourceWithSameVersionAs(newObject)
177-
|| (known && temporaryResourceCache.getResourceFromCache(resourceID).isEmpty())) {
162+
if (canSkipEvent(newObject, oldObject, resourceID)) {
178163
log.debug(
179164
"Skipping event propagation for {}, since was a result of a reconcile action. Resource ID: {}",
180165
operation,
@@ -194,16 +179,34 @@ private synchronized void onAddOrUpdate(Operation operation, R newObject, R oldO
194179
}
195180
}
196181

197-
private boolean temporaryCacheHasResourceWithSameVersionAs(R resource) {
198-
var resourceID = ResourceID.fromResource(resource);
182+
private boolean canSkipEvent(R newObject, R oldObject, ResourceID resourceID) {
199183
var res = temporaryResourceCache.getResourceFromCache(resourceID);
200-
return res.map(r -> {
201-
boolean resVersionsEqual = r.getMetadata().getResourceVersion()
202-
.equals(resource.getMetadata().getResourceVersion());
203-
log.debug("Resource found in temporal cache for id: {} resource versions equal: {}",
204-
resourceID, resVersionsEqual);
205-
return resVersionsEqual;
206-
}).orElse(false);
184+
if (res.isEmpty()) {
185+
return isEventKnownFromAnnotation(newObject, oldObject);
186+
}
187+
boolean resVersionsEqual = newObject.getMetadata().getResourceVersion()
188+
.equals(res.get().getMetadata().getResourceVersion());
189+
log.debug("Resource found in temporal cache for id: {} resource versions equal: {}",
190+
resourceID, resVersionsEqual);
191+
return resVersionsEqual;
192+
}
193+
194+
private boolean isEventKnownFromAnnotation(R newObject, R oldObject) {
195+
String previous = newObject.getMetadata().getAnnotations()
196+
.get(KubernetesDependentResource.PREVIOUS_ANNOTATION_KEY);
197+
boolean known = false;
198+
if (previous != null) {
199+
String[] parts = previous.split(",");
200+
if (id.equals(parts[0])) {
201+
if (oldObject == null && parts.length == 1) {
202+
known = true;
203+
} else if (oldObject != null && parts.length == 2
204+
&& oldObject.getMetadata().getResourceVersion().equals(parts[1])) {
205+
known = true;
206+
}
207+
}
208+
}
209+
return known;
207210
}
208211

209212
private void propagateEvent(R object) {

operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerEventSourceTest.java

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,13 +8,15 @@
88

99
import io.fabric8.kubernetes.api.model.ObjectMeta;
1010
import io.fabric8.kubernetes.api.model.apps.Deployment;
11+
import io.fabric8.kubernetes.api.model.apps.DeploymentBuilder;
1112
import io.fabric8.kubernetes.client.KubernetesClient;
1213
import io.javaoperatorsdk.operator.MockKubernetesClient;
1314
import io.javaoperatorsdk.operator.OperatorException;
1415
import io.javaoperatorsdk.operator.api.config.BaseConfigurationService;
1516
import io.javaoperatorsdk.operator.api.config.ConfigurationService;
1617
import io.javaoperatorsdk.operator.api.config.InformerStoppedHandler;
1718
import io.javaoperatorsdk.operator.api.config.informer.InformerConfiguration;
19+
import io.javaoperatorsdk.operator.processing.dependent.kubernetes.KubernetesDependentResource;
1820
import io.javaoperatorsdk.operator.processing.event.EventHandler;
1921
import io.javaoperatorsdk.operator.processing.event.ResourceID;
2022
import io.javaoperatorsdk.operator.processing.event.source.SecondaryToPrimaryMapper;
@@ -77,6 +79,37 @@ void skipsEventPropagationIfResourceWithSameVersionInResourceCache() {
7779
verify(eventHandlerMock, never()).handleEvent(any());
7880
}
7981

82+
@Test
83+
void skipsAddEventPropagationViaAnnotation() {
84+
informerEventSource.onAdd(new DeploymentBuilder(testDeployment()).editMetadata()
85+
.addToAnnotations(KubernetesDependentResource.PREVIOUS_ANNOTATION_KEY, informerEventSource.getId()).endMetadata().build());
86+
87+
verify(eventHandlerMock, never()).handleEvent(any());
88+
}
89+
90+
@Test
91+
void skipsUpdateEventPropagationViaAnnotation() {
92+
informerEventSource.onUpdate(testDeployment(), new DeploymentBuilder(testDeployment()).editMetadata()
93+
.addToAnnotations(KubernetesDependentResource.PREVIOUS_ANNOTATION_KEY, informerEventSource.getId() + ",1").endMetadata().build());
94+
95+
verify(eventHandlerMock, never()).handleEvent(any());
96+
}
97+
98+
@Test
99+
void processEventPropagationWithoutAnnotation() {
100+
informerEventSource.onUpdate(testDeployment(), testDeployment());
101+
102+
verify(eventHandlerMock, times(1)).handleEvent(any());
103+
}
104+
105+
@Test
106+
void processEventPropagationWithIncorrectAnnotation() {
107+
informerEventSource.onAdd(new DeploymentBuilder(testDeployment()).editMetadata()
108+
.addToAnnotations(KubernetesDependentResource.PREVIOUS_ANNOTATION_KEY, "invalid").endMetadata().build());
109+
110+
verify(eventHandlerMock, times(1)).handleEvent(any());
111+
}
112+
80113
@Test
81114
void propagateEventAndRemoveResourceFromTempCacheIfResourceVersionMismatch() {
82115
Deployment cachedDeployment = testDeployment();

0 commit comments

Comments
 (0)