search

lyft / cni-ipvlan-vpc-k8s

AWS VPC Kubernetes CNI driver using IPvlan
Apache License 2.0
360 stars 58 forks source link

Plugin does not work with containerd runtime #33

Closed lbernail closed 6 years ago

lbernail commented 6 years ago

I have tried to use the CNI plugin with containerd 1.1 (which ships with built-in CRI support) and it does not work.

After digging into the issue, it seems that containerd cannot retrieve the IP address of eth0 from the plugin output and fails to create the sandbox.

Looking at the code for unnumbered_ptp plugin, it seems that it is overriding the interface created by the ipvlan plugin: https://github.com/lyft/cni-ipvlan-vpc-k8s/blob/master/plugin/unnumbered-ptp/unnumbered-ptp.go#L244 so the final output does not contain information on eth0.

In addition, the plugin seems to "move" the IP address created for interface eth0 to interface veth0 (https://github.com/lyft/cni-ipvlan-vpc-k8s/blob/master/plugin/unnumbered-ptp/unnumbered-ptp.go#L241) which does not work with containerd because it is explicitly looking for the IP address associated to eth0.

I tried to modify this behavior to append the veth interfaces to the interface list generated by the ipvlan plugin and disable moving the IP address to veth0. After this change containerd successfully created network sandboxes. I may have missed something in the logic of the unnumbered_ptp plugin. I will submit a tentative PR fixing this. Happy to discuss this further.

paulnivin commented 6 years ago

Thanks for looking into this; we haven't tested with containerd. Somewhat related, I'm working on unforking our custom ipvlan plugin so we can switch back to upstream now that https://github.com/containernetworking/plugins/pull/88 has been merged and shipped with 0.7.0. I'll look into your PR as part of that work -- the ipvlan unfork change should land soon.

lbernail commented 6 years ago

Great thanks I think the workaround I used in my PR to populate the MAC address of the peered veth is probably an issue in the CNI ip pkg. I will check how it is used in other plugins

lbernail commented 6 years ago

We have been running this patch for a few weeks and it works with both docker and containerd.

However for containerd we found an issue: https://github.com/containerd/containerd/issues/2329

This issue is not a big problem in general but will break the CNI plugin when triggered (which requires manually creating a network namespace, for instance when using cnitool) due to how the ipam plugin identifies used IP addresses (by iterating over /var/run/netns which will contain invalid entries). I wonder if instead of iterating over /var/run/netns we could use the CRI API directly.

theatrus commented 6 years ago

Calling the CRI API to enumerate namespaces is likely the best plan as it will become the most portable state. I'm happy to work on this as a third (and preferred) path for discovery of existing namespaces this week - I haven't had a chance to do any digging into any API conformance issues. Or if you jump on it first, patches accepted :)

Random-Liu commented 6 years ago

I think all the CRI implementations today are assuming eth0 is the default interface, and none of them makes it configurable:

In addition, the plugin seems to "move" the IP address created for interface eth0 to interface veth0 (https://github.com/lyft/cni-ipvlan-vpc-k8s/blob/master/plugin/unnumbered-ptp/unnumbered-ptp.go#L241) which does not work with containerd because it is explicitly looking for the IP address associated to eth0.

Not quite familiar with this. If this is done after SetupPod, it works for containerd and cri-o, because both of them cache the ip address. But if this happens before, it won't work.

lbernail commented 6 years ago

It turns out the netns issue is actually a bug in iproute (see https://github.com/containerd/containerd/issues/2329 for more details) We will start testing containerd with the CNI plugin much more seriously very soon (still requires PR https://github.com/lyft/cni-ipvlan-vpc-k8s/pull/34)