Skip to content

Commit a5fdead

Browse files
committed
resource version parsing
Signed-off-by: Attila Mészáros <a_meszaros@apple.com>
1 parent 4a02911 commit a5fdead

File tree

6 files changed

+40
-219
lines changed

6 files changed

+40
-219
lines changed

operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationService.java

Lines changed: 0 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -447,64 +447,6 @@ default Set<Class<? extends HasMetadata>> defaultNonSSAResource() {
447447
return defaultNonSSAResources();
448448
}
449449

450-
/**
451-
* If a javaoperatorsdk.io/previous annotation should be used so that the operator sdk can detect
452-
* events from its own updates of dependent resources and then filter them.
453-
*
454-
* <p>Disable this if you want to react to your own dependent resource updates
455-
*
456-
* @return if special annotation should be used for dependent resource to filter events
457-
* @since 4.5.0
458-
*/
459-
default boolean previousAnnotationForDependentResourcesEventFiltering() {
460-
return true;
461-
}
462-
463-
/**
464-
* For dependent resources, the framework can add an annotation to filter out events resulting
465-
* directly from the framework's operation. There are, however, some resources that do not follow
466-
* the Kubernetes API conventions that changes in metadata should not increase the generation of
467-
* the resource (as recorded in the {@code generation} field of the resource's {@code metadata}).
468-
* For these resources, this convention is not respected and results in a new event for the
469-
* framework to process. If that particular case is not handled correctly in the resource matcher,
470-
* the framework will consider that the resource doesn't match the desired state and therefore
471-
* triggers an update, which in turn, will re-add the annotation, thus starting the loop again,
472-
* infinitely.
473-
*
474-
* <p>As a workaround, we automatically skip adding previous annotation for those well-known
475-
* resources. Note that if you are sure that the matcher works for your use case, and it should in
476-
* most instances, you can remove the resource type from the blocklist.
477-
*
478-
* <p>The consequence of adding a resource type to the set is that the framework will not use
479-
* event filtering to prevent events, initiated by changes made by the framework itself as a
480-
* result of its processing of dependent resources, to trigger the associated reconciler again.
481-
*
482-
* <p>Note that this method only takes effect if annotating dependent resources to prevent
483-
* dependent resources events from triggering the associated reconciler again is activated as
484-
* controlled by {@link #previousAnnotationForDependentResourcesEventFiltering()}
485-
*
486-
* @return a Set of resource classes where the previous version annotation won't be used.
487-
*/
488-
default Set<Class<? extends HasMetadata>> withPreviousAnnotationForDependentResourcesBlocklist() {
489-
return Set.of(Deployment.class, StatefulSet.class);
490-
}
491-
492-
/**
493-
* If the event logic should parse the resourceVersion to determine the ordering of dependent
494-
* resource events. This is typically not needed.
495-
*
496-
* <p>Disabled by default as Kubernetes does not support, and discourages, this interpretation of
497-
* resourceVersions. Enable only if your api server event processing seems to lag the operator
498-
* logic, and you want to further minimize the amount of work done / updates issued by the
499-
* operator.
500-
*
501-
* @return if resource version should be parsed (as integer)
502-
* @since 4.5.0
503-
*/
504-
default boolean parseResourceVersionsForEventFilteringAndCaching() {
505-
return false;
506-
}
507-
508450
/**
509451
* {@link io.javaoperatorsdk.operator.api.reconciler.UpdateControl} patch resource or status can
510452
* either use simple patches or SSA. Setting this to {@code true}, controllers will use SSA for

operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationServiceOverrider.java

Lines changed: 30 additions & 87 deletions
Original file line numberDiff line numberDiff line change
@@ -51,11 +51,8 @@ public class ConfigurationServiceOverrider {
5151
private Duration reconciliationTerminationTimeout;
5252
private Boolean ssaBasedCreateUpdateMatchForDependentResources;
5353
private Set<Class<? extends HasMetadata>> defaultNonSSAResource;
54-
private Boolean previousAnnotationForDependentResources;
55-
private Boolean parseResourceVersions;
5654
private Boolean useSSAToPatchPrimaryResource;
5755
private Boolean cloneSecondaryResourcesWhenGettingFromCache;
58-
private Set<Class<? extends HasMetadata>> previousAnnotationUsageBlocklist;
5956

6057
@SuppressWarnings("rawtypes")
6158
private DependentResourceFactory dependentResourceFactory;
@@ -168,31 +165,6 @@ public ConfigurationServiceOverrider withDefaultNonSSAResource(
168165
return this;
169166
}
170167

171-
public ConfigurationServiceOverrider withPreviousAnnotationForDependentResources(boolean value) {
172-
this.previousAnnotationForDependentResources = value;
173-
return this;
174-
}
175-
176-
/**
177-
* @param value true if internal algorithms can use metadata.resourceVersion as a numeric value.
178-
* @return this
179-
*/
180-
public ConfigurationServiceOverrider withParseResourceVersions(boolean value) {
181-
this.parseResourceVersions = value;
182-
return this;
183-
}
184-
185-
/**
186-
* @deprecated use withParseResourceVersions
187-
* @param value true if internal algorithms can use metadata.resourceVersion as a numeric value.
188-
* @return this
189-
*/
190-
@Deprecated(forRemoval = true)
191-
public ConfigurationServiceOverrider wihtParseResourceVersions(boolean value) {
192-
this.parseResourceVersions = value;
193-
return this;
194-
}
195-
196168
public ConfigurationServiceOverrider withUseSSAToPatchPrimaryResource(boolean value) {
197169
this.useSSAToPatchPrimaryResource = value;
198170
return this;
@@ -204,12 +176,6 @@ public ConfigurationServiceOverrider withCloneSecondaryResourcesWhenGettingFromC
204176
return this;
205177
}
206178

