Skip to content

Conversation

@rbtr
Copy link
Collaborator

@rbtr rbtr commented Jul 20, 2022

Signed-off-by: Evan Baker rbtr@users.noreply.github.com

Reason for Change:

Issue Fixed:

Requirements:

Notes:

@rbtr rbtr requested review from nddq and tamilmani1989 July 20, 2022 22:54
@rbtr rbtr self-assigned this Jul 20, 2022
@rbtr rbtr added cni Related to CNI. swift Related to SWIFT networking. labels Jul 20, 2022
@@ -0,0 +1,14 @@
{
"cniVersion": "0.3.1",
"name": "cilium",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we need to change the name here, otherwise Cilium won't invoke cmdDel from azure-ipam. Pinging @tamilmani1989 and @wedaly

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

apparently, the name conflicts with another existing chaining plugin's name, which makes the cmdDel flow returns early

Copy link
Member

@nddq nddq Jul 20, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link

@wedaly wedaly Jul 20, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think "cilium" will work, actually -- it's "azure" that was causing problems. The chainingapi.DefaultConfigName in the CNI code is "cilium", so setting name="cilium" tells the CNI to skip chaining (which is what we want).

Signed-off-by: Evan Baker <rbtr@users.noreply.github.com>
@rbtr rbtr requested a review from nddq July 21, 2022 22:01
Copy link
Member

@nddq nddq left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm 👌

@nddq nddq merged commit 6a89af2 into Azure:master Jul 21, 2022
@rbtr rbtr deleted the dropgz-cilium branch July 21, 2022 23:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cni Related to CNI. swift Related to SWIFT networking.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants