Incorrect bond name

[mmirecki]

The bond-cni uses the "name" parameter from the config to name the bond interface it creates.
This is not correct, as the cni spec says that the CNI_IFNAME parameter should be used to name the interface. It also says that naming the ifc differently is not allowed:
CNI_IFNAME: Name of the interface to create inside the container; if the plugin is unable to use this interface name it must return an error.

Not conforming to this can lead to potential issues. One is an issues with plugin chaining. A metaplugin following the bond-cni will assume the name of the created interface is that found in CNI_IFNAME, and if the names differ it will fail trying to act on an incorrect (not-existing) interface.

We can fix this by either:

• getting rid of the "name" parameter in the config - api will be backwards incompatible
• ensuring the value in both locations is the same, returning an error otherwise - not nice, but will not break the api

[mmirecki]

@killianmuldoon @Eoghan1232 could you please take a look?

[Eoghan1232]

apologies on delay, never got any notification on this.

Hi @mmirecki , agreed that the above is an issue.
I would prefer this approach:

getting rid of the "name" parameter in the config - api will be backwards incompatible

[mmirecki]

np, I only realized today that no one was added to the assignees.

Let me add a PR for this.

[killianmuldoon]

Looks right to me - by the way, I'm not working in this area any more so I'm not really the right person to review these PRs and issues 🙂

[mmirecki]

Fix proposal here:
#35

[mmirecki]

Looks right to me - by the way, I'm not working in this area any more so I'm not really the right person to review these PRs and issues slightly_smiling_face

I don't think anyone other than you and Eoghan can approve/merge here, so please don't abandon the repo :)

[killianmuldoon]

I don't think anyone other than you and Eoghan can approve/merge here, so please don't abandon the repo :)

@Eoghan1232 has it handled 🙂 But it would definitely be a good idea to add someone else as a maintainer. I'm happy to stay on the list to have some redundancy there - I'm still working in Kubernetes stuff, just no longer in the network plumbing group so I don't have any bandwidth to dedicate here.

[Eoghan1232]

@eoghanlawless is named as a maintainer on this repo, as well as @esiperu-est from what I see from the meeting notes.

[Eoghan1232]

Closing issue as PR has now been merged.