Skip to content

Commit b114845

Browse files
shawkinsshawkins
committed
removing tombstones
Signed-off-by: Steve Hawkins <shawkins@redhat.com>
1 parent 27f050c commit b114845

File tree

12 files changed

+150
-208
lines changed

12 files changed

+150
-208
lines changed

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

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -491,18 +491,16 @@ default Set<Class<? extends HasMetadata>> withPreviousAnnotationForDependentReso
491491

492492
/**
493493
* If the event logic should parse the resourceVersion to determine the ordering of dependent
494-
* resource events. This is typically not needed.
494+
* resource events.
495495
*
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.
496+
* <p>Enabled by default as Kubernetes does support this interpretation of resourceVersions.
497+
* Disable only if your api server provides non comparable resource versions..
500498
*
501499
* @return if resource version should be parsed (as integer)
502500
* @since 4.5.0
503501
*/
504502
default boolean parseResourceVersionsForEventFilteringAndCaching() {
505-
return false;
503+
return true;
506504
}
507505

508506
/**

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

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -96,18 +96,21 @@ class DefaultInformerEventSourceConfiguration<R extends HasMetadata>
9696
private final GroupVersionKind groupVersionKind;
9797
private final InformerConfiguration<R> informerConfig;
9898
private final KubernetesClient kubernetesClient;
99+
private final boolean comparableResourceVersions;
99100

100101
protected DefaultInformerEventSourceConfiguration(
101102
GroupVersionKind groupVersionKind,
102103
PrimaryToSecondaryMapper<?> primaryToSecondaryMapper,
103104
SecondaryToPrimaryMapper<R> secondaryToPrimaryMapper,
104105
InformerConfiguration<R> informerConfig,
105-
KubernetesClient kubernetesClient) {
106+
KubernetesClient kubernetesClient,
107+
boolean comparableResourceVersions) {
106108
this.informerConfig = Objects.requireNonNull(informerConfig);
107109
this.groupVersionKind = groupVersionKind;
108110
this.primaryToSecondaryMapper = primaryToSecondaryMapper;
109111
this.secondaryToPrimaryMapper = secondaryToPrimaryMapper;
110112
this.kubernetesClient = kubernetesClient;
113+
this.comparableResourceVersions = comparableResourceVersions;
111114
}
112115

113116
@Override
@@ -135,6 +138,11 @@ public Optional<GroupVersionKind> getGroupVersionKind() {
135138
public Optional<KubernetesClient> getKubernetesClient() {
136139
return Optional.ofNullable(kubernetesClient);
137140
}
141+
142+
@Override
143+
public boolean parseResourceVersionsForEventFilteringAndCaching() {
144+
return this.comparableResourceVersions;
145+
}
138146
}
139147

140148
@SuppressWarnings({"unused", "UnusedReturnValue"})
@@ -148,6 +156,7 @@ class Builder<R extends HasMetadata> {
148156
private PrimaryToSecondaryMapper<?> primaryToSecondaryMapper;
149157
private SecondaryToPrimaryMapper<R> secondaryToPrimaryMapper;
150158
private KubernetesClient kubernetesClient;
159+
private boolean comparableResourceVersions = true;
151160

152161
private Builder(Class<R> resourceClass, Class<? extends HasMetadata> primaryResourceClass) {
153162
this(resourceClass, primaryResourceClass, null);
@@ -285,6 +294,11 @@ public Builder<R> withFieldSelector(FieldSelector fieldSelector) {
285294
return this;
286295
}
287296

297+
public Builder<R> parseResourceVersionsForEventFilteringAndCaching(boolean parse) {
298+
this.comparableResourceVersions = parse;
299+
return this;
300+
}
301+
288302
public void updateFrom(InformerConfiguration<R> informerConfig) {
289303
if (informerConfig != null) {
290304
final var informerConfigName = informerConfig.getName();
@@ -324,7 +338,10 @@ public InformerEventSourceConfiguration<R> build() {
324338
HasMetadata.getKind(primaryResourceClass),
325339
false)),
326340
config.build(),
327-
kubernetesClient);
341+
kubernetesClient,
342+
comparableResourceVersions);
328343
}
329344
}
345+
346+
boolean parseResourceVersionsForEventFilteringAndCaching();
330347
}

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -451,6 +451,11 @@ public static <P extends HasMetadata> P addFinalizerWithSSA(
451451
}
452452
}
453453

454+
public static int compareResourceVersions(HasMetadata h1, HasMetadata h2) {
455+
return compareResourceVersions(
456+
h1.getMetadata().getResourceVersion(), h2.getMetadata().getResourceVersion());
457+
}
458+
454459
public static int compareResourceVersions(String v1, String v2) {
455460
var v1Length = v1.length();
456461
if (v1Length == 0) {

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

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,14 @@ public class ControllerEventSource<T extends HasMetadata>
4747

4848
@SuppressWarnings({"unchecked", "rawtypes"})
4949
public ControllerEventSource(Controller<T> controller) {
50-
super(NAME, controller.getCRClient(), controller.getConfiguration(), false);
50+
super(
51+
NAME,
52+
controller.getCRClient(),
53+
controller.getConfiguration(),
54+
controller
55+
.getConfiguration()
56+
.getConfigurationService()
57+
.parseResourceVersionsForEventFilteringAndCaching());
5158
this.controller = controller;
5259

5360
final var config = controller.getConfiguration();

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

Lines changed: 5 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -85,14 +85,11 @@ public InformerEventSource(
8585
this(
8686
configuration,
8787
configuration.getKubernetesClient().orElse(context.getClient()),
88-
context
89-
.getControllerConfiguration()
90-
.getConfigurationService()
91-
.parseResourceVersionsForEventFilteringAndCaching());
88+
configuration.parseResourceVersionsForEventFilteringAndCaching());
9289
}
9390

9491
InformerEventSource(InformerEventSourceConfiguration<R> configuration, KubernetesClient client) {
95-
this(configuration, client, false);
92+
this(configuration, client, true);
9693
}
9794

9895
@SuppressWarnings({"unchecked", "rawtypes"})
@@ -207,21 +204,8 @@ private synchronized void onAddOrUpdate(
207204
}
208205

209206
private boolean canSkipEvent(R newObject, R oldObject, ResourceID resourceID) {
210-
var res = temporaryResourceCache.getResourceFromCache(resourceID);
211-
if (res.isEmpty()) {
212-
return isEventKnownFromAnnotation(newObject, oldObject);
213-
}
214-
boolean resVersionsEqual =
215-
newObject
216-
.getMetadata()
217-
.getResourceVersion()
218-
.equals(res.get().getMetadata().getResourceVersion());
219-
log.debug(
220-
"Resource found in temporal cache for id: {} resource versions equal: {}",
221-
resourceID,
222-
resVersionsEqual);
223-
return resVersionsEqual
224-
|| temporaryResourceCache.isLaterResourceVersion(resourceID, res.get(), newObject);
207+
return temporaryResourceCache.canSkipEvent(resourceID, newObject)
208+
|| isEventKnownFromAnnotation(newObject, oldObject);
225209
}
226210

227211
private boolean isEventKnownFromAnnotation(R newObject, R oldObject) {
@@ -301,11 +285,7 @@ public synchronized void handleRecentResourceCreate(ResourceID resourceID, R res
301285

302286
private void handleRecentCreateOrUpdate(Operation operation, R newResource, R oldResource) {
303287
primaryToSecondaryIndex.onAddOrUpdate(newResource);
304-
temporaryResourceCache.putResource(
305-
newResource,
306-
Optional.ofNullable(oldResource)
307-
.map(r -> r.getMetadata().getResourceVersion())
308-
.orElse(null));
288+
temporaryResourceCache.putResource(newResource);
309289
}
310290

311291
private boolean useSecondaryToPrimaryIndex() {

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -221,6 +221,11 @@ public Optional<R> get(ResourceID resourceID) {
221221
: r);
222222
}
223223

224+
public Optional<String> getLastSyncResourceVersion(Optional<String> namespace) {
225+
return getSource(namespace.orElse(WATCH_ALL_NAMESPACES))
226+
.map(source -> source.getLastSyncResourceVersion());
227+
}
228+
224229
@Override
225230
public Stream<ResourceID> keys() {
226231
return sources.values().stream().flatMap(Cache::keys);

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -156,6 +156,10 @@ public Optional<T> get(ResourceID resourceID) {
156156
return Optional.ofNullable(cache.getByKey(getKey(resourceID)));
157157
}
158158

159+
public String getLastSyncResourceVersion() {
160+
return this.informer.lastSyncResourceVersion();
161+
}
162+
159163
private String getKey(ResourceID resourceID) {
160164
return Cache.namespaceKeyFunc(resourceID.getNamespace().orElse(null), resourceID.getName());
161165
}

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

Lines changed: 22 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@
3434
import io.javaoperatorsdk.operator.api.config.ControllerConfiguration;
3535
import io.javaoperatorsdk.operator.api.config.Informable;
3636
import io.javaoperatorsdk.operator.api.config.NamespaceChangeable;
37+
import io.javaoperatorsdk.operator.api.reconciler.PrimaryUpdateAndCacheUtils;
3738
import io.javaoperatorsdk.operator.api.reconciler.dependent.RecentOperationCacheFiller;
3839
import io.javaoperatorsdk.operator.health.InformerHealthIndicator;
3940
import io.javaoperatorsdk.operator.health.InformerWrappingEventSourceHealthIndicator;
@@ -122,30 +123,38 @@ public synchronized void stop() {
122123
@Override
123124
public void handleRecentResourceUpdate(
124125
ResourceID resourceID, R resource, R previousVersionOfResource) {
125-
temporaryResourceCache.putResource(
126-
resource, previousVersionOfResource.getMetadata().getResourceVersion());
126+
temporaryResourceCache.putResource(resource);
127127
}
128128

129129
@Override
130130
public void handleRecentResourceCreate(ResourceID resourceID, R resource) {
131-
temporaryResourceCache.putAddedResource(resource);
131+
temporaryResourceCache.putResource(resource);
132132
}
133133

134134
@Override
135135
public Optional<R> get(ResourceID resourceID) {
136+
var res = cache.get(resourceID);
136137
Optional<R> resource = temporaryResourceCache.getResourceFromCache(resourceID);
137-
if (resource.isPresent()) {
138-
log.debug("Resource found in temporary cache for Resource ID: {}", resourceID);
138+
if (parseResourceVersions
139+
&& resource.isPresent()
140+
&& res.filter(
141+
r ->
142+
PrimaryUpdateAndCacheUtils.compareResourceVersions(r, resource.orElseThrow())
143+
> 0)
144+
.isEmpty()) {
145+
log.debug("Latest resource found in temporary cache for Resource ID: {}", resourceID);
139146
return resource;
140-
} else {
141-
log.debug(
142-
"Resource not found in temporary cache reading it from informer cache,"
143-
+ " for Resource ID: {}",
144-
resourceID);
145-
var res = cache.get(resourceID);
146-
log.debug("Resource found in cache: {} for id: {}", res.isPresent(), resourceID);
147-
return res;
148147
}
148+
log.debug(
149+
"Resource not found, or older, in temporary cache. Found in informer cache {}, for"
150+
+ " Resource ID: {}",
151+
res.isPresent(),
152+
resourceID);
153+
return res;
154+
}
155+
156+
public Optional<String> getLastSyncResourceVersion(Optional<String> namespace) {
157+
return cache.getLastSyncResourceVersion(namespace);
149158
}
150159

151160
@SuppressWarnings("unused")

0 commit comments

Comments
 (0)