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

openstack: Implement the Routes provider API #32663

Merged
merged 3 commits into from
Dec 8, 2016
Merged

openstack: Implement the Routes provider API #32663

merged 3 commits into from
Dec 8, 2016

Conversation

anguslees
Copy link
Member

@anguslees anguslees commented Sep 14, 2016 •
edited by k8s-oncall 


Implement the Routes provider API for OpenStack using Neutron extraroute extension.  This removes the need for flannel/etc where supported.  To use, ensure all your nodes are on the same Neutron (private) network and specify the router ID in new `[Route]` section of provider config:

    [Route]
    router-id = <router UUID>

This change is Reviewable

@k8s-github-robot k8s-github-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. release-note-label-needed labels Sep 14, 2016
@smarterclayton
Copy link
Contributor

Probably someone better than me to review this @kubernetes/sig-openstack

@errordeveloper
Copy link
Member

errordeveloper commented Sep 14, 2016 •
edited

In the light of kubernetes/enhancements#88, would it be already possible to implement this functionality as an add-on without making any changes to KCM? / cc @kubernetes/sig-cluster-lifecycle @kubernetes/sig-network

@errordeveloper
Copy link
Member

errordeveloper commented Sep 14, 2016 •
edited

@anguslees my previous comment doesn't relate to your work here. However, I can see you are proposing changes to cluster/, and I believe no new features are accepted for that particular directory - it's been deprecated. I you would like to help maintaining the Heat template, we can discuss that on the list. I don't know who maintains it right now.

@anguslees
Copy link
Member Author

@errordeveloper: I'm happy to drop the cluster/ changes from this PR, if you think we're organisationally unable to reach a conclusion on that portion.

@k8s-bot
Copy link

k8s-bot commented Sep 15, 2016

GCE e2e build/test passed for commit 824ed9dd8af3229ba72961d84c0ce7db8d662b4c.

@k8s-github-robot k8s-github-robot mentioned this pull request Sep 15, 2016
Closed
@k8s-github-robot k8s-github-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 16, 2016
@anguslees
Copy link
Member Author

@dagnello: can you review this?

@k8s-github-robot k8s-github-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 23, 2016
@k8s-ci-robot
Copy link
Contributor

Jenkins Kubemark GCE e2e failed for commit b85953cd48ff88b7244153f744c95fbe04c67ad7. Full PR test history.

The magic incantation to run this job again is @k8s-bot kubemark e2e test this. Please help us cut down flakes by linking to an open flake issue when you hit one in your PR.

@k8s-github-robot k8s-github-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 28, 2016
@k8s-github-robot k8s-github-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 5, 2016
@anguslees
Copy link
Member Author

@k8s-bot kubemark e2e test this

@k8s-github-robot k8s-github-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 8, 2016
@justinsb
Copy link
Member

justinsb commented Oct 8, 2016

I don't think making cloudprovider functionality optional should impact merging this. This looks like good code that adds a valuable feature for OpenStack users, and we don't want it to bitrot to the point where we lose it.

I believe the most-recent plan I've heard for kubernetes/enhancements#88 is to create a separate kcm per cloudprovider, and then just require a kcm-core and a kcm-openstack. Having a few more controllers is not going to be the tricky bit there, I don't think: getting the first one out is the hard one; the rest are then easy.

@anguslees
Copy link
Member Author

ping @dagnello for openstack review

@k8s-ci-robot
Copy link
Contributor

Jenkins GCE e2e failed for commit b674b0041a5fe10db5460c38c84c5934c7230a88. Full PR test history.

The magic incantation to run this job again is @k8s-bot cvm gce e2e test this. Please help us cut down flakes by linking to an open flake issue when you hit one in your PR.

@k8s-github-robot k8s-github-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 14, 2016
@fejta
Copy link
Contributor

fejta commented Oct 14, 2016

@k8s-bot cvm gce e2e test this

@k8s-github-robot k8s-github-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 14, 2016
@anguslees
Copy link
Member Author

@mikedanese for re-lgtm following bazel-induced rebase

