Skip to content

[release/1.7] Fix NRI dropping requested CDI devices silently #12650

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.

Sign up for GitHub

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

Jump to bottom
Merged
mikebrow merged 1 commit into from
Dec 17, 2025
Merged

[release/1.7] Fix NRI dropping requested CDI devices silently #12650

mikebrow merged 1 commit into from
Dec 17, 2025

Conversation

@klihub
Copy link
Member

@klihub klihub commented Dec 9, 2025

Instead of just dropping requested CDI devices silently, if CDI device injection is enabled by configuration, set up NRI container adjustment with an extra option to allow CDI device injection to really take place.

@github-project-automation github-project-automation bot moved this to Needs Triage in Pull Request Review Dec 9, 2025
@klihub klihub requested a review from fuweid December 9, 2025 12:11
@dosubot dosubot bot added area/cri Container Runtime Interface (CRI) area/nri Node Resource Interface (NRI) labels Dec 9, 2025
@klihub klihub force-pushed the fixes/1.7.x/nri-cdi-device-injection branch from ff77528 to 327c6c4 Compare December 9, 2025 12:13
@klihub klihub mentioned this pull request Dec 9, 2025
Merged
@klihub klihub force-pushed the fixes/1.7.x/nri-cdi-device-injection branch from 327c6c4 to 1d0097e Compare December 9, 2025 12:44
@k8s-ci-robot k8s-ci-robot added size/S and removed size/M labels Dec 9, 2025
@klihub klihub changed the title cri,nri: don't drop requested CDI devices silently. [release/1.7] cri,nri: don't drop requested CDI devices silently. Dec 9, 2025
@klihub klihub requested a review from mikebrow December 9, 2025 17:04
mikebrow
mikebrow reviewed Dec 9, 2025
View reviewed changes
Copy link
Member

@mikebrow mikebrow left a comment •
edited
Loading

Choose a reason for hiding this comment

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

a nit on the error logging
and the question.. do we need to account for not throwing the error when devices count is at zero and cdi is disabled..

@dmcgowan dmcgowan mentioned this pull request Dec 9, 2025
Merged
1 task
tariq1890
tariq1890 reviewed Dec 9, 2025
View reviewed changes
mikebrow
mikebrow reviewed Dec 9, 2025
View reviewed changes
@AkihiroSuda
Copy link
Member

AkihiroSuda commented Dec 10, 2025

Where is the main PR?

@klihub klihub mentioned this pull request Dec 10, 2025
Closed
@elezar
Copy link
Contributor

elezar commented Dec 10, 2025

As I mentioned in the 2.0 PR, getting this in to a 1.7 release would simplify things for customers that are not able to upgrade to v2.1 or later at present.

@klihub klihub force-pushed the fixes/1.7.x/nri-cdi-device-injection branch from 1d0097e to d6d4365 Compare December 11, 2025 11:47
@klihub
cri,nri: don't drop requested CDI devices silently.
0bc74f4 
Set up NRI container adjustment with an extra option to allow
CDI device injection to really take place instead of silently
ignoring the injection request.

Signed-off-by: Krisztian Litkey <krisztian.litkey@intel.com>
@klihub klihub force-pushed the fixes/1.7.x/nri-cdi-device-injection branch from d6d4365 to 0bc74f4 Compare December 11, 2025 11:50
@AkihiroSuda AkihiroSuda requested a review from dmcgowan December 17, 2025 00:38
@klihub klihub requested a review from mikebrow December 17, 2025 09:08
mikebrow
mikebrow approved these changes Dec 17, 2025
View reviewed changes
Copy link
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

@github-project-automation github-project-automation bot moved this from Needs Triage to Review In Progress in Pull Request Review Dec 17, 2025
@mikebrow mikebrow merged commit a8ce78b into containerd:release/1.7 Dec 17, 2025
84 of 86 checks passed
@github-project-automation github-project-automation bot moved this from Review In Progress to Done in Pull Request Review Dec 17, 2025
@dmcgowan dmcgowan changed the title [release/1.7] cri,nri: don't drop requested CDI devices silently. [release/1.7] Fix NRI dropping requested CDI devices silently in CRI Dec 17, 2025
@dmcgowan dmcgowan removed the area/nri Node Resource Interface (NRI) label Dec 17, 2025
@dmcgowan dmcgowan changed the title [release/1.7] Fix NRI dropping requested CDI devices silently in CRI [release/1.7] Fix NRI dropping requested CDI devices silently Dec 17, 2025
@dmcgowan dmcgowan added area/nri Node Resource Interface (NRI) area/cri Container Runtime Interface (CRI) and removed area/cri Container Runtime Interface (CRI) area/nri Node Resource Interface (NRI) labels Dec 17, 2025
@klihub klihub deleted the fixes/1.7.x/nri-cdi-device-injection branch December 19, 2025 12:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@AkihiroSuda AkihiroSuda AkihiroSuda approved these changes

@mikebrow mikebrow mikebrow approved these changes

@fuweid fuweid Awaiting requested review from fuweid

@dmcgowan dmcgowan Awaiting requested review from dmcgowan

+2 more reviewers

@elezar elezar elezar left review comments

@tariq1890 tariq1890 tariq1890 left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

area/cri Container Runtime Interface (CRI) impact/changelog size/M

Projects

Pull Request Review
Archived in project

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@klihub @AkihiroSuda @elezar @tariq1890 @mikebrow @dmcgowan @k8s-ci-robot