[release-1.32] server,factory/container: delay CDI device injection later. #9296
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## release-1.32 #9296 +/- ##
================================================
- Coverage 47.09% 47.09% -0.01%
================================================
Files 154 154
Lines 22204 22207 +3
================================================
Hits 10458 10458
- Misses 10675 10677 +2
- Partials 1071 1072 +1 🚀 New features to boost your workflow:
|
|
thanks @klihub |
dd1ed2e to
1bea5b2
Compare
Signed-off-by: Krisztian Litkey <[email protected]>
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 <[email protected]>
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 <[email protected]>
1bea5b2 to
a6eaab8
Compare
|
@haircommander This is a cherry-pick/backport of #9292 to the 1.32 release branch. If we don't suspect those CI failures to be related to the changes this PR brings in, could we get this in ? |
|
lgtm |
|
/approve @cri-o/cri-o-maintainers PTAL |
|
/cherry-pick release-1.31 |
|
@haircommander: once the present PR merges, I will cherry-pick it on top of In response to this:
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. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: haircommander, klihub, sohankunkerkar 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:
Approvers can indicate their approval by writing |
59112fa
into
cri-o:release-1.32
|
@haircommander: new pull request created: #9353 In response to this:
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. |
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.
Which issue(s) this PR fixes:
None
Special notes for your reviewer:
Does this PR introduce a user-facing change?