Skip to content

[release/2.0] Move CDI device spec out of the OCI package#11265

Merged
mikebrow merged 2 commits intocontainerd:release/2.0from
dmcgowan:2.0-fix-cdi-oci-opt
Jan 23, 2025
Merged

[release/2.0] Move CDI device spec out of the OCI package#11265
mikebrow merged 2 commits intocontainerd:release/2.0from
dmcgowan:2.0-fix-cdi-oci-opt

Conversation

@dmcgowan
Copy link
Copy Markdown
Member

The CDI device injection spec opt was mistakenly added to the OCI package which brought in an unintended dependency on CDI and its transitive dependencies.

(cherry picked from commit e20f7f4)

Backport of #11262 with only the first commit. The function should remain and be marked deprecated in 2.0.

The CDI device injection spec opt was mistakenly added to the OCI
package which brought in an unintended dependency on CDI and its
transitive dependencies.

Signed-off-by: Derek McGowan <[email protected]>
(cherry picked from commit e20f7f4)
Signed-off-by: Derek McGowan <[email protected]>
@dosubot dosubot Bot added area/cri Container Runtime Interface (CRI) area/runtime Runtime labels Jan 15, 2025
Comment thread pkg/oci/spec_opts.go Outdated
// well. Therefore it is important that none of the corresponding
// OCI Spec fields are reset up in the call stack once we return.
return nil
return errors.New("must use cdi package for CDI device injection")
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The function should remain functional, as it is only "deprecated"?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Or maybe just remove the function, if the impact is considered low.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I'm ok removing completely

This function has been moved to prevent an unintended dependency on CDI.

Signed-off-by: Derek McGowan <[email protected]>
(cherry picked from commit bdc847f)
Signed-off-by: Derek McGowan <[email protected]>
@dmcgowan
Copy link
Copy Markdown
Member Author

/retest

@dmcgowan
Copy link
Copy Markdown
Member Author

/retest

Copy link
Copy Markdown
Member

@mikebrow mikebrow left a comment

Choose a reason for hiding this comment

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

LGTM

@mikebrow mikebrow merged commit e465b45 into containerd:release/2.0 Jan 23, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/cri Container Runtime Interface (CRI) area/runtime Runtime size/L

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

4 participants