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

Decrease GitHub.com/opencontainers/runtime spec #202

Merged

Conversation

elezar
Copy link
Contributor

@elezar elezar commented Apr 16, 2024

Since we are essenitally a library, we need to specify the MINIMUM version of the OCI runtime specification that we need.

This change:

  • Drops the required OCI runtime spec version in the CDI spec to v1.1.0 (where the IntelRdt.EnableCMT and IntelRdt.EnableMBM fields wer added).
  • Removes the dependabot update for the OCI runtime spec.

closes #201

@elezar elezar requested a review from klihub April 16, 2024 10:41
Signed-off-by: Evan Lezar <elezar@nvidia.com>
Copy link
Contributor

@marquiz marquiz left a comment

Choose a reason for hiding this comment

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

Makes sense. Should we roll back the version in cmd/cdi, too?

@elezar
Copy link
Contributor Author

elezar commented Apr 16, 2024

I'm not too concerned with the cmd dependencies since they're not imported into downstream dependencies. I suppose it does make sense to update them though.

@elezar elezar force-pushed the decrease-github.com/opencontainers/runtime-spec branch from a154cc2 to d1cd84a Compare April 16, 2024 12:00
@elezar
Copy link
Contributor Author

elezar commented Apr 16, 2024

@marquiz I have updated the module at cmd/cdi too.

@zvonkok if I recall correctly you may have been using this functionality for some prototyping. Does that depend on a newer spec version?

@marquiz
Copy link
Contributor

marquiz commented Apr 16, 2024

I'm not too concerned with the cmd dependencies since they're not imported into downstream dependencies. I suppose it does make sense to update them though.

Yeah, I'm not that concerned either, just for consistency as I noticed that cmd/validate was updated, too

Copy link
Contributor

@marquiz marquiz left a comment

Choose a reason for hiding this comment

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

Looks good, thanks @elezar

Copy link
Contributor

@zvonkok zvonkok left a comment

Choose a reason for hiding this comment

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

LGTM

@elezar elezar marked this pull request as draft April 16, 2024 14:36
The IntelRDT fields were added in v1.0.2. and v1.1.0. We use this latter
as the minimum version.

Signed-off-by: Evan Lezar <elezar@nvidia.com>
@elezar elezar marked this pull request as ready for review April 16, 2024 15:32
@elezar elezar force-pushed the decrease-github.com/opencontainers/runtime-spec branch from d1cd84a to ce887d5 Compare April 16, 2024 15:32
@elezar elezar requested review from klihub and marquiz April 16, 2024 15:32
@elezar
Copy link
Contributor Author

elezar commented Apr 16, 2024

@klihub @marquiz just realized that we actually need to the v1.1.0 spec.

@klihub
Copy link
Contributor

klihub commented Apr 16, 2024

Fair enough, LGTM.

@klihub klihub merged commit d7f8736 into cncf-tags:main Apr 16, 2024
7 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.

Decrease OCI runtime spec dependency version
4 participants