From 5312cd3884f3b99a7b7fbf94f5e93d0ac366cd9d Mon Sep 17 00:00:00 2001 From: Jose Date: Wed, 26 Jul 2023 09:25:42 +0200 Subject: [PATCH] Fix sidecars resource requirements properties in Kubernetes Before these changes, the resource requirements limits/requests were ignored for the sidecars containers. Note that these changes include a workaround for https://github.com/dekorateio/dekorate/pull/1234. Fix https://github.com/quarkusio/quarkus/issues/35006 --- .../deployment/AddSidecarDecorator.java | 10 +++- .../deployment/ContainerAdapter.java | 54 ------------------- .../KubernetesWithSidecarAndJibTest.java | 7 +++ ...kubernetes-with-sidecar-and-jib.properties | 4 ++ 4 files changed, 19 insertions(+), 56 deletions(-) delete mode 100644 extensions/kubernetes/vanilla/deployment/src/main/java/io/quarkus/kubernetes/deployment/ContainerAdapter.java diff --git a/extensions/kubernetes/vanilla/deployment/src/main/java/io/quarkus/kubernetes/deployment/AddSidecarDecorator.java b/extensions/kubernetes/vanilla/deployment/src/main/java/io/quarkus/kubernetes/deployment/AddSidecarDecorator.java index b7d9ea8c3b757..c2bfd91bf60b1 100644 --- a/extensions/kubernetes/vanilla/deployment/src/main/java/io/quarkus/kubernetes/deployment/AddSidecarDecorator.java +++ b/extensions/kubernetes/vanilla/deployment/src/main/java/io/quarkus/kubernetes/deployment/AddSidecarDecorator.java @@ -1,5 +1,6 @@ package io.quarkus.kubernetes.deployment; +import io.dekorate.kubernetes.adapter.ContainerAdapter; import io.dekorate.kubernetes.config.Container; import io.dekorate.kubernetes.decorator.Decorator; import io.dekorate.kubernetes.decorator.NamedResourceDecorator; @@ -8,7 +9,9 @@ import io.fabric8.kubernetes.api.model.PodSpecBuilder; /** - * Copied from dekorate in order to fix some issues + * Copied from dekorate in order to fix some issues. + * TODO: This decorator should be removed and replaced by the Dekorate AddSidecarDecorator class after + * https://github.com/dekorateio/dekorate/pull/1234 is merged and Dekorate is bumped. */ class AddSidecarDecorator extends NamedResourceDecorator { @@ -26,7 +29,10 @@ public AddSidecarDecorator(String deployment, Container container) { @Override public void andThenVisit(PodSpecBuilder podSpec, ObjectMeta resourceMeta) { // this was changed to use our patched adapter - podSpec.addToContainers(ContainerAdapter.adapt(container)); + var sidecarContainer = ContainerAdapter.adapt(container); + // This is necessary because of the issue that this pull request fixes https://github.com/dekorateio/dekorate/pull/1234 + sidecarContainer.setWorkingDir(container.getWorkingDir()); + podSpec.addToContainers(sidecarContainer); } public Class[] after() { diff --git a/extensions/kubernetes/vanilla/deployment/src/main/java/io/quarkus/kubernetes/deployment/ContainerAdapter.java b/extensions/kubernetes/vanilla/deployment/src/main/java/io/quarkus/kubernetes/deployment/ContainerAdapter.java deleted file mode 100644 index 85eef9a76476b..0000000000000 --- a/extensions/kubernetes/vanilla/deployment/src/main/java/io/quarkus/kubernetes/deployment/ContainerAdapter.java +++ /dev/null @@ -1,54 +0,0 @@ -package io.quarkus.kubernetes.deployment; - -import io.dekorate.kubernetes.config.Env; -import io.dekorate.kubernetes.config.Mount; -import io.dekorate.kubernetes.config.Port; -import io.dekorate.kubernetes.decorator.AddEnvVarDecorator; -import io.dekorate.kubernetes.decorator.AddLivenessProbeDecorator; -import io.dekorate.kubernetes.decorator.AddMountDecorator; -import io.dekorate.kubernetes.decorator.AddPortDecorator; -import io.dekorate.kubernetes.decorator.AddReadinessProbeDecorator; -import io.dekorate.kubernetes.decorator.ApplyImagePullPolicyDecorator; -import io.dekorate.utils.Images; -import io.dekorate.utils.Strings; -import io.fabric8.kubernetes.api.model.Container; -import io.fabric8.kubernetes.api.model.ContainerBuilder; - -/** - * Copied from dekorate in order to fix some issues - */ -public class ContainerAdapter { - - private static final String ANY = null; - - public static Container adapt(io.dekorate.kubernetes.config.Container container) { - String name = container.getName(); - if (Strings.isNullOrEmpty(name)) { - name = Images.getName(container.getImage()); - } - - ContainerBuilder builder = new ContainerBuilder() - .withName(name) - .withImage(container.getImage()) - // this was changed to add working dir - .withWorkingDir(container.getWorkingDir()) - .withCommand(container.getCommand()) - .withArgs(container.getArguments()); - - for (Env env : container.getEnvVars()) { - builder.accept(new AddEnvVarDecorator(ANY, name, env)); - } - for (Port port : container.getPorts()) { - // this was changed to use our patched port decorator - builder.accept(new AddPortDecorator(ANY, name, port)); - } - for (Mount mount : container.getMounts()) { - builder.accept(new AddMountDecorator(ANY, name, mount)); - } - - builder.accept(new ApplyImagePullPolicyDecorator(name, container.getImagePullPolicy())); - builder.accept(new AddLivenessProbeDecorator(name, container.getLivenessProbe())); - builder.accept(new AddReadinessProbeDecorator(name, container.getReadinessProbe())); - return builder.build(); - } -} diff --git a/integration-tests/kubernetes/quarkus-standard-way/src/test/java/io/quarkus/it/kubernetes/KubernetesWithSidecarAndJibTest.java b/integration-tests/kubernetes/quarkus-standard-way/src/test/java/io/quarkus/it/kubernetes/KubernetesWithSidecarAndJibTest.java index 9560575704d8f..6820b9aa0c750 100644 --- a/integration-tests/kubernetes/quarkus-standard-way/src/test/java/io/quarkus/it/kubernetes/KubernetesWithSidecarAndJibTest.java +++ b/integration-tests/kubernetes/quarkus-standard-way/src/test/java/io/quarkus/it/kubernetes/KubernetesWithSidecarAndJibTest.java @@ -10,6 +10,7 @@ import org.junit.jupiter.api.extension.RegisterExtension; import io.fabric8.kubernetes.api.model.HasMetadata; +import io.fabric8.kubernetes.api.model.Quantity; import io.fabric8.kubernetes.api.model.apps.Deployment; import io.quarkus.builder.Version; import io.quarkus.maven.dependency.Dependency; @@ -90,6 +91,12 @@ private void assertSidecar(io.fabric8.kubernetes.api.model.PodSpec podSpec) { assertThat(e.getName()).isEqualTo("FOO"); assertThat(e.getValue()).isEqualTo("bar"); }); + assertThat(c.getResources()).satisfies(r -> { + assertThat(r.getRequests().get("cpu")).isEqualTo(new Quantity("102m")); + assertThat(r.getRequests().get("memory")).isEqualTo(new Quantity("201Mi")); + assertThat(r.getLimits().get("cpu")).isEqualTo(new Quantity("100m")); + assertThat(r.getLimits().get("memory")).isEqualTo(new Quantity("200Mi")); + }); }); } } diff --git a/integration-tests/kubernetes/quarkus-standard-way/src/test/resources/kubernetes-with-sidecar-and-jib.properties b/integration-tests/kubernetes/quarkus-standard-way/src/test/resources/kubernetes-with-sidecar-and-jib.properties index 4eae1a7c43558..45a41b5cc8c68 100644 --- a/integration-tests/kubernetes/quarkus-standard-way/src/test/resources/kubernetes-with-sidecar-and-jib.properties +++ b/integration-tests/kubernetes/quarkus-standard-way/src/test/resources/kubernetes-with-sidecar-and-jib.properties @@ -9,3 +9,7 @@ quarkus.kubernetes.sidecars.sc.arguments=-l quarkus.kubernetes.sidecars.sc.mounts.app-config.path=/deployments/config quarkus.kubernetes.sidecars.sc.ports.http.container-port=3000 quarkus.kubernetes.sidecars.sc.env-vars.foo.value=bar +quarkus.kubernetes.sidecars.sc.resources.requests.memory=201Mi +quarkus.kubernetes.sidecars.sc.resources.limits.memory=200Mi +quarkus.kubernetes.sidecars.sc.resources.requests.cpu=102m +quarkus.kubernetes.sidecars.sc.resources.limits.cpu=100m \ No newline at end of file