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

Filter workspace containers like tools and user's #12386

Merged
merged 8 commits into from
Jan 30, 2019
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@
import static org.eclipse.che.api.core.model.workspace.config.MachineConfig.MEMORY_REQUEST_ATTRIBUTE;
import static org.eclipse.che.api.core.model.workspace.config.ServerConfig.SECURE_SERVER_COOKIES_AUTH_ENABLED_ATTRIBUTE;
import static org.eclipse.che.api.core.model.workspace.config.ServerConfig.UNSECURED_PATHS_ATTRIBUTE;
import static org.eclipse.che.api.workspace.shared.Constants.CONTAINER_SOURCE_ATTRIBUTE;
import static org.eclipse.che.api.workspace.shared.Constants.TOOL_CONTAINER_SOURCE;
import static org.eclipse.che.commons.lang.NameGenerator.generate;
import static org.eclipse.che.workspace.infrastructure.kubernetes.Constants.CHE_ORIGINAL_NAME_LABEL;
import static org.eclipse.che.workspace.infrastructure.kubernetes.server.KubernetesServerExposer.SERVER_PREFIX;
Expand Down Expand Up @@ -126,7 +128,9 @@ public JwtProxyProvisioner(
MEMORY_LIMIT_ATTRIBUTE,
Long.toString(memoryLimitLong),
MEMORY_REQUEST_ATTRIBUTE,
Long.toString(memoryLimitLong));
Long.toString(memoryLimitLong),
CONTAINER_SOURCE_ATTRIBUTE,
TOOL_CONTAINER_SOURCE);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be nice to have the corresponding check in JwtProxyProvisionerTest

}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@
import static java.lang.String.format;
import static java.util.Collections.emptyList;
import static org.eclipse.che.api.core.model.workspace.config.Command.WORKING_DIRECTORY_ATTRIBUTE;
import static org.eclipse.che.api.workspace.shared.Constants.CONTAINER_SOURCE_ATTRIBUTE;
import static org.eclipse.che.api.workspace.shared.Constants.TOOL_CONTAINER_SOURCE;
import static org.eclipse.che.workspace.infrastructure.kubernetes.server.secure.SecureServerExposerFactoryProvider.SECURE_EXPOSER_IMPL_PROPERTY;

import com.google.common.annotations.Beta;
Expand Down Expand Up @@ -194,6 +196,7 @@ private void addSidecar(
.build();

InternalMachineConfig machineConfig = machineResolver.resolve();
machineConfig.getAttributes().put(CONTAINER_SOURCE_ATTRIBUTE, TOOL_CONTAINER_SOURCE);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be nice to have the corresponding test in KubernetesPluginsToolingApplierTest

kubernetesEnvironment.getMachines().put(machineName, machineConfig);

sidecarRelatedCommands.forEach(c -> c.getAttributes().put("machineName", machineName));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -162,5 +162,14 @@ public final class Constants {

public static final String NO_ENVIRONMENT_RECIPE_TYPE = "no-environment";

/** Attribute to mark source of the container. */
public static final String CONTAINER_SOURCE_ATTRIBUTE = "source";

/** Mark containers applied to workspace with help recipe definition. */
public static final String RECIPE_CONTAINER_SOURCE = "recipe";

/** Mark containers created workspace api like tooling for user development. */
public static final String TOOL_CONTAINER_SOURCE = "tool";

private Constants() {}
}
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,9 @@
*/
package org.eclipse.che.api.workspace.server.spi.environment;

import static org.eclipse.che.api.workspace.shared.Constants.CONTAINER_SOURCE_ATTRIBUTE;
import static org.eclipse.che.api.workspace.shared.Constants.RECIPE_CONTAINER_SOURCE;

import com.google.common.annotations.VisibleForTesting;
import java.util.ArrayList;
import java.util.HashMap;
Expand Down Expand Up @@ -113,7 +116,14 @@ public T create(@Nullable final Environment sourceEnv)
machinesValidator.validate(machines);
}

