container-device-interface icon indicating copy to clipboard operation
container-device-interface copied to clipboard

Drive CDI specification to v1.0.0

Open elezar opened this issue 1 year ago • 3 comments

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

See the discussions here:

  • [x] https://github.com/cncf-tags/container-device-interface/pull/214
  • [x] #215
  • [ ] Document the version match process when loading specs.
  • [ ] Split API into two sections: Producer vs Consumer
  • [ ] Add reference implementation for hooks
    • [ ] #225
  • [x] Remove deprecated APIs
    • [x] #218
    • [x] #219
  • [x] Remove dependency on runtime-spec from spec-go for producer API
    • [x] https://github.com/cncf-tags/container-device-interface/pull/208
  • [ ] Code cleanup
    • [ ] #222
    • [x] #223
    • [x] #227

elezar avatar May 14 '24 13:05 elezar

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

bart0sh avatar Jul 31 '24 11:07 bart0sh

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

Yes, that sounds reasonable.

elezar avatar Jul 31 '24 12:07 elezar

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

Yes, that sounds reasonable.

Yup, we can do that, too.

klihub avatar Jul 31 '24 13:07 klihub

@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)?

bart0sh avatar Nov 07 '24 23:11 bart0sh

@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).

elezar avatar Nov 21 '24 15:11 elezar

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

pohly avatar Nov 22 '24 07:11 pohly

@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 avatar Feb 07 '25 10:02 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?

bart0sh avatar Feb 07 '25 10:02 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?

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.

klihub avatar Feb 10 '25 19:02 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 avatar Feb 11 '25 09:02 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.

@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.

klihub avatar Feb 11 '25 12:02 klihub

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

bart0sh avatar Feb 12 '25 06:02 bart0sh

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? https://github.com/cncf-tags/container-device-interface/issues/250
  2. Should we allow aliases? https://github.com/cncf-tags/container-device-interface/issues/86
  3. Should we support meta devices? https://github.com/cncf-tags/container-device-interface/issues/71
  4. Should we add support to specify required capabilities? https://github.com/cncf-tags/container-device-interface/issues/55

Did you have a chance to assess these at all?

elezar avatar Feb 12 '25 18:02 elezar

@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 ?

klihub avatar Feb 13 '25 11:02 klihub

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 avatar Feb 13 '25 14:02 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?

bart0sh avatar Feb 13 '25 18:02 bart0sh

@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 https://github.com/cncf-tags/container-device-interface/issues/250 causes us to have to perform extra steps when generating a spec. Carying this into v1.0.0 seems unnecessary.

elezar avatar Feb 14 '25 09:02 elezar

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.

bart0sh avatar Feb 14 '25 19:02 bart0sh

@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?

elezar avatar Feb 17 '25 14:02 elezar

I'd propose to close this issue and continue with https://github.com/cncf-tags/container-device-interface/issues/259

bart0sh avatar Mar 08 '25 10:03 bart0sh

No objectsion, it seems. closing.

bart0sh avatar Mar 10 '25 08:03 bart0sh