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.

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

KEP: Topology-aware service routing #640

Merged
merged 1 commit into from
Dec 8, 2018
Merged

KEP: Topology-aware service routing #640

merged 1 commit into from
Dec 8, 2018

Conversation

m1093782566
Copy link

@m1093782566 m1093782566 commented Dec 3, 2018 •
edited
Loading

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Dec 3, 2018
@k8s-ci-robot k8s-ci-robot added kind/kep Categorizes KEP tracking issues and PRs modifying the KEP directory sig/architecture Categorizes an issue or PR as relevant to SIG Architecture. sig/network Categorizes an issue or PR as relevant to SIG Network. sig/pm labels Dec 3, 2018
@m1093782566 m1093782566 mentioned this pull request Dec 3, 2018
Closed
@m1093782566
Copy link
Author

/assign @jbeda

@wojtek-t wojtek-t self-assigned this Dec 3, 2018
@johnbelamaric
Copy link
Member

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Dec 3, 2018
@m1093782566
Copy link
Author

@kubernetes/sig-architecture-api-reviews

@k8s-ci-robot k8s-ci-robot added the kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API label Dec 4, 2018
idvoretskyi
idvoretskyi approved these changes Dec 5, 2018
View reviewed changes
Copy link
Member

@idvoretskyi idvoretskyi left a comment

Choose a reason for hiding this comment

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

/lgtm

/hold

@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 Dec 5, 2018
@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Dec 5, 2018
@m1093782566
Copy link
Author

@thockin

Can you confirm that if we can merge this PR? I think we all defer to you.

thockin
thockin reviewed Dec 7, 2018
View reviewed changes
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.

/lgtm
/approve

We will need to revisit the use of PodLocator in kube-proxy before going beta.

We need to figure out if PodLocator is a CRD or a built-in. I think we should do CRD. @saad-ali can offer guidance on linked-in CRDs.

As for apigroup, let's start with topology.k8s.io but I doubt that is going to last past alpha. We'll have to discuss more broadly once we know it works well.

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: idvoretskyi, johnbelamaric, m1093782566, thockin

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

@m1093782566
Copy link
Author

@thockin Okay, and thanks for your patient guidance.

@idvoretskyi Would you please cancel the hold now? :)

@justaugustus
Copy link
Member

/hold cancel

@k8s-ci-robot k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Dec 8, 2018
@k8s-ci-robot k8s-ci-robot merged commit 588e217 into kubernetes:master Dec 8, 2018
This was referenced Dec 14, 2018
Merged
Closed
@m1093782566 m1093782566 mentioned this pull request Jan 7, 2019
Closed
1 task
@cmluciano cmluciano mentioned this pull request Jan 8, 2019
Closed
@Yiming-Jia
Copy link

@m1093782566 Hi Jun, do you know which version of Kubernetes will include this change? We'd like to have this topology routing feature in our service.

@m1093782566
Copy link
Author

I think we target v1.15. I am working on it and hopefully will raise a PR in one or two weeks.

@krmayankk
Copy link

@m1093782566 do you need help with any portions of it . If you want to divide this into small chunks let me know and I am happy to help

@m1093782566
Copy link
Author

Thanks @krmayankk

I will let you know once I need your help.

@Yiming-Jia
Copy link

@m1093782566 Hey Jun, do you know the release schedule for 1.15? Because this feature exactly solve a problem we're facing. We'd like to keep following the release schedule for this feature.

@justinsb justinsb mentioned this pull request Apr 7, 2019
Closed
@m1093782566
Copy link
Author

@thockin @johnbelamaric @krmayankk We have just finished the implementation in PR: kubernetes/kubernetes#72046, I think we need your review comments there. Thanks!

@johnbelamaric johnbelamaric mentioned this pull request Aug 8, 2019
Closed
jdef
jdef reviewed Nov 15, 2019
View reviewed changes
keps/sig-network/0033-service-topology.md
// topologyKeys is a preference-order list of topology keys. If backends exist for
// index [0], they will always be chosen; only if no backends exist for index [0] will backends for index [1] be considered.
// If this field is specified and all indices have no backends, the service has no backends, and connections will fail. We say these requirements are hard.
// In order to experss soft requirement, we may give a special node label key "" as it means "match all nodes".
Copy link

Choose a reason for hiding this comment

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

s/experss/express/

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

@jdef jdef jdef left review comments

@thockin thockin thockin left review comments

@idvoretskyi idvoretskyi idvoretskyi approved these changes

@justaugustus justaugustus Awaiting requested review from justaugustus

Assignees

@jbeda jbeda

@idvoretskyi idvoretskyi

@thockin thockin

@johnbelamaric johnbelamaric

@wojtek-t wojtek-t

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/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API 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/architecture Categorizes an issue or PR as relevant to SIG Architecture. 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
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.