return doCreate(recipe, machines, warnings);
T internalEnv = doCreate(recipe, machines, warnings);

internalEnv
.getMachines()
.values()
.forEach(m -> m.getAttributes().put(CONTAINER_SOURCE_ATTRIBUTE, RECIPE_CONTAINER_SOURCE));

return internalEnv;
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@
import static java.util.Collections.singletonList;
import static java.util.Collections.singletonMap;
import static org.eclipse.che.api.core.model.workspace.config.MachineConfig.MEMORY_LIMIT_ATTRIBUTE;
import static org.eclipse.che.api.workspace.shared.Constants.CONTAINER_SOURCE_ATTRIBUTE;
import static org.eclipse.che.api.workspace.shared.Constants.RECIPE_CONTAINER_SOURCE;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyList;
import static org.mockito.ArgumentMatchers.anyMap;
Expand Down Expand Up @@ -196,13 +198,70 @@ public void testDoNotOverrideRamLimitAttributeWhenMachineAlreadyContainsIt() thr
final InternalMachineConfig machine = mock(InternalMachineConfig.class);
when(environmentFactory.doCreate(any(), any(), any())).thenReturn(internalEnv);
when(internalEnv.getMachines()).thenReturn(ImmutableMap.of("testMachine", machine));
when(machine.getAttributes()).thenReturn(ImmutableMap.of(MEMORY_LIMIT_ATTRIBUTE, ramLimit));

Map<String, String> machineAttr = new HashMap<>();
machineAttr.put(MEMORY_LIMIT_ATTRIBUTE, ramLimit);
when(machine.getAttributes()).thenReturn(machineAttr);

environmentFactory.create(mock(Environment.class));

assertEquals(machine.getAttributes().get(MEMORY_LIMIT_ATTRIBUTE), ramLimit);
}

@Test
public void testApplyContainerSourceAttributeToTheMachineSpecifiedInEnv() throws Exception {
// given
final Environment sourceEnv = mock(Environment.class);

MachineConfigImpl machineConfig = mock(MachineConfigImpl.class);
final Map<String, MachineConfigImpl> machineConfigMap =
ImmutableMap.of("envMachine", machineConfig);
doReturn(machineConfigMap).when(sourceEnv).getMachines();

when(environmentFactory.doCreate(any(), any(), any()))
.thenAnswer(
invocation -> {
Map<String, InternalMachineConfig> envMachines = invocation.getArgument(1);

final InternalEnvironment internalEnv = mock(InternalEnvironment.class);
when(internalEnv.getMachines()).thenReturn(envMachines);
return internalEnv;
});

// when
InternalEnvironment resultEnv = environmentFactory.create(sourceEnv);

// then
assertEquals(
resultEnv.getMachines().get("envMachine").getAttributes().get(CONTAINER_SOURCE_ATTRIBUTE),
RECIPE_CONTAINER_SOURCE);
}

@Test
public void testApplyContainerSourceAttributeToTheMachineThatBecomesFromRecipe()
throws Exception {
// given
final Environment sourceEnv = mock(Environment.class);

final InternalEnvironment internalEnv = mock(InternalEnvironment.class);
final InternalMachineConfig internalMachine = new InternalMachineConfig();
when(internalEnv.getMachines()).thenReturn(ImmutableMap.of("internalMachine", internalMachine));

when(environmentFactory.doCreate(any(), any(), any())).thenReturn(internalEnv);

// when
InternalEnvironment resultEnv = environmentFactory.create(sourceEnv);

// then
assertEquals(
resultEnv
.getMachines()
.get("internalMachine")
.getAttributes()
.get(CONTAINER_SOURCE_ATTRIBUTE),
RECIPE_CONTAINER_SOURCE);
}

private InstallerImpl createInstaller(String id, String script) {
return new InstallerImpl(id, "any", "any", "any", emptyList(), emptyMap(), script, emptyMap());
}
Expand Down