cri: Fix TestUpdateOCILinuxResource for host w/o swap controller

mqasimsarfraz · thaJeztah · commit 91a68edd775b · 2023-01-24T22:05:53.000+01:00
Tested on Ubuntu 20.04 w/o swap controller: ``` $ stat -fc %T /sys/fs/cgroup/ tmpfs $ la -la /sys/fs/cgroup/memory/memory.memsw.limit_in_bytes ls: cannot access '/sys/fs/cgroup/memory/memory.memsw.limit_in_bytes': No such file or directory $ go test -v ./pkg/cri/sbserver/ -run TestUpdateOCILinuxResource === RUN TestUpdateOCILinuxResource === RUN TestUpdateOCILinuxResource/should_be_able_to_patch_the_unified_map === RUN TestUpdateOCILinuxResource/should_be_able_to_update_each_resource === RUN TestUpdateOCILinuxResource/should_skip_empty_fields === RUN TestUpdateOCILinuxResource/should_be_able_to_fill_empty_fields --- PASS: TestUpdateOCILinuxResource (0.00s) --- PASS: TestUpdateOCILinuxResource/should_be_able_to_patch_the_unified_map (0.00s) --- PASS: TestUpdateOCILinuxResource/should_be_able_to_update_each_resource (0.00s) --- PASS: TestUpdateOCILinuxResource/should_skip_empty_fields (0.00s) --- PASS: TestUpdateOCILinuxResource/should_be_able_to_fill_empty_fields (0.00s) PASS ok github.com/containerd/containerd/pkg/cri/sbserver (cached) $ go test -v ./pkg/cri/server/ -run TestUpdateOCILinuxResource === RUN TestUpdateOCILinuxResource === RUN TestUpdateOCILinuxResource/should_be_able_to_update_each_resource === RUN TestUpdateOCILinuxResource/should_skip_empty_fields === RUN TestUpdateOCILinuxResource/should_be_able_to_fill_empty_fields === RUN TestUpdateOCILinuxResource/should_be_able_to_patch_the_unified_map --- PASS: TestUpdateOCILinuxResource (0.00s) --- PASS: TestUpdateOCILinuxResource/should_be_able_to_update_each_resource (0.00s) --- PASS: TestUpdateOCILinuxResource/should_skip_empty_fields (0.00s) --- PASS: TestUpdateOCILinuxResource/should_be_able_to_fill_empty_fields (0.00s) --- PASS: TestUpdateOCILinuxResource/should_be_able_to_patch_the_unified_map (0.00s) PASS ok github.com/containerd/containerd/pkg/cri/server (cached) ``` Signed-off-by: Qasim Sarfraz <qasimsarfraz@microsoft.com> (cherry picked from commit 9c8c450) Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
diff --git a/pkg/cri/opts/spec_linux.go b/pkg/cri/opts/spec_linux.go
@@ -409,7 +409,8 @@ var (
 	swapControllerAvailabilityOnce sync.Once
 )
 
-func swapControllerAvailable() bool {
+// SwapControllerAvailable returns true if the swap controller is available
+func SwapControllerAvailable() bool {
 	swapControllerAvailabilityOnce.Do(func() {
 		const warn = "Failed to detect the availability of the swap controller, assuming not available"
 		p := "/sys/fs/cgroup/memory/memory.memsw.limit_in_bytes"
@@ -479,7 +480,7 @@ func WithResources(resources *runtime.LinuxContainerResources, tolerateMissingHu
 		if limit != 0 {
 			s.Linux.Resources.Memory.Limit = &limit
 			// swap/memory limit should be equal to prevent container from swapping by default
-			if swapLimit == 0 && swapControllerAvailable() {
+			if swapLimit == 0 && SwapControllerAvailable() {
 				s.Linux.Resources.Memory.Swap = &limit
 			}
 		}
diff --git a/pkg/cri/server/container_update_resources_linux_test.go b/pkg/cri/server/container_update_resources_linux_test.go
@@ -26,11 +26,18 @@ import (
 	runtime "k8s.io/cri-api/pkg/apis/runtime/v1"
 
 	criconfig "github.com/containerd/containerd/pkg/cri/config"
+	criopts "github.com/containerd/containerd/pkg/cri/opts"
 )
 
 func TestUpdateOCILinuxResource(t *testing.T) {
 	oomscoreadj := new(int)
 	*oomscoreadj = -500
+	expectedSwap := func(swap int64) *int64 {
+		if criopts.SwapControllerAvailable() {
+			return &swap
+		}
+		return nil
+	}
 	for desc, test := range map[string]struct {
 		spec      *runtimespec.Spec
 		request   *runtime.UpdateContainerResourcesRequest
@@ -72,7 +79,7 @@ func TestUpdateOCILinuxResource(t *testing.T) {
 					Resources: &runtimespec.LinuxResources{
 						Memory: &runtimespec.LinuxMemory{
 							Limit: proto.Int64(54321),
-							Swap:  proto.Int64(54321),
+							Swap:  expectedSwap(54321),
 						},
 						CPU: &runtimespec.LinuxCPU{
 							Shares: proto.Uint64(4444),
@@ -118,7 +125,7 @@ func TestUpdateOCILinuxResource(t *testing.T) {
 					Resources: &runtimespec.LinuxResources{
 						Memory: &runtimespec.LinuxMemory{
 							Limit: proto.Int64(54321),
-							Swap:  proto.Int64(54321),
+							Swap:  expectedSwap(54321),
 						},
 						CPU: &runtimespec.LinuxCPU{
 							Shares: proto.Uint64(4444),
@@ -159,7 +166,7 @@ func TestUpdateOCILinuxResource(t *testing.T) {
 					Resources: &runtimespec.LinuxResources{
 						Memory: &runtimespec.LinuxMemory{
 							Limit: proto.Int64(54321),
-							Swap:  proto.Int64(54321),
+							Swap:  expectedSwap(54321),
 						},
 						CPU: &runtimespec.LinuxCPU{
 							Shares: proto.Uint64(4444),
@@ -208,7 +215,7 @@ func TestUpdateOCILinuxResource(t *testing.T) {
 					Resources: &runtimespec.LinuxResources{
 						Memory: &runtimespec.LinuxMemory{
 							Limit: proto.Int64(54321),
-							Swap:  proto.Int64(54321),
+							Swap:  expectedSwap(54321),
 						},
 						CPU: &runtimespec.LinuxCPU{
 							Shares: proto.Uint64(4444),

Original file line number	Diff line number	Diff line change
`@@ -409,7 +409,8 @@ var (`
`409`	`409`	`swapControllerAvailabilityOnce sync.Once`
`410`	`410`	`)`
`411`	`411`
`412`		`-func swapControllerAvailable() bool {`
	`412`	`+// SwapControllerAvailable returns true if the swap controller is available`
	`413`	`+func SwapControllerAvailable() bool {`
`413`	`414`	`swapControllerAvailabilityOnce.Do(func() {`
`414`	`415`	`const warn = "Failed to detect the availability of the swap controller, assuming not available"`
`415`	`416`	`p := "/sys/fs/cgroup/memory/memory.memsw.limit_in_bytes"`
`@@ -479,7 +480,7 @@ func WithResources(resources *runtime.LinuxContainerResources, tolerateMissingHu`
`479`	`480`	`if limit != 0 {`
`480`	`481`	`s.Linux.Resources.Memory.Limit = &limit`
`481`	`482`	`// swap/memory limit should be equal to prevent container from swapping by default`
`482`		`- if swapLimit == 0 && swapControllerAvailable() {`
	`483`	`+ if swapLimit == 0 && SwapControllerAvailable() {`
`483`	`484`	`s.Linux.Resources.Memory.Swap = &limit`
`484`	`485`	`}`
`485`	`486`	`}`