Skip to content
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

spec, libcni: add cniVersions field in CNI configurations #1010

Merged
merged 3 commits into from
Dec 11, 2023

Conversation

squeed
Copy link
Member

@squeed squeed commented Jul 19, 2023

As outlined in #941, it would be useful to CNI vendors to specify that they support multiple versions. This PR provides the spec and libcni implementation of that feature.

Specifically, it allows writing config lists like

{
    "cniVersion": "1.0.0",
    "cniVersions": ["1.0.0", "1.1.0"],
    "plugins": [
        // ...
    ]
}

for which libcni would select version 1.1.0 when calling the plugins.

@squeed
Copy link
Member Author

squeed commented Dec 5, 2023

In retrospect, I think we do want this. It will make auto-generating CNI configuration file versions easier.

If a cluster is in the midst of changing container runtimes (e.g updating CRI-O), then it may not be possible for an administrator to easily determine the CNI version in use. Allowing for multiple versions simplifies the rollout strategy.

@squeed squeed requested a review from dcbw December 5, 2023 11:43
@squeed squeed added this to the CNI v1.1 milestone Dec 5, 2023
SPEC.md Outdated
@@ -185,6 +188,15 @@ Plugins may define additional fields that they accept and may generate an error
}
```

### Version considerations

CNI runtimes and plugins may support multiple versions of the protocol. To indicate that a particular configuration supports
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@squeed maybe "CNI runtimes, plugins, and configurations may each support multiple versions of the CNI specification independently. Plugins indicate supported CNI specification versions through the reply to the VERSION command, while network configurations may indicate multiple supported CNI specification versions through the cniVersions field.

Runtimes are thus able to query both plugins and configurations and select a commonly supported CNI specification version for the CNI request. The runtime MUST select the highest supplied version that it supports. If both cniVersion and cniVersions are supplied, the runtime MUST select the highest of the two.

@coveralls
Copy link

coveralls commented Dec 11, 2023

Coverage Status

coverage: 70.897% (+0.2%) from 70.742%
when pulling a6a9891 on squeed:cni-versions
into d2bbac8 on containernetworking:main.

This adds a new top-level configuration field, `cniVersions`, where
network configurations can indicate support for more than one version.

This is to enable easy upgrading of CNI versions in plugins without
lock-step updates to the runtime.

Signed-off-by: Casey Callendrello <c1@caseyc.net>
Signed-off-by: Casey Callendrello <c1@caseyc.net>
libcni/conf.go Outdated
}
}
sort.Sort(semver.Collection(vs))
cniVersion = vs[len(vs)-1].String()
Copy link
Member

@dcbw dcbw Dec 11, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@squeed What if vs is empty? eg if somebody writes cniVersions: [], or whatever it is in JSON

This implements the `cniVersions` field in the network configuration
list. It allows for CNI plugins to specify that they support multiple
versions, and the runtime may select the highest version it supports.

Signed-off-by: Casey Callendrello <c1@caseyc.net>
Copy link
Member

@dcbw dcbw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@squeed squeed merged commit 2317778 into containernetworking:main Dec 11, 2023
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants