IPs return by CNI plugins no longer "assigned" to a default interface if no interface is provided

[Brian-McM]

What happend:
IPs returned by the CNI plugin that don't have an interface index are dropped and not assigned to any interface in 4.1

What you expected to happen:
These IPs to continue to be assigned to a default interface

How to reproduce it (as minimally and precisely as possible):
Use a CNI plugin that returns IPs without an interface index, like calico

Anything else we need to know?:
This PR changed to use the CreateNetworkStatuses which doesn't have a reasonable default. I've created this issue there to request a reasonable default and fail if the IPs can't be assigned to an interface, instead of dropping them.

Environment:
Multus 4.1

• Multus version: 4.1
• Kubernetes version (use kubectl version): any
• Primary CNI for Kubernetes cluster: calico

[dougbtv]

Do you get this behavior in https://github.com/k8snetworkplumbingwg/multus-cni/releases/tag/v4.1.4 ? This is hot off the press.

4.1.3 should have all the fixes available from the client library, though.

[Brian-McM]

Thanks for the response @dougbtv, I haven't tried out the latest version but the problem seems to be in the network-attachment-definition-client which is available in 4.1.3.

I've dug through that multus code and verified that the issue is there and modifying the calico cni plugin to return the interface indices with the IPs (through a naive solution) fixes the issue.

Also to be clear, this breaks the multi nics feature for the enterprise solution, which relies on the annotations multus provides to assign the IPs to the calico managed interfaces, which are no longer there.

I'm discussing this in the other ticket I referenced with @maiqueb.

[Brian-McM]

@dougbtv would we be able to get a patch release going to upgrade the network client to the latest version (the PR for the fix is merged)? Is there anything I can do to help facilitate this?

This fix is needed for some customers we have who can't upgrade the multus version until this is fixed.

[Brian-McM]

@dougbtv (cc @maiqueb) any chance I could get some update on a timeline or what I could do to push this along? We're getting close to some customer deadlines where they want to upgrade the multus version but can't.

[Brian-McM]

@dougbtv @maiqueb is there any status on moving this forward, or how I might help?

[maiqueb]

AFAIU we need to:

1. issue a new release of the the net-attach-def-client library. @dougbtv must do this since I don't have maintainer rights.
2. bump multus to consume this new release. Anyone can push this PR once the library new release is available.
3. release a new multus version. @dougbtv (or any other multus maintainer) must do this

My hands are tied @Brian-McM .
I suggest you join the multus or networkplumbingwg meetings to try to expedite this forward.

[Brian-McM]

@maiqueb @dougbtv how can I join these meetings?

I'll admit that I'm pretty disappointed that I can't at the very least get some sort of timeline on this. I put my time into fixing a breaking change here thinking that it was going to go out in some sort of timely fashion instead of putting my efforts into other ways of fixing it (admittedly less desirable).

I would appreciate having some information about when this could possibly be released.

[dougbtv]

@Brian-McM -- totally fair, I get it. Thanks Miguel for chiming in and with the help.

First up, timeline. Let me see what I can do now, but, my goal is to get this merged, tagged, and then updated in Multus with a new release tagged for Multus next Thursday (the 27th of March), which is when the next maintainer's call happens. I'll see how much progress I can make before then, but, I will need other maintainers to review and be on board. I may be able to make progress with other maintainer's async between now and then.

Second up, connection information is in the multus maintainer's agenda doc.

Thirdly, and most importantly -- please know that I personally greatly value your contribution, and I understand the disappointment/frustration. Thank you for it, and thank you for the patience. Something we should think about is adding some e2e tests that reflect your scenarios, they'd both help the community and also encourage some stability around scenarios that you value.

[maiqueb]

@Brian-McM bumping multus to consume your fix in #1409

I'd appreciate a review there.

[Brian-McM]

@maiqueb just approved, thank you very much.

@dougbtv thank you for the response last week it was very much appreciated, and apologies for not replying then.

Something we should think about is adding some e2e tests that reflect your scenarios, they'd both help the community and also encourage some stability around scenarios that you value.

Could you link me to where e2es could be added? I can't promise anything right now, but I could be interested in helping out with that.

[dougbtv]

This should in theory be fixed in this release: https://github.com/k8snetworkplumbingwg/multus-cni/releases/tag/v4.2.0

Thank you yet again!

If you're interested in contributing to some e2e cases that will help test the functionality you're looking for in the future -- we'd be happy to have them in our test suite, certainly. Not required, and no timeline, but!! We'd of course love it, and it helps to ensure what was working continues to work.

Some pointers...

The Multus e2e test folder and associated README: https://github.com/k8snetworkplumbingwg/multus-cni/tree/master/e2e

These materials get run from the github action integration found here in: https://github.com/k8snetworkplumbingwg/multus-cni/blob/master/.github/workflows/kind-e2e.yml

Depending on your situation, it might require a kind of different kind environment setup, which is totally valid.