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-784: Update design-details #4337

Merged

Conversation

aroradaman
Copy link
Member

  • One-line PR description: Updated template with PRR Questionnaire
  • Other comments:

@k8s-ci-robot k8s-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. 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 labels Nov 17, 2023
@k8s-ci-robot k8s-ci-robot added sig/network Categorizes an issue or PR as relevant to SIG Network. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Nov 17, 2023
@aroradaman aroradaman force-pushed the kube-proxy-config-v1alpha2-design branch 2 times, most recently from c365eb8 to 256c437 Compare January 16, 2024 11:14
@aroradaman aroradaman changed the title [WIP] KEP-784: Update design-details KEP-784: Update design-details Jan 16, 2024
@k8s-ci-robot k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jan 16, 2024
| Field | Comments |
|---------|-----------------------------------------------------|
| Linux | new section for linux (platform-specific) options |
| Windows | new section for windows (platform-specific) options |
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 the plan is to get rid of windows directly, is not likely anybody is going to work on it

Copy link
Contributor

Choose a reason for hiding this comment

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

Well, we talked about this in SIG Network this week.

At any rate, having all the Windows-specific config in its own section(s) could be a step toward moving winkernel out of tree, if that eventually becomes the plan.

Copy link
Member

Choose a reason for hiding this comment

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

I thought we were going to do like with userspace proxy, just remove it completely

Copy link
Contributor

Choose a reason for hiding this comment

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

Someone has to maintain windows service proxying somewhere.

At any rate, my point was that if we eventually want the windows proxy code to be not-in-tree (whether that's because it's somewhere else, or because it no longer exists), then having all the windows-specific config all in one place wil lmake it easier to delete it later.

Copy link
Contributor

@danwinship danwinship left a comment

Choose a reason for hiding this comment

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

Maybe update the KEP title to something like "Kube Proxy component configuration updates and graduation"?

@aroradaman aroradaman force-pushed the kube-proxy-config-v1alpha2-design branch from 256c437 to d489b5a Compare January 21, 2024 19:26
@k8s-ci-robot k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Jan 21, 2024
@danwinship
Copy link
Contributor

/approve
with the above fix. I'll let @aojea lgtm if it lgth

@aroradaman aroradaman force-pushed the kube-proxy-config-v1alpha2-design branch from d489b5a to f79c15d Compare January 22, 2024 19:50
@danwinship
Copy link
Contributor

/lgtm

I'll let @aojea lgtm if it lgth

#4337 (comment) sounds like he's ok

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

@wojtek-t This is an old KEP that pre-dates PRRs so we need to add one belatedly now that we're getting back to it...

@wojtek-t
Copy link
Member

wojtek-t commented Feb 2, 2024

@wojtek-t This is an old KEP that pre-dates PRRs so we need to add one belatedly now that we're getting back to it...

Yeah - I remember this one. I'm happy to review this, but it needs to be filled in :)
@danwinship - you should probably also lead opt-in the corresponding issue for the release

@aroradaman - can you please fill in the PRR questions? It should be relatively simple (also for reference: https://github.com/kubernetes/enhancements/tree/master/keps/sig-scheduling/785-scheduler-component-config-api is somewhat similar for a different component).

@aroradaman aroradaman force-pushed the kube-proxy-config-v1alpha2-design branch from f79c15d to 2b2534f Compare February 4, 2024 05:54
@k8s-ci-robot k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Feb 4, 2024
@aroradaman aroradaman force-pushed the kube-proxy-config-v1alpha2-design branch 2 times, most recently from 89f83e4 to 1102e82 Compare February 5, 2024 06:03
@aroradaman aroradaman force-pushed the kube-proxy-config-v1alpha2-design branch 2 times, most recently from c5e16e7 to 188fe04 Compare February 5, 2024 18:53
Signed-off-by: Daman Arora <aroradaman@gmail.com>
Signed-off-by: Daman Arora <aroradaman@gmail.com>
@aroradaman aroradaman force-pushed the kube-proxy-config-v1alpha2-design branch from 188fe04 to c6895ad Compare February 5, 2024 18:58
@wojtek-t
Copy link
Member

wojtek-t commented Feb 6, 2024

/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 6, 2024
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: aroradaman, danwinship, 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 6, 2024
@k8s-ci-robot k8s-ci-robot merged commit 12cc497 into kubernetes:master Feb 6, 2024
4 checks passed
@k8s-ci-robot k8s-ci-robot added this to the v1.30 milestone Feb 6, 2024
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.

None yet

5 participants