Skip to content

Commit 1f6560c

Browse files
authored
improve: complete comparable resource version configs (#3027)
Signed-off-by: Attila Mészáros <[email protected]>
1 parent de30b10 commit 1f6560c

File tree

10 files changed

+52
-62
lines changed

10 files changed

+52
-62
lines changed

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

Lines changed: 0 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -44,8 +44,6 @@
4444
import io.javaoperatorsdk.operator.processing.dependent.workflow.ManagedWorkflowFactory;
4545
import io.javaoperatorsdk.operator.processing.event.source.controller.ControllerEventSource;
4646

47-
import static io.javaoperatorsdk.operator.api.reconciler.Constants.DEFAULT_COMPARABLE_RESOURCE_VERSIONS;
48-
4947
/** An interface from which to retrieve configuration information. */
5048
public interface ConfigurationService {
5149

@@ -447,19 +445,6 @@ default Set<Class<? extends HasMetadata>> defaultNonSSAResource() {
447445
return defaultNonSSAResources();
448446
}
449447

450-
/**
451-
* If the event logic can compare resourceVersions.
452-
*
453-
* <p>Enabled by default as Kubernetes does support this interpretation of resourceVersions.
454-
* Disable only if your api server provides non comparable resource versions.
455-
*
456-
* @return if resource versions are comparable
457-
* @since 5.3.0
458-
*/
459-
default boolean comparableResourceVersions() {
460-
return DEFAULT_COMPARABLE_RESOURCE_VERSIONS;
461-
}
462-
463448
/**
464449
* {@link io.javaoperatorsdk.operator.api.reconciler.UpdateControl} patch resource or status can
465450
* 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: 0 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,6 @@ public class ConfigurationServiceOverrider {
5151
private Duration reconciliationTerminationTimeout;
5252
private Boolean ssaBasedCreateUpdateMatchForDependentResources;
5353
private Set<Class<? extends HasMetadata>> defaultNonSSAResource;
54-
private Boolean comparableResourceVersions;
5554
private Boolean useSSAToPatchPrimaryResource;
5655
private Boolean cloneSecondaryResourcesWhenGettingFromCache;
5756

@@ -166,26 +165,6 @@ public ConfigurationServiceOverrider withDefaultNonSSAResource(
166165
return this;
167166
}
168167

169-
/**
170-
* @param value true if internal algorithms can use metadata.resourceVersion as a numeric value.
171-
* @return this
172-
*/
173-
public ConfigurationServiceOverrider withComparableResourceVersions(boolean value) {
174-
this.comparableResourceVersions = value;
175-
return this;
176-
}
177-
178-
/**
179-
* @deprecated use withComparableResourceVersions
180-
* @param value true if internal algorithms can use metadata.resourceVersion as a numeric value.
181-
* @return this
182-
*/
183-
@Deprecated(forRemoval = true)
184-
public ConfigurationServiceOverrider withParseResourceVersions(boolean value) {
185-
this.comparableResourceVersions = value;
186-
return this;
187-
}
188-
189168
public ConfigurationServiceOverrider withUseSSAToPatchPrimaryResource(boolean value) {
190169
this.useSSAToPatchPrimaryResource = value;
191170
return this;
@@ -330,12 +309,6 @@ public boolean cloneSecondaryResourcesWhenGettingFromCache() {
330309
cloneSecondaryResourcesWhenGettingFromCache,
331310
ConfigurationService::cloneSecondaryResourcesWhenGettingFromCache);
332311
}
333-
334-
@Override
335-
public boolean comparableResourceVersions() {
336-
return overriddenValueOrDefault(
337-
comparableResourceVersions, ConfigurationService::comparableResourceVersions);
338-
}
339312
};
340313
}
341314
}

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

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
import io.javaoperatorsdk.operator.processing.event.source.filter.OnDeleteFilter;
2929
import io.javaoperatorsdk.operator.processing.event.source.filter.OnUpdateFilter;
3030

