Skip to content
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

KEP-2433: Graduate Topology Aware Hints with only the Hints field to GA #5151

Merged
merged 2 commits into from
Feb 11, 2025

Conversation

gauravkghildiyal
Copy link
Member

  • One-line PR description: Update KEP-2433 to indicate that only the Hints field is planned to graduate to GA as part of this KEP

  • Issue link: Topology Aware Routing #2433

  • Other comments:
    • I've been scratching my head to find the right balance between updating the KEP to reflect only the Hints graduating while still maintaining all the valuable information present here. I didn't find that perfect balance and have started out with adding an explanatory note towards the beginning and some minor updates elsewhere. Happy to iterate on any feedback received.

/cc @robscott
/assign @aojea @danwinship @thockin

@k8s-ci-robot k8s-ci-robot requested a review from robscott February 7, 2025 04:14
@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/kep Categorizes KEP tracking issues and PRs modifying the KEP directory sig/network Categorizes an issue or PR as relevant to SIG Network. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Feb 7, 2025
@gauravkghildiyal
Copy link
Member Author

/assign @johnbelamaric (For PRR)

Copy link
Member

@thockin thockin left a comment

Choose a reason for hiding this comment

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

Thanks!

/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 Feb 7, 2025
@wojtek-t wojtek-t self-assigned this Feb 10, 2025
@@ -3,3 +3,5 @@ alpha:
approver: "@wojtek-t"
beta:
approver: "@wojtek-t"
stable:
approver: "@wojtek-t" # TODO(gauravkghildiyal): Change if someone else is reviewing.
Copy link
Member

Choose a reason for hiding this comment

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

I was OOO for last 3 weeks, but taking a look now.

Copy link
Member

Choose a reason for hiding this comment

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

Few minor comments, as I can't comment on unchanged lines

  1. "Were upgrade and rollback tested? Was the upgrade->downgrade->upgrade path tested?"

It's been 3+ years in beta already so it's pretty late, but were some manual tests run? In any case, can you please update this subsection?

  1. I'm assuming that release team will ask you to update to the new KEP template, which in addition contains one new question in scalability section.

Other than that, this lgtm.

Copy link
Member Author

@gauravkghildiyal gauravkghildiyal Feb 11, 2025

Choose a reason for hiding this comment

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

I was OOO for last 3 weeks, but taking a look now.

Welcome back :)

I've addressed the comments. I checked the latest PRR and have included the additional questions which were missing.

@k8s-ci-robot k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Feb 11, 2025
@wojtek-t
Copy link
Member

/lgtm
/approve PRR

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

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: gauravkghildiyal, thockin, wojtek-t

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

The pull request process is described 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

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Feb 11, 2025
@k8s-ci-robot k8s-ci-robot merged commit 537f858 into kubernetes:master Feb 11, 2025
4 checks passed
@k8s-ci-robot k8s-ci-robot added this to the v1.33 milestone Feb 11, 2025
@dipesh-rawat dipesh-rawat mentioned this pull request Feb 11, 2025
19 tasks
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. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. 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/network Categorizes an issue or PR as relevant to SIG Network. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants