-
Notifications
You must be signed in to change notification settings - Fork 57
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
Update libcni to latest version #115
Conversation
017099d
to
bd8f8a0
Compare
078beec
to
525aa26
Compare
84261d7
to
240a77e
Compare
7d2140e
to
59ac461
Compare
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.
Need a TestLibCNIType120 see:
https://github.com/containerd/go-cni/pull/115/files#diff-6cdf85f441267859d068399676e5f0ad8946811bd7e951b4e80db98d9fd2489cL200
may help draw out any issues that may arise as we move up..
Also we should open an issue for the new libcni1.2 changes that need further work..
- GC verb - allows runtimes to specify the set of known-good attachments, allowing plugins to clean up stale and leaked resources such as IPAM reservations. Libcni will also synthesize a CNI DEL for any stale cached attachments, so all users will have a form of GC, even if their plugins do not support CNI v1.1
- The STATUS verb allows a plugin to report its readiness to accept ADD requests.
- config now includes Version negotiation w/ new cniVersions
"cniVersion": "1.0.0",
"cniVersions": ["1.0.0", "1.1.0"]
-GetVersion() - ** check if version imbalance is an issue now that libcni version 1.20 matches to cni version 1.1.0 .. make sure no issues for the ci/cd/build test/upgrade/downgrade scenarios
59ac461
to
6466b22
Compare
go.mod
Outdated
github.com/stretchr/testify v1.8.0 | ||
) | ||
|
||
require ( | ||
github.com/Masterminds/semver/v3 v3.2.1 // indirect |
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.
background discussion with CNI team if exposure of this package was on purpose .. check if a newer tagged version of this semver project is needed etc..
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.
@squeed thoughts? We could just revert that pr unless we have a better route
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.
Noting nil version equality fix for panic.. Masterminds/semver#213 is not in this tagged version. Would prefer using golang public []semver
or []string
if possible to reduce externalized package dependencies.
That said we can handle the two asks separately if desired by the CNI team.
@squeed just want to get this to the top of your email |
Signed-off-by: Michael Zappa <[email protected]>
18983f5
to
853c44a
Compare
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.
LGTM thanks!
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.
LGTM
Updating libcni to support latest version