Skip to content

Commit 599002a

Browse files
committed
fix: use SSA matcher to filter events
closes: operator-framework#2249 Signed-off-by: Steve Hawkins <[email protected]>
1 parent 7725ff4 commit 599002a

File tree

2 files changed

+37
-12
lines changed

2 files changed

+37
-12
lines changed

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

+10-5
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
import io.fabric8.kubernetes.api.model.apps.Deployment;
2424
import io.fabric8.kubernetes.api.model.apps.ReplicaSet;
2525
import io.fabric8.kubernetes.api.model.apps.StatefulSet;
26+
import io.fabric8.kubernetes.client.KubernetesClient;
2627
import io.fabric8.kubernetes.client.utils.KubernetesSerialization;
2728
import io.javaoperatorsdk.operator.OperatorException;
2829
import io.javaoperatorsdk.operator.api.reconciler.Context;
@@ -79,10 +80,14 @@ public static <L extends HasMetadata> SSABasedGenericKubernetesResourceMatcher<L
7980
private static final Logger log =
8081
LoggerFactory.getLogger(SSABasedGenericKubernetesResourceMatcher.class);
8182

82-
@SuppressWarnings("unchecked")
8383
public boolean matches(R actual, R desired, Context<?> context) {
84-
var optionalManagedFieldsEntry =
85-
checkIfFieldManagerExists(actual, context.getControllerConfiguration().fieldManager());
84+
return matches(
85+
actual, desired, context.getControllerConfiguration().fieldManager(), context.getClient());
86+
}
87+
88+
@SuppressWarnings("unchecked")
89+
public boolean matches(R actual, R desired, String fieldManager, KubernetesClient client) {
90+
var optionalManagedFieldsEntry = checkIfFieldManagerExists(actual, fieldManager);
8691
// If no field is managed by our controller, that means the controller hasn't touched the
8792
// resource yet and the resource probably doesn't match the desired state. Not matching here
8893
// means that the resource will need to be updated and since this will be done using SSA, the
@@ -93,7 +98,7 @@ public boolean matches(R actual, R desired, Context<?> context) {
9398

9499
var managedFieldsEntry = optionalManagedFieldsEntry.orElseThrow();
95100

96-
var objectMapper = context.getClient().getKubernetesSerialization();
101+
var objectMapper = client.getKubernetesSerialization();
97102
var actualMap = objectMapper.convertValue(actual, Map.class);
98103
var desiredMap = objectMapper.convertValue(desired, Map.class);
99104
if (LoggingUtils.isNotSensitiveResource(desired)) {
@@ -438,7 +443,7 @@ private static Map.Entry<Integer, Map<String, Object>> selectListEntryBasedOnKey
438443
return new AbstractMap.SimpleEntry<>(lastIndex, possibleTargets.get(0));
439444
}
440445

441-
private Optional<ManagedFieldsEntry> checkIfFieldManagerExists(R actual, String fieldManager) {
446+
public Optional<ManagedFieldsEntry> checkIfFieldManagerExists(R actual, String fieldManager) {
442447
var targetManagedFields =
443448
actual.getMetadata().getManagedFields().stream()
444449
// Only the apply operations are interesting for us since those were created properly be

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

+27-7
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,8 @@
1414
import io.fabric8.kubernetes.client.informers.ResourceEventHandler;
1515
import io.javaoperatorsdk.operator.api.config.informer.InformerEventSourceConfiguration;
1616
import io.javaoperatorsdk.operator.api.reconciler.EventSourceContext;
17+
import io.javaoperatorsdk.operator.processing.dependent.kubernetes.GenericKubernetesResourceMatcher;
18+
import io.javaoperatorsdk.operator.processing.dependent.kubernetes.SSABasedGenericKubernetesResourceMatcher;
1719
import io.javaoperatorsdk.operator.processing.event.Event;
1820
import io.javaoperatorsdk.operator.processing.event.EventHandler;
1921
import io.javaoperatorsdk.operator.processing.event.ResourceID;
@@ -68,6 +70,8 @@ public class InformerEventSource<R extends HasMetadata, P extends HasMetadata>
6870
private final PrimaryToSecondaryIndex<R> primaryToSecondaryIndex;
6971
private final PrimaryToSecondaryMapper<P> primaryToSecondaryMapper;
7072
private final String id = UUID.randomUUID().toString();
73+
private final KubernetesClient client;
74+
private final String fieldManager;
7175

7276
public InformerEventSource(
7377
InformerEventSourceConfiguration<R> configuration, EventSourceContext<P> context) {
@@ -77,18 +81,20 @@ public InformerEventSource(
7781
context
7882
.getControllerConfiguration()
7983
.getConfigurationService()
80-
.parseResourceVersionsForEventFilteringAndCaching());
84+
.parseResourceVersionsForEventFilteringAndCaching(),
85+
context.getControllerConfiguration().fieldManager());
8186
}
8287

8388
InformerEventSource(InformerEventSourceConfiguration<R> configuration, KubernetesClient client) {
84-
this(configuration, client, false);
89+
this(configuration, client, false, "manager");
8590
}
8691

8792
@SuppressWarnings({"unchecked", "rawtypes"})
8893
private InformerEventSource(
8994
InformerEventSourceConfiguration<R> configuration,
9095
KubernetesClient client,
91-
boolean parseResourceVersions) {
96+
boolean parseResourceVersions,
97+
String fieldManager) {
9298
super(
9399
configuration.name(),
94100
configuration
@@ -112,6 +118,8 @@ private InformerEventSource(
112118
onUpdateFilter = informerConfig.getOnUpdateFilter();
113119
onDeleteFilter = informerConfig.getOnDeleteFilter();
114120
genericFilter = informerConfig.getGenericFilter();
121+
this.client = client;
122+
this.fieldManager = fieldManager;
115123
}
116124

117125
@Override
@@ -215,10 +223,22 @@ private boolean isEventKnownFromAnnotation(R newObject, R oldObject) {
215223
if (id.equals(parts[0])) {
216224
if (oldObject == null && parts.length == 1) {
217225
known = true;
218-
} else if (oldObject != null
219-
&& parts.length == 2
220-
&& oldObject.getMetadata().getResourceVersion().equals(parts[1])) {
221-
known = true;
226+
} else if (oldObject != null && parts.length == 2) {
227+
if (oldObject.getMetadata().getResourceVersion().equals(parts[1])) {
228+
known = true;
229+
} else {
230+
// if the resource version doesn't match, there's a chance that it's due
231+
// to a change that is not meaningful, but causes the matcher logic between
232+
// desired and newObject to see a change.
233+
// TODO: this likely should be conditional on useSSA, not just the presense of the
234+
// field manager
235+
var ssaMatcher = SSABasedGenericKubernetesResourceMatcher.getInstance();
236+
if (ssaMatcher.checkIfFieldManagerExists(newObject, fieldManager).isPresent()) {
237+
known = ssaMatcher.matches(newObject, oldObject, fieldManager, client);
238+
} else {
239+
// do we even need to worry about this case
240+
}
241+
}
222242
}
223243
}
224244
}

0 commit comments

Comments
 (0)