diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/PodTemplateSpecSanitizer.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/PodTemplateSpecSanitizer.java new file mode 100644 index 0000000000..962059961e --- /dev/null +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/PodTemplateSpecSanitizer.java @@ -0,0 +1,198 @@ +package io.javaoperatorsdk.operator.processing.dependent.kubernetes; + +import java.util.List; +import java.util.Map; +import java.util.Objects; +import java.util.Optional; + +import io.fabric8.kubernetes.api.model.Container; +import io.fabric8.kubernetes.api.model.EnvVar; +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; + +/** + * Sanitizes the {@link ResourceRequirements} and the {@link EnvVar} 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 fields, 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 fields 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} and {@link EnvVar} instances. Using the {@link + * GenericKubernetesResource#get(Map, Object...)} shortcut would need to create new instances just + * for the sanitization check. + */ +class PodTemplateSpecSanitizer { + + static void sanitizePodTemplateSpec( + final Map actualMap, + final PodTemplateSpec actualTemplate, + final PodTemplateSpec desiredTemplate) { + if (actualTemplate == null || desiredTemplate == null) { + return; + } + if (actualTemplate.getSpec() == null || desiredTemplate.getSpec() == null) { + return; + } + sanitizePodTemplateSpec( + actualMap, + actualTemplate.getSpec().getInitContainers(), + desiredTemplate.getSpec().getInitContainers(), + "initContainers"); + sanitizePodTemplateSpec( + actualMap, + actualTemplate.getSpec().getContainers(), + desiredTemplate.getSpec().getContainers(), + "containers"); + } + + private static void sanitizePodTemplateSpec( + 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++) { + final var desiredContainer = desiredContainers.get(containerIndex); + final var actualContainer = actualContainers.get(containerIndex); + if (!desiredContainer.getName().equals(actualContainer.getName())) { + return; + } + sanitizeEnvVars( + actualMap, + actualContainer.getEnv(), + desiredContainer.getEnv(), + containerPath, + containerIndex); + 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) -> { + final 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()); + } + })); + } + + @SuppressWarnings("unchecked") + private static void sanitizeEnvVars( + final Map actualMap, + final List actualEnvVars, + final List desiredEnvVars, + final String containerPath, + final int containerIndex) { + if (desiredEnvVars.isEmpty() || actualEnvVars.isEmpty()) { + return; + } + Optional.ofNullable( + GenericKubernetesResource.get( + actualMap, "spec", "template", "spec", containerPath, containerIndex, "env")) + .map(List.class::cast) + .ifPresent( + envVars -> + actualEnvVars.forEach( + actualEnvVar -> { + final var actualEnvVarName = actualEnvVar.getName(); + final var actualEnvVarValue = actualEnvVar.getValue(); + // check if the actual EnvVar value string is not null or the desired EnvVar + // already contains the same EnvVar name with a non empty EnvVar value + final var isDesiredEnvVarEmpty = + hasEnvVarNoEmptyValue(actualEnvVarName, desiredEnvVars); + if (actualEnvVarValue != null || isDesiredEnvVarEmpty) { + return; + } + envVars.stream() + .filter( + envVar -> + ((Map) envVar) + .get("name") + .equals(actualEnvVarName)) + // add the actual EnvVar value with an empty string to prevent a + // resource update + .forEach(envVar -> ((Map) envVar).put("value", "")); + })); + } + + private static boolean hasEnvVarNoEmptyValue( + final String envVarName, final List envVars) { + return envVars.stream() + .anyMatch( + envVar -> + Objects.equals(envVarName, envVar.getName()) + && envVar.getValue() != null + && !envVar.getValue().isEmpty()); + } +} 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 deleted file mode 100644 index 3d83002692..0000000000 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/ResourceRequirementsSanitizer.java +++ /dev/null @@ -1,100 +0,0 @@ -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; - -/** - * 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 261ab6c825..3564bfe449 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 @@ -31,7 +31,7 @@ import com.github.difflib.DiffUtils; import com.github.difflib.UnifiedDiffUtils; -import static io.javaoperatorsdk.operator.processing.dependent.kubernetes.ResourceRequirementsSanitizer.sanitizeResourceRequirements; +import static io.javaoperatorsdk.operator.processing.dependent.kubernetes.PodTemplateSpecSanitizer.sanitizePodTemplateSpec; /** * Matches the actual state on the server vs the desired state. Based on the managedFields of SSA. @@ -190,20 +190,23 @@ private void sanitizeState(R actual, R desired, Map actualMap) { } } } - sanitizeResourceRequirements(actualMap, actualSpec.getTemplate(), desiredSpec.getTemplate()); + sanitizePodTemplateSpec(actualMap, actualSpec.getTemplate(), desiredSpec.getTemplate()); } else if (actual instanceof Deployment actualDeployment && desired instanceof Deployment desiredDeployment) { - sanitizeResourceRequirements(actualMap, + sanitizePodTemplateSpec( + actualMap, actualDeployment.getSpec().getTemplate(), desiredDeployment.getSpec().getTemplate()); } else if (actual instanceof ReplicaSet actualReplicaSet && desired instanceof ReplicaSet desiredReplicaSet) { - sanitizeResourceRequirements(actualMap, + sanitizePodTemplateSpec( + actualMap, actualReplicaSet.getSpec().getTemplate(), desiredReplicaSet.getSpec().getTemplate()); } else if (actual instanceof DaemonSet actualDaemonSet && desired instanceof DaemonSet desiredDaemonSet) { - sanitizeResourceRequirements(actualMap, + sanitizePodTemplateSpec( + actualMap, actualDaemonSet.getSpec().getTemplate(), desiredDaemonSet.getSpec().getTemplate()); } diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/PodTemplateSpecSanitizerTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/PodTemplateSpecSanitizerTest.java new file mode 100644 index 0000000000..091a1a666c --- /dev/null +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/PodTemplateSpecSanitizerTest.java @@ -0,0 +1,408 @@ +package io.javaoperatorsdk.operator.processing.dependent.kubernetes; + +import java.util.List; +import java.util.Map; + +import org.assertj.core.api.ListAssert; +import org.assertj.core.api.MapAssert; +import org.junit.jupiter.api.Test; + +import io.fabric8.kubernetes.api.model.EnvVar; +import io.fabric8.kubernetes.api.model.EnvVarBuilder; +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.StatefulSet; +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 static io.javaoperatorsdk.operator.processing.dependent.kubernetes.PodTemplateSpecSanitizer.sanitizePodTemplateSpec; +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verifyNoInteractions; + +/** + * Tests the {@link PodTemplateSpecSanitizer} with combinations of matching and mismatching K8s + * resources, using a mix of containers and init containers, as well as resource requests and limits + * along with environment variables. + */ +class PodTemplateSpecSanitizerTest { + + private final Map actualMap = mock(); + + private final KubernetesClient client = MockKubernetesClient.client(HasMetadata.class); + private final KubernetesSerialization serialization = client.getKubernetesSerialization(); + + @Test + void testSanitizePodTemplateSpec_whenTemplateIsNull_doNothing() { + final var template = new PodTemplateSpecBuilder().build(); + + sanitizePodTemplateSpec(actualMap, null, template); + sanitizePodTemplateSpec(actualMap, template, null); + verifyNoInteractions(actualMap); + } + + @Test + void testSanitizePodTemplateSpec_whenTemplateSpecIsNull_doNothing() { + final var template = new PodTemplateSpecBuilder().withSpec(null).build(); + final var templateWithSpec = new PodTemplateSpecBuilder().withNewSpec().endSpec().build(); + + sanitizePodTemplateSpec(actualMap, template, templateWithSpec); + sanitizePodTemplateSpec(actualMap, templateWithSpec, template); + verifyNoInteractions(actualMap); + } + + @Test + void testSanitizePodTemplateSpec_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(); + + sanitizePodTemplateSpec(actualMap, template, templateWithTwoContainers); + sanitizePodTemplateSpec(actualMap, templateWithTwoContainers, template); + verifyNoInteractions(actualMap); + } + + @Test + void testSanitizePodTemplateSpec_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(); + + sanitizePodTemplateSpec(actualMap, template, templateWithNewContainerName); + sanitizePodTemplateSpec(actualMap, templateWithNewContainerName, template); + verifyNoInteractions(actualMap); + } + + @Test + void testSanitizePodTemplateSpec_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(); + + sanitizePodTemplateSpec(actualMap, template, templateWithResource); + sanitizePodTemplateSpec(actualMap, templateWithResource, template); + verifyNoInteractions(actualMap); + } + + @Test + void testSanitizeResourceRequirements_whenResourceSizeMismatch_doNothing() { + 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"))); + assertContainerResources(actualMap, "requests").hasSize(1).containsEntry("cpu", "2"); + assertContainerResources(actualMap, "limits").hasSize(1).containsEntry("cpu", "4"); + } + + @Test + void testSanitizeResourceRequirements_whenResourceKeyMismatch_doNothing() { + final var actualMap = + sanitizeRequestsAndLimits( + ContainerType.INIT_CONTAINER, + Map.of("cpu", new Quantity("2")), + Map.of("memory", new Quantity("4Gi")), + Map.of(), + Map.of()); + assertInitContainerResources(actualMap, "requests").hasSize(1).containsEntry("cpu", "2"); + assertInitContainerResources(actualMap, "limits").isNull(); + } + + @Test + void testSanitizePodTemplateSpec_whenResourcesHaveSameAmountAndFormat_doNothing() { + 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"))); + assertContainerResources(actualMap, "requests").hasSize(1).containsEntry("memory", "4Gi"); + assertContainerResources(actualMap, "limits").hasSize(1).containsEntry("cpu", "2"); + } + + @Test + void testSanitizePodTemplateSpec_whenResourcesHaveNumericalAmountMismatch_doNothing() { + 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"))); + assertInitContainerResources(actualMap, "requests") + .hasSize(2) + .containsEntry("cpu", "2") + .containsEntry("memory", "4Gi"); + assertInitContainerResources(actualMap, "limits").hasSize(1).containsEntry("cpu", "2"); + } + + @Test + void + testSanitizeResourceRequirements_whenResourcesHaveAmountAndFormatMismatchWithSameNumericalAmount_thenSanitizeActualMap() { + 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"))); + assertContainerResources(actualMap, "requests") + .hasSize(2) + .containsEntry("cpu", "2000m") + .containsEntry("memory", "4096Mi"); + assertContainerResources(actualMap, "limits").hasSize(1).containsEntry("cpu", "4000m"); + } + + @Test + void testSanitizePodTemplateSpec_whenEnvVarsIsEmpty_doNothing() { + final var template = + new PodTemplateSpecBuilder() + .withNewSpec() + .addNewContainer() + .withName("test") + .endContainer() + .endSpec() + .build(); + final var templateWithEnvVars = + new PodTemplateSpecBuilder() + .withNewSpec() + .addNewContainer() + .withName("test") + .withEnv(List.of(new EnvVarBuilder().withName("FOO").withValue("foobar").build())) + .endContainer() + .endSpec() + .build(); + + sanitizePodTemplateSpec(actualMap, template, templateWithEnvVars); + sanitizePodTemplateSpec(actualMap, templateWithEnvVars, template); + verifyNoInteractions(actualMap); + } + + @Test + void testSanitizePodTemplateSpec_whenActualEnvVarValueIsNotEmpty_doNothing() { + final var actualMap = + sanitizeEnvVars( + ContainerType.CONTAINER, + List.of( + new EnvVarBuilder().withName("FOO").withValue("foo").build(), + new EnvVarBuilder().withName("BAR").withValue("bar").build()), + List.of( + new EnvVarBuilder().withName("FOO").withValue("bar").build(), + new EnvVarBuilder().withName("BAR").withValue("foo").build())); + assertContainerEnvVars(actualMap) + .hasSize(2) + .containsExactly( + Map.of("name", "FOO", "value", "foo"), Map.of("name", "BAR", "value", "bar")); + } + + @Test + void testSanitizePodTemplateSpec_whenActualAndDesiredEnvVarsAreDifferent_doNothing() { + final var actualMap = + sanitizeEnvVars( + ContainerType.INIT_CONTAINER, + List.of(new EnvVarBuilder().withName("FOO").withValue("foo").build()), + List.of(new EnvVarBuilder().withName("BAR").withValue("bar").build())); + assertInitContainerEnvVars(actualMap) + .hasSize(1) + .containsExactly(Map.of("name", "FOO", "value", "foo")); + } + + @Test + void testSanitizePodTemplateSpec_whenActualEnvVarIsEmpty_doNothing() { + final var actualMap = + sanitizeEnvVars( + ContainerType.INIT_CONTAINER, + List.of( + new EnvVarBuilder().withName("FOO").withValue("").build(), + new EnvVarBuilder().withName("BAR").withValue("").build()), + List.of( + new EnvVarBuilder().withName("FOO").withValue("foo").build(), + new EnvVarBuilder().withName("BAR").withValue("").build())); + assertInitContainerEnvVars(actualMap) + .hasSize(2) + .containsExactly(Map.of("name", "FOO", "value", ""), Map.of("name", "BAR", "value", "")); + } + + @Test + void testSanitizePodTemplateSpec_whenActualEnvVarIsNull_doNothing() { + final var actualMap = + sanitizeEnvVars( + ContainerType.CONTAINER, + List.of( + new EnvVarBuilder().withName("FOO").withValue(null).build(), + new EnvVarBuilder().withName("BAR").withValue(null).build()), + List.of( + new EnvVarBuilder().withName("FOO").withValue("foo").build(), + new EnvVarBuilder().withName("BAR").withValue(" ").build())); + assertContainerEnvVars(actualMap) + .hasSize(2) + .containsExactly(Map.of("name", "FOO"), Map.of("name", "BAR")); + } + + @Test + void + testSanitizePodTemplateSpec_whenActualEnvVarIsNull_withDesiredEnvVarEmpty_thenSanitizeActualMap() { + final var actualMap = + sanitizeEnvVars( + ContainerType.CONTAINER, + List.of( + new EnvVarBuilder().withName("FOO").withValue(null).build(), + new EnvVarBuilder().withName("BAR").withValue(null).build()), + List.of( + new EnvVarBuilder().withName("FOO").withValue("").build(), + new EnvVarBuilder().withName("BAR").withValue("").build())); + assertContainerEnvVars(actualMap) + .hasSize(2) + .containsExactly(Map.of("name", "FOO", "value", ""), Map.of("name", "BAR", "value", "")); + } + + private Map sanitizeRequestsAndLimits( + final ContainerType type, + final Map actualRequests, + final Map desiredRequests, + final Map actualLimits, + final Map desiredLimits) { + return sanitize( + type, actualRequests, desiredRequests, actualLimits, desiredLimits, List.of(), List.of()); + } + + private Map sanitizeEnvVars( + final ContainerType type, + final List actualEnvVars, + final List desiredEnvVars) { + return sanitize(type, Map.of(), Map.of(), Map.of(), Map.of(), actualEnvVars, desiredEnvVars); + } + + @SuppressWarnings("unchecked") + private Map sanitize( + final ContainerType type, + final Map actualRequests, + final Map desiredRequests, + final Map actualLimits, + final Map desiredLimits, + final List actualEnvVars, + final List desiredEnvVars) { + final var actual = createStatefulSet(type, actualRequests, actualLimits, actualEnvVars); + final var desired = createStatefulSet(type, desiredRequests, desiredLimits, desiredEnvVars); + final var actualMap = serialization.convertValue(actual, Map.class); + sanitizePodTemplateSpec( + actualMap, actual.getSpec().getTemplate(), desired.getSpec().getTemplate()); + return actualMap; + } + + private enum ContainerType { + CONTAINER, + INIT_CONTAINER, + } + + private static StatefulSet createStatefulSet( + final ContainerType type, + final Map requests, + final Map limits, + final List envVars) { + var builder = new StatefulSetBuilder().withNewSpec().withNewTemplate().withNewSpec(); + if (type == ContainerType.CONTAINER) { + builder = + builder + .addNewContainer() + .withName("test") + .withNewResources() + .withRequests(requests) + .withLimits(limits) + .endResources() + .withEnv(envVars) + .endContainer(); + } else { + builder = + builder + .addNewInitContainer() + .withName("test") + .withNewResources() + .withRequests(requests) + .withLimits(limits) + .endResources() + .withEnv(envVars) + .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)); + } + + private static ListAssert> assertContainerEnvVars( + final Map actualMap) { + return assertThat( + GenericKubernetesResource.>>get( + actualMap, "spec", "template", "spec", "containers", 0, "env")); + } + + private static ListAssert> assertInitContainerEnvVars( + final Map actualMap) { + return assertThat( + GenericKubernetesResource.>>get( + actualMap, "spec", "template", "spec", "initContainers", 0, "env")); + } +} 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 deleted file mode 100644 index b1ed6f0080..0000000000 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/ResourceRequirementsSanitizerTest.java +++ /dev/null @@ -1,223 +0,0 @@ -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; -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; -import io.javaoperatorsdk.operator.MockKubernetesClient; - -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; - -/** - * 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(); - - 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(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"))); - assertContainerResources(actualMap, "requests") - .hasSize(1) - .containsEntry("cpu", "2"); - assertContainerResources(actualMap, "limits") - .hasSize(1) - .containsEntry("cpu", "4"); - } - - @Test - void testSanitizeResourceRequirements_whenResourceKeyMismatch_doNothing() { - final var actualMap = sanitizeRequestsAndLimits(ContainerType.INIT_CONTAINER, - Map.of("cpu", new Quantity("2")), - Map.of("memory", new Quantity("4Gi")), - Map.of(), - Map.of()); - assertInitContainerResources(actualMap, "requests") - .hasSize(1) - .containsEntry("cpu", "2"); - assertInitContainerResources(actualMap, "limits").isNull(); - } - - @Test - void testSanitizeResourceRequirements_whenResourcesHaveSameAmountAndFormat_doNothing() { - 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"))); - assertContainerResources(actualMap, "requests") - .hasSize(1) - .containsEntry("memory", "4Gi"); - assertContainerResources(actualMap, "limits") - .hasSize(1) - .containsEntry("cpu", "2"); - } - - @Test - void testSanitizeResourceRequirements_whenResourcesHaveNumericalAmountMismatch_doNothing() { - 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"))); - assertInitContainerResources(actualMap, "requests") - .hasSize(2) - .containsEntry("cpu", "2") - .containsEntry("memory", "4Gi"); - assertInitContainerResources(actualMap, "limits") - .hasSize(1) - .containsEntry("cpu", "2"); - } - - @Test - void testSanitizeResourceRequirements_whenResourcesHaveAmountAndFormatMismatchWithSameNumericalAmount_thenSanitizeActualMap() { - 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"))); - assertContainerResources(actualMap, "requests") - .hasSize(2) - .containsEntry("cpu", "2000m") - .containsEntry("memory", "4096Mi"); - assertContainerResources(actualMap, "limits") - .hasSize(1) - .containsEntry("cpu", "4000m"); - } - - @SuppressWarnings("unchecked") - private Map sanitizeRequestsAndLimits(final ContainerType type, - final Map actualRequests, final Map desiredRequests, - final Map actualLimits, final Map desiredLimits) { - 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(), - desired.getSpec().getTemplate()); - return actualMap; - } - - 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)); - } -} diff --git a/operator-framework/src/test/resources/io/javaoperatorsdk/operator/statefulset.yaml b/operator-framework/src/test/resources/io/javaoperatorsdk/operator/statefulset.yaml index f40fbeb607..bb8a2df04b 100644 --- a/operator-framework/src/test/resources/io/javaoperatorsdk/operator/statefulset.yaml +++ b/operator-framework/src/test/resources/io/javaoperatorsdk/operator/statefulset.yaml @@ -21,6 +21,13 @@ spec: ports: - containerPort: 80 name: web + env: + - name: APP1_HOST_NAME + value: "" + - name: APP2_HOST_NAME + value: "localhost" + - name: APP3_HOST_NAME + value: " " volumeMounts: - name: www mountPath: /usr/share/nginx/html