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

backend/aws-vpc: add support for multiple route tables #561

Closed
wants to merge 1 commit into from

Conversation

pingles
Copy link

@pingles pingles commented Dec 2, 2016

We're setting up a cluster in AWS that spans across availability zones. Because we're using private + public networks we have 3 route tables (1 for each AZ). This change adds support to Flannel for configuring more than one route table.

The implementation is pretty naive: all route tables will be configured with the same routes (and with blackholes removed etc.).

I'm not an expert in AWS networking so any comments would be appreciated. Likewise, if there's a good way to write some tests to prove the behaviour, or ways to improve the code then please let me know!

Thanks so much for the project!

allows running private clusters across az's- routes are added to each of the route tables specified.
@vaijab
Copy link
Contributor

vaijab commented Dec 6, 2016

Does that mean that flannel adds private subnet routes to public network routing tables?

@pingles
Copy link
Author

pingles commented Dec 7, 2016

@vaijab: nope. we have our VPC networking configured with a private + public subnet per-AZ, for example: eu-west-1a-public, eu-west-1a-private etc. The public subnets have routes to the internet gateway, the private networks have a 0.0.0.0 route pointing to nat gateways in the equivalent public subnet.

So, for Flannel we configure it to manage routes in the private route tables.

Hope that helps? I'm not really an AWS networking guru so I may have messed up some terminology.

@t0mmyt
Copy link
Contributor

t0mmyt commented Dec 7, 2016

To expand, if you want to have non internet facing subnets (in an AWS VPC) that have internet access, each subnet requires its own NAT gateway for HA. This means that each subnet must have its own route table (as each has a different default route) but each route table still needs the full set of routes set by flannel. Therefore the fix is to support propagating to multiple route tables.

@vaijab
Copy link
Contributor

vaijab commented Dec 7, 2016

@t0mmyt @pingles thanks. I think we wanted to have a similar AWS networking setup for our clusters, but flannel was the show stopper back then.

@pingles pingles changed the title aws-vpc: add support for multiple route tables backend/aws-vpc: add support for multiple route tables Dec 15, 2016
@lokesh-shekar
Copy link

This PR merge will help us greatly

@tomdee
Copy link
Contributor

tomdee commented May 4, 2017

@pingles Are you able to rebase this PR and fix the conflict?

@pingles
Copy link
Author

pingles commented May 4, 2017

Yep, let me take a look now.

@pingles
Copy link
Author

pingles commented May 4, 2017

So it looks a little more involved than a few straight fixes- it's pretty late here now so I'll take a look at work tomorrow and push an update.

@tomdee
Copy link
Contributor

tomdee commented May 4, 2017

Great, thanks @pingles

@pingles
Copy link
Author

pingles commented May 5, 2017

Really sorry but I've not had much time to do this today. I'm also away for the next week on holiday so I probably won't look at it until I get back. If someone else wants to give it a try please do- otherwise I'll do it asap.

t0mmyt pushed a commit to t0mmyt/flannel that referenced this pull request May 8, 2017
Resolving conflicts for pull request flannel-io#561
t0mmyt pushed a commit to t0mmyt/flannel that referenced this pull request May 9, 2017
Resolving conflicts for pull request flannel-io#561 and adding documentation.
t0mmyt pushed a commit to t0mmyt/flannel that referenced this pull request May 9, 2017
Resolving conflicts for pull request flannel-io#561 and adding documentation.
@t0mmyt
Copy link
Contributor

t0mmyt commented May 9, 2017

Rebase @ #717

@pingles
Copy link
Author

pingles commented May 15, 2017

Thanks for doing that @t0mmyt! @tomdee any suggestions on how we could improve to be merge-worthy? :)

@gunjan5
Copy link
Contributor

gunjan5 commented May 16, 2017

@pingles @t0mmyt I've added review comments over at #717

t0mmyt pushed a commit to t0mmyt/flannel that referenced this pull request May 16, 2017
Resolving conflicts for pull request flannel-io#561 and adding documentation.
@gunjan5
Copy link
Contributor

gunjan5 commented May 16, 2017

#717 is merged, so closing this one. Thanks @pingles and @t0mmyt!

@gunjan5 gunjan5 closed this May 16, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants