8000 server,factory/container: delay CDI device injection later. by klihub · Pull Request #9292 · cri-o/cri-o · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

server,factory/container: delay CDI device injection later. #9292

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

Merged

Conversation

klihub
Copy link
Contributor
@klihub klihub commented Jun 27, 2025

What type of PR is this?

/kind bug

What this PR does / why we need it:

Currently CDI device injection is performed right after injecting other devices into the container. This is problematic because CDI device injection might alter, among other things, the environment. However setting up the final environment happens only later during container creation and it involves setting environment variables from the image and the Pod Spec. If the same environment variable is injected both from an image or a container, and from a CDI Spec, now the former take precedence of the latter. This is unintentional and wrong.

This patch moves CDI device injection much later during container creation, between OCI Hook injection and *oci.Container creation.

Which issue(s) this PR fixes:

None

Special notes for your reviewer:

Does this PR introduce a user-facing change?

server: delay CDI device injection, to ensure that CDI Spec edits take precedence over image defaults and the Pod Spec.

@klihub klihub requested a review from mrunalp as a code owner June 27, 2025 14:07
@openshift-ci openshift-ci bot added release-note Denotes a PR that will be considered when it comes time to generate release notes. dco-signoff: yes Indicates the PR's author has DCO signed all their commits. kind/bug Categorizes issue or PR as related to a bug. labels Jun 27, 2025
@openshift-ci openshift-ci bot requested review from hasan4791 and littlejawa June 27, 2025 14:08
@klihub klihub force-pushed the fixes/delay-cdi-device-injection branch from 9fca08f to 1c81c93 Compare June 27, 2025 14:32
Copy link
codecov bot commented Jun 27, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 66.98%. Comparing base (632435d) to head (64a94f2).
Report is 7 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #9292      +/-   ##
==========================================
+ Coverage   66.91%   66.98%   +0.07%     
==========================================
  Files         198      198              
  Lines       27190    27194       +4     
==========================================
+ Hits        18193    18217      +24     
+ Misses       7494     7478      -16     
+ Partials     1503     1499       -4     
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@haircommander
Copy link
Member

/override ci/prow/ci-e2e-evented-pleg
/approve

LGTM thanks! @cri-o/cri-o-maintainers PTAL

Copy link
Contributor
openshift-ci bot commented Jun 27, 2025

@haircommander: Overrode contexts on behalf of haircommander: ci/prow/ci-e2e-evented-pleg

In response to this:

/override ci/prow/ci-e2e-evented-pleg
/approve

LGTM thanks! @cri-o/cri-o-maintainers PTAL

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-sigs/prow repository.

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 27, 2025
klihub added 2 commits June 27, 2025 22:44
Use a few environment variables with default values to verify
that evironment variables from CDI injection take precedence
over ones in the Pod Spec.

Signed-off-by: Krisztian Litkey <krisztian.litkey@intel.com>
Currently CDI device injection is performed right after injecting
other devices into the container. This is problematic because CDI
device injection might alter, among other things, the environment.
However setting up the final environment happens only later during
container creation and it involves setting environment variables
from the image and the Pod Spec. If the same environment variable
is injected both from an image or a container, and from a CDI Spec,
now the former take precedence of the latter. This is unintentional
and wrong.

This patch moves CDI device injection much later during container
creation, between OCI Hook injection and *oci.Container creation.

Signed-off-by: Krisztian Litkey <krisztian.litkey@intel.com>
@klihub klihub force-pushed the fixes/delay-cdi-device-injection branch from 1c81c93 to 64a94f2 Compare June 27, 2025 19:46
Copy link
Contributor
openshift-ci bot commented Jun 27, 2025

@klihub: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/ci-e2e-evented-pleg 64a94f2 link false /test ci-e2e-evented-pleg

Full PR test history. Your PR dashboard.

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-sigs/prow repository. I understand the commands that are listed here.

8000

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jun 30, 2025
Copy link
Contributor
openshift-ci bot commented Jun 30, 2025

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: haircommander, klihub, saschagrunert

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:
  • OWNERS [haircommander,saschagrunert]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-merge-bot openshift-merge-bot bot merged commit 4caef2f into cri-o:main Jun 30, 2025
73 of 75 checks passed
@klihub klihub deleted the fixes/delay-cdi-device-injection branch July 4, 2025 14:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign 81D4 in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. dco-signoff: yes Indicates the PR's author has DCO signed all their commits. kind/bug Categorizes issue or PR as related to a bug. lgtm Indicates that a PR is ready to be merged. release-note Denotes a PR that will be considered when it comes time to generate release notes.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants
0