Drive CDI specification to v1.0.0

[elezar]

This issue serves as a top-level tracker for achieving a v1.0.0 CDI specification.

See the discussions here:

• May 16 2023
• April 16 2024

• Move version APIs to the specs-go package #214
• Document fields in spec to indicate when these were added. #215
• Document the version match process when loading specs. #231
  • Currently we fail on unknown fields and then we check whether the version field in the spec file is supported by the consumer. We also use golang null default values for all new fields so that these remain valid when loading older spec versions.
• Split API into two sections: Producer vs Consumer #230
  • producer: Minimum spec version: https://github.com/NVIDIA/nvidia-container-toolkit/blob/f794d09df19b628232d872470772b50f34b45ae8/pkg/nvcdi/spec/set-minimum-version.go#L28
• Remove deprecated APIs
  • Remove deprecated oci and qualified-device APIs #218
  • Remove deprecated registry API #219
• Remove dependency on runtime-spec from spec-go for producer API
  • Remove the ToOCI functions from the specs-go package #208
• Code cleanup
  • golangci-lint reports errors #222
  • Code cleanup #223
  • Inconsistent naming of the specs-go module #227
• Ensure that YAML marshalling maintains field ordering #235
• [nice-to-have] Automated checks for parity between Go and Rust implementations
• [nice-to-have] Add reference implementation for hooks
  • Add description of standard hooks #225

[bart0sh]

@elezar @klihub should we also remove all deprecated APIs in v1.0.0 ?

[elezar]

@elezar @klihub should we also remove all deprecated APIs in v1.0.0 ?

Yes, that sounds reasonable.

[klihub]

@elezar @klihub should we also remove all deprecated APIs in v1.0.0 ?

Yes, that sounds reasonable.

Yup, we can do that, too.

[bart0sh]

@elezar @klihub Considering quite substantial amount of changes in #233, I started to worry about API stability. The change is too big to guarantee that CDI 1.0 is still stable from my point of view. Should we postpone it or do only small amount of changes, e.g. introduce a producer package and simple spec writing function(s)?

[elezar]

@bart0sh I think we should differentiate between specification stability and API stability. Note that although #233 adds a number of files, there are no changes to the spec and the changes to existing functionality is limited to the parts of the cache that are concerned with writing specs and validation. I don't think #233 should block a spec v1.0.0 release though. Do you have insights as to whether a v1.0.0 spec is considered a requirement for DRA going beta in the v1.32 k8s release (which was the original driver for this issue).

[pohly]

v1.0.0 is not a blocker for beta. It probably is for GA.

[bart0sh]

@elezar @klihub There are already quite a bit of changes between main branch and the latest release:

$ git diff v0.8.0  | wc -l
3466

Pending PRs will probably double this count if we merge them. Which is fine, unless we really want to make a 1.0 a stable release. Do we?

If we do, in my opinion, we should consider carefully picking up only fixes and documentation improvements and making 1.0 out of them. We can then gradually add more changes in dot releases.

Opinions?

[bart0sh]

The title of this issue suggests that the scope of it is just the CDI Spec, not the API. Is it still the case?

[klihub]

The title of this issue suggests that the scope of it is just the CDI Spec, not the API. Is it still the case?

Then again, some of the TODO items, like 'Split API into two sections: Producer vs Consumer' or 'Automated checks for parity between go and Rust implementations' suggest that this is about both the CDI Specification and the implementation/API.

[bart0sh]

So, we need to either update the issue title or the TODO list. I'd say it's probably safer to update the TODO list as it will reduce amount of changes we want to make to resolve this issue.

[klihub]

So, we need to either update the issue title or the TODO list. I'd say it's probably safer to update the TODO list as it will reduce amount of changes we want to make to resolve this issue.

@bart0sh @elezar
And maybe then have separate issues for bringing the CDI Specification itself vs. bringing the programmatic API to v1.0.0 ?

And if we do that, from the Specification point of view #225 is the only remaining item I can tell.

[bart0sh]

I agree with @klihub here. Let's move API changes out of this issue. @elezar WDYT?

[elezar]

Sure, I'd be ok with that. @bart0sh in the meeting on Jan 7, 2025 we decided that you'd look at the following points and whether these warrant spec extensions:

