kube-router icon indicating copy to clipboard operation
kube-router copied to clipboard

netpol: Add dual-stack support (updated)

Open thomasferrandiz opened this issue 1 year ago • 4 comments

This PR is an update of https://github.com/cloudnativelabs/kube-router/pull/1280. The original author left Suse so I'm following up on this instead of him.

I tried to apply all the changes requested in the original PR review. As requested, I didn't squash the commits for now.

Original PR description: This change allows to define two cluster CIDRs for compatibility with Kubernetes dual-stack, with an assumption that two CIDRs are usually IPv4 and IPv6.

thomasferrandiz avatar Aug 02 '22 09:08 thomasferrandiz

@thomasferrandiz Thanks for picking this up! I'd be really glad to get proper IPv6 support into kube-router!

Unfortunately, I've torn down my IPv6 test bed for kube-router. It may take me just a bit to respond to this as I need to set it up again and put kube-router through its paces with this change.

aauren avatar Aug 06 '22 02:08 aauren

Updating here, I'm very sorry that this has taken so long to review. Work and personal items have been extremely busy over the last couple of weeks.

I still very much appreciate the effort that was put into submitting and updating this PR, and am actively trying to find time to work on it. It is my first priority for the project.

aauren avatar Sep 11 '22 16:09 aauren

Hello Did you find some time to check the PR?

thomasferrandiz avatar Sep 26 '22 08:09 thomasferrandiz

Yep. I finally got an IPv6 environment going over the weekend and I started reviewing it. Hope to have it wrapped up by end of this week.

aauren avatar Oct 03 '22 12:10 aauren

Just giving a status update...

I'm still in the process of reviewing. After a couple of small modifications, I was able to get kube-router to run correctly with this code, but its taking me a bit longer, because while it looks like most of the code is correct, most of it won't run when combined with kube-router's other features (like the routing controller) that don't support dual stack. This is hindering my testing quite a bit as I'm having to add in that functionality as I go.

Still hoping to have it wrapped up by this week, but it depends on how much I have to unravel from the other controllers to get the NPC controller to exercise relatively simple dual-stack functionality.

aauren avatar Oct 05 '22 05:10 aauren

@thomasferrandiz Still working on this. The way I use and test kube-router is a composite of all of its functions, so this has kinda forced me to incorporate dual-stack into some of the other elements of kube-router. Specifically, the NetworkRoutesController had support for either IPv4 OR IPv6, but not both together.

I've refactored most of that code now and its gotten pretty large. Its all stuff that we needed to do one day, this PR has just kinda forced the issue. If you want to follow along you can see the progress over here: https://github.com/aauren/kube-router/tree/dual-stack-additions

aauren avatar Oct 08 '22 06:10 aauren

@thomasferrandiz if you would rather, we can close out this PR and I can open one off of my branch. Or you can leave this one open and pull in my changes into your branch, rebase, and push once I've wrapped up all of my changes.

Either works for me, let me know if you have a preference.

aauren avatar Oct 08 '22 06:10 aauren

@aauren if you don't mind, I think it would be simpler if you create a new PR from your branch.

thomasferrandiz avatar Oct 14 '22 15:10 thomasferrandiz

Sounds good! Thanks for your help Thomas.

As soon as I open the other PR, I'll close this one.

As an update, I hope to be done with testing by middle of next week. I've got it working pretty smooth now, so I'm just trying to test out corner cases to ensure that everything is set.

aauren avatar Oct 14 '22 17:10 aauren

Replaced by #1386 which incorporates previous commits

Thanks so much for all of your work @thomasferrandiz and @vadorovsky!

aauren avatar Oct 25 '22 21:10 aauren