cni icon indicating copy to clipboard operation
cni copied to clipboard

Clarify if the error payload's cniVersion should be the version passed in via the input payload (network config object) or the spec version the plugin supports

Open passcod opened this issue 4 years ago • 3 comments

For the success payload, the spec says:

cniVersion specifies a Semantic Version 2.0 of CNI specification used by the plugin. A plugin may support multiple CNI spec versions (as it reports via the VERSION command), here the cniVersion returned by the plugin in the result must be consistent with the cniVersion specified in Network Configuration. If the cniVersion in the network configuration is not supported by the plugin, the plugin should return an error code 1 (see Well-known Error Codes for details).

For the error payload:

Errors must be indicated by a non-zero return code and the following JSON being printed to stdout:

{
  "cniVersion": "1.0.0",
  "code": <numeric-error-code>,
  "msg": <short-error-message>,
  "details": <long-error-message> (optional)
}

Should the cniVersion of the error payload also match the input's cniVersion?

  • If yes, what should the value be when the input payload itself cannot be decoded, or if an error occurs before it (e.g. if checking env vars is done prior to reading the input)?
  • If no, what should the value be if the plugin supports multiple spec versions?

passcod avatar Mar 31 '21 05:03 passcod

Should the cniVersion of the error payload also match the input's cniVersion?

The spec now says "cniVersion: The same value as provided by the configuration", so yes.

what should the value be when the input payload itself cannot be decoded?

I would say the spec doesn't cover this case. Discussion in the maintainers' meeting suggests the plugin should go with its preferred version. Open to your (or anyone else's) input on this.

bboreham avatar Mar 31 '21 15:03 bboreham

From a plugin perspective that's fine and what I've been doing.

The one low-key additional worry I have with that is that a runtime could validate the version on an error result it reads from a plugin and discard the error message in favour of like "plugin replied in a version we don't support" which would be pretty opaque to the user.

passcod avatar Mar 31 '21 15:03 passcod

Given current implementation in Kubernetes, having the plugin send an error to stderr will make it visible (in kubelet logs). This now seems a bit below the level of the standard, more of a "quality of implementation" thing.

bboreham avatar Apr 07 '21 15:04 bboreham