Skip to content

Commit 3ce7e78

Browse files
csviricsviri
committed
unit tests
1 parent 295e907 commit 3ce7e78

File tree

9 files changed

+108
-34
lines changed

9 files changed

+108
-34
lines changed

‎operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/ReconciliationDispatcher.java

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -215,7 +215,6 @@ private boolean isErrorStatusHandlerPresent() {
215215
private Optional<R> updateStatusGenerationAware(R resource, R originalResource, boolean patch,
216216
boolean onlyIfChanged) {
217217
updateStatusObservedGenerationIfRequired(resource);
218-
// todo unit test order
219218
if (onlyIfChanged && statusEqual(resource, originalResource)) {
220219
return Optional.empty();
221220
}
@@ -312,17 +311,17 @@ private R updateCustomResourceWithFinalizer(R resource) {
312311
log.debug(
313312
"Adding finalizer for resource: {} version: {}", getUID(resource), getVersion(resource));
314313
resource.addFinalizer(configuration().getFinalizerName());
315-
return customResourceFacade.replaceResourceWithLock(resource);
314+
return customResourceFacade.updateResource(resource);
316315
}
317316

318317
private Optional<R> updateCustomResource(R resource, R originalResource, boolean onlyOnChange) {
319-
if (onlyOnChange && !specAndMetaEqual(resource, originalResource)) {
318+
if (onlyOnChange && specAndMetaEqual(resource, originalResource)) {
320319
return Optional.empty();
321320
}
322321

323322
log.debug("Updating resource: {} with version: {}", getUID(resource), getVersion(resource));
324323
log.trace("Resource before update: {}", resource);
325-
return Optional.of(customResourceFacade.replaceResourceWithLock(resource));
324+
return Optional.of(customResourceFacade.updateResource(resource));
326325
}
327326

328327
ControllerConfiguration<R> configuration() {
@@ -340,7 +339,7 @@ public R removeFinalizer(R resource, String finalizer) {
340339
if (!removed) {
341340
return resource;
342341
}
343-
return customResourceFacade.replaceResourceWithLock(resource);
342+
return customResourceFacade.updateResource(resource);
344343
} catch (KubernetesClientException e) {
345344
log.trace("Exception during finalizer removal for resource: {}", resource);
346345
retryIndex++;
@@ -393,7 +392,7 @@ public R getResource(String namespace, String name) {
393392
return resourceOperation.inNamespace(namespace).withName(name).get();
394393
}
395394

396-
public R replaceResourceWithLock(R resource) {
395+
public R updateResource(R resource) {
397396
log.debug(
398397
"Trying to replace resource {}, version: {}",
399398
getName(resource),

‎operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/ReconciliationDispatcherTest.java

Lines changed: 61 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -21,10 +21,7 @@
2121
import io.javaoperatorsdk.operator.MockKubernetesClient;
2222
import io.javaoperatorsdk.operator.OperatorException;
2323
import io.javaoperatorsdk.operator.TestUtils;
24-
import io.javaoperatorsdk.operator.api.config.Cloner;
25-
import io.javaoperatorsdk.operator.api.config.ConfigurationServiceProvider;
26-
import io.javaoperatorsdk.operator.api.config.ControllerConfiguration;
27-
import io.javaoperatorsdk.operator.api.config.MockControllerConfiguration;
24+
import io.javaoperatorsdk.operator.api.config.*;
2825
import io.javaoperatorsdk.operator.api.reconciler.Cleaner;
2926
import io.javaoperatorsdk.operator.api.reconciler.Context;
3027
import io.javaoperatorsdk.operator.api.reconciler.DeleteControl;
@@ -133,7 +130,7 @@ void addFinalizerOnNewResource() {
133130
verify(reconciler, never())
134131
.reconcile(ArgumentMatchers.eq(testCustomResource), any());
135132
verify(customResourceFacade, times(1))
136-
.replaceResourceWithLock(
133+
.updateResource(
137134
argThat(testCustomResource -> testCustomResource.hasFinalizer(DEFAULT_FINALIZER)));
138135
assertThat(testCustomResource.hasFinalizer(DEFAULT_FINALIZER)).isTrue();
139136
}
@@ -155,20 +152,20 @@ void updatesOnlyStatusSubResourceIfFinalizerSet() {
155152
reconciliationDispatcher.handleExecution(executionScopeWithCREvent(testCustomResource));
156153

157154
verify(customResourceFacade, times(1)).patchStatus(eq(testCustomResource), any());
158-
verify(customResourceFacade, never()).replaceResourceWithLock(any());
155+
verify(customResourceFacade, never()).updateResource(any());
159156
}
160157

161158
@Test
162159
void updatesBothResourceAndStatusIfFinalizerSet() {
163160
testCustomResource.addFinalizer(DEFAULT_FINALIZER);
164161

165162
reconciler.reconcile = (r, c) -> UpdateControl.updateResourceAndStatus(testCustomResource);
166-
when(customResourceFacade.replaceResourceWithLock(testCustomResource))
163+
when(customResourceFacade.updateResource(testCustomResource))
167164
.thenReturn(testCustomResource);
168165

169166
reconciliationDispatcher.handleExecution(executionScopeWithCREvent(testCustomResource));
170167

171-
verify(customResourceFacade, times(1)).replaceResourceWithLock(testCustomResource);
168+
verify(customResourceFacade, times(1)).updateResource(testCustomResource);
172169
verify(customResourceFacade, times(1)).updateStatus(testCustomResource);
173170
}
174171

@@ -182,7 +179,7 @@ void patchesStatus() {
182179

183180
verify(customResourceFacade, times(1)).patchStatus(eq(testCustomResource), any());
184181
verify(customResourceFacade, never()).updateStatus(any());
185-
verify(customResourceFacade, never()).replaceResourceWithLock(any());
182+
verify(customResourceFacade, never()).updateResource(any());
186183
}
187184

188185
@Test
@@ -214,7 +211,7 @@ void removesDefaultFinalizerOnDeleteIfSet() {
214211
reconciliationDispatcher.handleExecution(executionScopeWithCREvent(testCustomResource));
215212

216213
assertThat(postExecControl.isFinalizerRemoved()).isTrue();
217-
verify(customResourceFacade, times(1)).replaceResourceWithLock(testCustomResource);
214+
verify(customResourceFacade, times(1)).updateResource(testCustomResource);
218215
}
219216

220217
@Test
@@ -223,7 +220,7 @@ void retriesFinalizerRemovalWithFreshResource() {
223220
markForDeletion(testCustomResource);
224221
var resourceWithFinalizer = TestUtils.testCustomResource();
225222
resourceWithFinalizer.addFinalizer(DEFAULT_FINALIZER);
226-
when(customResourceFacade.replaceResourceWithLock(testCustomResource))
223+
when(customResourceFacade.updateResource(testCustomResource))
227224
.thenThrow(new KubernetesClientException(null, 409, null))
228225
.thenReturn(testCustomResource);
229226
when(customResourceFacade.getResource(any(), any())).thenReturn(resourceWithFinalizer);
@@ -232,15 +229,15 @@ void retriesFinalizerRemovalWithFreshResource() {
232229
reconciliationDispatcher.handleExecution(executionScopeWithCREvent(testCustomResource));
233230

234231
assertThat(postExecControl.isFinalizerRemoved()).isTrue();
235-
verify(customResourceFacade, times(2)).replaceResourceWithLock(any());
232+
verify(customResourceFacade, times(2)).updateResource(any());
236233
verify(customResourceFacade, times(1)).getResource(any(), any());
237234
}
238235

239236
@Test
240237
void throwsExceptionIfFinalizerRemovalRetryExceeded() {
241238
testCustomResource.addFinalizer(DEFAULT_FINALIZER);
242239
markForDeletion(testCustomResource);
243-
when(customResourceFacade.replaceResourceWithLock(any()))
240+
when(customResourceFacade.updateResource(any()))
244241
.thenThrow(new KubernetesClientException(null, 409, null));
245242
when(customResourceFacade.getResource(any(), any()))
246243
.thenAnswer((Answer<TestCustomResource>) invocationOnMock -> createResourceWithFinalizer());
@@ -252,7 +249,7 @@ void throwsExceptionIfFinalizerRemovalRetryExceeded() {
252249
assertThat(postExecControl.getRuntimeException()).isPresent();
253250
assertThat(postExecControl.getRuntimeException().get())
254251
.isInstanceOf(OperatorException.class);
255-
verify(customResourceFacade, times(MAX_FINALIZER_REMOVAL_RETRY)).replaceResourceWithLock(any());
252+
verify(customResourceFacade, times(MAX_FINALIZER_REMOVAL_RETRY)).updateResource(any());
256253
verify(customResourceFacade, times(MAX_FINALIZER_REMOVAL_RETRY - 1)).getResource(any(),
257254
any());
258255
}
@@ -261,15 +258,15 @@ void throwsExceptionIfFinalizerRemovalRetryExceeded() {
261258
void throwsExceptionIfFinalizerRemovalClientExceptionIsNotConflict() {
262259
testCustomResource.addFinalizer(DEFAULT_FINALIZER);
263260
markForDeletion(testCustomResource);
264-
when(customResourceFacade.replaceResourceWithLock(any()))
261+
when(customResourceFacade.updateResource(any()))
265262
.thenThrow(new KubernetesClientException(null, 400, null));
266263

267264
var res =
268265
reconciliationDispatcher.handleExecution(executionScopeWithCREvent(testCustomResource));
269266

270267
assertThat(res.getRuntimeException()).isPresent();
271268
assertThat(res.getRuntimeException().get()).isInstanceOf(KubernetesClientException.class);
272-
verify(customResourceFacade, times(1)).replaceResourceWithLock(any());
269+
verify(customResourceFacade, times(1)).updateResource(any());
273270
verify(customResourceFacade, never()).getResource(any(), any());
274271
}
275272

@@ -315,7 +312,7 @@ void doesNotRemovesTheSetFinalizerIfTheDeleteNotMethodInstructsIt() {
315312
reconciliationDispatcher.handleExecution(executionScopeWithCREvent(testCustomResource));
316313

317314
assertEquals(1, testCustomResource.getMetadata().getFinalizers().size());
318-
verify(customResourceFacade, never()).replaceResourceWithLock(any());
315+
verify(customResourceFacade, never()).updateResource(any());
319316
}
320317

321318
@Test
@@ -325,21 +322,21 @@ void doesNotUpdateTheResourceIfNoUpdateUpdateControlIfFinalizerSet() {
325322
reconciler.reconcile = (r, c) -> UpdateControl.noUpdate();
326323

327324
reconciliationDispatcher.handleExecution(executionScopeWithCREvent(testCustomResource));
328-
verify(customResourceFacade, never()).replaceResourceWithLock(any());
325+
verify(customResourceFacade, never()).updateResource(any());
329326
verify(customResourceFacade, never()).updateStatus(testCustomResource);
330327
}
331328

332329
@Test
333330
void addsFinalizerIfNotMarkedForDeletionAndEmptyCustomResourceReturned() {
334331
removeFinalizers(testCustomResource);
335332
reconciler.reconcile = (r, c) -> UpdateControl.noUpdate();
336-
when(customResourceFacade.replaceResourceWithLock(any())).thenReturn(testCustomResource);
333+
when(customResourceFacade.updateResource(any())).thenReturn(testCustomResource);
337334

338335
var postExecControl =
339336
reconciliationDispatcher.handleExecution(executionScopeWithCREvent(testCustomResource));
340337

341338
assertEquals(1, testCustomResource.getMetadata().getFinalizers().size());
342-
verify(customResourceFacade, times(1)).replaceResourceWithLock(any());
339+
verify(customResourceFacade, times(1)).updateResource(any());
343340
assertThat(postExecControl.updateIsStatusPatch()).isFalse();
344341
assertThat(postExecControl.getUpdatedCustomResource()).isPresent();
345342
}
@@ -351,7 +348,7 @@ void doesNotCallDeleteIfMarkedForDeletionButNotOurFinalizer() {
351348

352349
reconciliationDispatcher.handleExecution(executionScopeWithCREvent(testCustomResource));
353350

354-
verify(customResourceFacade, never()).replaceResourceWithLock(any());
351+
verify(customResourceFacade, never()).updateResource(any());
355352
verify(reconciler, never()).cleanup(eq(testCustomResource), any());
356353
}
357354

@@ -477,7 +474,7 @@ void updateObservedGenerationOnCustomResourceUpdate() throws Exception {
477474
when(config.isGenerationAware()).thenReturn(true);
478475
when(reconciler.reconcile(any(), any()))
479476
.thenReturn(UpdateControl.updateResource(observedGenResource));
480-
when(facade.replaceResourceWithLock(any())).thenReturn(observedGenResource);
477+
when(facade.updateResource(any())).thenReturn(observedGenResource);
481478
when(facade.updateStatus(observedGenResource)).thenReturn(observedGenResource);
482479
var dispatcher = init(observedGenResource, reconciler, config, facade, true);
483480

@@ -629,6 +626,48 @@ void canSkipSchedulingMaxDelayIf() {
629626
assertThat(control.getReScheduleDelay()).isNotPresent();
630627
}
631628

629+
@Test
630+
void noRequestOnConditionalUpdates() {
631+
checkConditionalUpdate((r, c) -> UpdateControl.patchStatusIfChanged(testCustomResource));
632+
checkConditionalUpdate(
633+
(r, c) -> UpdateControl.updateResourceAndPatchStatusIfChanged(testCustomResource));
634+
checkConditionalUpdate((r, c) -> UpdateControl.updateResourceIfChanged(testCustomResource));
635+
checkConditionalUpdate(
636+
(r, c) -> UpdateControl.updateResourceAndStatusIfChanged(testCustomResource));
637+
}
638+
639+
640+
@Test
641+
void conditionalUpdateSpecChangedNoStatus() {
642+
testCustomResource.addFinalizer(DEFAULT_FINALIZER);
643+
when(customResourceFacade.updateResource(any())).thenAnswer(a -> a.getArguments()[0]);
644+
645+
reconciler.reconcile = (r, c) -> {
646+
// in this test the cloning is turned off for easier verification, but required here
647+
r = ConfigurationService.DEFAULT_CLONER.clone(r);
648+
r.getSpec().setValue("otherValue");
649+
return UpdateControl.updateResourceAndPatchStatusIfChanged(r);
650+
};
651+
652+
reconciliationDispatcher.handleExecution(executionScopeWithCREvent(testCustomResource));
653+
654+
verify(customResourceFacade, never()).patchStatus(any(), any());
655+
verify(customResourceFacade, times(1)).updateResource(any());
656+
}
657+
658+
private void checkConditionalUpdate(
659+
BiFunction<TestCustomResource, Context, UpdateControl<TestCustomResource>> r) {
660+
testCustomResource.addFinalizer(DEFAULT_FINALIZER);
661+
662+
reconciler.reconcile = r;
663+
664+
reconciliationDispatcher.handleExecution(executionScopeWithCREvent(testCustomResource));
665+
666+
verify(customResourceFacade, never()).patchStatus(any(), any());
667+
verify(customResourceFacade, never()).updateResource(any());
668+
verify(customResourceFacade, never()).updateStatus(any());
669+
}
670+
632671
private ObservedGenCustomResource createObservedGenCustomResource() {
633672
ObservedGenCustomResource observedGenCustomResource = new ObservedGenCustomResource();
634673
observedGenCustomResource.setMetadata(new ObjectMeta());

‎sample-operators/mysql-schema/src/main/java/io/javaoperatorsdk/operator/sample/MySQLSchemaReconciler.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ public UpdateControl<MySQLSchema> reconcile(MySQLSchema schema, Context<MySQLSch
4343
updateStatusPojo(schema, s, secret.getMetadata().getName(),
4444
decode(secret.getData().get(MYSQL_SECRET_USERNAME)));
4545
log.info("Schema {} created - updating CR status", s.getName());
46-
return UpdateControl.patchStatus(schema);
46+
return UpdateControl.patchStatusIfChanged(schema);
4747
}).orElse(UpdateControl.noUpdate());
4848
}
4949

@@ -57,7 +57,7 @@ public ErrorStatusUpdateControl<MySQLSchema> updateErrorStatus(MySQLSchema schem
5757
status.setSecretName(null);
5858
status.setStatus("ERROR: " + e.getMessage());
5959
schema.setStatus(status);
60-
return ErrorStatusUpdateControl.updateStatus(schema);
60+
return ErrorStatusUpdateControl.patchStatus(schema);
6161
}
6262

6363

‎sample-operators/tomcat-operator/src/main/java/io/javaoperatorsdk/operator/sample/TomcatReconciler.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ public UpdateControl<Tomcat> reconcile(Tomcat tomcat, Context<Tomcat> context) {
3535
tomcat.getMetadata().getName(),
3636
tomcat.getMetadata().getNamespace(),
3737
tomcat.getStatus().getReadyReplicas());
38-
return UpdateControl.patchStatus(updatedTomcat);
38+
return UpdateControl.patchStatusIfChanged(updatedTomcat);
3939
}).orElse(UpdateControl.noUpdate());
4040
}
4141

‎sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/WebPageDependentsWorkflowReconciler.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ public UpdateControl<WebPage> reconcile(WebPage webPage, Context<WebPage> contex
7070
webPage.setStatus(
7171
createStatus(
7272
configMapDR.getSecondaryResource(webPage).orElseThrow().getMetadata().getName()));
73-
return UpdateControl.patchStatus(webPage);
73+
return UpdateControl.patchStatusIfChanged(webPage);
7474
}
7575

7676
@Override

‎sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/WebPageManagedDependentsReconciler.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,6 @@ public UpdateControl<WebPage> reconcile(WebPage webPage, Context<WebPage> contex
4343
final var name = context.getSecondaryResource(ConfigMap.class).orElseThrow()
4444
.getMetadata().getName();
4545
webPage.setStatus(createStatus(name));
46-
return UpdateControl.patchStatus(webPage);
46+
return UpdateControl.patchStatusIfChanged(webPage);
4747
}
4848
}

‎sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/WebPageReconciler.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -146,7 +146,7 @@ public UpdateControl<WebPage> reconcile(WebPage webPage, Context<WebPage> contex
146146
kubernetesClient.pods().inNamespace(ns).withLabel("app", deploymentName(webPage)).delete();
147147
}
148148
webPage.setStatus(createStatus(desiredHtmlConfigMap.getMetadata().getName()));
149-
return UpdateControl.updateStatus(webPage);
149+
return UpdateControl.patchStatusIfChanged(webPage);
150150
}
151151

152152
private boolean match(Ingress desiredIngress, Ingress existingIngress) {

‎sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/WebPageSpec.java

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
package io.javaoperatorsdk.operator.sample;
22

3+
import java.util.Objects;
4+
35
public class WebPageSpec {
46

57
private String html;
@@ -28,4 +30,19 @@ public String toString() {
2830
"html='" + html + '\'' +
2931
'}';
3032
}
33+
34+
@Override
35+
public boolean equals(Object o) {
36+
if (this == o)
37+
return true;
38+
if (o == null || getClass() != o.getClass())
39+
return false;
40+
WebPageSpec that = (WebPageSpec) o;
41+
return Objects.equals(html, that.html) && Objects.equals(exposed, that.exposed);
42+
}
43+
44+
@Override
45+
public int hashCode() {
46+
return Objects.hash(html, exposed);
47+
}
3148
}

‎sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/WebPageStatus.java

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
package io.javaoperatorsdk.operator.sample;
22

3+
import java.util.Objects;
4+
35
import io.javaoperatorsdk.operator.api.ObservedGenerationAwareStatus;
46

57
public class WebPageStatus extends ObservedGenerationAwareStatus {
@@ -43,4 +45,21 @@ public String toString() {
4345
", errorMessage='" + errorMessage + '\'' +
4446
'}';
4547
}
48+
49+
@Override
50+
public boolean equals(Object o) {
51+
if (this == o)
52+
return true;
53+
if (o == null || getClass() != o.getClass())
54+
return false;
55+
WebPageStatus that = (WebPageStatus) o;
56+
return Objects.equals(htmlConfigMap, that.htmlConfigMap)
57+
&& Objects.equals(areWeGood, that.areWeGood)
58+
&& Objects.equals(errorMessage, that.errorMessage);
59+
}
60+
61+
@Override
62+
public int hashCode() {
63+
return Objects.hash(htmlConfigMap, areWeGood, errorMessage);
64+
}
4665
}

0 commit comments

Comments
 (0)