Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Provision environment variables for initContainers of chePlugin/cheEditor #15508

Merged
merged 1 commit into from
Feb 5, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,8 @@ public class EnvVars {
* <p>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<? extends Env> env) {
Stream.concat(
Expand All @@ -45,7 +45,16 @@ public void apply(PodData podData, List<? extends Env> env) {
.forEach(c -> apply(c, env));
}

private void apply(Container container, List<? extends Env> toApply) {
/**
* Applies the specified env vars list to the specified containers.
*
* <p>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<? extends Env> toApply) {
List<EnvVar> targetEnv = container.getEnv();
if (targetEnv == null) {
targetEnv = new ArrayList<>();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@
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.
Expand All @@ -75,20 +76,23 @@ public class KubernetesPluginsToolingApplier implements ChePluginsApplier {
private final boolean isAuthEnabled;
private final ProjectsRootEnvVariableProvider projectsRootEnvVariableProvider;
private final ChePluginsVolumeApplier chePluginsVolumeApplier;
private final EnvVars envVars;

@Inject
public KubernetesPluginsToolingApplier(
@Named("che.workspace.sidecar.image_pull_policy") String sidecarImagePullPolicy,
@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
Expand Down Expand Up @@ -119,12 +123,6 @@ public void apply(

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<String, ComponentImpl> devfilePlugins =
k8sEnv
.getDevfile()
Expand All @@ -141,6 +139,13 @@ public void apply(
}
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<CommandImpl> pluginRelatedCommands = commandsResolver.resolve(chePlugin);

for (CheContainer container : chePlugin.getContainers()) {
Expand Down Expand Up @@ -237,6 +242,7 @@ private void addSidecar(
List<ChePluginEndpoint> containerEndpoints = k8sContainerResolver.getEndpoints();

Container k8sContainer = k8sContainerResolver.resolve();
envVars.apply(k8sContainer, pluginRelatedComponent.getEnv());
chePluginsVolumeApplier.applyVolumes(pod, k8sContainer, container.getVolumes(), k8sEnv);

String machineName = k8sContainer.getName();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@

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;
Expand All @@ -23,10 +24,8 @@
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;
Expand Down Expand Up @@ -67,7 +66,7 @@ public InternalMachineConfig resolve() throws InfrastructureException {
InternalMachineConfig machineConfig =
new InternalMachineConfig(
toServers(containerEndpoints),
component.getEnv().stream().collect(Collectors.toMap(Env::getName, Env::getValue)),
emptyMap(),
amisevsk marked this conversation as resolved.
Show resolved Hide resolved
resolveMachineAttributes(),
toWorkspaceVolumes(cheContainer));

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
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;
Expand Down Expand Up @@ -63,6 +64,7 @@
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;
Expand All @@ -79,6 +81,8 @@
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;
Expand Down Expand Up @@ -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;
Expand All @@ -122,7 +127,8 @@ public void setUp() {
MEMORY_LIMIT_MB,
false,
projectsRootEnvVariableProvider,
chePluginsVolumeApplier);
chePluginsVolumeApplier,
envVars);

Map<String, InternalMachineConfig> machines = new HashMap<>();
List<Container> containers = new ArrayList<>();
Expand Down Expand Up @@ -168,6 +174,33 @@ public void shouldProvisionPluginsCommandsToEnvironment() throws Exception {
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<Container> containerArgumentCaptor = ArgumentCaptor.forClass(Container.class);

// when
applier.apply(runtimeIdentity, internalEnvironment, singletonList(chePlugin));

// then
verify(envVars, times(2)).apply(containerArgumentCaptor.capture(), eq(component.getEnv()));
List<Container> 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 =
Expand Down Expand Up @@ -758,7 +791,8 @@ public void shouldSetJWTServerExposerAttributeIfAuthEnabled() throws Exception {
MEMORY_LIMIT_MB,
true,
projectsRootEnvVariableProvider,
chePluginsVolumeApplier);
chePluginsVolumeApplier,
envVars);

applier.apply(runtimeIdentity, internalEnvironment, singletonList(createChePlugin()));

Expand All @@ -774,7 +808,8 @@ public void shouldNotSetJWTServerExposerAttributeIfAuthEnabledButAttributeIsPres
MEMORY_LIMIT_MB,
true,
projectsRootEnvVariableProvider,
chePluginsVolumeApplier);
chePluginsVolumeApplier,
envVars);
internalEnvironment.getAttributes().put(SECURE_EXPOSER_IMPL_PROPERTY, "somethingElse");

applier.apply(runtimeIdentity, internalEnvironment, singletonList(createChePlugin()));
Expand All @@ -791,7 +826,8 @@ public void shouldSetSpecifiedImagePullPolicy() throws Exception {
MEMORY_LIMIT_MB,
true,
projectsRootEnvVariableProvider,
chePluginsVolumeApplier);
chePluginsVolumeApplier,
envVars);

applier.apply(runtimeIdentity, internalEnvironment, singletonList(createChePlugin()));

Expand All @@ -816,7 +852,8 @@ public void shouldSetNullImagePullPolicyIfValueIsNotStandard() throws Exception
MEMORY_LIMIT_MB,
true,
projectsRootEnvVariableProvider,
chePluginsVolumeApplier);
chePluginsVolumeApplier,
envVars);

applier.apply(runtimeIdentity, internalEnvironment, singletonList(createChePlugin()));

Expand Down Expand Up @@ -844,17 +881,26 @@ private ChePlugin createChePlugin() {
}

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<CheContainer> containers, List<CheContainer> 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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@
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;
Expand Down Expand Up @@ -164,18 +163,6 @@ public void shouldOverrideMemoryLimitOfASidecarIfCorrespondingWSConfigAttributeI
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);
Expand Down