@k8s-github-robot k8s-github-robot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Nov 9, 2016
@idvoretskyi idvoretskyi added the area/provider/openstack Issues or PRs related to openstack provider label Nov 16, 2016
@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Nov 30, 2016
@k8s-github-robot k8s-github-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 30, 2016
@anguslees
Copy link
Member Author

@mikedanese / @smarterclayton : quick, re-apply your lgtm label before another CI bot breaks!

@k8s-github-robot k8s-github-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Dec 4, 2016
@anguslees
Add gophercloud/.../layer3/routers
7a5aee2 
Import github.com/rackspace/gophercloud/openstack/networking/v2/extensions/layer3/routers/
@k8s-github-robot k8s-github-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Dec 5, 2016
@k8s-ci-robot
Copy link
Contributor

Jenkins GCE etcd3 e2e failed for commit ffaf8f710d6bf03628114dbe68fb894205be78d0. Full PR test history.

The magic incantation to run this job again is @k8s-bot gce etcd3 e2e test this. Please help us cut down flakes by linking to an open flake issue when you hit one in your PR.

@anguslees
Copy link
Member Author

@mikedanese / @smarterclayton: too late, the CI bots are all broken again :(

@anguslees
Copy link
Member Author

@k8s-bot gce etcd3 e2e test this
@k8s-bot verify test this
@k8s-bot kops aws e2e test this

@anguslees
openstack: Implement Routes API
0b57371 
This change implements the Routes API using Neutron's "extraroute"
extension.

To use, this requires all the nodes to be on the same Neutron network
and the UUID of the Neutron router on that network.

Required cloud provider config section:
  [Route]
  router-id = <UUID of Neutron router>

Ensure kube-controllermanager is started with (non-default)
`--allocate-node-cidrs=true` and set `--cluster-cidr` to the POD
super-subnet (a private /16 would be reasonable).

Based on an earlier version by @timbyr (kubernetes#19473)
@anguslees
openstack-heat: Drop flannel for cloud Routes API
29fadb3
@k8s-ci-robot
Copy link
Contributor

Jenkins verification failed for commit ffaf8f710d6bf03628114dbe68fb894205be78d0. Full PR test history.

The magic incantation to run this job again is @k8s-bot verify test this. Please help us cut down flakes by linking to an open flake issue when you hit one in your PR.

@anguslees
Copy link
Member Author

@k8s-bot kops aws e2e test this

@justinsb
Copy link
Member

justinsb commented Dec 5, 2016

@anguslees AWS kops bots are hitting rate limits so my understanding is that they are non-blocking until we work around / get higher limits.

@k8s-ci-robot
Copy link
Contributor

Jenkins kops AWS e2e failed for commit 29fadb3. Full PR test history.

The magic incantation to run this job again is @k8s-bot kops aws e2e test this. Please help us cut down flakes by linking to an open flake issue when you hit one in your PR.

@mikedanese mikedanese added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Dec 8, 2016
@k8s-github-robot
Copy link

@k8s-bot test this [submit-queue is verifying that this PR is safe to merge]

@k8s-github-robot
Copy link

Automatic merge from submit-queue

@k8s-github-robot k8s-github-robot merged commit 7f2622e into kubernetes:master Dec 8, 2016
@dcbw dcbw mentioned this pull request Dec 10, 2016
Merged
@anguslees anguslees deleted the extraroutes branch December 19, 2016 04:11
dims pushed a commit to dims/kubernetes that referenced this pull request Feb 8, 2018
Merge pull request kubernetes#32663 from anguslees/extraroutes
42f8892 
Automatic merge from submit-queue

openstack: Implement the `Routes` provider API

``` release-note

Implement the Routes provider API for OpenStack using Neutron extraroute extension.  This removes the need for flannel/etc where supported.  To use, ensure all your nodes are on the same Neutron (private) network and specify the router ID in new `[Route]` section of provider config:

    [Route]
    router-id = <router UUID>
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@smarterclayton smarterclayton

Labels
area/provider/openstack Issues or PRs related to openstack provider cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. release-note-none Denotes a PR that doesn't merit a release note. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet