Skip to content

Clear pod.Status.NominatedNodeName when pod is bound #132443

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: master
Choose a base branch
from

Conversation

utam0k
Copy link
Member

@utam0k utam0k commented Jun 22, 2025

What type of PR is this?

/kind feature
/sig scheduling
/cc sanposhiho

What this PR does / why we need it:

This PR implements the kube-apiserver side of KEP-5278 by clearing the NominatedNodeName field when a pod is successfully bound to a node through the binding API.

Previously, only the scheduler cleared NominatedNodeName, but with KEP-5278, external components like Cluster Autoscaler and Karpenter can also set this field. To prevent stale information from affecting scheduling decisions, the apiserver now clears this field upon successful binding, regardless of which component performed the scheduling.

Which issue(s) this PR is related to:

Fixes #132385

Special notes for your reviewer:

Does this PR introduce a user-facing change?

Whenever a pod is successfully bound to a node, the kube-apiserver now clears the pod's `nominatedNodeName` field. This prevents stale information from affecting external scheduling components.

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


@k8s-ci-robot k8s-ci-robot requested a review from sanposhiho June 22, 2025 12:39
@k8s-ci-robot k8s-ci-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. kind/feature Categorizes issue or PR as related to a new feature. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. sig/scheduling Categorizes an issue or PR as relevant to SIG Scheduling. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Jun 22, 2025
@k8s-ci-robot
Copy link
Contributor

This issue is currently awaiting triage.

If a SIG or subproject determines this is a relevant issue, they will accept it by applying the triage/accepted label and provide further guidance.

The triage/accepted label can be added by org members by writing /triage accepted in a comment.

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.

@k8s-ci-robot k8s-ci-robot added needs-priority Indicates a PR lacks a `priority/foo` label and requires one. sig/node Categorizes an issue or PR as relevant to SIG Node. labels Jun 22, 2025
@lmktfy
Copy link

lmktfy commented Jun 22, 2025

Changelog suggestion

-When a pod is successfully bound to a node, the kube-apiserver now clears the pod's NominatedNodeName field to prevent stale information from affecting external scheduling components.
+Whenever a pod is successfully bound to a node, the kube-apiserver now clears the pod's `nominatedNodeName` field. This prevents stale information from affecting external scheduling components.

@utam0k
Copy link
Member Author

utam0k commented Jun 23, 2025

@imikushin Thanks for your review. Updated.

Copy link
Member

@sanposhiho sanposhiho left a comment

Choose a reason for hiding this comment

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

We (i.e., including me in other PRs) forgot feature-gating the logic!!! 😅

@sanposhiho
Copy link
Member

/cc @macsko @dom4ha

this is part of nnn kep

@k8s-ci-robot k8s-ci-robot requested review from dom4ha and macsko June 23, 2025 22:25
@utam0k utam0k force-pushed the clearn-nnn-bind-api branch from 58b4179 to 0d030b4 Compare June 24, 2025 12:25
@k8s-ci-robot k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Jun 24, 2025
@utam0k utam0k requested a review from sanposhiho June 24, 2025 12:46
@utam0k utam0k force-pushed the clearn-nnn-bind-api branch from 0d030b4 to 5ae0d94 Compare June 26, 2025 10:49
@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jun 26, 2025
@k8s-ci-robot
Copy link
Contributor

LGTM label has been added.

Git tree hash: 3bd65025f4333173de0b4a08d018278185a54910

@sanposhiho
Copy link
Member

/assign @dchen1107

for approval

@utam0k utam0k force-pushed the clearn-nnn-bind-api branch from d05459f to 3106b92 Compare June 28, 2025 02:18
@k8s-ci-robot k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jun 28, 2025
@k8s-ci-robot k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 28, 2025
@utam0k utam0k requested a review from macsko June 28, 2025 02:19
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: sanposhiho, utam0k
Once this PR has been reviewed and has the lgtm label, please ask for approval from dchen1107. For more information see the 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:

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

@utam0k utam0k force-pushed the clearn-nnn-bind-api branch from 3106b92 to 395d168 Compare June 28, 2025 02:42
@k8s-ci-robot k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 28, 2025
@utam0k utam0k force-pushed the clearn-nnn-bind-api branch 2 times, most recently from ff52974 to 603b4f9 Compare June 28, 2025 12:07
Copy link
Member

@sanposhiho sanposhiho left a comment

Choose a reason for hiding this comment

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

still
/lgtm
from sig-scheduling

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jun 29, 2025
@k8s-ci-robot
Copy link
Contributor

LGTM label has been added.

Git tree hash: d73c51eeab270fa51f422436dc477f9cf8f222c2

@@ -973,6 +973,12 @@
lockToDefault: false
preRelease: Beta
version: "1.30"
- name: NominatedNodeNameForExpectation
versionedSpecs:
- default: false
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 it should be enabled by default?

@@ -1379,6 +1389,10 @@ var defaultVersionedKubernetesFeatureGates = map[featuregate.Feature]featuregate
{Version: version.MustParse("1.30"), Default: true, PreRelease: featuregate.Beta},
},

NominatedNodeNameForExpectation: {
{Version: version.MustParse("1.34"), Default: false, PreRelease: featuregate.Beta},
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
{Version: version.MustParse("1.34"), Default: false, PreRelease: featuregate.Beta},
{Version: version.MustParse("1.34"), Default: true, PreRelease: featuregate.Beta},

Copy link
Member Author

Choose a reason for hiding this comment

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

Hi, @dom4ha 👋 Thanks for your review.
I believe that the default is set to true when approaching GA.
cc: @sanposhiho

Copy link
Member

Choose a reason for hiding this comment

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

During beta, the feature gate should be enabled by default, so the @dom4ha's comment is valid

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm adding a feature gate for the first time and don't quite understand the process. Thanks for letting me know 🙏

Copy link
Member

Choose a reason for hiding this comment

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

I'm adding a feature gate for the first time and don't quite understand the process. Thanks for letting me know 🙏

AFAIK there is no rule regarding the default value in beta, but it's described in the KEP.

@utam0k utam0k force-pushed the clearn-nnn-bind-api branch from 603b4f9 to 8192a1c Compare July 1, 2025 13:20
@k8s-ci-robot k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jul 1, 2025
@k8s-ci-robot k8s-ci-robot requested a review from sanposhiho July 1, 2025 13:20
@utam0k utam0k requested a review from dom4ha July 1, 2025 13:21
@k8s-ci-robot
Copy link
Contributor

@utam0k: 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
pull-kubernetes-integration 8192a1c link true /test pull-kubernetes-integration

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

@dom4ha
Copy link
Member

dom4ha commented Jul 1, 2025

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jul 1, 2025
@k8s-ci-robot
Copy link
Contributor

LGTM label has been added.

Git tree hash: 19e0bf7e631766fe1c7768ff76f1ff54037ea53f

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/test cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/feature Categorizes issue or PR as related to a new feature. lgtm "Looks good to me", indicates that a PR is ready to be merged. needs-priority Indicates a PR lacks a `priority/foo` label and requires one. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. release-note Denotes a PR that will be considered when it comes time to generate release notes. sig/node Categorizes an issue or PR as relevant to SIG Node. sig/scheduling Categorizes an issue or PR as relevant to SIG Scheduling. sig/testing Categorizes an issue or PR as relevant to SIG Testing. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Development

Successfully merging this pull request may close these issues.

KEP-5278: kube-apiserver clears NNN at the Pod's bind API
7 participants