207-
public ConfigurationServiceOverrider withPreviousAnnotationForDependentResourcesBlocklist(
208-
Set<Class<? extends HasMetadata>> blocklist) {
209-
this.previousAnnotationUsageBlocklist = blocklist;
210-
return this;
211-
}
212-
213179
public ConfigurationService build() {
214180
return new BaseConfigurationService(original.getVersion(), cloner, client) {
215181
@Override
@@ -218,43 +184,43 @@ public Set<String> getKnownReconcilerNames() {
218184
}
219185

220186
private <T> T overriddenValueOrDefault(
221-
T value, Function<ConfigurationService, T> defaultValue) {
187+
T value, Function<ConfigurationService, T> defaultValue) {
222188
return value != null ? value : defaultValue.apply(original);
223189
}
224190

225191
@Override
226192
public boolean checkCRDAndValidateLocalModel() {
227193
return overriddenValueOrDefault(
228-
checkCR, ConfigurationService::checkCRDAndValidateLocalModel);
194+
checkCR, ConfigurationService::checkCRDAndValidateLocalModel);
229195
}
230196

231197
@SuppressWarnings("rawtypes")
232198
@Override
233199
public DependentResourceFactory dependentResourceFactory() {
234200
return overriddenValueOrDefault(
235-
dependentResourceFactory, ConfigurationService::dependentResourceFactory);
201+
dependentResourceFactory, ConfigurationService::dependentResourceFactory);
236202
}
237203

238204
@Override
239205
public int concurrentReconciliationThreads() {
240206
return Utils.ensureValid(
241-
overriddenValueOrDefault(
242-
concurrentReconciliationThreads,
243-
ConfigurationService::concurrentReconciliationThreads),
244-
"maximum reconciliation threads",
245-
1,
246-
original.concurrentReconciliationThreads());
207+
overriddenValueOrDefault(
208+
concurrentReconciliationThreads,
209+
ConfigurationService::concurrentReconciliationThreads),
210+
"maximum reconciliation threads",
211+
1,
212+
original.concurrentReconciliationThreads());
247213
}
248214

249215
@Override
250216
public int concurrentWorkflowExecutorThreads() {
251217
return Utils.ensureValid(
252-
overriddenValueOrDefault(
253-
concurrentWorkflowExecutorThreads,
254-
ConfigurationService::concurrentWorkflowExecutorThreads),
255-
"maximum workflow execution threads",
256-
1,
257-
original.concurrentWorkflowExecutorThreads());
218+
overriddenValueOrDefault(
219+
concurrentWorkflowExecutorThreads,
220+
ConfigurationService::concurrentWorkflowExecutorThreads),
221+
"maximum workflow execution threads",
222+
1,
223+
original.concurrentWorkflowExecutorThreads());
258224
}
259225

