Improve cgroup modeling

4 files changed, 56 insertions, 49 deletions

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/cgroup/ControlGroupTest.java
index 789f31f75c6..7c040f4ed06 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/cgroup/ControlGroupTest.java
@@ -1,7 +1,9 @@
 // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
-package com.yahoo.vespa.hosted.node.admin.container;
 
-import com.yahoo.collections.Pair;
+// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+package com.yahoo.vespa.hosted.node.admin.cgroup;
+
+import com.yahoo.vespa.hosted.node.admin.container.ContainerId;
 import com.yahoo.vespa.hosted.node.admin.nodeagent.NodeAgentContext;
 import com.yahoo.vespa.hosted.node.admin.nodeagent.NodeAgentContextImpl;
 import com.yahoo.vespa.hosted.node.admin.task.util.file.UnixPath;
@@ -12,16 +14,15 @@ import java.io.IOException;
 import java.nio.file.FileSystem;
 import java.util.Map;
 import java.util.Optional;
-import java.util.OptionalInt;
-
-import static com.yahoo.vespa.hosted.node.admin.container.CGroupV2.CpuStatField.SYSTEM_USAGE_USEC;
-import static com.yahoo.vespa.hosted.node.admin.container.CGroupV2.CpuStatField.THROTTLED_PERIODS;
-import static com.yahoo.vespa.hosted.node.admin.container.CGroupV2.CpuStatField.THROTTLED_TIME_USEC;
-import static com.yahoo.vespa.hosted.node.admin.container.CGroupV2.CpuStatField.TOTAL_PERIODS;
-import static com.yahoo.vespa.hosted.node.admin.container.CGroupV2.CpuStatField.TOTAL_USAGE_USEC;
-import static com.yahoo.vespa.hosted.node.admin.container.CGroupV2.CpuStatField.USER_USAGE_USEC;
-import static com.yahoo.vespa.hosted.node.admin.container.CGroupV2.sharesToWeight;
-import static com.yahoo.vespa.hosted.node.admin.container.CGroupV2.weightToShares;
+
+import static com.yahoo.vespa.hosted.node.admin.cgroup.CpuController.StatField.SYSTEM_USAGE_USEC;
+import static com.yahoo.vespa.hosted.node.admin.cgroup.CpuController.StatField.THROTTLED_PERIODS;
+import static com.yahoo.vespa.hosted.node.admin.cgroup.CpuController.StatField.THROTTLED_TIME_USEC;
+import static com.yahoo.vespa.hosted.node.admin.cgroup.CpuController.StatField.TOTAL_PERIODS;
+import static com.yahoo.vespa.hosted.node.admin.cgroup.CpuController.StatField.TOTAL_USAGE_USEC;
+import static com.yahoo.vespa.hosted.node.admin.cgroup.CpuController.StatField.USER_USAGE_USEC;
+import static com.yahoo.vespa.hosted.node.admin.cgroup.CpuController.sharesToWeight;
+import static com.yahoo.vespa.hosted.node.admin.cgroup.CpuController.weightToShares;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertFalse;
 import static org.junit.jupiter.api.Assertions.assertTrue;
@@ -29,47 +30,48 @@ import static org.junit.jupiter.api.Assertions.assertTrue;
 /**
  * @author freva
  */
