macvlan: support chaining for master interface #903
Conversation
firemiles
force-pushed
the
macvlan-support-chain
branch
2 times, most recently
from
2c8a3d7
to
468e2f3
Compare
If "master" is omitted from the macvlan configuration, then macvlan can get master from previous Result which contain a single interface name. Signed-off-by: firemiles <miles.dev@outlook.com>
firemiles
force-pushed
the
macvlan-support-chain
branch
from
468e2f3
to
77a0cc6
Compare
| } | ||
| n.Master = defaultRouteInterface | ||
| } else { | ||
| if len(result.Interfaces) == 1 && result.Interfaces[0].Name != "" { |
There was a problem hiding this comment.
Rather than just picking the first interface, what if we were a bit cleverer?
Right now, the macvlan plugin has the option to pick from the host namespace or container namespace. Let's preserve that when it is chained. Specifically, if n.LinkContNS is true, then pick the first value in prevResult where sandbox == true. Even better would be to pick the first result with routes or an IP address. Likewise, if n.LinkContNS is false, then do the same but for host-side interfaces.
Makes sense?
There was a problem hiding this comment.
Likewise, this brings up the interesting idea that, if n.LinkContNS is true, we should just use whatever interface name matches CNI_IFNAME, then we should rename that out and create our interface to CNI_IFNAME. Make sense?
There was a problem hiding this comment.
Rather than just picking the first interface, what if we were a bit cleverer?
Right now, the macvlan plugin has the option to pick from the host namespace or container namespace. Let's preserve that when it is chained. Specifically, if
n.LinkContNSis true, then pick the first value inprevResultwheresandbox == true. Even better would be to pick the first result with routes or an IP address. Likewise, ifn.LinkContNSis false, then do the same but for host-side interfaces.Makes sense?
We have a scenario where we need to dynamically select the host master to create macvlan, and I don't know much about the scenario where we need to select master in the container.
|
Thank you for the contribution. I agreed your concepts. I suppose we need to clarify how macvlan selects master interface from all combination of the options. We also need to describe somewhere in code otherwise we may get confused later. |
| if len(result.Interfaces) == 1 && result.Interfaces[0].Name != "" { | ||
| n.Master = result.Interfaces[0].Name | ||
| } else { | ||
| return nil, "", fmt.Errorf("chained master failure. PrevResult lacks a single named interface") |
There was a problem hiding this comment.
I am thinking that we could provide a way to pick interface from prevResult by 'master' option.
I'm just wondering how about to pick interface by 'master' option name if prevResult != nil and multiple interfaces in result.
There was a problem hiding this comment.
master option is a static option in cni config. how to dynamic config for different master? Give a special master option value to tell maclvan plugin to get master from prevResult ?
this patch support macvlan get master from previous Result
Closes: #901