260226
@Override
@@ -288,22 +254,22 @@ public ExecutorService getWorkflowExecutorService() {
288254
@Override
289255
public Optional<LeaderElectionConfiguration> getLeaderElectionConfiguration() {
290256
return leaderElectionConfiguration != null
291-
? Optional.of(leaderElectionConfiguration)
292-
: original.getLeaderElectionConfiguration();
257+
? Optional.of(leaderElectionConfiguration)
258+
: original.getLeaderElectionConfiguration();
293259
}
294260

295261
@Override
296262
public Optional<InformerStoppedHandler> getInformerStoppedHandler() {
297263
return informerStoppedHandler != null
298-
? Optional.of(informerStoppedHandler)
299-
: original.getInformerStoppedHandler();
264+
? Optional.of(informerStoppedHandler)
265+
: original.getInformerStoppedHandler();
300266
}
301267

302268
@Override
303269
public boolean stopOnInformerErrorDuringStartup() {
304270
return overriddenValueOrDefault(
305-
stopOnInformerErrorDuringStartup,
306-
ConfigurationService::stopOnInformerErrorDuringStartup);
271+
stopOnInformerErrorDuringStartup,
272+
ConfigurationService::stopOnInformerErrorDuringStartup);
307273
}
308274

309275
@Override
@@ -314,57 +280,34 @@ public Duration cacheSyncTimeout() {
314280
@Override
315281
public Duration reconciliationTerminationTimeout() {
316282
return overriddenValueOrDefault(
317-
reconciliationTerminationTimeout,
318-
ConfigurationService::reconciliationTerminationTimeout);
283+
reconciliationTerminationTimeout,
284+
ConfigurationService::reconciliationTerminationTimeout);
319285
}
320286

321287
@Override
322288
public boolean ssaBasedCreateUpdateMatchForDependentResources() {
323289
return overriddenValueOrDefault(
324-
ssaBasedCreateUpdateMatchForDependentResources,
325-
ConfigurationService::ssaBasedCreateUpdateMatchForDependentResources);
290+
ssaBasedCreateUpdateMatchForDependentResources,
291+
ConfigurationService::ssaBasedCreateUpdateMatchForDependentResources);
326292
}
327293

328294
@Override
329295
public Set<Class<? extends HasMetadata>> defaultNonSSAResources() {
330296
return overriddenValueOrDefault(
331-
defaultNonSSAResource, ConfigurationService::defaultNonSSAResources);
332-
}
333-
334-
@Override
335-
public boolean previousAnnotationForDependentResourcesEventFiltering() {
336-
return overriddenValueOrDefault(
337-
previousAnnotationForDependentResources,
338-
ConfigurationService::previousAnnotationForDependentResourcesEventFiltering);
339-
}
340-
341-
@Override
342-
public boolean parseResourceVersionsForEventFilteringAndCaching() {
343-
return overriddenValueOrDefault(
344-
parseResourceVersions,
345-
ConfigurationService::parseResourceVersionsForEventFilteringAndCaching);
297+
defaultNonSSAResource, ConfigurationService::defaultNonSSAResources);
346298
}
347299

348300
@Override
349301
public boolean useSSAToPatchPrimaryResource() {
350302
return overriddenValueOrDefault(
351-
useSSAToPatchPrimaryResource, ConfigurationService::useSSAToPatchPrimaryResource);
303+
useSSAToPatchPrimaryResource, ConfigurationService::useSSAToPatchPrimaryResource);
352304
}
353305

354306
@Override
355307
public boolean cloneSecondaryResourcesWhenGettingFromCache() {
356308
return overriddenValueOrDefault(
357-
cloneSecondaryResourcesWhenGettingFromCache,
358-
ConfigurationService::cloneSecondaryResourcesWhenGettingFromCache);
359-
}
360-
361-
@Override
362-
public Set<Class<? extends HasMetadata>>
363-
withPreviousAnnotationForDependentResourcesBlocklist() {
364-
return overriddenValueOrDefault(
365-
previousAnnotationUsageBlocklist,
366-
ConfigurationService::withPreviousAnnotationForDependentResourcesBlocklist);
309+
cloneSecondaryResourcesWhenGettingFromCache,
310+
ConfigurationService::cloneSecondaryResourcesWhenGettingFromCache);
367311
}
368312
};
369-
}
370-
}
313+
}}

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

