nokia / danm

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

Conflist support for delegated plugins #224

Open carstenkoester opened 4 years ago

carstenkoester commented 4 years ago

Is this a BUG REPORT or FEATURE REQUEST?: feature

What happened: For delegated CNI plugins, DANM currently only supports relaying to a single plugin (a /etc/cni/net.d/*.conf file), not a configuration list (/etc/cni/net.d/*.conflist file)

What you expected to happen:

Just trying to gauge what it'd take to support conflists as delegate plugins (for example, for the bootstrap network).

While admittedly conflists are unnecessary most of the time, this is especially interesting in scenarios where we want to offer DANM as an "optional" plugin, but deliver a transparent experience to other users.

I have a couple of setups where I'd like selected users to be able to have a choice of tenantnetworks/clusternetworks; for other users, this option should not even be available and they should, for lack of a better comparison, "not even know that DANM is in the picture". The access control aspect to that is another question -- but the point of this issue is, for those users not using custom tenant/cluster-networking, and in the absence of .conflist support, the experience can never be completely transparent. Many (most) off-the-shelf CNI plugins rely on the portmap plugin for hostPort services; some CNI plugins (eg. Calico) rely on the "bandwidth" plugin to provide bandwidth limiting. By truncating the delegated CNI conflist to a single plugin, we effectively deprive default network users of these options.

As said, not a hugely important feature, but it'd be interesting to hear if this is actually a major enhancement, or something that can be done with reasonable effort.

Levovar commented 4 years ago

it would be a feature, but doesn't need big architectural changes imo

the reason why I was never that keen to support it is because I always viewed conflist as a clumsy attempt from CNI to do multiple interfaces. but in a real multi-interface environment you don't really need this, you can explicitly define multiple connections, rather than relying on some implicit, hidden magic. also, if your networking setup depends on something why don't you call it yourself, instead of risking breaking the whole Pod functionality based on optional user configuration. I never understood that kind of design thinking :) yes, most of the time conflist is there because of portmapping. little known fact that the reference config of e.g. Flannel did not use it for a long-long time, and it works totally fine without

in any case, if one would still do this feature I think only cnidel package would need changes. currently, the package does not interpret the CNI config file, it just reads it, maybe manipulates the IPAM section but otherwise transparently passes it to the delegate plugin. before this happens some parsing logic could be added checking for conflist format, and cutting up the CNI config into atomic pieces if "plugins" keyword is found. I would prefer treating the whole list as one CNI operation for the sake of both ADD, and DELETE operations, and would create only one DanmEp for the whole chain. that's because the primary use-case of this functionality is to invoke utility / supplemental "CNI"s if somebody would like to create two network connections -> you are doing it in the wrong way. make the "plugins" entries into explicit network, and ask two interfaces explicitly.