add --system-reserved support for swap(cgroup v2 only): only for burstable pods #105271

pacoxu · 2021-09-27T10:30:27Z

/kind feature
/sig node
/triage accepted
/priority important-soon
/kind api-change

Pending on kubernetes/test-infra#24919 to test the e2e part of this PR.

kubelet: add the ability to set a the quantity of swap by `--system-reserved` flag on a node

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

- [KEP]: https://github.com/kubernetes/enhancements/tree/master/keps/sig-node/2400-node-swap

k8s-ci-robot · 2021-09-27T10:30:30Z

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

pacoxu · 2021-10-18T03:45:23Z

/cc ehashman kevindelgado @yxxhero
cc all who are working on Swap-related PRs.

k8s-ci-robot · 2021-10-18T03:45:26Z

@pacoxu: GitHub didn't allow me to request PR reviews from the following users: yxxhero.

Note that only kubernetes members and repo collaborators can review this PR, and authors cannot review their own PRs.

In response to this:

/cc ehashman kevindelgado yxxhero
cc all who are working on Swap-related PRs.

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.

pacoxu · 2021-10-18T03:47:59Z

Some testing on my cluster

[root@daocloud ~]# free -h
              total        used        free      shared  buff/cache   available
Mem:           15Gi       2.1Gi       9.7Gi       2.0Mi       3.6Gi        13Gi
Swap:         1.0Gi       148Mi       875Mi

[root@daocloud ~]# cat /var/lib/kubelet/config.yaml
...
failSwapOn: false
featureGates:
  NodeSwap: true
memorySwap:
  swapBehavior: LimitedSwap
systemReserved:
  cpu: 1000m
  memory: 1000Mi
  ephemeral-storage: 10Gi
  pid: "300"
  swap: "400Mi"
...

[root@daocloud ~]# kubectl get node -o yaml | grep allocatable -A 20
    allocatable:
      cpu: "7"
      ephemeral-storage: "83916597092"
      hugepages-1Gi: "0"
      hugepages-2Mi: "0"
      memory: 15107672Ki
      pods: "20"
      swap: 638972Ki
    capacity:
      cpu: "8"
      ephemeral-storage: 102706180Ki
      hugepages-1Gi: "0"
      hugepages-2Mi: "0"
      memory: 16234072Ki
      pods: "20"
      swap: 1048572Ki

iholder101

Thanks @pacoxu! Great Job!

Left some minor comments below

iholder101 · 2023-07-11T12:52:36Z

pkg/kubelet/cm/cgroup_manager_linux.go

