Skip to content

KEP-4742: promote PodTopologyLabelAdmission to beta #5428

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

KEP-4742: promote PodTopologyLabelAdmission to beta #5428

wants to merge 1 commit into from
+9 −5

Conversation

andrewsykim
Copy link
Member

@andrewsykim andrewsykim commented Jun 20, 2025 •
edited
Loading

  • One-line PR description: promote PodTopologyLabelAdmission to beta
  • Other comments: Also fixed some lint issues

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Jun 20, 2025
@k8s-ci-robot k8s-ci-robot added kind/kep Categorizes KEP tracking issues and PRs modifying the KEP directory sig/node Categorizes an issue or PR as relevant to SIG Node. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Jun 20, 2025
@haircommander
Copy link
Contributor

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jun 20, 2025
@k8s-ci-robot k8s-ci-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed lgtm "Looks good to me", indicates that a PR is ready to be merged. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Jun 20, 2025
@haircommander
Copy link
Contributor

still
/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jun 20, 2025
SergeyKanzhelev
SergeyKanzhelev reviewed Jun 20, 2025
View reviewed changes
keps/sig-node/4742-node-topology-downward-api/README.md Outdated
@@ -402,6 +405,7 @@ Revert feature gate and stop consuming downward API.
## Implementation History

- `v1.33`: initial KEP is accepeted and alpha implementation is complete
- `v1.34`: fix topology labels from topology.k8s.io to topology.kubernetes.io. Promote feature gate to Beta.
Copy link
Member

Choose a reason for hiding this comment

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

this is the fix in the KEP docs, not in actual code, correct?

Copy link
Member Author

Choose a reason for hiding this comment

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

No, I believe it's in the actual code too.

Copy link
Member Author

Choose a reason for hiding this comment

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

cc @munnerz to confirm

Copy link
Member

Choose a reason for hiding this comment

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

Yes this does require a code change too. That was on me to open and I dropped the ball - I'll open a PR shortly and include the link here 😊

SergeyKanzhelev
SergeyKanzhelev approved these changes Jun 20, 2025
View reviewed changes
Copy link
Member

@SergeyKanzhelev SergeyKanzhelev left a comment

Choose a reason for hiding this comment

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

/lgtm

/assign @dchen1107

@SergeyKanzhelev SergeyKanzhelev mentioned this pull request Jun 20, 2025
Open
8 tasks
@munnerz munnerz mentioned this pull request Jun 23, 2025
Open
@munnerz
Copy link
Member

munnerz commented Jun 23, 2025

/hold

opened kubernetes/kubernetes#132462 to address incorrect implementation. Should this PR be reworked to not graduate the feature, and instead prioritise merging this pull request to fix the implementation to allow for meaningful feedback during the upcoming release?

@k8s-ci-robot k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jun 23, 2025
@andrewsykim
Copy link
Member Author

No objections from me to defer beta graduation to v1.35 to ensure we get some initial feedback using the proper labels

@dchen1107 dchen1107 added this to the v1.35 milestone Jun 23, 2025
@dchen1107
Copy link
Member

Moving this to 1.35 milestone based on the above conversation.

@SergeyKanzhelev
Copy link
Member

SergeyKanzhelev commented Jun 24, 2025 •
edited
Loading

We still need to track this, but without the graduation. Keeping in alpha and changing names of labels.

@andrewsykim can you update PR to only change names of labels and the history update, without the stage change?

@k8s-ci-robot k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jun 24, 2025
@andrewsykim
KEP-4742: fix topology labels
0d48147 
Signed-off-by: Andrew Sy Kim <[email protected]>
@andrewsykim
Copy link
Member Author

andrewsykim commented Jun 24, 2025 •
edited
Loading

can you update PR to only change names of labels and the history update, without the stage change

Done

SergeyKanzhelev
SergeyKanzhelev approved these changes Jun 24, 2025
View reviewed changes
Copy link
Member

@SergeyKanzhelev SergeyKanzhelev left a comment

Choose a reason for hiding this comment

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

/lgtm
/approve

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

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: andrewsykim, SergeyKanzhelev
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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@munnerz munnerz munnerz left review comments

@SergeyKanzhelev SergeyKanzhelev SergeyKanzhelev approved these changes

@dchen1107 dchen1107 Awaiting requested review from dchen1107

@derekwaynecarr derekwaynecarr Awaiting requested review from derekwaynecarr

Assignees

@dchen1107 dchen1107

@haircommander haircommander

@SergeyKanzhelev SergeyKanzhelev

Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. kind/kep Categorizes KEP tracking issues and PRs modifying the KEP directory lgtm "Looks good to me", indicates that a PR is ready to be merged. sig/node Categorizes an issue or PR as relevant to SIG Node. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Milestone
v1.35
Development

Successfully merging this pull request may close these issues.
6 participants
@andrewsykim @haircommander @munnerz @dchen1107 @SergeyKanzhelev @k8s-ci-robot