diff --git a/core/che-core-api-model/src/main/java/org/eclipse/che/api/core/model/workspace/config/MachineConfig.java b/core/che-core-api-model/src/main/java/org/eclipse/che/api/core/model/workspace/config/MachineConfig.java index beeef5644d..63ea0ff9a4 100644 --- a/core/che-core-api-model/src/main/java/org/eclipse/che/api/core/model/workspace/config/MachineConfig.java +++ b/core/che-core-api-model/src/main/java/org/eclipse/che/api/core/model/workspace/config/MachineConfig.java @@ -21,9 +21,9 @@ import java.util.Map; public interface MachineConfig { /** - * Name of the attribute from {@link #getAttributes()} which if present sets memory limit of the - * machine in bytes. If memory limit is set in environment specific recipe this attribute should - * override value from recipe. + * Name of the attribute from {@link #getAttributes()} which if present defines memory limit of + * the machine in bytes. If memory limit is set in environment specific recipe this attribute used + * in {@code MachineConfig} should override value from recipe. */ String MEMORY_LIMIT_ATTRIBUTE = "memoryLimitBytes"; diff --git a/infrastructures/docker/environment/pom.xml b/infrastructures/docker/environment/pom.xml index 2b8efe8cc1..44b8454cac 100644 --- a/infrastructures/docker/environment/pom.xml +++ b/infrastructures/docker/environment/pom.xml @@ -74,6 +74,11 @@ org.eclipse.che.core che-core-api-workspace-shared + + ch.qos.logback + logback-classic + test + org.mockito mockito-core diff --git a/infrastructures/docker/environment/src/main/java/org/eclipse/che/workspace/infrastructure/docker/environment/compose/ComposeEnvironmentFactory.java b/infrastructures/docker/environment/src/main/java/org/eclipse/che/workspace/infrastructure/docker/environment/compose/ComposeEnvironmentFactory.java index 3387ca3fdb..d5525bac23 100644 --- a/infrastructures/docker/environment/src/main/java/org/eclipse/che/workspace/infrastructure/docker/environment/compose/ComposeEnvironmentFactory.java +++ b/infrastructures/docker/environment/src/main/java/org/eclipse/che/workspace/infrastructure/docker/environment/compose/ComposeEnvironmentFactory.java @@ -50,6 +50,7 @@ public class ComposeEnvironmentFactory extends InternalEnvironmentFactory 0) { attributes.put(MEMORY_LIMIT_ATTRIBUTE, String.valueOf(ramLimit)); + } else { + attributes.put(MEMORY_LIMIT_ATTRIBUTE, defaultMachineMemorySizeAttribute); } } } diff --git a/infrastructures/docker/environment/src/main/java/org/eclipse/che/workspace/infrastructure/docker/environment/dockerfile/DockerfileEnvironmentFactory.java b/infrastructures/docker/environment/src/main/java/org/eclipse/che/workspace/infrastructure/docker/environment/dockerfile/DockerfileEnvironmentFactory.java index 9b578908a4..06eabcbb3e 100644 --- a/infrastructures/docker/environment/src/main/java/org/eclipse/che/workspace/infrastructure/docker/environment/dockerfile/DockerfileEnvironmentFactory.java +++ b/infrastructures/docker/environment/src/main/java/org/eclipse/che/workspace/infrastructure/docker/environment/dockerfile/DockerfileEnvironmentFactory.java @@ -11,7 +11,9 @@ package org.eclipse.che.workspace.infrastructure.docker.environment.dockerfile; import static com.google.common.base.Preconditions.checkArgument; +import static com.google.common.base.Strings.isNullOrEmpty; import static java.lang.String.format; +import static org.eclipse.che.api.core.model.workspace.config.MachineConfig.MEMORY_LIMIT_ATTRIBUTE; import java.util.List; import java.util.Map; @@ -33,13 +35,17 @@ import org.eclipse.che.api.workspace.server.spi.environment.RecipeRetriever; public class DockerfileEnvironmentFactory extends InternalEnvironmentFactory { + private final String defaultMachineMemorySizeAttribute; + @Inject public DockerfileEnvironmentFactory( InstallerRegistry installerRegistry, RecipeRetriever recipeRetriever, MachineConfigsValidator machinesValidator, @Named("che.workspace.default_memory_mb") long defaultMachineMemorySizeMB) { - super(installerRegistry, recipeRetriever, machinesValidator, defaultMachineMemorySizeMB); + super(installerRegistry, recipeRetriever, machinesValidator); + this.defaultMachineMemorySizeAttribute = + String.valueOf(defaultMachineMemorySizeMB * 1024 * 1024); } @Override @@ -55,6 +61,19 @@ public class DockerfileEnvironmentFactory checkArgument(dockerfile != null, "Dockerfile content should not be null."); + addRamLimitAttribute(machines); + return new DockerfileEnvironment(dockerfile, recipe, machines, warnings); } + + void addRamLimitAttribute(Map machines) { + // sets default ram limit attribute if not present + for (InternalMachineConfig machineConfig : machines.values()) { + if (isNullOrEmpty(machineConfig.getAttributes().get(MEMORY_LIMIT_ATTRIBUTE))) { + machineConfig + .getAttributes() + .put(MEMORY_LIMIT_ATTRIBUTE, defaultMachineMemorySizeAttribute); + } + } + } } diff --git a/infrastructures/docker/environment/src/main/java/org/eclipse/che/workspace/infrastructure/docker/environment/dockerimage/DockerImageEnvironmentFactory.java b/infrastructures/docker/environment/src/main/java/org/eclipse/che/workspace/infrastructure/docker/environment/dockerimage/DockerImageEnvironmentFactory.java index 4c2a90e54e..851c538176 100644 --- a/infrastructures/docker/environment/src/main/java/org/eclipse/che/workspace/infrastructure/docker/environment/dockerimage/DockerImageEnvironmentFactory.java +++ b/infrastructures/docker/environment/src/main/java/org/eclipse/che/workspace/infrastructure/docker/environment/dockerimage/DockerImageEnvironmentFactory.java @@ -11,7 +11,9 @@ package org.eclipse.che.workspace.infrastructure.docker.environment.dockerimage; import static com.google.common.base.Preconditions.checkArgument; +import static com.google.common.base.Strings.isNullOrEmpty; import static java.lang.String.format; +import static org.eclipse.che.api.core.model.workspace.config.MachineConfig.MEMORY_LIMIT_ATTRIBUTE; import java.util.List; import java.util.Map; @@ -35,13 +37,17 @@ import org.eclipse.che.api.workspace.server.spi.environment.RecipeRetriever; public class DockerImageEnvironmentFactory extends InternalEnvironmentFactory { + private final String defaultMachineMemorySizeAttribute; + @Inject public DockerImageEnvironmentFactory( InstallerRegistry installerRegistry, RecipeRetriever recipeRetriever, MachineConfigsValidator machinesValidator, @Named("che.workspace.default_memory_mb") long defaultMachineMemorySizeMB) { - super(installerRegistry, recipeRetriever, machinesValidator, defaultMachineMemorySizeMB); + super(installerRegistry, recipeRetriever, machinesValidator); + this.defaultMachineMemorySizeAttribute = + String.valueOf(defaultMachineMemorySizeMB * 1024 * 1024); } @Override @@ -72,6 +78,19 @@ public class DockerImageEnvironmentFactory checkArgument(dockerImage != null, "Docker image should not be null."); + addRamLimitAttribute(machines); + return new DockerImageEnvironment(dockerImage, recipe, machines, warnings); } + + private void addRamLimitAttribute(Map machines) { + // sets default ram limit attribute if not present + for (InternalMachineConfig machineConfig : machines.values()) { + if (isNullOrEmpty(machineConfig.getAttributes().get(MEMORY_LIMIT_ATTRIBUTE))) { + machineConfig + .getAttributes() + .put(MEMORY_LIMIT_ATTRIBUTE, defaultMachineMemorySizeAttribute); + } + } + } } diff --git a/infrastructures/docker/environment/src/test/java/org/eclipse/che/workspace/infrastructure/docker/environment/compose/ComposeEnvironmentFactoryTest.java b/infrastructures/docker/environment/src/test/java/org/eclipse/che/workspace/infrastructure/docker/environment/compose/ComposeEnvironmentFactoryTest.java index a31356c1c3..9354aa9d52 100644 --- a/infrastructures/docker/environment/src/test/java/org/eclipse/che/workspace/infrastructure/docker/environment/compose/ComposeEnvironmentFactoryTest.java +++ b/infrastructures/docker/environment/src/test/java/org/eclipse/che/workspace/infrastructure/docker/environment/compose/ComposeEnvironmentFactoryTest.java @@ -40,6 +40,8 @@ import org.testng.annotations.Test; @Listeners(MockitoTestNGListener.class) public class ComposeEnvironmentFactoryTest { + private static final long DEFAULT_RAM_LIMIT_MB = 2048; + private static final long BYTES_IN_MB = 1024 * 1024; private static final String MACHINE_NAME_1 = "machine1"; private static final String MACHINE_NAME_2 = "machine2"; @@ -60,13 +62,13 @@ public class ComposeEnvironmentFactoryTest { machinesValidator, composeValidator, startStrategy, - 2048); + DEFAULT_RAM_LIMIT_MB); } @Test public void testSetsRamLimitAttributeFromComposeService() throws Exception { - final long firstMachineLimit = 3072; - final long secondMachineLimit = 1028; + final long firstMachineLimit = 3072 * BYTES_IN_MB; + final long secondMachineLimit = 1028 * BYTES_IN_MB; final Map machines = ImmutableMap.of( MACHINE_NAME_1, @@ -89,7 +91,7 @@ public class ComposeEnvironmentFactoryTest { @Test public void testDoNotOverrideRamLimitAttributeWhenItAlreadyPresent() throws Exception { - final long customRamLimit = 3072; + final long customRamLimit = 3072 * BYTES_IN_MB; final Map attributes = ImmutableMap.of(MEMORY_LIMIT_ATTRIBUTE, String.valueOf(customRamLimit)); final Map machines = @@ -108,7 +110,7 @@ public class ComposeEnvironmentFactoryTest { @Test public void testAddsMachineConfIntoEnvAndSetsRamLimAttributeWhenMachinePresentOnlyInRecipe() throws Exception { - final long customRamLimit = 2048; + final long customRamLimit = 3072 * BYTES_IN_MB; final Map machines = new HashMap<>(); final Map services = ImmutableMap.of(MACHINE_NAME_1, mockComposeService(customRamLimit)); @@ -121,6 +123,21 @@ public class ComposeEnvironmentFactoryTest { assertTrue(Arrays.equals(actual, expected)); } + @Test + public void testSetRamLimitAttributeWhenRamLimitIsMissingInRecipeAndConfig() throws Exception { + final Map machines = + ImmutableMap.of(MACHINE_NAME_1, mockInternalMachineConfig(new HashMap<>())); + final Map services = + ImmutableMap.of(MACHINE_NAME_1, mockComposeService(0)); + + composeEnvironmentFactory.addRamLimitAttribute(machines, services); + + final long[] actual = machinesRam(machines.values()); + final long[] expected = new long[actual.length]; + fill(expected, DEFAULT_RAM_LIMIT_MB * BYTES_IN_MB); + assertTrue(Arrays.equals(actual, expected)); + } + private static InternalMachineConfig mockInternalMachineConfig(Map attributes) { final InternalMachineConfig machineConfigMock = mock(InternalMachineConfig.class); when(machineConfigMock.getAttributes()).thenReturn(attributes); diff --git a/infrastructures/docker/environment/src/test/java/org/eclipse/che/workspace/infrastructure/docker/environment/dockerfile/DockerfileEnvironmentFactoryTest.java b/infrastructures/docker/environment/src/test/java/org/eclipse/che/workspace/infrastructure/docker/environment/dockerfile/DockerfileEnvironmentFactoryTest.java new file mode 100644 index 0000000000..775cd39792 --- /dev/null +++ b/infrastructures/docker/environment/src/test/java/org/eclipse/che/workspace/infrastructure/docker/environment/dockerfile/DockerfileEnvironmentFactoryTest.java @@ -0,0 +1,86 @@ +/* + * Copyright (c) 2012-2018 Red Hat, Inc. + * All rights reserved. This program and the accompanying materials + * are made available under the terms of the Eclipse Public License v1.0 + * which accompanies this distribution, and is available at + * http://www.eclipse.org/legal/epl-v10.html + * + * Contributors: + * Red Hat, Inc. - initial API and implementation + */ +package org.eclipse.che.workspace.infrastructure.docker.environment.dockerfile; + +import static java.util.Arrays.fill; +import static org.eclipse.che.api.core.model.workspace.config.MachineConfig.MEMORY_LIMIT_ATTRIBUTE; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; +import static org.testng.Assert.assertTrue; + +import com.google.common.collect.ImmutableMap; +import java.util.Arrays; +import java.util.Collection; +import java.util.Collections; +import java.util.HashMap; +import java.util.Map; +import org.eclipse.che.api.installer.server.InstallerRegistry; +import org.eclipse.che.api.workspace.server.spi.environment.InternalMachineConfig; +import org.eclipse.che.api.workspace.server.spi.environment.InternalRecipe; +import org.eclipse.che.api.workspace.server.spi.environment.MachineConfigsValidator; +import org.eclipse.che.api.workspace.server.spi.environment.RecipeRetriever; +import org.mockito.Mock; +import org.mockito.testng.MockitoTestNGListener; +import org.testng.annotations.BeforeMethod; +import org.testng.annotations.Listeners; +import org.testng.annotations.Test; + +/** @author Alexander Garagatyi */ +@Listeners(MockitoTestNGListener.class) +public class DockerfileEnvironmentFactoryTest { + + private static final long DEFAULT_RAM_LIMIT_MB = 2048; + private static final long BYTES_IN_MB = 1024 * 1024; + private static final String MACHINE_NAME = "machine"; + + @Mock private InternalRecipe recipe; + @Mock private InstallerRegistry installerRegistry; + @Mock private RecipeRetriever recipeRetriever; + @Mock private MachineConfigsValidator machinesValidator; + + private DockerfileEnvironmentFactory factory; + + @BeforeMethod + public void setUp() throws Exception { + factory = + new DockerfileEnvironmentFactory( + installerRegistry, recipeRetriever, machinesValidator, DEFAULT_RAM_LIMIT_MB); + + when(recipe.getType()).thenReturn(DockerfileEnvironment.TYPE); + when(recipe.getContent()).thenReturn(""); + } + + @Test + public void testSetRamLimitAttributeWhenRamLimitIsMissingInConfig() throws Exception { + final Map machines = + ImmutableMap.of(MACHINE_NAME, mockInternalMachineConfig(new HashMap<>())); + + factory.doCreate(recipe, machines, Collections.emptyList()); + + final long[] actual = machinesRam(machines.values()); + final long[] expected = new long[actual.length]; + fill(expected, DEFAULT_RAM_LIMIT_MB * BYTES_IN_MB); + assertTrue(Arrays.equals(actual, expected)); + } + + private static InternalMachineConfig mockInternalMachineConfig(Map attributes) { + final InternalMachineConfig machineConfigMock = mock(InternalMachineConfig.class); + when(machineConfigMock.getAttributes()).thenReturn(attributes); + return machineConfigMock; + } + + private static long[] machinesRam(Collection configs) { + return configs + .stream() + .mapToLong(m -> Long.parseLong(m.getAttributes().get(MEMORY_LIMIT_ATTRIBUTE))) + .toArray(); + } +} diff --git a/infrastructures/docker/environment/src/test/java/org/eclipse/che/workspace/infrastructure/docker/environment/dockerimage/DockerImageEnvironmentFactoryTest.java b/infrastructures/docker/environment/src/test/java/org/eclipse/che/workspace/infrastructure/docker/environment/dockerimage/DockerImageEnvironmentFactoryTest.java new file mode 100644 index 0000000000..660932ba60 --- /dev/null +++ b/infrastructures/docker/environment/src/test/java/org/eclipse/che/workspace/infrastructure/docker/environment/dockerimage/DockerImageEnvironmentFactoryTest.java @@ -0,0 +1,85 @@ +/* + * Copyright (c) 2012-2018 Red Hat, Inc. + * All rights reserved. This program and the accompanying materials + * are made available under the terms of the Eclipse Public License v1.0 + * which accompanies this distribution, and is available at + * http://www.eclipse.org/legal/epl-v10.html + * + * Contributors: + * Red Hat, Inc. - initial API and implementation + */ +package org.eclipse.che.workspace.infrastructure.docker.environment.dockerimage; + +import static java.util.Arrays.fill; +import static org.eclipse.che.api.core.model.workspace.config.MachineConfig.MEMORY_LIMIT_ATTRIBUTE; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; +import static org.testng.Assert.assertTrue; + +import com.google.common.collect.ImmutableMap; +import java.util.Arrays; +import java.util.Collection; +import java.util.Collections; +import java.util.HashMap; +import java.util.Map; +import org.eclipse.che.api.installer.server.InstallerRegistry; +import org.eclipse.che.api.workspace.server.spi.environment.InternalMachineConfig; +import org.eclipse.che.api.workspace.server.spi.environment.InternalRecipe; +import org.eclipse.che.api.workspace.server.spi.environment.MachineConfigsValidator; +import org.eclipse.che.api.workspace.server.spi.environment.RecipeRetriever; +import org.mockito.Mock; +import org.mockito.testng.MockitoTestNGListener; +import org.testng.annotations.BeforeMethod; +import org.testng.annotations.Listeners; +import org.testng.annotations.Test; + +/** @author Alexander Garagatyi */ +@Listeners(MockitoTestNGListener.class) +public class DockerImageEnvironmentFactoryTest { + private static final long DEFAULT_RAM_LIMIT_MB = 2048; + private static final long BYTES_IN_MB = 1024 * 1024; + private static final String MACHINE_NAME = "machine"; + + @Mock private InternalRecipe recipe; + @Mock private InstallerRegistry installerRegistry; + @Mock private RecipeRetriever recipeRetriever; + @Mock private MachineConfigsValidator machinesValidator; + + private DockerImageEnvironmentFactory factory; + + @BeforeMethod + public void setUp() throws Exception { + factory = + new DockerImageEnvironmentFactory( + installerRegistry, recipeRetriever, machinesValidator, DEFAULT_RAM_LIMIT_MB); + + when(recipe.getType()).thenReturn(DockerImageEnvironment.TYPE); + when(recipe.getContent()).thenReturn(""); + } + + @Test + public void testSetRamLimitAttributeWhenRamLimitIsMissingInConfig() throws Exception { + final Map machines = + ImmutableMap.of(MACHINE_NAME, mockInternalMachineConfig(new HashMap<>())); + + factory.doCreate(recipe, machines, Collections.emptyList()); + + final long[] actual = machinesRam(machines.values()); + final long[] expected = new long[actual.length]; + fill(expected, DEFAULT_RAM_LIMIT_MB * BYTES_IN_MB); + assertTrue(Arrays.equals(actual, expected)); + } + + private static InternalMachineConfig mockInternalMachineConfig(Map attributes) { + final InternalMachineConfig machineConfigMock = mock(InternalMachineConfig.class); + when(machineConfigMock.getAttributes()).thenReturn(attributes); + return machineConfigMock; + } + + private static long[] machinesRam(Collection configs) { + return configs + .stream() + .mapToLong(m -> Long.parseLong(m.getAttributes().get(MEMORY_LIMIT_ATTRIBUTE))) + .toArray(); + } +} diff --git a/infrastructures/docker/environment/src/test/resources/logback-test.xml b/infrastructures/docker/environment/src/test/resources/logback-test.xml new file mode 100644 index 0000000000..8d0bd11d3e --- /dev/null +++ b/infrastructures/docker/environment/src/test/resources/logback-test.xml @@ -0,0 +1,35 @@ + + + + + + + %-41(%date[%.15thread]) %-45([%-5level] [%.30logger{30} %L]) - %msg%n + + + + + target/log/log.log + + %-41(%date[%.15thread]) %-45([%-5level] [%.30logger{30} %L]) - %msg%n + + + + + + + + + + diff --git a/infrastructures/openshift/src/main/java/org/eclipse/che/workspace/infrastructure/openshift/environment/OpenShiftEnvironmentFactory.java b/infrastructures/openshift/src/main/java/org/eclipse/che/workspace/infrastructure/openshift/environment/OpenShiftEnvironmentFactory.java index 2ebf226ae2..f6079e436f 100644 --- a/infrastructures/openshift/src/main/java/org/eclipse/che/workspace/infrastructure/openshift/environment/OpenShiftEnvironmentFactory.java +++ b/infrastructures/openshift/src/main/java/org/eclipse/che/workspace/infrastructure/openshift/environment/OpenShiftEnvironmentFactory.java @@ -64,6 +64,7 @@ public class OpenShiftEnvironmentFactory extends InternalEnvironmentFactory 0) { attributes.put(MEMORY_LIMIT_ATTRIBUTE, String.valueOf(ramLimit)); + } else { + attributes.put(MEMORY_LIMIT_ATTRIBUTE, defaultMachineMemorySizeAttribute); } } } diff --git a/infrastructures/openshift/src/test/java/org/eclipse/che/workspace/infrastructure/openshift/environment/OpenShiftEnvironmentFactoryTest.java b/infrastructures/openshift/src/test/java/org/eclipse/che/workspace/infrastructure/openshift/environment/OpenShiftEnvironmentFactoryTest.java index 6711533c1c..b10d09e9ed 100644 --- a/infrastructures/openshift/src/test/java/org/eclipse/che/workspace/infrastructure/openshift/environment/OpenShiftEnvironmentFactoryTest.java +++ b/infrastructures/openshift/src/test/java/org/eclipse/che/workspace/infrastructure/openshift/environment/OpenShiftEnvironmentFactoryTest.java @@ -72,6 +72,7 @@ import org.testng.annotations.Test; public class OpenShiftEnvironmentFactoryTest { private static final String YAML_RECIPE = "application/x-yaml"; + private static final long BYTES_IN_MB = 1024 * 1024; private static final long DEFAULT_RAM_LIMIT_MB = 2048; public static final String MACHINE_NAME_1 = "machine1"; public static final String MACHINE_NAME_2 = "machine2"; @@ -141,8 +142,8 @@ public class OpenShiftEnvironmentFactoryTest { @Test public void testSetsRamLimitAttributeFromOpenShiftResource() throws Exception { - final long firstMachineRamLimit = 3072; - final long secondMachineRamLimit = 1024; + final long firstMachineRamLimit = 3072 * BYTES_IN_MB; + final long secondMachineRamLimit = 1024 * BYTES_IN_MB; when(machineConfig1.getAttributes()).thenReturn(new HashMap<>()); when(machineConfig2.getAttributes()).thenReturn(new HashMap<>()); final Set pods = @@ -159,13 +160,13 @@ public class OpenShiftEnvironmentFactoryTest { @Test public void testDoNotOverrideRamLimitAttributeWhenItAlreadyPresent() throws Exception { - final long customRamLimit = 3072; + final long customRamLimit = 3072 * BYTES_IN_MB; final Map attributes = ImmutableMap.of(MEMORY_LIMIT_ATTRIBUTE, String.valueOf(customRamLimit)); when(machineConfig1.getAttributes()).thenReturn(attributes); when(machineConfig2.getAttributes()).thenReturn(attributes); - final Pod pod1 = mockPod(MACHINE_NAME_1, 0); - final Pod pod2 = mockPod(MACHINE_NAME_2, 0); + final Pod pod1 = mockPod(MACHINE_NAME_1, 0L); + final Pod pod2 = mockPod(MACHINE_NAME_2, 0L); final Set pods = ImmutableSet.of(pod1, pod2); osEnvironmentFactory.addRamLimitAttribute(machines, pods); @@ -179,7 +180,7 @@ public class OpenShiftEnvironmentFactoryTest { @Test public void testAddsMachineConfIntoEnvAndSetsRamLimAttributeWhenMachinePresentOnlyInRecipe() throws Exception { - final long customRamLimit = 2048; + final long customRamLimit = 2048 * BYTES_IN_MB; final Map machines = new HashMap<>(); final Set pods = ImmutableSet.of(mockPod(MACHINE_NAME_2, customRamLimit)); @@ -191,20 +192,40 @@ public class OpenShiftEnvironmentFactoryTest { assertTrue(Arrays.equals(actual, expected)); } - private static Pod mockPod(String machineName, long ramLimit) { + @Test + public void testSetsDefaultRamLimitAttributeIfRamLimitIsMissingInRecipeAndConfig() + throws Exception { + final long firstMachineRamLimit = 3072 * BYTES_IN_MB; + when(machineConfig1.getAttributes()).thenReturn(new HashMap<>()); + when(machineConfig2.getAttributes()).thenReturn(new HashMap<>()); + final Set pods = + ImmutableSet.of( + mockPod(MACHINE_NAME_1, firstMachineRamLimit), mockPod(MACHINE_NAME_2, null)); + + osEnvironmentFactory.addRamLimitAttribute(machines, pods); + + final long[] actual = machinesRam(machines.values()); + final long[] expected = new long[] {firstMachineRamLimit, DEFAULT_RAM_LIMIT_MB * BYTES_IN_MB}; + assertTrue(Arrays.equals(actual, expected)); + } + + /** If provided {@code ramLimit} is {@code null} ram limit won't be set in POD */ + private static Pod mockPod(String machineName, Long ramLimit) { final String containerName = "container_" + machineName; final Container containerMock = mock(Container.class); - final ResourceRequirements resourcesMock = mock(ResourceRequirements.class); - final Quantity quantityMock = mock(Quantity.class); final Pod podMock = mock(Pod.class); final PodSpec specMock = mock(PodSpec.class); final ObjectMeta metadataMock = mock(ObjectMeta.class); when(podMock.getSpec()).thenReturn(specMock); when(podMock.getMetadata()).thenReturn(metadataMock); - when(quantityMock.getAmount()).thenReturn(String.valueOf(ramLimit)); - when(resourcesMock.getLimits()).thenReturn(ImmutableMap.of("memory", quantityMock)); + if (ramLimit != null) { + final Quantity quantityMock = mock(Quantity.class); + final ResourceRequirements resourcesMock = mock(ResourceRequirements.class); + when(quantityMock.getAmount()).thenReturn(String.valueOf(ramLimit)); + when(resourcesMock.getLimits()).thenReturn(ImmutableMap.of("memory", quantityMock)); + when(containerMock.getResources()).thenReturn(resourcesMock); + } when(containerMock.getName()).thenReturn(containerName); - when(containerMock.getResources()).thenReturn(resourcesMock); when(metadataMock.getAnnotations()) .thenReturn( ImmutableMap.of(format(MACHINE_NAME_ANNOTATION_FMT, containerName), machineName)); diff --git a/multiuser/api/che-multiuser-api-resource/src/main/java/org/eclipse/che/multiuser/resource/api/usage/tracker/EnvironmentRamCalculator.java b/multiuser/api/che-multiuser-api-resource/src/main/java/org/eclipse/che/multiuser/resource/api/usage/tracker/EnvironmentRamCalculator.java index 50ff955925..66d0ee8afc 100644 --- a/multiuser/api/che-multiuser-api-resource/src/main/java/org/eclipse/che/multiuser/resource/api/usage/tracker/EnvironmentRamCalculator.java +++ b/multiuser/api/che-multiuser-api-resource/src/main/java/org/eclipse/che/multiuser/resource/api/usage/tracker/EnvironmentRamCalculator.java @@ -20,6 +20,7 @@ import org.eclipse.che.api.core.ServerException; import org.eclipse.che.api.core.ValidationException; import org.eclipse.che.api.core.model.workspace.Runtime; import org.eclipse.che.api.core.model.workspace.config.Environment; +import org.eclipse.che.api.core.model.workspace.config.MachineConfig; 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.InternalEnvironmentFactory; @@ -50,7 +51,8 @@ public class EnvironmentRamCalculator { .getMachines() .values() .stream() - .mapToLong(m -> Long.parseLong(m.getAttributes().get(MEMORY_LIMIT_ATTRIBUTE))) + .mapToLong( + m -> parseMemoryAttributeValue(m.getAttributes().get(MEMORY_LIMIT_ATTRIBUTE))) .sum() / BYTES_TO_MEGABYTES_DIVIDER; } catch (InfrastructureException | ValidationException | NotFoundException ex) { @@ -68,11 +70,31 @@ public class EnvironmentRamCalculator { .getMachines() .values() .stream() - .mapToLong(m -> Long.parseLong(m.getAttributes().get(MEMORY_LIMIT_ATTRIBUTE))) + .mapToLong( + m -> parseMemoryAttributeValue(m.getAttributes().get(MEMORY_LIMIT_ATTRIBUTE))) .sum() / BYTES_TO_MEGABYTES_DIVIDER; } + /** + * Parse {@link MachineConfig#MEMORY_LIMIT_ATTRIBUTE} value to {@code Long}. + * + * @param attributeValue value of {@link MachineConfig#MEMORY_LIMIT_ATTRIBUTE} attribute from + * machine config or runtime + * @return long value parsed from provided string attribute value or {@code 0} if {@code null} is + * provided + * @throws NumberFormatException if provided value is neither {@code null} nor valid stringified + * long + * @see Long#parseLong(String) + */ + private long parseMemoryAttributeValue(String attributeValue) { + if (attributeValue == null) { + return 0; + } else { + return Long.parseLong(attributeValue); + } + } + private InternalEnvironment getInternalEnvironment(Environment environment) throws InfrastructureException, ValidationException, NotFoundException { final String recipeType = environment.getRecipe().getType(); diff --git a/multiuser/api/che-multiuser-api-resource/src/test/java/org/eclipse/che/multiuser/resource/api/usage/tracker/EnvironmentRamCalculatorTest.java b/multiuser/api/che-multiuser-api-resource/src/test/java/org/eclipse/che/multiuser/resource/api/usage/tracker/EnvironmentRamCalculatorTest.java index 2c721783a0..f561ed67e0 100644 --- a/multiuser/api/che-multiuser-api-resource/src/test/java/org/eclipse/che/multiuser/resource/api/usage/tracker/EnvironmentRamCalculatorTest.java +++ b/multiuser/api/che-multiuser-api-resource/src/test/java/org/eclipse/che/multiuser/resource/api/usage/tracker/EnvironmentRamCalculatorTest.java @@ -16,6 +16,7 @@ import static org.mockito.Mockito.when; import static org.testng.Assert.assertEquals; import com.google.common.collect.ImmutableMap; +import java.util.Collections; import org.eclipse.che.api.core.ServerException; import org.eclipse.che.api.core.model.workspace.Runtime; import org.eclipse.che.api.core.model.workspace.config.Environment; @@ -84,6 +85,18 @@ public class EnvironmentRamCalculatorTest { assertEquals(ram, 2560); } + @Test + public void testCalculatesRamOfEnvironmentWhenSomeMachineConfigHasNoAttribute() throws Exception { + when(machineConfig1.getAttributes()).thenReturn(Collections.emptyMap()); + when(machineConfig2.getAttributes()) + .thenReturn(ImmutableMap.of(MEMORY_LIMIT_ATTRIBUTE, "536870912")); + when(recipeMock.getType()).thenReturn(RECIPE_TYPE); + + final long ram = envRamCalculator.calculate(environment); + + assertEquals(ram, 512); + } + @Test(expectedExceptions = ServerException.class) public void testThrowServerExceptionWhenNoEnvFactoryForGivenRecipeTypeFound() throws Exception { when(recipeMock.getType()).thenReturn("unsupported"); @@ -100,4 +113,14 @@ public class EnvironmentRamCalculatorTest { assertEquals(ram, 1536); } + + @Test + public void testCalculatesRamOfRuntimeWhenSomeMachineHasNoAttribute() throws Exception { + when(machine1.getAttributes()).thenReturn(ImmutableMap.of(MEMORY_LIMIT_ATTRIBUTE, "805306368")); + when(machine2.getAttributes()).thenReturn(Collections.emptyMap()); + + final long ram = envRamCalculator.calculate(runtime); + + assertEquals(ram, 768); + } } diff --git a/wsmaster/che-core-api-workspace/src/main/java/org/eclipse/che/api/workspace/server/spi/environment/InternalEnvironmentFactory.java b/wsmaster/che-core-api-workspace/src/main/java/org/eclipse/che/api/workspace/server/spi/environment/InternalEnvironmentFactory.java index ce3c6a989b..0288178a59 100644 --- a/wsmaster/che-core-api-workspace/src/main/java/org/eclipse/che/api/workspace/server/spi/environment/InternalEnvironmentFactory.java +++ b/wsmaster/che-core-api-workspace/src/main/java/org/eclipse/che/api/workspace/server/spi/environment/InternalEnvironmentFactory.java @@ -10,9 +10,6 @@ */ package org.eclipse.che.api.workspace.server.spi.environment; -import static com.google.common.base.Strings.isNullOrEmpty; -import static org.eclipse.che.api.core.model.workspace.config.MachineConfig.MEMORY_LIMIT_ATTRIBUTE; - import com.google.common.annotations.VisibleForTesting; import java.util.ArrayList; import java.util.HashMap; @@ -50,18 +47,14 @@ public abstract class InternalEnvironmentFactory private final InstallerRegistry installerRegistry; private final RecipeRetriever recipeRetriever; private final MachineConfigsValidator machinesValidator; - private final String defaultMachineMemorySizeAttribute; public InternalEnvironmentFactory( InstallerRegistry installerRegistry, RecipeRetriever recipeRetriever, - MachineConfigsValidator machinesValidator, - long defaultMachineMemorySizeMB) { + MachineConfigsValidator machinesValidator) { this.installerRegistry = installerRegistry; this.recipeRetriever = recipeRetriever; this.machinesValidator = machinesValidator; - this.defaultMachineMemorySizeAttribute = - String.valueOf(defaultMachineMemorySizeMB * 1024 * 1024); } /** @@ -113,23 +106,16 @@ public abstract class InternalEnvironmentFactory machinesValidator.validate(machines); - final T environment = doCreate(recipe, machines, warnings); - - // sets default ram limit attribute if not present - for (InternalMachineConfig machineConfig : environment.getMachines().values()) { - if (isNullOrEmpty(machineConfig.getAttributes().get(MEMORY_LIMIT_ATTRIBUTE))) { - machineConfig - .getAttributes() - .put(MEMORY_LIMIT_ATTRIBUTE, defaultMachineMemorySizeAttribute); - } - } - return environment; + return doCreate(recipe, machines, warnings); } /** - * Implementation validates downloaded recipe and creates specific InternalEnvironment. Returned - * InternalEnvironment must contains all machine that are defined in recipe and in source machine - * collection. + * Implementation validates downloaded recipe and creates specific InternalEnvironment. + * + *

Returned InternalEnvironment must contains all machine that are defined in recipe and in + * source machines collection. Also, if memory limitation is supported, it may add memory limit + * attribute {@link MachineConfig#MEMORY_LIMIT_ATTRIBUTE} from recipe or configured system-wide + * default value. * * @param recipe downloaded recipe * @param machines machines configuration diff --git a/wsmaster/che-core-api-workspace/src/test/java/org/eclipse/che/api/workspace/server/spi/environment/InternalEnvironmentFactoryTest.java b/wsmaster/che-core-api-workspace/src/test/java/org/eclipse/che/api/workspace/server/spi/environment/InternalEnvironmentFactoryTest.java index 803d53c908..211c42fa59 100644 --- a/wsmaster/che-core-api-workspace/src/test/java/org/eclipse/che/api/workspace/server/spi/environment/InternalEnvironmentFactoryTest.java +++ b/wsmaster/che-core-api-workspace/src/test/java/org/eclipse/che/api/workspace/server/spi/environment/InternalEnvironmentFactoryTest.java @@ -25,7 +25,6 @@ import static org.mockito.Mockito.spy; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import static org.testng.Assert.assertEquals; -import static org.testng.Assert.assertTrue; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; @@ -176,21 +175,6 @@ public class InternalEnvironmentFactoryTest { ImmutableMap.of("serverWithoutProtocol", normalizedServer, "udpServer", udpServer)); } - @Test - public void testSetsDefaultRamLimitAttributeToMachineThatDoesNotContainIt() throws Exception { - final Map attributes = new HashMap<>(); - final InternalEnvironment internalEnv = mock(InternalEnvironment.class); - 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(attributes); - - environmentFactory.create(mock(Environment.class)); - - assertTrue(attributes.containsKey(MEMORY_LIMIT_ATTRIBUTE)); - assertEquals(attributes.get(MEMORY_LIMIT_ATTRIBUTE), String.valueOf(RAM_LIMIT * 2 << 19)); - } - @Test public void testDoNotOverrideRamLimitAttributeWhenMachineAlreadyContainsIt() throws Exception { final String ramLimit = "2147483648"; @@ -216,7 +200,7 @@ public class InternalEnvironmentFactoryTest { InstallerRegistry installerRegistry, RecipeRetriever recipeRetriever, MachineConfigsValidator machinesValidator) { - super(installerRegistry, recipeRetriever, machinesValidator, RAM_LIMIT); + super(installerRegistry, recipeRetriever, machinesValidator); } @Override