Do not return empty CPU quota if unlimited

5 files changed, 16 insertions, 6 deletions

diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/container/CGroup.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/container/CGroup.java
index d194198b2d7..df7043ceb78 100644
--- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/container/CGroup.java
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/container/CGroup.java
@@ -19,10 +19,21 @@ import static com.yahoo.vespa.hosted.node.admin.container.ContainerStatsCollecto
  */
 public interface CGroup {
 
+    /**
+     * Returns quota and period values used for CPU scheduling. This serves as hard cap on CPU usage by allowing
+     * the CGroup to use up to {@code quota} each {@code period}. If uncapped, quota will be negative.
+     *
+     * @param containerId full container ID.
+     * @return CPU quota and period for the given container. Empty if CGroup for this container is not found.
+     */
     Optional<Pair<Integer, Integer>> cpuQuotaPeriod(ContainerId containerId);
+
+    /** @return number of shares allocated to this CGroup for purposes of CPU time scheduling, empty if CGroup not found */
     OptionalInt cpuShares(ContainerId containerId);
 
+    /** Update CPU quota and period for the given container ID, set quota to -1 value for unlimited */
     boolean updateCpuQuotaPeriod(NodeAgentContext context, ContainerId containerId, int cpuQuotaUs, int periodUs);
+
     boolean updateCpuShares(NodeAgentContext context, ContainerId containerId, int shares);
 
     Map<CpuStatField, Long> cpuStats(ContainerId containerId) throws IOException;
diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/container/CGroupV1.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/container/CGroupV1.java
index 62e70ca26aa..7607858ec85 100644
--- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/container/CGroupV1.java
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/container/CGroupV1.java
@@ -37,7 +37,7 @@ public class CGroupV1 implements CGroup {
     @Override
     public Optional<Pair<Integer, Integer>> cpuQuotaPeriod(ContainerId containerId) {
         OptionalInt quota = readCgroupsCpuInt(cfsQuotaPath(containerId));
-        if (quota.isEmpty() || quota.getAsInt() < 0) return Optional.empty();
+        if (quota.isEmpty()) return Optional.empty();
         OptionalInt period = readCgroupsCpuInt(cfsPeriodPath(containerId));
         if (period.isEmpty()) return Optional.empty();
         return Optional.of(new Pair<>(quota.getAsInt(), period.getAsInt()));
diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/container/CGroupV2.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/container/CGroupV2.java
index d9499455d19..0c86829b96d 100644
--- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/container/CGroupV2.java
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/container/CGroupV2.java
@@ -36,10 +36,9 @@ public class CGroupV2 implements CGroup {
     @Override
     public Optional<Pair<Integer, Integer>> cpuQuotaPeriod(ContainerId containerId) {
         return cpuMaxPath(containerId).readUtf8FileIfExists()
-                .filter(s -> !s.startsWith(MAX))
                 .map(s -> {
                     String[] parts = s.strip().split(" ");
-                    return new Pair<>(Integer.parseInt(parts[0]), Integer.parseInt(parts[1]));
+                    return new Pair<>(MAX.equals(parts[0]) ? -1 : Integer.parseInt(parts[0]), Integer.parseInt(parts[1]));
                 });
     }
 
diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/container/CGroupV1Test.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/container/CGroupV1Test.java
index 02ee68a245b..f25001d77cd 100644
--- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/container/CGroupV1Test.java
+++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/container/CGroupV1Test.java
@@ -42,7 +42,7 @@ public class CGroupV1Test {
         UnixPath cpu = new UnixPath(fileSystem.getPath("/sys/fs/cgroup/cpu/machine.slice/libpod-4aec78cc.scope")).createDirectories();
         cpu.resolve("cpu.cfs_period_us").writeUtf8File("123456\n");
         cpu.resolve("cpu.cfs_quota_us").writeUtf8File("-1\n");
-        assertEquals(Optional.empty(), cgroup.cpuQuotaPeriod(containerId));
+        assertEquals(Optional.of(new Pair<>(-1, 123456)), cgroup.cpuQuotaPeriod(containerId));
 
         cpu.resolve("cpu.cfs_quota_us").writeUtf8File("456\n");
         assertEquals(Optional.of(new Pair<>(456, 123456)), cgroup.cpuQuotaPeriod(containerId));
diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/container/CGroupV2Test.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/container/CGroupV2Test.java
index 4bcd983133c..909979342ea 100644
--- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/container/CGroupV2Test.java
+++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/container/CGroupV2Test.java
@@ -43,7 +43,7 @@ public class CGroupV2Test {
         assertEquals(Optional.empty(), cgroup.cpuQuotaPeriod(containerId));
 
         cgroupRoot.resolve("cpu.max").writeUtf8File("max 100000\n");
-        assertEquals(Optional.empty(), cgroup.cpuQuotaPeriod(containerId));
+        assertEquals(Optional.of(new Pair<>(-1, 100000)), cgroup.cpuQuotaPeriod(containerId));
 
         cgroupRoot.resolve("cpu.max").writeUtf8File("456 123456\n");
         assertEquals(Optional.of(new Pair<>(456, 123456)), cgroup.cpuQuotaPeriod(containerId));
@@ -55,7 +55,7 @@ public class CGroupV2Test {
         assertEquals("654 123456", cgroupRoot.resolve("cpu.max").readUtf8File());
 
         assertTrue(cgroup.updateCpuQuotaPeriod(context, containerId, -1, 123456));
-        assertEquals(Optional.empty(), cgroup.cpuQuotaPeriod(containerId));
+        assertEquals(Optional.of(new Pair<>(-1, 123456)), cgroup.cpuQuotaPeriod(containerId));
         assertEquals("max 123456", cgroupRoot.resolve("cpu.max").readUtf8File());
     }