31+
import static io.javaoperatorsdk.operator.api.reconciler.Constants.DEFAULT_COMPARABLE_RESOURCE_VERSION;
3132
import static io.javaoperatorsdk.operator.api.reconciler.Constants.DEFAULT_FOLLOW_CONTROLLER_NAMESPACE_CHANGES;
3233
import static io.javaoperatorsdk.operator.api.reconciler.Constants.NO_LONG_VALUE_SET;
3334
import static io.javaoperatorsdk.operator.api.reconciler.Constants.NO_VALUE_SET;
@@ -131,4 +132,11 @@
131132

132133
/** Kubernetes field selector for additional resource filtering */
133134
Field[] fieldSelector() default {};
135+
136+
/**
137+
* true if we can consider resource versions as integers, therefore it is valid to compare them
138+
*
139+
* @since 5.3.0
140+
*/
141+
boolean comparableResourceVersions() default DEFAULT_COMPARABLE_RESOURCE_VERSION;
134142
}

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

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@ public class InformerConfiguration<R extends HasMetadata> {
5353
private ItemStore<R> itemStore;
5454
private Long informerListLimit;
5555
private FieldSelector fieldSelector;
56+
private boolean comparableResourceVersions;
5657

5758
protected InformerConfiguration(
5859
Class<R> resourceClass,
@@ -66,7 +67,8 @@ protected InformerConfiguration(
6667
GenericFilter<? super R> genericFilter,
6768
ItemStore<R> itemStore,
6869
Long informerListLimit,
69-
FieldSelector fieldSelector) {
70+
FieldSelector fieldSelector,
71+
boolean comparableResourceVersions) {
7072
this(resourceClass);
7173
this.name = name;
7274
this.namespaces = namespaces;
@@ -79,6 +81,7 @@ protected InformerConfiguration(
7981
this.itemStore = itemStore;
8082
this.informerListLimit = informerListLimit;
8183
this.fieldSelector = fieldSelector;
84+
this.comparableResourceVersions = comparableResourceVersions;
8285
}
8386

8487
private InformerConfiguration(Class<R> resourceClass) {
@@ -113,7 +116,8 @@ public static <R extends HasMetadata> InformerConfiguration<R>.Builder builder(
113116
original.genericFilter,
114117
original.itemStore,
115118
original.informerListLimit,
116-
original.fieldSelector)
119+
original.fieldSelector,
120+
original.comparableResourceVersions)
117121
.builder;
118122
}
119123

@@ -288,6 +292,10 @@ public FieldSelector getFieldSelector() {
288292
return fieldSelector;
289293
}
290294

295+
public boolean isComparableResourceVersions() {
296+
return comparableResourceVersions;
297+
}
298+
291299
@SuppressWarnings("UnusedReturnValue")
292300
public class Builder {
293301

@@ -359,6 +367,7 @@ public InformerConfiguration<R>.Builder initFromAnnotation(
359367
Arrays.stream(informerConfig.fieldSelector())
360368
.map(f -> new FieldSelector.Field(f.path(), f.value(), f.negated()))
361369
.toList()));
370+
withComparableResourceVersions(informerConfig.comparableResourceVersions());
362371
}
363372
return this;
364373
}
@@ -459,5 +468,10 @@ public Builder withFieldSelector(FieldSelector fieldSelector) {
459468
InformerConfiguration.this.fieldSelector = fieldSelector;
460469
return this;
461470
}
471+
472+
public Builder withComparableResourceVersions(boolean comparableResourceVersions) {
473+
InformerConfiguration.this.comparableResourceVersions = comparableResourceVersions;
474+
return this;
475+
}
462476
}
463477
}

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

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@
3333
import io.javaoperatorsdk.operator.processing.event.source.filter.OnUpdateFilter;
3434
import io.javaoperatorsdk.operator.processing.event.source.informer.Mappers;
3535

