Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

do not touch swap for cgroup v1 if not available #119486

Merged
merged 2 commits into from Jul 24, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion go.mod
Expand Up @@ -21,6 +21,7 @@ require (
github.com/armon/go-socks5 v0.0.0-20160902184237-e75332964ef5
github.com/blang/semver/v4 v4.0.0
github.com/container-storage-interface/spec v1.8.0
github.com/containerd/cgroups v1.1.0
github.com/coredns/corefile-migration v1.0.20
github.com/coreos/go-oidc v2.2.1+incompatible
github.com/coreos/go-systemd/v22 v22.5.0
Expand Down Expand Up @@ -151,7 +152,6 @@ require (
github.com/chai2010/gettext-go v1.0.2 // indirect
github.com/checkpoint-restore/go-criu/v5 v5.3.0 // indirect
github.com/cilium/ebpf v0.9.1 // indirect
github.com/containerd/cgroups v1.1.0 // indirect
github.com/containerd/console v1.0.3 // indirect
github.com/containerd/ttrpc v1.2.2 // indirect
github.com/coredns/caddy v1.1.1 // indirect
Expand Down
49 changes: 43 additions & 6 deletions pkg/kubelet/kuberuntime/kuberuntime_container_linux.go
Expand Up @@ -20,21 +20,26 @@ limitations under the License.
package kuberuntime

import (
"errors"
"fmt"
cadvisorv1 "github.com/google/cadvisor/info/v1"
kubeapiqos "k8s.io/kubernetes/pkg/apis/core/v1/helper/qos"
"math"
"os"
"path/filepath"
"strconv"
"sync"
"time"

"github.com/containerd/cgroups"
cadvisorv1 "github.com/google/cadvisor/info/v1"
libcontainercgroups "github.com/opencontainers/runc/libcontainer/cgroups"

v1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/resource"
utilfeature "k8s.io/apiserver/pkg/util/feature"
runtimeapi "k8s.io/cri-api/pkg/apis/runtime/v1"
"k8s.io/klog/v2"
v1helper "k8s.io/kubernetes/pkg/apis/core/v1/helper"
kubeapiqos "k8s.io/kubernetes/pkg/apis/core/v1/helper/qos"
kubefeatures "k8s.io/kubernetes/pkg/features"
"k8s.io/kubernetes/pkg/kubelet/cm"
kubecontainer "k8s.io/kubernetes/pkg/kubelet/container"
Expand Down Expand Up @@ -305,6 +310,36 @@ var isCgroup2UnifiedMode = func() bool {
return libcontainercgroups.IsCgroup2UnifiedMode()
}

var (
swapControllerAvailability bool
swapControllerAvailabilityOnce sync.Once
)

func swapControllerAvailable() bool {
// See https://github.com/containerd/containerd/pull/7838/
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"
if libcontainercgroups.IsCgroup2UnifiedMode() {
// memory.swap.max does not exist in the cgroup root, so we check /sys/fs/cgroup/<SELF>/memory.swap.max
_, unified, err := cgroups.ParseCgroupFileUnified("/proc/self/cgroup")
if err != nil {
klog.V(5).ErrorS(fmt.Errorf("failed to parse /proc/self/cgroup: %w", err), warn)
return
}
p = filepath.Join("/sys/fs/cgroup", unified, "memory.swap.max")
}
if _, err := os.Stat(p); err != nil {
if !errors.Is(err, os.ErrNotExist) {
klog.V(5).ErrorS(err, warn)
}
return
}
swapControllerAvailability = true
})
return swapControllerAvailability
}

type swapConfigurationHelper struct {
machineInfo cadvisorv1.MachineInfo
}
Expand Down Expand Up @@ -337,10 +372,12 @@ func (m swapConfigurationHelper) ConfigureLimitedSwap(lcr *runtimeapi.LinuxConta

func (m swapConfigurationHelper) ConfigureNoSwap(lcr *runtimeapi.LinuxContainerResources) {
if !isCgroup2UnifiedMode() {
// memorySwapLimit = total permitted memory+swap; if equal to memory limit, => 0 swap above memory limit
// Some swapping is still possible.
// Note that if memory limit is 0, memory swap limit is ignored.
lcr.MemorySwapLimitInBytes = lcr.MemoryLimitInBytes
if swapControllerAvailable() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Question: Should this not be checked regardless of whether unified mode is enabled. Otherwise we explicitly set memory.swap.max even if swap is disabled?

For reference, I am having issues launching a >= 1.28.0 using kind on a node where swap is disabled. (I will spend some time creating an issue tomorrow).

cc @iholder101

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this fix a problem? I cannot catch up with you here.

This added a check for no-cgroup v2: to not set swap if swap controller is not available.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this fix is fine. I was asking whether this check should also be done on cgroup v2 systems. I have a PR #120784 under review to do this -- effectively moving the check higher up the call chain.

Feel free to review this as well.

// memorySwapLimit = total permitted memory+swap; if equal to memory limit, => 0 swap above memory limit
// Some swapping is still possible.
// Note that if memory limit is 0, memory swap limit is ignored.
lcr.MemorySwapLimitInBytes = lcr.MemoryLimitInBytes
}
return
}

Expand Down
2 changes: 2 additions & 0 deletions vendor/github.com/containerd/cgroups/.gitignore

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

24 changes: 24 additions & 0 deletions vendor/github.com/containerd/cgroups/Makefile

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

46 changes: 46 additions & 0 deletions vendor/github.com/containerd/cgroups/Protobuild.toml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

204 changes: 204 additions & 0 deletions vendor/github.com/containerd/cgroups/README.md

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.