Skip to content

Conversation

ngopalak-redhat
Copy link
Contributor

@ngopalak-redhat ngopalak-redhat commented Sep 23, 2025

Setting the NodeSwap feature gate to false blocks the use of memorySwap -> swapDesired option in the KubeletConfig. With 1.34 the NodeSwap feature gate is always set to true. Hence this PR sets the NodeSwap feature gate to true for all the types of clusters.

Tested by removing the Flag completely:

Worker nodes:
  - failSwapOn: false
  - swapBehavior: NoSwap

  Control plane nodes:
  - failSwapOn: true
  - swapBehavior: NoSwap

The cluster was created with:

launch 4.21,openshift/machine-config-operator#5294,openshift/api#2494 gcp

This PR needs to be merged before openshift/machine-config-operator#5294.

This PR is tested along with openshift/machine-config-operator#5294
Fixes: https://issues.redhat.com/browse/OCPBUGS-62068

Installer using NodeSwap Feature Gate: https://github.com/openshift/installer/blob/main/pkg/types/defaults/validation/featuregates.go#L39

  • Since the default from upstream will be used it won't be impacted.
  • Also swapBehavior is controls the use of swap

Copy link
Contributor

openshift-ci bot commented Sep 23, 2025

Hello @ngopalak-redhat! Some important instructions when contributing to openshift/api:
API design plays an important part in the user experience of OpenShift and as such API PRs are subject to a high level of scrutiny to ensure they follow our best practices. If you haven't already done so, please review the OpenShift API Conventions and ensure that your proposed changes are compliant. Following these conventions will help expedite the api review process for your PR.

@openshift-ci openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Sep 23, 2025
Copy link
Contributor

openshift-ci bot commented Sep 23, 2025

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

@openshift-ci openshift-ci bot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Sep 23, 2025
@ngopalak-redhat ngopalak-redhat changed the title WIP: Draft: Disable Swap mode WIP: Draft: NodeSwap Feature to be set to true always Sep 23, 2025
@ngopalak-redhat ngopalak-redhat changed the title WIP: Draft: NodeSwap Feature to be set to true always OCPBUGS-62068: WIP: Draft: NodeSwap Feature to be set to true always Sep 23, 2025
@openshift-ci-robot openshift-ci-robot added jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. jira/valid-bug Indicates that a referenced Jira bug is valid for the branch this PR is targeting. labels Sep 23, 2025
@openshift-ci-robot
Copy link

@ngopalak-redhat: This pull request references Jira Issue OCPBUGS-62068, which is valid. The bug has been moved to the POST state.

3 validation(s) were run on this bug
  • bug is open, matching expected state (open)
  • bug target version (4.21.0) matches configured target version for branch (4.21.0)
  • bug is in the state New, which is one of the valid states (NEW, ASSIGNED, POST)

Requesting review from QA contact:
/cc @mrniranjan

The bug has been updated to refer to the pull request using the external bug tracker.

In response to this:

Setting the NodeSwap feature gate to false blocks the use of memorySwap -> swapDesired option in the KubeletConfig. With 1.34 the NodeSwap feature gate is always set to true. Hence this PR sets the NodeSwap feature gate to true for all the types of clusters.

This PR is tested along with openshift/machine-config-operator#5294

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 openshift-eng/jira-lifecycle-plugin repository.

@openshift-ci openshift-ci bot requested a review from mrniranjan September 23, 2025 14:39
@openshift-ci-robot
Copy link

@ngopalak-redhat: This pull request references Jira Issue OCPBUGS-62068, which is valid.

3 validation(s) were run on this bug
  • bug is open, matching expected state (open)
  • bug target version (4.21.0) matches configured target version for branch (4.21.0)
  • bug is in the state POST, which is one of the valid states (NEW, ASSIGNED, POST)

Requesting review from QA contact:
/cc @mrniranjan

In response to this:

Setting the NodeSwap feature gate to false blocks the use of memorySwap -> swapDesired option in the KubeletConfig. With 1.34 the NodeSwap feature gate is always set to true. Hence this PR sets the NodeSwap feature gate to true for all the types of clusters.

This PR is tested along with openshift/machine-config-operator#5294
Fixes: https://issues.redhat.com/browse/OCPBUGS-62068

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 openshift-eng/jira-lifecycle-plugin repository.

Comment on lines 95 to 96
// NodeSwap is now GA and locked to true in Kubernetes 1.34+
// Must be enabled to align with upstream Kubernetes behavior.
Copy link
Member

Choose a reason for hiding this comment

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

I don't think this comment is necessary anymore

Comment on lines 97 to 102
FeatureGateNodeSwap = newFeatureGate("NodeSwap").
reportProblemsToJiraComponent("node").
contactPerson("haircommander").
productScope(kubernetes).
enhancementPR("https://github.com/kubernetes/enhancements/issues/2400").
enableIn(configv1.DevPreviewNoUpgrade, configv1.TechPreviewNoUpgrade).
enableIn(configv1.Default, configv1.DevPreviewNoUpgrade, configv1.TechPreviewNoUpgrade).
Copy link
Member

Choose a reason for hiding this comment

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

@JoelSpeed do we still need to keep this standza if we're just going to follow a GA feature upstream? as in: should we even have this in the feature gates openshift is aware of anymore?

Copy link
Contributor

Choose a reason for hiding this comment

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

You have a choice. You can enable it in default now (1.33), or leave it to the promotion.

If you leave it to the promotion, you need to remove the gate now so that we fall back to the built-in behaviour, rather than being dictated by the feature gate opinions here.

What is the default upstream feature state in 1.33?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

Do we have any testing to show what this feature is doing when it is enabled? Any E2E? Looks like nothing is showing up in sippy tagged by this gate, but perhaps there's some upstream tests?

Copy link
Member

Choose a reason for hiding this comment

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

I think we only want it in 1.34/4.21 and onward, and I'm inclined to just leave it to promotion as it's not only on by default but GA (and there's another toggle in the kubelet config to actually use it)

Copy link
Member

Choose a reason for hiding this comment

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

we won't have any e2e testing it downstream I believe because we're only using it (kubeletconfig swapMode: limitedSwap) in CNV environments, otherwise we'll block users from setting it (see openshift/machine-config-operator#5294)

Copy link
Contributor Author

@ngopalak-redhat ngopalak-redhat Sep 24, 2025

Choose a reason for hiding this comment

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

@JoelSpeed @haircommander Here's are the steps as I understand for the removal of the NodeSwap gate:

  1. Ensure the NodeSwap gate is not used anywhere in openshift code and docs. All the references from this search https://github.com/search?q=org%3Aopenshift%20NodeSwap&type=code have to be addressed for removal.
  2. Remove it from the NodeSwap gate in this repo after all the references are removed.
    This will allow the promotion to happen when 1.34 merges into openshift. Right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have removed the Flag and tested. Results are good and updated in the description

@JoelSpeed
Copy link
Contributor

/test verify-feature-promotion

@openshift-ci openshift-ci bot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Sep 24, 2025
@ngopalak-redhat
Copy link
Contributor Author

/test verify-feature-promotion

@ngopalak-redhat ngopalak-redhat changed the title OCPBUGS-62068: WIP: Draft: NodeSwap Feature to be set to true always OCPBUGS-62068: NodeSwap Feature Gate usage to be removed Sep 24, 2025
@ngopalak-redhat ngopalak-redhat marked this pull request as ready for review September 24, 2025 05:13
@openshift-ci openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Sep 24, 2025
@JoelSpeed
Copy link
Contributor

/lgtm

Assuming CI works, this should be good

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Sep 24, 2025
@openshift-ci-robot
Copy link

@ngopalak-redhat: This pull request references Jira Issue OCPBUGS-62068, which is valid.

3 validation(s) were run on this bug
  • bug is open, matching expected state (open)
  • bug target version (4.21.0) matches configured target version for branch (4.21.0)
  • bug is in the state POST, which is one of the valid states (NEW, ASSIGNED, POST)

Requesting review from QA contact:
/cc @mrniranjan

The bug has been updated to refer to the pull request using the external bug tracker.

In response to this:

/jira refresh

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 openshift-eng/jira-lifecycle-plugin repository.

@ngopalak-redhat
Copy link
Contributor Author

/test remaining-required

Copy link
Contributor

openshift-ci bot commented Sep 24, 2025

@ngopalak-redhat: The specified target(s) for /test were not found.
The following commands are available to trigger required jobs:

/test build
/test e2e-aws-ovn
/test e2e-aws-ovn-hypershift
/test e2e-aws-ovn-hypershift-conformance
/test e2e-aws-ovn-techpreview
/test e2e-aws-serial-1of2
/test e2e-aws-serial-2of2
/test e2e-aws-serial-techpreview-1of2
/test e2e-aws-serial-techpreview-2of2
/test e2e-azure
/test e2e-gcp
/test e2e-upgrade
/test e2e-upgrade-out-of-change
/test images
/test integration
/test lint
/test minor-e2e-upgrade-minor
/test minor-images
/test okd-scos-images
/test unit
/test verify
/test verify-client-go
/test verify-crd-schema
/test verify-crdify
/test verify-deps
/test verify-feature-promotion

The following commands are available to trigger optional jobs:

/test okd-scos-e2e-aws-ovn

Use /test all to run the following jobs that were automatically triggered:

pull-ci-openshift-api-master-build
pull-ci-openshift-api-master-images
pull-ci-openshift-api-master-integration
pull-ci-openshift-api-master-lint
pull-ci-openshift-api-master-minor-e2e-upgrade-minor
pull-ci-openshift-api-master-minor-images
pull-ci-openshift-api-master-okd-scos-e2e-aws-ovn
pull-ci-openshift-api-master-okd-scos-images
pull-ci-openshift-api-master-unit
pull-ci-openshift-api-master-verify
pull-ci-openshift-api-master-verify-client-go
pull-ci-openshift-api-master-verify-crd-schema
pull-ci-openshift-api-master-verify-crdify
pull-ci-openshift-api-master-verify-deps
pull-ci-openshift-api-master-verify-feature-promotion

In response to this:

/test remaining-required

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.

@ngopalak-redhat
Copy link
Contributor Author

/test e2e-aws-ovn
/test e2e-aws-ovn-hypershift
/test e2e-aws-ovn-hypershift-conformance
/test e2e-aws-ovn-techpreview
/test e2e-aws-serial-1of2
/test e2e-aws-serial-2of2

@haircommander
Copy link
Member

/retest-required

4 similar comments
@ngopalak-redhat
Copy link
Contributor Author

/retest-required

@jacobsee
Copy link
Member

/retest-required

@ngopalak-redhat
Copy link
Contributor Author

/retest-required

@ngopalak-redhat
Copy link
Contributor Author

/retest-required

@ngopalak-redhat
Copy link
Contributor Author

/verified later @BhargaviGudi

@openshift-ci-robot openshift-ci-robot added verified-later verified Signifies that the PR passed pre-merge verification criteria labels Sep 29, 2025
@openshift-ci-robot
Copy link

@ngopalak-redhat: This PR has been marked to be verified later by @BhargaviGudi.

In response to this:

/verified later @BhargaviGudi

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 openshift-eng/jira-lifecycle-plugin repository.

@openshift-ci-robot
Copy link

/retest-required

Remaining retests: 0 against base HEAD 1517fca and 2 for PR HEAD cb33f9f in total

@ngopalak-redhat
Copy link
Contributor Author

/retest-required

@ngopalak-redhat
Copy link
Contributor Author

/test e2e-aws-ovn-techpreview

4 similar comments
@ngopalak-redhat
Copy link
Contributor Author

/test e2e-aws-ovn-techpreview

@ngopalak-redhat
Copy link
Contributor Author

/test e2e-aws-ovn-techpreview

@ngopalak-redhat
Copy link
Contributor Author

/test e2e-aws-ovn-techpreview

@ngopalak-redhat
Copy link
Contributor Author

/test e2e-aws-ovn-techpreview

@ngopalak-redhat
Copy link
Contributor Author

/retest-required

@ngopalak-redhat
Copy link
Contributor Author

/test e2e-aws-ovn-techpreview

1 similar comment
@ngopalak-redhat
Copy link
Contributor Author

/test e2e-aws-ovn-techpreview

@haircommander
Copy link
Member

/retest

@ngopalak-redhat
Copy link
Contributor Author

/retest-required

2 similar comments
@ngopalak-redhat
Copy link
Contributor Author

/retest-required

@ngopalak-redhat
Copy link
Contributor Author

/retest-required

Copy link
Contributor

openshift-ci bot commented Sep 30, 2025

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

Test name Commit Details Required Rerun command
ci/prow/okd-scos-e2e-aws-ovn cb33f9f link false /test okd-scos-e2e-aws-ovn

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.

@openshift-merge-bot openshift-merge-bot bot merged commit 7f24529 into openshift:master Sep 30, 2025
27 of 28 checks passed
@openshift-ci-robot
Copy link

@ngopalak-redhat: Jira Issue OCPBUGS-62068: All pull requests linked via external trackers have merged:

This pull request has the verified-later tag and will need to be manually moved to VERIFIED after testing. Jira Issue OCPBUGS-62068 has been moved to the MODIFIED state.

In response to this:

Setting the NodeSwap feature gate to false blocks the use of memorySwap -> swapDesired option in the KubeletConfig. With 1.34 the NodeSwap feature gate is always set to true. Hence this PR sets the NodeSwap feature gate to true for all the types of clusters.

Tested by removing the Flag completely:

Worker nodes:
 - failSwapOn: false
 - swapBehavior: NoSwap

 Control plane nodes:
 - failSwapOn: true
 - swapBehavior: NoSwap

The cluster was created with:

launch 4.21,openshift/machine-config-operator#5294,openshift/api#2494 gcp

This PR needs to be merged before openshift/machine-config-operator#5294.

This PR is tested along with openshift/machine-config-operator#5294
Fixes: https://issues.redhat.com/browse/OCPBUGS-62068

Installer using NodeSwap Feature Gate: https://github.com/openshift/installer/blob/main/pkg/types/defaults/validation/featuregates.go#L39

  • Since the default from upstream will be used it won't be impacted.
  • Also swapBehavior is controls the use of swap

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 openshift-eng/jira-lifecycle-plugin repository.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. jira/valid-bug Indicates that a referenced Jira bug is valid for the branch this PR is targeting. jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. lgtm Indicates that a PR is ready to be merged. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. verified Signifies that the PR passed pre-merge verification criteria verified-later
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants