diff --git a/docs/documentation/dependent-resources.md b/docs/documentation/dependent-resources.md index b23e19f006..06149cf8ba 100644 --- a/docs/documentation/dependent-resources.md +++ b/docs/documentation/dependent-resources.md @@ -370,6 +370,8 @@ server. To bypass the matching feature completely, simply override the `match` m return `false`, thus telling JOSDK that the actual state never matches the desired one, making it always update the resources using SSA. +WARNING: Older versions of Kubernetes before 1.25 would create an additional resource version for every SSA update performed with certain resources - even though there were no actual changes in the stored resource - leading to infinite reconciliations. This behavior was seen with Secrets using `stringData`, Ingresses using empty string fields, and StatefulSets using volume claim templates. The operator framework has added built-in handling for the StatefulSet issue. If you encounter this issue on an older Kubernetes version, consider changing your desired state, turning off SSA for that resource, or even upgrading your Kubernetes version. If you encounter it on a newer Kubernetes version, please log an issue with the JOSDK and with upstream Kubernetes. + ## Telling JOSDK how to find which secondary resources are associated with a given primary resource [`KubernetesDependentResource`](https://github.com/java-operator-sdk/java-operator-sdk/blob/main/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/KubernetesDependentResource.java) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/DesiredResourceSanitizer.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/DesiredResourceSanitizer.java deleted file mode 100644 index 45b781af7a..0000000000 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/DesiredResourceSanitizer.java +++ /dev/null @@ -1,47 +0,0 @@ -package io.javaoperatorsdk.operator.processing.dependent.kubernetes; - -import io.fabric8.kubernetes.api.model.HasMetadata; -import io.fabric8.kubernetes.api.model.PersistentVolumeClaimStatus; -import io.fabric8.kubernetes.api.model.Secret; -import io.fabric8.kubernetes.api.model.apps.StatefulSet; -import io.javaoperatorsdk.operator.api.reconciler.Context; - -public class DesiredResourceSanitizer { - - private DesiredResourceSanitizer() {} - - public static void sanitizeDesired(R desired, R actual, P primary, - Context

context, boolean useSSA) { - if (useSSA) { - if (desired instanceof StatefulSet) { - fillDefaultsOnVolumeClaimTemplate((StatefulSet) desired); - } - if (desired instanceof Secret) { - checkIfStringDataUsed((Secret) desired); - } - } - } - - private static void checkIfStringDataUsed(Secret secret) { - if (secret.getStringData() != null && !secret.getStringData().isEmpty()) { - throw new IllegalStateException( - "There is a known issue using StringData with SSA. Use data instead."); - } - } - - private static void fillDefaultsOnVolumeClaimTemplate(StatefulSet statefulSet) { - if (!statefulSet.getSpec().getVolumeClaimTemplates().isEmpty()) { - statefulSet.getSpec().getVolumeClaimTemplates().forEach(t -> { - if (t.getSpec().getVolumeMode() == null) { - t.getSpec().setVolumeMode("Filesystem"); - } - if (t.getStatus() == null) { - t.setStatus(new PersistentVolumeClaimStatus()); - } - if (t.getStatus().getPhase() == null) { - t.getStatus().setPhase("pending"); - } - }); - } - } -} diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/KubernetesDependentResource.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/KubernetesDependentResource.java index f81f98f12c..174cf10619 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/KubernetesDependentResource.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/KubernetesDependentResource.java @@ -43,11 +43,14 @@ public abstract class KubernetesDependentResource kubernetesDependentResourceConfig; + private boolean usingCustomResourceUpdateMatcher; + @SuppressWarnings("unchecked") public KubernetesDependentResource(Class resourceType) { super(resourceType); - updaterMatcher = this instanceof ResourceUpdaterMatcher + usingCustomResourceUpdateMatcher = this instanceof ResourceUpdaterMatcher; + updaterMatcher = usingCustomResourceUpdateMatcher ? (ResourceUpdaterMatcher) this : GenericResourceUpdaterMatcher.updaterMatcherFor(resourceType); } @@ -114,7 +117,6 @@ public R create(R desired, P primary, Context

context) { } } addMetadata(false, null, desired, primary, context); - sanitizeDesired(desired, null, primary, context); final var resource = prepare(desired, primary, "Creating"); return useSSA(context) ? resource @@ -131,7 +133,6 @@ public R update(R actual, R desired, P primary, Context

context) { } R updatedResource; addMetadata(false, actual, desired, primary, context); - sanitizeDesired(desired, actual, primary, context); if (useSSA(context)) { updatedResource = prepare(desired, primary, "Updating") .fieldManager(context.getControllerConfiguration().fieldManager()) @@ -148,7 +149,6 @@ public R update(R actual, R desired, P primary, Context

context) { @Override public Result match(R actualResource, P primary, Context

context) { final var desired = desired(primary, context); - sanitizeDesired(desired, actualResource, primary, context); return match(actualResource, desired, primary, updaterMatcher, context); } @@ -193,11 +193,10 @@ protected void addMetadata(boolean forMatch, R actualResource, final R target, P addReferenceHandlingMetadata(target, primary); } - protected void sanitizeDesired(R desired, R actual, P primary, Context

context) { - DesiredResourceSanitizer.sanitizeDesired(desired, actual, primary, context, useSSA(context)); - } - protected boolean useSSA(Context

context) { + if (usingCustomResourceUpdateMatcher) { + return false; + } Optional useSSAConfig = configuration().flatMap(KubernetesDependentResourceConfig::useSSA); var configService = context.getControllerConfiguration().getConfigurationService(); 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 37a5fa9dd2..d699ff2822 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 @@ -7,8 +7,10 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import io.fabric8.kubernetes.api.model.GenericKubernetesResource; import io.fabric8.kubernetes.api.model.HasMetadata; import io.fabric8.kubernetes.api.model.ManagedFieldsEntry; +import io.fabric8.kubernetes.api.model.apps.StatefulSet; import io.fabric8.kubernetes.client.utils.KubernetesSerialization; import io.javaoperatorsdk.operator.OperatorException; import io.javaoperatorsdk.operator.api.reconciler.Context; @@ -74,6 +76,9 @@ public boolean matches(R actual, R desired, Context context) { var objectMapper = context.getClient().getKubernetesSerialization(); var actualMap = objectMapper.convertValue(actual, Map.class); + + sanitizeState(actual, desired, actualMap); + var desiredMap = objectMapper.convertValue(desired, Map.class); if (LoggingUtils.isNotSensitiveResource(desired)) { log.trace("Original actual: \n {} \n original desired: \n {} ", actual, desiredMap); @@ -92,6 +97,35 @@ public boolean matches(R actual, R desired, Context context) { return prunedActual.equals(desiredMap); } + /** + * Correct for known issue with SSA + */ + @SuppressWarnings("unchecked") + private void sanitizeState(R actual, R desired, Map actualMap) { + if (desired instanceof StatefulSet) { + StatefulSet desiredStatefulSet = (StatefulSet) desired; + StatefulSet actualStatefulSet = (StatefulSet) actual; + int claims = desiredStatefulSet.getSpec().getVolumeClaimTemplates().size(); + if (claims == actualStatefulSet.getSpec().getVolumeClaimTemplates().size()) { + for (int i = 0; i < claims; i++) { + if (desiredStatefulSet.getSpec().getVolumeClaimTemplates().get(i).getSpec() + .getVolumeMode() == null) { + Optional + .ofNullable(GenericKubernetesResource.get(actualMap, "spec", "volumeClaimTemplates", + i, "spec")) + .map(Map.class::cast).ifPresent(m -> m.remove("volumeMode")); + } + if (desiredStatefulSet.getSpec().getVolumeClaimTemplates().get(i).getStatus() == null) { + Optional + .ofNullable( + GenericKubernetesResource.get(actualMap, "spec", "volumeClaimTemplates", i)) + .map(Map.class::cast).ifPresent(m -> m.remove("status")); + } + } + } + } + } + @SuppressWarnings("unchecked") private static void removeIrrelevantValues(Map desiredMap) { var metadata = (Map) desiredMap.get(METADATA_KEY); @@ -153,8 +187,7 @@ private static void fillResultsAndTraverseFurther(Map result, var emptyMapValue = new HashMap(); result.put(keyInActual, emptyMapValue); var actualMapValue = actualMap.getOrDefault(keyInActual, Collections.emptyMap()); - log.trace("key: {} actual map value: {} managedFieldValue: {}", keyInActual, - actualMapValue, managedFieldValue); + log.debug("key: {} actual map value: managedFieldValue: {}", keyInActual, managedFieldValue); keepOnlyManagedFields(emptyMapValue, (Map) actualMapValue, (Map) managedFields.get(key), objectMapper); @@ -282,29 +315,16 @@ private static java.util.Map.Entry> selectListEntry } if (possibleTargets.isEmpty()) { throw new IllegalStateException( - "Cannot find list element for key:" + key + ", in map: " + values); + "Cannot find list element for key:" + key + ", in map: " + + values.stream().map(Map::keySet).collect(Collectors.toList())); } if (possibleTargets.size() > 1) { throw new IllegalStateException( - "More targets found in list element for key:" + key + ", in map: " + values); + "More targets found in list element for key:" + key + ", in map: " + + values.stream().map(Map::keySet).collect(Collectors.toList())); } final var finalIndex = index; - return new Map.Entry<>() { - @Override - public Integer getKey() { - return finalIndex; - } - - @Override - public Map getValue() { - return possibleTargets.get(0); - } - - @Override - public Map setValue(Map stringObjectMap) { - throw new IllegalStateException("should not be called"); - } - }; + return new AbstractMap.SimpleEntry<>(finalIndex, possibleTargets.get(0)); } @@ -318,14 +338,14 @@ private Optional checkIfFieldManagerExists(R actual, String .collect(Collectors.toList()); if (targetManagedFields.isEmpty()) { log.debug("No field manager exists for resource {} with name: {} and operation Apply ", - actual, actual.getMetadata().getName()); + actual.getKind(), actual.getMetadata().getName()); return Optional.empty(); } // this should not happen in theory if (targetManagedFields.size() > 1) { throw new OperatorException( "More than one field manager exists with name: " + fieldManager + "in resource: " + - actual + " with name: " + actual.getMetadata().getName()); + actual.getKind() + " with name: " + actual.getMetadata().getName()); } return Optional.of(targetManagedFields.get(0)); }