From 6f81e16f3faea6ae99c2a89aae549fde55868834 Mon Sep 17 00:00:00 2001 From: David Sondermann Date: Fri, 25 Oct 2024 06:52:38 +0000 Subject: [PATCH 1/5] refactor: clean up SSABasedGenericKubernetesResourceMatcher Signed-off-by: David Sondermann --- ...BasedGenericKubernetesResourceMatcher.java | 139 ++++++++---------- 1 file changed, 65 insertions(+), 74 deletions(-) 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 bcfaa52d1a..8188fe8ea3 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 @@ -1,12 +1,5 @@ package io.javaoperatorsdk.operator.processing.dependent.kubernetes; -import java.util.*; -import java.util.Map.Entry; -import java.util.stream.Collectors; - -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; @@ -15,17 +8,32 @@ import io.javaoperatorsdk.operator.OperatorException; import io.javaoperatorsdk.operator.api.reconciler.Context; import io.javaoperatorsdk.operator.processing.LoggingUtils; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import java.util.AbstractMap; +import java.util.ArrayList; +import java.util.Collections; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.Map.Entry; +import java.util.Objects; +import java.util.Optional; +import java.util.Set; +import java.util.TreeMap; +import java.util.stream.Collectors; /** * Matches the actual state on the server vs the desired state. Based on the managedFields of SSA. - * *

- * The basis of algorithm is to extract the fields managed we convert resources to Map/List + * The basis of the algorithm is to extract the managed fields by converting resources to a Map/List * composition. The actual resource (from the server) is pruned, all the fields which are not - * mentioed in managedFields of the target manager is removed. Some irrelevant fields are also - * removed from desired. And the two resulted Maps are compared for equality. The implementation is - * a bit nasty since have to deal with some specific cases of managedFields format. - *

+ * mentioned in managedFields of the target manager are removed. Some irrelevant fields are also + * removed from the desired resource. Finally, the two resulting maps are compared for equality. + *

+ * The implementation is a bit nasty since we have to deal with some specific cases of managedFields + * formats. * * @param matched resource type */ @@ -35,15 +43,14 @@ // see also: https://kubernetes.slack.com/archives/C0123CNN8F3/p1686141087220719 public class SSABasedGenericKubernetesResourceMatcher { - @SuppressWarnings("rawtypes") - private static final SSABasedGenericKubernetesResourceMatcher INSTANCE = - new SSABasedGenericKubernetesResourceMatcher<>(); public static final String APPLY_OPERATION = "Apply"; public static final String DOT_KEY = "."; + @SuppressWarnings("rawtypes") + private static final SSABasedGenericKubernetesResourceMatcher INSTANCE = + new SSABasedGenericKubernetesResourceMatcher<>(); private static final List IGNORED_METADATA = - Arrays.asList("creationTimestamp", "deletionTimestamp", - "generation", "selfLink", "uid"); + List.of("creationTimestamp", "deletionTimestamp", "generation", "selfLink", "uid"); @SuppressWarnings("unchecked") public static SSABasedGenericKubernetesResourceMatcher getInstance() { @@ -77,16 +84,13 @@ public boolean matches(R actual, R desired, Context context) { var managedFieldsEntry = optionalManagedFieldsEntry.orElseThrow(); 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); + log.trace("Original actual:\n {}\n original desired:\n {}", actualMap, desiredMap); } + sanitizeState(actual, desired, actualMap); var prunedActual = new HashMap(actualMap.size()); keepOnlyManagedFields(prunedActual, actualMap, managedFieldsEntry.getFieldsV1().getAdditionalProperties(), @@ -104,24 +108,22 @@ public boolean matches(R actual, R desired, Context context) { /** * 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()) { + if (actual instanceof StatefulSet) { + var actualSpec = (((StatefulSet) actual)).getSpec(); + var desiredSpec = (((StatefulSet) desired)).getSpec(); + int claims = desiredSpec.getVolumeClaimTemplates().size(); + if (claims == actualSpec.getVolumeClaimTemplates().size()) { for (int i = 0; i < claims; i++) { - if (desiredStatefulSet.getSpec().getVolumeClaimTemplates().get(i).getSpec() - .getVolumeMode() == null) { + var claim = desiredSpec.getVolumeClaimTemplates().get(i); + if (claim.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)) + if (claim.getStatus() == null) { + Optional.ofNullable( + GenericKubernetesResource.get(actualMap, "spec", "volumeClaimTemplates", i)) .map(Map.class::cast).ifPresent(m -> m.remove("status")); } } @@ -146,19 +148,17 @@ private static void removeIrrelevantValues(Map desiredMap) { private static void keepOnlyManagedFields(Map result, Map actualMap, Map managedFields, KubernetesSerialization objectMapper) { - if (managedFields.isEmpty()) { result.putAll(actualMap); return; } - for (Map.Entry entry : managedFields.entrySet()) { - String key = entry.getKey(); + for (var entry : managedFields.entrySet()) { + var key = entry.getKey(); if (key.startsWith(F_PREFIX)) { - String keyInActual = keyWithoutPrefix(key); + var keyInActual = keyWithoutPrefix(key); var managedFieldValue = (Map) entry.getValue(); if (isNestedValue(managedFieldValue)) { var managedEntrySet = managedFieldValue.entrySet(); - // two special cases "k:" and "v:" prefixes if (isListKeyEntrySet(managedEntrySet)) { handleListKeyEntrySet(result, actualMap, objectMapper, keyInActual, managedEntrySet); @@ -194,7 +194,6 @@ private static void fillResultsAndTraverseFurther(Map result, result.put(keyInActual, emptyMapValue); var actualMapValue = actualMap.getOrDefault(keyInActual, Collections.emptyMap()); log.debug("key: {} actual map value: managedFieldValue: {}", keyInActual, managedFieldValue); - keepOnlyManagedFields(emptyMapValue, (Map) actualMapValue, (Map) managedFields.get(key), objectMapper); } @@ -222,10 +221,10 @@ private static void handleListKeyEntrySet(Map result, result.put(keyInActual, valueList); var actualValueList = (List>) actualMap.get(keyInActual); - SortedMap> targetValuesByIndex = new TreeMap<>(); - Map> managedEntryByIndex = new HashMap<>(); + var targetValuesByIndex = new TreeMap>(); + var managedEntryByIndex = new HashMap>(); - for (Map.Entry listEntry : managedEntrySet) { + for (var listEntry : managedEntrySet) { if (DOT_KEY.equals(listEntry.getKey())) { continue; } @@ -244,29 +243,26 @@ private static void handleListKeyEntrySet(Map result, } /** - * Set values, the "v:" prefix. Form in managed fields: "f:some-set":{"v:1":{}},"v:2":{},"v:3":{}} + * Set values, the {@code "v:"} prefix. Form in managed fields: + * {@code "f:some-set":{"v:1":{}},"v:2":{},"v:3":{}}. + *

