8000 networking: add Xen logic by sstabellini · Pull Request #3695 · rkt/rkt · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content
This repository was archived by the owner on Feb 24, 2020. It is now read-only.

networking: add Xen logic #3695

Closed
wants to merge 2 commits into from
Closed

Conversation

sstabellini
Copy link

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.

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>
@ghost
Copy link
ghost commented Jun 2, 2017

Can one of the admins verify this patch?

@lucab
Copy link
Member
lucab commented Jun 7, 2017

ok to test

@lucab lucab requested a review from squeed June 7, 2017 10:39
@sstabellini
Copy link
Author

make check fails on debian-testing, but as far as I can tell, it also fails on master in the same way

@lucab
Copy link
Member
lucab commented Jun 8, 2017

@sstabellini yes, it was a flake. Re-triggered, it is now green.

@squeed
Copy link
Contributor
squeed commented Jun 8, 2017

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 stage1/init/init.go that would make this easier.

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?

@lucab lucab changed the title Xen Networking networking: add Xen logic Jun 8, 2017
@sstabellini
Copy link
Author
sstabellini commented Jun 8, 2017 via email

@squeed
Copy link
Contributor
squeed commented Jun 16, 2017

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?

@sstabellini
Copy link
Author

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 :-)

@lucab
Copy link
Member
lucab commented Jul 18, 2017

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.

@lucab lucab modified the milestones: 1.29.0, 1.28.0 Jul 18, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants
0