cri: memory.memsw.limit_in_bytes: no such file or directory

[mqasimsarfraz]

If kubelet passes the swap limit (default memory limit = swap limit ), it is configured for container irrespective if the node supports swap. This results in following error:

Jul 20 18:14:22.460: INFO: At 2023-07-20 17:58:34 +0000 UTC - event for coredns-5dd5756b68-6fvpf: {kubelet ip-10-0-116-157.us-west-2.compute.internal} Failed: Error: failed to create containerd task: failed to create shim task: OCI runtime create failed: runc create failed: unable to start container process: error during container init: error setting cgroup config for procHooks process: open /sys/fs/cgroup/memory/kubepods.slice/kubepods-burstable.slice/kubepods-burstable-pod05d8a876_3004_479d_baa8_eac4f3309f4d.slice/cri-containerd-coredns.scope/memory.memsw.limit_in_bytes: no such file or directory: unknown

The reason this is coming to surface now is kubelet (recently) by default sets memory limit = swap limit even without NodeSwap=true feature gate is enabled. [1] [2] [3]

Fixes: #8855
Related: kubernetes/kubernetes#119486

[k8s-ci-robot]

Hi @mqasimsarfraz. Thanks for your PR.

I'm waiting for a containerd member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[mikebrow]

/ok-to-test

[mqasimsarfraz]

I am currently trying to validate the fix with Kubernetes env. I will post an update once I have done that!

[mqasimsarfraz]

I am able to validate this on Ubuntu 20.04 with (GRUB_CMDLINE_LINUX_DEFAULT="swapaccount=0") and running Kubernetes using local-up-cluster.sh:

w/o fix

# start the cluster using local-up-cluster.sh
$ ./bin/containerd -v
containerd github.com/containerd/containerd 59959b31f 59959b31ff22fec86841b6f8cec2feec9c2d77b4
$ ./bin/containerd
# start a pod with memory limit
ERRO[2023-07-21T16:59:58.575846524Z] StartContainer for "113289a05db3c8f46e0a4f3b63ced9bce88a6bb958a99c7c8b46c533a34b4a5f" failed  error="failed to create containerd task: failed to create shim task: OCI runtime create failed: runc create failed: unable to start container process: error during container init: error setting cgroup config for procHooks process: openat2 /sys/fs/cgroup/memory/kubepods/burstable/pod42b7dd8a-bfe1-482a-8479-cecc7df747bc/113289a05db3c8f46e0a4f3b63ced9bce88a6bb958a99c7c8b46c533a34b4a5f/memory.memsw.limit_in_bytes: no such file or directory: unknown"

with fix

# start the cluster using local-up-cluster.sh
$ ./bin/containerd -v
containerd github.com/containerd/containerd 06f18c69d 06f18c69d22b81fe040596d50f32d6bca2079a3d
$ ./bin/containerd
# start a pod with memory limit
INFO[2023-07-21T17:09:42.872471778Z] StartContainer for "2e6a487947094a32ab1dca9b5f21af9ccaf7500f724065e78ac29c882d77f2a2" 
INFO[2023-07-21T17:09:42.945758498Z] StartContainer for "2e6a487947094a32ab1dca9b5f21af9ccaf7500f724065e78ac29c882d77f2a2" returns successfully 
INFO[2023-07-21T17:09:42.947833393Z] CreateContainer within sandbox "f4c7e390be92280b29eea8518034e043b010fc1e922cb88b08cd86c6fceffc68" for container &ContainerMetadata{Name:log-aggregator,Attempt:0,} 
INFO[2023-07-21T17:09:42.989804856Z] CreateContainer within sandbox "f4c7e390be92280b29eea8518034e043b010fc1e922cb88b08cd86c6fceffc68" for &ContainerMetadata{Name:log-aggregator,Attempt:0,} returns container id 
...

cc @pacoxu

[estesp]

Do we need this in 1.6 LTS? I assume we will support 1.6 with K8s 1.28, and if so, I assume this fix is needed there as well? Marked them for backport/cherry-pick; someone can unset if that isn't the case.

[fuweid]

LGTM

need to confirm the fix

[fuweid]

I just realized that https://github.com/containerd/containerd/pull/7838/files has fixed it before. @mqasimsarfraz so, this patch will fix it?

[mqasimsarfraz]

I just realized that https://github.com/containerd/containerd/pull/7838/files has fixed it before

@fuweid https://github.com/containerd/containerd/pull/7838/files fixed the case when kubelet wasn't passing any swap limit but we were setting it for consistency ( along side memory limit) while in current case if kubelet passes a swap limit we handle it correctly. So the fix was needed at couple of locations (L483 and L487) in #7836 #7838. Does that answer the question?

[fuweid]

LGTM

[fuweid]

@mqasimsarfraz thanks

[mqasimsarfraz]

I see we are target new stable releases. I opened the cherry-picks for release/1.6 and release/1.7 . I think it would make sense to include this fix? @fuweid @dcantah