Skip to content

Commit e4790d6

Browse files
csviricsviri
authored
feat: patching with update control (#1147)
1 parent 5ccacca commit e4790d6

File tree

5 files changed

+140
-39
lines changed

5 files changed

+140
-39
lines changed

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

Lines changed: 25 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7,15 +7,17 @@ public class UpdateControl<T extends HasMetadata> extends BaseControl<UpdateCont
77
private final T resource;
88
private final boolean updateStatus;
99
private final boolean updateResource;
10+
private final boolean patch;
1011

1112
private UpdateControl(
12-
T resource, boolean updateStatus, boolean updateResource) {
13+
T resource, boolean updateStatus, boolean updateResource, boolean patch) {
1314
if ((updateResource || updateStatus) && resource == null) {
1415
throw new IllegalArgumentException("CustomResource cannot be null in case of update");
1516
}
1617
this.resource = resource;
1718
this.updateStatus = updateStatus;
1819
this.updateResource = updateResource;
20+
this.patch = patch;
1921
}
2022

2123
/**
@@ -28,11 +30,19 @@ private UpdateControl(
2830
* @return initialized update control
2931
*/
3032
public static <T extends HasMetadata> UpdateControl<T> updateResource(T customResource) {
31-
return new UpdateControl<>(customResource, false, true);
33+
return new UpdateControl<>(customResource, false, true, false);
3234
}
3335

3436
public static <T extends HasMetadata> UpdateControl<T> updateStatus(T customResource) {
35-
return new UpdateControl<>(customResource, true, false);
37+
return new UpdateControl<>(customResource, true, false, false);
38+
}
39+
40+
public static <T extends HasMetadata> UpdateControl<T> patchResource(T customResource) {
41+
return new UpdateControl<>(customResource, false, true, true);
42+
}
43+
44+
public static <T extends HasMetadata> UpdateControl<T> patchStatus(T customResource) {
45+
return new UpdateControl<>(customResource, true, false, true);
3646
}
3747

3848
/**
@@ -45,11 +55,17 @@ public static <T extends HasMetadata> UpdateControl<T> updateStatus(T customReso
4555
*/
4656
public static <T extends HasMetadata> UpdateControl<T> updateResourceAndStatus(
4757
T customResource) {
48-
return new UpdateControl<>(customResource, true, true);
58+
return new UpdateControl<>(customResource, true, true, false);
59+
}
60+
61+
public static <T extends HasMetadata> UpdateControl<T> patchResourceAndStatus(
62+
T customResource) {
63+
return new UpdateControl<>(customResource, true, true, true);
4964
}
5065

66+
5167
public static <T extends HasMetadata> UpdateControl<T> noUpdate() {
52-
return new UpdateControl<>(null, false, false);
68+
return new UpdateControl<>(null, false, false, false);
5369
}
5470

5571
public T getResource() {
@@ -64,6 +80,10 @@ public boolean isUpdateResource() {
6480
return updateResource;
6581
}
6682

83+
public boolean isPatch() {
84+
return patch;
85+
}
86+
6787
public boolean isNoUpdate() {
6888
return !updateResource && !updateStatus;
6989
}

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

Lines changed: 48 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -133,22 +133,28 @@ private PostExecutionControl<R> reconcileExecution(ExecutionScope<R> executionSc
133133
UpdateControl<R> updateControl = controller.reconcile(resourceForExecution, context);
134134
R updatedCustomResource = null;
135135
if (updateControl.isUpdateResourceAndStatus()) {
136-
updatedCustomResource = updateCustomResource(updateControl.getResource());
136+
updatedCustomResource =
137+
updateCustomResource(updateControl.getResource(), updateControl.isPatch());
137138
updateControl
138139
.getResource()
139140
.getMetadata()
140141
.setResourceVersion(updatedCustomResource.getMetadata().getResourceVersion());
141-
updatedCustomResource = updateStatusGenerationAware(updateControl.getResource());
142+
updatedCustomResource =
143+
updateStatusGenerationAware(updateControl.getResource(), updateControl.isPatch());
142144
} else if (updateControl.isUpdateStatus()) {
143-
updatedCustomResource = updateStatusGenerationAware(updateControl.getResource());
145+
updatedCustomResource =
146+
updateStatusGenerationAware(updateControl.getResource(), updateControl.isPatch());
144147
} else if (updateControl.isUpdateResource()) {
145-
updatedCustomResource = updateCustomResource(updateControl.getResource());
148+
updatedCustomResource =
149+
updateCustomResource(updateControl.getResource(), updateControl.isPatch());
146150
if (shouldUpdateObservedGenerationAutomatically(updatedCustomResource)) {
147-
updatedCustomResource = updateStatusGenerationAware(originalResource);
151+
updatedCustomResource =
152+
updateStatusGenerationAware(originalResource, updateControl.isPatch());
148153
}
149154
} else if (updateControl.isNoUpdate()
150155
&& shouldUpdateObservedGenerationAutomatically(resourceForExecution)) {
151-
updatedCustomResource = updateStatusGenerationAware(originalResource);
156+
updatedCustomResource =
157+
updateStatusGenerationAware(originalResource, updateControl.isPatch());
152158
}
153159
return createPostExecutionControl(updatedCustomResource, updateControl);
154160
}
@@ -197,9 +203,13 @@ private boolean isErrorStatusHandlerPresent() {
197203
return controller.getReconciler() instanceof ErrorStatusHandler;
198204
}
199205

200-
private R updateStatusGenerationAware(R resource) {
206+
private R updateStatusGenerationAware(R resource, boolean patch) {
201207
updateStatusObservedGenerationIfRequired(resource);
202-
return customResourceFacade.updateStatus(resource);
208+
if (patch) {
209+
return customResourceFacade.patchStatus(resource);
210+
} else {
211+
return customResourceFacade.updateStatus(resource);
212+
}
203213
}
204214

205215
private boolean shouldUpdateObservedGenerationAutomatically(R resource) {
@@ -281,13 +291,17 @@ private void updateCustomResourceWithFinalizer(R resource) {
281291
log.debug(
282292
"Adding finalizer for resource: {} version: {}", getUID(resource), getVersion(resource));
283293
resource.addFinalizer(configuration().getFinalizerName());
284-
replace(resource);
294+
customResourceFacade.replaceResourceWithLock(resource);
285295
}
286296

287-
private R updateCustomResource(R resource) {
297+
private R updateCustomResource(R resource, boolean patch) {
288298
log.debug("Updating resource: {} with version: {}", getUID(resource), getVersion(resource));
289299
log.trace("Resource before update: {}", resource);
290-
return replace(resource);
300+
if (patch) {
301+
return customResourceFacade.patchResource(resource);
302+
} else {
303+
return customResourceFacade.replaceResourceWithLock(resource);
304+
}
291305
}
292306

293307
private R removeFinalizer(R resource) {
@@ -296,16 +310,9 @@ private R removeFinalizer(R resource) {
296310
getUID(resource),
297311
getVersion(resource));
298312
resource.removeFinalizer(configuration().getFinalizerName());
299-
return customResourceFacade.replaceWithLock(resource);
313+
return customResourceFacade.replaceResourceWithLock(resource);
300314
}
301315

302-
private R replace(R resource) {
303-
log.debug(
304-
"Trying to replace resource {}, version: {}",
305-
getName(resource),
306-
resource.getMetadata().getResourceVersion());
307-
return customResourceFacade.replaceWithLock(resource);
308-
}
309316

310317
ControllerConfiguration<R> configuration() {
311318
return controller.getConfiguration();
@@ -321,6 +328,25 @@ public CustomResourceFacade(
321328
this.resourceOperation = resourceOperation;
322329
}
323330

331+
public R replaceResourceWithLock(R resource) {
332+
log.debug(
333+
"Trying to replace resource {}, version: {}",
334+
getName(resource),
335+
resource.getMetadata().getResourceVersion());
336+
return resourceOperation
337+
.inNamespace(resource.getMetadata().getNamespace())
338+
.withName(getName(resource))
339+
.lockResourceVersion(resource.getMetadata().getResourceVersion())
340+
.replace(resource);
341+
}
342+
343+
public R patchResource(R resource) {
344+
return resourceOperation
345+
.inNamespace(resource.getMetadata().getNamespace())
346+
.withName(getName(resource))
347+
.patch(resource);
348+
}
349+
324350
public R updateStatus(R resource) {
325351
log.trace("Updating status for resource: {}", resource);
326352
return resourceOperation
@@ -329,12 +355,12 @@ public R updateStatus(R resource) {
329355
.replaceStatus(resource);
330356
}
331357

332-
public R replaceWithLock(R resource) {
358+
public R patchStatus(R resource) {
359+
log.trace("Updating status for resource: {}", resource);
333360
return resourceOperation
334361
.inNamespace(resource.getMetadata().getNamespace())
335362
.withName(getName(resource))
336-
.lockResourceVersion(resource.getMetadata().getResourceVersion())
337-
.replace(resource);
363+
.patchStatus(resource);
338364
}
339365
}
340366
}

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

Lines changed: 38 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,7 @@ void addFinalizerOnNewResource() throws Exception {
120120
verify(reconciler, never())
121121
.reconcile(ArgumentMatchers.eq(testCustomResource), any());
122122
verify(customResourceFacade, times(1))
123-
.replaceWithLock(
123+
.replaceResourceWithLock(
124124
argThat(testCustomResource -> testCustomResource.hasFinalizer(DEFAULT_FINALIZER)));
125125
assertThat(testCustomResource.hasFinalizer(DEFAULT_FINALIZER)).isTrue();
126126
}
@@ -142,22 +142,50 @@ void updatesOnlyStatusSubResourceIfFinalizerSet() throws Exception {
142142
reconciliationDispatcher.handleExecution(executionScopeWithCREvent(testCustomResource));
143143

144144
verify(customResourceFacade, times(1)).updateStatus(testCustomResource);
145-
verify(customResourceFacade, never()).replaceWithLock(any());
145+
verify(customResourceFacade, never()).replaceResourceWithLock(any());
146146
}
147147

148148
@Test
149149
void updatesBothResourceAndStatusIfFinalizerSet() throws Exception {
150150
testCustomResource.addFinalizer(DEFAULT_FINALIZER);
151151

152152
reconciler.reconcile = (r, c) -> UpdateControl.updateResourceAndStatus(testCustomResource);
153-
when(customResourceFacade.replaceWithLock(testCustomResource)).thenReturn(testCustomResource);
153+
when(customResourceFacade.replaceResourceWithLock(testCustomResource))
154+
.thenReturn(testCustomResource);
154155

155156
reconciliationDispatcher.handleExecution(executionScopeWithCREvent(testCustomResource));
156157

157-
verify(customResourceFacade, times(1)).replaceWithLock(testCustomResource);
158+
verify(customResourceFacade, times(1)).replaceResourceWithLock(testCustomResource);
158159
verify(customResourceFacade, times(1)).updateStatus(testCustomResource);
159160
}
160161

162+
163+
@Test
164+
void patchesResource() {
165+
testCustomResource.addFinalizer(DEFAULT_FINALIZER);
166+
167+
reconciler.reconcile = (r, c) -> UpdateControl.patchResource(testCustomResource);
168+
169+
reconciliationDispatcher.handleExecution(executionScopeWithCREvent(testCustomResource));
170+
171+
verify(customResourceFacade, times(1)).patchResource(testCustomResource);
172+
verify(customResourceFacade, never()).updateStatus(any());
173+
verify(customResourceFacade, never()).replaceResourceWithLock(any());
174+
}
175+
176+
@Test
177+
void patchesStatus() {
178+
testCustomResource.addFinalizer(DEFAULT_FINALIZER);
179+
180+
reconciler.reconcile = (r, c) -> UpdateControl.patchStatus(testCustomResource);
181+
182+
reconciliationDispatcher.handleExecution(executionScopeWithCREvent(testCustomResource));
183+
184+
verify(customResourceFacade, times(1)).patchStatus(testCustomResource);
185+
verify(customResourceFacade, never()).updateStatus(any());
186+
verify(customResourceFacade, never()).replaceResourceWithLock(any());
187+
}
188+
161189
@Test
162190
void callCreateOrUpdateOnModifiedResourceIfFinalizerSet() throws Exception {
163191
testCustomResource.addFinalizer(DEFAULT_FINALIZER);
@@ -216,7 +244,7 @@ void removesDefaultFinalizerOnDeleteIfSet() {
216244
reconciliationDispatcher.handleExecution(executionScopeWithCREvent(testCustomResource));
217245

218246
assertEquals(0, testCustomResource.getMetadata().getFinalizers().size());
219-
verify(customResourceFacade, times(1)).replaceWithLock(any());
247+
verify(customResourceFacade, times(1)).replaceResourceWithLock(any());
220248
}
221249

222250
@Test
@@ -229,7 +257,7 @@ void doesNotRemovesTheSetFinalizerIfTheDeleteNotMethodInstructsIt() {
229257
reconciliationDispatcher.handleExecution(executionScopeWithCREvent(testCustomResource));
230258

231259
assertEquals(1, testCustomResource.getMetadata().getFinalizers().size());
232-
verify(customResourceFacade, never()).replaceWithLock(any());
260+
verify(customResourceFacade, never()).replaceResourceWithLock(any());
233261
}
234262

235263
@Test
@@ -239,7 +267,7 @@ void doesNotUpdateTheResourceIfNoUpdateUpdateControlIfFinalizerSet() throws Exce
239267
reconciler.reconcile = (r, c) -> UpdateControl.noUpdate();
240268

241269
reconciliationDispatcher.handleExecution(executionScopeWithCREvent(testCustomResource));
242-
verify(customResourceFacade, never()).replaceWithLock(any());
270+
verify(customResourceFacade, never()).replaceResourceWithLock(any());
243271
verify(customResourceFacade, never()).updateStatus(testCustomResource);
244272
}
245273

@@ -252,7 +280,7 @@ void addsFinalizerIfNotMarkedForDeletionAndEmptyCustomResourceReturned() throws
252280
reconciliationDispatcher.handleExecution(executionScopeWithCREvent(testCustomResource));
253281

254282
assertEquals(1, testCustomResource.getMetadata().getFinalizers().size());
255-
verify(customResourceFacade, times(1)).replaceWithLock(any());
283+
verify(customResourceFacade, times(1)).replaceResourceWithLock(any());
256284
}
257285

258286
@Test
@@ -262,7 +290,7 @@ void doesNotCallDeleteIfMarkedForDeletionButNotOurFinalizer() {
262290

263291
reconciliationDispatcher.handleExecution(executionScopeWithCREvent(testCustomResource));
264292

265-
verify(customResourceFacade, never()).replaceWithLock(any());
293+
verify(customResourceFacade, never()).replaceResourceWithLock(any());
266294
verify(reconciler, never()).cleanup(eq(testCustomResource), any());
267295
}
268296

@@ -388,7 +416,7 @@ void updateObservedGenerationOnCustomResourceUpdate() throws Exception {
388416
when(config.isGenerationAware()).thenReturn(true);
389417
when(reconciler.reconcile(any(), any()))
390418
.thenReturn(UpdateControl.updateResource(observedGenResource));
391-
when(facade.replaceWithLock(any())).thenReturn(observedGenResource);
419+
when(facade.replaceResourceWithLock(any())).thenReturn(observedGenResource);
392420
when(facade.updateStatus(observedGenResource)).thenReturn(observedGenResource);
393421
var dispatcher = init(observedGenResource, reconciler, config, facade, true);
394422

‎operator-framework/src/test/java/io/javaoperatorsdk/operator/ControllerExecutionIT.java

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
import static org.awaitility.Awaitility.await;
1717

1818
class ControllerExecutionIT {
19+
1920
@RegisterExtension
2021
OperatorExtension operator =
2122
OperatorExtension.builder().withReconciler(new TestReconciler(true)).build();
@@ -32,6 +33,17 @@ void configMapGetsCreatedForTestCustomResource() {
3233
assertThat(TestUtils.getNumberOfExecutions(operator)).isEqualTo(2);
3334
}
3435

36+
@Test
37+
void patchesStatusForTestCustomResource() {
38+
operator.getControllerOfType(TestReconciler.class).setPatchStatus(true);
39+
operator.getControllerOfType(TestReconciler.class).setUpdateStatus(true);
40+
41+
TestCustomResource resource = TestUtils.testCustomResource();
42+
operator.create(TestCustomResource.class, resource);
43+
44+
awaitStatusUpdated();
45+
}
46+
3547
@Test
3648
void eventIsSkippedChangedOnMetadataOnlyUpdate() {
3749
operator.getControllerOfType(TestReconciler.class).setUpdateStatus(false);

‎operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/simple/TestReconciler.java

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,10 +31,21 @@ public class TestReconciler
3131
private final AtomicInteger numberOfExecutions = new AtomicInteger(0);
3232
private final AtomicInteger numberOfCleanupExecutions = new AtomicInteger(0);
3333
private KubernetesClient kubernetesClient;
34-
private boolean updateStatus;
34+
private volatile boolean updateStatus;
35+
private volatile boolean patchStatus;
3536

3637
public TestReconciler(boolean updateStatus) {
38+
this(updateStatus, false);
39+
}
40+
41+
public TestReconciler(boolean updateStatus, boolean patchStatus) {
3742
this.updateStatus = updateStatus;
43+
this.patchStatus = patchStatus;
44+
}
45+
46+
public TestReconciler setPatchStatus(boolean patchStatus) {
47+
this.patchStatus = patchStatus;
48+
return this;
3849
}
3950

4051
public void setUpdateStatus(boolean updateStatus) {
@@ -122,7 +133,11 @@ public UpdateControl<TestCustomResource> reconcile(
122133
}
123134
resource.getStatus().setConfigMapStatus("ConfigMap Ready");
124135
}
125-
return UpdateControl.updateStatus(resource);
136+
if (patchStatus) {
137+
return UpdateControl.patchStatus(resource);
138+
} else {
139+
return UpdateControl.updateStatus(resource);
140+
}
126141
}
127142

128143
private Map<String, String> configMapData(TestCustomResource resource) {

0 commit comments

Comments
 (0)