search

containers / virtcontainers

A Go package for building hardware virtualized container runtimes
Apache License 2.0
139 stars 43 forks source link

network: Replace CNI Result structure #470

Closed sboeuf closed 7 years ago

sboeuf commented 7 years ago

The goal of this PR is to replace the dependency on CNI structure types.Result with a new one. The new structure relies on very generic Go structures such as net.Interface, netlink.Route, and net.IPNet in order to provide a lot of information about the underlying network.

The choice has been made to translate only the DNS information coming from the CNI network implementation because other data can be found through a simple network scan. And because this scan has to be performed anyway (need MTU which is not provided by CNI result for instance), it is easier to fill network interfaces, IP addresses and routes structures from scratch.

Also, notice that CNI vendoring has been updated to rely on latest definition of what is considered to be a network Result.

sboeuf commented 7 years ago

@mcastelino I like the idea of relying more on the netlink package, it's gonna be consistent with the Route structure that I get from netlink and I agree it will bring more info ;)

sboeuf commented 7 years ago

@mcastelino @sameo @amshinde please take a look (again). I have rebased on top of @amshinde PR for SRIOV support. I think I did this right and then I have used netlink library, which really helped me getting the code simplified. Tell me what you think about it. @amshinde BTW, one of my commit is fixing CNI case for physical device. Indeed, I have realized that we had missed the CNI case since we were not scanning the underlying network at that time.

sameo commented 7 years ago

LGTM

Approved with PullApprove Approved with PullApprove

sboeuf commented 7 years ago

@amshinde I'd like to have your input before to merge this PR since I have rebased on top of your SRIOV work ;)