* Note that this should be just used in very rare cases, actually was not able to produce a * sample. Kubernetes developers who worked on this feature were not able to provide one either * when prompted. Basically this method just adds the values from {@code "v:"} to the * result. */ - @SuppressWarnings("rawtypes") private static void handleSetValues(Map result, Map actualMap, KubernetesSerialization objectMapper, String keyInActual, Set> managedEntrySet) { var valueList = new ArrayList<>(); result.put(keyInActual, valueList); - for (Map.Entry valueEntry : managedEntrySet) { + for (var valueEntry : managedEntrySet) { // not clear if this can happen if (DOT_KEY.equals(valueEntry.getKey())) { continue; } - Class targetClass = null; - List values = (List) actualMap.get(keyInActual); - if (!(values.get(0) instanceof Map)) { - targetClass = values.get(0).getClass(); - } - + var values = (List) actualMap.get(keyInActual); + var targetClass = (values.get(0) instanceof Map) ? null : values.get(0).getClass(); var value = parseKeyValue(keyWithoutPrefix(valueEntry.getKey()), targetClass, objectMapper); valueList.add(value); } @@ -274,12 +270,8 @@ private static void handleSetValues(Map result, Map targetClass, KubernetesSerialization objectMapper) { - stringValue = stringValue.trim(); - if (targetClass != null) { - return objectMapper.unmarshal(stringValue, targetClass); - } else { - return objectMapper.unmarshal(stringValue, Map.class); - } + var type = Objects.requireNonNullElse(targetClass, Map.class); + return objectMapper.unmarshal(stringValue.trim(), type); } private static boolean isSetValueField(Set> managedEntrySet) { @@ -306,30 +298,29 @@ private static boolean isKeyPrefixedSkippingDotKey(Set } @SuppressWarnings("unchecked") - private static java.util.Map.Entry> selectListEntryBasedOnKey( + private static Map.Entry> selectListEntryBasedOnKey( String key, List> values, KubernetesSerialization objectMapper) { Map ids = objectMapper.unmarshal(key, Map.class); - List> possibleTargets = new ArrayList<>(1); - int index = -1; + var possibleTargets = new ArrayList>(1); + int lastIndex = -1; for (int i = 0; i < values.size(); i++) { - var v = values.get(i); - if (v.entrySet().containsAll(ids.entrySet())) { - possibleTargets.add(v); - index = i; + var value = values.get(i); + if (value.entrySet().containsAll(ids.entrySet())) { + possibleTargets.add(value); + lastIndex = i; } } if (possibleTargets.isEmpty()) { - throw new IllegalStateException("Cannot find list element for key:" + key + ", in map: " + throw new IllegalStateException("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: " + "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 AbstractMap.SimpleEntry<>(finalIndex, possibleTargets.get(0)); + return new AbstractMap.SimpleEntry<>(lastIndex, possibleTargets.get(0)); } private Optional checkIfFieldManagerExists(R actual, String fieldManager) { @@ -341,15 +332,16 @@ private Optional checkIfFieldManagerExists(R actual, String f -> f.getManager().equals(fieldManager) && f.getOperation().equals(APPLY_OPERATION)) .collect(Collectors.toList()); if (targetManagedFields.isEmpty()) { - log.debug("No field manager exists for resource {} with name: {} and operation Apply ", + log.debug("No field manager exists for resource: {} with name: {} and operation {}", actual.getKind(), - actual.getMetadata().getName()); + actual.getMetadata().getName(), + APPLY_OPERATION); 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.getKind() + " with name: " + actual.getMetadata().getName()); + + " in resource: " + actual.getKind() + " with name: " + actual.getMetadata().getName()); } return Optional.of(targetManagedFields.get(0)); } @@ -357,5 +349,4 @@ private Optional checkIfFieldManagerExists(R actual, String private static String keyWithoutPrefix(String key) { return key.substring(2); } - } From 1a4cdea9751b8242963f976f04f1985d2be8819b Mon Sep 17 00:00:00 2001 From: David Sondermann Date: Sun, 27 Oct 2024 09:19:56 +0000 Subject: [PATCH 2/5] test: add missing tests for StatefulSet with VolumeClaimTemplates for SSABasedGenericKubernetesResourceMatcher Signed-off-by: David Sondermann --- operator-framework-core/pom.xml | 5 ++ ...dGenericKubernetesResourceMatcherTest.java | 53 ++++++++++---- ...-sts-volumeclaimtemplates-desired-add.yaml | 43 ++++++++++++ ...s-volumeclaimtemplates-desired-update.yaml | 34 +++++++++ ...emplates-desired-with-status-mismatch.yaml | 36 ++++++++++ ...umeclaimtemplates-desired-with-status.yaml | 36 ++++++++++ ...ates-desired-with-volumemode-mismatch.yaml | 35 ++++++++++ ...laimtemplates-desired-with-volumemode.yaml | 35 ++++++++++ ...mple-sts-volumeclaimtemplates-desired.yaml | 34 +++++++++ .../sample-sts-volumeclaimtemplates.yaml | 69 +++++++++++++++++++ 10 files changed, 366 insertions(+), 14 deletions(-) create mode 100644 operator-framework-core/src/test/resources/io/javaoperatorsdk/operator/processing/dependent/kubernetes/sample-sts-volumeclaimtemplates-desired-add.yaml create mode 100644 operator-framework-core/src/test/resources/io/javaoperatorsdk/operator/processing/dependent/kubernetes/sample-sts-volumeclaimtemplates-desired-update.yaml create mode 100644 operator-framework-core/src/test/resources/io/javaoperatorsdk/operator/processing/dependent/kubernetes/sample-sts-volumeclaimtemplates-desired-with-status-mismatch.yaml create mode 100644 operator-framework-core/src/test/resources/io/javaoperatorsdk/operator/processing/dependent/kubernetes/sample-sts-volumeclaimtemplates-desired-with-status.yaml create mode 100644 operator-framework-core/src/test/resources/io/javaoperatorsdk/operator/processing/dependent/kubernetes/sample-sts-volumeclaimtemplates-desired-with-volumemode-mismatch.yaml create mode 100644 operator-framework-core/src/test/resources/io/javaoperatorsdk/operator/processing/dependent/kubernetes/sample-sts-volumeclaimtemplates-desired-with-volumemode.yaml create mode 100644 operator-framework-core/src/test/resources/io/javaoperatorsdk/operator/processing/dependent/kubernetes/sample-sts-volumeclaimtemplates-desired.yaml create mode 100644 operator-framework-core/src/test/resources/io/javaoperatorsdk/operator/processing/dependent/kubernetes/sample-sts-volumeclaimtemplates.yaml diff --git a/operator-framework-core/pom.xml b/operator-framework-core/pom.xml index 1875f33bea..92f1c62297 100644 --- a/operator-framework-core/pom.xml +++ b/operator-framework-core/pom.xml @@ -101,6 +101,11 @@ junit-jupiter-engine test + + org.junit.jupiter + junit-jupiter-params + test + org.mockito mockito-core 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 cf0f67887b..22bf40c7a0 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 @@ -1,18 +1,20 @@ package io.javaoperatorsdk.operator.processing.dependent.kubernetes; -import java.util.Map; - -import org.junit.jupiter.api.BeforeEach; -import org.junit.jupiter.api.Test; - import io.fabric8.kubernetes.api.model.ConfigMap; import io.fabric8.kubernetes.api.model.HasMetadata; import io.fabric8.kubernetes.api.model.apps.Deployment; +import io.fabric8.kubernetes.api.model.apps.StatefulSet; import io.javaoperatorsdk.operator.MockKubernetesClient; import io.javaoperatorsdk.operator.ReconcilerUtils; import io.javaoperatorsdk.operator.api.config.ConfigurationService; import io.javaoperatorsdk.operator.api.config.ControllerConfiguration; import io.javaoperatorsdk.operator.api.reconciler.Context; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; + +import java.util.Map; import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.Mockito.mock; @@ -20,22 +22,21 @@ class SSABasedGenericKubernetesResourceMatcherTest { - Context mockedContext = mock(Context.class); + private final Context mockedContext = mock(); - SSABasedGenericKubernetesResourceMatcher matcher = - new SSABasedGenericKubernetesResourceMatcher<>(); + private final SSABasedGenericKubernetesResourceMatcher matcher = + SSABasedGenericKubernetesResourceMatcher.getInstance(); @BeforeEach @SuppressWarnings("unchecked") void setup() { - var controllerConfiguration = mock(ControllerConfiguration.class); - when(controllerConfiguration.fieldManager()).thenReturn("controller"); - var configurationService = mock(ConfigurationService.class); - final var client = MockKubernetesClient.client(HasMetadata.class); when(mockedContext.getClient()).thenReturn(client); + final var configurationService = mock(ConfigurationService.class); + final var controllerConfiguration = mock(ControllerConfiguration.class); when(controllerConfiguration.getConfigurationService()).thenReturn(configurationService); + when(controllerConfiguration.fieldManager()).thenReturn("controller"); when(mockedContext.getControllerConfiguration()).thenReturn(controllerConfiguration); } @@ -116,9 +117,33 @@ void addedLabelInDesiredMakesMatchFail() { assertThat(matcher.matches(actualConfigMap, desiredConfigMap, mockedContext)).isFalse(); } - private R loadResource(String fileName, Class clazz) { + @ParameterizedTest + @ValueSource(strings = {"sample-sts-volumeclaimtemplates-desired.yaml", + "sample-sts-volumeclaimtemplates-desired-with-status.yaml", + "sample-sts-volumeclaimtemplates-desired-with-volumemode.yaml"}) + void testSanitizeState_statefulSetWithVolumeClaims(String desiredResourceFileName) { + var desiredStatefulSet = loadResource(desiredResourceFileName, StatefulSet.class); + var actualStatefulSet = loadResource("sample-sts-volumeclaimtemplates.yaml", + StatefulSet.class); + + assertThat(matcher.matches(actualStatefulSet, desiredStatefulSet, mockedContext)).isTrue(); + } + + @ParameterizedTest + @ValueSource(strings = {"sample-sts-volumeclaimtemplates-desired-add.yaml", + "sample-sts-volumeclaimtemplates-desired-update.yaml", + "sample-sts-volumeclaimtemplates-desired-with-status-mismatch.yaml", + "sample-sts-volumeclaimtemplates-desired-with-volumemode-mismatch.yaml"}) + void testSanitizeState_statefulSetWithVolumeClaims_withMismatch(String desiredResourceFileName) { + var desiredStatefulSet = loadResource(desiredResourceFileName, StatefulSet.class); + var actualStatefulSet = loadResource("sample-sts-volumeclaimtemplates.yaml", + StatefulSet.class); + + assertThat(matcher.matches(actualStatefulSet, desiredStatefulSet, mockedContext)).isFalse(); + } + + private static R loadResource(String fileName, Class clazz) { return ReconcilerUtils.loadYaml(clazz, SSABasedGenericKubernetesResourceMatcherTest.class, fileName); } - } diff --git a/operator-framework-core/src/test/resources/io/javaoperatorsdk/operator/processing/dependent/kubernetes/sample-sts-volumeclaimtemplates-desired-add.yaml b/operator-framework-core/src/test/resources/io/javaoperatorsdk/operator/processing/dependent/kubernetes/sample-sts-volumeclaimtemplates-desired-add.yaml new file mode 100644 index 0000000000..289baa7f11 --- /dev/null +++ b/operator-framework-core/src/test/resources/io/javaoperatorsdk/operator/processing/dependent/kubernetes/sample-sts-volumeclaimtemplates-desired-add.yaml @@ -0,0 +1,43 @@ +# desired StatefulSet with a VolumeClaimTemplate with an additional VolumeClaimTemplate +apiVersion: apps/v1 +kind: StatefulSet +metadata: + name: "test" +spec: + replicas: 1 + selector: + matchLabels: + app: test-app + serviceName: "nginx-service" + template: + metadata: + labels: + app: test-app + spec: + containers: + - name: nginx + image: nginx:1.17.0 + ports: + - containerPort: 80 + volumeMounts: + - name: persistent-storage + mountPath: /usr/share/nginx/html + volumeClaimTemplates: + - metadata: + name: persistent-storage + spec: + accessModes: + - ReadWriteOnce + resources: + requests: + storage: 1Gi + storageClassName: standard + - metadata: + name: persistent-storage-new + spec: + accessModes: + - ReadWriteOnce + resources: + requests: + storage: 10Gi + storageClassName: standard diff --git a/operator-framework-core/src/test/resources/io/javaoperatorsdk/operator/processing/dependent/kubernetes/sample-sts-volumeclaimtemplates-desired-update.yaml b/operator-framework-core/src/test/resources/io/javaoperatorsdk/operator/processing/dependent/kubernetes/sample-sts-volumeclaimtemplates-desired-update.yaml new file mode 100644 index 0000000000..c46d522c2a --- /dev/null +++ b/operator-framework-core/src/test/resources/io/javaoperatorsdk/operator/processing/dependent/kubernetes/sample-sts-volumeclaimtemplates-desired-update.yaml @@ -0,0 +1,34 @@ +# desired StatefulSet with a VolumeClaimTemplate with an updated VolumeClaimTemplate +apiVersion: apps/v1 +kind: StatefulSet +metadata: + name: "test" +spec: + replicas: 1 + selector: + matchLabels: + app: test-app + serviceName: "nginx-service" + template: + metadata: + labels: + app: test-app + spec: + containers: + - name: nginx + image: nginx:1.17.0 + ports: + - containerPort: 80 + volumeMounts: + - name: persistent-storage + mountPath: /usr/share/nginx/html + volumeClaimTemplates: + - metadata: + name: persistent-storage + spec: + accessModes: + - ReadWriteOnce + resources: + requests: + storage: 2Gi + storageClassName: standard diff --git a/operator-framework-core/src/test/resources/io/javaoperatorsdk/operator/processing/dependent/kubernetes/sample-sts-volumeclaimtemplates-desired-with-status-mismatch.yaml b/operator-framework-core/src/test/resources/io/javaoperatorsdk/operator/processing/dependent/kubernetes/sample-sts-volumeclaimtemplates-desired-with-status-mismatch.yaml new file mode 100644 index 0000000000..df7f05790b --- /dev/null +++ b/operator-framework-core/src/test/resources/io/javaoperatorsdk/operator/processing/dependent/kubernetes/sample-sts-volumeclaimtemplates-desired-with-status-mismatch.yaml @@ -0,0 +1,36 @@ +# desired StatefulSet with a VolumeClaimTemplate with a mismatching spec.volumeClaimTemplates.spec.status +apiVersion: apps/v1 +kind: StatefulSet +metadata: + name: "test" +spec: + replicas: 1 + selector: + matchLabels: + app: test-app + serviceName: "nginx-service" + template: + metadata: + labels: + app: test-app + spec: + containers: + - name: nginx + image: nginx:1.17.0 + ports: + - containerPort: 80 + volumeMounts: + - name: persistent-storage + mountPath: /usr/share/nginx/html + volumeClaimTemplates: + - metadata: + name: persistent-storage + spec: + accessModes: + - ReadWriteOnce + resources: + requests: + storage: 1Gi + storageClassName: standard + status: + phase: Bound diff --git a/operator-framework-core/src/test/resources/io/javaoperatorsdk/operator/processing/dependent/kubernetes/sample-sts-volumeclaimtemplates-desired-with-status.yaml b/operator-framework-core/src/test/resources/io/javaoperatorsdk/operator/processing/dependent/kubernetes/sample-sts-volumeclaimtemplates-desired-with-status.yaml new file mode 100644 index 0000000000..79d9eebdb2 --- /dev/null +++ b/operator-framework-core/src/test/resources/io/javaoperatorsdk/operator/processing/dependent/kubernetes/sample-sts-volumeclaimtemplates-desired-with-status.yaml @@ -0,0 +1,36 @@ +# desired StatefulSet with a VolumeClaimTemplate with a matching spec.volumeClaimTemplates.spec.status +apiVersion: apps/v1 +kind: StatefulSet +metadata: + name: "test" +spec: + replicas: 1 + selector: + matchLabels: + app: test-app + serviceName: "nginx-service" + template: + metadata: + labels: + app: test-app + spec: + containers: + - name: nginx + image: nginx:1.17.0 + ports: + - containerPort: 80 + volumeMounts: + - name: persistent-storage + mountPath: /usr/share/nginx/html + volumeClaimTemplates: + - metadata: + name: persistent-storage + spec: + accessModes: + - ReadWriteOnce + resources: + requests: + storage: 1Gi + storageClassName: standard + status: + phase: Pending diff --git a/operator-framework-core/src/test/resources/io/javaoperatorsdk/operator/processing/dependent/kubernetes/sample-sts-volumeclaimtemplates-desired-with-volumemode-mismatch.yaml b/operator-framework-core/src/test/resources/io/javaoperatorsdk/operator/processing/dependent/kubernetes/sample-sts-volumeclaimtemplates-desired-with-volumemode-mismatch.yaml new file mode 100644 index 0000000000..9b38361951 --- /dev/null +++ b/operator-framework-core/src/test/resources/io/javaoperatorsdk/operator/processing/dependent/kubernetes/sample-sts-volumeclaimtemplates-desired-with-volumemode-mismatch.yaml @@ -0,0 +1,35 @@ +# desired StatefulSet with a VolumeClaimTemplate with a mismatching spec.volumeClaimTemplates.spec.volumeMode +apiVersion: apps/v1 +kind: StatefulSet +metadata: + name: "test" +spec: + replicas: 1 + selector: + matchLabels: + app: test-app + serviceName: "nginx-service" + template: + metadata: + labels: + app: test-app + spec: + containers: + - name: nginx + image: nginx:1.17.0 + ports: + - containerPort: 80 + volumeMounts: + - name: persistent-storage + mountPath: /usr/share/nginx/html + volumeClaimTemplates: + - metadata: + name: persistent-storage + spec: + accessModes: + - ReadWriteOnce + resources: + requests: + storage: 1Gi + storageClassName: standard + volumeMode: Block diff --git a/operator-framework-core/src/test/resources/io/javaoperatorsdk/operator/processing/dependent/kubernetes/sample-sts-volumeclaimtemplates-desired-with-volumemode.yaml b/operator-framework-core/src/test/resources/io/javaoperatorsdk/operator/processing/dependent/kubernetes/sample-sts-volumeclaimtemplates-desired-with-volumemode.yaml new file mode 100644 index 0000000000..03fa30eb8a --- /dev/null +++ b/operator-framework-core/src/test/resources/io/javaoperatorsdk/operator/processing/dependent/kubernetes/sample-sts-volumeclaimtemplates-desired-with-volumemode.yaml @@ -0,0 +1,35 @@ +# desired StatefulSet with a VolumeClaimTemplate with a matching spec.volumeClaimTemplates.spec.volumeMode +apiVersion: apps/v1 +kind: StatefulSet +metadata: + name: "test" +spec: + replicas: 1 + selector: + matchLabels: + app: test-app + serviceName: "nginx-service" + template: + metadata: + labels: + app: test-app + spec: + containers: + - name: nginx + image: nginx:1.17.0 + ports: + - containerPort: 80 + volumeMounts: + - name: persistent-storage + mountPath: /usr/share/nginx/html + volumeClaimTemplates: + - metadata: + name: persistent-storage + spec: + accessModes: + - ReadWriteOnce + resources: + requests: + storage: 1Gi + storageClassName: standard + volumeMode: Filesystem diff --git a/operator-framework-core/src/test/resources/io/javaoperatorsdk/operator/processing/dependent/kubernetes/sample-sts-volumeclaimtemplates-desired.yaml b/operator-framework-core/src/test/resources/io/javaoperatorsdk/operator/processing/dependent/kubernetes/sample-sts-volumeclaimtemplates-desired.yaml new file mode 100644 index 0000000000..c44ef17062 --- /dev/null +++ b/operator-framework-core/src/test/resources/io/javaoperatorsdk/operator/processing/dependent/kubernetes/sample-sts-volumeclaimtemplates-desired.yaml @@ -0,0 +1,34 @@ +# desired StatefulSet with a VolumeClaimTemplate +apiVersion: apps/v1 +kind: StatefulSet +metadata: + name: "test" +spec: + replicas: 1 + selector: + matchLabels: + app: test-app + serviceName: "nginx-service" + template: + metadata: + labels: + app: test-app + spec: + containers: + - name: nginx + image: nginx:1.17.0 + ports: + - containerPort: 80 + volumeMounts: + - name: persistent-storage + mountPath: /usr/share/nginx/html + volumeClaimTemplates: + - metadata: + name: persistent-storage + spec: + accessModes: + - ReadWriteOnce + resources: + requests: + storage: 1Gi + storageClassName: standard diff --git a/operator-framework-core/src/test/resources/io/javaoperatorsdk/operator/processing/dependent/kubernetes/sample-sts-volumeclaimtemplates.yaml b/operator-framework-core/src/test/resources/io/javaoperatorsdk/operator/processing/dependent/kubernetes/sample-sts-volumeclaimtemplates.yaml new file mode 100644 index 0000000000..4d8cf6789d --- /dev/null +++ b/operator-framework-core/src/test/resources/io/javaoperatorsdk/operator/processing/dependent/kubernetes/sample-sts-volumeclaimtemplates.yaml @@ -0,0 +1,69 @@ +# actual StatefulSet with a VolumeClaimTemplate +apiVersion: apps/v1 +kind: StatefulSet +metadata: + managedFields: + - manager: controller + operation: Apply + apiVersion: apps/v1 + time: '2024-10-24T19:15:25Z' + fieldsType: FieldsV1 + fieldsV1: + f:spec: + f:replicas: { } + f:selector: { } + f:serviceName: { } + f:template: + f:metadata: + f:labels: + f:app: { } + f:spec: + f:containers: + k:{"name":"nginx"}: + .: { } + f:image: { } + f:name: { } + f:ports: + k:{"containerPort":80}: + .: { } + f:containerPort: { } + f:volumeMounts: + k:{"mountPath":"/usr/share/nginx/html"}: + .: { } + f:mountPath: { } + f:name: { } + f:volumeClaimTemplates: { } + name: "test" + uid: 50913e35-e855-469f-bec6-3e8cd2607ab4 +spec: + replicas: 1 + selector: + matchLabels: + app: test-app + serviceName: "nginx-service" + template: + metadata: + labels: + app: test-app + spec: + containers: + - name: nginx + image: nginx:1.17.0 + ports: + - containerPort: 80 + volumeMounts: + - name: persistent-storage + mountPath: /usr/share/nginx/html + volumeClaimTemplates: + - metadata: + name: persistent-storage + spec: + accessModes: + - ReadWriteOnce + resources: + requests: + storage: 1Gi + storageClassName: standard + volumeMode: Filesystem + status: + phase: Pending From 262927d33ba7d22a3a5d083f55d9eed078f3497f Mon Sep 17 00:00:00 2001 From: David Sondermann Date: Sun, 27 Oct 2024 09:31:34 +0000 Subject: [PATCH 3/5] fix: Fix infinite resource updates due to canonical format conversion of resource requirements Signed-off-by: David Sondermann --- .../ResourceRequirementsSanitizer.java | 100 +++++++++ ...BasedGenericKubernetesResourceMatcher.java | 18 ++ .../ResourceRequirementsSanitizerTest.java | 198 ++++++++++++++++++ ...dGenericKubernetesResourceMatcherTest.java | 59 ++++++ .../sample-ds-resources-desired-update.yaml | 28 +++ .../sample-ds-resources-desired.yaml | 28 +++ .../kubernetes/sample-ds-resources.yaml | 53 +++++ .../sample-rs-resources-desired-update.yaml | 29 +++ .../sample-rs-resources-desired.yaml | 29 +++ .../kubernetes/sample-rs-resources.yaml | 55 +++++ .../sample-sts-resources-desired-update.yaml | 30 +++ .../sample-sts-resources-desired.yaml | 30 +++ .../kubernetes/sample-sts-resources.yaml | 57 +++++ 13 files changed, 714 insertions(+) create mode 100644 operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/ResourceRequirementsSanitizer.java create mode 100644 operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/ResourceRequirementsSanitizerTest.java create mode 100644 operator-framework-core/src/test/resources/io/javaoperatorsdk/operator/processing/dependent/kubernetes/sample-ds-resources-desired-update.yaml create mode 100644 operator-framework-core/src/test/resources/io/javaoperatorsdk/operator/processing/dependent/kubernetes/sample-ds-resources-desired.yaml create mode 100644 operator-framework-core/src/test/resources/io/javaoperatorsdk/operator/processing/dependent/kubernetes/sample-ds-resources.yaml create mode 100644 operator-framework-core/src/test/resources/io/javaoperatorsdk/operator/processing/dependent/kubernetes/sample-rs-resources-desired-update.yaml create mode 100644 operator-framework-core/src/test/resources/io/javaoperatorsdk/operator/processing/dependent/kubernetes/sample-rs-resources-desired.yaml create mode 100644 operator-framework-core/src/test/resources/io/javaoperatorsdk/operator/processing/dependent/kubernetes/sample-rs-resources.yaml create mode 100644 operator-framework-core/src/test/resources/io/javaoperatorsdk/operator/processing/dependent/kubernetes/sample-sts-resources-desired-update.yaml create mode 100644 operator-framework-core/src/test/resources/io/javaoperatorsdk/operator/processing/dependent/kubernetes/sample-sts-resources-desired.yaml create mode 100644 operator-framework-core/src/test/resources/io/javaoperatorsdk/operator/processing/dependent/kubernetes/sample-sts-resources.yaml diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/ResourceRequirementsSanitizer.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/ResourceRequirementsSanitizer.java new file mode 100644 index 0000000000..7e877ed35f --- /dev/null +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/ResourceRequirementsSanitizer.java @@ -0,0 +1,100 @@ +package io.javaoperatorsdk.operator.processing.dependent.kubernetes; + +import io.fabric8.kubernetes.api.model.Container; +import io.fabric8.kubernetes.api.model.GenericKubernetesResource; +import io.fabric8.kubernetes.api.model.PodTemplateSpec; +import io.fabric8.kubernetes.api.model.Quantity; +import io.fabric8.kubernetes.api.model.ResourceRequirements; + +import java.util.List; +import java.util.Map; +import java.util.Optional; + +/** + * Sanitizes the {@link ResourceRequirements} in the containers of a pair of {@link PodTemplateSpec} + * instances. + *

+ * When the sanitizer finds a mismatch in the structure of the given templates, before it gets to + * the nested resource limits and requests, it returns early without fixing the actual map. This is + * an optimization because the given templates will anyway differ at this point. This means we do + * not have to attempt to sanitize the resources for these use cases, since there will anyway be an + * update of the K8s resource. + *

+ * The algorithm traverses the whole template structure because we need the actual and desired + * {@link Quantity} instances to compare their numerical amount. Using the + * {@link GenericKubernetesResource#get(Map, Object...)} shortcut would need to create new instances + * just for the sanitization check. + */ +class ResourceRequirementsSanitizer { + + static void sanitizeResourceRequirements(final Map actualMap, + final PodTemplateSpec actualTemplate, final PodTemplateSpec desiredTemplate) { + if (actualTemplate == null || desiredTemplate == null) { + return; + } + if (actualTemplate.getSpec() == null || desiredTemplate.getSpec() == null) { + return; + } + sanitizeResourceRequirements(actualMap, actualTemplate.getSpec().getInitContainers(), + desiredTemplate.getSpec().getInitContainers(), "initContainers"); + sanitizeResourceRequirements(actualMap, actualTemplate.getSpec().getContainers(), + desiredTemplate.getSpec().getContainers(), "containers"); + } + + private static void sanitizeResourceRequirements(final Map actualMap, + final List actualContainers, final List desiredContainers, + final String containerPath) { + int containers = desiredContainers.size(); + if (containers == actualContainers.size()) { + for (int containerIndex = 0; containerIndex < containers; containerIndex++) { + var desiredContainer = desiredContainers.get(containerIndex); + var actualContainer = actualContainers.get(containerIndex); + if (!desiredContainer.getName().equals(actualContainer.getName())) { + return; + } + sanitizeResourceRequirements(actualMap, actualContainer.getResources(), + desiredContainer.getResources(), + containerPath, containerIndex); + } + } + } + + private static void sanitizeResourceRequirements(final Map actualMap, + final ResourceRequirements actualResource, final ResourceRequirements desiredResource, + final String containerPath, final int containerIndex) { + if (desiredResource == null || actualResource == null) { + return; + } + sanitizeQuantities(actualMap, actualResource.getRequests(), desiredResource.getRequests(), + containerPath, containerIndex, "requests"); + sanitizeQuantities(actualMap, actualResource.getLimits(), desiredResource.getLimits(), + containerPath, containerIndex, "limits"); + } + + @SuppressWarnings("unchecked") + private static void sanitizeQuantities(final Map actualMap, + final Map actualResource, final Map desiredResource, + final String containerPath, final int containerIndex, final String quantityPath) { + Optional.ofNullable( + GenericKubernetesResource.get(actualMap, "spec", "template", "spec", containerPath, + containerIndex, "resources", quantityPath)) + .map(Map.class::cast) + .filter(m -> m.size() == desiredResource.size()) + .ifPresent(m -> actualResource.forEach((key, actualQuantity) -> { + var desiredQuantity = desiredResource.get(key); + if (desiredQuantity == null) { + return; + } + // check if the string representation of the Quantity instances is equal + if (actualQuantity.getAmount().equals(desiredQuantity.getAmount()) + && actualQuantity.getFormat().equals(desiredQuantity.getFormat())) { + return; + } + // check if the numerical amount of the Quantity instances is equal + if (actualQuantity.equals(desiredQuantity)) { + // replace the actual Quantity with the desired Quantity to prevent a resource update + m.replace(key, desiredQuantity.toString()); + } + })); + } +} 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 8188fe8ea3..87fc2ad92b 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 @@ -3,6 +3,9 @@ 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.DaemonSet; +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.utils.KubernetesSerialization; import io.javaoperatorsdk.operator.OperatorException; @@ -24,6 +27,8 @@ import java.util.TreeMap; import java.util.stream.Collectors; +import static io.javaoperatorsdk.operator.processing.dependent.kubernetes.ResourceRequirementsSanitizer.sanitizeResourceRequirements; + /** * Matches the actual state on the server vs the desired state. Based on the managedFields of SSA. *

@@ -128,6 +133,19 @@ private void sanitizeState(R actual, R desired, Map actualMap) { } } } + sanitizeResourceRequirements(actualMap, actualSpec.getTemplate(), desiredSpec.getTemplate()); + } else if (actual instanceof Deployment) { + sanitizeResourceRequirements(actualMap, + ((Deployment) actual).getSpec().getTemplate(), + ((Deployment) desired).getSpec().getTemplate()); + } else if (actual instanceof ReplicaSet) { + sanitizeResourceRequirements(actualMap, + ((ReplicaSet) actual).getSpec().getTemplate(), + ((ReplicaSet) desired).getSpec().getTemplate()); + } else if (actual instanceof DaemonSet) { + sanitizeResourceRequirements(actualMap, + ((DaemonSet) actual).getSpec().getTemplate(), + ((DaemonSet) desired).getSpec().getTemplate()); } } diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/ResourceRequirementsSanitizerTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/ResourceRequirementsSanitizerTest.java new file mode 100644 index 0000000000..71d36a4d84 --- /dev/null +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/ResourceRequirementsSanitizerTest.java @@ -0,0 +1,198 @@ +package io.javaoperatorsdk.operator.processing.dependent.kubernetes; + +import io.fabric8.kubernetes.api.model.GenericKubernetesResource; +import io.fabric8.kubernetes.api.model.HasMetadata; +import io.fabric8.kubernetes.api.model.PodTemplateSpecBuilder; +import io.fabric8.kubernetes.api.model.Quantity; +import io.fabric8.kubernetes.api.model.apps.StatefulSetBuilder; +import io.fabric8.kubernetes.client.KubernetesClient; +import io.fabric8.kubernetes.client.utils.KubernetesSerialization; +import io.javaoperatorsdk.operator.MockKubernetesClient; +import org.assertj.core.api.MapAssert; +import org.junit.jupiter.api.Test; + +import java.util.Map; + +import static io.javaoperatorsdk.operator.processing.dependent.kubernetes.ResourceRequirementsSanitizer.sanitizeResourceRequirements; +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verifyNoInteractions; + +class ResourceRequirementsSanitizerTest { + + private final Map actualMap = mock(); + + private final KubernetesClient client = MockKubernetesClient.client(HasMetadata.class); + private final KubernetesSerialization serialization = client.getKubernetesSerialization(); + + @Test + void testSanitizeResourceRequirements_whenTemplateIsNull_doNothing() { + final var template = new PodTemplateSpecBuilder().build(); + + sanitizeResourceRequirements(actualMap, null, template); + sanitizeResourceRequirements(actualMap, template, null); + verifyNoInteractions(actualMap); + } + + @Test + void testSanitizeResourceRequirements_whenTemplateSpecIsNull_doNothing() { + final var template = new PodTemplateSpecBuilder().withSpec(null).build(); + final var templateWithSpec = new PodTemplateSpecBuilder().withNewSpec().endSpec().build(); + + sanitizeResourceRequirements(actualMap, template, templateWithSpec); + sanitizeResourceRequirements(actualMap, templateWithSpec, template); + verifyNoInteractions(actualMap); + } + + @Test + void testSanitizeResourceRequirements_whenContainerSizeMismatch_doNothing() { + final var template = new PodTemplateSpecBuilder().withNewSpec() + .addNewContainer().withName("test").endContainer() + .endSpec().build(); + final var templateWithTwoContainers = new PodTemplateSpecBuilder().withNewSpec() + .addNewContainer().withName("test").endContainer() + .addNewContainer().withName("test-new").endContainer() + .endSpec().build(); + + sanitizeResourceRequirements(actualMap, template, templateWithTwoContainers); + sanitizeResourceRequirements(actualMap, templateWithTwoContainers, template); + verifyNoInteractions(actualMap); + } + + @Test + void testSanitizeResourceRequirements_whenContainerNameMismatch_doNothing() { + final var template = new PodTemplateSpecBuilder().withNewSpec() + .addNewContainer().withName("test").endContainer() + .endSpec().build(); + final var templateWithNewContainerName = new PodTemplateSpecBuilder().withNewSpec() + .addNewContainer().withName("test-new").endContainer() + .endSpec().build(); + + sanitizeResourceRequirements(actualMap, template, templateWithNewContainerName); + sanitizeResourceRequirements(actualMap, templateWithNewContainerName, template); + verifyNoInteractions(actualMap); + } + + @Test + void testSanitizeResourceRequirements_whenResourceIsNull_doNothing() { + final var template = new PodTemplateSpecBuilder().withNewSpec() + .addNewContainer().withName("test").endContainer() + .endSpec().build(); + final var templateWithResource = new PodTemplateSpecBuilder().withNewSpec() + .addNewContainer().withName("test").withNewResources().endResources().endContainer() + .endSpec().build(); + + sanitizeResourceRequirements(actualMap, template, templateWithResource); + sanitizeResourceRequirements(actualMap, templateWithResource, template); + verifyNoInteractions(actualMap); + } + + @Test + void testSanitizeResourceRequirements_whenResourceSizeMismatch_doNothing() { + final var actualMap = sanitizeRequestsAndLimits( + Map.of("cpu", new Quantity("2")), + Map.of(), + Map.of("cpu", new Quantity("4")), + Map.of("cpu", new Quantity("4"), "memory", new Quantity("4Gi"))); + assertResources(actualMap, "requests") + .hasSize(1) + .containsEntry("cpu", "2"); + assertResources(actualMap, "limits") + .hasSize(1) + .containsEntry("cpu", "4"); + } + + @Test + void testSanitizeResourceRequirements_whenResourceKeyMismatch_doNothing() { + final var actualMap = sanitizeRequestsAndLimits( + Map.of("cpu", new Quantity("2")), + Map.of("memory", new Quantity("4Gi")), + Map.of(), + Map.of()); + assertResources(actualMap, "requests") + .hasSize(1) + .containsEntry("cpu", "2"); + assertResources(actualMap, "limits").isNull(); + } + + @Test + void testSanitizeResourceRequirements_whenResourcesHaveSameAmountAndFormat_doNothing() { + final var actualMap = sanitizeRequestsAndLimits( + Map.of("memory", new Quantity("4Gi")), + Map.of("memory", new Quantity("4Gi")), + Map.of("cpu", new Quantity("2")), + Map.of("cpu", new Quantity("2"))); + assertResources(actualMap, "requests") + .hasSize(1) + .containsEntry("memory", "4Gi"); + assertResources(actualMap, "limits") + .hasSize(1) + .containsEntry("cpu", "2"); + } + + @Test + void testSanitizeResourceRequirements_whenResourcesHaveNumericalAmountMismatch_doNothing() { + final var actualMap = sanitizeRequestsAndLimits( + Map.of("cpu", new Quantity("2"), "memory", new Quantity("4Gi")), + Map.of("cpu", new Quantity("4"), "memory", new Quantity("4Ti")), + Map.of("cpu", new Quantity("2")), + Map.of("cpu", new Quantity("4000m"))); + assertResources(actualMap, "requests") + .hasSize(2) + .containsEntry("cpu", "2") + .containsEntry("memory", "4Gi"); + assertResources(actualMap, "limits") + .hasSize(1) + .containsEntry("cpu", "2"); + } + + @Test + void testSanitizeResourceRequirements_whenResourcesHaveAmountAndFormatMismatchWithSameNumericalAmount_thenSanitizeActualMap() { + final var actualMap = sanitizeRequestsAndLimits( + Map.of("cpu", new Quantity("2"), "memory", new Quantity("4Gi")), + Map.of("cpu", new Quantity("2000m"), "memory", new Quantity("4096Mi")), + Map.of("cpu", new Quantity("4")), + Map.of("cpu", new Quantity("4000m"))); + assertResources(actualMap, "requests") + .hasSize(2) + .containsEntry("cpu", "2000m") + .containsEntry("memory", "4096Mi"); + assertResources(actualMap, "limits") + .hasSize(1) + .containsEntry("cpu", "4000m"); + } + + @SuppressWarnings("unchecked") + private Map sanitizeRequestsAndLimits( + final Map actualRequests, final Map desiredRequests, + final Map actualLimits, final Map desiredLimits) { + final var actual = new StatefulSetBuilder().withNewSpec().withNewTemplate().withNewSpec() + .addNewContainer() + .withName("test") + .withNewResources() + .withRequests(actualRequests).withLimits(actualLimits) + .endResources() + .endContainer() + .endSpec().endTemplate().endSpec().build(); + final var desired = new StatefulSetBuilder().withNewSpec().withNewTemplate().withNewSpec() + .addNewContainer() + .withName("test") + .withNewResources() + .withRequests(desiredRequests).withLimits(desiredLimits) + .endResources() + .endContainer() + .endSpec().endTemplate().endSpec().build(); + + final var actualMap = serialization.convertValue(actual, Map.class); + sanitizeResourceRequirements(actualMap, + actual.getSpec().getTemplate(), + desired.getSpec().getTemplate()); + return actualMap; + } + + private static MapAssert assertResources(final Map actualMap, + final String resourceName) { + return assertThat(GenericKubernetesResource.>get(actualMap, + "spec", "template", "spec", "containers", 0, "resources", resourceName)); + } +} 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 22bf40c7a0..c2555bc7e9 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 @@ -2,7 +2,9 @@ import io.fabric8.kubernetes.api.model.ConfigMap; import io.fabric8.kubernetes.api.model.HasMetadata; +import io.fabric8.kubernetes.api.model.apps.DaemonSet; 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.javaoperatorsdk.operator.MockKubernetesClient; import io.javaoperatorsdk.operator.ReconcilerUtils; @@ -142,6 +144,63 @@ void testSanitizeState_statefulSetWithVolumeClaims_withMismatch(String desiredRe assertThat(matcher.matches(actualStatefulSet, desiredStatefulSet, mockedContext)).isFalse(); } + @Test + void testSanitizeState_statefulSetWithResources() { + var desiredStatefulSet = loadResource("sample-sts-resources-desired.yaml", StatefulSet.class); + var actualStatefulSet = loadResource("sample-sts-resources.yaml", + StatefulSet.class); + + assertThat(matcher.matches(actualStatefulSet, desiredStatefulSet, mockedContext)).isTrue(); + } + + @Test + void testSanitizeState_statefulSetWithResources_withMismatch() { + var desiredStatefulSet = + loadResource("sample-sts-resources-desired-update.yaml", StatefulSet.class); + var actualStatefulSet = loadResource("sample-sts-resources.yaml", + StatefulSet.class); + + assertThat(matcher.matches(actualStatefulSet, desiredStatefulSet, mockedContext)).isFalse(); + } + + @Test + void testSanitizeState_replicaSetWithResources() { + var desiredReplicaSet = loadResource("sample-rs-resources-desired.yaml", ReplicaSet.class); + var actualReplicaSet = loadResource("sample-rs-resources.yaml", + ReplicaSet.class); + + assertThat(matcher.matches(actualReplicaSet, desiredReplicaSet, mockedContext)).isTrue(); + } + + @Test + void testSanitizeState_replicaSetWithResources_withMismatch() { + var desiredReplicaSet = + loadResource("sample-rs-resources-desired-update.yaml", ReplicaSet.class); + var actualReplicaSet = loadResource("sample-rs-resources.yaml", + ReplicaSet.class); + + assertThat(matcher.matches(actualReplicaSet, desiredReplicaSet, mockedContext)).isFalse(); + } + + @Test + void testSanitizeState_daemonSetWithResources() { + var desiredDaemonSet = loadResource("sample-ds-resources-desired.yaml", DaemonSet.class); + var actualDaemonSet = loadResource("sample-ds-resources.yaml", + DaemonSet.class); + + assertThat(matcher.matches(actualDaemonSet, desiredDaemonSet, mockedContext)).isTrue(); + } + + @Test + void testSanitizeState_daemonSetWithResources_withMismatch() { + var desiredDaemonSet = + loadResource("sample-ds-resources-desired-update.yaml", DaemonSet.class); + var actualDaemonSet = loadResource("sample-ds-resources.yaml", + DaemonSet.class); + + assertThat(matcher.matches(actualDaemonSet, desiredDaemonSet, mockedContext)).isFalse(); + } + private static R loadResource(String fileName, Class clazz) { return ReconcilerUtils.loadYaml(clazz, SSABasedGenericKubernetesResourceMatcherTest.class, fileName); diff --git a/operator-framework-core/src/test/resources/io/javaoperatorsdk/operator/processing/dependent/kubernetes/sample-ds-resources-desired-update.yaml b/operator-framework-core/src/test/resources/io/javaoperatorsdk/operator/processing/dependent/kubernetes/sample-ds-resources-desired-update.yaml new file mode 100644 index 0000000000..b8e330a19e --- /dev/null +++ b/operator-framework-core/src/test/resources/io/javaoperatorsdk/operator/processing/dependent/kubernetes/sample-ds-resources-desired-update.yaml @@ -0,0 +1,28 @@ +# desired DaemonSet with Resources with an updated resource limit +apiVersion: apps/v1 +kind: DaemonSet +metadata: + name: "test" +spec: + selector: + matchLabels: + app: test-app + template: + metadata: + labels: + app: test-app + spec: + containers: + - name: nginx + image: nginx:1.17.0 + ports: + - containerPort: 80 + resources: + limits: + cpu: "4000m" + memory: "2Gi" + ephemeral-storage: "100G" + requests: + cpu: "1000m" + memory: "2Gi" + ephemeral-storage: "100G" diff --git a/operator-framework-core/src/test/resources/io/javaoperatorsdk/operator/processing/dependent/kubernetes/sample-ds-resources-desired.yaml b/operator-framework-core/src/test/resources/io/javaoperatorsdk/operator/processing/dependent/kubernetes/sample-ds-resources-desired.yaml new file mode 100644 index 0000000000..9cfa95d06e --- /dev/null +++ b/operator-framework-core/src/test/resources/io/javaoperatorsdk/operator/processing/dependent/kubernetes/sample-ds-resources-desired.yaml @@ -0,0 +1,28 @@ +# desired DaemonSet with Resources +apiVersion: apps/v1 +kind: DaemonSet +metadata: + name: "test" +spec: + selector: + matchLabels: + app: test-app + template: + metadata: + labels: + app: test-app + spec: + containers: + - name: nginx + image: nginx:1.17.0 + ports: + - containerPort: 80 + resources: + limits: + cpu: "2000m" + memory: "2Gi" + ephemeral-storage: "100G" + requests: + cpu: "1000m" + memory: "2Gi" + ephemeral-storage: "100G" diff --git a/operator-framework-core/src/test/resources/io/javaoperatorsdk/operator/processing/dependent/kubernetes/sample-ds-resources.yaml b/operator-framework-core/src/test/resources/io/javaoperatorsdk/operator/processing/dependent/kubernetes/sample-ds-resources.yaml new file mode 100644 index 0000000000..f22730efd5 --- /dev/null +++ b/operator-framework-core/src/test/resources/io/javaoperatorsdk/operator/processing/dependent/kubernetes/sample-ds-resources.yaml @@ -0,0 +1,53 @@ +# actual DaemonSet with Resources +apiVersion: apps/v1 +kind: DaemonSet +metadata: + managedFields: + - manager: controller + operation: Apply + apiVersion: apps/v1 + time: '2024-10-24T19:15:25Z' + fieldsType: FieldsV1 + fieldsV1: + f:spec: + f:selector: { } + f:template: + f:metadata: + f:labels: + f:app: { } + f:spec: + f:containers: + k:{"name":"nginx"}: + .: { } + f:image: { } + f:name: { } + f:ports: + k:{"containerPort":80}: + .: { } + f:containerPort: { } + f:resources: { } + name: "test" + uid: 50913e35-e855-469f-bec6-3e8cd2607ab4 +spec: + selector: + matchLabels: + app: test-app + template: + metadata: + labels: + app: test-app + spec: + containers: + - name: nginx + image: nginx:1.17.0 + ports: + - containerPort: 80 + resources: + limits: + cpu: "2" + memory: "2Gi" + ephemeral-storage: "100G" + requests: + cpu: "1" + memory: "2Gi" + ephemeral-storage: "100G" diff --git a/operator-framework-core/src/test/resources/io/javaoperatorsdk/operator/processing/dependent/kubernetes/sample-rs-resources-desired-update.yaml b/operator-framework-core/src/test/resources/io/javaoperatorsdk/operator/processing/dependent/kubernetes/sample-rs-resources-desired-update.yaml new file mode 100644 index 0000000000..6a4236c1ee --- /dev/null +++ b/operator-framework-core/src/test/resources/io/javaoperatorsdk/operator/processing/dependent/kubernetes/sample-rs-resources-desired-update.yaml @@ -0,0 +1,29 @@ +# desired ReplicaSet with Resources with an updated resource limit +apiVersion: apps/v1 +kind: ReplicaSet +metadata: + name: "test" +spec: + replicas: 1 + selector: + matchLabels: + app: test-app + template: + metadata: + labels: + app: test-app + spec: + containers: + - name: nginx + image: nginx:1.17.0 + ports: + - containerPort: 80 + resources: + limits: + cpu: "4000m" + memory: "2Gi" + ephemeral-storage: "100G" + requests: + cpu: "1000m" + memory: "2Gi" + ephemeral-storage: "100G" diff --git a/operator-framework-core/src/test/resources/io/javaoperatorsdk/operator/processing/dependent/kubernetes/sample-rs-resources-desired.yaml b/operator-framework-core/src/test/resources/io/javaoperatorsdk/operator/processing/dependent/kubernetes/sample-rs-resources-desired.yaml new file mode 100644 index 0000000000..95dcefecc5 --- /dev/null +++ b/operator-framework-core/src/test/resources/io/javaoperatorsdk/operator/processing/dependent/kubernetes/sample-rs-resources-desired.yaml @@ -0,0 +1,29 @@ +# desired ReplicaSet with Resources +apiVersion: apps/v1 +kind: ReplicaSet +metadata: + name: "test" +spec: + replicas: 1 + selector: + matchLabels: + app: test-app + template: + metadata: + labels: + app: test-app + spec: + containers: + - name: nginx + image: nginx:1.17.0 + ports: + - containerPort: 80 + resources: + limits: + cpu: "2000m" + memory: "2Gi" + ephemeral-storage: "100G" + requests: + cpu: "1000m" + memory: "2Gi" + ephemeral-storage: "100G" diff --git a/operator-framework-core/src/test/resources/io/javaoperatorsdk/operator/processing/dependent/kubernetes/sample-rs-resources.yaml b/operator-framework-core/src/test/resources/io/javaoperatorsdk/operator/processing/dependent/kubernetes/sample-rs-resources.yaml new file mode 100644 index 0000000000..59a66b91f4 --- /dev/null +++ b/operator-framework-core/src/test/resources/io/javaoperatorsdk/operator/processing/dependent/kubernetes/sample-rs-resources.yaml @@ -0,0 +1,55 @@ +# actual ReplicaSet with Resources +apiVersion: apps/v1 +kind: ReplicaSet +metadata: + managedFields: + - manager: controller + operation: Apply + apiVersion: apps/v1 + time: '2024-10-24T19:15:25Z' + fieldsType: FieldsV1 + fieldsV1: + f:spec: + f:replicas: { } + f:selector: { } + f:template: + f:metadata: + f:labels: + f:app: { } + f:spec: + f:containers: + k:{"name":"nginx"}: + .: { } + f:image: { } + f:name: { } + f:ports: + k:{"containerPort":80}: + .: { } + f:containerPort: { } + f:resources: { } + name: "test" + uid: 50913e35-e855-469f-bec6-3e8cd2607ab4 +spec: + replicas: 1 + selector: + matchLabels: + app: test-app + template: + metadata: + labels: + app: test-app + spec: + containers: + - name: nginx + image: nginx:1.17.0 + ports: + - containerPort: 80 + resources: + limits: + cpu: "2" + memory: "2Gi" + ephemeral-storage: "100G" + requests: + cpu: "1" + memory: "2Gi" + ephemeral-storage: "100G" diff --git a/operator-framework-core/src/test/resources/io/javaoperatorsdk/operator/processing/dependent/kubernetes/sample-sts-resources-desired-update.yaml b/operator-framework-core/src/test/resources/io/javaoperatorsdk/operator/processing/dependent/kubernetes/sample-sts-resources-desired-update.yaml new file mode 100644 index 0000000000..721d2bfe51 --- /dev/null +++ b/operator-framework-core/src/test/resources/io/javaoperatorsdk/operator/processing/dependent/kubernetes/sample-sts-resources-desired-update.yaml @@ -0,0 +1,30 @@ +# desired StatefulSet with Resources with an updated resource limit +apiVersion: apps/v1 +kind: StatefulSet +metadata: + name: "test" +spec: + replicas: 1 + selector: + matchLabels: + app: test-app + serviceName: "nginx-service" + template: + metadata: + labels: + app: test-app + spec: + containers: + - name: nginx + image: nginx:1.17.0 + ports: + - containerPort: 80 + resources: + limits: + cpu: "4000m" + memory: "2Gi" + ephemeral-storage: "100G" + requests: + cpu: "1000m" + memory: "2Gi" + ephemeral-storage: "100G" diff --git a/operator-framework-core/src/test/resources/io/javaoperatorsdk/operator/processing/dependent/kubernetes/sample-sts-resources-desired.yaml b/operator-framework-core/src/test/resources/io/javaoperatorsdk/operator/processing/dependent/kubernetes/sample-sts-resources-desired.yaml new file mode 100644 index 0000000000..a23c1b1aae --- /dev/null +++ b/operator-framework-core/src/test/resources/io/javaoperatorsdk/operator/processing/dependent/kubernetes/sample-sts-resources-desired.yaml @@ -0,0 +1,30 @@ +# desired StatefulSet with Resources +apiVersion: apps/v1 +kind: StatefulSet +metadata: + name: "test" +spec: + replicas: 1 + selector: + matchLabels: + app: test-app + serviceName: "nginx-service" + template: + metadata: + labels: + app: test-app + spec: + containers: + - name: nginx + image: nginx:1.17.0 + ports: + - containerPort: 80 + resources: + limits: + cpu: "2000m" + memory: "2Gi" + ephemeral-storage: "100G" + requests: + cpu: "1000m" + memory: "2Gi" + ephemeral-storage: "100G" diff --git a/operator-framework-core/src/test/resources/io/javaoperatorsdk/operator/processing/dependent/kubernetes/sample-sts-resources.yaml b/operator-framework-core/src/test/resources/io/javaoperatorsdk/operator/processing/dependent/kubernetes/sample-sts-resources.yaml new file mode 100644 index 0000000000..948035017a --- /dev/null +++ b/operator-framework-core/src/test/resources/io/javaoperatorsdk/operator/processing/dependent/kubernetes/sample-sts-resources.yaml @@ -0,0 +1,57 @@ +# actual StatefulSet with Resources +apiVersion: apps/v1 +kind: StatefulSet +metadata: + managedFields: + - manager: controller + operation: Apply + apiVersion: apps/v1 + time: '2024-10-24T19:15:25Z' + fieldsType: FieldsV1 + fieldsV1: + f:spec: + f:replicas: { } + f:selector: { } + f:serviceName: { } + f:template: + f:metadata: + f:labels: + f:app: { } + f:spec: + f:containers: + k:{"name":"nginx"}: + .: { } + f:image: { } + f:name: { } + f:ports: + k:{"containerPort":80}: + .: { } + f:containerPort: { } + f:resources: { } + name: "test" + uid: 50913e35-e855-469f-bec6-3e8cd2607ab4 +spec: + replicas: 1 + selector: + matchLabels: + app: test-app + serviceName: "nginx-service" + template: + metadata: + labels: + app: test-app + spec: + containers: + - name: nginx + image: nginx:1.17.0 + ports: + - containerPort: 80 + resources: + limits: + cpu: "2" + memory: "2Gi" + ephemeral-storage: "100G" + requests: + cpu: "1" + memory: "2Gi" + ephemeral-storage: "100G" From c39a5eff68facea016ce5af610ebd2264c9b71fd Mon Sep 17 00:00:00 2001 From: David Sondermann Date: Tue, 29 Oct 2024 09:16:44 +0000 Subject: [PATCH 4/5] test: Add test cases with init containers to ResourceRequirementsSanitizerTest Signed-off-by: David Sondermann --- .../ResourceRequirementsSanitizerTest.java | 94 ++++++++++++------- 1 file changed, 59 insertions(+), 35 deletions(-) diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/ResourceRequirementsSanitizerTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/ResourceRequirementsSanitizerTest.java index 71d36a4d84..7c3e3e8fe0 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/ResourceRequirementsSanitizerTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/ResourceRequirementsSanitizerTest.java @@ -4,6 +4,7 @@ import io.fabric8.kubernetes.api.model.HasMetadata; import io.fabric8.kubernetes.api.model.PodTemplateSpecBuilder; import io.fabric8.kubernetes.api.model.Quantity; +import io.fabric8.kubernetes.api.model.apps.StatefulSet; import io.fabric8.kubernetes.api.model.apps.StatefulSetBuilder; import io.fabric8.kubernetes.client.KubernetesClient; import io.fabric8.kubernetes.client.utils.KubernetesSerialization; @@ -18,6 +19,11 @@ import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verifyNoInteractions; +/** + * Tests the {@link ResourceRequirementsSanitizer} with combinations of matching and mismatching K8s + * resources, using a mix of containers and init containers, as well as resource requests and + * limits. + */ class ResourceRequirementsSanitizerTest { private final Map actualMap = mock(); @@ -89,100 +95,85 @@ void testSanitizeResourceRequirements_whenResourceIsNull_doNothing() { @Test void testSanitizeResourceRequirements_whenResourceSizeMismatch_doNothing() { - final var actualMap = sanitizeRequestsAndLimits( + final var actualMap = sanitizeRequestsAndLimits(ContainerType.CONTAINER, Map.of("cpu", new Quantity("2")), Map.of(), Map.of("cpu", new Quantity("4")), Map.of("cpu", new Quantity("4"), "memory", new Quantity("4Gi"))); - assertResources(actualMap, "requests") + assertContainerResources(actualMap, "requests") .hasSize(1) .containsEntry("cpu", "2"); - assertResources(actualMap, "limits") + assertContainerResources(actualMap, "limits") .hasSize(1) .containsEntry("cpu", "4"); } @Test void testSanitizeResourceRequirements_whenResourceKeyMismatch_doNothing() { - final var actualMap = sanitizeRequestsAndLimits( + final var actualMap = sanitizeRequestsAndLimits(ContainerType.INIT_CONTAINER, Map.of("cpu", new Quantity("2")), Map.of("memory", new Quantity("4Gi")), Map.of(), Map.of()); - assertResources(actualMap, "requests") + assertInitContainerResources(actualMap, "requests") .hasSize(1) .containsEntry("cpu", "2"); - assertResources(actualMap, "limits").isNull(); + assertInitContainerResources(actualMap, "limits").isNull(); } @Test void testSanitizeResourceRequirements_whenResourcesHaveSameAmountAndFormat_doNothing() { - final var actualMap = sanitizeRequestsAndLimits( + final var actualMap = sanitizeRequestsAndLimits(ContainerType.CONTAINER, Map.of("memory", new Quantity("4Gi")), Map.of("memory", new Quantity("4Gi")), Map.of("cpu", new Quantity("2")), Map.of("cpu", new Quantity("2"))); - assertResources(actualMap, "requests") + assertContainerResources(actualMap, "requests") .hasSize(1) .containsEntry("memory", "4Gi"); - assertResources(actualMap, "limits") + assertContainerResources(actualMap, "limits") .hasSize(1) .containsEntry("cpu", "2"); } @Test void testSanitizeResourceRequirements_whenResourcesHaveNumericalAmountMismatch_doNothing() { - final var actualMap = sanitizeRequestsAndLimits( + final var actualMap = sanitizeRequestsAndLimits(ContainerType.INIT_CONTAINER, Map.of("cpu", new Quantity("2"), "memory", new Quantity("4Gi")), Map.of("cpu", new Quantity("4"), "memory", new Quantity("4Ti")), Map.of("cpu", new Quantity("2")), Map.of("cpu", new Quantity("4000m"))); - assertResources(actualMap, "requests") + assertInitContainerResources(actualMap, "requests") .hasSize(2) .containsEntry("cpu", "2") .containsEntry("memory", "4Gi"); - assertResources(actualMap, "limits") + assertInitContainerResources(actualMap, "limits") .hasSize(1) .containsEntry("cpu", "2"); } @Test void testSanitizeResourceRequirements_whenResourcesHaveAmountAndFormatMismatchWithSameNumericalAmount_thenSanitizeActualMap() { - final var actualMap = sanitizeRequestsAndLimits( + final var actualMap = sanitizeRequestsAndLimits(ContainerType.CONTAINER, Map.of("cpu", new Quantity("2"), "memory", new Quantity("4Gi")), Map.of("cpu", new Quantity("2000m"), "memory", new Quantity("4096Mi")), Map.of("cpu", new Quantity("4")), Map.of("cpu", new Quantity("4000m"))); - assertResources(actualMap, "requests") + assertContainerResources(actualMap, "requests") .hasSize(2) .containsEntry("cpu", "2000m") .containsEntry("memory", "4096Mi"); - assertResources(actualMap, "limits") + assertContainerResources(actualMap, "limits") .hasSize(1) .containsEntry("cpu", "4000m"); } @SuppressWarnings("unchecked") - private Map sanitizeRequestsAndLimits( + private Map sanitizeRequestsAndLimits(final ContainerType type, final Map actualRequests, final Map desiredRequests, final Map actualLimits, final Map desiredLimits) { - final var actual = new StatefulSetBuilder().withNewSpec().withNewTemplate().withNewSpec() - .addNewContainer() - .withName("test") - .withNewResources() - .withRequests(actualRequests).withLimits(actualLimits) - .endResources() - .endContainer() - .endSpec().endTemplate().endSpec().build(); - final var desired = new StatefulSetBuilder().withNewSpec().withNewTemplate().withNewSpec() - .addNewContainer() - .withName("test") - .withNewResources() - .withRequests(desiredRequests).withLimits(desiredLimits) - .endResources() - .endContainer() - .endSpec().endTemplate().endSpec().build(); - + final var actual = createStatefulSet(type, actualRequests, actualLimits); + final var desired = createStatefulSet(type, desiredRequests, desiredLimits); final var actualMap = serialization.convertValue(actual, Map.class); sanitizeResourceRequirements(actualMap, actual.getSpec().getTemplate(), @@ -190,9 +181,42 @@ private Map sanitizeRequestsAndLimits( return actualMap; } - private static MapAssert assertResources(final Map actualMap, - final String resourceName) { + private enum ContainerType { + CONTAINER, INIT_CONTAINER, + } + + private static StatefulSet createStatefulSet(final ContainerType type, + final Map requests, final Map limits) { + var builder = new StatefulSetBuilder().withNewSpec().withNewTemplate().withNewSpec(); + if (type == ContainerType.CONTAINER) { + builder = builder.addNewContainer() + .withName("test") + .withNewResources() + .withRequests(requests) + .withLimits(limits) + .endResources() + .endContainer(); + } else { + builder = builder.addNewInitContainer() + .withName("test") + .withNewResources() + .withRequests(requests) + .withLimits(limits) + .endResources() + .endInitContainer(); + } + return builder.endSpec().endTemplate().endSpec().build(); + } + + private static MapAssert assertContainerResources( + final Map actualMap, final String resourceName) { return assertThat(GenericKubernetesResource.>get(actualMap, "spec", "template", "spec", "containers", 0, "resources", resourceName)); } + + private static MapAssert assertInitContainerResources( + final Map actualMap, final String resourceName) { + return assertThat(GenericKubernetesResource.>get(actualMap, + "spec", "template", "spec", "initContainers", 0, "resources", resourceName)); + } } From 5dda16c6fc8997a99bea14be53c4246c22b61379 Mon Sep 17 00:00:00 2001 From: David Sondermann Date: Wed, 6 Nov 2024 05:45:54 +0000 Subject: [PATCH 5/5] refactor: fix import order Signed-off-by: David Sondermann --- .../ResourceRequirementsSanitizer.java | 8 ++--- ...BasedGenericKubernetesResourceMatcher.java | 29 ++++++++++--------- .../ResourceRequirementsSanitizerTest.java | 9 +++--- ...dGenericKubernetesResourceMatcherTest.java | 13 +++++---- 4 files changed, 31 insertions(+), 28 deletions(-) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/ResourceRequirementsSanitizer.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/ResourceRequirementsSanitizer.java index 7e877ed35f..3d83002692 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/ResourceRequirementsSanitizer.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/ResourceRequirementsSanitizer.java @@ -1,15 +1,15 @@ package io.javaoperatorsdk.operator.processing.dependent.kubernetes; +import java.util.List; +import java.util.Map; +import java.util.Optional; + import io.fabric8.kubernetes.api.model.Container; import io.fabric8.kubernetes.api.model.GenericKubernetesResource; import io.fabric8.kubernetes.api.model.PodTemplateSpec; import io.fabric8.kubernetes.api.model.Quantity; import io.fabric8.kubernetes.api.model.ResourceRequirements; -import java.util.List; -import java.util.Map; -import java.util.Optional; - /** * Sanitizes the {@link ResourceRequirements} in the containers of a pair of {@link PodTemplateSpec} * instances. 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 87fc2ad92b..5987352960 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 @@ -1,19 +1,5 @@ package io.javaoperatorsdk.operator.processing.dependent.kubernetes; -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.DaemonSet; -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.utils.KubernetesSerialization; -import io.javaoperatorsdk.operator.OperatorException; -import io.javaoperatorsdk.operator.api.reconciler.Context; -import io.javaoperatorsdk.operator.processing.LoggingUtils; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - import java.util.AbstractMap; import java.util.ArrayList; import java.util.Collections; @@ -27,6 +13,21 @@ import java.util.TreeMap; import java.util.stream.Collectors; +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.DaemonSet; +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.utils.KubernetesSerialization; +import io.javaoperatorsdk.operator.OperatorException; +import io.javaoperatorsdk.operator.api.reconciler.Context; +import io.javaoperatorsdk.operator.processing.LoggingUtils; + import static io.javaoperatorsdk.operator.processing.dependent.kubernetes.ResourceRequirementsSanitizer.sanitizeResourceRequirements; /** diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/ResourceRequirementsSanitizerTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/ResourceRequirementsSanitizerTest.java index 7c3e3e8fe0..b1ed6f0080 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/ResourceRequirementsSanitizerTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/ResourceRequirementsSanitizerTest.java @@ -1,5 +1,10 @@ package io.javaoperatorsdk.operator.processing.dependent.kubernetes; +import java.util.Map; + +import org.assertj.core.api.MapAssert; +import org.junit.jupiter.api.Test; + import io.fabric8.kubernetes.api.model.GenericKubernetesResource; import io.fabric8.kubernetes.api.model.HasMetadata; import io.fabric8.kubernetes.api.model.PodTemplateSpecBuilder; @@ -9,10 +14,6 @@ import io.fabric8.kubernetes.client.KubernetesClient; import io.fabric8.kubernetes.client.utils.KubernetesSerialization; import io.javaoperatorsdk.operator.MockKubernetesClient; -import org.assertj.core.api.MapAssert; -import org.junit.jupiter.api.Test; - -import java.util.Map; import static io.javaoperatorsdk.operator.processing.dependent.kubernetes.ResourceRequirementsSanitizer.sanitizeResourceRequirements; import static org.assertj.core.api.Assertions.assertThat; 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 c2555bc7e9..bfbd7eacbb 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 @@ -1,5 +1,12 @@ package io.javaoperatorsdk.operator.processing.dependent.kubernetes; +import java.util.Map; + +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; + import io.fabric8.kubernetes.api.model.ConfigMap; import io.fabric8.kubernetes.api.model.HasMetadata; import io.fabric8.kubernetes.api.model.apps.DaemonSet; @@ -11,12 +18,6 @@ import io.javaoperatorsdk.operator.api.config.ConfigurationService; import io.javaoperatorsdk.operator.api.config.ControllerConfiguration; import io.javaoperatorsdk.operator.api.reconciler.Context; -import org.junit.jupiter.api.BeforeEach; -import org.junit.jupiter.api.Test; -import org.junit.jupiter.params.ParameterizedTest; -import org.junit.jupiter.params.provider.ValueSource; - -import java.util.Map; import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.Mockito.mock;