Lines changed: 0 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,6 @@ public abstract class KubernetesDependentResource<R extends HasMetadata, P exten
5555
private final boolean garbageCollected = this instanceof GarbageCollected;
5656
private KubernetesDependentResourceConfig<R> kubernetesDependentResourceConfig;
5757
private volatile Boolean useSSA;
58-
private volatile Boolean usePreviousAnnotationForEventFiltering;
5958

6059
public KubernetesDependentResource() {}
6160

@@ -158,14 +157,6 @@ protected void addMetadata(
158157
} else {
159158
annotations.remove(InformerEventSource.PREVIOUS_ANNOTATION_KEY);
160159
}
161-
} else if (usePreviousAnnotation(context)) { // set a new one
162-
eventSource()
163-
.orElseThrow()
164-
.addPreviousAnnotation(
165-
Optional.ofNullable(actualResource)
166-
.map(r -> r.getMetadata().getResourceVersion())
167-
.orElse(null),
168-
target);
169160
}
170161
addReferenceHandlingMetadata(target, primary);
171162
}
@@ -181,22 +172,6 @@ protected boolean useSSA(Context<P> context) {
181172
return useSSA;
182173
}
183174

184-
private boolean usePreviousAnnotation(Context<P> context) {
185-
if (usePreviousAnnotationForEventFiltering == null) {
186-
usePreviousAnnotationForEventFiltering =
187-
context
188-
.getControllerConfiguration()
189-
.getConfigurationService()
190-
.previousAnnotationForDependentResourcesEventFiltering()
191-
&& !context
192-
.getControllerConfiguration()
193-
.getConfigurationService()
194-
.withPreviousAnnotationForDependentResourcesBlocklist()
195-
.contains(this.resourceType());
196-
}
197-
return usePreviousAnnotationForEventFiltering;
198-
}
199-
200175
@Override
201176
protected void handleDelete(P primary, R secondary, Context<P> context) {
202177
if (secondary != null) {

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

Lines changed: 3 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -84,30 +84,20 @@ public InformerEventSource(
8484
InformerEventSourceConfiguration<R> configuration, EventSourceContext<P> context) {
8585
this(
8686
configuration,
87-
configuration.getKubernetesClient().orElse(context.getClient()),
88-
context
89-
.getControllerConfiguration()
90-
.getConfigurationService()
91-
.parseResourceVersionsForEventFilteringAndCaching());
92-
}
93-
94-
InformerEventSource(InformerEventSourceConfiguration<R> configuration, KubernetesClient client) {
95-
this(configuration, client, false);
87+
configuration.getKubernetesClient().orElse(context.getClient()));
9688
}
9789

9890
@SuppressWarnings({"unchecked", "rawtypes"})
9991
private InformerEventSource(
10092
InformerEventSourceConfiguration<R> configuration,
101-
KubernetesClient client,
102-
boolean parseResourceVersions) {
93+
KubernetesClient client) {
10394
super(
10495
configuration.name(),
10596
configuration
10697
.getGroupVersionKind()
10798
.map(gvk -> client.genericKubernetesResources(gvk.apiVersion(), gvk.getKind()))
10899
.orElseGet(() -> (MixedOperation) client.resources(configuration.getResourceClass())),
109-
configuration,
110-
parseResourceVersions);
100+
configuration);
111101
// If there is a primary to secondary mapper there is no need for primary to secondary index.
112102
primaryToSecondaryMapper = configuration.getPrimaryToSecondaryMapper();
113103
if (useSecondaryToPrimaryIndex()) {
@@ -333,22 +323,6 @@ private boolean acceptedByDeleteFilters(R resource, boolean b) {
333323
&& (genericFilter == null || genericFilter.accept(resource));
334324
}
335325

336-
/**
337-
* Add an annotation to the resource so that the subsequent will be omitted
338-
*
339-
* @param resourceVersion null if there is no prior version
340-
* @param target mutable resource that will be returned
341-
*/
342-
public R addPreviousAnnotation(String resourceVersion, R target) {
343-
target
344-
.getMetadata()
345-
.getAnnotations()
346-
.put(
347-
PREVIOUS_ANNOTATION_KEY,
348-
id + Optional.ofNullable(resourceVersion).map(rv -> "," + rv).orElse(""));
349-
return target;
350-
}
351-
352326
private enum Operation {
353327
ADD,
354328
UPDATE

0 commit comments

Comments
 (0)