add loadbalancer address allocator

[whooo]

This adds a simple controller that will watch for services of type LoadBalancer and try to allocated addresses from the specified IPv4 and/or IPv6 ranges. It's assumed that kube-router (or another network controller) will announce the addresses.

As the controller uses leases for leader election and updates the service status new RBAC permissions are required.

Tested on a small IPv6-only cluster, so IPv4/dual stack is not tested outside the unit tests.

Fixes #242

[aauren]

This seems really interesting. I'm hoping to get some time soon to take a look through it.

[aauren]

Hey @whooo can you please rebase on prep-v2.0 again? prep-v2.0 was recently rebased on master to catch up on some bug fixes and dependency updates. I know its a bit annoying, but I try to keep prep-v2.0 merge-able with master so that they don't drift too far apart.

[whooo]

Hey @whooo can you please rebase on prep-v2.0 again? prep-v2.0 was recently rebased on master to catch up on some bug fixes and dependency updates. I know its a bit annoying, but I try to keep prep-v2.0 merge-able with master so that they don't drift too far apart.

Done

[aauren]

This was quite a bit of work, thanks for thinking of us and submitting it upstream @whooo! I know there is a lot of feedback on this, but at least half of it is stylistic and nitpics so hopefully it will be easy to move through.

I wanted to mention a lot of the things that I really liked about it as well:

• I liked your function granularity, it helped make the review easier as a lot of it was essentially self-documented via function name
• I liked how you made it idempotent and stateless via how you handle looking across ranges. This made the logic a bit more complex, but it means that you don't have to catch up on state when new leaders take over

After these changes are made, I'll do a functional review as a last pass where I exercise the controller a bit, but I don't expect many additional gotchas there. However, there may be 1 or 2 more asks on this PR. After that, we'll merge it.

Again, thanks for contributing!

[aauren]

Alright, I got a chance to test it out. By in large, everything worked really great!

Having some experience with other LB allocation implementations, I noticed that this one wasn't able to handle self-assigned LB addresses like another implementation I've used. When I tried to debug this down, I found that apparently the status doesn't come through upon add (i.e. Kubernetes isn't passing it to the informer for a newly added service). Not sure how those other implementations achieved that, but people can always use externalIP for that use-case. So I think that we're good there unless you want to mention something in your documentation.

[whooo]

Having some experience with other LB allocation implementations, I noticed that this one wasn't able to handle self-assigned LB addresses like another implementation I've used. When I tried to debug this down, I found that apparently the status doesn't come through upon add (i.e. Kubernetes isn't passing it to the informer for a newly added service). Not sure how those other implementations achieved that, but people can always use externalIP for that use-case. So I think that we're good there unless you want to mention something in your documentation.

I added a note about it in the documentation, it would be possible to add some kind of support via annotations in the future

[aauren]

So it looks like some of the unit tests are failing now.

And I found one other small thing that we need to do. In the network_policy_controller we need to add some handling for this new range to ensure that it gets allow listed when the service resolves to the host.

To make this a bit easier for you, I've created a list of code references below that show the handling of external IPs, you would basically just need to do the same with the load-balancer CIDRs:

• https://github.com/cloudnativelabs/kube-router/blob/prep-v2.0/pkg/controllers/netpol/network_policy_controller.go#L800-L807
• https://github.com/cloudnativelabs/kube-router/blob/prep-v2.0/pkg/controllers/netpol/network_policy_controller.go#L472-L496
• https://github.com/cloudnativelabs/kube-router/blob/prep-v2.0/pkg/controllers/netpol/network_policy_controller.go#L69

[whooo]

So it looks like some of the unit tests are failing now.

I don't know why it failed, the checks passed on my computer, but looks good now

And I found one other small thing that we need to do. In the network_policy_controller we need to add some handling for this new range to ensure that it gets allow listed when the service resolves to the host.

To make this a bit easier for you, I've created a list of code references below that show the handling of external IPs, you would basically just need to do the same with the load-balancer CIDRs:

* https://github.com/cloudnativelabs/kube-router/blob/prep-v2.0/pkg/controllers/netpol/network_policy_controller.go#L800-L807

* https://github.com/cloudnativelabs/kube-router/blob/prep-v2.0/pkg/controllers/netpol/network_policy_controller.go#L472-L496

* https://github.com/cloudnativelabs/kube-router/blob/prep-v2.0/pkg/controllers/netpol/network_policy_controller.go#L69

Done, more or less just did a copy and paste from the external IP parts

[aauren]

LGTM! Thanks for all of your work on this @whooo! I'm sure that lots of people will be happy for the addition.