Skip to content

Commit 754f920

Browse files
csviricsviri
committed
fix: event filtering edge case with no-op updates - extended
Signed-off-by: Attila Mészáros <a_meszaros@apple.com>
1 parent ba7a7fa commit 754f920

7 files changed

Lines changed: 188 additions & 42 deletions

File tree

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

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
/*
2+
* Copyright Java Operator SDK Authors
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
package io.javaoperatorsdk.operator.api.reconciler;
17+
18+
import io.fabric8.kubernetes.api.model.HasMetadata;
19+
20+
public interface Matcher {
21+
22+
<R extends HasMetadata> boolean matches(R desired, R actual, Context<?> context);
23+
}

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

Lines changed: 144 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -166,6 +166,10 @@ public <R extends HasMetadata> R serverSideApplyStatus(R resource) {
166166
return serverSideApplyStatus(resource, null);
167167
}
168168

169+
public P serverSideApplyPrimary(P resource) {
170+
return serverSideApplyPrimary(resource, Options.defaultMode());
171+
}
172+
169173
/**
170174
* Server-Side Apply the primary resource.
171175
*
@@ -180,7 +184,7 @@ public <R extends HasMetadata> R serverSideApplyStatus(R resource) {
180184
* @param resource primary resource for server side apply
181185
* @return updated resource
182186
*/
183-
public P serverSideApplyPrimary(P resource) {
187+
public P serverSideApplyPrimary(P resource, Options options) {
184188
return resourcePatch(
185189
resource,
186190
r ->
@@ -193,7 +197,8 @@ public P serverSideApplyPrimary(P resource) {
193197
.withFieldManager(context.getControllerConfiguration().fieldManager())
194198
.withPatchType(PatchType.SERVER_SIDE_APPLY)
195199
.build()),
196-
context.eventSourceRetriever().getControllerEventSource());
200+
context.eventSourceRetriever().getControllerEventSource(),
201+
options);
197202
}
198203

199204
/**
@@ -211,6 +216,10 @@ public P serverSideApplyPrimary(P resource) {
211216
* @return updated resource
212217
*/
213218
public P serverSideApplyPrimaryStatus(P resource) {
219+
return serverSideApplyPrimaryStatus(resource, Options.defaultMode());
220+
}
221+
222+
public P serverSideApplyPrimaryStatus(P resource, Options options) {
214223
return resourcePatch(
215224
resource,
216225
r ->
@@ -224,11 +233,12 @@ public P serverSideApplyPrimaryStatus(P resource) {
224233
.withFieldManager(context.getControllerConfiguration().fieldManager())
225234
.withPatchType(PatchType.SERVER_SIDE_APPLY)
226235
.build()),
227-
context.eventSourceRetriever().getControllerEventSource());
236+
context.eventSourceRetriever().getControllerEventSource(),
237+
options);
228238
}
229239

230240
public <R extends HasMetadata> R update(R resource) {
231-
return update(resource, (Options) null);
241+
return update(resource, Options.defaultMode());
232242
}
233243

234244
/**
@@ -251,7 +261,7 @@ public <R extends HasMetadata> R update(R resource, Options options) {
251261

252262
public <R extends HasMetadata> R update(
253263
R resource, InformerEventSource<R, P> informerEventSource) {
254-
return update(resource, informerEventSource, new Options(true));
264+
return update(resource, informerEventSource, Options.defaultMode());
255265
}
256266

257267
/**
@@ -292,8 +302,12 @@ public <R extends HasMetadata> R update(
292302
* @param <R> resource type
293303
*/
294304
public <R extends HasMetadata> R create(R resource) {
295-
return resourcePatch(
296-
resource, r -> context.getClient().resource(r).create(), new Options(true));
305+
// it is safe to do event filtering for create since check if the resource already exists.
306+
return create(resource, Options.forcedFiltering());
307+
}
308+
309+
public <R extends HasMetadata> R create(R resource, Options options) {
310+
return resourcePatch(resource, r -> context.getClient().resource(r).create(), options);
297311
}
298312

299313
/**
@@ -311,16 +325,13 @@ public <R extends HasMetadata> R create(R resource) {
311325
* @param <R> resource type
312326
*/
313327
public <R extends HasMetadata> R create(
314-
R resource, InformerEventSource<R, P> informerEventSource) {
328+
R resource, InformerEventSource<R, P> informerEventSource, Options options) {
315329
if (informerEventSource == null) {
316330
return create(resource);
317331
}
318332
// it is safe to do event filtering for create since check if the resource already exists.
319333
return resourcePatch(
320-
resource,
321-
r -> context.getClient().resource(r).create(),
322-
informerEventSource,
323-
new Options(true));
334+
resource, r -> context.getClient().resource(r).create(), informerEventSource, options);
324335
}
325336

326337
/**
@@ -339,8 +350,11 @@ public <R extends HasMetadata> R create(
339350
* @param <R> resource type
340351
*/
341352
public <R extends HasMetadata> R updateStatus(R resource) {
342-
return resourcePatch(
343-
resource, r -> context.getClient().resource(r).updateStatus(), new Options(true));
353+
return updateStatus(resource, Options.defaultMode());
354+
}
355+
356+
public <R extends HasMetadata> R updateStatus(R resource, Options options) {
357+
return resourcePatch(resource, r -> context.getClient().resource(r).updateStatus(), options);
344358
}
345359

346360
/**
@@ -358,10 +372,15 @@ public <R extends HasMetadata> R updateStatus(R resource) {
358372
* @return updated resource
359373
*/
360374
public P updatePrimary(P resource) {
375+
return updatePrimary(resource, Options.defaultMode());
376+
}
377+
378+
public P updatePrimary(P resource, Options options) {
361379
return resourcePatch(
362380
resource,
363381
r -> context.getClient().resource(r).update(),
364-
context.eventSourceRetriever().getControllerEventSource());
382+
context.eventSourceRetriever().getControllerEventSource(),
383+
options);
365384
}
366385

367386
/**
@@ -412,6 +431,18 @@ public <R extends HasMetadata> R jsonPatch(
412431
resource, r -> context.getClient().resource(r).edit(unaryOperator), options);
413432
}
414433

434+
public <R extends HasMetadata> R jsonPatch(
435+
R resource,
436+
UnaryOperator<R> unaryOperator,
437+
InformerEventSource<R, P> informerEventSource,
438+
Options options) {
439+
return resourcePatch(
440+
resource,
441+
r -> context.getClient().resource(r).edit(unaryOperator),
442+
informerEventSource,
443+
options);
444+
}
445+
415446
/**
416447
* Applies a JSON Patch to the resource status subresource. The unaryOperator function is used to
417448
* modify the resource status, and the differences are sent as a JSON Patch.
@@ -436,7 +467,19 @@ public <R extends HasMetadata> R jsonPatchStatus(
436467
}
437468

438469
public <R extends HasMetadata> R jsonPatchStatus(R resource, UnaryOperator<R> unaryOperator) {
439-
return jsonPatchStatus(resource, unaryOperator, null);
470+
return jsonPatchStatus(resource, unaryOperator, Options.defaultMode());
471+
}
472+
473+
public <R extends HasMetadata> R jsonPatchStatus(
474+
R resource,
475+
UnaryOperator<R> unaryOperator,
476+
InformerEventSource<R, P> informerEventSource,
477+
Options options) {
478+
return resourcePatch(
479+
resource,
480+
r -> context.getClient().resource(r).editStatus(unaryOperator),
481+
informerEventSource,
482+
options);
440483
}
441484

442485
/**
@@ -455,10 +498,15 @@ public <R extends HasMetadata> R jsonPatchStatus(R resource, UnaryOperator<R> un
455498
* @return updated resource
456499
*/
457500
public P jsonPatchPrimary(P resource, UnaryOperator<P> unaryOperator) {
501+
return jsonPatchPrimary(resource, unaryOperator, Options.defaultMode());
502+
}
503+
504+
public P jsonPatchPrimary(P resource, UnaryOperator<P> unaryOperator, Options options) {
458505
return resourcePatch(
459506
resource,
460507
r -> context.getClient().resource(r).edit(unaryOperator),
461-
context.eventSourceRetriever().getControllerEventSource());
508+
context.eventSourceRetriever().getControllerEventSource(),
509+
options);
462510
}
463511

464512
/**
@@ -477,10 +525,15 @@ public P jsonPatchPrimary(P resource, UnaryOperator<P> unaryOperator) {
477525
* @return updated resource
478526
*/
479527
public P jsonPatchPrimaryStatus(P resource, UnaryOperator<P> unaryOperator) {
528+
return jsonPatchPrimaryStatus(resource, unaryOperator, Options.defaultMode());
529+
}
530+
531+
public P jsonPatchPrimaryStatus(P resource, UnaryOperator<P> unaryOperator, Options options) {
480532
return resourcePatch(
481533
resource,
482534
r -> context.getClient().resource(r).editStatus(unaryOperator),
483-
context.eventSourceRetriever().getControllerEventSource());
535+
context.eventSourceRetriever().getControllerEventSource(),
536+
options);
484537
}
485538

486539
/**
@@ -500,13 +553,19 @@ public P jsonPatchPrimaryStatus(P resource, UnaryOperator<P> unaryOperator) {
500553
* @param <R> resource type
501554
*/
502555
public <R extends HasMetadata> R jsonMergePatch(R resource) {
503-
return jsonMergePatch(resource, null);
556+
return jsonMergePatch(resource, Options.defaultMode());
504557
}
505558

506559
public <R extends HasMetadata> R jsonMergePatch(R resource, Options options) {
507560
return resourcePatch(resource, r -> context.getClient().resource(r).patch(), options);
508561
}
509562

563+
public <R extends HasMetadata> R jsonMergePatch(
564+
R resource, InformerEventSource<R, P> informerEventSource, Options options) {
565+
return resourcePatch(
566+
resource, r -> context.getClient().resource(r).patch(), informerEventSource, options);
567+
}
568+
510569
/**
511570
* Applies a JSON Merge Patch to the resource status subresource.
512571
*
@@ -530,6 +589,12 @@ public <R extends HasMetadata> R jsonMergePatchStatus(R resource, Options option
530589
return resourcePatch(resource, r -> context.getClient().resource(r).patchStatus(), options);
531590
}
532591

592+
public <R extends HasMetadata> R jsonMergePatchStatus(
593+
R resource, InformerEventSource<R, P> informerEventSource, Options options) {
594+
return resourcePatch(
595+
resource, r -> context.getClient().resource(r).patchStatus(), informerEventSource, options);
596+
}
597+
533598
/**
534599
* Applies a JSON Merge Patch to the primary resource. Caches the response using the controller's
535600
* event source.
@@ -546,10 +611,15 @@ public <R extends HasMetadata> R jsonMergePatchStatus(R resource, Options option
546611
* @return updated resource
547612
*/
548613
public P jsonMergePatchPrimary(P resource) {
614+
return jsonMergePatchPrimary(resource, Options.defaultMode());
615+
}
616+
617+
public P jsonMergePatchPrimary(P resource, Options options) {
549618
return resourcePatch(
550619
resource,
551620
r -> context.getClient().resource(r).patch(),
552-
context.eventSourceRetriever().getControllerEventSource());
621+
context.eventSourceRetriever().getControllerEventSource(),
622+
options);
553623
}
554624

555625
/**
@@ -568,14 +638,19 @@ public P jsonMergePatchPrimary(P resource) {
568638
* @see #jsonMergePatchPrimaryStatus(HasMetadata)
569639
*/
570640
public P jsonMergePatchPrimaryStatus(P resource) {
641+
return jsonMergePatchPrimaryStatus(resource, Options.defaultMode());
642+
}
643+
644+
public P jsonMergePatchPrimaryStatus(P resource, Options options) {
571645
return resourcePatch(
572646
resource,
573647
r -> context.getClient().resource(r).patchStatus(),
574-
context.eventSourceRetriever().getControllerEventSource());
648+
context.eventSourceRetriever().getControllerEventSource(),
649+
options);
575650
}
576651

577652
public <R extends HasMetadata> R resourcePatch(R resource, UnaryOperator<R> updateOperation) {
578-
return resourcePatch(resource, updateOperation, (Options) null);
653+
return resourcePatch(resource, updateOperation, Options.defaultMode());
579654
}
580655

581656
/**
@@ -636,8 +711,12 @@ public <R extends HasMetadata> R resourcePatch(
636711
UnaryOperator<R> updateOperation,
637712
ManagedInformerEventSource<R, P, ?> ies,
638713
Options options) {
639-
return ies.eventFilteringUpdateAndCacheResource(
640-
resource, updateOperation, options != null && options.forceEventFiltering());
714+
if (options != null && options.getMode() == Mode.ONLY_CACHE) {
715+
return ies.updateAndCacheResource(resource, updateOperation);
716+
} else {
717+
return ies.eventFilteringUpdateAndCacheResource(
718+
resource, updateOperation, options != null && options.isForcedFiltering());
719+
}
641720
}
642721

643722
/**
@@ -827,11 +906,45 @@ public P addFinalizerWithSSA(String finalizerName) {
827906
}
828907
}
829908

830-
/**
831-
* Force filtering only if it is made sure that the update not results on a no-op change. See
832-
* details here: <a href="https://github.com/operator-framework/java-operator-sdk/pull/3484">PR
833-
* 3484</a>
834-
*/
835-
@Experimental("This API might change")
836-
public record Options(boolean forceEventFiltering) {}
909+
// this is designed with forward compatibility in mynd
910+
@Experimental(API_MIGHT_CHANGE)
911+
public static class Options {
912+
913+
private static final Options FORCED_FILTERING = new Options(Mode.FORCED_FILTERING);
914+
private static final Options ONLY_CACHE = new Options(Mode.ONLY_CACHE);
915+
private static final Options DEFAULT = new Options(Mode.FILTERING_IF_OPTIMISTIC_LOCKING);
916+
917+
private final Mode mode;
918+
919+
public static Options forcedFiltering() {
920+
return FORCED_FILTERING;
921+
}
922+
923+
public static Options onlyCache() {
924+
return ONLY_CACHE;
925+
}
926+
927+
public static Options defaultMode() {
928+
return DEFAULT;
929+
}
930+
931+
private Options(Mode mode) {
932+
this.mode = mode;
933+
}
934+
935+
public Mode getMode() {
936+
return mode;
937+
}
938+
939+
public boolean isForcedFiltering() {
940+
return mode != Mode.ONLY_CACHE;
941+
}
942+
}
943+
944+
@Experimental(API_MIGHT_CHANGE)
945+
public enum Mode {
946+
FILTERING_IF_OPTIMISTIC_LOCKING,
947+
FORCED_FILTERING,
948+
ONLY_CACHE,
949+
}
837950
}

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

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,10 @@ public R create(R desired, P primary, Context<P> context) {
9393
ssa);
9494
return ssa
9595
? context.resourceOperations().serverSideApply(desired, eventSource().orElse(null))
96-
: context.resourceOperations().create(desired, eventSource().orElse(null));
96+
: context
97+
.resourceOperations()
98+
.create(
99+
desired, eventSource().orElse(null), ResourceOperations.Options.forcedFiltering());
97100
}
98101

99102
public R update(R actual, R desired, P primary, Context<P> context) {
@@ -117,7 +120,9 @@ public R update(R actual, R desired, P primary, Context<P> context) {
117120
context
118121
.resourceOperations()
119122
.serverSideApply(
120-
desired, eventSource().orElse(null), new ResourceOperations.Options(true));
123+
desired,
124+
eventSource().orElse(null),
125+
ResourceOperations.Options.forcedFiltering());
121126
} else {
122127
var updatedActual = GenericResourceUpdater.updateResource(actual, desired, context);
123128
updatedResource =

0 commit comments

Comments
 (0)