Skip to content

Nodes swap memory fix 2 #92421

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

mburke5678
Copy link
Contributor

@mburke5678 mburke5678 commented Apr 21, 2025

The docs about enabling swap memory are incorrect that cgroup v2 clusters cannot use limited swap memory. It should be cgroup v1 cluster cannot use limited swap.

Also:

See the current docs: Enabling swap memory use on nodes

4.17+

Preview: Allowing swap memory use on nodes:

  • Slight re-write of title for clarity
  • Added intro paragraph
  • Moved one of the (ick) stacked admonitions to the end of the section, right before the prereqs
  • Reworked the three paragraphs following the (ick) stacked admonitions
  • Added new prereq bullet 2.
  • Removed (former) prereq bullet 4 (after the NOTE), as it is the same as Step 3, I think, and should come after the procedure.
  • Procedure Step 2, call-out two, reworded, added NoSwap.
  • Procedure Step 3, reworded.

QE review:

  • QE has approved this change.

Related to: #92409

@openshift-ci openshift-ci bot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Apr 21, 2025
@ocpdocs-previewbot
Copy link

ocpdocs-previewbot commented Apr 21, 2025

🤖 Fri Apr 25 21:32:45 - Prow CI generated the docs preview:

https://92421--ocpdocs-pr.netlify.app/openshift-enterprise/latest/nodes/nodes/nodes-nodes-managing.html


. Enable swap memory on the machines.
. Enable swap memory on the nodesS by setting the `swapaccount=1` kernel argument and configure swap memory as needed.

Choose a reason for hiding this comment

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

Suggested change
. Enable swap memory on the nodesS by setting the `swapaccount=1` kernel argument and configure swap memory as needed.
. Enable swap memory on the nodes by setting the `swapaccount=1` kernel argument and configure swap memory as needed.


* Unlimited: Use the `UnlimitedSwap` value to allow workloads to use as much swap memory as they request, up to the system limit.
Regardless of the `swapBehavior` setting, any workloads that are not managed by {product-title} on that node can still use swap memory if the `failSwapOn` parameter is `false`.

Choose a reason for hiding this comment

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

So I'm not entirely sure what this means.

It wouldn't be possible to boot openshift if failSwapOn is true.

Any thing on the node could use swap but kubelet would not start if swap is enabled on the node.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@kannon92 What I was getting at is that when you set failSwapOn: false, whether you set swapBehavior: NoSwap or swapBehavior: LimitedSwap, and non-OCP workloads can use swap.
That was how I read the Kubernetes docs you suggested. (They have a nice example in system daemons, that perhaps I could add.)

@asahay19
Copy link

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Apr 24, 2025
@mburke5678 mburke5678 force-pushed the nodes-swap-memory-fix-2 branch from 6053e6b to b5fc3c9 Compare April 25, 2025 21:20
@openshift-ci openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Apr 25, 2025
Copy link

openshift-ci bot commented Apr 25, 2025

New changes are detected. LGTM label has been removed.

@mburke5678 mburke5678 added this to the Continuous Release milestone Apr 25, 2025
@mburke5678 mburke5678 added the peer-review-needed Signifies that the peer review team needs to review this PR label Apr 25, 2025
Copy link

openshift-ci bot commented Apr 25, 2025

@mburke5678: all tests passed!

Full PR test history. Your PR dashboard.

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

@GroceryBoyJr
Copy link
Contributor

/label peer-review-in-progress
/remove-label peer-review-needed
/assign GroceryBoyJr

@openshift-ci openshift-ci bot added peer-review-in-progress Signifies that the peer review team is reviewing this PR and removed peer-review-needed Signifies that the peer review team needs to review this PR labels Apr 26, 2025
Copy link
Contributor

@GroceryBoyJr GroceryBoyJr left a comment

Choose a reason for hiding this comment

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

LGTM

@GroceryBoyJr
Copy link
Contributor

/label peer-review-done
/remove-label peer-review-in-progress
/unassign GroceryBoyJr

@openshift-ci openshift-ci bot added peer-review-done Signifies that the peer review team has reviewed this PR and removed peer-review-in-progress Signifies that the peer review team is reviewing this PR labels Apr 26, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
branch/enterprise-4.17 branch/enterprise-4.18 branch/enterprise-4.19 peer-review-done Signifies that the peer review team has reviewed this PR size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants