-
Notifications
You must be signed in to change notification settings - Fork 788
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Pass status along ipam update #1082
Pass status along ipam update #1082
Conversation
} | ||
} | ||
|
||
// TODO: Check if hode device exists. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is awkward -- you're right, the host device does not exist anymore, but it may have been moved with an ADD. I worry this could cause more trouble than its worth. Hmm.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I fixed the miss-spelling.
The idea comes from this comment: #1021 (comment)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice!!!!
a33a4c8
to
aea27c7
Compare
@LionelJouin you need to be rebase |
044df64
to
c5883a2
Compare
@LionelJouin needs another rebase :-) |
@LionelJouin somehow this PR still does not have the green button. Perhaps the latest changes to Can you rebase and drop your merge commit? That helps keep the history clean. |
212689b
to
79b1e5e
Compare
b540e47
to
ef665d3
Compare
I saw the tests of the VRF plugin are also flaky, I haven't looked into them yet. |
71acd07
to
1a3f6a2
Compare
Signed-off-by: Lionel Jouin <[email protected]>
1a3f6a2
to
b034c1b
Compare
This PR fixes the missing parts of #1050 (pass STATUS to a delegated plugin)
I also updated the cni dependency to v1.2.3