Skip to content

Commit 5a49799

Browse files
committed
test fixes and improvements
Signed-off-by: Attila Mészáros <a_meszaros@apple.com>
1 parent a5a27fa commit 5a49799

4 files changed

Lines changed: 51 additions & 28 deletions

File tree

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

Lines changed: 17 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -543,7 +543,7 @@ public <R extends HasMetadata> R jsonPatch(
543543
return resourcePatch(
544544
desired,
545545
actualResource,
546-
r -> context.getClient().resource(actualResource).edit(unaryOperator),
546+
r -> context.getClient().resource(actualResource).edit(rr -> desired),
547547
options);
548548
}
549549

@@ -607,7 +607,7 @@ public <R extends HasMetadata> R jsonPatchStatus(
607607
return resourcePatch(
608608
desired,
609609
actualResource,
610-
r -> context.getClient().resource(r).editStatus(unaryOperator),
610+
r -> context.getClient().resource(actualResource).editStatus(rr -> desired),
611611
options);
612612
}
613613

@@ -632,7 +632,7 @@ public <R extends HasMetadata> R jsonPatchStatus(
632632
return resourcePatch(
633633
desired,
634634
actualResource,
635-
r -> context.getClient().resource(r).editStatus(unaryOperator),
635+
r -> context.getClient().resource(actualResource).editStatus(rr -> desired),
636636
informerEventSource,
637637
options);
638638
}
@@ -667,7 +667,7 @@ public P jsonPatchPrimary(P actualResource, UnaryOperator<P> unaryOperator, Opti
667667
return resourcePatch(
668668
desired,
669669
actualResource,
670-
r -> context.getClient().resource(actualResource).edit(unaryOperator),
670+
r -> context.getClient().resource(actualResource).edit(rr -> desired),
671671
context.eventSourceRetriever().getControllerEventSource(),
672672
options);
673673
}
@@ -971,25 +971,24 @@ <R extends HasMetadata> R resourcePatch(
971971
throw new IllegalArgumentException("Mode : " + options.mode + " requires matcher");
972972
}
973973
// this is to cover special case for jsonPatch were we should use actual resource as base
974-
var targetBaseResource = desiredResource != null ? desiredResource : actualResource;
975974
if (matches) {
976975
if (log.isDebugEnabled()) {
977976
log.debug(
978977
"Resource match resource id: {}, type: {}, version: {}",
979-
ResourceID.fromResource(targetBaseResource),
980-
targetBaseResource.getClass().getSimpleName(),
981-
targetBaseResource.getMetadata().getResourceVersion());
978+
ResourceID.fromResource(desiredResource),
979+
desiredResource.getClass().getSimpleName(),
980+
desiredResource.getMetadata().getResourceVersion());
982981
}
983982
return actualResource;
984983
}
985984

986-
boolean optimisticLocking = targetBaseResource.getMetadata().getResourceVersion() != null;
985+
boolean optimisticLocking = desiredResource.getMetadata().getResourceVersion() != null;
987986

988987
if (options.getMode() == Mode.CACHE_ONLY
989988
|| (options.getMode() == Mode.FILTER_IF_OPTIMISTIC_LOCKING && !optimisticLocking)) {
990-
return ies.updateAndCacheResource(targetBaseResource, updateOperation);
989+
return ies.updateAndCacheResource(desiredResource, updateOperation);
991990
} else {
992-
return ies.eventFilteringUpdateAndCacheResource(targetBaseResource, updateOperation);
991+
return ies.eventFilteringUpdateAndCacheResource(desiredResource, updateOperation);
993992
}
994993
}
995994

@@ -1381,15 +1380,12 @@ enum Mode {
13811380

13821381
private <T extends HasMetadata> T desiredForJsonPatch(
13831382
T actualResource, UnaryOperator<T> unaryOperator, Options options) {
1384-
if (options.getMatcher().isPresent()) {
1385-
var cloned =
1386-
context
1387-
.getControllerConfiguration()
1388-
.getConfigurationService()
1389-
.getResourceCloner()
1390-
.clone(actualResource);
1391-
return unaryOperator.apply(cloned);
1392-
}
1393-
return null;
1383+
var cloned =
1384+
context
1385+
.getControllerConfiguration()
1386+
.getConfigurationService()
1387+
.getResourceCloner()
1388+
.clone(actualResource);
1389+
return unaryOperator.apply(cloned);
13941390
}
13951391
}

operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/readcacheafterwrite/specchangeduringstatuspatch/SpecChangeDuringStatusPatchIT.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ class SpecChangeDuringStatusPatchIT {
4949
LocallyRunOperatorExtension extension =
5050
LocallyRunOperatorExtension.builder().withReconciler(reconciler).build();
5151

52-
@RepeatedTest(10)
52+
@RepeatedTest(3)
5353
void specChangeDuringStatusPatchIsReconciled() throws InterruptedException {
5454
var res = extension.create(testResource());
5555
var statusRes = testResource();

operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/subresource/SubResourceTestCustomReconciler.java

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -44,9 +44,17 @@ public UpdateControl<SubResourceTestCustomResource> reconcile(
4444
log.info("Value: " + resource.getSpec().getValue());
4545

4646
ensureStatusExists(resource);
47-
resource.getStatus().setState(SubResourceTestCustomResourceStatus.State.SUCCESS);
47+
4848
waitXms(RECONCILER_MIN_EXEC_TIME);
49-
return UpdateControl.patchStatus(resource);
49+
context
50+
.resourceOperations()
51+
.jsonPatchPrimaryStatus(
52+
resource,
53+
r -> {
54+
r.getStatus().setState(SubResourceTestCustomResourceStatus.State.SUCCESS);
55+
return r;
56+
});
57+
return UpdateControl.noUpdate();
5058
}
5159

5260
private void ensureStatusExists(SubResourceTestCustomResource resource) {

operator-framework/src/test/java/io/javaoperatorsdk/operator/workflow/workflowmultipleactivation/WorkflowMultipleActivationIT.java

Lines changed: 23 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
package io.javaoperatorsdk.operator.workflow.workflowmultipleactivation;
1717

1818
import java.time.Duration;
19+
import java.util.concurrent.atomic.AtomicInteger;
1920

2021
import org.junit.jupiter.api.Test;
2122
import org.junit.jupiter.api.extension.RegisterExtension;
@@ -115,10 +116,7 @@ void deactivatingAndReactivatingDependent() {
115116
assertThat(cm.getData()).containsEntry(DATA_KEY, CHANGED_VALUE);
116117
});
117118

118-
var numOfReconciliation =
119-
extension
120-
.getReconcilerOfType(WorkflowMultipleActivationReconciler.class)
121-
.getNumberOfReconciliationExecution();
119+
var numOfReconciliation = awaitStableReconciliationCount();
122120
var actualCM = extension.get(ConfigMap.class, TEST_RESOURCE1);
123121
actualCM.getData().put("data2", "additionaldata");
124122
extension.replace(actualCM);
@@ -146,6 +144,27 @@ void deactivatingAndReactivatingDependent() {
146144
});
147145
}
148146

147+
/**
148+
* Snapshots the reconciliation count only once it has stopped changing. The framework adds the
149+
* finalizer with a cache-only (non event-filtered) write, so each (re)creation of the resource
150+
* emits a finalizer-add event that drives a follow-up reconciliation. A few of these may still be
151+
* trailing from the preceding create/delete/recreate and spec changes; capturing the count before
152+
* they settle would race with them and inflate the later assertion.
153+
*/
154+
private int awaitStableReconciliationCount() {
155+
var reconciler = extension.getReconcilerOfType(WorkflowMultipleActivationReconciler.class);
156+
var lastSeen = new AtomicInteger(-1);
157+
await()
158+
.pollInterval(Duration.ofMillis(POLL_DELAY))
159+
.atMost(Duration.ofSeconds(10))
160+
.until(
161+
() -> {
162+
int current = reconciler.getNumberOfReconciliationExecution();
163+
return current == lastSeen.getAndSet(current);
164+
});
165+
return lastSeen.get();
166+
}
167+
149168
WorkflowMultipleActivationCustomResource testResource(String name) {
150169
var res = new WorkflowMultipleActivationCustomResource();
151170
res.setMetadata(new ObjectMetaBuilder().withName(name).build());

0 commit comments

Comments
 (0)