From 2fa4e9c0e212337ff981426d9618fe634fbdff63 Mon Sep 17 00:00:00 2001 From: Danielle Lancashire Date: Thu, 2 Dec 2021 17:25:13 +0100 Subject: [PATCH 1/3] cri: add support for configuring swap Signed-off-by: Danielle Lancashire --- .../container_update_resources_test.go | 164 +++++++++++++++++- pkg/cri/opts/spec_linux.go | 5 + 2 files changed, 168 insertions(+), 1 deletion(-) diff --git a/integration/container_update_resources_test.go b/integration/container_update_resources_test.go index 2b2868629..7da55278d 100644 --- a/integration/container_update_resources_test.go +++ b/integration/container_update_resources_test.go @@ -20,9 +20,14 @@ package integration import ( + "bytes" + "io/ioutil" + "strings" "testing" "github.com/containerd/cgroups" + cgroupsv2 "github.com/containerd/cgroups/v2" + "github.com/containerd/containerd" runtimespec "github.com/opencontainers/runtime-spec/specs-go" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -39,7 +44,164 @@ func checkMemoryLimit(t *testing.T, spec *runtimespec.Spec, memLimit int64) { assert.Equal(t, memLimit, *spec.Linux.Resources.Memory.Limit) } -func TestUpdateContainerResources(t *testing.T) { +func checkMemorySwapLimit(t *testing.T, spec *runtimespec.Spec, memLimit *int64) { + require.NotNil(t, spec) + require.NotNil(t, spec.Linux) + require.NotNil(t, spec.Linux.Resources) + require.NotNil(t, spec.Linux.Resources.Memory) + if memLimit == nil { + require.Nil(t, spec.Linux.Resources.Memory.Swap) + } else { + require.NotNil(t, spec.Linux.Resources.Memory.Swap) + assert.Equal(t, *memLimit, *spec.Linux.Resources.Memory.Swap) + } +} + +func getCgroupSwapLimitForTask(t *testing.T, task containerd.Task) uint64 { + if cgroups.Mode() == cgroups.Unified { + groupPath, err := cgroupsv2.PidGroupPath(int(task.Pid())) + if err != nil { + t.Fatal(err) + } + cgroup2, err := cgroupsv2.LoadManager("/sys/fs/cgroup", groupPath) + if err != nil { + t.Fatal(err) + } + stat, err := cgroup2.Stat() + if err != nil { + t.Fatal(err) + } + return stat.Memory.SwapLimit + } + cgroup, err := cgroups.Load(cgroups.V1, cgroups.PidPath(int(task.Pid()))) + if err != nil { + t.Fatal(err) + } + stat, err := cgroup.Stat(cgroups.IgnoreNotExist) + if err != nil { + t.Fatal(err) + } + return stat.Memory.HierarchicalSwapLimit +} + +func getCgroupMemoryLimitForTask(t *testing.T, task containerd.Task) uint64 { + if cgroups.Mode() == cgroups.Unified { + groupPath, err := cgroupsv2.PidGroupPath(int(task.Pid())) + if err != nil { + t.Fatal(err) + } + cgroup2, err := cgroupsv2.LoadManager("/sys/fs/cgroup", groupPath) + if err != nil { + t.Fatal(err) + } + stat, err := cgroup2.Stat() + if err != nil { + t.Fatal(err) + } + return stat.Memory.UsageLimit + } + + cgroup, err := cgroups.Load(cgroups.V1, cgroups.PidPath(int(task.Pid()))) + if err != nil { + t.Fatal(err) + } + stat, err := cgroup.Stat(cgroups.IgnoreNotExist) + if err != nil { + t.Fatal(err) + } + return stat.Memory.Usage.Limit +} + +func isSwapLikelyEnabled() bool { + // Check whether swap is enabled. + swapFile := "/proc/swaps" + swapData, err := ioutil.ReadFile(swapFile) + if err != nil { + // We can't read the file or it doesn't exist, assume we don't have swap. + return false + } + + swapData = bytes.TrimSpace(swapData) // extra trailing \n + swapLines := strings.Split(string(swapData), "\n") + // If there is more than one line (table headers) in /proc/swaps, swap is enabled + return len(swapLines) > 1 +} + +func TestUpdateContainerResources_MemorySwap(t *testing.T) { + if !isSwapLikelyEnabled() { + t.Skipf("Swap is not enabled, or /proc/swaps is not readable. Swap is required for this test") + return + } + + t.Log("Create a sandbox") + sb, sbConfig := PodSandboxConfigWithCleanup(t, "sandbox", "update-container-swap-resources") + + EnsureImageExists(t, pauseImage) + + memoryLimit := int64(128 * 1024 * 1024) + baseSwapLimit := int64(200 * 1024 * 1024) + increasedSwapLimit := int64(256 * 1024 * 1024) + + t.Log("Create a container with memory limit but no swap") + cnConfig := ContainerConfig( + "container", + pauseImage, + WithResources(&runtime.LinuxContainerResources{ + MemoryLimitInBytes: memoryLimit, + MemorySwapLimitInBytes: baseSwapLimit, + }), + ) + cn, err := runtimeService.CreateContainer(sb, cnConfig, sbConfig) + require.NoError(t, err) + + t.Log("Check memory limit in container OCI spec") + container, err := containerdClient.LoadContainer(context.Background(), cn) + require.NoError(t, err) + spec, err := container.Spec(context.Background()) + require.NoError(t, err) + checkMemoryLimit(t, spec, memoryLimit) + checkMemorySwapLimit(t, spec, &baseSwapLimit) + + t.Log("Update container swap limit after created") + err = runtimeService.UpdateContainerResources(cn, &runtime.LinuxContainerResources{ + MemorySwapLimitInBytes: baseSwapLimit, + }, nil) + require.NoError(t, err) + + t.Log("Check memory limit in container OCI spec") + spec, err = container.Spec(context.Background()) + require.NoError(t, err) + sw1 := baseSwapLimit + checkMemorySwapLimit(t, spec, &sw1) + + t.Log("Start the container") + require.NoError(t, runtimeService.StartContainer(cn)) + task, err := container.Task(context.Background(), nil) + require.NoError(t, err) + + t.Log("Check memory limit in cgroup") + memLimit := getCgroupMemoryLimitForTask(t, task) + swapLimit := getCgroupSwapLimitForTask(t, task) + assert.Equal(t, uint64(memoryLimit), memLimit) + assert.Equal(t, uint64(baseSwapLimit-memoryLimit), swapLimit) + + t.Log("Update container memory limit after started") + err = runtimeService.UpdateContainerResources(cn, &runtime.LinuxContainerResources{ + MemorySwapLimitInBytes: increasedSwapLimit, + }, nil) + require.NoError(t, err) + + t.Log("Check memory limit in container OCI spec") + spec, err = container.Spec(context.Background()) + require.NoError(t, err) + checkMemorySwapLimit(t, spec, &increasedSwapLimit) + + t.Log("Check memory limit in cgroup") + swapLimit = getCgroupSwapLimitForTask(t, task) + assert.Equal(t, uint64(increasedSwapLimit-memoryLimit), swapLimit) +} + +func TestUpdateContainerResources_MemoryLimit(t *testing.T) { // TODO(claudiub): Make this test work once https://github.com/microsoft/hcsshim/pull/931 merges. t.Log("Create a sandbox") sb, sbConfig := PodSandboxConfigWithCleanup(t, "sandbox", "update-container-resources") diff --git a/pkg/cri/opts/spec_linux.go b/pkg/cri/opts/spec_linux.go index ba002c93a..1491875d0 100644 --- a/pkg/cri/opts/spec_linux.go +++ b/pkg/cri/opts/spec_linux.go @@ -450,6 +450,7 @@ func WithResources(resources *runtime.LinuxContainerResources, tolerateMissingHu q = resources.GetCpuQuota() shares = uint64(resources.GetCpuShares()) limit = resources.GetMemoryLimitInBytes() + swapLimit = resources.GetMemorySwapLimitInBytes() hugepages = resources.GetHugepageLimits() ) @@ -471,6 +472,10 @@ func WithResources(resources *runtime.LinuxContainerResources, tolerateMissingHu if limit != 0 { s.Linux.Resources.Memory.Limit = &limit } + if swapLimit != 0 { + s.Linux.Resources.Memory.Swap = &swapLimit + } + if !disableHugetlbController { if isHugetlbControllerPresent() { for _, limit := range hugepages { From c5b0a18b6ea568ceda225260906b524193078b47 Mon Sep 17 00:00:00 2001 From: Danielle Lancashire Date: Fri, 3 Dec 2021 14:32:35 +0100 Subject: [PATCH 2/3] fixup: handle diff between cgroupsv1 and v2 Signed-off-by: Danielle Lancashire --- .../container_update_resources_test.go | 20 ++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/integration/container_update_resources_test.go b/integration/container_update_resources_test.go index 7da55278d..12ea400df 100644 --- a/integration/container_update_resources_test.go +++ b/integration/container_update_resources_test.go @@ -142,6 +142,14 @@ func TestUpdateContainerResources_MemorySwap(t *testing.T) { baseSwapLimit := int64(200 * 1024 * 1024) increasedSwapLimit := int64(256 * 1024 * 1024) + expectedBaseSwap := baseSwapLimit + expectedIncreasedSwap := increasedSwapLimit + + if cgroups.Mode() == cgroups.Unified { + expectedBaseSwap = baseSwapLimit - memoryLimit + expectedIncreasedSwap = increasedSwapLimit - memoryLimit + } + t.Log("Create a container with memory limit but no swap") cnConfig := ContainerConfig( "container", @@ -160,13 +168,7 @@ func TestUpdateContainerResources_MemorySwap(t *testing.T) { spec, err := container.Spec(context.Background()) require.NoError(t, err) checkMemoryLimit(t, spec, memoryLimit) - checkMemorySwapLimit(t, spec, &baseSwapLimit) - - t.Log("Update container swap limit after created") - err = runtimeService.UpdateContainerResources(cn, &runtime.LinuxContainerResources{ - MemorySwapLimitInBytes: baseSwapLimit, - }, nil) - require.NoError(t, err) + checkMemorySwapLimit(t, spec, &expectedBaseSwap) t.Log("Check memory limit in container OCI spec") spec, err = container.Spec(context.Background()) @@ -183,7 +185,7 @@ func TestUpdateContainerResources_MemorySwap(t *testing.T) { memLimit := getCgroupMemoryLimitForTask(t, task) swapLimit := getCgroupSwapLimitForTask(t, task) assert.Equal(t, uint64(memoryLimit), memLimit) - assert.Equal(t, uint64(baseSwapLimit-memoryLimit), swapLimit) + assert.Equal(t, uint64(expectedBaseSwap), swapLimit) t.Log("Update container memory limit after started") err = runtimeService.UpdateContainerResources(cn, &runtime.LinuxContainerResources{ @@ -198,7 +200,7 @@ func TestUpdateContainerResources_MemorySwap(t *testing.T) { t.Log("Check memory limit in cgroup") swapLimit = getCgroupSwapLimitForTask(t, task) - assert.Equal(t, uint64(increasedSwapLimit-memoryLimit), swapLimit) + assert.Equal(t, uint64(expectedIncreasedSwap), swapLimit) } func TestUpdateContainerResources_MemoryLimit(t *testing.T) { From 533dd1c0ee3a2837ac7ba366f9711b0fab800e75 Mon Sep 17 00:00:00 2001 From: Danielle Lancashire Date: Fri, 3 Dec 2021 16:04:01 +0100 Subject: [PATCH 3/3] fixup: check for swap accounting Signed-off-by: Danielle Lancashire --- .../container_update_resources_test.go | 20 +++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/integration/container_update_resources_test.go b/integration/container_update_resources_test.go index 12ea400df..5067b54d2 100644 --- a/integration/container_update_resources_test.go +++ b/integration/container_update_resources_test.go @@ -22,6 +22,7 @@ package integration import ( "bytes" "io/ioutil" + "os" "strings" "testing" @@ -123,13 +124,28 @@ func isSwapLikelyEnabled() bool { swapData = bytes.TrimSpace(swapData) // extra trailing \n swapLines := strings.Split(string(swapData), "\n") + // If there is more than one line (table headers) in /proc/swaps, swap is enabled - return len(swapLines) > 1 + if len(swapLines) <= 1 { + return false + } + + // Linux Kernel's prior to 5.8 can disable swap accounting and is disabled + // by default on Ubuntu. Most systems that run with cgroupsv2 enabled likely + // have swap accounting enabled, here we assume that is true when running with + // cgroupsv2 and check on cgroupsv1. + if cgroups.Mode() == cgroups.Unified { + return true + } + + _, err = os.Stat("/sys/fs/cgroup/memory/memory.memsw.limit_in_bytes") + // Assume any error means this test can't run for now. + return err == nil } func TestUpdateContainerResources_MemorySwap(t *testing.T) { if !isSwapLikelyEnabled() { - t.Skipf("Swap is not enabled, or /proc/swaps is not readable. Swap is required for this test") + t.Skipf("Swap or swap accounting are not enabled. Swap is required for this test") return }