adaptation,stub: exchange NRI versions in use.#271
adaptation,stub: exchange NRI versions in use.#271klihub wants to merge 3 commits intocontainerd:mainfrom
Conversation
| "golang.org/x/mod/semver" | ||
| ) | ||
|
|
||
| var ( |
There was a problem hiding this comment.
Is there a particular reason for these redefinitions?
There was a problem hiding this comment.
The (bit vague) idea was to shadow x/mod/semver, providing (a symbol for) everything we need from it through pkg/version so they do not leak visibily through to our other packages that import pkg/version. This is split out from #265 with capability advertisement, where we need a bit more from semver (but not this much). I can remove them if it's considered a bad idea.
There was a problem hiding this comment.
None of these are called from outside the version package though, as far as I can tell.
I'd probably recommend just removing them for now. If we end up needing more usage of x/mod/semver elsewhere later, we can address it then. Right now this is a bit of a YAGNI smell.
Signed-off-by: Krisztian Litkey <krisztian.litkey@intel.com>
Exchange NRI versions in use during plugin registration. If we do not get an NRI version from the runtime (too old peer NRI) try to infer it using the runtime type and version. The NRI version in use is discovered using the golang runtime provided build info. For plugins hosted in the main NRI repo this does not produce any useful result (because NRI is subject to a replace directive in the plugins' go.mod). Therefore within the repo we fall back to using a git-describe generated version. Signed-off-by: Krisztian Litkey <krisztian.litkey@intel.com>
Add version exchange test and related test option to adaptation. Signed-off-by: Krisztian Litkey <krisztian.litkey@intel.com>
e18227b to
ed419da
Compare
| name: "working tree version with git suffix", | ||
| version: "v0.11.0-37-g41b9c58", | ||
| expected: "v0.11.0", |
There was a problem hiding this comment.
Maybe a couple more that don't quite match:
v1.2.3-45v1.2.3-45-gv1.2.3-45-gz
| version: "v0.11.0-37-g41b9c58", | ||
| candidates: candidates, | ||
| expected: "v0.10.9", |
There was a problem hiding this comment.
maybe test v0.10.9-37-g41b9c58 too so we know that the git working tree is correctly excluded too
This PR adds NRI version to the pieces of information exchanged between a plugin and the runtime during plugin registration.
@mikebrow @samuelkarp @chrishenzie I split this out from #265 for easier review. These are self-contained and do not depend on the rest in #265. Some bits there (version based inferred capabilities for old releases) depend on this, but I'm not sure if there is consensus about adding capability advertisement in the first place. If there is we'll just need to merge this first then rebase.