nokia / danm

TelCo grade network management in a Kubernetes cluster
BSD 3-Clause "New" or "Revised" License
373 stars 81 forks source link

Fixing CNI result Interface indexing #255

Closed Levovar closed 3 years ago

Levovar commented 3 years ago

When DANM merges together multiple CNI results it needs to change the indexing of the IPs to reference the proper Interface records from the merged result, rather than using the ID from their original one This can cause issues when DANM is invoked directly by CRIs such as containerd instead of Kubelet. CRIs do parse the returned result, and the improper indexing can result in the wrong interface getting identified by CRI as belonging to the "cluster network"

Levovar commented 3 years ago

As usual and should have already anticipated after this many years, nobody actually gives a danm about CNI spec: Calico puts wrong interface name into its result, and doesn't do indexing on its own at all And containerd treats "Interfaces" as mandatory, but is actually prepared to do indexing on its own (I guess they also realized somebody need to streamline the mess in code, otherwise nothing will ever work)

so, at the end of the day: eh. We just make sure the interface / IPs belonging the K8s Service Network are first in the CNI result, and let CRIs deal with the rest. Seems to work, containerd recognizes the Calico V4/V6 addresses as PodIPs with this code when the Pod had 2 dual-stakc interfaces, one Calico and 1 IPVLAN

Levovar commented 3 years ago

So for this CNI result: 2021/05/20 16:07:56.421615 { "interfaces": [ { "name": "cali5823a3ba4ac" }, { "name": "eth1", "sandbox": "f128ffec7b20430d6e3cda7d576affc530217830b784501ea513a1e0093a1d3d" } ], "ips": [ { "version": "4", "address": "192.168.180.222/32" }, { "version": "6", "address": "fc00:caa5::14c1:6948:7f8f:d810/128" }, { "version": "4", "address": "99.98.10.1/24" } ], "dns": {} }

these IPs were recognized by K8s: IP: 192.168.180.222 IPs: IP: 192.168.180.222 IP: fc00:caa5::14c1:6948:7f8f:d810

TothFerenc commented 3 years ago

I could test it again by myself. It works fine. Ready to be merged. Thanks!

MikeZappa87 commented 1 year ago

@Levovar I am one of the CNI && containerd maintainers. I am curious about your thoughts on the spec and issues you have noticed.

Levovar commented 1 year ago

@Levovar I am one of the CNI && containerd maintainers. I am curious about your thoughts on the spec and issues you have noticed.

generally speaking or specifically related to this issue? :)

MikeZappa87 commented 1 year ago

@Levovar I am one of the CNI && containerd maintainers. I am curious about your thoughts on the spec and issues you have noticed.

generally speaking or specifically related to this issue? :)

I can discuss both :-) Please reach out .... Michael.Zappa@gmail.com or slack @Michael Zappa