diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/SSABasedGenericKubernetesResourceMatcher.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/SSABasedGenericKubernetesResourceMatcher.java index 1868c2872d..8dcf9fae08 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/SSABasedGenericKubernetesResourceMatcher.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/SSABasedGenericKubernetesResourceMatcher.java @@ -23,6 +23,7 @@ import io.fabric8.kubernetes.api.model.apps.Deployment; import io.fabric8.kubernetes.api.model.apps.ReplicaSet; import io.fabric8.kubernetes.api.model.apps.StatefulSet; +import io.fabric8.kubernetes.client.KubernetesClient; import io.fabric8.kubernetes.client.utils.KubernetesSerialization; import io.javaoperatorsdk.operator.OperatorException; import io.javaoperatorsdk.operator.api.reconciler.Context; @@ -79,10 +80,14 @@ public static SSABasedGenericKubernetesResourceMatcher context) { - var optionalManagedFieldsEntry = - checkIfFieldManagerExists(actual, context.getControllerConfiguration().fieldManager()); + return matches( + actual, desired, context.getControllerConfiguration().fieldManager(), context.getClient(), false); + } + + @SuppressWarnings("unchecked") + public boolean matches(R actual, R desired, String fieldManager, KubernetesClient client, boolean compareActual) { + var optionalManagedFieldsEntry = checkIfFieldManagerExists(actual, fieldManager); // If no field is managed by our controller, that means the controller hasn't touched the // resource yet and the resource probably doesn't match the desired state. Not matching here // 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) { var managedFieldsEntry = optionalManagedFieldsEntry.orElseThrow(); - var objectMapper = context.getClient().getKubernetesSerialization(); + var objectMapper = client.getKubernetesSerialization(); var actualMap = objectMapper.convertValue(actual, Map.class); var desiredMap = objectMapper.convertValue(desired, Map.class); if (LoggingUtils.isNotSensitiveResource(desired)) { @@ -108,7 +113,18 @@ public boolean matches(R actual, R desired, Context context) { managedFieldsEntry.getFieldsV1().getAdditionalProperties(), objectMapper); - removeIrrelevantValues(desiredMap); + if (compareActual) { + var prunedDesired = new HashMap(actualMap.size()); + keepOnlyManagedFields( + prunedDesired, + desiredMap, + managedFieldsEntry.getFieldsV1().getAdditionalProperties(), + objectMapper); + + desiredMap = prunedDesired; + } else { + removeIrrelevantValues(desiredMap); + } var matches = prunedActual.equals(desiredMap); if (!matches && log.isDebugEnabled() && LoggingUtils.isNotSensitiveResource(desired)) { @@ -438,7 +454,7 @@ private static Map.Entry> selectListEntryBasedOnKey return new AbstractMap.SimpleEntry<>(lastIndex, possibleTargets.get(0)); } - private Optional checkIfFieldManagerExists(R actual, String fieldManager) { + public Optional checkIfFieldManagerExists(R actual, String fieldManager) { var targetManagedFields = actual.getMetadata().getManagedFields().stream() // Only the apply operations are interesting for us since those were created properly be diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerEventSource.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerEventSource.java index 688a88ae22..ad9db86273 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerEventSource.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerEventSource.java @@ -14,6 +14,7 @@ import io.fabric8.kubernetes.client.informers.ResourceEventHandler; import io.javaoperatorsdk.operator.api.config.informer.InformerEventSourceConfiguration; import io.javaoperatorsdk.operator.api.reconciler.EventSourceContext; +import io.javaoperatorsdk.operator.processing.dependent.kubernetes.SSABasedGenericKubernetesResourceMatcher; import io.javaoperatorsdk.operator.processing.event.Event; import io.javaoperatorsdk.operator.processing.event.EventHandler; import io.javaoperatorsdk.operator.processing.event.ResourceID; @@ -68,6 +69,8 @@ public class InformerEventSource private final PrimaryToSecondaryIndex primaryToSecondaryIndex; private final PrimaryToSecondaryMapper

primaryToSecondaryMapper; private final String id = UUID.randomUUID().toString(); + private final KubernetesClient client; + private final String fieldManager; public InformerEventSource( InformerEventSourceConfiguration configuration, EventSourceContext

context) { @@ -77,18 +80,20 @@ public InformerEventSource( context .getControllerConfiguration() .getConfigurationService() - .parseResourceVersionsForEventFilteringAndCaching()); + .parseResourceVersionsForEventFilteringAndCaching(), + context.getControllerConfiguration().fieldManager()); } InformerEventSource(InformerEventSourceConfiguration configuration, KubernetesClient client) { - this(configuration, client, false); + this(configuration, client, false, "manager"); } @SuppressWarnings({"unchecked", "rawtypes"}) private InformerEventSource( InformerEventSourceConfiguration configuration, KubernetesClient client, - boolean parseResourceVersions) { + boolean parseResourceVersions, + String fieldManager) { super( configuration.name(), configuration @@ -112,6 +117,8 @@ private InformerEventSource( onUpdateFilter = informerConfig.getOnUpdateFilter(); onDeleteFilter = informerConfig.getOnDeleteFilter(); genericFilter = informerConfig.getGenericFilter(); + this.client = client; + this.fieldManager = fieldManager; } @Override @@ -215,10 +222,22 @@ private boolean isEventKnownFromAnnotation(R newObject, R oldObject) { if (id.equals(parts[0])) { if (oldObject == null && parts.length == 1) { known = true; - } else if (oldObject != null - && parts.length == 2 - && oldObject.getMetadata().getResourceVersion().equals(parts[1])) { - known = true; + } else if (oldObject != null && parts.length == 2) { + if (oldObject.getMetadata().getResourceVersion().equals(parts[1])) { + known = true; + } else { + // if the resource version doesn't match, there's a chance that it's due + // to a change that is not meaningful, but causes the matcher logic between + // desired and newObject to see a change. + // TODO: this likely should be conditional on useSSA, not just the presence of the + // field manager + var ssaMatcher = SSABasedGenericKubernetesResourceMatcher.getInstance(); + if (ssaMatcher.checkIfFieldManagerExists(newObject, fieldManager).isPresent()) { + known = ssaMatcher.matches(oldObject, newObject, fieldManager, client, true); + } else { + // do we even need to worry about this case + } + } } } } diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/SSABasedGenericKubernetesResourceMatcherTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/SSABasedGenericKubernetesResourceMatcherTest.java index 69bdf59aff..ec59794f6e 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/SSABasedGenericKubernetesResourceMatcherTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/SSABasedGenericKubernetesResourceMatcherTest.java @@ -114,7 +114,25 @@ void addedLabelInDesiredMakesMatchFail() { assertThat(matcher.matches(actualConfigMap, desiredConfigMap, mockedContext)).isFalse(); } + + @Test + void compareActualSame() { + var actualConfigMap = loadResource("configmap.empty-owner-reference.yaml", ConfigMap.class); + assertThat(matcher.matches(actualConfigMap, actualConfigMap, mockedContext.getControllerConfiguration().fieldManager(), mockedContext.getClient(), true)).isTrue(); + } + + @Test + void compareActualSameWithUnownedChange() { + var actualConfigMap = loadResource("configmap.empty-owner-reference.yaml", ConfigMap.class); + + var newConfigMap = loadResource("configmap.empty-owner-reference.yaml", ConfigMap.class); + newConfigMap.getMetadata().setLabels(Map.of("newlabel", "val")); + + assertThat(matcher.matches(newConfigMap, actualConfigMap, mockedContext.getControllerConfiguration().fieldManager(), mockedContext.getClient(), true)).isTrue(); + assertThat(matcher.matches(actualConfigMap, newConfigMap, mockedContext.getControllerConfiguration().fieldManager(), mockedContext.getClient(), true)).isTrue(); + } + @Test @SuppressWarnings("unchecked") void sortListItemsTest() {