Skip to content

CDI: Use CRI Config.CDIDevices field for CDI injection#8252

Merged
fuweid merged 1 commit intocontainerd:mainfrom
bart0sh:PR008-CDI-use-CRI-field
May 10, 2023
Merged

CDI: Use CRI Config.CDIDevices field for CDI injection#8252
fuweid merged 1 commit intocontainerd:mainfrom
bart0sh:PR008-CDI-use-CRI-field

Conversation

@bart0sh
Copy link
Copy Markdown
Contributor

@bart0sh bart0sh commented Mar 11, 2023

This PR updates CRI and makes use of new CRI fields Config.CDIDevices to perform CDI injection.

Previously CDI device names were passed through annotations.

We don't remove annotation support just yet as we want to maintain compatibility with Containerd versions that only support CDI annotations.

@k8s-ci-robot
Copy link
Copy Markdown

k8s-ci-robot commented Mar 11, 2023

Hi @bart0sh. Thanks for your PR.

I'm waiting for a containerd member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@bart0sh bart0sh marked this pull request as ready for review March 11, 2023 23:32
@bart0sh
Copy link
Copy Markdown
Contributor Author

bart0sh commented Mar 11, 2023

/assign @mikebrow @AkihiroSuda @fuweid

@bart0sh
Copy link
Copy Markdown
Contributor Author

bart0sh commented Mar 12, 2023

/retest

@k8s-ci-robot
Copy link
Copy Markdown

k8s-ci-robot commented Mar 12, 2023

@bart0sh: Cannot trigger testing until a trusted user reviews the PR and leaves an /ok-to-test message.

Details

In response to this:

/retest

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@bart0sh
Copy link
Copy Markdown
Contributor Author

bart0sh commented Mar 14, 2023

@mikebrow @AkihiroSuda @fuweid friendly ping: PTAL

ruiwen-zhao
ruiwen-zhao reviewed Mar 24, 2023
View reviewed changes
ruiwen-zhao
ruiwen-zhao reviewed Mar 24, 2023
View reviewed changes
ruiwen-zhao
ruiwen-zhao reviewed Mar 24, 2023
View reviewed changes
@bart0sh bart0sh force-pushed the PR008-CDI-use-CRI-field branch from 3bfdf7d to 033a647 Compare March 26, 2023 17:44
@TBBle TBBle mentioned this pull request Mar 27, 2023
Closed
@bart0sh bart0sh force-pushed the PR008-CDI-use-CRI-field branch 2 times, most recently from 20bee39 to 8e7e8aa Compare March 29, 2023 12:23
@ruiwen-zhao
Copy link
Copy Markdown
Member

ruiwen-zhao commented Mar 29, 2023

/lgtm

@mikebrow
Copy link
Copy Markdown
Member

mikebrow commented Mar 29, 2023

/ok-to-test

mikebrow
mikebrow reviewed Mar 29, 2023
View reviewed changes
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.

some requested small changes

@bart0sh bart0sh force-pushed the PR008-CDI-use-CRI-field branch from 8e7e8aa to dab762b Compare March 30, 2023 10:10
@bart0sh
Copy link
Copy Markdown
Contributor Author

bart0sh commented Apr 2, 2023 •
edited
Loading

@mikebrow

some requested small changes

Updated the code according to your comments. PTAL.

@bart0sh bart0sh force-pushed the PR008-CDI-use-CRI-field branch 2 times, most recently from b6fe714 to 8ef74d4 Compare April 5, 2023 11:22
mikebrow
mikebrow reviewed Apr 12, 2023
View reviewed changes
mikebrow
mikebrow reviewed Apr 12, 2023
View reviewed changes
@bart0sh bart0sh force-pushed the PR008-CDI-use-CRI-field branch from 8ef74d4 to 1e43e36 Compare April 12, 2023 14:52
@bart0sh
Get CDI devices from CRI Config.CDIDevices field
cd16b31 
Signed-off-by: Ed Bartosh <eduard.bartosh@intel.com>
@bart0sh bart0sh force-pushed the PR008-CDI-use-CRI-field branch from 1e43e36 to cd16b31 Compare April 14, 2023 10:41
mikebrow
mikebrow approved these changes Apr 27, 2023
View reviewed changes
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

@bart0sh
Copy link
Copy Markdown
Contributor Author

bart0sh commented Apr 30, 2023 •
edited
Loading

@AkihiroSuda @fuweid friendly ping for a second review & possible approval.

@bart0sh
Copy link
Copy Markdown
Contributor Author

bart0sh commented May 10, 2023

@AkihiroSuda @fuweid @mikebrow Can you point me out what's needed to merge this PR?
Windows integration test failure is not related to these changes, I think.
Please, advice how to proceed further with this PR.

fuweid
fuweid approved these changes May 10, 2023
View reviewed changes
Copy link
Copy Markdown
Member

@fuweid fuweid left a comment

Choose a reason for hiding this comment

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

LGTM

@fuweid fuweid merged commit dc60137 into containerd:main May 10, 2023
@fuweid
Copy link
Copy Markdown
Member

fuweid commented May 10, 2023

@bart0sh Sorry for taking it so long. Merged.

@bart0sh
Copy link
Copy Markdown
Contributor Author

bart0sh commented May 10, 2023

@fuweid Thank you!

@bart0sh bart0sh mentioned this pull request May 16, 2023
Merged
@fuweid fuweid added the cherry-pick/1.7.x Change to be cherry picked to release/1.7 branch label May 16, 2023
@elezar elezar mentioned this pull request May 17, 2023
Merged
@samuelkarp samuelkarp added cherry-picked/1.7.x PR commits are cherry-picked into release/1.7 branch and removed cherry-pick/1.7.x Change to be cherry picked to release/1.7 branch labels Aug 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ruiwen-zhao ruiwen-zhao ruiwen-zhao left review comments

@mikebrow mikebrow mikebrow approved these changes

@fuweid fuweid fuweid approved these changes

+1 more reviewer

@TBBle TBBle TBBle left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@AkihiroSuda AkihiroSuda

@mikebrow mikebrow

@fuweid fuweid

Labels

cherry-picked/1.7.x PR commits are cherry-picked into release/1.7 branch ok-to-test

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

8 participants

@bart0sh @k8s-ci-robot @ruiwen-zhao @mikebrow @fuweid @TBBle @samuelkarp @AkihiroSuda