-
Notifications
You must be signed in to change notification settings - Fork 882
Conversation
It will be used by other hypervisors (xen), so move it out of kvm.go and rename it to something more generic. Signed-off-by: Stefano Stabellini <stefano@aporeto.com>
Add networking support for the xen stage1 flavor (which lives as a separate repository in rkt/stage1-xen). This allows stage1-xen/init/init.go to call networking.Setup provided by the main rkt repository. Signed-off-by: Stefano Stabellini <stefano@aporeto.com>
Can one of the admins verify this patch? |
ok to test |
make check fails on debian-testing, but as far as I can tell, it also fails on master in the same way |
@sstabellini yes, it was a flake. Re-triggered, it is now green. |
I'm uncomfortable with adding more cruft to the rkt kvm networking codebase. This code is already quite problematic, and is making it really hard to actually upgrade rkt to cni v0.5. I understand your position - this is not your fault - but I'd like to avoid adding any more cruft if possible. I'm also happy to refactor anything in Is there any way to keep this entirely in the xen codebase? I'm happy to help. For example, we could probably take this super fragile CNI-config-to-VM conversion logic and isolate it. I know that @mcastelino is working on the CNI infrastructure for VMs - perhaps you can share in the effort? |
On Thu, 8 Jun 2017, Casey Callendrello wrote:
I'm uncomfortable with adding more cruft to the rkt kvm networking codebase. This code is already quite problematic, and is making it really hard to actually upgrade rkt to
cni v0.5.
I understand your position - this is not your fault - but I'd like to avoid adding any more cruft if possible.
The two patches only do three things:
1) rename and move kvmSetupNetAddressing
2) add a call to a new function named "xenSetup" in networking.go:Setup
3) introduce "xenSetup" in networking.go
Unless you count "xenSetup" as cruft, the only cruft here amounts to 3
lines of code in networking.go:Setup.
I'm also happy to refactor anything in stage1/init/init.go that would make this easier.
stage1-xen doesn't use stage1/init/init.go right now. I chose to
duplicate some code to reduce the amount of changes required to upstream
rkt (I had the feeling that they weren't welcome). The stage1-xen init
code is here:
https://github.com/rkt/stage1-xen/blob/master/init/init.go
As you can see at the top of the file, among the imports there is
"github.com/sstabellini/rkt/networking"
Which is my private fork of rkt with these two patches applied.
I would love to get rid of it as it looks bad for both stage1-xen and
rkt that stage1-xen cannot use the upstream version of the networking
library.
Is there any way to keep this entirely in the xen codebase? I'm happy to help. For example, we could probably take this super fragile CNI-config-to-VM conversion logic and
isolate it.
Basically, stage1-xen/init/init.go needs to call xenSetup. I would be
happy to move xenSetup to stage1-xen/init/init.go, but it uses a number
of unexported names such as kvmSetupNetAddressing, ensureBridgeIsUp,
podEnv, activeNet, NetConf.
Would you prefer to export them and move xenSetup to
stage1-xen/init/init.go?
If you see a better way to do this, please let me know. I would be
thrilled to merge a pull request to stage1-xen to get rid of the ugly
import of sstabellini/rkt/networking.
|
Ah, I see your point. The KVM networking code is also holding back my efforts to upgrade rkt to CNIv3. I think I'd like to split it in to a separate library (or two), making the functions you need public. This will also benefit me greatly. Sound good? |
Sounds great! It would be fantastic if you could make sure to add the xenSetup function to one of the libraries, or export all the names it needs, since you are at it :-) |
I'm de-milestoning this, my understaing is that this is now a subset of #3733. If that is the case, @sstabellini please feel free to close it or to use this for the xen-specific bits still required. |
Add networking support for the Xen stage1 flavor, which lives as a separate repository under rkt/stage1-xen, but still would like to use rkt networking code to setup containers networking. Most of the code is common with kvm. These changes allow stage1-xen/init/init.go to call networking.Setup to setup the network.