From edb72c2ca0cfc4c41ffcf8f2f1e4eeeeddea7a3b Mon Sep 17 00:00:00 2001 From: Sergii Leshchenko Date: Wed, 5 Feb 2020 11:08:20 +0200 Subject: [PATCH] Provision environment variables for initContainers of chePlugin/cheEditor (#15508) Signed-off-by: Sergii Leshchenko --- .../kubernetes/util/EnvVars.java | 15 ++++- .../KubernetesPluginsToolingApplier.java | 20 ++++--- .../kubernetes/wsplugins/MachineResolver.java | 5 +- .../KubernetesPluginsToolingApplierTest.java | 60 ++++++++++++++++--- .../wsplugins/MachineResolverTest.java | 13 ---- 5 files changed, 80 insertions(+), 33 deletions(-) diff --git a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/util/EnvVars.java b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/util/EnvVars.java index 2c8f8f876e..ad130032bb 100644 --- a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/util/EnvVars.java +++ b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/util/EnvVars.java @@ -35,8 +35,8 @@ public class EnvVars { *

If a container does not have the corresponding env - it will be provisioned, if it has - the * value will be overridden. * - * @param podData pod to apply env - * @param env env var to apply + * @param podData pod to supply env vars + * @param env env vars to apply */ public void apply(PodData podData, List env) { Stream.concat( @@ -45,7 +45,16 @@ public class EnvVars { .forEach(c -> apply(c, env)); } - private void apply(Container container, List toApply) { + /** + * Applies the specified env vars list to the specified containers. + * + *

If a container does not have the corresponding env - it will be provisioned, if it has - the + * value will be overridden. + * + * @param container pod to supply env vars + * @param toApply env vars to apply + */ + public void apply(Container container, List toApply) { List targetEnv = container.getEnv(); if (targetEnv == null) { targetEnv = new ArrayList<>(); diff --git a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/wsplugins/KubernetesPluginsToolingApplier.java b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/wsplugins/KubernetesPluginsToolingApplier.java index df87fa7872..f8cfcc2ca8 100644 --- a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/wsplugins/KubernetesPluginsToolingApplier.java +++ b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/wsplugins/KubernetesPluginsToolingApplier.java @@ -55,6 +55,7 @@ import org.eclipse.che.workspace.infrastructure.kubernetes.Names; import org.eclipse.che.workspace.infrastructure.kubernetes.Warnings; import org.eclipse.che.workspace.infrastructure.kubernetes.environment.KubernetesEnvironment; import org.eclipse.che.workspace.infrastructure.kubernetes.environment.KubernetesEnvironment.PodData; +import org.eclipse.che.workspace.infrastructure.kubernetes.util.EnvVars; /** * Applies Che plugins tooling configuration to a kubernetes internal runtime object. @@ -75,6 +76,7 @@ public class KubernetesPluginsToolingApplier implements ChePluginsApplier { private final boolean isAuthEnabled; private final ProjectsRootEnvVariableProvider projectsRootEnvVariableProvider; private final ChePluginsVolumeApplier chePluginsVolumeApplier; + private final EnvVars envVars; @Inject public KubernetesPluginsToolingApplier( @@ -82,13 +84,15 @@ public class KubernetesPluginsToolingApplier implements ChePluginsApplier { @Named("che.workspace.sidecar.default_memory_limit_mb") long defaultSidecarMemoryLimitMB, @Named("che.agents.auth_enabled") boolean isAuthEnabled, ProjectsRootEnvVariableProvider projectsRootEnvVariableProvider, - ChePluginsVolumeApplier chePluginsVolumeApplier) { + ChePluginsVolumeApplier chePluginsVolumeApplier, + EnvVars envVars) { this.defaultSidecarMemoryLimitBytes = String.valueOf(defaultSidecarMemoryLimitMB * 1024 * 1024); this.isAuthEnabled = isAuthEnabled; this.sidecarImagePullPolicy = validImagePullPolicies.contains(sidecarImagePullPolicy) ? sidecarImagePullPolicy : null; this.projectsRootEnvVariableProvider = projectsRootEnvVariableProvider; this.chePluginsVolumeApplier = chePluginsVolumeApplier; + this.envVars = envVars; } @Override @@ -119,12 +123,6 @@ public class KubernetesPluginsToolingApplier implements ChePluginsApplier { CommandsResolver commandsResolver = new CommandsResolver(k8sEnv); for (ChePlugin chePlugin : chePlugins) { - for (CheContainer container : chePlugin.getInitContainers()) { - Container k8sInitContainer = toK8sContainer(container); - pod.getSpec().getInitContainers().add(k8sInitContainer); - chePluginsVolumeApplier.applyVolumes(pod, k8sInitContainer, container.getVolumes(), k8sEnv); - } - Map devfilePlugins = k8sEnv .getDevfile() @@ -141,6 +139,13 @@ public class KubernetesPluginsToolingApplier implements ChePluginsApplier { } ComponentImpl pluginRelatedComponent = devfilePlugins.get(chePlugin.getId()); + for (CheContainer container : chePlugin.getInitContainers()) { + Container k8sInitContainer = toK8sContainer(container); + envVars.apply(k8sInitContainer, pluginRelatedComponent.getEnv()); + chePluginsVolumeApplier.applyVolumes(pod, k8sInitContainer, container.getVolumes(), k8sEnv); + pod.getSpec().getInitContainers().add(k8sInitContainer); + } + Collection pluginRelatedCommands = commandsResolver.resolve(chePlugin); for (CheContainer container : chePlugin.getContainers()) { @@ -237,6 +242,7 @@ public class KubernetesPluginsToolingApplier implements ChePluginsApplier { List containerEndpoints = k8sContainerResolver.getEndpoints(); Container k8sContainer = k8sContainerResolver.resolve(); + envVars.apply(k8sContainer, pluginRelatedComponent.getEnv()); chePluginsVolumeApplier.applyVolumes(pod, k8sContainer, container.getVolumes(), k8sEnv); String machineName = k8sContainer.getName(); diff --git a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/wsplugins/MachineResolver.java b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/wsplugins/MachineResolver.java index 821fac73d5..0829923a66 100644 --- a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/wsplugins/MachineResolver.java +++ b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/wsplugins/MachineResolver.java @@ -13,6 +13,7 @@ package org.eclipse.che.workspace.infrastructure.kubernetes.wsplugins; import static com.google.common.base.Strings.isNullOrEmpty; import static java.lang.String.format; +import static java.util.Collections.emptyMap; import static java.util.stream.Collectors.toMap; import static org.eclipse.che.api.core.model.workspace.config.MachineConfig.DEVFILE_COMPONENT_ALIAS_ATTRIBUTE; import static org.eclipse.che.api.core.model.workspace.config.MachineConfig.MEMORY_LIMIT_ATTRIBUTE; @@ -23,10 +24,8 @@ import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.Map.Entry; -import java.util.stream.Collectors; import org.eclipse.che.api.core.model.workspace.config.ServerConfig; import org.eclipse.che.api.core.model.workspace.devfile.Component; -import org.eclipse.che.api.core.model.workspace.devfile.Env; import org.eclipse.che.api.workspace.server.model.impl.ServerConfigImpl; import org.eclipse.che.api.workspace.server.model.impl.VolumeImpl; import org.eclipse.che.api.workspace.server.spi.InfrastructureException; @@ -67,7 +66,7 @@ public class MachineResolver { InternalMachineConfig machineConfig = new InternalMachineConfig( toServers(containerEndpoints), - component.getEnv().stream().collect(Collectors.toMap(Env::getName, Env::getValue)), + emptyMap(), resolveMachineAttributes(), toWorkspaceVolumes(cheContainer)); diff --git a/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/wsplugins/KubernetesPluginsToolingApplierTest.java b/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/wsplugins/KubernetesPluginsToolingApplierTest.java index ee92e1e15f..6a3fe5ac28 100644 --- a/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/wsplugins/KubernetesPluginsToolingApplierTest.java +++ b/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/wsplugins/KubernetesPluginsToolingApplierTest.java @@ -30,6 +30,7 @@ import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.lenient; import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import static org.testng.Assert.assertEquals; @@ -63,6 +64,7 @@ import org.eclipse.che.api.workspace.server.model.impl.CommandImpl; import org.eclipse.che.api.workspace.server.model.impl.ServerConfigImpl; import org.eclipse.che.api.workspace.server.model.impl.devfile.ComponentImpl; import org.eclipse.che.api.workspace.server.model.impl.devfile.DevfileImpl; +import org.eclipse.che.api.workspace.server.model.impl.devfile.EnvImpl; import org.eclipse.che.api.workspace.server.spi.InfrastructureException; import org.eclipse.che.api.workspace.server.spi.environment.InternalEnvironment; import org.eclipse.che.api.workspace.server.spi.environment.InternalMachineConfig; @@ -79,6 +81,8 @@ import org.eclipse.che.commons.lang.Pair; import org.eclipse.che.workspace.infrastructure.kubernetes.Warnings; import org.eclipse.che.workspace.infrastructure.kubernetes.environment.KubernetesEnvironment; import org.eclipse.che.workspace.infrastructure.kubernetes.environment.KubernetesEnvironment.PodData; +import org.eclipse.che.workspace.infrastructure.kubernetes.util.EnvVars; +import org.mockito.ArgumentCaptor; import org.mockito.Mock; import org.mockito.testng.MockitoTestNGListener; import org.testng.annotations.BeforeMethod; @@ -108,6 +112,7 @@ public class KubernetesPluginsToolingApplierTest { @Mock private RuntimeIdentity runtimeIdentity; @Mock private ProjectsRootEnvVariableProvider projectsRootEnvVariableProvider; @Mock private ChePluginsVolumeApplier chePluginsVolumeApplier; + @Mock private EnvVars envVars; private KubernetesEnvironment internalEnvironment; private KubernetesPluginsToolingApplier applier; @@ -122,7 +127,8 @@ public class KubernetesPluginsToolingApplierTest { MEMORY_LIMIT_MB, false, projectsRootEnvVariableProvider, - chePluginsVolumeApplier); + chePluginsVolumeApplier, + envVars); Map machines = new HashMap<>(); List containers = new ArrayList<>(); @@ -168,6 +174,33 @@ public class KubernetesPluginsToolingApplierTest { validateContainerNameName(envCommand.getAttributes().get(MACHINE_NAME_ATTRIBUTE), "container"); } + @Test + public void shouldProvisionApplyEnvironmentVariableToContainersAndInitContainersOfPlugin() + throws Exception { + // given + CheContainer container = new CheContainer(); + container.setName("container"); + CheContainer initContainer = new CheContainer(); + initContainer.setName("initContainer"); + + ChePlugin chePlugin = + createChePlugin( + "publisher/id/1.0.0", singletonList(container), singletonList(initContainer)); + ComponentImpl component = internalEnvironment.getDevfile().getComponents().get(0); + component.getEnv().add(new EnvImpl("TEST", "VALUE")); + ArgumentCaptor containerArgumentCaptor = ArgumentCaptor.forClass(Container.class); + + // when + applier.apply(runtimeIdentity, internalEnvironment, singletonList(chePlugin)); + + // then + verify(envVars, times(2)).apply(containerArgumentCaptor.capture(), eq(component.getEnv())); + List containers = containerArgumentCaptor.getAllValues(); + // containers names are suffixed to provide uniqueness and converted to be k8s API compatible + assertTrue(containers.get(0).getName().startsWith("initcontainer")); + assertTrue(containers.get(1).getName().startsWith("container")); + } + @Test( expectedExceptions = InfrastructureException.class, expectedExceptionsMessageRegExp = @@ -758,7 +791,8 @@ public class KubernetesPluginsToolingApplierTest { MEMORY_LIMIT_MB, true, projectsRootEnvVariableProvider, - chePluginsVolumeApplier); + chePluginsVolumeApplier, + envVars); applier.apply(runtimeIdentity, internalEnvironment, singletonList(createChePlugin())); @@ -774,7 +808,8 @@ public class KubernetesPluginsToolingApplierTest { MEMORY_LIMIT_MB, true, projectsRootEnvVariableProvider, - chePluginsVolumeApplier); + chePluginsVolumeApplier, + envVars); internalEnvironment.getAttributes().put(SECURE_EXPOSER_IMPL_PROPERTY, "somethingElse"); applier.apply(runtimeIdentity, internalEnvironment, singletonList(createChePlugin())); @@ -791,7 +826,8 @@ public class KubernetesPluginsToolingApplierTest { MEMORY_LIMIT_MB, true, projectsRootEnvVariableProvider, - chePluginsVolumeApplier); + chePluginsVolumeApplier, + envVars); applier.apply(runtimeIdentity, internalEnvironment, singletonList(createChePlugin())); @@ -816,7 +852,8 @@ public class KubernetesPluginsToolingApplierTest { MEMORY_LIMIT_MB, true, projectsRootEnvVariableProvider, - chePluginsVolumeApplier); + chePluginsVolumeApplier, + envVars); applier.apply(runtimeIdentity, internalEnvironment, singletonList(createChePlugin())); @@ -844,17 +881,26 @@ public class KubernetesPluginsToolingApplierTest { } private ChePlugin createChePlugin(CheContainer... containers) { - return createChePlugin("publisher/" + NameGenerator.generate("name", 3) + "/0.0.1", containers); + return createChePlugin( + "publisher/" + NameGenerator.generate("name", 3) + "/0.0.1", + Arrays.asList(containers), + emptyList()); } private ChePlugin createChePlugin(String id, CheContainer... containers) { + return createChePlugin(id, Arrays.asList(containers), emptyList()); + } + + private ChePlugin createChePlugin( + String id, List containers, List initContainers) { String[] splittedId = id.split("/"); ChePlugin plugin = new ChePlugin(); plugin.setPublisher(splittedId[0]); plugin.setName(splittedId[1]); plugin.setVersion(splittedId[2]); plugin.setId(id); - plugin.setContainers(Arrays.asList(containers)); + plugin.setContainers(containers); + plugin.setInitContainers(initContainers); internalEnvironment.getDevfile().getComponents().add(new ComponentImpl("chePlugin", id)); return plugin; diff --git a/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/wsplugins/MachineResolverTest.java b/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/wsplugins/MachineResolverTest.java index 78a0fbaa16..6b859084bf 100644 --- a/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/wsplugins/MachineResolverTest.java +++ b/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/wsplugins/MachineResolverTest.java @@ -25,7 +25,6 @@ import org.eclipse.che.api.core.model.workspace.config.ServerConfig; import org.eclipse.che.api.workspace.server.model.impl.ServerConfigImpl; import org.eclipse.che.api.workspace.server.model.impl.VolumeImpl; import org.eclipse.che.api.workspace.server.model.impl.devfile.ComponentImpl; -import org.eclipse.che.api.workspace.server.model.impl.devfile.EnvImpl; import org.eclipse.che.api.workspace.server.spi.InfrastructureException; import org.eclipse.che.api.workspace.server.spi.environment.InternalMachineConfig; import org.eclipse.che.api.workspace.server.wsplugins.model.CheContainer; @@ -164,18 +163,6 @@ public class MachineResolverTest { assertEquals(machineConfig.getAttributes().get(MEMORY_LIMIT_ATTRIBUTE), expectedMemLimit); } - @Test - public void - shouldProvisionEnvironmentVarsIntoMachineConfigOfASidecarIfTheyAreSetInTheCorrespondingComponent() - throws InfrastructureException { - component.getEnv().add(new EnvImpl("test", "value")); - - InternalMachineConfig machineConfig = resolver.resolve(); - - assertEquals(machineConfig.getEnv().size(), 1); - assertEquals(machineConfig.getEnv().get("test"), "value"); - } - @Test public void shouldNotSetMemLimitAttributeIfLimitIsInContainer() throws InfrastructureException { Containers.addRamLimit(container, 123456789);