36-
import static io.javaoperatorsdk.operator.api.reconciler.Constants.DEFAULT_COMPARABLE_RESOURCE_VERSIONS;
36+
import static io.javaoperatorsdk.operator.api.reconciler.Constants.DEFAULT_COMPARABLE_RESOURCE_VERSION;
3737
import static io.javaoperatorsdk.operator.api.reconciler.Constants.SAME_AS_CONTROLLER_NAMESPACES_SET;
3838
import static io.javaoperatorsdk.operator.api.reconciler.Constants.WATCH_ALL_NAMESPACE_SET;
3939
import static io.javaoperatorsdk.operator.api.reconciler.Constants.WATCH_CURRENT_NAMESPACE_SET;
@@ -97,21 +97,21 @@ class DefaultInformerEventSourceConfiguration<R extends HasMetadata>
9797
private final GroupVersionKind groupVersionKind;
9898
private final InformerConfiguration<R> informerConfig;
9999
private final KubernetesClient kubernetesClient;
100-
private final boolean comparableResourceVersions;
100+
private final boolean comparableResourceVersion;
101101

102102
protected DefaultInformerEventSourceConfiguration(
103103
GroupVersionKind groupVersionKind,
104104
PrimaryToSecondaryMapper<?> primaryToSecondaryMapper,
105105
SecondaryToPrimaryMapper<R> secondaryToPrimaryMapper,
106106
InformerConfiguration<R> informerConfig,
107107
KubernetesClient kubernetesClient,
108-
boolean comparableResourceVersions) {
108+
boolean comparableResourceVersion) {
109109
this.informerConfig = Objects.requireNonNull(informerConfig);
110110
this.groupVersionKind = groupVersionKind;
111111
this.primaryToSecondaryMapper = primaryToSecondaryMapper;
112112
this.secondaryToPrimaryMapper = secondaryToPrimaryMapper;
113113
this.kubernetesClient = kubernetesClient;
114-
this.comparableResourceVersions = comparableResourceVersions;
114+
this.comparableResourceVersion = comparableResourceVersion;
115115
}
116116

117117
@Override
@@ -141,8 +141,8 @@ public Optional<KubernetesClient> getKubernetesClient() {
141141
}
142142

143143
@Override
144-
public boolean comparableResourceVersions() {
145-
return this.comparableResourceVersions;
144+
public boolean comparableResourceVersion() {
145+
return this.comparableResourceVersion;
146146
}
147147
}
148148

@@ -157,7 +157,7 @@ class Builder<R extends HasMetadata> {
157157
private PrimaryToSecondaryMapper<?> primaryToSecondaryMapper;
158158
private SecondaryToPrimaryMapper<R> secondaryToPrimaryMapper;
159159
private KubernetesClient kubernetesClient;
160-
private boolean comparableResourceVersions = DEFAULT_COMPARABLE_RESOURCE_VERSIONS;
160+
private boolean comparableResourceVersion = DEFAULT_COMPARABLE_RESOURCE_VERSION;
161161

162162
private Builder(Class<R> resourceClass, Class<? extends HasMetadata> primaryResourceClass) {
163163
this(resourceClass, primaryResourceClass, null);
@@ -295,8 +295,8 @@ public Builder<R> withFieldSelector(FieldSelector fieldSelector) {
295295
return this;
296296
}
297297

298-
public Builder<R> withComparableResourceVersions(boolean comparableResourceVersions) {
299-
this.comparableResourceVersions = comparableResourceVersions;
298+
public Builder<R> withComparableResourceVersion(boolean comparableResourceVersion) {
299+
this.comparableResourceVersion = comparableResourceVersion;
300300
return this;
301301
}
302302

@@ -340,9 +340,9 @@ public InformerEventSourceConfiguration<R> build() {
340340
false)),
341341
config.build(),
342342
kubernetesClient,
343-
comparableResourceVersions);
343+
comparableResourceVersion);
344344
}
345345
}
346346

347-
boolean comparableResourceVersions();
347+
boolean comparableResourceVersion();
348348
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ public final class Constants {
4141
public static final String RESOURCE_GVK_KEY = "josdk.resource.gvk";
4242
public static final String CONTROLLER_NAME = "controller.name";
4343
public static final boolean DEFAULT_FOLLOW_CONTROLLER_NAMESPACE_CHANGES = true;
44-
public static final boolean DEFAULT_COMPARABLE_RESOURCE_VERSIONS = true;
44+
public static final boolean DEFAULT_COMPARABLE_RESOURCE_VERSION = true;
4545

4646
private Constants() {}
4747
}

operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/controller/ControllerEventSource.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ public ControllerEventSource(Controller<T> controller) {
5151
NAME,
5252
controller.getCRClient(),
5353
controller.getConfiguration(),
54-
controller.getConfiguration().getConfigurationService().comparableResourceVersions());
54+
controller.getConfiguration().getInformerConfig().isComparableResourceVersions());
5555
this.controller = controller;
5656

5757
final var config = controller.getConfiguration();

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@
3535
import io.javaoperatorsdk.operator.processing.event.ResourceID;
3636
import io.javaoperatorsdk.operator.processing.event.source.PrimaryToSecondaryMapper;
3737

38-
import static io.javaoperatorsdk.operator.api.reconciler.Constants.DEFAULT_COMPARABLE_RESOURCE_VERSIONS;
38+
import static io.javaoperatorsdk.operator.api.reconciler.Constants.DEFAULT_COMPARABLE_RESOURCE_VERSION;
3939

4040
/**
4141
* Wraps informer(s) so they are connected to the eventing system of the framework. Note that since
@@ -86,11 +86,11 @@ public InformerEventSource(
8686
this(
8787
configuration,
8888
configuration.getKubernetesClient().orElse(context.getClient()),
89-
configuration.comparableResourceVersions());
89+
configuration.comparableResourceVersion());
9090
}
9191

9292
InformerEventSource(InformerEventSourceConfiguration<R> configuration, KubernetesClient client) {
93-
this(configuration, client, DEFAULT_COMPARABLE_RESOURCE_VERSIONS);
93+
this(configuration, client, DEFAULT_COMPARABLE_RESOURCE_VERSION);
9494
}
9595

9696
@SuppressWarnings({"unchecked", "rawtypes"})

operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/createupdateeventfilter/ComparableResourceVersionsDisabledIT.java

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,8 +25,7 @@ class ComparableResourceVersionsDisabledIT {
2525
@RegisterExtension
2626
LocallyRunOperatorExtension operator =
2727
LocallyRunOperatorExtension.builder()
28-
.withReconciler(new CreateUpdateEventFilterTestReconciler())
29-
.withConfigurationService(overrider -> overrider.withComparableResourceVersions(false))
28+
.withReconciler(new CreateUpdateEventFilterTestReconciler(false))
3029
.build();
3130

3231
@Test

operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/createupdateeventfilter/CreateUpdateEventFilterTestReconciler.java

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,16 @@ public class CreateUpdateEventFilterTestReconciler
4141
private final DirectConfigMapDependentResource configMapDR =
4242
new DirectConfigMapDependentResource(ConfigMap.class);
4343

44+
private final boolean comparableResourceVersion;
45+
46+
public CreateUpdateEventFilterTestReconciler(boolean comparableResourceVersion) {
47+
this.comparableResourceVersion = comparableResourceVersion;
48+
}
49+
50+
public CreateUpdateEventFilterTestReconciler() {
51+
this(true);
52+
}
53+
4454
@Override
4555
public UpdateControl<CreateUpdateEventFilterTestCustomResource> reconcile(
4656
CreateUpdateEventFilterTestCustomResource resource,
@@ -89,6 +99,7 @@ public List<EventSource<?, CreateUpdateEventFilterTestCustomResource>> prepareEv
8999
InformerEventSourceConfiguration.from(
90100
ConfigMap.class, CreateUpdateEventFilterTestCustomResource.class)
91101
.withLabelSelector("integrationtest = " + this.getClass().getSimpleName())
102+
.withComparableResourceVersion(comparableResourceVersion)
92103
.build();
93104

94105
final var informerEventSource = new InformerEventSource<>(informerConfiguration, context);

0 commit comments

Comments
 (0)