Validate cpu shares better and scale by 32 for each minVcpu

2 files changed, 52 insertions, 3 deletions

diff --git a/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/ContainerResources.java b/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/ContainerResources.java
index c3c4ca19555..70ba58cd9cf 100644
--- a/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/ContainerResources.java
+++ b/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/ContainerResources.java
@@ -35,8 +35,8 @@ public class ContainerResources {
 
         if (cpus < 0)
             throw new IllegalArgumentException("CPUs must be a positive number or 0 for unlimited, was " + cpus);
-        if (cpuShares < 0)
-            throw new IllegalArgumentException("CPU shares must be a positive integer or 0 for unlimited, was " + cpuShares);
+        if (cpuShares != 0 && (cpuShares < 2 || cpuShares > 262_144))
+            throw new IllegalArgumentException("CPU shares must be a positive integer in [2, 262144] or 0 for unlimited, was " + cpuShares);
         if (memoryBytes < 0)
             throw new IllegalArgumentException("memoryBytes must be a positive integer or 0 for unlimited, was " + memoryBytes);
     }
@@ -54,7 +54,7 @@ public class ContainerResources {
      */
     public static ContainerResources from(double maxVcpu, double minVcpu, double memoryGb) {
         return new ContainerResources(maxVcpu,
-                                      (int) Math.round(10 * minVcpu),
+                                      (int) Math.round(32 * minVcpu),
                                       (long) ((1L << 30) * memoryGb));
     }
 
diff --git a/docker-api/src/test/java/com/yahoo/vespa/hosted/dockerapi/ContainerResourcesTest.java b/docker-api/src/test/java/com/yahoo/vespa/hosted/dockerapi/ContainerResourcesTest.java
new file mode 100644
index 00000000000..daf4639ad29
--- /dev/null
+++ b/docker-api/src/test/java/com/yahoo/vespa/hosted/dockerapi/ContainerResourcesTest.java
@@ -0,0 +1,49 @@
+// Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+package com.yahoo.vespa.hosted.dockerapi;
+
+import org.junit.Test;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.fail;
+
+/**
+ * @author freva
+ */
+public class ContainerResourcesTest {
+
+    @Test
+    public void verify_unlimited() {
+        assertEquals(-1, ContainerResources.UNLIMITED.cpuQuota());
+        assertEquals(100_000, ContainerResources.UNLIMITED.cpuPeriod());
+        assertEquals(0, ContainerResources.UNLIMITED.cpuShares());
+    }
+
+    @Test
+    public void validate_shares() {
+        new ContainerResources(0, 0, 0);
+        new ContainerResources(0, 2, 0);
+        new ContainerResources(0, 2048, 0);
+        new ContainerResources(0, 262_144, 0);
+
+        assertThrows(IllegalArgumentException.class, () -> new ContainerResources(0, -1, 0)); // Negative shares not allowed
+        assertThrows(IllegalArgumentException.class, () -> new ContainerResources(0, 1, 0)); // 1 share not allowed
+        assertThrows(IllegalArgumentException.class, () -> new ContainerResources(0, 262_145, 0));
+    }
+
+    @Test
+    public void cpu_shares_scaling() {
+        ContainerResources resources = ContainerResources.from(5.3, 2.5, 0);
+        assertEquals(530_000, resources.cpuQuota());
+        assertEquals(100_000, resources.cpuPeriod());
+        assertEquals(80, resources.cpuShares());
+    }
+
+    private static void assertThrows(Class<? extends Throwable> clazz, Runnable runnable) {
+        try {
+            runnable.run();
+            fail("Expected " + clazz);
+        } catch (Throwable e) {
+            if (!clazz.isInstance(e)) throw e;
+        }
+    }
+}