api,adaptation,stub: version exchange and capability advertisement.

[klihub]

This patch series

• adds new fields for version exchange to the NRI protocol
• updates the adaptation and the plugin stub to exchange NRI versions used
• [removed: adds a default NRI version check (which can be disabled) to the plugin stub]
• introduces capabilities intended to describe supported/required runtime NRI features
• adds an option for the adaptation to advertise NRI features for the runtime
• adds a plugins/stub option to declare capabilities required from the runtime
• checks any required capabilities against advertised or inferred runtime capabilities

Updates:
@samuelkarp @mikebrow I reworked this, ditching version-based checks and implementing more complete support for capability-based checks. Also added capability requirements to plugins which need some specific features that correspond to a capability.

Original description:
@samuelkarp @mikebrow This is what I cooked up based on some ideas thrown around in the discussions when folks were trying to figure out why a plugin by @micahhausler was not working. It was tracked then down to running against a runtime with too old NRI version for all the necessary bits to be in place. I don't want to push more stuff to this or polish it too much before getting some initial feedback/A-OK/NAK from you guys, to check if this is something we want to push on with for an NRI 1.0, and if it is this way to push on with then...

[samuelkarp]

I'm not entirely sure I understand the value of having both version comparison and capabilities. They both effectively serve the same purpose of understanding compatibility between plugin and runtime, but operate in slightly different ways. I tend to think capabilities are generally a better option in that they're more descriptive of the required functionality and need less manual mapping over time, but I can see some debugging value in exposing version too.

[samuelkarp]

Rather than generating a Go file (which can be a little weird from a compilation perspective, which you're working around with a build-tag...) I think go:embed will work more reliably; we can have this tool generate a plain text file, use //go:embed to read it in a built binary, and have some conditional that determines which value to use.

[klihub]

I'm not sure if that would eliminate the need for conditional compilation. I mean, the conditional compilation is only in place so that we can use a different method to determine the NRI version for plugins which are hosted in the main NRI repository. And the motivation for this is that (it looks to me as if) the golang runtime build info reports "(devel)" for the NRI repository itself, even when we are building from unmodified sources at an exact tag.

So then we have two choices for these plugins. 1) Either we ignore the replacement directive and take the (replaced) NRI version in the plugins' go.mod at face value, or 2) we try some other way to figure out what the NRI version of the repository might be and use that. This PR tried to do the second. Hence the go-generated git-based version detection at compile time and the extra build tag for the repo-hosted plugins to opt in for using that.

But maybe instead of this extra complication for a few sample plugins, we should instead just go with 1) above. Then whatever happens to be in a plugin's go.mod gets advertised as its NRI version.

[samuelkarp]

I'm not sure if that would eliminate the need for conditional compilation.

go:embed wouldn't need to be conditional, though we'd still generate the same $version and store it in a text file. Something like:

package version

//go:embed version.txt
var fallbackVersion string

func FallbackVersion() string {
	if fallbackVersion == "" {
		return UnknownVersion
	}
	return fallbackVersion
}

To be clear I think the approach (2) above is reasonable, but I'd rather use go:embed like here than the build tag approach.

[klihub]

Okay, but the problem with embed is that it is not optional. You cannot seed a string by go:embedding something only if it exists. Only using a bigger fs.FS hammer would allow semantically something equivalent. With a plain version string we also have to commit an empty version.txt to the repository. Otherwise compiling against NRI outside the repo would fail with a missing version.txt.

But then when compiling the repo-hosted plugins, we'd always end up with a dirty repository once we put the git-described NRI repo version in the originally empty file. So without extra logic, we'd never have a repo-hosted plugin reporting a clean NRI version without the .m suffix. Also, without extra logic I think making the repo dirty prior to compilation would break CI, where we verify that we build, run tests, etc. from a clean repo.

So if we go with embedding, then should we try to accommodate for a dirty repo in the affected places (version.txt generation, clean slate CI check), or go with fs.FS and version.txt becoming an embedded file which is present for in-repo compiled plugins but absent for others ? To me the latter then starts to sound more compelling.

[klihub]

@samuelkarp Okay, I reworked fallback to git version using a simpler go:generator, go:embed:ing, and without using related build tags. I suspect those bits are now as good/simple as they can get.

[klihub]

I'm not entirely sure I understand the value of having both version comparison and capabilities. They both effectively serve the same purpose of understanding compatibility between plugin and runtime, but operate in slightly different ways. I tend to think capabilities are generally a better option in that they're more descriptive of the required functionality and need less manual mapping over time, but I can see some debugging value in exposing version too.

Yes, I'd like to add one more thing for existing runtime versions: supported capability inference based on runtime NRI version. Then as long as a plugin declares required capabilities, we could perform a capability based compatibility check for both old and new runtime versions, either using inferred or reported capabilities.

The PR now does a version check only if a plugin does not declare required capabilities. I added it so that similar cases than what @micahhausler ran into would be flagged as an error of (assumed) incompatibility by default. We can leave it out, and then if a plugin does not declare required capabilities, we don't do any compatibility check either...