From a0cdf3f0188eb75b1b319b1746478adb34f49239 Mon Sep 17 00:00:00 2001 From: Max Shaposhnik Date: Wed, 5 Feb 2020 17:02:13 +0200 Subject: [PATCH 01/29] Add CPU limits part 1; --- .../model/workspace/config/MachineConfig.java | 14 +++ .../kubernetes/util/Containers.java | 95 +++++++++++++++++++ .../kubernetes/util/KubernetesSize.java | 36 ++++++- .../wsplugins/K8sContainerResolver.java | 55 ++++++++++- .../KubernetesPluginsToolingApplier.java | 14 ++- .../kubernetes/wsplugins/MachineResolver.java | 52 ++++++++++ .../wsplugins/MachineResolverBuilder.java | 39 ++++++-- .../wsplugins/MachineResolverTest.java | 1 + .../server/wsplugins/model/CheContainer.java | 51 ++++++++++ 9 files changed, 347 insertions(+), 10 deletions(-) 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 d287b1c5114..15ec581854d 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 @@ -38,6 +38,20 @@ public interface MachineConfig { */ String MEMORY_REQUEST_ATTRIBUTE = "memoryRequestBytes"; + + /** + * Name of the attribute from {@link #getAttributes()} which if present defines CPU limit of + * the machine in millicores. + */ + String CPU_LIMIT_ATTRIBUTE = "cpuLimitMillicores"; + + /** + * Name of the attribute from {@link #getAttributes()} which if present defines requested CPU + * allocation of the machine in millicores. + */ + String CPU_REQUEST_ATTRIBUTE = "cpuRequestMillicores"; + + /** * Name of the attribute from {@link #getAttributes()} which, if present, defines the entrypoint * command to be executed in the machine/container. diff --git a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/util/Containers.java b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/util/Containers.java index 78086b975f1..05d95c7baf6 100644 --- a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/util/Containers.java +++ b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/util/Containers.java @@ -116,4 +116,99 @@ public static void addRamRequest(Container container, String limitInK8sNotion) { container.setResources( resourceBuilder.addToRequests("memory", new Quantity(limitInK8sNotion)).build()); } + + /** + * Returns the CPU limit in cores, if it is present in given container otherwise 0 will be + * returned. + */ + public static float getCpuLimit(Container container) { + final ResourceRequirements resources = container.getResources(); + final Quantity quantity; + if (resources != null + && resources.getLimits() != null + && (quantity = resources.getLimits().get("cpu")) != null + && quantity.getAmount() != null) { + return KubernetesSize.toCores(quantity.getAmount()); + } + return 0; + } + + /** + * Sets given CPU limit in cores to specified container. Note if the container already contains a + * CPU limit, it will be overridden, other resources won't be affected. + */ + public static void addCpuLimit(Container container, float cpuLimit) { + final ResourceRequirementsBuilder resourceBuilder; + if (container.getResources() != null) { + resourceBuilder = new ResourceRequirementsBuilder(container.getResources()); + } else { + resourceBuilder = new ResourceRequirementsBuilder(); + } + container.setResources( + resourceBuilder.addToLimits("memory", new Quantity(Float.toString(cpuLimit))).build()); + } + + /** + * Sets given CPU limit in kubernetes notion to specified container. Note if the container already + * contains a CPU limit, it will be overridden, other resources won't be affected. + */ + public static void addCpuLimit(Container container, String limitInK8sNotion) { + final ResourceRequirementsBuilder resourceBuilder; + if (container.getResources() != null) { + resourceBuilder = new ResourceRequirementsBuilder(container.getResources()); + } else { + resourceBuilder = new ResourceRequirementsBuilder(); + } + container.setResources( + resourceBuilder.addToLimits("cpu", new Quantity(limitInK8sNotion)).build()); + } + + + /** + * Returns the CPU request in bytes, if it is present in given container otherwise 0 will be + * returned. + */ + public static float getCpuRequest(Container container) { + final ResourceRequirements resources = container.getResources(); + final Quantity quantity; + if (resources != null + && resources.getRequests() != null + && (quantity = resources.getRequests().get("cpu")) != null + && quantity.getAmount() != null) { + return KubernetesSize.toCores(quantity.getAmount()); + } + return 0; + } + + /** + * Sets given CPU request in bytes to specified container. Note if the container already contains + * a CPU limit, it will be overridden, other resources won't be affected. + */ + public static void addCpuRequest(Container container, float cpuRequest) { + final ResourceRequirementsBuilder resourceBuilder; + if (container.getResources() != null) { + resourceBuilder = new ResourceRequirementsBuilder(container.getResources()); + } else { + resourceBuilder = new ResourceRequirementsBuilder(); + } + container.setResources( + resourceBuilder.addToRequests("cpu", new Quantity(Float.toString(cpuRequest))).build()); + } + + /** + * Sets given CPU request in kubernetes notion to specified container. Note if the container + * already contains a CPU request, it will be overridden, other resources won't be affected. + */ + public static void addCpuRequest(Container container, String limitInK8sNotion) { + final ResourceRequirementsBuilder resourceBuilder; + if (container.getResources() != null) { + resourceBuilder = new ResourceRequirementsBuilder(container.getResources()); + } else { + resourceBuilder = new ResourceRequirementsBuilder(); + } + container.setResources( + resourceBuilder.addToRequests("cpu", new Quantity(limitInK8sNotion)).build()); + } + + } diff --git a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/util/KubernetesSize.java b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/util/KubernetesSize.java index 7d084a2e822..d6bc3da2e87 100644 --- a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/util/KubernetesSize.java +++ b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/util/KubernetesSize.java @@ -37,9 +37,12 @@ public class KubernetesSize { private static final long PI = TI * KI; private static final long EI = PI * KI; - private static final Pattern HUMAN_SIZE_PATTERN = + private static final Pattern HUMAN_SIZE_MEMORY_PATTERN = Pattern.compile("^([-+]?[0-9]*\\.?[0-9]+([eE][-+]?[0-9]+)?)\\s*(\\S+)?$"); + private static final Pattern HUMAN_SIZE_CPU_PATTERN = + Pattern.compile("^([0-9]*\\.?[0-9]+)(m)?$"); + /** * Converts memory in Kubernetes format to bytes. * @@ -68,7 +71,7 @@ public class KubernetesSize { */ public static long toBytes(String sizeString) { final Matcher matcher; - if ((matcher = HUMAN_SIZE_PATTERN.matcher(sizeString)).matches()) { + if ((matcher = HUMAN_SIZE_MEMORY_PATTERN.matcher(sizeString)).matches()) { final float size = Float.parseFloat(matcher.group(1)); final String suffix = matcher.group(3); if (suffix == null) { @@ -124,4 +127,33 @@ public static String toKubeSize(long bytes, boolean si) { final float size = bytes / (float) Math.pow(multiplier, e); return String.format((size % 1.0f == 0) ? "%.0f%s" : "%.1f%s", size, unit); } + + + /** + * Converts CPU resource in Kubernetes format to cores. + * + *

Format: "< number >< m>"
+ * + *

+ * + * @throws IllegalArgumentException if specified string can not be parsed + */ + public static float toCores(String cpuString) { + final Matcher matcher; + if ((matcher = HUMAN_SIZE_CPU_PATTERN.matcher(cpuString)).matches()) { + final float size = Float.parseFloat(matcher.group(1)); + final String suffix = matcher.group(2); + if (suffix == null) { + return size; + } + switch (suffix.toLowerCase()) { + case "m": + return size / K; + } + } + throw new IllegalArgumentException("Invalid Kubernetes CPU size format provided: " + cpuString); + } } diff --git a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/wsplugins/K8sContainerResolver.java b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/wsplugins/K8sContainerResolver.java index bfe1f028f68..29bccc28acf 100644 --- a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/wsplugins/K8sContainerResolver.java +++ b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/wsplugins/K8sContainerResolver.java @@ -67,6 +67,9 @@ public Container resolve() throws InfrastructureException { .build(); provisionMemoryLimit(container, cheContainer); + provisionMemoryRequest(container, cheContainer); + provisionCpuLimit(container, cheContainer); + provisionCpuRequest(container, cheContainer); return container; } @@ -86,7 +89,57 @@ private void provisionMemoryLimit(Container container, CheContainer cheContainer memoryLimit, e.getMessage())); } Containers.addRamLimit(container, memoryLimit); - Containers.addRamRequest(container, memoryLimit); + } + + private void provisionMemoryRequest(Container container, CheContainer cheContainer) + throws InfrastructureException { + String memoryRequest = cheContainer.getMemoryRequest(); + if (isNullOrEmpty(memoryRequest)) { + return; + } + try { + KubernetesSize.toBytes(memoryRequest); + } catch (IllegalArgumentException e) { + throw new InfrastructureException( + format( + "Sidecar memory request field contains illegal value '%s'. Error: '%s'", + memoryRequest, e.getMessage())); + } + Containers.addRamRequest(container, memoryRequest); + } + + private void provisionCpuLimit(Container container, CheContainer cheContainer) + throws InfrastructureException { + String cpuLimit = cheContainer.getCpuLimit(); + if (isNullOrEmpty(cpuLimit)) { + return; + } + try { + KubernetesSize.toCores(cpuLimit); + } catch (IllegalArgumentException e) { + throw new InfrastructureException( + format( + "Sidecar CPU limit field contains illegal value '%s'. Error: '%s'", + cpuLimit, e.getMessage())); + } + Containers.addCpuLimit(container, cpuLimit); + } + + private void provisionCpuRequest(Container container, CheContainer cheContainer) + throws InfrastructureException { + String cpuRequest = cheContainer.getCpuRequest(); + if (isNullOrEmpty(cpuRequest)) { + return; + } + try { + KubernetesSize.toCores(cpuRequest); + } catch (IllegalArgumentException e) { + throw new InfrastructureException( + format( + "Sidecar CPU request field contains illegal value '%s'. Error: '%s'", + cpuRequest, e.getMessage())); + } + Containers.addCpuRequest(container, cpuRequest); } private List getContainerPorts() { 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 df87fa78729..6bdcd5ccb9f 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 @@ -71,7 +71,10 @@ public class KubernetesPluginsToolingApplier implements ChePluginsApplier { private static final String CHE_WORKSPACE_POD = "che-workspace-pod"; private final String defaultSidecarMemoryLimitBytes; + private final String defaultSidecarMemoryRequestBytes; private final String sidecarImagePullPolicy; + private final String defaultSidecarCpuLimitCores; + private final String defaultSidecarCpuRequestCores; private final boolean isAuthEnabled; private final ProjectsRootEnvVariableProvider projectsRootEnvVariableProvider; private final ChePluginsVolumeApplier chePluginsVolumeApplier; @@ -80,10 +83,16 @@ public class KubernetesPluginsToolingApplier implements ChePluginsApplier { public KubernetesPluginsToolingApplier( @Named("che.workspace.sidecar.image_pull_policy") String sidecarImagePullPolicy, @Named("che.workspace.sidecar.default_memory_limit_mb") long defaultSidecarMemoryLimitMB, + @Named("che.workspace.sidecar.default_memory_request_mb") long defaultSidecarMemoryRequestMB, + @Named("che.workspace.sidecar.default_cpu_limit_cores") String defaultSidecarCpuLimitCores, + @Named("che.workspace.sidecar.default_cpu_request_cores") String defaultSidecarCpuRequestCores, @Named("che.agents.auth_enabled") boolean isAuthEnabled, ProjectsRootEnvVariableProvider projectsRootEnvVariableProvider, ChePluginsVolumeApplier chePluginsVolumeApplier) { this.defaultSidecarMemoryLimitBytes = String.valueOf(defaultSidecarMemoryLimitMB * 1024 * 1024); + this.defaultSidecarMemoryRequestBytes = String.valueOf(defaultSidecarMemoryRequestMB * 1024 * 1024); + this.defaultSidecarCpuLimitCores = defaultSidecarCpuLimitCores; + this.defaultSidecarCpuRequestCores = defaultSidecarCpuRequestCores; this.isAuthEnabled = isAuthEnabled; this.sidecarImagePullPolicy = validImagePullPolicies.contains(sidecarImagePullPolicy) ? sidecarImagePullPolicy : null; @@ -248,7 +257,10 @@ private void addSidecar( .setCheContainer(container) .setContainer(k8sContainer) .setContainerEndpoints(containerEndpoints) - .setDefaultSidecarMemorySizeAttribute(defaultSidecarMemoryLimitBytes) + .setDefaultSidecarMemoryLimitAttribute(defaultSidecarMemoryLimitBytes) + .setDefaultSidecarMemoryRequestAttribute(defaultSidecarMemoryRequestBytes) + .setDefaultSidecarCpuLimitAttribute(defaultSidecarCpuLimitCores) + .setDefaultSidecarCpuRequestAttribute(defaultSidecarCpuRequestCores) .setProjectsRootPathEnvVar(projectsRootEnvVariableProvider.get(runtimeIdentity)) .setComponent(pluginRelatedComponent) .build(); 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 821fac73d56..902d64fcfa5 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 @@ -44,6 +44,9 @@ public class MachineResolver { private final Container container; private final CheContainer cheContainer; private final String defaultSidecarMemoryLimitBytes; + private final String defaultSidecarMemoryRequestBytes; + private final String defaultSidecarCpuLimitCores; + private final String defaultSidecarCpuRequestCores; private final List containerEndpoints; private final Pair projectsRootPathEnvVar; private final Component component; @@ -53,11 +56,17 @@ public MachineResolver( Container container, CheContainer cheContainer, String defaultSidecarMemoryLimitBytes, + String defaultSidecarMemoryRequestBytes, + String defaultSidecarCpuLimitCores, + String defaultSidecarCpuRequestCores, List containerEndpoints, Component component) { this.container = container; this.cheContainer = cheContainer; this.defaultSidecarMemoryLimitBytes = defaultSidecarMemoryLimitBytes; + this.defaultSidecarMemoryRequestBytes = defaultSidecarMemoryRequestBytes; + this.defaultSidecarCpuLimitCores = defaultSidecarCpuLimitCores; + this.defaultSidecarCpuRequestCores = defaultSidecarCpuRequestCores; this.containerEndpoints = containerEndpoints; this.projectsRootPathEnvVar = projectsRootPathEnvVar; this.component = component; @@ -72,6 +81,7 @@ public InternalMachineConfig resolve() throws InfrastructureException { toWorkspaceVolumes(cheContainer)); normalizeMemory(container, machineConfig); + normalizeCpu(container, machineConfig); return machineConfig; } @@ -99,6 +109,48 @@ private void normalizeMemory(Container container, InternalMachineConfig machineC MEMORY_LIMIT_ATTRIBUTE, Long.toString(KubernetesSize.toBytes(overriddenSidecarMemLimit))); } + + long ramRequest = Containers.getRamRequest(container); + if (ramRequest == 0) { + machineConfig.getAttributes().put(MEMORY_LIMIT_ATTRIBUTE, defaultSidecarMemoryRequestBytes); + } +// String overriddenSidecarMemRequest = component.getRamRequest(); +// if (!isNullOrEmpty(overriddenSidecarMemRequest)) { +// machineConfig +// .getAttributes() +// .put( +// MEMORY_REQUEST_ATTRIBUTE, +// Long.toString(KubernetesSize.toBytes(overriddenSidecarMemRequest))); +// } + } + + + private void normalizeCpu(Container container, InternalMachineConfig machineConfig) { + float cpuLimit = Containers.getCpuLimit(container); + if (cpuLimit == 0) { + machineConfig.getAttributes().put(MEMORY_LIMIT_ATTRIBUTE, defaultSidecarCpuLimitCores); + } +// String overriddenSidecarCpuLimit = component.getCpuLimit(); +// if (!isNullOrEmpty(overriddenSidecarCpuLimit)) { +// machineConfig +// .getAttributes() +// .put( +// CPU_LIMIT_ATTRIBUTE, +// Long.toString(KubernetesSize.toCores(overriddenSidecarCpuLimit))); +// } + + float cpuRequest = Containers.getCpuRequest(container); + if (cpuRequest == 0) { + machineConfig.getAttributes().put(MEMORY_LIMIT_ATTRIBUTE, defaultSidecarCpuRequestCores); + } +// String overriddenSidecarCpuRequest = component.getCpuRequest(); +// if (!isNullOrEmpty(overriddenSidecarCpuRequest)) { +// machineConfig +// .getAttributes() +// .put( +// CPU_REQUEST_ATTRIBUTE, +// Long.toString(KubernetesSize.toCores(overriddenSidecarCpuRequest))); +// } } private Map diff --git a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/wsplugins/MachineResolverBuilder.java b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/wsplugins/MachineResolverBuilder.java index d28396984d2..3ede76ce8d3 100644 --- a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/wsplugins/MachineResolverBuilder.java +++ b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/wsplugins/MachineResolverBuilder.java @@ -23,7 +23,10 @@ public class MachineResolverBuilder { private Container container; private CheContainer cheContainer; - private String defaultSidecarMemorySizeAttribute; + private String defaultSidecarMemoryLimitAttribute; + private String defaultSidecarMemoryRequestAttribute; + private String defaultSidecarCpuLimitAttribute; + private String defaultSidecarCpuRequestAttribute; private List containerEndpoints; private Pair projectsRootPathEnvVar; private Component component; @@ -31,7 +34,10 @@ public class MachineResolverBuilder { public MachineResolver build() { if (container == null || cheContainer == null - || defaultSidecarMemorySizeAttribute == null + || defaultSidecarMemoryLimitAttribute == null + || defaultSidecarMemoryRequestAttribute == null + || defaultSidecarCpuLimitAttribute == null + || defaultSidecarCpuRequestAttribute == null || containerEndpoints == null || projectsRootPathEnvVar == null) { throw new IllegalStateException(); @@ -41,7 +47,10 @@ public MachineResolver build() { projectsRootPathEnvVar, container, cheContainer, - defaultSidecarMemorySizeAttribute, + defaultSidecarMemoryLimitAttribute, + defaultSidecarMemoryRequestAttribute, + defaultSidecarCpuLimitAttribute, + defaultSidecarCpuRequestAttribute, containerEndpoints, component); } @@ -56,9 +65,27 @@ public MachineResolverBuilder setCheContainer(CheContainer cheContainer) { return this; } - public MachineResolverBuilder setDefaultSidecarMemorySizeAttribute( - String defaultSidecarMemorySizeAttribute) { - this.defaultSidecarMemorySizeAttribute = defaultSidecarMemorySizeAttribute; + public MachineResolverBuilder setDefaultSidecarMemoryLimitAttribute( + String defaultSidecarMemoryLimitAttribute) { + this.defaultSidecarMemoryLimitAttribute = defaultSidecarMemoryLimitAttribute; + return this; + } + + public MachineResolverBuilder setDefaultSidecarMemoryRequestAttribute( + String defaultSidecarMemoryRequestAttribute) { + this.defaultSidecarMemoryRequestAttribute = defaultSidecarMemoryRequestAttribute; + return this; + } + + public MachineResolverBuilder setDefaultSidecarCpuLimitAttribute( + String defaultSidecarCpuLimitAttribute) { + this.defaultSidecarCpuLimitAttribute = defaultSidecarCpuLimitAttribute; + return this; + } + + public MachineResolverBuilder setDefaultSidecarCpuRequestAttribute( + String defaultSidecarCpuRequestAttribute) { + this.defaultSidecarCpuRequestAttribute = defaultSidecarCpuRequestAttribute; return this; } 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 78a0fbaa16b..476e61378af 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 @@ -71,6 +71,7 @@ public void setUp() { container, cheContainer, DEFAULT_MEM_LIMIT, + defaultSidecarMemoryRequestBytes, defaultSidecarCpuLimit, defaultSidecarCpuRequest, endpoints, component); } diff --git a/wsmaster/che-core-api-workspace/src/main/java/org/eclipse/che/api/workspace/server/wsplugins/model/CheContainer.java b/wsmaster/che-core-api-workspace/src/main/java/org/eclipse/che/api/workspace/server/wsplugins/model/CheContainer.java index e50f6c1878c..ddc46680f3e 100644 --- a/wsmaster/che-core-api-workspace/src/main/java/org/eclipse/che/api/workspace/server/wsplugins/model/CheContainer.java +++ b/wsmaster/che-core-api-workspace/src/main/java/org/eclipse/che/api/workspace/server/wsplugins/model/CheContainer.java @@ -34,6 +34,15 @@ public class CheContainer { @JsonProperty("memoryLimit") private String memoryLimit = null; + @JsonProperty("memoryRequest") + private String memoryRequest = null; + + @JsonProperty("CPULimit") + private String cpuLimit = null; + + @JsonProperty("CPURequest") + private String cpuRequest = null; + @JsonProperty("mountSources") private boolean mountSources = false; @@ -149,6 +158,48 @@ public void setMemoryLimit(String memoryLimit) { this.memoryLimit = memoryLimit; } + + public CheContainer memoryRequest(String memoryRequest) { + this.memoryRequest = memoryRequest; + return this; + } + + public String getMemoryRequest() { + return memoryRequest; + } + + public void setMemoryRequest(String memoryRequest) { + this.memoryRequest = memoryRequest; + } + + public CheContainer cpuLimit(String cpuLimit) { + this.cpuLimit = cpuLimit; + return this; + } + + public String getCpuLimit() { + return cpuLimit; + } + + public void setCpuLimit(String cpuLimit) { + this.cpuLimit = cpuLimit; + } + + + public CheContainer cpuRequest(String cpuRequest) { + this.cpuRequest = cpuRequest; + return this; + } + + public String getCpuRequest() { + return cpuRequest; + } + + public void setCpuRequest(String cpuRequest) { + this.cpuRequest = cpuRequest; + } + + public CheContainer mountSources(boolean mountSources) { this.mountSources = mountSources; return this; From 21e0bec0bac03e892062ec802c05015468ddb2b3 Mon Sep 17 00:00:00 2001 From: Max Shaposhnik Date: Thu, 6 Feb 2020 17:59:11 +0200 Subject: [PATCH 02/29] Add CPU limits part 2; --- .../model/workspace/config/MachineConfig.java | 6 +- .../KubernetesEnvironmentProvisioner.java | 10 +- .../KubernetesEnvironmentFactory.java | 44 +++--- ...tainerResourceLimitRequestProvisioner.java | 111 ++++++++++++++ .../ram/RamLimitRequestProvisioner.java | 72 --------- .../kubernetes/util/Containers.java | 5 +- .../kubernetes/util/KubernetesSize.java | 4 +- .../KubernetesPluginsToolingApplier.java | 13 +- .../kubernetes/wsplugins/MachineResolver.java | 58 ++++---- .../KubernetesEnvironmentProvisionerTest.java | 4 +- .../KubernetesEnvironmentFactoryTest.java | 48 +++--- ...rResourceLimitRequestProvisionerTest.java} | 44 +++--- .../wsplugins/K8sContainerResolverTest.java | 1 + .../KubernetesPluginsToolingApplierTest.java | 18 +++ .../wsplugins/MachineResolverTest.java | 7 +- .../OpenShiftEnvironmentProvisioner.java | 10 +- .../OpenShiftEnvironmentFactory.java | 45 +++--- .../OpenShiftEnvironmentProvisionerTest.java | 4 +- .../OpenShiftEnvironmentFactoryTest.java | 4 +- .../MemoryAttributeProvisioner.java | 107 ------------- .../ResourceLimitAttributesProvisioner.java | 140 ++++++++++++++++++ .../server/wsplugins/model/CheContainer.java | 3 - ...sourceLimitAttributesProvisionerTest.java} | 78 +++++----- 23 files changed, 456 insertions(+), 380 deletions(-) create mode 100644 infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/provision/limits/ram/ContainerResourceLimitRequestProvisioner.java delete mode 100644 infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/provision/limits/ram/RamLimitRequestProvisioner.java rename infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/provision/limits/ram/{RamLimitRequestProvisionerTest.java => ContainerResourceLimitRequestProvisionerTest.java} (74%) delete mode 100644 wsmaster/che-core-api-workspace/src/main/java/org/eclipse/che/api/workspace/server/spi/environment/MemoryAttributeProvisioner.java create mode 100644 wsmaster/che-core-api-workspace/src/main/java/org/eclipse/che/api/workspace/server/spi/environment/ResourceLimitAttributesProvisioner.java rename wsmaster/che-core-api-workspace/src/test/java/org/eclipse/che/api/workspace/server/spi/environment/{MemoryAttributeProvisionerTest.java => ResourceLimitAttributesProvisionerTest.java} (68%) 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 15ec581854d..76234253bd1 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 @@ -38,10 +38,9 @@ public interface MachineConfig { */ String MEMORY_REQUEST_ATTRIBUTE = "memoryRequestBytes"; - /** - * Name of the attribute from {@link #getAttributes()} which if present defines CPU limit of - * the machine in millicores. + * Name of the attribute from {@link #getAttributes()} which if present defines CPU limit of the + * machine in millicores. */ String CPU_LIMIT_ATTRIBUTE = "cpuLimitMillicores"; @@ -51,7 +50,6 @@ public interface MachineConfig { */ String CPU_REQUEST_ATTRIBUTE = "cpuRequestMillicores"; - /** * Name of the attribute from {@link #getAttributes()} which, if present, defines the entrypoint * command to be executed in the machine/container. diff --git a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/KubernetesEnvironmentProvisioner.java b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/KubernetesEnvironmentProvisioner.java index 1db4a3bd995..e6ec7006023 100644 --- a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/KubernetesEnvironmentProvisioner.java +++ b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/KubernetesEnvironmentProvisioner.java @@ -33,7 +33,7 @@ import org.eclipse.che.workspace.infrastructure.kubernetes.provision.VcsSshKeysProvisioner; import org.eclipse.che.workspace.infrastructure.kubernetes.provision.VcsSslCertificateProvisioner; import org.eclipse.che.workspace.infrastructure.kubernetes.provision.env.EnvVarsConverter; -import org.eclipse.che.workspace.infrastructure.kubernetes.provision.limits.ram.RamLimitRequestProvisioner; +import org.eclipse.che.workspace.infrastructure.kubernetes.provision.limits.ram.ContainerResourceLimitRequestProvisioner; import org.eclipse.che.workspace.infrastructure.kubernetes.provision.restartpolicy.RestartPolicyRewriter; import org.eclipse.che.workspace.infrastructure.kubernetes.provision.server.ServersConverter; import org.eclipse.che.workspace.infrastructure.kubernetes.server.PreviewUrlExposer; @@ -64,7 +64,7 @@ class KubernetesEnvironmentProvisionerImpl private final ServersConverter serversConverter; private final EnvVarsConverter envVarsConverter; private final RestartPolicyRewriter restartPolicyRewriter; - private final RamLimitRequestProvisioner ramLimitProvisioner; + private final ContainerResourceLimitRequestProvisioner resourceLimitRequestProvisioner; private final LogsVolumeMachineProvisioner logsVolumeMachineProvisioner; private final SecurityContextProvisioner securityContextProvisioner; private final PodTerminationGracePeriodProvisioner podTerminationGracePeriodProvisioner; @@ -86,7 +86,7 @@ public KubernetesEnvironmentProvisionerImpl( EnvVarsConverter envVarsConverter, RestartPolicyRewriter restartPolicyRewriter, WorkspaceVolumesStrategy volumesStrategy, - RamLimitRequestProvisioner ramLimitProvisioner, + ContainerResourceLimitRequestProvisioner resourceLimitRequestProvisioner, LogsVolumeMachineProvisioner logsVolumeMachineProvisioner, SecurityContextProvisioner securityContextProvisioner, PodTerminationGracePeriodProvisioner podTerminationGracePeriodProvisioner, @@ -105,7 +105,7 @@ public KubernetesEnvironmentProvisionerImpl( this.serversConverter = serversConverter; this.envVarsConverter = envVarsConverter; this.restartPolicyRewriter = restartPolicyRewriter; - this.ramLimitProvisioner = ramLimitProvisioner; + this.resourceLimitRequestProvisioner = resourceLimitRequestProvisioner; this.logsVolumeMachineProvisioner = logsVolumeMachineProvisioner; this.securityContextProvisioner = securityContextProvisioner; this.podTerminationGracePeriodProvisioner = podTerminationGracePeriodProvisioner; @@ -146,7 +146,7 @@ public void provision(KubernetesEnvironment k8sEnv, RuntimeIdentity identity) LOG.debug("Provisioning environment items for workspace '{}'", workspaceId); restartPolicyRewriter.provision(k8sEnv, identity); uniqueNamesProvisioner.provision(k8sEnv, identity); - ramLimitProvisioner.provision(k8sEnv, identity); + resourceLimitRequestProvisioner.provision(k8sEnv, identity); externalServerIngressTlsProvisioner.provision(k8sEnv, identity); securityContextProvisioner.provision(k8sEnv, identity); podTerminationGracePeriodProvisioner.provision(k8sEnv, identity); diff --git a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/environment/KubernetesEnvironmentFactory.java b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/environment/KubernetesEnvironmentFactory.java index 5d44913ab5d..f2bb510d264 100644 --- a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/environment/KubernetesEnvironmentFactory.java +++ b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/environment/KubernetesEnvironmentFactory.java @@ -15,9 +15,7 @@ import static org.eclipse.che.workspace.infrastructure.kubernetes.environment.PodMerger.DEPLOYMENT_NAME_LABEL; import static org.eclipse.che.workspace.infrastructure.kubernetes.namespace.KubernetesObjectUtil.setSelector; -import com.google.common.annotations.VisibleForTesting; import io.fabric8.kubernetes.api.model.ConfigMap; -import io.fabric8.kubernetes.api.model.Container; import io.fabric8.kubernetes.api.model.HasMetadata; import io.fabric8.kubernetes.api.model.PersistentVolumeClaim; import io.fabric8.kubernetes.api.model.Pod; @@ -26,7 +24,6 @@ import io.fabric8.kubernetes.api.model.apps.Deployment; import io.fabric8.kubernetes.api.model.extensions.Ingress; import java.util.ArrayList; -import java.util.Collection; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -42,13 +39,11 @@ 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.MemoryAttributeProvisioner; import org.eclipse.che.api.workspace.server.spi.environment.RecipeRetriever; +import org.eclipse.che.api.workspace.server.spi.environment.ResourceLimitAttributesProvisioner; import org.eclipse.che.commons.annotation.Nullable; -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.PodData; -import org.eclipse.che.workspace.infrastructure.kubernetes.util.Containers; /** * Parses {@link InternalEnvironment} into {@link KubernetesEnvironment}. @@ -60,7 +55,7 @@ public class KubernetesEnvironmentFactory private final KubernetesRecipeParser recipeParser; private final KubernetesEnvironmentValidator envValidator; - private final MemoryAttributeProvisioner memoryProvisioner; + private final ResourceLimitAttributesProvisioner memoryProvisioner; private final PodMerger podMerger; @Inject @@ -69,7 +64,7 @@ public KubernetesEnvironmentFactory( MachineConfigsValidator machinesValidator, KubernetesRecipeParser recipeParser, KubernetesEnvironmentValidator envValidator, - MemoryAttributeProvisioner memoryProvisioner, + ResourceLimitAttributesProvisioner memoryProvisioner, PodMerger podMerger) { super(recipeRetriever, machinesValidator); this.recipeParser = recipeParser; @@ -151,7 +146,7 @@ protected KubernetesEnvironment doCreate( .setConfigMaps(configMaps) .build(); - addRamAttributes(k8sEnv.getMachines(), k8sEnv.getPodsData().values()); + // addRamAttributes(k8sEnv.getMachines(), k8sEnv.getPodsData().values()); envValidator.validate(k8sEnv); @@ -218,21 +213,22 @@ private void putInto(Map map, String key, T v } } - @VisibleForTesting - void addRamAttributes(Map machines, Collection pods) { - for (PodData pod : pods) { - for (Container container : pod.getSpec().getContainers()) { - final String machineName = Names.machineName(pod, container); - InternalMachineConfig machineConfig; - if ((machineConfig = machines.get(machineName)) == null) { - machineConfig = new InternalMachineConfig(); - machines.put(machineName, machineConfig); - } - memoryProvisioner.provision( - machineConfig, Containers.getRamLimit(container), Containers.getRamRequest(container)); - } - } - } + // @VisibleForTesting + // void addRamAttributes(Map machines, Collection pods) { + // for (PodData pod : pods) { + // for (Container container : pod.getSpec().getContainers()) { + // final String machineName = Names.machineName(pod, container); + // InternalMachineConfig machineConfig; + // if ((machineConfig = machines.get(machineName)) == null) { + // machineConfig = new InternalMachineConfig(); + // machines.put(machineName, machineConfig); + // } + // memoryProvisioner.provisionMemory( + // machineConfig, Containers.getRamLimit(container), + // Containers.getRamRequest(container)); + // } + // } + // } private void checkNotNull(Object object, String errorMessage) throws ValidationException { if (object == null) { diff --git a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/provision/limits/ram/ContainerResourceLimitRequestProvisioner.java b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/provision/limits/ram/ContainerResourceLimitRequestProvisioner.java new file mode 100644 index 00000000000..0d859f4caaf --- /dev/null +++ b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/provision/limits/ram/ContainerResourceLimitRequestProvisioner.java @@ -0,0 +1,111 @@ +/* + * Copyright (c) 2012-2018 Red Hat, Inc. + * This program and the accompanying materials are made + * available under the terms of the Eclipse Public License 2.0 + * which is available at https://www.eclipse.org/legal/epl-2.0/ + * + * SPDX-License-Identifier: EPL-2.0 + * + * Contributors: + * Red Hat, Inc. - initial API and implementation + */ +package org.eclipse.che.workspace.infrastructure.kubernetes.provision.limits.ram; + +import static org.eclipse.che.api.core.model.workspace.config.MachineConfig.CPU_LIMIT_ATTRIBUTE; +import static org.eclipse.che.api.core.model.workspace.config.MachineConfig.CPU_REQUEST_ATTRIBUTE; +import static org.eclipse.che.api.core.model.workspace.config.MachineConfig.MEMORY_LIMIT_ATTRIBUTE; +import static org.eclipse.che.api.core.model.workspace.config.MachineConfig.MEMORY_REQUEST_ATTRIBUTE; +import static org.eclipse.che.workspace.infrastructure.kubernetes.Names.machineName; + +import io.fabric8.kubernetes.api.model.Container; +import java.util.Map; +import javax.inject.Inject; +import javax.inject.Named; +import javax.inject.Singleton; +import org.eclipse.che.api.core.model.workspace.runtime.RuntimeIdentity; +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.spi.environment.ResourceLimitAttributesProvisioner; +import org.eclipse.che.commons.annotation.Traced; +import org.eclipse.che.commons.tracing.TracingTags; +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.provision.ConfigurationProvisioner; +import org.eclipse.che.workspace.infrastructure.kubernetes.util.Containers; +import org.eclipse.che.workspace.infrastructure.kubernetes.util.KubernetesSize; + +/** + * Sets or overrides Kubernetes container RAM and CPU limit and request if corresponding attributes + * are present in machine corresponding to the container. + * + * @author Anton Korneta + * @auhtor Max Shaposhnyk + */ +@Singleton +public class ContainerResourceLimitRequestProvisioner implements ConfigurationProvisioner { + + private final ResourceLimitAttributesProvisioner resourceLimitAttributesProvisioner; + + private final long defaultMachineMaxMemorySizeAttribute; + private final long defaultMachineRequestMemorySizeAttribute; + private final float defaultMachineCpuLimitAttribute; + private final float defaultMachineCpuRequestAttribute; + + @Inject + public ContainerResourceLimitRequestProvisioner( + @Named("che.workspace.default_memory_limit_mb") long defaultMachineMaxMemorySizeAttribute, + @Named("che.workspace.default_memory_request_mb") + long defaultMachineRequestMemorySizeAttribute, + @Named("che.workspace.default_cpu_limit_cores") String defaultMachineCpuLimitAttribute, + @Named("che.workspace.default_cpu_request_cores") String defaultMachineCpuRequestAttribute, + ResourceLimitAttributesProvisioner resourceLimitAttributesProvisioner) { + this.resourceLimitAttributesProvisioner = resourceLimitAttributesProvisioner; + this.defaultMachineMaxMemorySizeAttribute = defaultMachineMaxMemorySizeAttribute * 1024 * 1024; + this.defaultMachineRequestMemorySizeAttribute = + defaultMachineRequestMemorySizeAttribute * 1024 * 1024; + this.defaultMachineCpuLimitAttribute = KubernetesSize.toCores(defaultMachineCpuLimitAttribute); + this.defaultMachineCpuRequestAttribute = + KubernetesSize.toCores(defaultMachineCpuRequestAttribute); + } + + @Override + @Traced + public void provision(KubernetesEnvironment k8sEnv, RuntimeIdentity identity) + throws InfrastructureException { + + TracingTags.WORKSPACE_ID.set(identity::getWorkspaceId); + + final Map machines = k8sEnv.getMachines(); + for (PodData pod : k8sEnv.getPodsData().values()) { + for (Container container : pod.getSpec().getContainers()) { + + // make sure that machine configs have settings for RAM limit and request + InternalMachineConfig machineConfig = machines.get(machineName(pod, container)); + resourceLimitAttributesProvisioner.provisionMemory( + machineConfig, + Containers.getRamLimit(container), + Containers.getRamRequest(container), + defaultMachineMaxMemorySizeAttribute, + defaultMachineRequestMemorySizeAttribute); + + // make sure that machine configs have settings for CPU limit and request + resourceLimitAttributesProvisioner.provisionCPU( + machineConfig, + Containers.getCpuLimit(container), + Containers.getCpuRequest(container), + defaultMachineCpuLimitAttribute, + defaultMachineCpuRequestAttribute); + + // reapply memory and CPU settings to k8s container to make sure that provisioned + // values above are set + final Map attributes = machineConfig.getAttributes(); + Containers.addRamLimit(container, Long.parseLong(attributes.get(MEMORY_LIMIT_ATTRIBUTE))); + Containers.addRamRequest( + container, Long.parseLong(attributes.get(MEMORY_REQUEST_ATTRIBUTE))); + Containers.addCpuLimit(container, Float.parseFloat(attributes.get(CPU_LIMIT_ATTRIBUTE))); + Containers.addCpuRequest( + container, Float.parseFloat(attributes.get(CPU_REQUEST_ATTRIBUTE))); + } + } + } +} diff --git a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/provision/limits/ram/RamLimitRequestProvisioner.java b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/provision/limits/ram/RamLimitRequestProvisioner.java deleted file mode 100644 index b01a9626658..00000000000 --- a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/provision/limits/ram/RamLimitRequestProvisioner.java +++ /dev/null @@ -1,72 +0,0 @@ -/* - * Copyright (c) 2012-2018 Red Hat, Inc. - * This program and the accompanying materials are made - * available under the terms of the Eclipse Public License 2.0 - * which is available at https://www.eclipse.org/legal/epl-2.0/ - * - * SPDX-License-Identifier: EPL-2.0 - * - * Contributors: - * Red Hat, Inc. - initial API and implementation - */ -package org.eclipse.che.workspace.infrastructure.kubernetes.provision.limits.ram; - -import static org.eclipse.che.api.core.model.workspace.config.MachineConfig.MEMORY_LIMIT_ATTRIBUTE; -import static org.eclipse.che.api.core.model.workspace.config.MachineConfig.MEMORY_REQUEST_ATTRIBUTE; -import static org.eclipse.che.workspace.infrastructure.kubernetes.Names.machineName; - -import io.fabric8.kubernetes.api.model.Container; -import java.util.Map; -import javax.inject.Inject; -import org.eclipse.che.api.core.model.workspace.runtime.RuntimeIdentity; -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.spi.environment.MemoryAttributeProvisioner; -import org.eclipse.che.commons.annotation.Traced; -import org.eclipse.che.commons.tracing.TracingTags; -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.provision.ConfigurationProvisioner; -import org.eclipse.che.workspace.infrastructure.kubernetes.util.Containers; - -/** - * Sets or overrides Kubernetes container RAM limit and request if corresponding attributes are - * present in machine corresponding to the container. - * - * @author Anton Korneta - */ -public class RamLimitRequestProvisioner implements ConfigurationProvisioner { - - private final MemoryAttributeProvisioner memoryAttributeProvisioner; - - @Inject - public RamLimitRequestProvisioner(MemoryAttributeProvisioner memoryAttributeProvisioner) { - this.memoryAttributeProvisioner = memoryAttributeProvisioner; - } - - @Override - @Traced - public void provision(KubernetesEnvironment k8sEnv, RuntimeIdentity identity) - throws InfrastructureException { - - TracingTags.WORKSPACE_ID.set(identity::getWorkspaceId); - - final Map machines = k8sEnv.getMachines(); - for (PodData pod : k8sEnv.getPodsData().values()) { - for (Container container : pod.getSpec().getContainers()) { - InternalMachineConfig machineConfig = machines.get(machineName(pod, container)); - memoryAttributeProvisioner.provision( - machineConfig, Containers.getRamLimit(container), Containers.getRamRequest(container)); - final Map attributes = machineConfig.getAttributes(); - String memoryLimitAttribute = attributes.get(MEMORY_LIMIT_ATTRIBUTE); - if (memoryLimitAttribute != null) { - Containers.addRamLimit(container, Long.parseLong(memoryLimitAttribute)); - } - String memoryRequestAttribute = attributes.get(MEMORY_REQUEST_ATTRIBUTE); - if (memoryRequestAttribute != null) { - Containers.addRamRequest(container, Long.parseLong(memoryRequestAttribute)); - } - } - } - } -} diff --git a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/util/Containers.java b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/util/Containers.java index 05d95c7baf6..b04ebb6fa23 100644 --- a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/util/Containers.java +++ b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/util/Containers.java @@ -145,7 +145,7 @@ public static void addCpuLimit(Container container, float cpuLimit) { resourceBuilder = new ResourceRequirementsBuilder(); } container.setResources( - resourceBuilder.addToLimits("memory", new Quantity(Float.toString(cpuLimit))).build()); + resourceBuilder.addToLimits("cpu", new Quantity(Float.toString(cpuLimit))).build()); } /** @@ -163,7 +163,6 @@ public static void addCpuLimit(Container container, String limitInK8sNotion) { resourceBuilder.addToLimits("cpu", new Quantity(limitInK8sNotion)).build()); } - /** * Returns the CPU request in bytes, if it is present in given container otherwise 0 will be * returned. @@ -209,6 +208,4 @@ public static void addCpuRequest(Container container, String limitInK8sNotion) { container.setResources( resourceBuilder.addToRequests("cpu", new Quantity(limitInK8sNotion)).build()); } - - } diff --git a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/util/KubernetesSize.java b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/util/KubernetesSize.java index d6bc3da2e87..33a45359a01 100644 --- a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/util/KubernetesSize.java +++ b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/util/KubernetesSize.java @@ -40,8 +40,7 @@ public class KubernetesSize { private static final Pattern HUMAN_SIZE_MEMORY_PATTERN = Pattern.compile("^([-+]?[0-9]*\\.?[0-9]+([eE][-+]?[0-9]+)?)\\s*(\\S+)?$"); - private static final Pattern HUMAN_SIZE_CPU_PATTERN = - Pattern.compile("^([0-9]*\\.?[0-9]+)(m)?$"); + private static final Pattern HUMAN_SIZE_CPU_PATTERN = Pattern.compile("^([0-9]*\\.?[0-9]+)(m)?$"); /** * Converts memory in Kubernetes format to bytes. @@ -128,7 +127,6 @@ public static String toKubeSize(long bytes, boolean si) { return String.format((size % 1.0f == 0) ? "%.0f%s" : "%.1f%s", size, unit); } - /** * Converts CPU resource in Kubernetes format to cores. * 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 6bdcd5ccb9f..5c73508be82 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.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.KubernetesSize; /** * Applies Che plugins tooling configuration to a kubernetes internal runtime object. @@ -85,14 +86,18 @@ public KubernetesPluginsToolingApplier( @Named("che.workspace.sidecar.default_memory_limit_mb") long defaultSidecarMemoryLimitMB, @Named("che.workspace.sidecar.default_memory_request_mb") long defaultSidecarMemoryRequestMB, @Named("che.workspace.sidecar.default_cpu_limit_cores") String defaultSidecarCpuLimitCores, - @Named("che.workspace.sidecar.default_cpu_request_cores") String defaultSidecarCpuRequestCores, + @Named("che.workspace.sidecar.default_cpu_request_cores") + String defaultSidecarCpuRequestCores, @Named("che.agents.auth_enabled") boolean isAuthEnabled, ProjectsRootEnvVariableProvider projectsRootEnvVariableProvider, ChePluginsVolumeApplier chePluginsVolumeApplier) { this.defaultSidecarMemoryLimitBytes = String.valueOf(defaultSidecarMemoryLimitMB * 1024 * 1024); - this.defaultSidecarMemoryRequestBytes = String.valueOf(defaultSidecarMemoryRequestMB * 1024 * 1024); - this.defaultSidecarCpuLimitCores = defaultSidecarCpuLimitCores; - this.defaultSidecarCpuRequestCores = defaultSidecarCpuRequestCores; + this.defaultSidecarMemoryRequestBytes = + String.valueOf(defaultSidecarMemoryRequestMB * 1024 * 1024); + this.defaultSidecarCpuLimitCores = + Float.toString(KubernetesSize.toCores(defaultSidecarCpuLimitCores)); + this.defaultSidecarCpuRequestCores = + Float.toString(KubernetesSize.toCores(defaultSidecarCpuRequestCores)); this.isAuthEnabled = isAuthEnabled; this.sidecarImagePullPolicy = validImagePullPolicies.contains(sidecarImagePullPolicy) ? sidecarImagePullPolicy : null; 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 902d64fcfa5..daf054197b2 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 @@ -14,8 +14,11 @@ import static com.google.common.base.Strings.isNullOrEmpty; import static java.lang.String.format; import static java.util.stream.Collectors.toMap; +import static org.eclipse.che.api.core.model.workspace.config.MachineConfig.CPU_LIMIT_ATTRIBUTE; +import static org.eclipse.che.api.core.model.workspace.config.MachineConfig.CPU_REQUEST_ATTRIBUTE; 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; +import static org.eclipse.che.api.core.model.workspace.config.MachineConfig.MEMORY_REQUEST_ATTRIBUTE; import static org.eclipse.che.api.workspace.shared.Constants.PROJECTS_VOLUME_NAME; import io.fabric8.kubernetes.api.model.Container; @@ -112,45 +115,44 @@ private void normalizeMemory(Container container, InternalMachineConfig machineC long ramRequest = Containers.getRamRequest(container); if (ramRequest == 0) { - machineConfig.getAttributes().put(MEMORY_LIMIT_ATTRIBUTE, defaultSidecarMemoryRequestBytes); + machineConfig.getAttributes().put(MEMORY_REQUEST_ATTRIBUTE, defaultSidecarMemoryRequestBytes); } -// String overriddenSidecarMemRequest = component.getRamRequest(); -// if (!isNullOrEmpty(overriddenSidecarMemRequest)) { -// machineConfig -// .getAttributes() -// .put( -// MEMORY_REQUEST_ATTRIBUTE, -// Long.toString(KubernetesSize.toBytes(overriddenSidecarMemRequest))); -// } + // String overriddenSidecarMemRequest = component.getRamRequest(); + // if (!isNullOrEmpty(overriddenSidecarMemRequest)) { + // machineConfig + // .getAttributes() + // .put( + // MEMORY_REQUEST_ATTRIBUTE, + // Long.toString(KubernetesSize.toBytes(overriddenSidecarMemRequest))); + // } } - private void normalizeCpu(Container container, InternalMachineConfig machineConfig) { float cpuLimit = Containers.getCpuLimit(container); if (cpuLimit == 0) { - machineConfig.getAttributes().put(MEMORY_LIMIT_ATTRIBUTE, defaultSidecarCpuLimitCores); + machineConfig.getAttributes().put(CPU_LIMIT_ATTRIBUTE, defaultSidecarCpuLimitCores); } -// String overriddenSidecarCpuLimit = component.getCpuLimit(); -// if (!isNullOrEmpty(overriddenSidecarCpuLimit)) { -// machineConfig -// .getAttributes() -// .put( -// CPU_LIMIT_ATTRIBUTE, -// Long.toString(KubernetesSize.toCores(overriddenSidecarCpuLimit))); -// } + // String overriddenSidecarCpuLimit = component.getCpuLimit(); + // if (!isNullOrEmpty(overriddenSidecarCpuLimit)) { + // machineConfig + // .getAttributes() + // .put( + // CPU_LIMIT_ATTRIBUTE, + // Long.toString(KubernetesSize.toCores(overriddenSidecarCpuLimit))); + // } float cpuRequest = Containers.getCpuRequest(container); if (cpuRequest == 0) { - machineConfig.getAttributes().put(MEMORY_LIMIT_ATTRIBUTE, defaultSidecarCpuRequestCores); + machineConfig.getAttributes().put(CPU_REQUEST_ATTRIBUTE, defaultSidecarCpuRequestCores); } -// String overriddenSidecarCpuRequest = component.getCpuRequest(); -// if (!isNullOrEmpty(overriddenSidecarCpuRequest)) { -// machineConfig -// .getAttributes() -// .put( -// CPU_REQUEST_ATTRIBUTE, -// Long.toString(KubernetesSize.toCores(overriddenSidecarCpuRequest))); -// } + // String overriddenSidecarCpuRequest = component.getCpuRequest(); + // if (!isNullOrEmpty(overriddenSidecarCpuRequest)) { + // machineConfig + // .getAttributes() + // .put( + // CPU_REQUEST_ATTRIBUTE, + // Long.toString(KubernetesSize.toCores(overriddenSidecarCpuRequest))); + // } } private Map diff --git a/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/KubernetesEnvironmentProvisionerTest.java b/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/KubernetesEnvironmentProvisionerTest.java index ed60bdd2daa..febbc74e6b1 100644 --- a/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/KubernetesEnvironmentProvisionerTest.java +++ b/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/KubernetesEnvironmentProvisionerTest.java @@ -31,7 +31,7 @@ import org.eclipse.che.workspace.infrastructure.kubernetes.provision.VcsSshKeysProvisioner; import org.eclipse.che.workspace.infrastructure.kubernetes.provision.VcsSslCertificateProvisioner; import org.eclipse.che.workspace.infrastructure.kubernetes.provision.env.EnvVarsConverter; -import org.eclipse.che.workspace.infrastructure.kubernetes.provision.limits.ram.RamLimitRequestProvisioner; +import org.eclipse.che.workspace.infrastructure.kubernetes.provision.limits.ram.ContainerResourceLimitRequestProvisioner; import org.eclipse.che.workspace.infrastructure.kubernetes.provision.restartpolicy.RestartPolicyRewriter; import org.eclipse.che.workspace.infrastructure.kubernetes.provision.server.ServersConverter; import org.eclipse.che.workspace.infrastructure.kubernetes.server.PreviewUrlExposer; @@ -57,7 +57,7 @@ public class KubernetesEnvironmentProvisionerTest { @Mock private EnvVarsConverter envVarsProvisioner; @Mock private ServersConverter serversProvisioner; @Mock private RestartPolicyRewriter restartPolicyRewriter; - @Mock private RamLimitRequestProvisioner ramLimitProvisioner; + @Mock private ContainerResourceLimitRequestProvisioner ramLimitProvisioner; @Mock private LogsVolumeMachineProvisioner logsVolumeMachineProvisioner; @Mock private SecurityContextProvisioner securityContextProvisioner; @Mock private PodTerminationGracePeriodProvisioner podTerminationGracePeriodProvisioner; diff --git a/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/environment/KubernetesEnvironmentFactoryTest.java b/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/environment/KubernetesEnvironmentFactoryTest.java index 3bd0d717529..65dee6da543 100644 --- a/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/environment/KubernetesEnvironmentFactoryTest.java +++ b/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/environment/KubernetesEnvironmentFactoryTest.java @@ -19,7 +19,6 @@ import static org.eclipse.che.workspace.infrastructure.kubernetes.Warnings.INGRESSES_IGNORED_WARNING_MESSAGE; import static org.eclipse.che.workspace.infrastructure.kubernetes.environment.PodMerger.DEPLOYMENT_NAME_LABEL; import static org.mockito.ArgumentMatchers.any; -import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.lenient; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; @@ -30,7 +29,6 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; -import com.google.common.collect.ImmutableSet; import io.fabric8.kubernetes.api.model.ConfigMap; import io.fabric8.kubernetes.api.model.ConfigMapBuilder; import io.fabric8.kubernetes.api.model.Container; @@ -53,15 +51,13 @@ import io.fabric8.kubernetes.api.model.apps.Deployment; import io.fabric8.kubernetes.api.model.apps.DeploymentBuilder; import io.fabric8.kubernetes.api.model.extensions.IngressBuilder; -import java.util.HashMap; import java.util.Map; -import java.util.Set; import org.eclipse.che.api.core.ValidationException; import org.eclipse.che.api.workspace.server.model.impl.WarningImpl; import org.eclipse.che.api.workspace.server.spi.environment.InternalEnvironment; 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.MemoryAttributeProvisioner; +import org.eclipse.che.api.workspace.server.spi.environment.ResourceLimitAttributesProvisioner; import org.eclipse.che.workspace.infrastructure.kubernetes.Names; import org.eclipse.che.workspace.infrastructure.kubernetes.environment.KubernetesEnvironment.PodData; import org.mockito.Mock; @@ -88,7 +84,7 @@ public class KubernetesEnvironmentFactoryTest { @Mock private InternalRecipe internalRecipe; @Mock private InternalMachineConfig machineConfig1; @Mock private InternalMachineConfig machineConfig2; - @Mock private MemoryAttributeProvisioner memoryProvisioner; + @Mock private ResourceLimitAttributesProvisioner memoryProvisioner; @Mock private KubernetesRecipeParser k8sRecipeParser; @Mock private PodMerger podMerger; @@ -439,26 +435,26 @@ public void exceptionOnObjectWithNoNameSpecified() throws Exception { k8sEnvFactory.doCreate(internalRecipe, emptyMap(), emptyList()); } - @Test - public void testProvisionRamAttributesIsInvoked() { - final long firstMachineRamLimit = 3072; - final long firstMachineRamRequest = 1536; - final long secondMachineRamLimit = 1024; - final long secondMachineRamRequest = 512; - when(machineConfig1.getAttributes()).thenReturn(new HashMap<>()); - when(machineConfig2.getAttributes()).thenReturn(new HashMap<>()); - final Set pods = - ImmutableSet.of( - createPodData(MACHINE_NAME_1, firstMachineRamLimit, firstMachineRamRequest), - createPodData(MACHINE_NAME_2, secondMachineRamLimit, secondMachineRamRequest)); - - k8sEnvFactory.addRamAttributes(machines, pods); - - verify(memoryProvisioner) - .provision(eq(machineConfig1), eq(firstMachineRamLimit), eq(firstMachineRamRequest)); - verify(memoryProvisioner) - .provision(eq(machineConfig2), eq(secondMachineRamLimit), eq(secondMachineRamRequest)); - } + // @Test + // public void testProvisionRamAttributesIsInvoked() { + // final long firstMachineRamLimit = 3072; + // final long firstMachineRamRequest = 1536; + // final long secondMachineRamLimit = 1024; + // final long secondMachineRamRequest = 512; + // when(machineConfig1.getAttributes()).thenReturn(new HashMap<>()); + // when(machineConfig2.getAttributes()).thenReturn(new HashMap<>()); + // final Set pods = + // ImmutableSet.of( + // createPodData(MACHINE_NAME_1, firstMachineRamLimit, firstMachineRamRequest), + // createPodData(MACHINE_NAME_2, secondMachineRamLimit, secondMachineRamRequest)); + // + // k8sEnvFactory.addRamAttributes(machines, pods); + // + // verify(memoryProvisioner) + // .provision(eq(machineConfig1), eq(firstMachineRamLimit), eq(firstMachineRamRequest)); + // verify(memoryProvisioner) + // .provision(eq(machineConfig2), eq(secondMachineRamLimit), eq(secondMachineRamRequest)); + // } private static PodData createPodData(String machineName, long ramLimit, long ramRequest) { final String containerName = "container_" + machineName; diff --git a/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/provision/limits/ram/RamLimitRequestProvisionerTest.java b/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/provision/limits/ram/ContainerResourceLimitRequestProvisionerTest.java similarity index 74% rename from infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/provision/limits/ram/RamLimitRequestProvisionerTest.java rename to infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/provision/limits/ram/ContainerResourceLimitRequestProvisionerTest.java index 8eaf82fdb3d..441df00057c 100644 --- a/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/provision/limits/ram/RamLimitRequestProvisionerTest.java +++ b/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/provision/limits/ram/ContainerResourceLimitRequestProvisionerTest.java @@ -12,6 +12,8 @@ package org.eclipse.che.workspace.infrastructure.kubernetes.provision.limits.ram; import static com.google.common.collect.ImmutableMap.of; +import static org.eclipse.che.api.core.model.workspace.config.MachineConfig.CPU_LIMIT_ATTRIBUTE; +import static org.eclipse.che.api.core.model.workspace.config.MachineConfig.CPU_REQUEST_ATTRIBUTE; import static org.eclipse.che.api.core.model.workspace.config.MachineConfig.MEMORY_LIMIT_ATTRIBUTE; import static org.eclipse.che.api.core.model.workspace.config.MachineConfig.MEMORY_REQUEST_ATTRIBUTE; import static org.mockito.Mockito.mock; @@ -27,11 +29,9 @@ import java.util.Collections; import org.eclipse.che.api.core.model.workspace.runtime.RuntimeIdentity; import org.eclipse.che.api.workspace.server.spi.environment.InternalMachineConfig; -import org.eclipse.che.api.workspace.server.spi.environment.MemoryAttributeProvisioner; +import org.eclipse.che.api.workspace.server.spi.environment.ResourceLimitAttributesProvisioner; import org.eclipse.che.workspace.infrastructure.kubernetes.environment.KubernetesEnvironment; import org.eclipse.che.workspace.infrastructure.kubernetes.environment.KubernetesEnvironment.PodData; -import org.mockito.ArgumentCaptor; -import org.mockito.Captor; import org.mockito.Mock; import org.mockito.testng.MockitoTestNGListener; import org.testng.annotations.BeforeMethod; @@ -39,32 +39,34 @@ import org.testng.annotations.Test; /** - * Tests {@link RamLimitRequestProvisioner}. + * Tests {@link ContainerResourceLimitRequestProvisioner}. * * @author Anton Korneta */ @Listeners(MockitoTestNGListener.class) -public class RamLimitRequestProvisionerTest { +public class ContainerResourceLimitRequestProvisionerTest { public static final String POD_NAME = "web"; public static final String CONTAINER_NAME = "app"; public static final String MACHINE_NAME = POD_NAME + '/' + CONTAINER_NAME; - public static final String RAM_LIMIT_ATTRIBUTE = "2147483648"; - public static final String RAM_REQUEST_ATTRIBUTE = "1234567890"; + public static final String RAM_LIMIT_VALUE = "2147483648"; + public static final String RAM_REQUEST_VALUE = "1234567890"; + public static final String CPU_LIMIT_VALUE = "0.4"; + public static final String CPU_REQUEST_VALUE = "0.150"; @Mock private KubernetesEnvironment k8sEnv; @Mock private RuntimeIdentity identity; @Mock private InternalMachineConfig internalMachineConfig; - @Mock private MemoryAttributeProvisioner memoryAttributeProvisioner; - - @Captor private ArgumentCaptor resourceCaptor; + @Mock private ResourceLimitAttributesProvisioner resourceLimitAttributesProvisioner; private Container container; - private RamLimitRequestProvisioner ramProvisioner; + private ContainerResourceLimitRequestProvisioner ramProvisioner; @BeforeMethod public void setup() { - ramProvisioner = new RamLimitRequestProvisioner(memoryAttributeProvisioner); + ramProvisioner = + new ContainerResourceLimitRequestProvisioner( + 1024, 512, "500m", "100m", resourceLimitAttributesProvisioner); container = new Container(); container.setName(CONTAINER_NAME); when(k8sEnv.getMachines()).thenReturn(of(MACHINE_NAME, internalMachineConfig)); @@ -72,9 +74,13 @@ public void setup() { .thenReturn( of( MEMORY_LIMIT_ATTRIBUTE, - RAM_LIMIT_ATTRIBUTE, + RAM_LIMIT_VALUE, MEMORY_REQUEST_ATTRIBUTE, - RAM_REQUEST_ATTRIBUTE)); + RAM_REQUEST_VALUE, + CPU_LIMIT_ATTRIBUTE, + CPU_LIMIT_VALUE, + CPU_REQUEST_ATTRIBUTE, + CPU_REQUEST_VALUE)); final ObjectMeta podMetadata = mock(ObjectMeta.class); when(podMetadata.getName()).thenReturn(POD_NAME); final PodSpec podSpec = mock(PodSpec.class); @@ -86,8 +92,7 @@ public void setup() { public void testProvisionRamLimitAttributeToContainer() throws Exception { ramProvisioner.provision(k8sEnv, identity); - assertEquals( - container.getResources().getLimits().get("memory").getAmount(), RAM_LIMIT_ATTRIBUTE); + assertEquals(container.getResources().getLimits().get("memory").getAmount(), RAM_LIMIT_VALUE); } @Test @@ -100,8 +105,7 @@ public void testOverridesContainerRamLimitFromMachineAttribute() throws Exceptio ramProvisioner.provision(k8sEnv, identity); - assertEquals( - container.getResources().getLimits().get("memory").getAmount(), RAM_LIMIT_ATTRIBUTE); + assertEquals(container.getResources().getLimits().get("memory").getAmount(), RAM_LIMIT_VALUE); } @Test @@ -109,7 +113,7 @@ public void testProvisionRamRequestAttributeToContainer() throws Exception { ramProvisioner.provision(k8sEnv, identity); assertEquals( - container.getResources().getRequests().get("memory").getAmount(), RAM_REQUEST_ATTRIBUTE); + container.getResources().getRequests().get("memory").getAmount(), RAM_REQUEST_VALUE); } @Test @@ -123,6 +127,6 @@ public void testOverridesContainerRamRequestFromMachineAttribute() throws Except ramProvisioner.provision(k8sEnv, identity); assertEquals( - container.getResources().getRequests().get("memory").getAmount(), RAM_REQUEST_ATTRIBUTE); + container.getResources().getRequests().get("memory").getAmount(), RAM_REQUEST_VALUE); } } diff --git a/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/wsplugins/K8sContainerResolverTest.java b/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/wsplugins/K8sContainerResolverTest.java index 78227d04b32..110b9bd4651 100644 --- a/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/wsplugins/K8sContainerResolverTest.java +++ b/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/wsplugins/K8sContainerResolverTest.java @@ -103,6 +103,7 @@ public void shouldSetPortsFromContainerEndpoints() throws Exception { public void shouldProvisionSidecarMemoryLimitAndRequest( String sidecarMemLimit, ResourceRequirements resources) throws Exception { cheContainer.setMemoryLimit(sidecarMemLimit); + cheContainer.setMemoryRequest(sidecarMemLimit); Container container = resolver.resolve(); 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 ee92e1e15fd..36abfd92fca 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 @@ -98,6 +98,9 @@ public class KubernetesPluginsToolingApplierTest { private static final String VOLUME_MOUNT_PATH = "/path/test"; private static final String USER_MACHINE_NAME = POD_NAME + "/userContainer"; private static final int MEMORY_LIMIT_MB = 200; + private static final int MEMORY_REQUEST_MB = 100; + private static final String CPU_LIMIT = "200m"; + private static final String CPU_REQUEST = "100m"; private static final String CHE_PLUGIN_ENDPOINT_NAME = "test-endpoint-1"; @Mock private Pod pod; @@ -120,6 +123,9 @@ public void setUp() { new KubernetesPluginsToolingApplier( TEST_IMAGE_POLICY, MEMORY_LIMIT_MB, + MEMORY_REQUEST_MB, + CPU_LIMIT, + CPU_REQUEST, false, projectsRootEnvVariableProvider, chePluginsVolumeApplier); @@ -756,6 +762,9 @@ public void shouldSetJWTServerExposerAttributeIfAuthEnabled() throws Exception { new KubernetesPluginsToolingApplier( TEST_IMAGE_POLICY, MEMORY_LIMIT_MB, + MEMORY_REQUEST_MB, + CPU_LIMIT, + CPU_REQUEST, true, projectsRootEnvVariableProvider, chePluginsVolumeApplier); @@ -772,6 +781,9 @@ public void shouldNotSetJWTServerExposerAttributeIfAuthEnabledButAttributeIsPres new KubernetesPluginsToolingApplier( TEST_IMAGE_POLICY, MEMORY_LIMIT_MB, + MEMORY_REQUEST_MB, + CPU_LIMIT, + CPU_REQUEST, true, projectsRootEnvVariableProvider, chePluginsVolumeApplier); @@ -789,6 +801,9 @@ public void shouldSetSpecifiedImagePullPolicy() throws Exception { new KubernetesPluginsToolingApplier( TEST_IMAGE_POLICY, MEMORY_LIMIT_MB, + MEMORY_REQUEST_MB, + CPU_LIMIT, + CPU_REQUEST, true, projectsRootEnvVariableProvider, chePluginsVolumeApplier); @@ -814,6 +829,9 @@ public void shouldSetNullImagePullPolicyIfValueIsNotStandard() throws Exception new KubernetesPluginsToolingApplier( "None", MEMORY_LIMIT_MB, + MEMORY_REQUEST_MB, + CPU_LIMIT, + CPU_REQUEST, true, projectsRootEnvVariableProvider, chePluginsVolumeApplier); 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 476e61378af..6c387eafa4d 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 @@ -46,6 +46,9 @@ public class MachineResolverTest { private static final String DEFAULT_MEM_LIMIT = "100001"; + private static final String DEFAULT_MEM_REQUEST = "5001"; + private static final String DEFAULT_CPU_LIMIT = "2"; + private static final String DEFAULT_CPU_REQUEST = "1"; private static final String PLUGIN_NAME = "testplugin"; private static final String PLUGIN_PUBLISHER = "testpublisher"; private static final String PLUGIN_PUBLISHER_NAME = PLUGIN_PUBLISHER + "/" + PLUGIN_NAME; @@ -71,7 +74,9 @@ public void setUp() { container, cheContainer, DEFAULT_MEM_LIMIT, - defaultSidecarMemoryRequestBytes, defaultSidecarCpuLimit, defaultSidecarCpuRequest, + DEFAULT_MEM_REQUEST, + DEFAULT_CPU_LIMIT, + DEFAULT_CPU_REQUEST, endpoints, component); } diff --git a/infrastructures/openshift/src/main/java/org/eclipse/che/workspace/infrastructure/openshift/OpenShiftEnvironmentProvisioner.java b/infrastructures/openshift/src/main/java/org/eclipse/che/workspace/infrastructure/openshift/OpenShiftEnvironmentProvisioner.java index d199567fb7b..2c2d318ec0c 100644 --- a/infrastructures/openshift/src/main/java/org/eclipse/che/workspace/infrastructure/openshift/OpenShiftEnvironmentProvisioner.java +++ b/infrastructures/openshift/src/main/java/org/eclipse/che/workspace/infrastructure/openshift/OpenShiftEnvironmentProvisioner.java @@ -31,7 +31,7 @@ import org.eclipse.che.workspace.infrastructure.kubernetes.provision.VcsSshKeysProvisioner; import org.eclipse.che.workspace.infrastructure.kubernetes.provision.VcsSslCertificateProvisioner; import org.eclipse.che.workspace.infrastructure.kubernetes.provision.env.EnvVarsConverter; -import org.eclipse.che.workspace.infrastructure.kubernetes.provision.limits.ram.RamLimitRequestProvisioner; +import org.eclipse.che.workspace.infrastructure.kubernetes.provision.limits.ram.ContainerResourceLimitRequestProvisioner; import org.eclipse.che.workspace.infrastructure.kubernetes.provision.restartpolicy.RestartPolicyRewriter; import org.eclipse.che.workspace.infrastructure.kubernetes.provision.server.ServersConverter; import org.eclipse.che.workspace.infrastructure.kubernetes.server.PreviewUrlExposer; @@ -62,7 +62,7 @@ public class OpenShiftEnvironmentProvisioner private final ServersConverter serversConverter; private final EnvVarsConverter envVarsConverter; private final RestartPolicyRewriter restartPolicyRewriter; - private final RamLimitRequestProvisioner ramLimitProvisioner; + private final ContainerResourceLimitRequestProvisioner resourceLimitRequestProvisioner; private final LogsVolumeMachineProvisioner logsVolumeMachineProvisioner; private final PodTerminationGracePeriodProvisioner podTerminationGracePeriodProvisioner; private final ImagePullSecretProvisioner imagePullSecretProvisioner; @@ -83,7 +83,7 @@ public OpenShiftEnvironmentProvisioner( EnvVarsConverter envVarsConverter, RestartPolicyRewriter restartPolicyRewriter, WorkspaceVolumesStrategy volumesStrategy, - RamLimitRequestProvisioner ramLimitProvisioner, + ContainerResourceLimitRequestProvisioner resourceLimitRequestProvisioner, LogsVolumeMachineProvisioner logsVolumeMachineProvisioner, PodTerminationGracePeriodProvisioner podTerminationGracePeriodProvisioner, ImagePullSecretProvisioner imagePullSecretProvisioner, @@ -101,7 +101,7 @@ public OpenShiftEnvironmentProvisioner( this.serversConverter = serversConverter; this.envVarsConverter = envVarsConverter; this.restartPolicyRewriter = restartPolicyRewriter; - this.ramLimitProvisioner = ramLimitProvisioner; + this.resourceLimitRequestProvisioner = resourceLimitRequestProvisioner; this.logsVolumeMachineProvisioner = logsVolumeMachineProvisioner; this.podTerminationGracePeriodProvisioner = podTerminationGracePeriodProvisioner; this.imagePullSecretProvisioner = imagePullSecretProvisioner; @@ -139,7 +139,7 @@ public void provision(OpenShiftEnvironment osEnv, RuntimeIdentity identity) restartPolicyRewriter.provision(osEnv, identity); uniqueNamesProvisioner.provision(osEnv, identity); routeTlsProvisioner.provision(osEnv, identity); - ramLimitProvisioner.provision(osEnv, identity); + resourceLimitRequestProvisioner.provision(osEnv, identity); podTerminationGracePeriodProvisioner.provision(osEnv, identity); imagePullSecretProvisioner.provision(osEnv, identity); proxySettingsProvisioner.provision(osEnv, identity); 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 65038cd87c0..33067770900 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 @@ -15,9 +15,7 @@ import static org.eclipse.che.workspace.infrastructure.kubernetes.environment.PodMerger.DEPLOYMENT_NAME_LABEL; import static org.eclipse.che.workspace.infrastructure.kubernetes.namespace.KubernetesObjectUtil.setSelector; -import com.google.common.annotations.VisibleForTesting; import io.fabric8.kubernetes.api.model.ConfigMap; -import io.fabric8.kubernetes.api.model.Container; import io.fabric8.kubernetes.api.model.HasMetadata; import io.fabric8.kubernetes.api.model.PersistentVolumeClaim; import io.fabric8.kubernetes.api.model.Pod; @@ -27,7 +25,6 @@ import io.fabric8.openshift.api.model.DeploymentConfig; import io.fabric8.openshift.api.model.Route; import java.util.ArrayList; -import java.util.Collection; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -42,14 +39,12 @@ 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.MemoryAttributeProvisioner; import org.eclipse.che.api.workspace.server.spi.environment.RecipeRetriever; +import org.eclipse.che.api.workspace.server.spi.environment.ResourceLimitAttributesProvisioner; import org.eclipse.che.commons.annotation.Nullable; -import org.eclipse.che.workspace.infrastructure.kubernetes.Names; import org.eclipse.che.workspace.infrastructure.kubernetes.environment.KubernetesEnvironment.PodData; import org.eclipse.che.workspace.infrastructure.kubernetes.environment.KubernetesRecipeParser; import org.eclipse.che.workspace.infrastructure.kubernetes.environment.PodMerger; -import org.eclipse.che.workspace.infrastructure.kubernetes.util.Containers; /** * Parses {@link InternalEnvironment} into {@link OpenShiftEnvironment}. @@ -60,7 +55,7 @@ public class OpenShiftEnvironmentFactory extends InternalEnvironmentFactory void putInto(Map map, String key, T v } } - @VisibleForTesting - void addRamAttributes(Map machines, Collection pods) { - for (PodData pod : pods) { - for (Container container : pod.getSpec().getContainers()) { - final String machineName = Names.machineName(pod, container); - InternalMachineConfig machineConfig; - if ((machineConfig = machines.get(machineName)) == null) { - machineConfig = new InternalMachineConfig(); - machines.put(machineName, machineConfig); - } - memoryProvisioner.provision( - machineConfig, Containers.getRamLimit(container), Containers.getRamRequest(container)); - } - } - } +// @VisibleForTesting +// void addRamAttributes(Map machines, Collection pods) { +// for (PodData pod : pods) { +// for (Container container : pod.getSpec().getContainers()) { +// final String machineName = Names.machineName(pod, container); +// InternalMachineConfig machineConfig; +// if ((machineConfig = machines.get(machineName)) == null) { +// machineConfig = new InternalMachineConfig(); +// machines.put(machineName, machineConfig); +// } +// resourceLimitAttributesProvisioner.provision( +// machineConfig, Containers.getRamLimit(container), Containers.getRamRequest(container)); +// } +// } +// } private void checkNotNull(Object object, String errorMessage) throws ValidationException { if (object == null) { diff --git a/infrastructures/openshift/src/test/java/org/eclipse/che/workspace/infrastructure/openshift/OpenShiftEnvironmentProvisionerTest.java b/infrastructures/openshift/src/test/java/org/eclipse/che/workspace/infrastructure/openshift/OpenShiftEnvironmentProvisionerTest.java index 50ad61c3653..648fe9b4744 100644 --- a/infrastructures/openshift/src/test/java/org/eclipse/che/workspace/infrastructure/openshift/OpenShiftEnvironmentProvisionerTest.java +++ b/infrastructures/openshift/src/test/java/org/eclipse/che/workspace/infrastructure/openshift/OpenShiftEnvironmentProvisionerTest.java @@ -26,7 +26,7 @@ import org.eclipse.che.workspace.infrastructure.kubernetes.provision.VcsSshKeysProvisioner; import org.eclipse.che.workspace.infrastructure.kubernetes.provision.VcsSslCertificateProvisioner; import org.eclipse.che.workspace.infrastructure.kubernetes.provision.env.EnvVarsConverter; -import org.eclipse.che.workspace.infrastructure.kubernetes.provision.limits.ram.RamLimitRequestProvisioner; +import org.eclipse.che.workspace.infrastructure.kubernetes.provision.limits.ram.ContainerResourceLimitRequestProvisioner; import org.eclipse.che.workspace.infrastructure.kubernetes.provision.restartpolicy.RestartPolicyRewriter; import org.eclipse.che.workspace.infrastructure.kubernetes.provision.server.ServersConverter; import org.eclipse.che.workspace.infrastructure.openshift.environment.OpenShiftEnvironment; @@ -56,7 +56,7 @@ public class OpenShiftEnvironmentProvisionerTest { @Mock private EnvVarsConverter envVarsProvisioner; @Mock private ServersConverter serversProvisioner; @Mock private RestartPolicyRewriter restartPolicyRewriter; - @Mock private RamLimitRequestProvisioner ramLimitProvisioner; + @Mock private ContainerResourceLimitRequestProvisioner ramLimitProvisioner; @Mock private LogsVolumeMachineProvisioner logsVolumeMachineProvisioner; @Mock private PodTerminationGracePeriodProvisioner podTerminationGracePeriodProvisioner; @Mock private ImagePullSecretProvisioner imagePullSecretProvisioner; 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 d3653f23e5f..3cc82096c5a 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 @@ -57,7 +57,7 @@ import org.eclipse.che.api.core.ValidationException; 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.MemoryAttributeProvisioner; +import org.eclipse.che.api.workspace.server.spi.environment.ResourceLimitAttributesProvisioner; import org.eclipse.che.workspace.infrastructure.kubernetes.Names; import org.eclipse.che.workspace.infrastructure.kubernetes.environment.KubernetesEnvironment; import org.eclipse.che.workspace.infrastructure.kubernetes.environment.KubernetesEnvironment.PodData; @@ -87,7 +87,7 @@ public class OpenShiftEnvironmentFactoryTest { @Mock private InternalRecipe internalRecipe; @Mock private InternalMachineConfig machineConfig1; @Mock private InternalMachineConfig machineConfig2; - @Mock private MemoryAttributeProvisioner memoryProvisioner; + @Mock private ResourceLimitAttributesProvisioner memoryProvisioner; @Mock private KubernetesRecipeParser k8sRecipeParser; @Mock private PodMerger podMerger; diff --git a/wsmaster/che-core-api-workspace/src/main/java/org/eclipse/che/api/workspace/server/spi/environment/MemoryAttributeProvisioner.java b/wsmaster/che-core-api-workspace/src/main/java/org/eclipse/che/api/workspace/server/spi/environment/MemoryAttributeProvisioner.java deleted file mode 100644 index da0fdff74f7..00000000000 --- a/wsmaster/che-core-api-workspace/src/main/java/org/eclipse/che/api/workspace/server/spi/environment/MemoryAttributeProvisioner.java +++ /dev/null @@ -1,107 +0,0 @@ -/* - * Copyright (c) 2012-2018 Red Hat, Inc. - * This program and the accompanying materials are made - * available under the terms of the Eclipse Public License 2.0 - * which is available at https://www.eclipse.org/legal/epl-2.0/ - * - * SPDX-License-Identifier: EPL-2.0 - * - * Contributors: - * Red Hat, Inc. - initial API and implementation - */ -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 static org.eclipse.che.api.core.model.workspace.config.MachineConfig.MEMORY_REQUEST_ATTRIBUTE; - -import java.util.Map; -import javax.inject.Inject; -import javax.inject.Named; -import javax.inject.Singleton; -import org.eclipse.che.api.core.model.workspace.config.MachineConfig; -import org.eclipse.che.commons.annotation.Nullable; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - -/** - * Configures memory attributes for a given machine, if they are not present in {@link - * MachineConfig} the attributes are taken from recipe, when available, by the specific - * infrastructure implementation, or from wsmaster properties as a fallback - * - *

There are two memory-related properties: - che.workspace.default_memory_limit_mb - defines - * default machine memory limit - che.workspace.default_memory_request_mb - defines default - * requested machine memory allocation - * - *

if default requested memory allocation is greater then default memory limit, requested memory - * allocation is set to be equal to memory limit. - */ -@Singleton -public class MemoryAttributeProvisioner { - - private static final Logger LOG = LoggerFactory.getLogger(MemoryAttributeProvisioner.class); - - private final String defaultMachineMaxMemorySizeAttribute; - private final String defaultMachineRequestMemorySizeAttribute; - - @Inject - public MemoryAttributeProvisioner( - @Named("che.workspace.default_memory_limit_mb") long defaultMachineMaxMemorySizeAttribute, - @Named("che.workspace.default_memory_request_mb") - long defaultMachineRequestMemorySizeAttribute) { - // if the passed default request is greater than the default limit, request is ignored - if (defaultMachineRequestMemorySizeAttribute > defaultMachineMaxMemorySizeAttribute) { - defaultMachineRequestMemorySizeAttribute = defaultMachineMaxMemorySizeAttribute; - LOG.error( - "Requested default container memory limit is less than default memory request. Memory request parameter is ignored."); - } - - this.defaultMachineMaxMemorySizeAttribute = - String.valueOf(defaultMachineMaxMemorySizeAttribute * 1024 * 1024); - this.defaultMachineRequestMemorySizeAttribute = - String.valueOf(defaultMachineRequestMemorySizeAttribute * 1024 * 1024); - } - - /** - * Configures memory attributes, if they are missing in {@link MachineConfig} - * - *

Note: Default memory request and memory will only be used if BOTH memoryLimit and - * memoryRequest are null or 0, otherwise the provided value will be used for both parameters. - * - * @param machineConfig - given machine configuration - * @param memoryLimit - memory limit parameter configured by user in specific infra recipe. Can be - * null or 0 if defaults should be used - * @param memoryRequest - memory request parameter configured by user in specific infra recipe. - * Can be null or 0 if defaults should be used - */ - public void provision( - InternalMachineConfig machineConfig, - @Nullable Long memoryLimit, - @Nullable Long memoryRequest) { - // if both properties are not defined - if ((memoryLimit == null || memoryLimit <= 0) - && (memoryRequest == null || memoryRequest <= 0)) { - memoryLimit = Long.valueOf(defaultMachineMaxMemorySizeAttribute); - memoryRequest = Long.valueOf(defaultMachineRequestMemorySizeAttribute); - } else if ((memoryLimit == null || memoryLimit <= 0)) { // if memoryLimit only is undefined - memoryLimit = memoryRequest; - } else if ((memoryRequest == null - || memoryRequest <= 0)) { // if memoryRequest only is undefined - memoryRequest = memoryLimit; - } else if (memoryRequest > memoryLimit) { // if both properties are defined, but not consistent - memoryRequest = memoryLimit; - } - - final Map attributes = machineConfig.getAttributes(); - String configuredLimit = attributes.get(MEMORY_LIMIT_ATTRIBUTE); - String configuredRequest = attributes.get(MEMORY_REQUEST_ATTRIBUTE); - if (isNullOrEmpty(configuredLimit) && isNullOrEmpty(configuredRequest)) { - attributes.put(MEMORY_LIMIT_ATTRIBUTE, String.valueOf(memoryLimit)); - attributes.put(MEMORY_REQUEST_ATTRIBUTE, String.valueOf(memoryRequest)); - } else if (isNullOrEmpty(configuredLimit)) { - attributes.put(MEMORY_LIMIT_ATTRIBUTE, configuredRequest); - } else if (isNullOrEmpty(configuredRequest)) { - attributes.put(MEMORY_REQUEST_ATTRIBUTE, configuredLimit); - } - } -} diff --git a/wsmaster/che-core-api-workspace/src/main/java/org/eclipse/che/api/workspace/server/spi/environment/ResourceLimitAttributesProvisioner.java b/wsmaster/che-core-api-workspace/src/main/java/org/eclipse/che/api/workspace/server/spi/environment/ResourceLimitAttributesProvisioner.java new file mode 100644 index 00000000000..bc081d10292 --- /dev/null +++ b/wsmaster/che-core-api-workspace/src/main/java/org/eclipse/che/api/workspace/server/spi/environment/ResourceLimitAttributesProvisioner.java @@ -0,0 +1,140 @@ +/* + * Copyright (c) 2012-2018 Red Hat, Inc. + * This program and the accompanying materials are made + * available under the terms of the Eclipse Public License 2.0 + * which is available at https://www.eclipse.org/legal/epl-2.0/ + * + * SPDX-License-Identifier: EPL-2.0 + * + * Contributors: + * Red Hat, Inc. - initial API and implementation + */ +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.CPU_LIMIT_ATTRIBUTE; +import static org.eclipse.che.api.core.model.workspace.config.MachineConfig.CPU_REQUEST_ATTRIBUTE; +import static org.eclipse.che.api.core.model.workspace.config.MachineConfig.MEMORY_LIMIT_ATTRIBUTE; +import static org.eclipse.che.api.core.model.workspace.config.MachineConfig.MEMORY_REQUEST_ATTRIBUTE; + +import java.util.Map; +import org.eclipse.che.api.core.model.workspace.config.MachineConfig; +import org.eclipse.che.commons.annotation.Nullable; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/** + * Configures memory attributes for a given machine, if they are not present in {@link + * MachineConfig} the attributes are taken from recipe, when available, by the specific + * infrastructure implementation, or from wsmaster properties as a fallback + * + *

There are two memory-related properties: - che.workspace.default_memory_limit_mb - defines + * default machine memory limit - che.workspace.default_memory_request_mb - defines default + * requested machine memory allocation + * + *

if default requested memory allocation is greater then default memory limit, requested memory + * allocation is set to be equal to memory limit. + */ +public class ResourceLimitAttributesProvisioner { + + private static final Logger LOG = + LoggerFactory.getLogger(ResourceLimitAttributesProvisioner.class); + + /** + * Configures memory attributes, if they are missing in {@link MachineConfig} + * + *

Note: Default memory request and limit will only be used if BOTH memoryLimit and + * memoryRequest are null or 0, otherwise the provided value will be used for both parameters. + * + * @param machineConfig - given machine configuration + * @param limit - resource limit parameter configured by user in specific infra recipe. Can be 0 + * if defaults should be used + * @param request - memory request parameter configured by user in specific infra recipe. Can be 0 + * if defaults should be used + */ + public static void provisionMemory( + InternalMachineConfig machineConfig, + @Nullable long limit, + @Nullable long request, + long defaultLimit, + long defaultRequest) { + if (defaultRequest > defaultLimit) { + defaultRequest = defaultLimit; + LOG.error( + "Requested default container resource limit is less than default request. Request parameter will be ignored."); + } + + // if both properties are not defined + if (limit <= 0 && request <= 0) { + limit = defaultLimit; + request = defaultRequest; + } else if (limit <= 0) { // if limit only is undefined + limit = request; + } else if (request <= 0) { // if request only is undefined + request = limit; + } else if (request > limit) { // if both properties are defined, but not consistent + request = limit; + } + + final Map attributes = machineConfig.getAttributes(); + String configuredLimit = attributes.get(MEMORY_LIMIT_ATTRIBUTE); + String configuredRequest = attributes.get(MEMORY_REQUEST_ATTRIBUTE); + if (isNullOrEmpty(configuredLimit) && isNullOrEmpty(configuredRequest)) { + attributes.put(MEMORY_LIMIT_ATTRIBUTE, String.valueOf(limit)); + attributes.put(MEMORY_REQUEST_ATTRIBUTE, String.valueOf(request)); + } else if (isNullOrEmpty(configuredLimit)) { + attributes.put(MEMORY_LIMIT_ATTRIBUTE, configuredRequest); + } else if (isNullOrEmpty(configuredRequest)) { + attributes.put(MEMORY_REQUEST_ATTRIBUTE, configuredLimit); + } + } + + /** + * Configures CPU attributes, if they are missing in {@link MachineConfig} + * + *

Note: Default CPU request and memory will only be used if BOTH memoryLimit and memoryRequest + * are null or 0, otherwise the provided value will be used for both parameters. + * + * @param machineConfig - given machine configuration + * @param limit - resource limit parameter configured by user in specific infra recipe. Can be 0 + * if defaults should be used + * @param request - memory request parameter configured by user in specific infra recipe. Can be 0 + * if defaults should be used + */ + public static void provisionCPU( + InternalMachineConfig machineConfig, + @Nullable float limit, + @Nullable float request, + float defaultLimit, + float defaultRequest) { + if (defaultRequest > defaultLimit) { + defaultRequest = defaultLimit; + LOG.error( + "Requested default container resource limit is less than default request. Request parameter will be ignored."); + } + + // if both properties are not defined + if (limit <= 0 && request <= 0) { + limit = defaultLimit; + request = defaultRequest; + } else if (limit <= 0) { // if limit only is undefined + limit = request; + } else if (request <= 0) { // if request only is undefined + request = limit; + } else if (request > limit) { // if both properties are defined, but not consistent + request = limit; + } + + final Map attributes = machineConfig.getAttributes(); + String configuredLimit = attributes.get(CPU_LIMIT_ATTRIBUTE); + String configuredRequest = attributes.get(CPU_REQUEST_ATTRIBUTE); + if (isNullOrEmpty(configuredLimit) && isNullOrEmpty(configuredRequest)) { + attributes.put(CPU_LIMIT_ATTRIBUTE, Float.toString(limit)); + attributes.put(CPU_REQUEST_ATTRIBUTE, Float.toString(request)); + } else if (isNullOrEmpty(configuredLimit)) { + attributes.put(CPU_LIMIT_ATTRIBUTE, configuredRequest); + } else if (isNullOrEmpty(configuredRequest)) { + attributes.put(CPU_REQUEST_ATTRIBUTE, configuredLimit); + } + } +} diff --git a/wsmaster/che-core-api-workspace/src/main/java/org/eclipse/che/api/workspace/server/wsplugins/model/CheContainer.java b/wsmaster/che-core-api-workspace/src/main/java/org/eclipse/che/api/workspace/server/wsplugins/model/CheContainer.java index ddc46680f3e..92707d6524f 100644 --- a/wsmaster/che-core-api-workspace/src/main/java/org/eclipse/che/api/workspace/server/wsplugins/model/CheContainer.java +++ b/wsmaster/che-core-api-workspace/src/main/java/org/eclipse/che/api/workspace/server/wsplugins/model/CheContainer.java @@ -158,7 +158,6 @@ public void setMemoryLimit(String memoryLimit) { this.memoryLimit = memoryLimit; } - public CheContainer memoryRequest(String memoryRequest) { this.memoryRequest = memoryRequest; return this; @@ -185,7 +184,6 @@ public void setCpuLimit(String cpuLimit) { this.cpuLimit = cpuLimit; } - public CheContainer cpuRequest(String cpuRequest) { this.cpuRequest = cpuRequest; return this; @@ -199,7 +197,6 @@ public void setCpuRequest(String cpuRequest) { this.cpuRequest = cpuRequest; } - public CheContainer mountSources(boolean mountSources) { this.mountSources = mountSources; return this; diff --git a/wsmaster/che-core-api-workspace/src/test/java/org/eclipse/che/api/workspace/server/spi/environment/MemoryAttributeProvisionerTest.java b/wsmaster/che-core-api-workspace/src/test/java/org/eclipse/che/api/workspace/server/spi/environment/ResourceLimitAttributesProvisionerTest.java similarity index 68% rename from wsmaster/che-core-api-workspace/src/test/java/org/eclipse/che/api/workspace/server/spi/environment/MemoryAttributeProvisionerTest.java rename to wsmaster/che-core-api-workspace/src/test/java/org/eclipse/che/api/workspace/server/spi/environment/ResourceLimitAttributesProvisionerTest.java index 668ed68cbc4..4aa4c407a18 100644 --- a/wsmaster/che-core-api-workspace/src/test/java/org/eclipse/che/api/workspace/server/spi/environment/MemoryAttributeProvisionerTest.java +++ b/wsmaster/che-core-api-workspace/src/test/java/org/eclipse/che/api/workspace/server/spi/environment/ResourceLimitAttributesProvisionerTest.java @@ -24,52 +24,50 @@ import org.testng.annotations.Listeners; import org.testng.annotations.Test; -/** Tests {@link MemoryAttributeProvisioner} */ +/** Tests {@link ResourceLimitAttributesProvisioner} */ @Listeners(MockitoTestNGListener.class) -public class MemoryAttributeProvisionerTest { - - private MemoryAttributeProvisioner memoryAttributeProvisioner; +public class ResourceLimitAttributesProvisionerTest { @Test public void testSetsRamDefaultAttributesWhenTheyAreMissingInConfigAndNotPassedInRecipe() { long defaultMemoryLimit = 2048L; long defaultMemoryRequest = 1024L; - InternalMachineConfig machineConfig = - getInternalMachineConfig(defaultMemoryLimit, defaultMemoryRequest, new HashMap<>()); + InternalMachineConfig machineConfig = mockInternalMachineConfig(new HashMap<>()); - memoryAttributeProvisioner.provision(machineConfig, 0L, 0L); + ResourceLimitAttributesProvisioner.provisionMemory( + machineConfig, 0L, 0L, defaultMemoryLimit, defaultMemoryRequest); long memLimit = Long.parseLong(machineConfig.getAttributes().get(MEMORY_LIMIT_ATTRIBUTE)); long memRequest = Long.parseLong(machineConfig.getAttributes().get(MEMORY_REQUEST_ATTRIBUTE)); - assertEquals(memLimit, defaultMemoryLimit * 1024 * 1024); - assertEquals(memRequest, defaultMemoryRequest * 1024 * 1024); + assertEquals(memLimit, defaultMemoryLimit); + assertEquals(memRequest, defaultMemoryRequest); } @Test public void testRamDefaultMemoryRequestIsIgnoredIfGreaterThanDefaultRamLimit() { long defaultMemoryLimit = 1024L; long defaultMemoryRequest = 2048L; - InternalMachineConfig machineConfig = - getInternalMachineConfig(defaultMemoryLimit, defaultMemoryRequest, new HashMap<>()); + InternalMachineConfig machineConfig = mockInternalMachineConfig(new HashMap<>()); - memoryAttributeProvisioner.provision(machineConfig, 0L, 0L); + ResourceLimitAttributesProvisioner.provisionMemory( + machineConfig, 0L, 0L, defaultMemoryLimit, defaultMemoryRequest); long memLimit = Long.parseLong(machineConfig.getAttributes().get(MEMORY_LIMIT_ATTRIBUTE)); long memRequest = Long.parseLong(machineConfig.getAttributes().get(MEMORY_REQUEST_ATTRIBUTE)); - assertEquals(memLimit, defaultMemoryLimit * 1024 * 1024); - assertEquals(memRequest, defaultMemoryLimit * 1024 * 1024); + assertEquals(memLimit, defaultMemoryLimit); + assertEquals(memRequest, defaultMemoryLimit); } @Test public void testRamAttributesAreTakenFromRecipeWhenNotPresentInConfig() { long defaultMemoryLimit = 1024L; long defaultMemoryRequest = 2048L; - InternalMachineConfig machineConfig = - getInternalMachineConfig(defaultMemoryLimit, defaultMemoryRequest, new HashMap<>()); + InternalMachineConfig machineConfig = mockInternalMachineConfig(new HashMap<>()); long recipeLimit = 4096L; long recipeRequest = 2048L; - memoryAttributeProvisioner.provision(machineConfig, recipeLimit, recipeRequest); + ResourceLimitAttributesProvisioner.provisionMemory( + machineConfig, recipeLimit, recipeRequest, defaultMemoryLimit, defaultMemoryRequest); assertEquals( machineConfig.getAttributes().get(MEMORY_LIMIT_ATTRIBUTE), String.valueOf(recipeLimit)); @@ -82,13 +80,13 @@ public void testRamAttributesAreTakenFromRecipeWhenNotPresentInConfig() { testWhenRamAttributesTakenFromRecipeAreInconsistentAndNotPresentInConfigRequestIsIgnored() { long defaultMemoryLimit = 1024L; long defaultMemoryRequest = 2048L; - InternalMachineConfig machineConfig = - getInternalMachineConfig(defaultMemoryLimit, defaultMemoryRequest, new HashMap<>()); + InternalMachineConfig machineConfig = mockInternalMachineConfig(new HashMap<>()); // inconsistent attributes mean request > limit long recipeLimit = 2048L; long recipeRequest = 4096L; - memoryAttributeProvisioner.provision(machineConfig, recipeLimit, recipeRequest); + ResourceLimitAttributesProvisioner.provisionMemory( + machineConfig, recipeLimit, recipeRequest, defaultMemoryLimit, defaultMemoryRequest); assertEquals( machineConfig.getAttributes().get(MEMORY_LIMIT_ATTRIBUTE), String.valueOf(recipeLimit)); @@ -101,14 +99,13 @@ public void testWhenRamAttributesArePresentInMachineConfigValuesInRecipeAreIgnor long defaultMemoryLimit = 1024L; long defaultMemoryRequest = 2048L; InternalMachineConfig machineConfig = - getInternalMachineConfig( - defaultMemoryLimit, - defaultMemoryRequest, + mockInternalMachineConfig( ImmutableMap.of(MEMORY_LIMIT_ATTRIBUTE, "1526", MEMORY_REQUEST_ATTRIBUTE, "512")); long recipeLimit = 4096L; long recipeRequest = 2048L; - memoryAttributeProvisioner.provision(machineConfig, recipeLimit, recipeRequest); + ResourceLimitAttributesProvisioner.provisionMemory( + machineConfig, recipeLimit, recipeRequest, defaultMemoryLimit, defaultMemoryRequest); assertEquals(machineConfig.getAttributes().get(MEMORY_LIMIT_ATTRIBUTE), String.valueOf(1526L)); assertEquals(machineConfig.getAttributes().get(MEMORY_REQUEST_ATTRIBUTE), String.valueOf(512L)); @@ -120,12 +117,12 @@ public void testWhenRamRequestAttributeIsPresentInMachineConfigValuesInRecipeAre long defaultMemoryRequest = 2048L; Map attributes = new HashMap<>(); attributes.put(MEMORY_REQUEST_ATTRIBUTE, "512"); - InternalMachineConfig machineConfig = - getInternalMachineConfig(defaultMemoryLimit, defaultMemoryRequest, attributes); + InternalMachineConfig machineConfig = mockInternalMachineConfig(attributes); long recipeLimit = 4096L; long recipeRequest = 2048L; - memoryAttributeProvisioner.provision(machineConfig, recipeLimit, recipeRequest); + ResourceLimitAttributesProvisioner.provisionMemory( + machineConfig, recipeLimit, recipeRequest, defaultMemoryLimit, defaultMemoryRequest); assertEquals(machineConfig.getAttributes().get(MEMORY_LIMIT_ATTRIBUTE), String.valueOf(512L)); assertEquals(machineConfig.getAttributes().get(MEMORY_REQUEST_ATTRIBUTE), String.valueOf(512L)); @@ -137,12 +134,12 @@ public void testWhenRamLimitAttributeIsPresentInMachineConfigValuesInRecipeAreIg long defaultMemoryRequest = 2048L; Map attributes = new HashMap<>(); attributes.put(MEMORY_LIMIT_ATTRIBUTE, "1526"); - InternalMachineConfig machineConfig = - getInternalMachineConfig(defaultMemoryLimit, defaultMemoryRequest, attributes); + InternalMachineConfig machineConfig = mockInternalMachineConfig(attributes); long recipeLimit = 4096L; long recipeRequest = 2048L; - memoryAttributeProvisioner.provision(machineConfig, recipeLimit, recipeRequest); + ResourceLimitAttributesProvisioner.provisionMemory( + machineConfig, recipeLimit, recipeRequest, defaultMemoryLimit, defaultMemoryRequest); assertEquals(machineConfig.getAttributes().get(MEMORY_LIMIT_ATTRIBUTE), String.valueOf(1526L)); assertEquals( @@ -153,11 +150,11 @@ public void testWhenRamLimitAttributeIsPresentInMachineConfigValuesInRecipeAreIg public void testWhenRamAttributesAreNotPresentInMachineConfigAndOnlyRequestIsProvidedInRecipe() { long defaultMemoryLimit = 1024L; long defaultMemoryRequest = 2048L; - InternalMachineConfig machineConfig = - getInternalMachineConfig(defaultMemoryLimit, defaultMemoryRequest, new HashMap<>()); + InternalMachineConfig machineConfig = mockInternalMachineConfig(new HashMap<>()); long recipeRequest = 1526L; - memoryAttributeProvisioner.provision(machineConfig, null, recipeRequest); + ResourceLimitAttributesProvisioner.provisionMemory( + machineConfig, 0, recipeRequest, defaultMemoryLimit, defaultMemoryRequest); assertEquals( machineConfig.getAttributes().get(MEMORY_LIMIT_ATTRIBUTE), String.valueOf(recipeRequest)); @@ -169,11 +166,11 @@ public void testWhenRamAttributesAreNotPresentInMachineConfigAndOnlyRequestIsPro public void testWhenRamAttributesAreNotPresentInMachineConfigAndOnlyLimitIsProvidedInRecipe() { long defaultMemoryLimit = 1024L; long defaultMemoryRequest = 2048L; - InternalMachineConfig machineConfig = - getInternalMachineConfig(defaultMemoryLimit, defaultMemoryRequest, new HashMap<>()); + InternalMachineConfig machineConfig = mockInternalMachineConfig(new HashMap<>()); long recipeLimit = 1526L; - memoryAttributeProvisioner.provision(machineConfig, recipeLimit, null); + ResourceLimitAttributesProvisioner.provisionMemory( + machineConfig, recipeLimit, 0, defaultMemoryLimit, defaultMemoryRequest); assertEquals( machineConfig.getAttributes().get(MEMORY_LIMIT_ATTRIBUTE), String.valueOf(recipeLimit)); @@ -181,16 +178,11 @@ public void testWhenRamAttributesAreNotPresentInMachineConfigAndOnlyLimitIsProvi machineConfig.getAttributes().get(MEMORY_REQUEST_ATTRIBUTE), String.valueOf(recipeLimit)); } - private InternalMachineConfig getInternalMachineConfig( - long defaultMemoryLimit, long defaultMemoryRequest, Map attributes) { - memoryAttributeProvisioner = - new MemoryAttributeProvisioner(defaultMemoryLimit, defaultMemoryRequest); - return mockInternalMachineConfig(attributes); - } - private static InternalMachineConfig mockInternalMachineConfig(Map attributes) { final InternalMachineConfig machineConfigMock = mock(InternalMachineConfig.class); when(machineConfigMock.getAttributes()).thenReturn(attributes); return machineConfigMock; } + + // TODO: CPU tests } From 7eefdbb55d2083d32cf08f2a2e0955b2855c146f Mon Sep 17 00:00:00 2001 From: Max Shaposhnik Date: Fri, 7 Feb 2020 11:10:02 +0200 Subject: [PATCH 03/29] Added tests on workspace api --- ...tainerResourceLimitRequestProvisioner.java | 6 + .../ResourceLimitAttributesProvisioner.java | 94 +++++------ ...esourceLimitAttributesProvisionerTest.java | 153 +++++++++++++++++- 3 files changed, 204 insertions(+), 49 deletions(-) diff --git a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/provision/limits/ram/ContainerResourceLimitRequestProvisioner.java b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/provision/limits/ram/ContainerResourceLimitRequestProvisioner.java index 0d859f4caaf..6fd2bfe5eea 100644 --- a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/provision/limits/ram/ContainerResourceLimitRequestProvisioner.java +++ b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/provision/limits/ram/ContainerResourceLimitRequestProvisioner.java @@ -38,6 +38,12 @@ * Sets or overrides Kubernetes container RAM and CPU limit and request if corresponding attributes * are present in machine corresponding to the container. * + *

There are two memory-related properties: - che.workspace.default_memory_limit_mb - defines + * default machine memory limit - che.workspace.default_memory_request_mb - defines default + * requested machine memory allocation. Similarly, two CPU-related properties: + * - che.workspace.default_cpu_limit_cores - defines default machine CPU limit and + * - che.workspace.default_cpu_request_cores - defines default machine CPU request + * * @author Anton Korneta * @auhtor Max Shaposhnyk */ diff --git a/wsmaster/che-core-api-workspace/src/main/java/org/eclipse/che/api/workspace/server/spi/environment/ResourceLimitAttributesProvisioner.java b/wsmaster/che-core-api-workspace/src/main/java/org/eclipse/che/api/workspace/server/spi/environment/ResourceLimitAttributesProvisioner.java index bc081d10292..b0ec4998805 100644 --- a/wsmaster/che-core-api-workspace/src/main/java/org/eclipse/che/api/workspace/server/spi/environment/ResourceLimitAttributesProvisioner.java +++ b/wsmaster/che-core-api-workspace/src/main/java/org/eclipse/che/api/workspace/server/spi/environment/ResourceLimitAttributesProvisioner.java @@ -24,14 +24,10 @@ import org.slf4j.LoggerFactory; /** - * Configures memory attributes for a given machine, if they are not present in {@link + * Configures resource limits attributes for a given machine, if they are not present in {@link * MachineConfig} the attributes are taken from recipe, when available, by the specific * infrastructure implementation, or from wsmaster properties as a fallback * - *

There are two memory-related properties: - che.workspace.default_memory_limit_mb - defines - * default machine memory limit - che.workspace.default_memory_request_mb - defines default - * requested machine memory allocation - * *

if default requested memory allocation is greater then default memory limit, requested memory * allocation is set to be equal to memory limit. */ @@ -47,41 +43,43 @@ public class ResourceLimitAttributesProvisioner { * memoryRequest are null or 0, otherwise the provided value will be used for both parameters. * * @param machineConfig - given machine configuration - * @param limit - resource limit parameter configured by user in specific infra recipe. Can be 0 - * if defaults should be used - * @param request - memory request parameter configured by user in specific infra recipe. Can be 0 - * if defaults should be used + * @param memoryLimit - resource limit parameter configured by user in specific infra recipe. Can + * be 0 if defaults should be used + * @param memoryRequest - memory request parameter configured by user in specific infra recipe. + * Can be 0 if defaults should be used + * @param defaultMemoryLimit - default memory limit resource parameter value + * @param defaultMemoryRequest - default memory request resource parameter value */ public static void provisionMemory( InternalMachineConfig machineConfig, - @Nullable long limit, - @Nullable long request, - long defaultLimit, - long defaultRequest) { - if (defaultRequest > defaultLimit) { - defaultRequest = defaultLimit; + @Nullable long memoryLimit, + @Nullable long memoryRequest, + long defaultMemoryLimit, + long defaultMemoryRequest) { + if (defaultMemoryRequest > defaultMemoryLimit) { + defaultMemoryRequest = defaultMemoryLimit; LOG.error( "Requested default container resource limit is less than default request. Request parameter will be ignored."); } // if both properties are not defined - if (limit <= 0 && request <= 0) { - limit = defaultLimit; - request = defaultRequest; - } else if (limit <= 0) { // if limit only is undefined - limit = request; - } else if (request <= 0) { // if request only is undefined - request = limit; - } else if (request > limit) { // if both properties are defined, but not consistent - request = limit; + if (memoryLimit <= 0 && memoryRequest <= 0) { + memoryLimit = defaultMemoryLimit; + memoryRequest = defaultMemoryRequest; + } else if (memoryLimit <= 0) { // if limit only is undefined + memoryLimit = memoryRequest; + } else if (memoryRequest <= 0) { // if request only is undefined + memoryRequest = memoryLimit; + } else if (memoryRequest > memoryLimit) { // if both properties are defined, but not consistent + memoryRequest = memoryLimit; } final Map attributes = machineConfig.getAttributes(); String configuredLimit = attributes.get(MEMORY_LIMIT_ATTRIBUTE); String configuredRequest = attributes.get(MEMORY_REQUEST_ATTRIBUTE); if (isNullOrEmpty(configuredLimit) && isNullOrEmpty(configuredRequest)) { - attributes.put(MEMORY_LIMIT_ATTRIBUTE, String.valueOf(limit)); - attributes.put(MEMORY_REQUEST_ATTRIBUTE, String.valueOf(request)); + attributes.put(MEMORY_LIMIT_ATTRIBUTE, String.valueOf(memoryLimit)); + attributes.put(MEMORY_REQUEST_ATTRIBUTE, String.valueOf(memoryRequest)); } else if (isNullOrEmpty(configuredLimit)) { attributes.put(MEMORY_LIMIT_ATTRIBUTE, configuredRequest); } else if (isNullOrEmpty(configuredRequest)) { @@ -96,41 +94,43 @@ public static void provisionMemory( * are null or 0, otherwise the provided value will be used for both parameters. * * @param machineConfig - given machine configuration - * @param limit - resource limit parameter configured by user in specific infra recipe. Can be 0 - * if defaults should be used - * @param request - memory request parameter configured by user in specific infra recipe. Can be 0 - * if defaults should be used + * @param cpuLimit - CPU resource limit parameter configured by user in specific infra recipe. Can + * be 0 if defaults should be used + * @param cpuRequest - CPU resource request parameter configured by user in specific infra recipe. + * Can be 0 if defaults should be used + * @param defaultCPULimit - default CPU limit resource parameter value + * @param defaultCPURequest - default CPU request resource parameter value */ public static void provisionCPU( InternalMachineConfig machineConfig, - @Nullable float limit, - @Nullable float request, - float defaultLimit, - float defaultRequest) { - if (defaultRequest > defaultLimit) { - defaultRequest = defaultLimit; + @Nullable float cpuLimit, + @Nullable float cpuRequest, + float defaultCPULimit, + float defaultCPURequest) { + if (defaultCPURequest > defaultCPULimit) { + defaultCPURequest = defaultCPULimit; LOG.error( "Requested default container resource limit is less than default request. Request parameter will be ignored."); } // if both properties are not defined - if (limit <= 0 && request <= 0) { - limit = defaultLimit; - request = defaultRequest; - } else if (limit <= 0) { // if limit only is undefined - limit = request; - } else if (request <= 0) { // if request only is undefined - request = limit; - } else if (request > limit) { // if both properties are defined, but not consistent - request = limit; + if (cpuLimit <= 0 && cpuRequest <= 0) { + cpuLimit = defaultCPULimit; + cpuRequest = defaultCPURequest; + } else if (cpuLimit <= 0) { // if limit only is undefined + cpuLimit = cpuRequest; + } else if (cpuRequest <= 0) { // if request only is undefined + cpuRequest = cpuLimit; + } else if (cpuRequest > cpuLimit) { // if both properties are defined, but not consistent + cpuRequest = cpuLimit; } final Map attributes = machineConfig.getAttributes(); String configuredLimit = attributes.get(CPU_LIMIT_ATTRIBUTE); String configuredRequest = attributes.get(CPU_REQUEST_ATTRIBUTE); if (isNullOrEmpty(configuredLimit) && isNullOrEmpty(configuredRequest)) { - attributes.put(CPU_LIMIT_ATTRIBUTE, Float.toString(limit)); - attributes.put(CPU_REQUEST_ATTRIBUTE, Float.toString(request)); + attributes.put(CPU_LIMIT_ATTRIBUTE, Float.toString(cpuLimit)); + attributes.put(CPU_REQUEST_ATTRIBUTE, Float.toString(cpuRequest)); } else if (isNullOrEmpty(configuredLimit)) { attributes.put(CPU_LIMIT_ATTRIBUTE, configuredRequest); } else if (isNullOrEmpty(configuredRequest)) { diff --git a/wsmaster/che-core-api-workspace/src/test/java/org/eclipse/che/api/workspace/server/spi/environment/ResourceLimitAttributesProvisionerTest.java b/wsmaster/che-core-api-workspace/src/test/java/org/eclipse/che/api/workspace/server/spi/environment/ResourceLimitAttributesProvisionerTest.java index 4aa4c407a18..3f517ec09b8 100644 --- a/wsmaster/che-core-api-workspace/src/test/java/org/eclipse/che/api/workspace/server/spi/environment/ResourceLimitAttributesProvisionerTest.java +++ b/wsmaster/che-core-api-workspace/src/test/java/org/eclipse/che/api/workspace/server/spi/environment/ResourceLimitAttributesProvisionerTest.java @@ -11,6 +11,8 @@ */ package org.eclipse.che.api.workspace.server.spi.environment; +import static org.eclipse.che.api.core.model.workspace.config.MachineConfig.CPU_LIMIT_ATTRIBUTE; +import static org.eclipse.che.api.core.model.workspace.config.MachineConfig.CPU_REQUEST_ATTRIBUTE; import static org.eclipse.che.api.core.model.workspace.config.MachineConfig.MEMORY_LIMIT_ATTRIBUTE; import static org.eclipse.che.api.core.model.workspace.config.MachineConfig.MEMORY_REQUEST_ATTRIBUTE; import static org.mockito.Mockito.mock; @@ -178,11 +180,158 @@ public void testWhenRamAttributesAreNotPresentInMachineConfigAndOnlyLimitIsProvi machineConfig.getAttributes().get(MEMORY_REQUEST_ATTRIBUTE), String.valueOf(recipeLimit)); } + @Test + public void testSetsCPUDefaultAttributesWhenTheyAreMissingInConfigAndNotPassedInRecipe() { + float defaultCPULimit = 0.500f; + float defaultCPURequest = 0.200f; + InternalMachineConfig machineConfig = mockInternalMachineConfig(new HashMap<>()); + + ResourceLimitAttributesProvisioner.provisionCPU( + machineConfig, 0, 0, defaultCPULimit, defaultCPURequest); + float cpuLimit = Float.parseFloat(machineConfig.getAttributes().get(CPU_LIMIT_ATTRIBUTE)); + float cpuRequest = Float.parseFloat(machineConfig.getAttributes().get(CPU_REQUEST_ATTRIBUTE)); + + assertEquals(cpuLimit, defaultCPULimit); + assertEquals(cpuRequest, defaultCPURequest); + } + + @Test + public void testRamDefaultCPURequestIsIgnoredIfGreaterThanDefaultCPULimit() { + float defaultCPULimit = 0.2f; + float defaultCPURequest = 0.5f; + InternalMachineConfig machineConfig = mockInternalMachineConfig(new HashMap<>()); + + ResourceLimitAttributesProvisioner.provisionCPU( + machineConfig, 0L, 0L, defaultCPULimit, defaultCPURequest); + float memLimit = Float.parseFloat(machineConfig.getAttributes().get(CPU_LIMIT_ATTRIBUTE)); + float memRequest = Float.parseFloat(machineConfig.getAttributes().get(CPU_REQUEST_ATTRIBUTE)); + + assertEquals(memLimit, defaultCPULimit); + assertEquals(memRequest, defaultCPULimit); + } + + @Test + public void testCPUAttributesAreTakenFromRecipeWhenNotPresentInConfig() { + float defaultCPULimit = 1.0f; + float defaultCPURequest = 0.2f; + InternalMachineConfig machineConfig = mockInternalMachineConfig(new HashMap<>()); + + float recipeLimit = 4f; + float recipeRequest = 2f; + ResourceLimitAttributesProvisioner.provisionCPU( + machineConfig, recipeLimit, recipeRequest, defaultCPULimit, defaultCPURequest); + + assertEquals( + machineConfig.getAttributes().get(CPU_LIMIT_ATTRIBUTE), String.valueOf(recipeLimit)); + assertEquals( + machineConfig.getAttributes().get(CPU_REQUEST_ATTRIBUTE), String.valueOf(recipeRequest)); + } + + @Test + public void + testWhenCPUAttributesTakenFromRecipeAreInconsistentAndNotPresentInConfigRequestIsIgnored() { + float defaultCPULimit = 0.2f; + float defaultCPURequest = 0.5f; + InternalMachineConfig machineConfig = mockInternalMachineConfig(new HashMap<>()); + + // inconsistent attributes mean request > limit + float recipeLimit = 0.3f; + float recipeRequest = 0.6f; + ResourceLimitAttributesProvisioner.provisionCPU( + machineConfig, recipeLimit, recipeRequest, defaultCPULimit, defaultCPURequest); + + assertEquals( + machineConfig.getAttributes().get(CPU_LIMIT_ATTRIBUTE), String.valueOf(recipeLimit)); + assertEquals( + machineConfig.getAttributes().get(CPU_REQUEST_ATTRIBUTE), String.valueOf(recipeLimit)); + } + + @Test + public void testWhenCPUAttributesArePresentInMachineConfigValuesInRecipeAreIgnored() { + float defaultCPULimit = 0.2f; + float defaultCPURequest = 0.5f; + InternalMachineConfig machineConfig = + mockInternalMachineConfig( + ImmutableMap.of(CPU_LIMIT_ATTRIBUTE, "0.512", CPU_REQUEST_ATTRIBUTE, "0.152")); + + float recipeLimit = 0.3f; + float recipeRequest = 0.6f; + ResourceLimitAttributesProvisioner.provisionCPU( + machineConfig, recipeLimit, recipeRequest, defaultCPULimit, defaultCPURequest); + + assertEquals(machineConfig.getAttributes().get(CPU_LIMIT_ATTRIBUTE), String.valueOf(0.512f)); + assertEquals(machineConfig.getAttributes().get(CPU_REQUEST_ATTRIBUTE), String.valueOf(0.152f)); + } + + @Test + public void testWhenCPURequestAttributeIsPresentInMachineConfigValuesInRecipeAreIgnored() { + float defaultCPULimit = 0.2f; + float defaultCPURequest = 0.5f; + Map attributes = new HashMap<>(); + attributes.put(CPU_REQUEST_ATTRIBUTE, "0.512"); + InternalMachineConfig machineConfig = mockInternalMachineConfig(attributes); + + float recipeLimit = 0.3f; + float recipeRequest = 0.6f; + ResourceLimitAttributesProvisioner.provisionCPU( + machineConfig, recipeLimit, recipeRequest, defaultCPULimit, defaultCPURequest); + + assertEquals(machineConfig.getAttributes().get(CPU_LIMIT_ATTRIBUTE), String.valueOf(0.512f)); + assertEquals(machineConfig.getAttributes().get(CPU_REQUEST_ATTRIBUTE), String.valueOf(0.512f)); + } + + @Test + public void testWhenCPULimitAttributeIsPresentInMachineConfigValuesInRecipeAreIgnored() { + float defaultCPULimit = 0.2f; + float defaultCPURequest = 0.5f; + Map attributes = new HashMap<>(); + attributes.put(CPU_LIMIT_ATTRIBUTE, "0.152"); + InternalMachineConfig machineConfig = mockInternalMachineConfig(attributes); + + float recipeLimit = 0.3f; + float recipeRequest = 0.6f; + ResourceLimitAttributesProvisioner.provisionCPU( + machineConfig, recipeLimit, recipeRequest, defaultCPULimit, defaultCPURequest); + + assertEquals(machineConfig.getAttributes().get(CPU_LIMIT_ATTRIBUTE), String.valueOf(0.152f)); + assertEquals(machineConfig.getAttributes().get(CPU_REQUEST_ATTRIBUTE), String.valueOf(0.152f)); + } + + @Test + public void testWhenCPUAttributesAreNotPresentInMachineConfigAndOnlyRequestIsProvidedInRecipe() { + float defaultCPULimit = 0.2f; + float defaultCPURequest = 0.5f; + InternalMachineConfig machineConfig = mockInternalMachineConfig(new HashMap<>()); + + float recipeRequest = 0.1526f; + ResourceLimitAttributesProvisioner.provisionCPU( + machineConfig, 0, recipeRequest, defaultCPULimit, defaultCPURequest); + + assertEquals( + machineConfig.getAttributes().get(CPU_LIMIT_ATTRIBUTE), String.valueOf(recipeRequest)); + assertEquals( + machineConfig.getAttributes().get(CPU_REQUEST_ATTRIBUTE), String.valueOf(recipeRequest)); + } + + @Test + public void testWhenCPUAttributesAreNotPresentInMachineConfigAndOnlyLimitIsProvidedInRecipe() { + float defaultCPULimit = 0.2f; + float defaultCPURequest = 0.5f; + InternalMachineConfig machineConfig = mockInternalMachineConfig(new HashMap<>()); + + float recipeLimit = 0.152f; + ResourceLimitAttributesProvisioner.provisionCPU( + machineConfig, recipeLimit, 0, defaultCPULimit, defaultCPURequest); + + assertEquals( + machineConfig.getAttributes().get(CPU_LIMIT_ATTRIBUTE), String.valueOf(recipeLimit)); + assertEquals( + machineConfig.getAttributes().get(CPU_REQUEST_ATTRIBUTE), String.valueOf(recipeLimit)); + } + private static InternalMachineConfig mockInternalMachineConfig(Map attributes) { final InternalMachineConfig machineConfigMock = mock(InternalMachineConfig.class); when(machineConfigMock.getAttributes()).thenReturn(attributes); return machineConfigMock; } - - // TODO: CPU tests } From a168e418b94c4a017da932cddce690fbe07203af Mon Sep 17 00:00:00 2001 From: Max Shaposhnik Date: Fri, 7 Feb 2020 15:54:34 +0200 Subject: [PATCH 04/29] Tests, tests... --- ...tainerResourceLimitRequestProvisioner.java | 6 +-- .../kubernetes/wsplugins/MachineResolver.java | 3 ++ ...erResourceLimitRequestProvisionerTest.java | 54 +++++++------------ .../wsplugins/K8sContainerResolverTest.java | 37 +++++++++++-- .../wsplugins/MachineResolverTest.java | 50 ++++++++++++++++- 5 files changed, 107 insertions(+), 43 deletions(-) diff --git a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/provision/limits/ram/ContainerResourceLimitRequestProvisioner.java b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/provision/limits/ram/ContainerResourceLimitRequestProvisioner.java index 6fd2bfe5eea..25b213a6f44 100644 --- a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/provision/limits/ram/ContainerResourceLimitRequestProvisioner.java +++ b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/provision/limits/ram/ContainerResourceLimitRequestProvisioner.java @@ -40,9 +40,9 @@ * *

There are two memory-related properties: - che.workspace.default_memory_limit_mb - defines * default machine memory limit - che.workspace.default_memory_request_mb - defines default - * requested machine memory allocation. Similarly, two CPU-related properties: - * - che.workspace.default_cpu_limit_cores - defines default machine CPU limit and - * - che.workspace.default_cpu_request_cores - defines default machine CPU request + * requested machine memory allocation. Similarly, two CPU-related properties: - + * che.workspace.default_cpu_limit_cores - defines default machine CPU limit and - + * che.workspace.default_cpu_request_cores - defines default machine CPU request * * @author Anton Korneta * @auhtor Max Shaposhnyk 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 b9e7b5c82ff..77e379c3b54 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 @@ -116,6 +116,7 @@ private void normalizeMemory(Container container, InternalMachineConfig machineC if (ramRequest == 0) { machineConfig.getAttributes().put(MEMORY_REQUEST_ATTRIBUTE, defaultSidecarMemoryRequestBytes); } + // TODO: uncomment when CPU limit added into devfile // String overriddenSidecarMemRequest = component.getRamRequest(); // if (!isNullOrEmpty(overriddenSidecarMemRequest)) { // machineConfig @@ -131,6 +132,7 @@ private void normalizeCpu(Container container, InternalMachineConfig machineConf if (cpuLimit == 0) { machineConfig.getAttributes().put(CPU_LIMIT_ATTRIBUTE, defaultSidecarCpuLimitCores); } + // TODO: uncomment when CPU limit added into devfile // String overriddenSidecarCpuLimit = component.getCpuLimit(); // if (!isNullOrEmpty(overriddenSidecarCpuLimit)) { // machineConfig @@ -144,6 +146,7 @@ private void normalizeCpu(Container container, InternalMachineConfig machineConf if (cpuRequest == 0) { machineConfig.getAttributes().put(CPU_REQUEST_ATTRIBUTE, defaultSidecarCpuRequestCores); } + // TODO: uncomment when CPU limit added into devfile // String overriddenSidecarCpuRequest = component.getCpuRequest(); // if (!isNullOrEmpty(overriddenSidecarCpuRequest)) { // machineConfig diff --git a/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/provision/limits/ram/ContainerResourceLimitRequestProvisionerTest.java b/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/provision/limits/ram/ContainerResourceLimitRequestProvisionerTest.java index 441df00057c..b35272e524b 100644 --- a/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/provision/limits/ram/ContainerResourceLimitRequestProvisionerTest.java +++ b/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/provision/limits/ram/ContainerResourceLimitRequestProvisionerTest.java @@ -46,13 +46,13 @@ @Listeners(MockitoTestNGListener.class) public class ContainerResourceLimitRequestProvisionerTest { - public static final String POD_NAME = "web"; - public static final String CONTAINER_NAME = "app"; - public static final String MACHINE_NAME = POD_NAME + '/' + CONTAINER_NAME; - public static final String RAM_LIMIT_VALUE = "2147483648"; - public static final String RAM_REQUEST_VALUE = "1234567890"; - public static final String CPU_LIMIT_VALUE = "0.4"; - public static final String CPU_REQUEST_VALUE = "0.150"; + private static final String POD_NAME = "web"; + private static final String CONTAINER_NAME = "app"; + private static final String MACHINE_NAME = POD_NAME + '/' + CONTAINER_NAME; + private static final String RAM_LIMIT_VALUE = "2147483648"; + private static final String RAM_REQUEST_VALUE = "1234567890"; + private static final String CPU_LIMIT_VALUE = "0.4"; + private static final String CPU_REQUEST_VALUE = "0.15"; @Mock private KubernetesEnvironment k8sEnv; @Mock private RuntimeIdentity identity; @@ -60,11 +60,11 @@ public class ContainerResourceLimitRequestProvisionerTest { @Mock private ResourceLimitAttributesProvisioner resourceLimitAttributesProvisioner; private Container container; - private ContainerResourceLimitRequestProvisioner ramProvisioner; + private ContainerResourceLimitRequestProvisioner resourceProvisioner; @BeforeMethod public void setup() { - ramProvisioner = + resourceProvisioner = new ContainerResourceLimitRequestProvisioner( 1024, 512, "500m", "100m", resourceLimitAttributesProvisioner); container = new Container(); @@ -89,44 +89,30 @@ public void setup() { } @Test - public void testProvisionRamLimitAttributeToContainer() throws Exception { - ramProvisioner.provision(k8sEnv, identity); - - assertEquals(container.getResources().getLimits().get("memory").getAmount(), RAM_LIMIT_VALUE); - } - - @Test - public void testOverridesContainerRamLimitFromMachineAttribute() throws Exception { - ResourceRequirements resourceRequirements = - new ResourceRequirementsBuilder() - .addToLimits(of("memory", new Quantity("3221225472"))) - .build(); - container.setResources(resourceRequirements); - - ramProvisioner.provision(k8sEnv, identity); - + public void testProvisionResourcesLimitAndRequestAttributeToContainer() throws Exception { + resourceProvisioner.provision(k8sEnv, identity); assertEquals(container.getResources().getLimits().get("memory").getAmount(), RAM_LIMIT_VALUE); - } - - @Test - public void testProvisionRamRequestAttributeToContainer() throws Exception { - ramProvisioner.provision(k8sEnv, identity); - + assertEquals(container.getResources().getLimits().get("cpu").getAmount(), CPU_LIMIT_VALUE); assertEquals( container.getResources().getRequests().get("memory").getAmount(), RAM_REQUEST_VALUE); + assertEquals(container.getResources().getRequests().get("cpu").getAmount(), CPU_REQUEST_VALUE); } @Test - public void testOverridesContainerRamRequestFromMachineAttribute() throws Exception { + public void testOverridesContainerRamLimitAndRequestFromMachineAttribute() throws Exception { ResourceRequirements resourceRequirements = new ResourceRequirementsBuilder() - .addToRequests(of("memory", new Quantity("3221225472"))) + .addToLimits(of("memory", new Quantity("3221225472"), "cpu", new Quantity("0.678"))) + .addToRequests(of("memory", new Quantity("1231231423"), "cpu", new Quantity("0.333"))) .build(); container.setResources(resourceRequirements); - ramProvisioner.provision(k8sEnv, identity); + resourceProvisioner.provision(k8sEnv, identity); + assertEquals(container.getResources().getLimits().get("memory").getAmount(), RAM_LIMIT_VALUE); + assertEquals(container.getResources().getLimits().get("cpu").getAmount(), CPU_LIMIT_VALUE); assertEquals( container.getResources().getRequests().get("memory").getAmount(), RAM_REQUEST_VALUE); + assertEquals(container.getResources().getRequests().get("cpu").getAmount(), CPU_REQUEST_VALUE); } } diff --git a/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/wsplugins/K8sContainerResolverTest.java b/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/wsplugins/K8sContainerResolverTest.java index 110b9bd4651..88a68158964 100644 --- a/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/wsplugins/K8sContainerResolverTest.java +++ b/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/wsplugins/K8sContainerResolverTest.java @@ -110,14 +110,36 @@ public void shouldProvisionSidecarMemoryLimitAndRequest( assertEquals(container.getResources(), resources); } + @Test(dataProvider = "cpuLimitResourcesProvider") + public void shouldProvisionSidecarCPULimitAndRequest( + String sidecarCpuLimit, ResourceRequirements resources) throws Exception { + cheContainer.setCpuLimit(sidecarCpuLimit); + cheContainer.setCpuRequest(sidecarCpuLimit); + + Container container = resolver.resolve(); + + assertEquals(container.getResources(), resources); + } + @DataProvider public static Object[][] memLimitResourcesProvider() { return new Object[][] { {"", null}, {null, null}, - {"123456789", toK8sLimitRequestResources("123456789")}, - {"1Ki", toK8sLimitRequestResources("1Ki")}, - {"100M", toK8sLimitRequestResources("100M")}, + {"123456789", toK8sMemoryLimitRequestResources("123456789")}, + {"1Ki", toK8sMemoryLimitRequestResources("1Ki")}, + {"100M", toK8sMemoryLimitRequestResources("100M")}, + }; + } + + @DataProvider + public static Object[][] cpuLimitResourcesProvider() { + return new Object[][] { + {"", null}, + {null, null}, + {"0.156", toK8sCPULimitRequestResources("0.156")}, + {"1", toK8sCPULimitRequestResources("1")}, + {"100m", toK8sCPULimitRequestResources("100m")}, }; } @@ -130,13 +152,20 @@ public void shouldThrowExceptionIfMemoryLimitIsInIllegalFormat() throws Exceptio resolver.resolve(); } - private static ResourceRequirements toK8sLimitRequestResources(String memLimit) { + private static ResourceRequirements toK8sMemoryLimitRequestResources(String memLimit) { return new ResourceRequirementsBuilder() .addToLimits("memory", new Quantity(memLimit)) .addToRequests("memory", new Quantity(memLimit)) .build(); } + private static ResourceRequirements toK8sCPULimitRequestResources(String cpuLimit) { + return new ResourceRequirementsBuilder() + .addToLimits("cpu", new Quantity(cpuLimit)) + .addToRequests("cpu", new Quantity(cpuLimit)) + .build(); + } + private List toSidecarEnvVars(Map envVars) { return envVars .entrySet() 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 dc2b5be3259..81cdb5945be 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 @@ -13,7 +13,10 @@ import static com.google.common.collect.ImmutableMap.of; import static java.util.Arrays.asList; +import static org.eclipse.che.api.core.model.workspace.config.MachineConfig.CPU_LIMIT_ATTRIBUTE; +import static org.eclipse.che.api.core.model.workspace.config.MachineConfig.CPU_REQUEST_ATTRIBUTE; import static org.eclipse.che.api.core.model.workspace.config.MachineConfig.MEMORY_LIMIT_ATTRIBUTE; +import static org.eclipse.che.api.core.model.workspace.config.MachineConfig.MEMORY_REQUEST_ATTRIBUTE; import static org.testng.Assert.assertEquals; import static org.testng.Assert.assertNull; @@ -129,14 +132,25 @@ public static Object[][] serverProvider() { } @Test - public void shouldSetDefaultMemLimitIfSidecarDoesNotHaveOne() throws InfrastructureException { + public void shouldSetDefaultMemLimitAndRequestIfSidecarDoesNotHaveOne() + throws InfrastructureException { InternalMachineConfig machineConfig = resolver.resolve(); assertEquals(machineConfig.getAttributes().get(MEMORY_LIMIT_ATTRIBUTE), DEFAULT_MEM_LIMIT); + assertEquals(machineConfig.getAttributes().get(MEMORY_REQUEST_ATTRIBUTE), DEFAULT_MEM_REQUEST); + } + + @Test + public void shouldSetDefaultCPULimitAndRequestIfSidecarDoesNotHaveOne() + throws InfrastructureException { + InternalMachineConfig machineConfig = resolver.resolve(); + + assertEquals(machineConfig.getAttributes().get(CPU_LIMIT_ATTRIBUTE), DEFAULT_CPU_LIMIT); + assertEquals(machineConfig.getAttributes().get(CPU_REQUEST_ATTRIBUTE), DEFAULT_CPU_REQUEST); } @Test(dataProvider = "memoryAttributeProvider") - public void shouldSetMemoryLimitOfASidecarIfCorrespondingWSConfigAttributeIsSet( + public void shouldSetMemoryLimitAndRequestOfASidecarIfCorrespondingComponentFieldIsSet( String memoryLimit, String expectedMemLimit) throws InfrastructureException { component.setMemoryLimit(memoryLimit); @@ -156,6 +170,38 @@ public static Object[][] memoryAttributeProvider() { }; } + // TODO: uncomment when CPU limit added into devfile + // @Test(dataProvider = "cpuAttributeProvider") + // public void shouldSetCPULimitAndRequestOfASidecarIfCorrespondingComponentFieldIsSet( + // String cpuLimit, String expectedCpuLimit) throws InfrastructureException { + // component.setCpuLimit(memoryLimit); + // + // InternalMachineConfig machineConfig = resolver.resolve(); + // + // assertEquals(machineConfig.getAttributes().get(CPU_LIMIT_ATTRIBUTE), expectedCpuLimit); + // } + + // @Test(dataProvider = "cpuAttributeProvider") + // public void shouldSetCPULimitOfASidecarIfCorrespondingComponentFieldIsSet( + // String cpuLimit, String expectedCpuLimit) throws InfrastructureException { + // component.setC(cpuLimit); + // + // InternalMachineConfig machineConfig = resolver.resolve(); + // + // assertEquals(machineConfig.getAttributes().get(CPU_LIMIT_ATTRIBUTE), expectedCpuLimit); + // } + // + // @DataProvider + // public static Object[][] cpuAttributeProvider() { + // return new Object[][] { + // {"", DEFAULT_MEM_LIMIT}, + // {null, DEFAULT_MEM_LIMIT}, + // {"100Ki", toBytesString("100Ki")}, + // {"1M", toBytesString("1M")}, + // {"10Gi", toBytesString("10Gi")}, + // }; + // } + @Test public void shouldOverrideMemoryLimitOfASidecarIfCorrespondingWSConfigAttributeIsSet() throws InfrastructureException { From 5e42d6f9ed753090dc30a630096b8238d24948b9 Mon Sep 17 00:00:00 2001 From: Max Shaposhnik Date: Mon, 10 Feb 2020 17:36:28 +0200 Subject: [PATCH 05/29] Code & tests fixups --- .../webapp/WEB-INF/classes/che/che.properties | 33 ++- .../kubernetes/util/ContainersTest.java | 203 +++++++++++++++--- .../OpenShiftEnvironmentFactory.java | 33 +-- .../OpenShiftEnvironmentFactoryTest.java | 44 ++-- .../server/wsplugins/model/CheContainer.java | 4 +- 5 files changed, 230 insertions(+), 87 deletions(-) diff --git a/assembly/assembly-wsmaster-war/src/main/webapp/WEB-INF/classes/che/che.properties b/assembly/assembly-wsmaster-war/src/main/webapp/WEB-INF/classes/che/che.properties index 6c30ada0bf7..248387124f1 100644 --- a/assembly/assembly-wsmaster-war/src/main/webapp/WEB-INF/classes/che/che.properties +++ b/assembly/assembly-wsmaster-war/src/main/webapp/WEB-INF/classes/che/che.properties @@ -87,8 +87,28 @@ che.workspace.maven_server_java_options=-XX:MaxRAM=128m -XX:MaxRAMFraction=1 -XX # RAM limit default for each machine that has no RAM settings in environment. che.workspace.default_memory_limit_mb=1024 -# RAM limit default for each sidecar that has no RAM settings in Che plugin configuration. +# RAM request default for each machine that has no explicit RAM settings in environment. +# this amount will be allocated on workspace container creation +# this property might not be supported by all infrastructure implementations: +# currently it is supported by k8s and openshift +# if default memory request is more than the memory limit request will be ignored, +# and only limit will be used +che.workspace.default_memory_request_mb=512 + + +# CPU limit default for each machine that has no CPU settings in environment. +che.workspace.default_cpu_limit_cores=2 + +# CPU request default for each machine that has no CPU settings in environment. +che.workspace.default_cpu_request_cores=0.125 + +# RAM limit and request default for each sidecar that has no RAM settings in Che plugin configuration. che.workspace.sidecar.default_memory_limit_mb=128 +che.workspace.sidecar.default_memory_request_mb=64 + +# CPU limit and request default for each sidecar that has no CPU settings in Che plugin configuration. +che.workspace.sidecar.default_cpu_limit_cores=1 +che.workspace.sidecar.default_cpu_request_cores=0.115 # Define image pulling strategy for sidecars. # Possible values are: Always, Never, IfNotPresent. Any other value @@ -96,13 +116,6 @@ che.workspace.sidecar.default_memory_limit_mb=128 # or IfNotPresent otherwise.) che.workspace.sidecar.image_pull_policy=Always -# RAM request default for each machine that has no explicit RAM settings in environment. -# this amount will be allocated on workspace container creation -# this property might not be supported by all infrastructure implementations: -# currently it is supported by k8s and openshift -# if default memory request is more than the memory limit request will be ignored, -# and only limit will be used -che.workspace.default_memory_request_mb=512 # Period of inactive workspaces suspend job execution. che.workspace.activity_check_scheduler_period_s=60 @@ -462,8 +475,8 @@ che.singleport.wildcard_domain.ipless=false # Docker image of Che plugin broker app that resolves workspace tooling configuration and copies # plugins dependencies to a workspace -che.workspace.plugin_broker.metadata.image=quay.io/eclipse/che-plugin-metadata-broker:v3.1.0 -che.workspace.plugin_broker.artifacts.image=quay.io/eclipse/che-plugin-artifacts-broker:v3.1.0 +che.workspace.plugin_broker.metadata.image=mshaposh/che-plugin-metadata-broker:latest +che.workspace.plugin_broker.artifacts.image=mshaposh/che-plugin-artifacts-broker:latest # Docker image of Che plugin broker app that resolves workspace tooling configuration and copies # plugins dependencies to a workspace diff --git a/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/util/ContainersTest.java b/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/util/ContainersTest.java index 94a6b0a77c2..fb0e64438f6 100644 --- a/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/util/ContainersTest.java +++ b/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/util/ContainersTest.java @@ -15,8 +15,6 @@ import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import static org.testng.Assert.assertEquals; -import static org.testng.Assert.assertNotEquals; -import static org.testng.Assert.assertTrue; import com.google.common.collect.ImmutableMap; import io.fabric8.kubernetes.api.model.Container; @@ -42,7 +40,10 @@ @Listeners(MockitoTestNGListener.class) public class ContainersTest { - public static final long RAM_LIMIT = 2147483648L; + private static final long RAM_LIMIT = 2147483648L; + private static final long RAM_REQUEST = 1474836480L; + private static final float CPU_LIMIT = 1.782f; + private static final float CPU_REQUEST = 0.223f; @Mock private Container container; @Mock private ResourceRequirements resource; @@ -59,70 +60,136 @@ public void setup() { limits.put("cpu", new Quantity("1.5")); lenient() .when(resource.getLimits()) - .thenReturn(ImmutableMap.of("memory", new Quantity(String.valueOf(RAM_LIMIT)))); + .thenReturn( + ImmutableMap.of( + "memory", new Quantity(String.valueOf(RAM_LIMIT)), + "cpu", new Quantity(String.valueOf(CPU_LIMIT)))); + lenient() + .when(resource.getRequests()) + .thenReturn( + ImmutableMap.of( + "memory", new Quantity(String.valueOf(RAM_REQUEST)), + "cpu", new Quantity(String.valueOf(CPU_REQUEST)))); } @Test - public void testReturnContainerRamLimit() { - long actual = Containers.getRamLimit(container); + public void testReturnContainerRamLimitAndRequest() { + long limit = Containers.getRamLimit(container); + long request = (Containers.getRamRequest(container)); - assertEquals(actual, RAM_LIMIT); + assertEquals(limit, RAM_LIMIT); + assertEquals(request, RAM_REQUEST); } @Test - public void testReturnsZeroContainerRamLimitWhenResourceIsNull() { - when(container.getResources()).thenReturn(null); + public void testReturnContainerCPULimitAndRequest() { + float limit = Containers.getCpuLimit(container); + float request = (Containers.getCpuRequest(container)); + + assertEquals(limit, CPU_LIMIT); + assertEquals(request, CPU_REQUEST); + } - final long actual = Containers.getRamLimit(container); + @Test + public void testReturnsZeroWhenContainerResourcesIsNull() { + when(container.getResources()).thenReturn(null); - assertEquals(actual, 0); + assertEquals(Containers.getRamLimit(container), 0); + assertEquals(Containers.getRamRequest(container), 0); + assertEquals(Containers.getCpuLimit(container), 0, 0.0); + assertEquals(Containers.getCpuRequest(container), 0, 0.0); } @Test - public void testReturnsZeroContainerRamLimitWhenResourceDoesNotContainIt() { + public void testReturnsZeroResourceWhenResourcesDoesNotContainIt() { when(resource.getLimits()).thenReturn(Collections.emptyMap()); + when(resource.getRequests()).thenReturn(Collections.emptyMap()); - final long actual = Containers.getRamLimit(container); + assertEquals(Containers.getRamLimit(container), 0); + assertEquals(Containers.getRamRequest(container), 0); + assertEquals(Containers.getCpuLimit(container), 0, 0.0); + assertEquals(Containers.getCpuRequest(container), 0, 0.0); + } + + @Test + public void testReturnsZeroContainerLimitWhenActualValueIsNull() { + when(resource.getLimits()) + .thenReturn(ImmutableMap.of("memory", new Quantity(), "cpu", new Quantity())); - assertEquals(actual, 0); + assertEquals(Containers.getRamLimit(container), 0); + assertEquals(Containers.getCpuLimit(container), 0, 0.0); } @Test - public void testReturnsZeroContainerRamLimitWhenActualValueIsNull() { - when(resource.getLimits()).thenReturn(ImmutableMap.of("memory", new Quantity())); + public void testReturnsZeroContainerRequestWhenActualValueIsNull() { + when(resource.getRequests()) + .thenReturn(ImmutableMap.of("memory", new Quantity(), "cpu", new Quantity())); + + assertEquals(Containers.getRamRequest(container), 0); + assertEquals(Containers.getCpuRequest(container), 0, 0.0); + } - final long actual = Containers.getRamLimit(container); + // I cannot understand what we're testing here. Can you? + // @Test + // public void testOverridesContainerRamLimit() { + // Containers.addRamLimit(container, 3221225472L); + // + // assertTrue(limits.containsKey("cpu")); + // assertNotEquals(limits.get("memory"), "3221225472"); + // } + // + // @Test + // public void testAddContainerRamLimitWhenItNotPresent() { + // final Map limits = new HashMap<>(); + // when(resource.getLimits()).thenReturn(limits); + // + // Containers.addRamLimit(container, RAM_LIMIT); + // + // assertNotEquals(limits.get("memory"), String.valueOf(RAM_LIMIT)); + // } - assertEquals(actual, 0); + @Test + public void testAddContainerRamLimitWhenResourceIsNull() { + when(container.getResources()).thenReturn(null); + + Containers.addRamLimit(container, RAM_LIMIT); + + verify(container).setResources(resourceCaptor.capture()); + final ResourceRequirements captured = resourceCaptor.getValue(); + assertEquals(captured.getLimits().get("memory").getAmount(), String.valueOf(RAM_LIMIT)); } @Test - public void testOverridesContainerRamLimit() { - Containers.addRamLimit(container, 3221225472L); + public void testAddContainerCPULimitWhenResourceIsNull() { + when(container.getResources()).thenReturn(null); - assertTrue(limits.containsKey("cpu")); - assertNotEquals(limits.get("memory"), "3221225472"); + Containers.addCpuLimit(container, CPU_LIMIT); + + verify(container).setResources(resourceCaptor.capture()); + final ResourceRequirements captured = resourceCaptor.getValue(); + assertEquals(captured.getLimits().get("cpu").getAmount(), String.valueOf(CPU_LIMIT)); } @Test - public void testAddContainerRamLimitWhenItNotPresent() { - final Map limits = new HashMap<>(); - when(resource.getLimits()).thenReturn(limits); + public void testAddContainerRamRequestWhenResourceIsNull() { + when(container.getResources()).thenReturn(null); - Containers.addRamLimit(container, RAM_LIMIT); + Containers.addRamRequest(container, RAM_REQUEST); - assertNotEquals(limits.get("memory"), String.valueOf(RAM_LIMIT)); + verify(container).setResources(resourceCaptor.capture()); + final ResourceRequirements captured = resourceCaptor.getValue(); + assertEquals(captured.getRequests().get("memory").getAmount(), String.valueOf(RAM_REQUEST)); } @Test - public void testAddContainerRamLimitWhenResourceIsNull() { + public void testAddContainerCPURequestWhenResourceIsNull() { when(container.getResources()).thenReturn(null); - Containers.addRamLimit(container, RAM_LIMIT); + Containers.addCpuRequest(container, CPU_REQUEST); verify(container).setResources(resourceCaptor.capture()); final ResourceRequirements captured = resourceCaptor.getValue(); - assertEquals(captured.getLimits().get("memory").getAmount(), String.valueOf(RAM_LIMIT)); + assertEquals(captured.getRequests().get("cpu").getAmount(), String.valueOf(CPU_REQUEST)); } @Test @@ -136,6 +203,39 @@ public void testAddContainerRamLimitWhenResourceDoesNotContainAnyLimits() { assertEquals(captured.getLimits().get("memory").getAmount(), String.valueOf(RAM_LIMIT)); } + @Test + public void testAddContainerCPULimitWhenResourceDoesNotContainAnyLimits() { + when(resource.getLimits()).thenReturn(null); + + Containers.addCpuLimit(container, CPU_LIMIT); + + verify(container).setResources(resourceCaptor.capture()); + final ResourceRequirements captured = resourceCaptor.getValue(); + assertEquals(captured.getLimits().get("cpu").getAmount(), String.valueOf(CPU_LIMIT)); + } + + @Test + public void testAddContainerRamRequestWhenResourceDoesNotContainAnyLimits() { + when(resource.getLimits()).thenReturn(null); + + Containers.addRamRequest(container, RAM_REQUEST); + + verify(container).setResources(resourceCaptor.capture()); + final ResourceRequirements captured = resourceCaptor.getValue(); + assertEquals(captured.getRequests().get("memory").getAmount(), String.valueOf(RAM_REQUEST)); + } + + @Test + public void testAddContainerCPURequestWhenResourceDoesNotContainAnyLimits() { + when(resource.getLimits()).thenReturn(null); + + Containers.addCpuRequest(container, CPU_REQUEST); + + verify(container).setResources(resourceCaptor.capture()); + final ResourceRequirements captured = resourceCaptor.getValue(); + assertEquals(captured.getRequests().get("cpu").getAmount(), String.valueOf(CPU_REQUEST)); + } + @Test(dataProvider = "k8sNotionRamLimitProvider") public void testAddContainerRamLimitInK8sNotion(String ramLimit, ResourceRequirements resources) { when(container.getResources()).thenReturn(resources); @@ -147,6 +247,18 @@ public void testAddContainerRamLimitInK8sNotion(String ramLimit, ResourceRequire assertEquals(captured.getLimits().get("memory").getAmount(), ramLimit); } + @Test(dataProvider = "k8sNotionRamLimitProvider") + public void testAddContainerRamRequestInK8sNotion( + String ramRequest, ResourceRequirements resources) { + when(container.getResources()).thenReturn(resources); + + Containers.addRamRequest(container, ramRequest); + + verify(container).setResources(resourceCaptor.capture()); + ResourceRequirements captured = resourceCaptor.getValue(); + assertEquals(captured.getRequests().get("memory").getAmount(), ramRequest); + } + @DataProvider public static Object[][] k8sNotionRamLimitProvider() { return new Object[][] { @@ -157,15 +269,36 @@ public static Object[][] k8sNotionRamLimitProvider() { }; } - @Test(dataProvider = "k8sNotionRamLimitProvider") - public void testAddContainerRamRequestInK8sNotion( - String ramRequest, ResourceRequirements resources) { + @Test(dataProvider = "k8sNotionCpuLimitProvider") + public void testAddContainerCPULimitInK8sNotion(String cpuLimit, ResourceRequirements resources) { when(container.getResources()).thenReturn(resources); - Containers.addRamRequest(container, ramRequest); + Containers.addCpuLimit(container, cpuLimit); verify(container).setResources(resourceCaptor.capture()); ResourceRequirements captured = resourceCaptor.getValue(); - assertEquals(captured.getRequests().get("memory").getAmount(), ramRequest); + assertEquals(captured.getLimits().get("cpu").getAmount(), cpuLimit); + } + + @Test(dataProvider = "k8sNotionCpuLimitProvider") + public void testAddContainerCpuRequestInK8sNotion( + String cpuRequest, ResourceRequirements resources) { + when(container.getResources()).thenReturn(resources); + + Containers.addCpuRequest(container, cpuRequest); + + verify(container).setResources(resourceCaptor.capture()); + ResourceRequirements captured = resourceCaptor.getValue(); + assertEquals(captured.getRequests().get("cpu").getAmount(), cpuRequest); + } + + @DataProvider + public static Object[][] k8sNotionCpuLimitProvider() { + return new Object[][] { + {"100m", new ResourceRequirements()}, + {"1000m", new ResourceRequirements()}, + {"112m", null}, + {"155m", null}, + }; } } 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 33067770900..547bee17e1e 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 @@ -141,7 +141,7 @@ protected OpenShiftEnvironment doCreate( .setRoutes(routes) .build(); - //addRamAttributes(osEnv.getMachines(), osEnv.getPodsData().values()); + // addRamAttributes(osEnv.getMachines(), osEnv.getPodsData().values()); envValidator.validate(osEnv); @@ -208,21 +208,22 @@ private void putInto(Map map, String key, T v } } -// @VisibleForTesting -// void addRamAttributes(Map machines, Collection pods) { -// for (PodData pod : pods) { -// for (Container container : pod.getSpec().getContainers()) { -// final String machineName = Names.machineName(pod, container); -// InternalMachineConfig machineConfig; -// if ((machineConfig = machines.get(machineName)) == null) { -// machineConfig = new InternalMachineConfig(); -// machines.put(machineName, machineConfig); -// } -// resourceLimitAttributesProvisioner.provision( -// machineConfig, Containers.getRamLimit(container), Containers.getRamRequest(container)); -// } -// } -// } + // @VisibleForTesting + // void addRamAttributes(Map machines, Collection pods) { + // for (PodData pod : pods) { + // for (Container container : pod.getSpec().getContainers()) { + // final String machineName = Names.machineName(pod, container); + // InternalMachineConfig machineConfig; + // if ((machineConfig = machines.get(machineName)) == null) { + // machineConfig = new InternalMachineConfig(); + // machines.put(machineName, machineConfig); + // } + // resourceLimitAttributesProvisioner.provision( + // machineConfig, Containers.getRamLimit(container), + // Containers.getRamRequest(container)); + // } + // } + // } private void checkNotNull(Object object, String errorMessage) throws ValidationException { if (object == null) { 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 3cc82096c5a..5339bcaf523 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 @@ -17,7 +17,6 @@ import static java.util.Collections.singletonList; import static org.eclipse.che.workspace.infrastructure.kubernetes.environment.PodMerger.DEPLOYMENT_NAME_LABEL; import static org.mockito.ArgumentMatchers.any; -import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -27,7 +26,6 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; -import com.google.common.collect.ImmutableSet; import io.fabric8.kubernetes.api.model.ConfigMap; import io.fabric8.kubernetes.api.model.ConfigMapBuilder; import io.fabric8.kubernetes.api.model.Container; @@ -51,9 +49,7 @@ import io.fabric8.kubernetes.api.model.apps.DeploymentBuilder; import io.fabric8.openshift.api.model.Route; import io.fabric8.openshift.api.model.RouteBuilder; -import java.util.HashMap; import java.util.Map; -import java.util.Set; import org.eclipse.che.api.core.ValidationException; import org.eclipse.che.api.workspace.server.spi.environment.InternalMachineConfig; import org.eclipse.che.api.workspace.server.spi.environment.InternalRecipe; @@ -435,26 +431,26 @@ public void exceptionOnObjectWithNoNameSpecified() throws Exception { osEnvFactory.doCreate(internalRecipe, emptyMap(), emptyList()); } - @Test - public void testProvisionRamAttributesIsInvoked() { - final long firstMachineRamLimit = 3072 * BYTES_IN_MB; - final long secondMachineRamLimit = 1024 * BYTES_IN_MB; - final long firstMachineRamRequest = 1536 * BYTES_IN_MB; - final long secondMachineRamRequest = 512 * BYTES_IN_MB; - when(machineConfig1.getAttributes()).thenReturn(new HashMap<>()); - when(machineConfig2.getAttributes()).thenReturn(new HashMap<>()); - final Set pods = - ImmutableSet.of( - createPodData(MACHINE_NAME_1, firstMachineRamLimit, firstMachineRamRequest), - createPodData(MACHINE_NAME_2, secondMachineRamLimit, secondMachineRamRequest)); - - osEnvFactory.addRamAttributes(machines, pods); - - verify(memoryProvisioner) - .provision(eq(machineConfig1), eq(firstMachineRamLimit), eq(firstMachineRamRequest)); - verify(memoryProvisioner) - .provision(eq(machineConfig2), eq(secondMachineRamLimit), eq(secondMachineRamRequest)); - } + // @Test + // public void testProvisionRamAttributesIsInvoked() { + // final long firstMachineRamLimit = 3072 * BYTES_IN_MB; + // final long secondMachineRamLimit = 1024 * BYTES_IN_MB; + // final long firstMachineRamRequest = 1536 * BYTES_IN_MB; + // final long secondMachineRamRequest = 512 * BYTES_IN_MB; + // when(machineConfig1.getAttributes()).thenReturn(new HashMap<>()); + // when(machineConfig2.getAttributes()).thenReturn(new HashMap<>()); + // final Set pods = + // ImmutableSet.of( + // createPodData(MACHINE_NAME_1, firstMachineRamLimit, firstMachineRamRequest), + // createPodData(MACHINE_NAME_2, secondMachineRamLimit, secondMachineRamRequest)); + // + // osEnvFactory.addRamAttributes(machines, pods); + // + // verify(memoryProvisioner) + // .provision(eq(machineConfig1), eq(firstMachineRamLimit), eq(firstMachineRamRequest)); + // verify(memoryProvisioner) + // .provision(eq(machineConfig2), eq(secondMachineRamLimit), eq(secondMachineRamRequest)); + // } /** If provided {@code ramLimit} is {@code null} ram limit won't be set in POD */ private static PodData createPodData(String machineName, Long ramLimit, Long ramRequest) { diff --git a/wsmaster/che-core-api-workspace/src/main/java/org/eclipse/che/api/workspace/server/wsplugins/model/CheContainer.java b/wsmaster/che-core-api-workspace/src/main/java/org/eclipse/che/api/workspace/server/wsplugins/model/CheContainer.java index 92707d6524f..98f59a6aafa 100644 --- a/wsmaster/che-core-api-workspace/src/main/java/org/eclipse/che/api/workspace/server/wsplugins/model/CheContainer.java +++ b/wsmaster/che-core-api-workspace/src/main/java/org/eclipse/che/api/workspace/server/wsplugins/model/CheContainer.java @@ -37,10 +37,10 @@ public class CheContainer { @JsonProperty("memoryRequest") private String memoryRequest = null; - @JsonProperty("CPULimit") + @JsonProperty("cpuLimit") private String cpuLimit = null; - @JsonProperty("CPURequest") + @JsonProperty("cpuRequest") private String cpuRequest = null; @JsonProperty("mountSources") From c5fabfebaf47d56073e47c8cc0ab5bff8cc6358b Mon Sep 17 00:00:00 2001 From: Max Shaposhnik Date: Mon, 10 Feb 2020 18:07:36 +0200 Subject: [PATCH 06/29] fixup! Code & tests fixups --- .../src/main/webapp/WEB-INF/classes/che/che.properties | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/assembly/assembly-wsmaster-war/src/main/webapp/WEB-INF/classes/che/che.properties b/assembly/assembly-wsmaster-war/src/main/webapp/WEB-INF/classes/che/che.properties index 248387124f1..32d566a1379 100644 --- a/assembly/assembly-wsmaster-war/src/main/webapp/WEB-INF/classes/che/che.properties +++ b/assembly/assembly-wsmaster-war/src/main/webapp/WEB-INF/classes/che/che.properties @@ -475,8 +475,8 @@ che.singleport.wildcard_domain.ipless=false # Docker image of Che plugin broker app that resolves workspace tooling configuration and copies # plugins dependencies to a workspace -che.workspace.plugin_broker.metadata.image=mshaposh/che-plugin-metadata-broker:latest -che.workspace.plugin_broker.artifacts.image=mshaposh/che-plugin-artifacts-broker:latest +che.workspace.plugin_broker.metadata.image=quay.io/eclipse/che-plugin-metadata-broker:v3.1.0 +che.workspace.plugin_broker.artifacts.image=quay.io/eclipse/che-plugin-artifacts-broker:v3.1.0 # Docker image of Che plugin broker app that resolves workspace tooling configuration and copies # plugins dependencies to a workspace From e43e0f7ec87b06e5d1be349944cfb6949aae5b6f Mon Sep 17 00:00:00 2001 From: Max Shaposhnik Date: Tue, 11 Feb 2020 08:33:36 +0200 Subject: [PATCH 07/29] Remove commented code --- .../KubernetesEnvironmentFactory.java | 23 -------------- .../KubernetesEnvironmentFactoryTest.java | 30 +------------------ .../OpenShiftEnvironmentFactory.java | 23 -------------- .../OpenShiftEnvironmentFactoryTest.java | 26 +--------------- 4 files changed, 2 insertions(+), 100 deletions(-) diff --git a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/environment/KubernetesEnvironmentFactory.java b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/environment/KubernetesEnvironmentFactory.java index f2bb510d264..fc6ebe9f246 100644 --- a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/environment/KubernetesEnvironmentFactory.java +++ b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/environment/KubernetesEnvironmentFactory.java @@ -40,7 +40,6 @@ 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.eclipse.che.api.workspace.server.spi.environment.ResourceLimitAttributesProvisioner; import org.eclipse.che.commons.annotation.Nullable; import org.eclipse.che.workspace.infrastructure.kubernetes.Warnings; import org.eclipse.che.workspace.infrastructure.kubernetes.environment.KubernetesEnvironment.PodData; @@ -55,7 +54,6 @@ public class KubernetesEnvironmentFactory private final KubernetesRecipeParser recipeParser; private final KubernetesEnvironmentValidator envValidator; - private final ResourceLimitAttributesProvisioner memoryProvisioner; private final PodMerger podMerger; @Inject @@ -64,12 +62,10 @@ public KubernetesEnvironmentFactory( MachineConfigsValidator machinesValidator, KubernetesRecipeParser recipeParser, KubernetesEnvironmentValidator envValidator, - ResourceLimitAttributesProvisioner memoryProvisioner, PodMerger podMerger) { super(recipeRetriever, machinesValidator); this.recipeParser = recipeParser; this.envValidator = envValidator; - this.memoryProvisioner = memoryProvisioner; this.podMerger = podMerger; } @@ -146,8 +142,6 @@ protected KubernetesEnvironment doCreate( .setConfigMaps(configMaps) .build(); - // addRamAttributes(k8sEnv.getMachines(), k8sEnv.getPodsData().values()); - envValidator.validate(k8sEnv); return k8sEnv; @@ -213,23 +207,6 @@ private void putInto(Map map, String key, T v } } - // @VisibleForTesting - // void addRamAttributes(Map machines, Collection pods) { - // for (PodData pod : pods) { - // for (Container container : pod.getSpec().getContainers()) { - // final String machineName = Names.machineName(pod, container); - // InternalMachineConfig machineConfig; - // if ((machineConfig = machines.get(machineName)) == null) { - // machineConfig = new InternalMachineConfig(); - // machines.put(machineName, machineConfig); - // } - // memoryProvisioner.provisionMemory( - // machineConfig, Containers.getRamLimit(container), - // Containers.getRamRequest(container)); - // } - // } - // } - private void checkNotNull(Object object, String errorMessage) throws ValidationException { if (object == null) { throw new ValidationException(errorMessage); diff --git a/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/environment/KubernetesEnvironmentFactoryTest.java b/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/environment/KubernetesEnvironmentFactoryTest.java index 65dee6da543..c43076a4fa6 100644 --- a/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/environment/KubernetesEnvironmentFactoryTest.java +++ b/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/environment/KubernetesEnvironmentFactoryTest.java @@ -51,13 +51,11 @@ import io.fabric8.kubernetes.api.model.apps.Deployment; import io.fabric8.kubernetes.api.model.apps.DeploymentBuilder; import io.fabric8.kubernetes.api.model.extensions.IngressBuilder; -import java.util.Map; import org.eclipse.che.api.core.ValidationException; import org.eclipse.che.api.workspace.server.model.impl.WarningImpl; import org.eclipse.che.api.workspace.server.spi.environment.InternalEnvironment; 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.ResourceLimitAttributesProvisioner; import org.eclipse.che.workspace.infrastructure.kubernetes.Names; import org.eclipse.che.workspace.infrastructure.kubernetes.environment.KubernetesEnvironment.PodData; import org.mockito.Mock; @@ -84,19 +82,14 @@ public class KubernetesEnvironmentFactoryTest { @Mock private InternalRecipe internalRecipe; @Mock private InternalMachineConfig machineConfig1; @Mock private InternalMachineConfig machineConfig2; - @Mock private ResourceLimitAttributesProvisioner memoryProvisioner; @Mock private KubernetesRecipeParser k8sRecipeParser; @Mock private PodMerger podMerger; - private Map machines; - @BeforeMethod public void setup() throws Exception { k8sEnvFactory = - new KubernetesEnvironmentFactory( - null, null, k8sRecipeParser, k8sEnvValidator, memoryProvisioner, podMerger); + new KubernetesEnvironmentFactory(null, null, k8sRecipeParser, k8sEnvValidator, podMerger); lenient().when(internalEnvironment.getRecipe()).thenReturn(internalRecipe); - machines = ImmutableMap.of(MACHINE_NAME_1, machineConfig1, MACHINE_NAME_2, machineConfig2); } @Test @@ -435,27 +428,6 @@ public void exceptionOnObjectWithNoNameSpecified() throws Exception { k8sEnvFactory.doCreate(internalRecipe, emptyMap(), emptyList()); } - // @Test - // public void testProvisionRamAttributesIsInvoked() { - // final long firstMachineRamLimit = 3072; - // final long firstMachineRamRequest = 1536; - // final long secondMachineRamLimit = 1024; - // final long secondMachineRamRequest = 512; - // when(machineConfig1.getAttributes()).thenReturn(new HashMap<>()); - // when(machineConfig2.getAttributes()).thenReturn(new HashMap<>()); - // final Set pods = - // ImmutableSet.of( - // createPodData(MACHINE_NAME_1, firstMachineRamLimit, firstMachineRamRequest), - // createPodData(MACHINE_NAME_2, secondMachineRamLimit, secondMachineRamRequest)); - // - // k8sEnvFactory.addRamAttributes(machines, pods); - // - // verify(memoryProvisioner) - // .provision(eq(machineConfig1), eq(firstMachineRamLimit), eq(firstMachineRamRequest)); - // verify(memoryProvisioner) - // .provision(eq(machineConfig2), eq(secondMachineRamLimit), eq(secondMachineRamRequest)); - // } - private static PodData createPodData(String machineName, long ramLimit, long ramRequest) { final String containerName = "container_" + machineName; final Container containerMock = mock(Container.class); 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 547bee17e1e..cf9de2c671e 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 @@ -40,7 +40,6 @@ 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.eclipse.che.api.workspace.server.spi.environment.ResourceLimitAttributesProvisioner; import org.eclipse.che.commons.annotation.Nullable; import org.eclipse.che.workspace.infrastructure.kubernetes.environment.KubernetesEnvironment.PodData; import org.eclipse.che.workspace.infrastructure.kubernetes.environment.KubernetesRecipeParser; @@ -55,7 +54,6 @@ public class OpenShiftEnvironmentFactory extends InternalEnvironmentFactory void putInto(Map map, String key, T v } } - // @VisibleForTesting - // void addRamAttributes(Map machines, Collection pods) { - // for (PodData pod : pods) { - // for (Container container : pod.getSpec().getContainers()) { - // final String machineName = Names.machineName(pod, container); - // InternalMachineConfig machineConfig; - // if ((machineConfig = machines.get(machineName)) == null) { - // machineConfig = new InternalMachineConfig(); - // machines.put(machineName, machineConfig); - // } - // resourceLimitAttributesProvisioner.provision( - // machineConfig, Containers.getRamLimit(container), - // Containers.getRamRequest(container)); - // } - // } - // } - private void checkNotNull(Object object, String errorMessage) throws ValidationException { if (object == null) { throw new ValidationException(errorMessage); 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 5339bcaf523..1c5b68b45d9 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 @@ -53,7 +53,6 @@ import org.eclipse.che.api.core.ValidationException; 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.ResourceLimitAttributesProvisioner; import org.eclipse.che.workspace.infrastructure.kubernetes.Names; import org.eclipse.che.workspace.infrastructure.kubernetes.environment.KubernetesEnvironment; import org.eclipse.che.workspace.infrastructure.kubernetes.environment.KubernetesEnvironment.PodData; @@ -73,7 +72,6 @@ @Listeners(MockitoTestNGListener.class) public class OpenShiftEnvironmentFactoryTest { - 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"; @@ -83,7 +81,6 @@ public class OpenShiftEnvironmentFactoryTest { @Mock private InternalRecipe internalRecipe; @Mock private InternalMachineConfig machineConfig1; @Mock private InternalMachineConfig machineConfig2; - @Mock private ResourceLimitAttributesProvisioner memoryProvisioner; @Mock private KubernetesRecipeParser k8sRecipeParser; @Mock private PodMerger podMerger; @@ -93,7 +90,7 @@ public class OpenShiftEnvironmentFactoryTest { public void setup() throws Exception { osEnvFactory = new OpenShiftEnvironmentFactory( - null, null, openShiftEnvValidator, k8sRecipeParser, memoryProvisioner, podMerger); + null, null, openShiftEnvValidator, k8sRecipeParser, podMerger); machines = ImmutableMap.of(MACHINE_NAME_1, machineConfig1, MACHINE_NAME_2, machineConfig2); } @@ -431,27 +428,6 @@ public void exceptionOnObjectWithNoNameSpecified() throws Exception { osEnvFactory.doCreate(internalRecipe, emptyMap(), emptyList()); } - // @Test - // public void testProvisionRamAttributesIsInvoked() { - // final long firstMachineRamLimit = 3072 * BYTES_IN_MB; - // final long secondMachineRamLimit = 1024 * BYTES_IN_MB; - // final long firstMachineRamRequest = 1536 * BYTES_IN_MB; - // final long secondMachineRamRequest = 512 * BYTES_IN_MB; - // when(machineConfig1.getAttributes()).thenReturn(new HashMap<>()); - // when(machineConfig2.getAttributes()).thenReturn(new HashMap<>()); - // final Set pods = - // ImmutableSet.of( - // createPodData(MACHINE_NAME_1, firstMachineRamLimit, firstMachineRamRequest), - // createPodData(MACHINE_NAME_2, secondMachineRamLimit, secondMachineRamRequest)); - // - // osEnvFactory.addRamAttributes(machines, pods); - // - // verify(memoryProvisioner) - // .provision(eq(machineConfig1), eq(firstMachineRamLimit), eq(firstMachineRamRequest)); - // verify(memoryProvisioner) - // .provision(eq(machineConfig2), eq(secondMachineRamLimit), eq(secondMachineRamRequest)); - // } - /** If provided {@code ramLimit} is {@code null} ram limit won't be set in POD */ private static PodData createPodData(String machineName, Long ramLimit, Long ramRequest) { final String containerName = "container_" + machineName; From c04b049f5966cec964fcb97f093cffdf8500abd6 Mon Sep 17 00:00:00 2001 From: Max Shaposhnik Date: Tue, 11 Feb 2020 09:02:10 +0200 Subject: [PATCH 08/29] Javadoc fixes --- .../src/main/webapp/WEB-INF/classes/che/che.properties | 6 +++--- .../api/core/model/workspace/config/MachineConfig.java | 8 ++++---- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/assembly/assembly-wsmaster-war/src/main/webapp/WEB-INF/classes/che/che.properties b/assembly/assembly-wsmaster-war/src/main/webapp/WEB-INF/classes/che/che.properties index 32d566a1379..94d661c681e 100644 --- a/assembly/assembly-wsmaster-war/src/main/webapp/WEB-INF/classes/che/che.properties +++ b/assembly/assembly-wsmaster-war/src/main/webapp/WEB-INF/classes/che/che.properties @@ -96,10 +96,9 @@ che.workspace.default_memory_limit_mb=1024 che.workspace.default_memory_request_mb=512 -# CPU limit default for each machine that has no CPU settings in environment. +# CPU limit and reqquest default for each machine that has no CPU settings in environment. +# Can be specified either in floating point cores number, e.g. 0.125 or in K8S format integer millicores e.g. 125m che.workspace.default_cpu_limit_cores=2 - -# CPU request default for each machine that has no CPU settings in environment. che.workspace.default_cpu_request_cores=0.125 # RAM limit and request default for each sidecar that has no RAM settings in Che plugin configuration. @@ -107,6 +106,7 @@ che.workspace.sidecar.default_memory_limit_mb=128 che.workspace.sidecar.default_memory_request_mb=64 # CPU limit and request default for each sidecar that has no CPU settings in Che plugin configuration. +# Can be specified either in floating point cores number, e.g. 0.125 or in K8S format integer millicores e.g. 125m che.workspace.sidecar.default_cpu_limit_cores=1 che.workspace.sidecar.default_cpu_request_cores=0.115 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 76234253bd1..6d36e4d4446 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 @@ -40,15 +40,15 @@ public interface MachineConfig { /** * Name of the attribute from {@link #getAttributes()} which if present defines CPU limit of the - * machine in millicores. + * machine in cores. */ - String CPU_LIMIT_ATTRIBUTE = "cpuLimitMillicores"; + String CPU_LIMIT_ATTRIBUTE = "cpuLimitCores"; /** * Name of the attribute from {@link #getAttributes()} which if present defines requested CPU - * allocation of the machine in millicores. + * allocation of the machine in cores. */ - String CPU_REQUEST_ATTRIBUTE = "cpuRequestMillicores"; + String CPU_REQUEST_ATTRIBUTE = "cpuRequestCores"; /** * Name of the attribute from {@link #getAttributes()} which, if present, defines the entrypoint From 44b45a84fe62efb91be0f82dc7873ae552afcaa6 Mon Sep 17 00:00:00 2001 From: Max Shaposhnik Date: Tue, 11 Feb 2020 17:18:53 +0200 Subject: [PATCH 09/29] Fix bug wrong limits applied --- .../ResourceLimitAttributesProvisioner.java | 16 +++---- ...esourceLimitAttributesProvisionerTest.java | 44 ++++++++++--------- 2 files changed, 30 insertions(+), 30 deletions(-) diff --git a/wsmaster/che-core-api-workspace/src/main/java/org/eclipse/che/api/workspace/server/spi/environment/ResourceLimitAttributesProvisioner.java b/wsmaster/che-core-api-workspace/src/main/java/org/eclipse/che/api/workspace/server/spi/environment/ResourceLimitAttributesProvisioner.java index b0ec4998805..6368ead32bc 100644 --- a/wsmaster/che-core-api-workspace/src/main/java/org/eclipse/che/api/workspace/server/spi/environment/ResourceLimitAttributesProvisioner.java +++ b/wsmaster/che-core-api-workspace/src/main/java/org/eclipse/che/api/workspace/server/spi/environment/ResourceLimitAttributesProvisioner.java @@ -77,13 +77,11 @@ public static void provisionMemory( final Map attributes = machineConfig.getAttributes(); String configuredLimit = attributes.get(MEMORY_LIMIT_ATTRIBUTE); String configuredRequest = attributes.get(MEMORY_REQUEST_ATTRIBUTE); - if (isNullOrEmpty(configuredLimit) && isNullOrEmpty(configuredRequest)) { + if (isNullOrEmpty(configuredLimit)) { attributes.put(MEMORY_LIMIT_ATTRIBUTE, String.valueOf(memoryLimit)); + } + if (isNullOrEmpty(configuredRequest)) { attributes.put(MEMORY_REQUEST_ATTRIBUTE, String.valueOf(memoryRequest)); - } else if (isNullOrEmpty(configuredLimit)) { - attributes.put(MEMORY_LIMIT_ATTRIBUTE, configuredRequest); - } else if (isNullOrEmpty(configuredRequest)) { - attributes.put(MEMORY_REQUEST_ATTRIBUTE, configuredLimit); } } @@ -128,13 +126,11 @@ public static void provisionCPU( final Map attributes = machineConfig.getAttributes(); String configuredLimit = attributes.get(CPU_LIMIT_ATTRIBUTE); String configuredRequest = attributes.get(CPU_REQUEST_ATTRIBUTE); - if (isNullOrEmpty(configuredLimit) && isNullOrEmpty(configuredRequest)) { + if (isNullOrEmpty(configuredLimit)) { attributes.put(CPU_LIMIT_ATTRIBUTE, Float.toString(cpuLimit)); + } + if (isNullOrEmpty(configuredRequest)) { attributes.put(CPU_REQUEST_ATTRIBUTE, Float.toString(cpuRequest)); - } else if (isNullOrEmpty(configuredLimit)) { - attributes.put(CPU_LIMIT_ATTRIBUTE, configuredRequest); - } else if (isNullOrEmpty(configuredRequest)) { - attributes.put(CPU_REQUEST_ATTRIBUTE, configuredLimit); } } } diff --git a/wsmaster/che-core-api-workspace/src/test/java/org/eclipse/che/api/workspace/server/spi/environment/ResourceLimitAttributesProvisionerTest.java b/wsmaster/che-core-api-workspace/src/test/java/org/eclipse/che/api/workspace/server/spi/environment/ResourceLimitAttributesProvisionerTest.java index 3f517ec09b8..978beb6e67f 100644 --- a/wsmaster/che-core-api-workspace/src/test/java/org/eclipse/che/api/workspace/server/spi/environment/ResourceLimitAttributesProvisionerTest.java +++ b/wsmaster/che-core-api-workspace/src/test/java/org/eclipse/che/api/workspace/server/spi/environment/ResourceLimitAttributesProvisionerTest.java @@ -126,7 +126,8 @@ public void testWhenRamRequestAttributeIsPresentInMachineConfigValuesInRecipeAre ResourceLimitAttributesProvisioner.provisionMemory( machineConfig, recipeLimit, recipeRequest, defaultMemoryLimit, defaultMemoryRequest); - assertEquals(machineConfig.getAttributes().get(MEMORY_LIMIT_ATTRIBUTE), String.valueOf(512L)); + assertEquals( + machineConfig.getAttributes().get(MEMORY_LIMIT_ATTRIBUTE), String.valueOf(recipeLimit)); assertEquals(machineConfig.getAttributes().get(MEMORY_REQUEST_ATTRIBUTE), String.valueOf(512L)); } @@ -145,7 +146,8 @@ public void testWhenRamLimitAttributeIsPresentInMachineConfigValuesInRecipeAreIg assertEquals(machineConfig.getAttributes().get(MEMORY_LIMIT_ATTRIBUTE), String.valueOf(1526L)); assertEquals( - machineConfig.getAttributes().get(MEMORY_REQUEST_ATTRIBUTE), String.valueOf(1526L)); + machineConfig.getAttributes().get(MEMORY_REQUEST_ATTRIBUTE), + String.valueOf(defaultMemoryRequest)); } @Test @@ -248,14 +250,14 @@ public void testCPUAttributesAreTakenFromRecipeWhenNotPresentInConfig() { @Test public void testWhenCPUAttributesArePresentInMachineConfigValuesInRecipeAreIgnored() { - float defaultCPULimit = 0.2f; - float defaultCPURequest = 0.5f; + float defaultCPULimit = 0.5f; + float defaultCPURequest = 0.2f; InternalMachineConfig machineConfig = mockInternalMachineConfig( ImmutableMap.of(CPU_LIMIT_ATTRIBUTE, "0.512", CPU_REQUEST_ATTRIBUTE, "0.152")); - float recipeLimit = 0.3f; - float recipeRequest = 0.6f; + float recipeLimit = 0.6f; + float recipeRequest = 0.3f; ResourceLimitAttributesProvisioner.provisionCPU( machineConfig, recipeLimit, recipeRequest, defaultCPULimit, defaultCPURequest); @@ -265,42 +267,44 @@ public void testWhenCPUAttributesArePresentInMachineConfigValuesInRecipeAreIgnor @Test public void testWhenCPURequestAttributeIsPresentInMachineConfigValuesInRecipeAreIgnored() { - float defaultCPULimit = 0.2f; - float defaultCPURequest = 0.5f; + float defaultCPULimit = 0.5f; + float defaultCPURequest = 0.2f; Map attributes = new HashMap<>(); attributes.put(CPU_REQUEST_ATTRIBUTE, "0.512"); InternalMachineConfig machineConfig = mockInternalMachineConfig(attributes); - float recipeLimit = 0.3f; - float recipeRequest = 0.6f; + float recipeLimit = 0.6f; + float recipeRequest = 0.3f; ResourceLimitAttributesProvisioner.provisionCPU( machineConfig, recipeLimit, recipeRequest, defaultCPULimit, defaultCPURequest); - assertEquals(machineConfig.getAttributes().get(CPU_LIMIT_ATTRIBUTE), String.valueOf(0.512f)); + assertEquals( + machineConfig.getAttributes().get(CPU_LIMIT_ATTRIBUTE), String.valueOf(recipeLimit)); assertEquals(machineConfig.getAttributes().get(CPU_REQUEST_ATTRIBUTE), String.valueOf(0.512f)); } @Test public void testWhenCPULimitAttributeIsPresentInMachineConfigValuesInRecipeAreIgnored() { - float defaultCPULimit = 0.2f; - float defaultCPURequest = 0.5f; + float defaultCPULimit = 0.5f; + float defaultCPURequest = 0.2f; Map attributes = new HashMap<>(); attributes.put(CPU_LIMIT_ATTRIBUTE, "0.152"); InternalMachineConfig machineConfig = mockInternalMachineConfig(attributes); - float recipeLimit = 0.3f; - float recipeRequest = 0.6f; + float recipeLimit = 0.6f; + float recipeRequest = 0.3f; ResourceLimitAttributesProvisioner.provisionCPU( machineConfig, recipeLimit, recipeRequest, defaultCPULimit, defaultCPURequest); assertEquals(machineConfig.getAttributes().get(CPU_LIMIT_ATTRIBUTE), String.valueOf(0.152f)); - assertEquals(machineConfig.getAttributes().get(CPU_REQUEST_ATTRIBUTE), String.valueOf(0.152f)); + assertEquals( + machineConfig.getAttributes().get(CPU_REQUEST_ATTRIBUTE), String.valueOf(recipeRequest)); } @Test public void testWhenCPUAttributesAreNotPresentInMachineConfigAndOnlyRequestIsProvidedInRecipe() { - float defaultCPULimit = 0.2f; - float defaultCPURequest = 0.5f; + float defaultCPULimit = 0.5f; + float defaultCPURequest = 0.2f; InternalMachineConfig machineConfig = mockInternalMachineConfig(new HashMap<>()); float recipeRequest = 0.1526f; @@ -315,8 +319,8 @@ public void testWhenCPUAttributesAreNotPresentInMachineConfigAndOnlyRequestIsPro @Test public void testWhenCPUAttributesAreNotPresentInMachineConfigAndOnlyLimitIsProvidedInRecipe() { - float defaultCPULimit = 0.2f; - float defaultCPURequest = 0.5f; + float defaultCPULimit = 0.5f; + float defaultCPURequest = 0.2f; InternalMachineConfig machineConfig = mockInternalMachineConfig(new HashMap<>()); float recipeLimit = 0.152f; From 83eeaee47e6297559a38758029207ab73e04dd6b Mon Sep 17 00:00:00 2001 From: Max Shaposhnik Date: Tue, 11 Feb 2020 17:32:11 +0200 Subject: [PATCH 10/29] review fixup --- .../ResourceLimitAttributesProvisioner.java | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/wsmaster/che-core-api-workspace/src/main/java/org/eclipse/che/api/workspace/server/spi/environment/ResourceLimitAttributesProvisioner.java b/wsmaster/che-core-api-workspace/src/main/java/org/eclipse/che/api/workspace/server/spi/environment/ResourceLimitAttributesProvisioner.java index 6368ead32bc..98d37a77a9e 100644 --- a/wsmaster/che-core-api-workspace/src/main/java/org/eclipse/che/api/workspace/server/spi/environment/ResourceLimitAttributesProvisioner.java +++ b/wsmaster/che-core-api-workspace/src/main/java/org/eclipse/che/api/workspace/server/spi/environment/ResourceLimitAttributesProvisioner.java @@ -19,7 +19,6 @@ import java.util.Map; import org.eclipse.che.api.core.model.workspace.config.MachineConfig; -import org.eclipse.che.commons.annotation.Nullable; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -52,8 +51,8 @@ public class ResourceLimitAttributesProvisioner { */ public static void provisionMemory( InternalMachineConfig machineConfig, - @Nullable long memoryLimit, - @Nullable long memoryRequest, + long memoryLimit, + long memoryRequest, long defaultMemoryLimit, long defaultMemoryRequest) { if (defaultMemoryRequest > defaultMemoryLimit) { @@ -101,13 +100,13 @@ public static void provisionMemory( */ public static void provisionCPU( InternalMachineConfig machineConfig, - @Nullable float cpuLimit, - @Nullable float cpuRequest, + float cpuLimit, + float cpuRequest, float defaultCPULimit, float defaultCPURequest) { if (defaultCPURequest > defaultCPULimit) { defaultCPURequest = defaultCPULimit; - LOG.error( + LOG.warn( "Requested default container resource limit is less than default request. Request parameter will be ignored."); } From 0323aa6e75dbcd3b769d6af3c2589e7da75f8012 Mon Sep 17 00:00:00 2001 From: Max Shaposhnik Date: Tue, 11 Feb 2020 17:35:36 +0200 Subject: [PATCH 11/29] fixup! review fixup --- .../spi/environment/ResourceLimitAttributesProvisioner.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/wsmaster/che-core-api-workspace/src/main/java/org/eclipse/che/api/workspace/server/spi/environment/ResourceLimitAttributesProvisioner.java b/wsmaster/che-core-api-workspace/src/main/java/org/eclipse/che/api/workspace/server/spi/environment/ResourceLimitAttributesProvisioner.java index 98d37a77a9e..14a042cf115 100644 --- a/wsmaster/che-core-api-workspace/src/main/java/org/eclipse/che/api/workspace/server/spi/environment/ResourceLimitAttributesProvisioner.java +++ b/wsmaster/che-core-api-workspace/src/main/java/org/eclipse/che/api/workspace/server/spi/environment/ResourceLimitAttributesProvisioner.java @@ -57,7 +57,7 @@ public static void provisionMemory( long defaultMemoryRequest) { if (defaultMemoryRequest > defaultMemoryLimit) { defaultMemoryRequest = defaultMemoryLimit; - LOG.error( + LOG.warn( "Requested default container resource limit is less than default request. Request parameter will be ignored."); } From 533ee40b92d614db972e240c54c752428dda6d0f Mon Sep 17 00:00:00 2001 From: Max Shaposhnik Date: Wed, 12 Feb 2020 11:34:34 +0200 Subject: [PATCH 12/29] Change default request Co-Authored-By: Sergii Kabashniuk --- .../src/main/webapp/WEB-INF/classes/che/che.properties | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/assembly/assembly-wsmaster-war/src/main/webapp/WEB-INF/classes/che/che.properties b/assembly/assembly-wsmaster-war/src/main/webapp/WEB-INF/classes/che/che.properties index 94d661c681e..680e364dc98 100644 --- a/assembly/assembly-wsmaster-war/src/main/webapp/WEB-INF/classes/che/che.properties +++ b/assembly/assembly-wsmaster-war/src/main/webapp/WEB-INF/classes/che/che.properties @@ -93,7 +93,7 @@ che.workspace.default_memory_limit_mb=1024 # currently it is supported by k8s and openshift # if default memory request is more than the memory limit request will be ignored, # and only limit will be used -che.workspace.default_memory_request_mb=512 +che.workspace.default_memory_request_mb=200 # CPU limit and reqquest default for each machine that has no CPU settings in environment. From 496b2734739a2fd35cdafea98237ad8d7c2c2e32 Mon Sep 17 00:00:00 2001 From: Max Shaposhnik Date: Wed, 12 Feb 2020 14:07:39 +0200 Subject: [PATCH 13/29] Refactor some descriptions; --- .../webapp/WEB-INF/classes/che/che.properties | 8 +++++-- ...tainerResourceLimitRequestProvisioner.java | 21 +++++++++++++------ 2 files changed, 21 insertions(+), 8 deletions(-) diff --git a/assembly/assembly-wsmaster-war/src/main/webapp/WEB-INF/classes/che/che.properties b/assembly/assembly-wsmaster-war/src/main/webapp/WEB-INF/classes/che/che.properties index 94d661c681e..e36c62ef982 100644 --- a/assembly/assembly-wsmaster-war/src/main/webapp/WEB-INF/classes/che/che.properties +++ b/assembly/assembly-wsmaster-war/src/main/webapp/WEB-INF/classes/che/che.properties @@ -91,14 +91,18 @@ che.workspace.default_memory_limit_mb=1024 # this amount will be allocated on workspace container creation # this property might not be supported by all infrastructure implementations: # currently it is supported by k8s and openshift -# if default memory request is more than the memory limit request will be ignored, +# if default memory request is more than the memory limit, request will be ignored, # and only limit will be used che.workspace.default_memory_request_mb=512 -# CPU limit and reqquest default for each machine that has no CPU settings in environment. +# CPU limit default for each machine that has no CPU settings in environment. # Can be specified either in floating point cores number, e.g. 0.125 or in K8S format integer millicores e.g. 125m che.workspace.default_cpu_limit_cores=2 + +# CPU request default for each machine that has no CPU settings in environment. +# if default CPU request is more than the CPU limit, request will be ignored, +# and only limit will be used che.workspace.default_cpu_request_cores=0.125 # RAM limit and request default for each sidecar that has no RAM settings in Che plugin configuration. diff --git a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/provision/limits/ram/ContainerResourceLimitRequestProvisioner.java b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/provision/limits/ram/ContainerResourceLimitRequestProvisioner.java index 25b213a6f44..867022b1e03 100644 --- a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/provision/limits/ram/ContainerResourceLimitRequestProvisioner.java +++ b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/provision/limits/ram/ContainerResourceLimitRequestProvisioner.java @@ -36,13 +36,22 @@ /** * Sets or overrides Kubernetes container RAM and CPU limit and request if corresponding attributes - * are present in machine corresponding to the container. + * are present in machine config corresponding to the container. * - *

There are two memory-related properties: - che.workspace.default_memory_limit_mb - defines - * default machine memory limit - che.workspace.default_memory_request_mb - defines default - * requested machine memory allocation. Similarly, two CPU-related properties: - - * che.workspace.default_cpu_limit_cores - defines default machine CPU limit and - - * che.workspace.default_cpu_request_cores - defines default machine CPU request + *

There are two memory-related properties: + * + *

    + *
  • -che.workspace.default_memory_limit_mb - defines default machine memory limit + *
  • -che.workspace.default_memory_request_mb - defines default requested machine memory + * allocation. + *
+ * + *

Similarly, there are two CPU-related properties: + * + *

    + *
  • -che.workspace.default_cpu_limit_cores - defines default machine CPU limit + *
  • -che.workspace.default_cpu_request_cores - defines default machine CPU request + *
* * @author Anton Korneta * @auhtor Max Shaposhnyk From 013dcb002a31086ad9588e892a06c0950fa7056b Mon Sep 17 00:00:00 2001 From: Max Shaposhnik Date: Thu, 13 Feb 2020 11:01:11 +0200 Subject: [PATCH 14/29] Model class fixup --- .../server/wsplugins/model/CheContainer.java | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/wsmaster/che-core-api-workspace/src/main/java/org/eclipse/che/api/workspace/server/wsplugins/model/CheContainer.java b/wsmaster/che-core-api-workspace/src/main/java/org/eclipse/che/api/workspace/server/wsplugins/model/CheContainer.java index 98f59a6aafa..936b6abdff5 100644 --- a/wsmaster/che-core-api-workspace/src/main/java/org/eclipse/che/api/workspace/server/wsplugins/model/CheContainer.java +++ b/wsmaster/che-core-api-workspace/src/main/java/org/eclipse/che/api/workspace/server/wsplugins/model/CheContainer.java @@ -257,6 +257,9 @@ public boolean equals(Object o) { && Objects.equals(getVolumes(), that.getVolumes()) && Objects.equals(getPorts(), that.getPorts()) && Objects.equals(getMemoryLimit(), that.getMemoryLimit()) + && Objects.equals(getMemoryRequest(), that.getMemoryRequest()) + && Objects.equals(getCpuLimit(), that.getCpuLimit()) + && Objects.equals(getCpuRequest(), that.getCpuRequest()) && Objects.equals(getName(), that.getName()) && isMountSources() == that.isMountSources() && Objects.equals(getCommand(), that.getCommand()) @@ -272,6 +275,9 @@ public int hashCode() { getVolumes(), getPorts(), getMemoryLimit(), + getMemoryRequest(), + getCpuLimit(), + getCpuRequest(), getName(), isMountSources(), getCommand(), @@ -294,6 +300,12 @@ public String toString() { + ports + ", memoryLimit=" + memoryLimit + + ", memoryRequest=" + + memoryRequest + + ", cpuLimit=" + + cpuLimit + + ", cpuRequest=" + + cpuRequest + ", name=" + name + ", mountSources=" From a3d1e745ed72aeba1e6ee256b0e6e725c7ffae76 Mon Sep 17 00:00:00 2001 From: Max Shaposhnik Date: Thu, 13 Feb 2020 15:43:57 +0200 Subject: [PATCH 15/29] Review fixes --- .../kubernetes/util/KubernetesSize.java | 5 ++-- .../KubernetesPluginsToolingApplier.java | 9 +++++--- .../wsplugins/MachineResolverBuilder.java | 2 +- .../kubernetes/util/ContainersTest.java | 23 ++----------------- 4 files changed, 11 insertions(+), 28 deletions(-) diff --git a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/util/KubernetesSize.java b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/util/KubernetesSize.java index 33a45359a01..686b65698bf 100644 --- a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/util/KubernetesSize.java +++ b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/util/KubernetesSize.java @@ -147,9 +147,8 @@ public static float toCores(String cpuString) { if (suffix == null) { return size; } - switch (suffix.toLowerCase()) { - case "m": - return size / K; + if (suffix.toLowerCase().equals("m")) { + return size / K; } } throw new IllegalArgumentException("Invalid Kubernetes CPU size format provided: " + cpuString); 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 7d88d6b55c2..91c2a9609de 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 @@ -94,9 +94,8 @@ public KubernetesPluginsToolingApplier( ProjectsRootEnvVariableProvider projectsRootEnvVariableProvider, ChePluginsVolumeApplier chePluginsVolumeApplier, EnvVars envVars) { - this.defaultSidecarMemoryLimitBytes = String.valueOf(defaultSidecarMemoryLimitMB * 1024 * 1024); - this.defaultSidecarMemoryRequestBytes = - String.valueOf(defaultSidecarMemoryRequestMB * 1024 * 1024); + this.defaultSidecarMemoryLimitBytes = toBytesString(defaultSidecarMemoryLimitMB); + this.defaultSidecarMemoryRequestBytes = toBytesString(defaultSidecarMemoryRequestMB); this.defaultSidecarCpuLimitCores = Float.toString(KubernetesSize.toCores(defaultSidecarCpuLimitCores)); this.defaultSidecarCpuRequestCores = @@ -307,6 +306,10 @@ private CommandImpl asCommand(String machineName, Command command) { return cmd; } + private String toBytesString(long memoryLimitMB) { + return String.valueOf(memoryLimitMB * 1024L * 1024L); + } + private static class CommandsResolver { private final KubernetesEnvironment k8sEnvironment; diff --git a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/wsplugins/MachineResolverBuilder.java b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/wsplugins/MachineResolverBuilder.java index 3ede76ce8d3..db996172134 100644 --- a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/wsplugins/MachineResolverBuilder.java +++ b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/wsplugins/MachineResolverBuilder.java @@ -40,7 +40,7 @@ public MachineResolver build() { || defaultSidecarCpuRequestAttribute == null || containerEndpoints == null || projectsRootPathEnvVar == null) { - throw new IllegalStateException(); + throw new IllegalStateException("Unable to build MachineResolver cause some fields are null"); } return new MachineResolver( diff --git a/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/util/ContainersTest.java b/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/util/ContainersTest.java index fb0e64438f6..4995aa693bb 100644 --- a/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/util/ContainersTest.java +++ b/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/util/ContainersTest.java @@ -75,7 +75,7 @@ public void setup() { @Test public void testReturnContainerRamLimitAndRequest() { long limit = Containers.getRamLimit(container); - long request = (Containers.getRamRequest(container)); + long request = Containers.getRamRequest(container); assertEquals(limit, RAM_LIMIT); assertEquals(request, RAM_REQUEST); @@ -84,7 +84,7 @@ public void testReturnContainerRamLimitAndRequest() { @Test public void testReturnContainerCPULimitAndRequest() { float limit = Containers.getCpuLimit(container); - float request = (Containers.getCpuRequest(container)); + float request = Containers.getCpuRequest(container); assertEquals(limit, CPU_LIMIT); assertEquals(request, CPU_REQUEST); @@ -129,25 +129,6 @@ public void testReturnsZeroContainerRequestWhenActualValueIsNull() { assertEquals(Containers.getCpuRequest(container), 0, 0.0); } - // I cannot understand what we're testing here. Can you? - // @Test - // public void testOverridesContainerRamLimit() { - // Containers.addRamLimit(container, 3221225472L); - // - // assertTrue(limits.containsKey("cpu")); - // assertNotEquals(limits.get("memory"), "3221225472"); - // } - // - // @Test - // public void testAddContainerRamLimitWhenItNotPresent() { - // final Map limits = new HashMap<>(); - // when(resource.getLimits()).thenReturn(limits); - // - // Containers.addRamLimit(container, RAM_LIMIT); - // - // assertNotEquals(limits.get("memory"), String.valueOf(RAM_LIMIT)); - // } - @Test public void testAddContainerRamLimitWhenResourceIsNull() { when(container.getResources()).thenReturn(null); From 1961f5d34923cdd393b9a1584cbbd8c237ac41f0 Mon Sep 17 00:00:00 2001 From: Max Shaposhnik Date: Fri, 14 Feb 2020 12:31:06 +0200 Subject: [PATCH 16/29] Fix imits apply --- .../ResourceLimitAttributesProvisioner.java | 30 +++++++------------ ...esourceLimitAttributesProvisionerTest.java | 16 +++++----- 2 files changed, 19 insertions(+), 27 deletions(-) diff --git a/wsmaster/che-core-api-workspace/src/main/java/org/eclipse/che/api/workspace/server/spi/environment/ResourceLimitAttributesProvisioner.java b/wsmaster/che-core-api-workspace/src/main/java/org/eclipse/che/api/workspace/server/spi/environment/ResourceLimitAttributesProvisioner.java index 14a042cf115..37c99d6b7b3 100644 --- a/wsmaster/che-core-api-workspace/src/main/java/org/eclipse/che/api/workspace/server/spi/environment/ResourceLimitAttributesProvisioner.java +++ b/wsmaster/che-core-api-workspace/src/main/java/org/eclipse/che/api/workspace/server/spi/environment/ResourceLimitAttributesProvisioner.java @@ -38,9 +38,6 @@ public class ResourceLimitAttributesProvisioner { /** * Configures memory attributes, if they are missing in {@link MachineConfig} * - *

Note: Default memory request and limit will only be used if BOTH memoryLimit and - * memoryRequest are null or 0, otherwise the provided value will be used for both parameters. - * * @param machineConfig - given machine configuration * @param memoryLimit - resource limit parameter configured by user in specific infra recipe. Can * be 0 if defaults should be used @@ -61,15 +58,13 @@ public static void provisionMemory( "Requested default container resource limit is less than default request. Request parameter will be ignored."); } - // if both properties are not defined - if (memoryLimit <= 0 && memoryRequest <= 0) { + if (memoryLimit <= 0) { // if limit only is undefined memoryLimit = defaultMemoryLimit; + } + if (memoryRequest <= 0) { // if request only is undefined memoryRequest = defaultMemoryRequest; - } else if (memoryLimit <= 0) { // if limit only is undefined - memoryLimit = memoryRequest; - } else if (memoryRequest <= 0) { // if request only is undefined - memoryRequest = memoryLimit; - } else if (memoryRequest > memoryLimit) { // if both properties are defined, but not consistent + } + if (memoryRequest > memoryLimit) { // if both properties are defined, but not consistent memoryRequest = memoryLimit; } @@ -87,9 +82,6 @@ public static void provisionMemory( /** * Configures CPU attributes, if they are missing in {@link MachineConfig} * - *

Note: Default CPU request and memory will only be used if BOTH memoryLimit and memoryRequest - * are null or 0, otherwise the provided value will be used for both parameters. - * * @param machineConfig - given machine configuration * @param cpuLimit - CPU resource limit parameter configured by user in specific infra recipe. Can * be 0 if defaults should be used @@ -110,15 +102,13 @@ public static void provisionCPU( "Requested default container resource limit is less than default request. Request parameter will be ignored."); } - // if both properties are not defined - if (cpuLimit <= 0 && cpuRequest <= 0) { + if (cpuLimit <= 0) { // if limit only is undefined cpuLimit = defaultCPULimit; + } + if (cpuRequest <= 0) { // if request only is undefined cpuRequest = defaultCPURequest; - } else if (cpuLimit <= 0) { // if limit only is undefined - cpuLimit = cpuRequest; - } else if (cpuRequest <= 0) { // if request only is undefined - cpuRequest = cpuLimit; - } else if (cpuRequest > cpuLimit) { // if both properties are defined, but not consistent + } + if (cpuRequest > cpuLimit) { // if both properties are defined, but not consistent cpuRequest = cpuLimit; } diff --git a/wsmaster/che-core-api-workspace/src/test/java/org/eclipse/che/api/workspace/server/spi/environment/ResourceLimitAttributesProvisionerTest.java b/wsmaster/che-core-api-workspace/src/test/java/org/eclipse/che/api/workspace/server/spi/environment/ResourceLimitAttributesProvisionerTest.java index 978beb6e67f..cc4dfc6345a 100644 --- a/wsmaster/che-core-api-workspace/src/test/java/org/eclipse/che/api/workspace/server/spi/environment/ResourceLimitAttributesProvisionerTest.java +++ b/wsmaster/che-core-api-workspace/src/test/java/org/eclipse/che/api/workspace/server/spi/environment/ResourceLimitAttributesProvisionerTest.java @@ -152,8 +152,8 @@ public void testWhenRamLimitAttributeIsPresentInMachineConfigValuesInRecipeAreIg @Test public void testWhenRamAttributesAreNotPresentInMachineConfigAndOnlyRequestIsProvidedInRecipe() { - long defaultMemoryLimit = 1024L; - long defaultMemoryRequest = 2048L; + long defaultMemoryLimit = 2048L; + long defaultMemoryRequest = 1024L; InternalMachineConfig machineConfig = mockInternalMachineConfig(new HashMap<>()); long recipeRequest = 1526L; @@ -161,15 +161,16 @@ public void testWhenRamAttributesAreNotPresentInMachineConfigAndOnlyRequestIsPro machineConfig, 0, recipeRequest, defaultMemoryLimit, defaultMemoryRequest); assertEquals( - machineConfig.getAttributes().get(MEMORY_LIMIT_ATTRIBUTE), String.valueOf(recipeRequest)); + machineConfig.getAttributes().get(MEMORY_LIMIT_ATTRIBUTE), + String.valueOf(defaultMemoryLimit)); assertEquals( machineConfig.getAttributes().get(MEMORY_REQUEST_ATTRIBUTE), String.valueOf(recipeRequest)); } @Test public void testWhenRamAttributesAreNotPresentInMachineConfigAndOnlyLimitIsProvidedInRecipe() { - long defaultMemoryLimit = 1024L; - long defaultMemoryRequest = 2048L; + long defaultMemoryLimit = 2048L; + long defaultMemoryRequest = 1024L; InternalMachineConfig machineConfig = mockInternalMachineConfig(new HashMap<>()); long recipeLimit = 1526L; @@ -179,7 +180,8 @@ public void testWhenRamAttributesAreNotPresentInMachineConfigAndOnlyLimitIsProvi assertEquals( machineConfig.getAttributes().get(MEMORY_LIMIT_ATTRIBUTE), String.valueOf(recipeLimit)); assertEquals( - machineConfig.getAttributes().get(MEMORY_REQUEST_ATTRIBUTE), String.valueOf(recipeLimit)); + machineConfig.getAttributes().get(MEMORY_REQUEST_ATTRIBUTE), + String.valueOf(defaultMemoryRequest)); } @Test @@ -312,7 +314,7 @@ public void testWhenCPUAttributesAreNotPresentInMachineConfigAndOnlyRequestIsPro machineConfig, 0, recipeRequest, defaultCPULimit, defaultCPURequest); assertEquals( - machineConfig.getAttributes().get(CPU_LIMIT_ATTRIBUTE), String.valueOf(recipeRequest)); + machineConfig.getAttributes().get(CPU_LIMIT_ATTRIBUTE), String.valueOf(defaultCPULimit)); assertEquals( machineConfig.getAttributes().get(CPU_REQUEST_ATTRIBUTE), String.valueOf(recipeRequest)); } From de8357c119c523d4977eb9d85a84d23249ee0823 Mon Sep 17 00:00:00 2001 From: Max Shaposhnik Date: Mon, 17 Feb 2020 11:44:55 +0200 Subject: [PATCH 17/29] Fixup javadoc Co-Authored-By: Sergii Leshchenko --- .../src/main/webapp/WEB-INF/classes/che/che.properties | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/assembly/assembly-wsmaster-war/src/main/webapp/WEB-INF/classes/che/che.properties b/assembly/assembly-wsmaster-war/src/main/webapp/WEB-INF/classes/che/che.properties index bdd1a81fa3e..766496e7c67 100644 --- a/assembly/assembly-wsmaster-war/src/main/webapp/WEB-INF/classes/che/che.properties +++ b/assembly/assembly-wsmaster-war/src/main/webapp/WEB-INF/classes/che/che.properties @@ -87,7 +87,7 @@ che.workspace.maven_server_java_options=-XX:MaxRAM=128m -XX:MaxRAMFraction=1 -XX # RAM limit default for each machine that has no RAM settings in environment. che.workspace.default_memory_limit_mb=1024 -# RAM request default for each machine that has no explicit RAM settings in environment. +# RAM request default for each container that has no explicit RAM settings in environment. # this amount will be allocated on workspace container creation # this property might not be supported by all infrastructure implementations: # currently it is supported by k8s and openshift From f7e1b482f135d46c576ea99c73be150c8e757419 Mon Sep 17 00:00:00 2001 From: Max Shaposhnik Date: Mon, 17 Feb 2020 11:45:35 +0200 Subject: [PATCH 18/29] Fixup javadoc Co-Authored-By: Sergii Leshchenko --- .../src/main/webapp/WEB-INF/classes/che/che.properties | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/assembly/assembly-wsmaster-war/src/main/webapp/WEB-INF/classes/che/che.properties b/assembly/assembly-wsmaster-war/src/main/webapp/WEB-INF/classes/che/che.properties index 766496e7c67..0a0283966b8 100644 --- a/assembly/assembly-wsmaster-war/src/main/webapp/WEB-INF/classes/che/che.properties +++ b/assembly/assembly-wsmaster-war/src/main/webapp/WEB-INF/classes/che/che.properties @@ -91,7 +91,7 @@ che.workspace.default_memory_limit_mb=1024 # this amount will be allocated on workspace container creation # this property might not be supported by all infrastructure implementations: # currently it is supported by k8s and openshift -# if default memory request is more than the memory limit, request will be ignored, +# if default memory request is more than the memory limit, request will be ignored, # and only limit will be used che.workspace.default_memory_request_mb=200 From 50058124cf3a8d15d6efff11edc02612bad3847c Mon Sep 17 00:00:00 2001 From: Max Shaposhnik Date: Mon, 17 Feb 2020 11:45:49 +0200 Subject: [PATCH 19/29] Fixup javadoc Co-Authored-By: Sergii Leshchenko --- .../src/main/webapp/WEB-INF/classes/che/che.properties | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/assembly/assembly-wsmaster-war/src/main/webapp/WEB-INF/classes/che/che.properties b/assembly/assembly-wsmaster-war/src/main/webapp/WEB-INF/classes/che/che.properties index 0a0283966b8..640df603030 100644 --- a/assembly/assembly-wsmaster-war/src/main/webapp/WEB-INF/classes/che/che.properties +++ b/assembly/assembly-wsmaster-war/src/main/webapp/WEB-INF/classes/che/che.properties @@ -96,7 +96,7 @@ che.workspace.default_memory_limit_mb=1024 che.workspace.default_memory_request_mb=200 -# CPU limit default for each machine that has no CPU settings in environment. +# CPU limit default for each container that has no CPU settings in environment. # Can be specified either in floating point cores number, e.g. 0.125 or in K8S format integer millicores e.g. 125m che.workspace.default_cpu_limit_cores=2 From 1a27454a44d0997cc33f2405aa9c6b023e6161f7 Mon Sep 17 00:00:00 2001 From: Max Shaposhnik Date: Mon, 17 Feb 2020 11:46:11 +0200 Subject: [PATCH 20/29] Fixup javadoc Co-Authored-By: Sergii Leshchenko --- .../src/main/webapp/WEB-INF/classes/che/che.properties | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/assembly/assembly-wsmaster-war/src/main/webapp/WEB-INF/classes/che/che.properties b/assembly/assembly-wsmaster-war/src/main/webapp/WEB-INF/classes/che/che.properties index 640df603030..0756a1a5fe5 100644 --- a/assembly/assembly-wsmaster-war/src/main/webapp/WEB-INF/classes/che/che.properties +++ b/assembly/assembly-wsmaster-war/src/main/webapp/WEB-INF/classes/che/che.properties @@ -100,7 +100,7 @@ che.workspace.default_memory_request_mb=200 # Can be specified either in floating point cores number, e.g. 0.125 or in K8S format integer millicores e.g. 125m che.workspace.default_cpu_limit_cores=2 -# CPU request default for each machine that has no CPU settings in environment. +# CPU request default for each container that has no CPU settings in environment. # if default CPU request is more than the CPU limit, request will be ignored, # and only limit will be used che.workspace.default_cpu_request_cores=0.125 From c9db8885f8f681e47ec3f2dcf5db919916173ab0 Mon Sep 17 00:00:00 2001 From: Max Shaposhnik Date: Mon, 17 Feb 2020 11:46:24 +0200 Subject: [PATCH 21/29] Fixup javadoc Co-Authored-By: Sergii Leshchenko --- .../src/main/webapp/WEB-INF/classes/che/che.properties | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/assembly/assembly-wsmaster-war/src/main/webapp/WEB-INF/classes/che/che.properties b/assembly/assembly-wsmaster-war/src/main/webapp/WEB-INF/classes/che/che.properties index 0756a1a5fe5..fad1f056fa3 100644 --- a/assembly/assembly-wsmaster-war/src/main/webapp/WEB-INF/classes/che/che.properties +++ b/assembly/assembly-wsmaster-war/src/main/webapp/WEB-INF/classes/che/che.properties @@ -101,7 +101,7 @@ che.workspace.default_memory_request_mb=200 che.workspace.default_cpu_limit_cores=2 # CPU request default for each container that has no CPU settings in environment. -# if default CPU request is more than the CPU limit, request will be ignored, +# if default CPU request is more than the CPU limit, request will be ignored, # and only limit will be used che.workspace.default_cpu_request_cores=0.125 From b7dc1ddbdfca21fdd4034519d6495f7f88d5673e Mon Sep 17 00:00:00 2001 From: Max Shaposhnik Date: Mon, 17 Feb 2020 11:46:38 +0200 Subject: [PATCH 22/29] Fixup javadoc Co-Authored-By: Sergii Leshchenko --- .../limits/ram/ContainerResourceLimitRequestProvisioner.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/provision/limits/ram/ContainerResourceLimitRequestProvisioner.java b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/provision/limits/ram/ContainerResourceLimitRequestProvisioner.java index 867022b1e03..0ccbc1c955c 100644 --- a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/provision/limits/ram/ContainerResourceLimitRequestProvisioner.java +++ b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/provision/limits/ram/ContainerResourceLimitRequestProvisioner.java @@ -49,7 +49,7 @@ *

Similarly, there are two CPU-related properties: * *

    - *
  • -che.workspace.default_cpu_limit_cores - defines default machine CPU limit + *
  • che.workspace.default_cpu_limit_cores - defines default machine CPU limit *
  • -che.workspace.default_cpu_request_cores - defines default machine CPU request *
* From a301187338bf6d21da4a71aa96879ccd7dcdf7d4 Mon Sep 17 00:00:00 2001 From: Max Shaposhnik Date: Mon, 17 Feb 2020 11:47:09 +0200 Subject: [PATCH 23/29] Fixup javadoc Co-Authored-By: Sergii Leshchenko --- .../limits/ram/ContainerResourceLimitRequestProvisioner.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/provision/limits/ram/ContainerResourceLimitRequestProvisioner.java b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/provision/limits/ram/ContainerResourceLimitRequestProvisioner.java index 0ccbc1c955c..c0b7e587d21 100644 --- a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/provision/limits/ram/ContainerResourceLimitRequestProvisioner.java +++ b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/provision/limits/ram/ContainerResourceLimitRequestProvisioner.java @@ -41,7 +41,7 @@ *

There are two memory-related properties: * *

    - *
  • -che.workspace.default_memory_limit_mb - defines default machine memory limit + *
  • che.workspace.default_memory_limit_mb - defines default machine memory limit *
  • -che.workspace.default_memory_request_mb - defines default requested machine memory * allocation. *
From 9ae0d4ef7ce171abe8ca772a6c80b59f65b175d5 Mon Sep 17 00:00:00 2001 From: Max Shaposhnik Date: Mon, 17 Feb 2020 11:47:21 +0200 Subject: [PATCH 24/29] Fixup javadoc Co-Authored-By: Sergii Leshchenko --- .../limits/ram/ContainerResourceLimitRequestProvisioner.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/provision/limits/ram/ContainerResourceLimitRequestProvisioner.java b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/provision/limits/ram/ContainerResourceLimitRequestProvisioner.java index c0b7e587d21..034493aa1c1 100644 --- a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/provision/limits/ram/ContainerResourceLimitRequestProvisioner.java +++ b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/provision/limits/ram/ContainerResourceLimitRequestProvisioner.java @@ -42,7 +42,7 @@ * *
    *
  • che.workspace.default_memory_limit_mb - defines default machine memory limit - *
  • -che.workspace.default_memory_request_mb - defines default requested machine memory + *
  • che.workspace.default_memory_request_mb - defines default requested machine memory * allocation. *
* From 68c0ba8991ea07d979dfac7932c09bec0552bcd5 Mon Sep 17 00:00:00 2001 From: Max Shaposhnik Date: Mon, 17 Feb 2020 11:47:34 +0200 Subject: [PATCH 25/29] Fixup javadoc Co-Authored-By: Sergii Leshchenko --- .../limits/ram/ContainerResourceLimitRequestProvisioner.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/provision/limits/ram/ContainerResourceLimitRequestProvisioner.java b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/provision/limits/ram/ContainerResourceLimitRequestProvisioner.java index 034493aa1c1..d26a6bf5815 100644 --- a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/provision/limits/ram/ContainerResourceLimitRequestProvisioner.java +++ b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/provision/limits/ram/ContainerResourceLimitRequestProvisioner.java @@ -50,7 +50,7 @@ * *
    *
  • che.workspace.default_cpu_limit_cores - defines default machine CPU limit - *
  • -che.workspace.default_cpu_request_cores - defines default machine CPU request + *
  • che.workspace.default_cpu_request_cores - defines default machine CPU request *
* * @author Anton Korneta From 7fed9b60e0aa1232e0e3651a337156389d59c992 Mon Sep 17 00:00:00 2001 From: Max Shaposhnik Date: Mon, 17 Feb 2020 11:49:05 +0200 Subject: [PATCH 26/29] review fixups --- .../KubernetesEnvironmentProvisioner.java | 6 +- ...java => ContainerResourceProvisioner.java} | 4 +- .../kubernetes/wsplugins/MachineResolver.java | 27 ---- .../KubernetesEnvironmentProvisionerTest.java | 4 +- ...erResourceLimitRequestProvisionerTest.java | 118 ------------------ .../OpenShiftEnvironmentProvisioner.java | 6 +- .../OpenShiftEnvironmentProvisionerTest.java | 4 +- 7 files changed, 12 insertions(+), 157 deletions(-) rename infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/provision/limits/ram/{ContainerResourceLimitRequestProvisioner.java => ContainerResourceProvisioner.java} (97%) delete mode 100644 infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/provision/limits/ram/ContainerResourceLimitRequestProvisionerTest.java diff --git a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/KubernetesEnvironmentProvisioner.java b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/KubernetesEnvironmentProvisioner.java index e6ec7006023..75a5f28910f 100644 --- a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/KubernetesEnvironmentProvisioner.java +++ b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/KubernetesEnvironmentProvisioner.java @@ -33,7 +33,7 @@ import org.eclipse.che.workspace.infrastructure.kubernetes.provision.VcsSshKeysProvisioner; import org.eclipse.che.workspace.infrastructure.kubernetes.provision.VcsSslCertificateProvisioner; import org.eclipse.che.workspace.infrastructure.kubernetes.provision.env.EnvVarsConverter; -import org.eclipse.che.workspace.infrastructure.kubernetes.provision.limits.ram.ContainerResourceLimitRequestProvisioner; +import org.eclipse.che.workspace.infrastructure.kubernetes.provision.limits.ram.ContainerResourceProvisioner; import org.eclipse.che.workspace.infrastructure.kubernetes.provision.restartpolicy.RestartPolicyRewriter; import org.eclipse.che.workspace.infrastructure.kubernetes.provision.server.ServersConverter; import org.eclipse.che.workspace.infrastructure.kubernetes.server.PreviewUrlExposer; @@ -64,7 +64,7 @@ class KubernetesEnvironmentProvisionerImpl private final ServersConverter serversConverter; private final EnvVarsConverter envVarsConverter; private final RestartPolicyRewriter restartPolicyRewriter; - private final ContainerResourceLimitRequestProvisioner resourceLimitRequestProvisioner; + private final ContainerResourceProvisioner resourceLimitRequestProvisioner; private final LogsVolumeMachineProvisioner logsVolumeMachineProvisioner; private final SecurityContextProvisioner securityContextProvisioner; private final PodTerminationGracePeriodProvisioner podTerminationGracePeriodProvisioner; @@ -86,7 +86,7 @@ public KubernetesEnvironmentProvisionerImpl( EnvVarsConverter envVarsConverter, RestartPolicyRewriter restartPolicyRewriter, WorkspaceVolumesStrategy volumesStrategy, - ContainerResourceLimitRequestProvisioner resourceLimitRequestProvisioner, + ContainerResourceProvisioner resourceLimitRequestProvisioner, LogsVolumeMachineProvisioner logsVolumeMachineProvisioner, SecurityContextProvisioner securityContextProvisioner, PodTerminationGracePeriodProvisioner podTerminationGracePeriodProvisioner, diff --git a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/provision/limits/ram/ContainerResourceLimitRequestProvisioner.java b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/provision/limits/ram/ContainerResourceProvisioner.java similarity index 97% rename from infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/provision/limits/ram/ContainerResourceLimitRequestProvisioner.java rename to infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/provision/limits/ram/ContainerResourceProvisioner.java index 867022b1e03..cb2d06da1bd 100644 --- a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/provision/limits/ram/ContainerResourceLimitRequestProvisioner.java +++ b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/provision/limits/ram/ContainerResourceProvisioner.java @@ -57,7 +57,7 @@ * @auhtor Max Shaposhnyk */ @Singleton -public class ContainerResourceLimitRequestProvisioner implements ConfigurationProvisioner { +public class ContainerResourceProvisioner implements ConfigurationProvisioner { private final ResourceLimitAttributesProvisioner resourceLimitAttributesProvisioner; @@ -67,7 +67,7 @@ public class ContainerResourceLimitRequestProvisioner implements ConfigurationPr private final float defaultMachineCpuRequestAttribute; @Inject - public ContainerResourceLimitRequestProvisioner( + public ContainerResourceProvisioner( @Named("che.workspace.default_memory_limit_mb") long defaultMachineMaxMemorySizeAttribute, @Named("che.workspace.default_memory_request_mb") long defaultMachineRequestMemorySizeAttribute, 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 77e379c3b54..4382eca9135 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 @@ -116,15 +116,6 @@ private void normalizeMemory(Container container, InternalMachineConfig machineC if (ramRequest == 0) { machineConfig.getAttributes().put(MEMORY_REQUEST_ATTRIBUTE, defaultSidecarMemoryRequestBytes); } - // TODO: uncomment when CPU limit added into devfile - // String overriddenSidecarMemRequest = component.getRamRequest(); - // if (!isNullOrEmpty(overriddenSidecarMemRequest)) { - // machineConfig - // .getAttributes() - // .put( - // MEMORY_REQUEST_ATTRIBUTE, - // Long.toString(KubernetesSize.toBytes(overriddenSidecarMemRequest))); - // } } private void normalizeCpu(Container container, InternalMachineConfig machineConfig) { @@ -132,29 +123,11 @@ private void normalizeCpu(Container container, InternalMachineConfig machineConf if (cpuLimit == 0) { machineConfig.getAttributes().put(CPU_LIMIT_ATTRIBUTE, defaultSidecarCpuLimitCores); } - // TODO: uncomment when CPU limit added into devfile - // String overriddenSidecarCpuLimit = component.getCpuLimit(); - // if (!isNullOrEmpty(overriddenSidecarCpuLimit)) { - // machineConfig - // .getAttributes() - // .put( - // CPU_LIMIT_ATTRIBUTE, - // Long.toString(KubernetesSize.toCores(overriddenSidecarCpuLimit))); - // } float cpuRequest = Containers.getCpuRequest(container); if (cpuRequest == 0) { machineConfig.getAttributes().put(CPU_REQUEST_ATTRIBUTE, defaultSidecarCpuRequestCores); } - // TODO: uncomment when CPU limit added into devfile - // String overriddenSidecarCpuRequest = component.getCpuRequest(); - // if (!isNullOrEmpty(overriddenSidecarCpuRequest)) { - // machineConfig - // .getAttributes() - // .put( - // CPU_REQUEST_ATTRIBUTE, - // Long.toString(KubernetesSize.toCores(overriddenSidecarCpuRequest))); - // } } private Map diff --git a/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/KubernetesEnvironmentProvisionerTest.java b/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/KubernetesEnvironmentProvisionerTest.java index febbc74e6b1..7a118a64965 100644 --- a/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/KubernetesEnvironmentProvisionerTest.java +++ b/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/KubernetesEnvironmentProvisionerTest.java @@ -31,7 +31,7 @@ import org.eclipse.che.workspace.infrastructure.kubernetes.provision.VcsSshKeysProvisioner; import org.eclipse.che.workspace.infrastructure.kubernetes.provision.VcsSslCertificateProvisioner; import org.eclipse.che.workspace.infrastructure.kubernetes.provision.env.EnvVarsConverter; -import org.eclipse.che.workspace.infrastructure.kubernetes.provision.limits.ram.ContainerResourceLimitRequestProvisioner; +import org.eclipse.che.workspace.infrastructure.kubernetes.provision.limits.ram.ContainerResourceProvisioner; import org.eclipse.che.workspace.infrastructure.kubernetes.provision.restartpolicy.RestartPolicyRewriter; import org.eclipse.che.workspace.infrastructure.kubernetes.provision.server.ServersConverter; import org.eclipse.che.workspace.infrastructure.kubernetes.server.PreviewUrlExposer; @@ -57,7 +57,7 @@ public class KubernetesEnvironmentProvisionerTest { @Mock private EnvVarsConverter envVarsProvisioner; @Mock private ServersConverter serversProvisioner; @Mock private RestartPolicyRewriter restartPolicyRewriter; - @Mock private ContainerResourceLimitRequestProvisioner ramLimitProvisioner; + @Mock private ContainerResourceProvisioner ramLimitProvisioner; @Mock private LogsVolumeMachineProvisioner logsVolumeMachineProvisioner; @Mock private SecurityContextProvisioner securityContextProvisioner; @Mock private PodTerminationGracePeriodProvisioner podTerminationGracePeriodProvisioner; diff --git a/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/provision/limits/ram/ContainerResourceLimitRequestProvisionerTest.java b/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/provision/limits/ram/ContainerResourceLimitRequestProvisionerTest.java deleted file mode 100644 index b35272e524b..00000000000 --- a/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/provision/limits/ram/ContainerResourceLimitRequestProvisionerTest.java +++ /dev/null @@ -1,118 +0,0 @@ -/* - * Copyright (c) 2012-2018 Red Hat, Inc. - * This program and the accompanying materials are made - * available under the terms of the Eclipse Public License 2.0 - * which is available at https://www.eclipse.org/legal/epl-2.0/ - * - * SPDX-License-Identifier: EPL-2.0 - * - * Contributors: - * Red Hat, Inc. - initial API and implementation - */ -package org.eclipse.che.workspace.infrastructure.kubernetes.provision.limits.ram; - -import static com.google.common.collect.ImmutableMap.of; -import static org.eclipse.che.api.core.model.workspace.config.MachineConfig.CPU_LIMIT_ATTRIBUTE; -import static org.eclipse.che.api.core.model.workspace.config.MachineConfig.CPU_REQUEST_ATTRIBUTE; -import static org.eclipse.che.api.core.model.workspace.config.MachineConfig.MEMORY_LIMIT_ATTRIBUTE; -import static org.eclipse.che.api.core.model.workspace.config.MachineConfig.MEMORY_REQUEST_ATTRIBUTE; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.when; -import static org.testng.Assert.assertEquals; - -import io.fabric8.kubernetes.api.model.Container; -import io.fabric8.kubernetes.api.model.ObjectMeta; -import io.fabric8.kubernetes.api.model.PodSpec; -import io.fabric8.kubernetes.api.model.Quantity; -import io.fabric8.kubernetes.api.model.ResourceRequirements; -import io.fabric8.kubernetes.api.model.ResourceRequirementsBuilder; -import java.util.Collections; -import org.eclipse.che.api.core.model.workspace.runtime.RuntimeIdentity; -import org.eclipse.che.api.workspace.server.spi.environment.InternalMachineConfig; -import org.eclipse.che.api.workspace.server.spi.environment.ResourceLimitAttributesProvisioner; -import org.eclipse.che.workspace.infrastructure.kubernetes.environment.KubernetesEnvironment; -import org.eclipse.che.workspace.infrastructure.kubernetes.environment.KubernetesEnvironment.PodData; -import org.mockito.Mock; -import org.mockito.testng.MockitoTestNGListener; -import org.testng.annotations.BeforeMethod; -import org.testng.annotations.Listeners; -import org.testng.annotations.Test; - -/** - * Tests {@link ContainerResourceLimitRequestProvisioner}. - * - * @author Anton Korneta - */ -@Listeners(MockitoTestNGListener.class) -public class ContainerResourceLimitRequestProvisionerTest { - - private static final String POD_NAME = "web"; - private static final String CONTAINER_NAME = "app"; - private static final String MACHINE_NAME = POD_NAME + '/' + CONTAINER_NAME; - private static final String RAM_LIMIT_VALUE = "2147483648"; - private static final String RAM_REQUEST_VALUE = "1234567890"; - private static final String CPU_LIMIT_VALUE = "0.4"; - private static final String CPU_REQUEST_VALUE = "0.15"; - - @Mock private KubernetesEnvironment k8sEnv; - @Mock private RuntimeIdentity identity; - @Mock private InternalMachineConfig internalMachineConfig; - @Mock private ResourceLimitAttributesProvisioner resourceLimitAttributesProvisioner; - - private Container container; - private ContainerResourceLimitRequestProvisioner resourceProvisioner; - - @BeforeMethod - public void setup() { - resourceProvisioner = - new ContainerResourceLimitRequestProvisioner( - 1024, 512, "500m", "100m", resourceLimitAttributesProvisioner); - container = new Container(); - container.setName(CONTAINER_NAME); - when(k8sEnv.getMachines()).thenReturn(of(MACHINE_NAME, internalMachineConfig)); - when(internalMachineConfig.getAttributes()) - .thenReturn( - of( - MEMORY_LIMIT_ATTRIBUTE, - RAM_LIMIT_VALUE, - MEMORY_REQUEST_ATTRIBUTE, - RAM_REQUEST_VALUE, - CPU_LIMIT_ATTRIBUTE, - CPU_LIMIT_VALUE, - CPU_REQUEST_ATTRIBUTE, - CPU_REQUEST_VALUE)); - final ObjectMeta podMetadata = mock(ObjectMeta.class); - when(podMetadata.getName()).thenReturn(POD_NAME); - final PodSpec podSpec = mock(PodSpec.class); - when(podSpec.getContainers()).thenReturn(Collections.singletonList(container)); - when(k8sEnv.getPodsData()).thenReturn(of(POD_NAME, new PodData(podSpec, podMetadata))); - } - - @Test - public void testProvisionResourcesLimitAndRequestAttributeToContainer() throws Exception { - resourceProvisioner.provision(k8sEnv, identity); - assertEquals(container.getResources().getLimits().get("memory").getAmount(), RAM_LIMIT_VALUE); - assertEquals(container.getResources().getLimits().get("cpu").getAmount(), CPU_LIMIT_VALUE); - assertEquals( - container.getResources().getRequests().get("memory").getAmount(), RAM_REQUEST_VALUE); - assertEquals(container.getResources().getRequests().get("cpu").getAmount(), CPU_REQUEST_VALUE); - } - - @Test - public void testOverridesContainerRamLimitAndRequestFromMachineAttribute() throws Exception { - ResourceRequirements resourceRequirements = - new ResourceRequirementsBuilder() - .addToLimits(of("memory", new Quantity("3221225472"), "cpu", new Quantity("0.678"))) - .addToRequests(of("memory", new Quantity("1231231423"), "cpu", new Quantity("0.333"))) - .build(); - container.setResources(resourceRequirements); - - resourceProvisioner.provision(k8sEnv, identity); - - assertEquals(container.getResources().getLimits().get("memory").getAmount(), RAM_LIMIT_VALUE); - assertEquals(container.getResources().getLimits().get("cpu").getAmount(), CPU_LIMIT_VALUE); - assertEquals( - container.getResources().getRequests().get("memory").getAmount(), RAM_REQUEST_VALUE); - assertEquals(container.getResources().getRequests().get("cpu").getAmount(), CPU_REQUEST_VALUE); - } -} diff --git a/infrastructures/openshift/src/main/java/org/eclipse/che/workspace/infrastructure/openshift/OpenShiftEnvironmentProvisioner.java b/infrastructures/openshift/src/main/java/org/eclipse/che/workspace/infrastructure/openshift/OpenShiftEnvironmentProvisioner.java index 2c2d318ec0c..9ea5504dce2 100644 --- a/infrastructures/openshift/src/main/java/org/eclipse/che/workspace/infrastructure/openshift/OpenShiftEnvironmentProvisioner.java +++ b/infrastructures/openshift/src/main/java/org/eclipse/che/workspace/infrastructure/openshift/OpenShiftEnvironmentProvisioner.java @@ -31,7 +31,7 @@ import org.eclipse.che.workspace.infrastructure.kubernetes.provision.VcsSshKeysProvisioner; import org.eclipse.che.workspace.infrastructure.kubernetes.provision.VcsSslCertificateProvisioner; import org.eclipse.che.workspace.infrastructure.kubernetes.provision.env.EnvVarsConverter; -import org.eclipse.che.workspace.infrastructure.kubernetes.provision.limits.ram.ContainerResourceLimitRequestProvisioner; +import org.eclipse.che.workspace.infrastructure.kubernetes.provision.limits.ram.ContainerResourceProvisioner; import org.eclipse.che.workspace.infrastructure.kubernetes.provision.restartpolicy.RestartPolicyRewriter; import org.eclipse.che.workspace.infrastructure.kubernetes.provision.server.ServersConverter; import org.eclipse.che.workspace.infrastructure.kubernetes.server.PreviewUrlExposer; @@ -62,7 +62,7 @@ public class OpenShiftEnvironmentProvisioner private final ServersConverter serversConverter; private final EnvVarsConverter envVarsConverter; private final RestartPolicyRewriter restartPolicyRewriter; - private final ContainerResourceLimitRequestProvisioner resourceLimitRequestProvisioner; + private final ContainerResourceProvisioner resourceLimitRequestProvisioner; private final LogsVolumeMachineProvisioner logsVolumeMachineProvisioner; private final PodTerminationGracePeriodProvisioner podTerminationGracePeriodProvisioner; private final ImagePullSecretProvisioner imagePullSecretProvisioner; @@ -83,7 +83,7 @@ public OpenShiftEnvironmentProvisioner( EnvVarsConverter envVarsConverter, RestartPolicyRewriter restartPolicyRewriter, WorkspaceVolumesStrategy volumesStrategy, - ContainerResourceLimitRequestProvisioner resourceLimitRequestProvisioner, + ContainerResourceProvisioner resourceLimitRequestProvisioner, LogsVolumeMachineProvisioner logsVolumeMachineProvisioner, PodTerminationGracePeriodProvisioner podTerminationGracePeriodProvisioner, ImagePullSecretProvisioner imagePullSecretProvisioner, diff --git a/infrastructures/openshift/src/test/java/org/eclipse/che/workspace/infrastructure/openshift/OpenShiftEnvironmentProvisionerTest.java b/infrastructures/openshift/src/test/java/org/eclipse/che/workspace/infrastructure/openshift/OpenShiftEnvironmentProvisionerTest.java index 648fe9b4744..7571135306a 100644 --- a/infrastructures/openshift/src/test/java/org/eclipse/che/workspace/infrastructure/openshift/OpenShiftEnvironmentProvisionerTest.java +++ b/infrastructures/openshift/src/test/java/org/eclipse/che/workspace/infrastructure/openshift/OpenShiftEnvironmentProvisionerTest.java @@ -26,7 +26,7 @@ import org.eclipse.che.workspace.infrastructure.kubernetes.provision.VcsSshKeysProvisioner; import org.eclipse.che.workspace.infrastructure.kubernetes.provision.VcsSslCertificateProvisioner; import org.eclipse.che.workspace.infrastructure.kubernetes.provision.env.EnvVarsConverter; -import org.eclipse.che.workspace.infrastructure.kubernetes.provision.limits.ram.ContainerResourceLimitRequestProvisioner; +import org.eclipse.che.workspace.infrastructure.kubernetes.provision.limits.ram.ContainerResourceProvisioner; import org.eclipse.che.workspace.infrastructure.kubernetes.provision.restartpolicy.RestartPolicyRewriter; import org.eclipse.che.workspace.infrastructure.kubernetes.provision.server.ServersConverter; import org.eclipse.che.workspace.infrastructure.openshift.environment.OpenShiftEnvironment; @@ -56,7 +56,7 @@ public class OpenShiftEnvironmentProvisionerTest { @Mock private EnvVarsConverter envVarsProvisioner; @Mock private ServersConverter serversProvisioner; @Mock private RestartPolicyRewriter restartPolicyRewriter; - @Mock private ContainerResourceLimitRequestProvisioner ramLimitProvisioner; + @Mock private ContainerResourceProvisioner ramLimitProvisioner; @Mock private LogsVolumeMachineProvisioner logsVolumeMachineProvisioner; @Mock private PodTerminationGracePeriodProvisioner podTerminationGracePeriodProvisioner; @Mock private ImagePullSecretProvisioner imagePullSecretProvisioner; From 1e1639c2dd366bc77cd61306a2b90c4bc324baaa Mon Sep 17 00:00:00 2001 From: Max Shaposhnik Date: Mon, 17 Feb 2020 11:49:22 +0200 Subject: [PATCH 27/29] review fixups --- .../ram/ContainerResourceProvisionerTest.java | 118 ++++++++++++++++++ 1 file changed, 118 insertions(+) create mode 100644 infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/provision/limits/ram/ContainerResourceProvisionerTest.java diff --git a/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/provision/limits/ram/ContainerResourceProvisionerTest.java b/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/provision/limits/ram/ContainerResourceProvisionerTest.java new file mode 100644 index 00000000000..cc76fccb3ad --- /dev/null +++ b/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/provision/limits/ram/ContainerResourceProvisionerTest.java @@ -0,0 +1,118 @@ +/* + * Copyright (c) 2012-2018 Red Hat, Inc. + * This program and the accompanying materials are made + * available under the terms of the Eclipse Public License 2.0 + * which is available at https://www.eclipse.org/legal/epl-2.0/ + * + * SPDX-License-Identifier: EPL-2.0 + * + * Contributors: + * Red Hat, Inc. - initial API and implementation + */ +package org.eclipse.che.workspace.infrastructure.kubernetes.provision.limits.ram; + +import static com.google.common.collect.ImmutableMap.of; +import static org.eclipse.che.api.core.model.workspace.config.MachineConfig.CPU_LIMIT_ATTRIBUTE; +import static org.eclipse.che.api.core.model.workspace.config.MachineConfig.CPU_REQUEST_ATTRIBUTE; +import static org.eclipse.che.api.core.model.workspace.config.MachineConfig.MEMORY_LIMIT_ATTRIBUTE; +import static org.eclipse.che.api.core.model.workspace.config.MachineConfig.MEMORY_REQUEST_ATTRIBUTE; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; +import static org.testng.Assert.assertEquals; + +import io.fabric8.kubernetes.api.model.Container; +import io.fabric8.kubernetes.api.model.ObjectMeta; +import io.fabric8.kubernetes.api.model.PodSpec; +import io.fabric8.kubernetes.api.model.Quantity; +import io.fabric8.kubernetes.api.model.ResourceRequirements; +import io.fabric8.kubernetes.api.model.ResourceRequirementsBuilder; +import java.util.Collections; +import org.eclipse.che.api.core.model.workspace.runtime.RuntimeIdentity; +import org.eclipse.che.api.workspace.server.spi.environment.InternalMachineConfig; +import org.eclipse.che.api.workspace.server.spi.environment.ResourceLimitAttributesProvisioner; +import org.eclipse.che.workspace.infrastructure.kubernetes.environment.KubernetesEnvironment; +import org.eclipse.che.workspace.infrastructure.kubernetes.environment.KubernetesEnvironment.PodData; +import org.mockito.Mock; +import org.mockito.testng.MockitoTestNGListener; +import org.testng.annotations.BeforeMethod; +import org.testng.annotations.Listeners; +import org.testng.annotations.Test; + +/** + * Tests {@link ContainerResourceProvisioner}. + * + * @author Anton Korneta + */ +@Listeners(MockitoTestNGListener.class) +public class ContainerResourceProvisionerTest { + + private static final String POD_NAME = "web"; + private static final String CONTAINER_NAME = "app"; + private static final String MACHINE_NAME = POD_NAME + '/' + CONTAINER_NAME; + private static final String RAM_LIMIT_VALUE = "2147483648"; + private static final String RAM_REQUEST_VALUE = "1234567890"; + private static final String CPU_LIMIT_VALUE = "0.4"; + private static final String CPU_REQUEST_VALUE = "0.15"; + + @Mock private KubernetesEnvironment k8sEnv; + @Mock private RuntimeIdentity identity; + @Mock private InternalMachineConfig internalMachineConfig; + @Mock private ResourceLimitAttributesProvisioner resourceLimitAttributesProvisioner; + + private Container container; + private ContainerResourceProvisioner resourceProvisioner; + + @BeforeMethod + public void setup() { + resourceProvisioner = + new ContainerResourceProvisioner( + 1024, 512, "500m", "100m", resourceLimitAttributesProvisioner); + container = new Container(); + container.setName(CONTAINER_NAME); + when(k8sEnv.getMachines()).thenReturn(of(MACHINE_NAME, internalMachineConfig)); + when(internalMachineConfig.getAttributes()) + .thenReturn( + of( + MEMORY_LIMIT_ATTRIBUTE, + RAM_LIMIT_VALUE, + MEMORY_REQUEST_ATTRIBUTE, + RAM_REQUEST_VALUE, + CPU_LIMIT_ATTRIBUTE, + CPU_LIMIT_VALUE, + CPU_REQUEST_ATTRIBUTE, + CPU_REQUEST_VALUE)); + final ObjectMeta podMetadata = mock(ObjectMeta.class); + when(podMetadata.getName()).thenReturn(POD_NAME); + final PodSpec podSpec = mock(PodSpec.class); + when(podSpec.getContainers()).thenReturn(Collections.singletonList(container)); + when(k8sEnv.getPodsData()).thenReturn(of(POD_NAME, new PodData(podSpec, podMetadata))); + } + + @Test + public void testProvisionResourcesLimitAndRequestAttributeToContainer() throws Exception { + resourceProvisioner.provision(k8sEnv, identity); + assertEquals(container.getResources().getLimits().get("memory").getAmount(), RAM_LIMIT_VALUE); + assertEquals(container.getResources().getLimits().get("cpu").getAmount(), CPU_LIMIT_VALUE); + assertEquals( + container.getResources().getRequests().get("memory").getAmount(), RAM_REQUEST_VALUE); + assertEquals(container.getResources().getRequests().get("cpu").getAmount(), CPU_REQUEST_VALUE); + } + + @Test + public void testOverridesContainerRamLimitAndRequestFromMachineAttribute() throws Exception { + ResourceRequirements resourceRequirements = + new ResourceRequirementsBuilder() + .addToLimits(of("memory", new Quantity("3221225472"), "cpu", new Quantity("0.678"))) + .addToRequests(of("memory", new Quantity("1231231423"), "cpu", new Quantity("0.333"))) + .build(); + container.setResources(resourceRequirements); + + resourceProvisioner.provision(k8sEnv, identity); + + assertEquals(container.getResources().getLimits().get("memory").getAmount(), RAM_LIMIT_VALUE); + assertEquals(container.getResources().getLimits().get("cpu").getAmount(), CPU_LIMIT_VALUE); + assertEquals( + container.getResources().getRequests().get("memory").getAmount(), RAM_REQUEST_VALUE); + assertEquals(container.getResources().getRequests().get("cpu").getAmount(), CPU_REQUEST_VALUE); + } +} From 8f3e650677f4821d038a63d246aa20b9483d37b1 Mon Sep 17 00:00:00 2001 From: Max Shaposhnik Date: Tue, 25 Feb 2020 09:49:31 +0200 Subject: [PATCH 28/29] Fixup messsage --- .../kubernetes/wsplugins/MachineResolverBuilder.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/wsplugins/MachineResolverBuilder.java b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/wsplugins/MachineResolverBuilder.java index db996172134..f8db1f7751d 100644 --- a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/wsplugins/MachineResolverBuilder.java +++ b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/wsplugins/MachineResolverBuilder.java @@ -40,7 +40,7 @@ public MachineResolver build() { || defaultSidecarCpuRequestAttribute == null || containerEndpoints == null || projectsRootPathEnvVar == null) { - throw new IllegalStateException("Unable to build MachineResolver cause some fields are null"); + throw new IllegalStateException("Unable to build MachineResolver because some fields are null"); } return new MachineResolver( From f28ab3755b4896879d141cff628c539ebe456aad Mon Sep 17 00:00:00 2001 From: Max Shaposhnik Date: Tue, 25 Feb 2020 13:46:15 +0200 Subject: [PATCH 29/29] Format fix --- .../kubernetes/wsplugins/MachineResolverBuilder.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/wsplugins/MachineResolverBuilder.java b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/wsplugins/MachineResolverBuilder.java index f8db1f7751d..47fe2516d22 100644 --- a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/wsplugins/MachineResolverBuilder.java +++ b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/wsplugins/MachineResolverBuilder.java @@ -40,7 +40,8 @@ public MachineResolver build() { || defaultSidecarCpuRequestAttribute == null || containerEndpoints == null || projectsRootPathEnvVar == null) { - throw new IllegalStateException("Unable to build MachineResolver because some fields are null"); + throw new IllegalStateException( + "Unable to build MachineResolver because some fields are null"); } return new MachineResolver(