invoke: if Result CNIVersion is empty use netconf CNIVersion #896
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
dcbw
force-pushed
the
empty-result-version-regression
branch
from
6d9b35e to
40fb57b
Compare
dcbw
changed the title
|
This needs testcases still... |
dcbw
force-pushed
the
empty-result-version-regression
branch
from
40fb57b to
605e654
Compare
dcbw
changed the title
dcbw
force-pushed
the
empty-result-version-regression
branch
from
605e654 to
5670150
Compare
|
Now with testcases (which exposed a bug, which I then fixed). |
dcbw
force-pushed
the
empty-result-version-regression
branch
2 times, most recently
from
3b1a18a to
d6ee597
Compare
|
ping @squeed @MikeZappa87 ~ |
|
@dcbw I won’t be able to join the meeting today. What is the work remaining for this? Lmk if you need help, I’m sure someone on the containerd might be able to help to get this to completion :-) |
The CNI Spec requires plugins to return a CNIVersion in the Result that is the same as the CNIVersion of the incoming CNI config. Empty CNIVersions are specified to map to 0.1.0 (since the first CNI spec didn't have versioning) and libcni handles that automatically. Unfortunately some versions of Weave don't do that and depend on a libcni <= 0.8 quirk that used the netconf version if the result version was empty. This is technically a libcni regression, though the plugin itself is violating the specification. Thus for backwards compatibility assume that if the Result CNIVersion is empty, the netconf CNIVersion should be used as the Result version. Fixes: containernetworking#895 Signed-off-by: Dan Williams <[email protected]>
dcbw
force-pushed
the
empty-result-version-regression
branch
from
d6ee597 to
55fe94e
Compare
|
lgtm, feel free to merge on green. |
MikeZappa87
approved these changes
May 25, 2022
mccv1r0
approved these changes
May 25, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The CNI Spec requires plugins to return a CNIVersion in the Result
that is the same as the CNIVersion of the incoming CNI config.
Empty CNIVersions are specified to map to 0.1.0 (since the first
CNI spec didn't have versioning) and libcni handles that automatically.
Unfortunately some versions of Weave don't do that and depend on
a libcni <= 0.8 quirk that used the netconf version if the result
version was empty. This is technically a libcni regression, though
the plugin itself is violating the specification.
Thus for backwards compatibility assume that if the Result
CNIVersion is empty, the netconf CNIVersion should be used as
the Result version.
Fixes: #895