@@ -260,6 +267,9 @@ func (m *cgroupManagerImpl) Validate(name CgroupName) error {
 	// in https://github.com/opencontainers/runc/issues/1440
 	// once resolved, we can remove this code.
 	allowlistControllers := sets.NewString("cpu", "cpuacct", "cpuset", "memory", "systemd", "pids")
+	if utilfeature.DefaultFeatureGate.Enabled(kubefeatures.NodeSwap) && swapControllerV2Available() {


IIUC, there is no "swap" controller, but there are swap knobs that are part of the memory controller.
AFAIK, all cgroups with the memory controller enabled are also exposed to swap knobs.

Please correct me if I'm wrong.

This function swapControllerV2Available is // swap controller is available only if cgroup v2 swap file is accessible..

So we may rename it to something like swapCgroupV2FileAvailable?

In which situations the swap knobs are missing with cgroups v2 and memory controller enabled?

IIUC the whole purpose here is to add swap to allowlistControllers. Is that necessary?

So it seems that no need to run swapCgroupV2FileAvailable here. I drop it.

pkg/kubelet/cm/container_manager_linux.go

pkg/apis/node/types.go

pacoxu · 2023-07-11T15:17:19Z

@iholder101 thanks for the review and updated accordingly.

k8s-ci-robot · 2023-07-11T15:17:39Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: mmiranda96, pacoxu
Once this PR has been reviewed and has the lgtm label, please assign liggitt for approval. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

cmd/kubelet/OWNERS
pkg/api/OWNERS
pkg/generated/openapi/OWNERS
pkg/kubelet/OWNERS
pkg/kubelet/apis/config/OWNERS
staging/src/k8s.io/api/OWNERS
staging/src/k8s.io/kubelet/config/OWNERS
test/e2e_node/OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Signed-off-by: Paco Xu <paco.xu@daocloud.io>

Signed-off-by: Paco Xu <paco.xu@daocloud.io> Co-authored-by: Elana Hashman <ehashman@users.noreply.github.com>

Signed-off-by: Paco Xu <paco.xu@daocloud.io> Co-authored-by: haoyun <yun.hao@daocloud.io>

Signed-off-by: Paco Xu <paco.xu@daocloud.io>

k8s-ci-robot · 2023-07-20T08:54:13Z

@pacoxu: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
pull-kubernetes-node-kubelet-serial	fb337bcda5e2198a87b543104294fce8305fdc35	link	false	`/test pull-kubernetes-node-kubelet-serial`
pull-kubernetes-node-swap-ubuntu	2a3bcf7e8a1c22c3bf9d424308f8331146603490	link	false	`/test pull-kubernetes-node-swap-ubuntu`
pull-kubernetes-node-swap-fedora-serial	556b74b3cd87a98e029c259f01c66fb7a64f31b4	link	false	`/test pull-kubernetes-node-swap-fedora-serial`

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

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. I understand the commands that are listed here.

k8s-ci-robot · 2023-10-20T23:25:50Z

PR needs rebase.

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.

k8s-triage-robot · 2024-02-04T07:17:26Z

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.

This bot triages PRs according to the following rules:

After 90d of inactivity, lifecycle/stale is applied
After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
After 30d of inactivity since lifecycle/rotten was applied, the PR is closed

You can:

Reopen this PR with /reopen
Mark this PR as fresh with /remove-lifecycle rotten
Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/close

k8s-ci-robot · 2024-02-04T07:17:31Z

@k8s-triage-robot: Closed this PR.

In response to this:

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.

This bot triages PRs according to the following rules:

After 90d of inactivity, lifecycle/stale is applied

After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied

After 30d of inactivity since lifecycle/rotten was applied, the PR is closed

You can:

Reopen this PR with /reopen

Mark this PR as fresh with /remove-lifecycle rotten

Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/close

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.

k8s-ci-robot requested review from bobbypage and derekwaynecarr September 27, 2021 10:31

k8s-ci-robot added the area/kubelet label Sep 27, 2021

k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Oct 15, 2021

pacoxu marked this pull request as ready for review October 15, 2021 09:26

k8s-ci-robot added the sig/apps Categorizes an issue or PR as relevant to SIG Apps. label Oct 15, 2021

pacoxu force-pushed the swap-beta-dev branch from 003fcc4 to 7aab907 Compare October 18, 2021 03:30

pacoxu changed the title ~~wip: add --system-reserved support for swap~~ add --system-reserved support for swap Oct 18, 2021

k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Oct 18, 2021

k8s-ci-robot requested review from ehashman and kevindelgado October 18, 2021 03:45

pacoxu marked this pull request as draft October 18, 2021 04:00

k8s-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Oct 18, 2021

pacoxu force-pushed the swap-beta-dev branch 2 times, most recently from 99da08f to d865da9 Compare October 18, 2021 07:32

pacoxu mentioned this pull request Jul 11, 2023

[WIP] [merge 105271 and 118764 into one]Swap/burstable qo s impl #119218

Closed

iholder101 reviewed Jul 11, 2023

View reviewed changes

pacoxu force-pushed the swap-beta-dev branch from 556b74b to 9dbdaa4 Compare July 11, 2023 15:16

pacoxu force-pushed the swap-beta-dev branch from 9dbdaa4 to ddedd3b Compare July 11, 2023 17:05

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 19, 2023

iholder101 mentioned this pull request Jul 19, 2023

[KEP-2400] Node memory swap support (replacement for inactive issue #2400) kubernetes/enhancements#4128

Closed

35 tasks

pacoxu and others added 5 commits July 20, 2023 15:09

kubelet: add --system-reserved support for swap

329ed16

Signed-off-by: Paco Xu <paco.xu@daocloud.io>

kubelet: swap support for system-reserve and allocatable calculation

61079bc

Signed-off-by: Paco Xu <paco.xu@daocloud.io> Co-authored-by: Elana Hashman <ehashman@users.noreply.github.com>

kubelet(swap): add e2e test for system-reserved swap

d8879db

Signed-off-by: Paco Xu <paco.xu@daocloud.io> Co-authored-by: haoyun <yun.hao@daocloud.io>

add parseCgroupFileUnified from containerd cgroup v1

fd8d31c

Signed-off-by: Paco Xu <paco.xu@daocloud.io>

drop swap support for cgroup v1

5009ef0

pacoxu force-pushed the swap-beta-dev branch from ddedd3b to aac67c5 Compare July 20, 2023 07:42

k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 20, 2023

pacoxu force-pushed the swap-beta-dev branch from aac67c5 to 79a71f6 Compare July 20, 2023 07:59

fix swap e2e test

b69dc5c

pacoxu force-pushed the swap-beta-dev branch from 79a71f6 to b69dc5c Compare July 20, 2023 09:07

iholder101 mentioned this pull request Jul 21, 2023

REQUEST: New membership for iholder101 kubernetes/org#4342

Closed

31 tasks

iholder101 mentioned this pull request Sep 21, 2023

[KEP-2400] [Swap] Add the ability to set a system-reserved quantity of swap on a node #120802

Open

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 20, 2023

dims added the lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. label Oct 24, 2023

k8s-ci-robot closed this Feb 4, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add --system-reserved support for swap(cgroup v2 only): only for burstable pods #105271

add --system-reserved support for swap(cgroup v2 only): only for burstable pods #105271

pacoxu commented Sep 27, 2021 •

edited

Loading

k8s-ci-robot commented Sep 27, 2021

pacoxu commented Oct 18, 2021 •

edited

Loading

k8s-ci-robot commented Oct 18, 2021

pacoxu commented Oct 18, 2021

iholder101 left a comment

iholder101 Jul 11, 2023

pacoxu Jul 11, 2023

iholder101 Jul 11, 2023

pacoxu Jul 11, 2023

pacoxu commented Jul 11, 2023

k8s-ci-robot commented Jul 11, 2023

k8s-ci-robot commented Jul 20, 2023 •

edited

Loading

k8s-ci-robot commented Oct 20, 2023

k8s-triage-robot commented Feb 4, 2024

k8s-ci-robot commented Feb 4, 2024

add --system-reserved support for swap(cgroup v2 only): only for burstable pods #105271

add --system-reserved support for swap(cgroup v2 only): only for burstable pods #105271

Conversation

pacoxu commented Sep 27, 2021 • edited Loading

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

k8s-ci-robot commented Sep 27, 2021

pacoxu commented Oct 18, 2021 • edited Loading

k8s-ci-robot commented Oct 18, 2021

pacoxu commented Oct 18, 2021

iholder101 left a comment

Choose a reason for hiding this comment

iholder101 Jul 11, 2023

Choose a reason for hiding this comment

pacoxu Jul 11, 2023

Choose a reason for hiding this comment

iholder101 Jul 11, 2023

Choose a reason for hiding this comment

pacoxu Jul 11, 2023

Choose a reason for hiding this comment

pacoxu commented Jul 11, 2023

k8s-ci-robot commented Jul 11, 2023

k8s-ci-robot commented Jul 20, 2023 • edited Loading

k8s-ci-robot commented Oct 20, 2023

k8s-triage-robot commented Feb 4, 2024

k8s-ci-robot commented Feb 4, 2024

pacoxu commented Sep 27, 2021 •

edited

Loading

pacoxu commented Oct 18, 2021 •

edited

Loading

k8s-ci-robot commented Jul 20, 2023 •

edited

Loading