-public class CGroupV2Test {
+public class ControlGroupTest {
 
     private static final ContainerId containerId = new ContainerId("4aec78cc");
 
     private final FileSystem fileSystem = TestFileSystem.create();
-    private final CGroupV2 cgroup = new CGroupV2(fileSystem);
+    private final ControlGroup containerCgroup = ControlGroup.root(fileSystem).resolveContainer(containerId);
+    private final CpuController containerCpu = containerCgroup.cpu();
     private final NodeAgentContext context = NodeAgentContextImpl.builder("node123.yahoo.com").fileSystem(fileSystem).build();
     private final UnixPath cgroupRoot = new UnixPath(fileSystem.getPath("/sys/fs/cgroup/machine.slice/libpod-4aec78cc.scope/container")).createDirectories();
 
     @Test
     public void updates_cpu_quota_and_period() {
-        assertEquals(Optional.empty(), cgroup.cpuQuotaPeriod(containerId));
+        assertEquals(Optional.empty(), containerCgroup.cpu().readMax());
 
         cgroupRoot.resolve("cpu.max").writeUtf8File("max 100000\n");
-        assertEquals(Optional.of(new Pair<>(-1, 100000)), cgroup.cpuQuotaPeriod(containerId));
+        assertEquals(Optional.of(new CpuController.Max(Size.max(), 100000)), containerCpu.readMax());
 
         cgroupRoot.resolve("cpu.max").writeUtf8File("456 123456\n");
-        assertEquals(Optional.of(new Pair<>(456, 123456)), cgroup.cpuQuotaPeriod(containerId));
+        assertEquals(Optional.of(new CpuController.Max(Size.from(456), 123456)), containerCpu.readMax());
 
-        assertFalse(cgroup.updateCpuQuotaPeriod(context, containerId, 456, 123456));
+        containerCgroup.cpu().updateMax(context, 456, 123456);
 
-        assertTrue(cgroup.updateCpuQuotaPeriod(context, containerId, 654, 123456));
-        assertEquals(Optional.of(new Pair<>(654, 123456)), cgroup.cpuQuotaPeriod(containerId));
+        assertTrue(containerCgroup.cpu().updateMax(context, 654, 123456));
+        assertEquals(Optional.of(new CpuController.Max(Size.from(654), 123456)), containerCpu.readMax());
         assertEquals("654 123456", cgroupRoot.resolve("cpu.max").readUtf8File());
 
-        assertTrue(cgroup.updateCpuQuotaPeriod(context, containerId, -1, 123456));
-        assertEquals(Optional.of(new Pair<>(-1, 123456)), cgroup.cpuQuotaPeriod(containerId));
+        assertTrue(containerCgroup.cpu().updateMax(context, -1, 123456));
+        assertEquals(Optional.of(new CpuController.Max(Size.max(), 123456)), containerCpu.readMax());
         assertEquals("max 123456", cgroupRoot.resolve("cpu.max").readUtf8File());
     }
 
     @Test
     public void updates_cpu_shares() {
-        assertEquals(OptionalInt.empty(), cgroup.cpuShares(containerId));
+        assertEquals(Optional.empty(), containerCgroup.cpu().readShares());
 
         cgroupRoot.resolve("cpu.weight").writeUtf8File("1\n");
-        assertEquals(OptionalInt.of(2), cgroup.cpuShares(containerId));
+        assertEquals(Optional.of(2), containerCgroup.cpu().readShares());
 
-        assertFalse(cgroup.updateCpuShares(context, containerId, 2));
+        assertFalse(containerCgroup.cpu().updateShares(context, 2));
 
-        assertTrue(cgroup.updateCpuShares(context, containerId, 12345));
-        assertEquals(OptionalInt.of(12323), cgroup.cpuShares(containerId));
+        assertTrue(containerCgroup.cpu().updateShares(context, 12345));
+        assertEquals(Optional.of(12323), containerCgroup.cpu().readShares());
     }
 
     @Test
@@ -82,16 +84,16 @@ public class CGroupV2Test {
                 "throttled_usec 14256\n");
 
         assertEquals(Map.of(TOTAL_USAGE_USEC, 17794243L, USER_USAGE_USEC, 16099205L, SYSTEM_USAGE_USEC, 1695038L,
-                TOTAL_PERIODS, 12465L, THROTTLED_PERIODS, 25L, THROTTLED_TIME_USEC, 14256L), cgroup.cpuStats(containerId));
+                TOTAL_PERIODS, 12465L, THROTTLED_PERIODS, 25L, THROTTLED_TIME_USEC, 14256L), containerCgroup.cpu().readStats());
     }
 
     @Test
     public void reads_memory_metrics() throws IOException {
         cgroupRoot.resolve("memory.current").writeUtf8File("2525093888\n");
-        assertEquals(2525093888L, cgroup.memoryUsageInBytes(containerId));
+        assertEquals(2525093888L, containerCgroup.memory().readCurrent().value());
 
         cgroupRoot.resolve("memory.max").writeUtf8File("4322885632\n");
-        assertEquals(4322885632L, cgroup.memoryLimitInBytes(containerId));
+        assertEquals(4322885632L, containerCgroup.memory().readMax().value());
 
         cgroupRoot.resolve("memory.stat").writeUtf8File("anon 3481600\n" +
                 "file 69206016\n" +
@@ -102,7 +104,7 @@ public class CGroupV2Test {
                 "shmem 8380416\n" +
                 "file_mapped 1081344\n" +
                 "file_dirty 135168\n");
-        assertEquals(69206016L, cgroup.memoryCacheInBytes(containerId));
+        assertEquals(69206016L, containerCgroup.memory().readFileSystemCache().value());
     }
 
     @Test
diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/container/ContainerOperationsTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/container/ContainerOperationsTest.java
index 701dd33cf55..5dbc2128051 100644
--- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/container/ContainerOperationsTest.java
+++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/container/ContainerOperationsTest.java
@@ -2,6 +2,7 @@
 package com.yahoo.vespa.hosted.node.admin.container;
 
 import com.yahoo.config.provision.DockerImage;
+import com.yahoo.vespa.hosted.node.admin.cgroup.ControlGroup;
 import com.yahoo.vespa.hosted.node.admin.component.TestTaskContext;
 import com.yahoo.vespa.test.file.TestFileSystem;
 import org.junit.jupiter.api.Test;
@@ -11,9 +12,11 @@ import java.time.Instant;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
-import java.util.stream.Collectors;
 
-import static org.junit.jupiter.api.Assertions.*;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.mockito.Mockito.mock;
 
 /**
  * @author mpolden
@@ -23,7 +26,7 @@ public class ContainerOperationsTest {
     private final TestTaskContext context = new TestTaskContext();
     private final ContainerEngineMock containerEngine = new ContainerEngineMock();
     private final FileSystem fileSystem = TestFileSystem.create();
-    private final ContainerOperations containerOperations = new ContainerOperations(containerEngine, new CGroupV2(fileSystem), fileSystem);
+    private final ContainerOperations containerOperations = new ContainerOperations(containerEngine, mock(ControlGroup.class), fileSystem);
 
     @Test
     void no_managed_containers_running() {
diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/container/ContainerStatsCollectorTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/container/ContainerStatsCollectorTest.java
index 72c5d016a47..59110ef4bd2 100644
--- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/container/ContainerStatsCollectorTest.java
+++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/container/ContainerStatsCollectorTest.java
@@ -1,6 +1,8 @@
 // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.vespa.hosted.node.admin.container;
 
+import com.yahoo.vespa.hosted.node.admin.cgroup.ControlGroup;
+import com.yahoo.vespa.hosted.node.admin.cgroup.Size;
 import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.NodeSpec;
 import com.yahoo.vespa.hosted.node.admin.nodeagent.NodeAgentContext;
 import com.yahoo.vespa.hosted.node.admin.nodeagent.NodeAgentContextImpl;
@@ -8,6 +10,7 @@ import com.yahoo.vespa.hosted.node.admin.task.util.file.UnixPath;
 import com.yahoo.vespa.hosted.node.admin.task.util.process.TestTerminal;
 import com.yahoo.vespa.test.file.TestFileSystem;
 import org.junit.jupiter.api.Test;
+import org.mockito.Answers;
 
 import java.io.IOException;
 import java.nio.file.FileSystem;
@@ -17,12 +20,12 @@ import java.util.List;
 import java.util.Map;
 import java.util.Optional;
 
-import static com.yahoo.vespa.hosted.node.admin.container.CGroupV2.CpuStatField.SYSTEM_USAGE_USEC;
-import static com.yahoo.vespa.hosted.node.admin.container.CGroupV2.CpuStatField.THROTTLED_PERIODS;
-import static com.yahoo.vespa.hosted.node.admin.container.CGroupV2.CpuStatField.THROTTLED_TIME_USEC;
-import static com.yahoo.vespa.hosted.node.admin.container.CGroupV2.CpuStatField.TOTAL_PERIODS;
-import static com.yahoo.vespa.hosted.node.admin.container.CGroupV2.CpuStatField.TOTAL_USAGE_USEC;
-import static com.yahoo.vespa.hosted.node.admin.container.CGroupV2.CpuStatField.USER_USAGE_USEC;
+import static com.yahoo.vespa.hosted.node.admin.cgroup.CpuController.StatField.SYSTEM_USAGE_USEC;
+import static com.yahoo.vespa.hosted.node.admin.cgroup.CpuController.StatField.THROTTLED_PERIODS;
+import static com.yahoo.vespa.hosted.node.admin.cgroup.CpuController.StatField.THROTTLED_TIME_USEC;
+import static com.yahoo.vespa.hosted.node.admin.cgroup.CpuController.StatField.TOTAL_PERIODS;
+import static com.yahoo.vespa.hosted.node.admin.cgroup.CpuController.StatField.TOTAL_USAGE_USEC;
+import static com.yahoo.vespa.hosted.node.admin.cgroup.CpuController.StatField.USER_USAGE_USEC;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 import static org.mockito.ArgumentMatchers.eq;
@@ -37,11 +40,10 @@ public class ContainerStatsCollectorTest {
     private final TestTerminal testTerminal = new TestTerminal();
     private final ContainerEngineMock containerEngine = new ContainerEngineMock(testTerminal);
     private final FileSystem fileSystem = TestFileSystem.create();
-    private final CGroupV2 cgroup = mock(CGroupV2.class);
+    private final ControlGroup cgroup = mock(ControlGroup.class, Answers.RETURNS_DEEP_STUBS);
     private final NodeAgentContext context = NodeAgentContextImpl.builder(NodeSpec.Builder.testSpec("c1").build())
                                                                  .fileSystem(TestFileSystem.create())
                                                                  .build();
-
     @Test
     void collect() throws Exception {
         ContainerStatsCollector collector = new ContainerStatsCollector(containerEngine, cgroup, fileSystem, 24);
@@ -92,17 +94,17 @@ public class ContainerStatsCollectorTest {
                                "  eth0: 22280813  118083    3    4    0     0          0         0 19859383  115415    5    6    0     0       0          0\n");
     }
 
-    private void mockMemoryStats(ContainerId containerId) throws IOException {
-        when(cgroup.memoryUsageInBytes(eq(containerId))).thenReturn(1228017664L);
-        when(cgroup.memoryLimitInBytes(eq(containerId))).thenReturn(2147483648L);
-        when(cgroup.memoryCacheInBytes(eq(containerId))).thenReturn(470790144L);
+    private void mockMemoryStats(ContainerId containerId) {
+        when(cgroup.resolveContainer(eq(containerId)).memory().readCurrent()).thenReturn(Size.from(1228017664L));
+        when(cgroup.resolveContainer(eq(containerId)).memory().readMax()).thenReturn(Size.from(2147483648L));
+        when(cgroup.resolveContainer(eq(containerId)).memory().readFileSystemCache()).thenReturn(Size.from(470790144L));
     }
 
     private void mockCpuStats(ContainerId containerId) throws IOException {
         UnixPath proc = new UnixPath(fileSystem.getPath("/proc"));
         proc.createDirectories();
 
-        when(cgroup.cpuStats(eq(containerId))).thenReturn(Map.of(
+        when(cgroup.resolveContainer(eq(containerId)).cpu().readStats()).thenReturn(Map.of(
                 TOTAL_USAGE_USEC, 691675615472L, SYSTEM_USAGE_USEC, 262190000000L, USER_USAGE_USEC, 40900L,
                 TOTAL_PERIODS, 1L, THROTTLED_PERIODS, 2L, THROTTLED_TIME_USEC, 3L));
 
diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integration/ContainerTester.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integration/ContainerTester.java
index 7676d0e1790..990b8c5997b 100644
--- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integration/ContainerTester.java
+++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integration/ContainerTester.java
@@ -5,9 +5,9 @@ import com.yahoo.config.provision.DockerImage;
 import com.yahoo.config.provision.HostName;
 import com.yahoo.config.provision.NodeType;
 import com.yahoo.vespa.flags.InMemoryFlagSource;
+import com.yahoo.vespa.hosted.node.admin.cgroup.ControlGroup;
 import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.NodeSpec;
 import com.yahoo.vespa.hosted.node.admin.configserver.orchestrator.Orchestrator;
-import com.yahoo.vespa.hosted.node.admin.container.CGroupV2;
 import com.yahoo.vespa.hosted.node.admin.container.ContainerEngineMock;
 import com.yahoo.vespa.hosted.node.admin.container.ContainerName;
 import com.yahoo.vespa.hosted.node.admin.container.ContainerOperations;
@@ -60,7 +60,7 @@ public class ContainerTester implements AutoCloseable {
 
     private final ContainerEngineMock containerEngine = new ContainerEngineMock();
     private final FileSystem fileSystem = TestFileSystem.create();
-    final ContainerOperations containerOperations = spy(new ContainerOperations(containerEngine, new CGroupV2(fileSystem), fileSystem));
+    final ContainerOperations containerOperations = spy(new ContainerOperations(containerEngine, mock(ControlGroup.class), fileSystem));
     final NodeRepoMock nodeRepository = spy(new NodeRepoMock());
     final Orchestrator orchestrator = mock(Orchestrator.class);
     final StorageMaintainer storageMaintainer = mock(StorageMaintainer.class);