1. Should we allow empty container edits at a device level if there are top-level edits specified? Consider spec-level when checking whether a device is valid #250
2. Should we allow aliases? [Feature Request] Add Aliases in Device spec #86
3. Should we support meta devices? Extend spec to allow for "meta" devices. #71
4. Should we add support to specify required capabilities?
   [Spec extension] Allow for IPC, NET, and CAP modifications to be specified in the CDI spec #55

Did you have a chance to assess these at all?

[klihub]

@elezar @klihub There are already quite a bit of changes between main branch and the latest release:

$ git diff v0.8.0  | wc -l
3466

Pending PRs will probably double this count if we merge them. Which is fine, unless we really want to make a 1.0 a stable release. Do we?

If we do, in my opinion, we should consider carefully picking up only fixes and documentation improvements and making 1.0 out of them. We can then gradually add more changes in dot releases.

Opinions?

Maybe we could roll a v0.9.0 first instead of trying to go straight for a 1.0.0 ?

[bart0sh]

Did you have a chance to assess these at all?

Yep, looked at that. I think I mentioned it on one of the last meetings that I don't think any of this should go into 1.0.

[bart0sh]

Maybe we could roll a v0.9.0 first instead of trying to go straight for a 1.0.0 ?

I'd prefer to go for 1.0.0 with documentation and bugfixes and shift the rest for the 1.* minor releases. We're seemingly ready for stable release back in August last year or even earlier IIRC. Is this changed somehow?

[elezar]

@bart0sh are you suggesting that we only tag the spec as v1.0.0 and leave the rest of the repo as is as part of this release (that is to say we don't bump the go package at all? Is that possible? Alternatively we would let the versions diverge at this point, releasing a v1.0.0 spec and a v0.9.0 package.

Yep, looked at that. I think I mentioned it on one of the last meetings that I don't think any of this should go into 1.0.

As a spec producer, I think #250 causes us to have to perform extra steps when generating a spec. Carying this into v1.0.0 seems unnecessary.

[bart0sh]

No need to release Spec 1.0 separately if we don't want to have different spec and API versions. I'm trying to make 1.0 a stable release, i.e. with only documentaiton and bug fixes. I don't believe we can call 1.0 stable with >6000 lines of difference with the previous release.

[elezar]

@bart0sh although I agree with the desire to produce a stable release, I don't agree with the use of "number of changed lines" as a valid metric to measure stability. The changes that have been made against main since v0.8.0 have been focussed on simplifying APIs and cleaning up dependencies so as to provide a cleaner implementation when we do release a v1.0.0.

If you have concerns about any of the following changes in terms of stability and correctness, let us address them:

$ git log HEAD...v0.8.0 --pretty="%s" | grep Merge
Merge pull request #236 from elezar/yaml-ordering
Merge pull request #251 from elezar/bump-golangci-lint
Merge pull request #248 from elezar/schema-submodule
Merge pull request #249 from elezar/fix-darwin-tests
Merge pull request #241 from Epsilon314/sort-mounts-stable
Merge pull request #238 from bart0sh/PR024-document-version-match
Merge pull request #237 from elezar/update-golang.org/x/sys
Merge pull request #232 from klihub/fixes/golangci-lint
Merge pull request #223 from bart0sh/PR023-fix-gopls-findings
Merge pull request #228 from klihub/fixes/consistent-imports
Merge pull request #219 from bart0sh/PR020-remove-registry-API
Merge pull request #226 from cncf-tags/elezar-patch-1
Merge pull request #220 from bart0sh/PR021-update-release-history
Merge pull request #221 from bart0sh/PR022-specs-go-remove-parsing-functions
Merge pull request #218 from bart0sh/PR019-remove-deprecated-APIs
Merge pull request #217 from elezar/update-dir-description
Merge pull request #215 from bart0sh/PR018-document-spec-fields
Merge pull request #214 from bart0sh/PR017-move-version-to-go-specs-go

Note that due to how we have been working in this repo, we don't have the tooling in place to easily select some subset of these changes to call a v1.0.0 release. This means that any manual steps that we do decide to use to get this done may also be error-prone.

Maybe with this in mind, it may be a good idea to release v0.9.0 with what is currently at HEAD and then have the discussion relative to that instead? Would you be ok with this as a proposal?

[bart0sh]

I'd propose to close this issue and continue with #259