pkg/api: honor cdi devices from the hostconfig #25171
Conversation
giuseppe
added
the
No New Tests
openshift-ci
bot
added
release-note
approved
github-actions
bot
added
the
kind/api-change
|
LGTM |
|
This fixes #19338 right? |
|
Code LGTM but we really need CI testing for this |
If so make sure to add the fixes to the commit please. As for tests I guess someone needs to take care of #21448 first so we can create a small example cdi spec in a tmp dir foe a test to use. Then use that in either the compose or apiv2 tests and ensure the cdi spec was parsed and the right device was added. But unless the other PR is in I see no way to correctly test this. I really, really hate the practise of tests dumping stuff in my /etc... and that never works rootless anyway so I would be fine without a test for now. |
giuseppe
force-pushed
the
pass-device-requests-to-cli
branch
from
d8347cf to
f4bf555
Compare
yeah I've avoided a test because it is not easy to add one, and the current one we have for CDI writes to I've added a test for compose, it works only for root, and it is slightly better than the integration test, as it creates a tmpfs instead of |
I didn't initially add it because I was not sure it covers all that was discussed there |
giuseppe
force-pushed
the
pass-device-requests-to-cli
branch
2 times, most recently
from
9d2910a to
23654c6
Compare
giuseppe
force-pushed
the
pass-device-requests-to-cli
branch
2 times, most recently
from
707ab97 to
9223c57
Compare
pass down the devices specifies in the resources block so that CDI
devices in the compose file are honored.
Tested manually with the following compose file:
services:
testgpupodman_count:
image: ubuntu:latest
command: ["nvidia-smi"]
profiles: [gpu]
deploy:
resources:
reservations:
devices:
- driver: nvidia
count: 1
capabilities: [gpu]
testgpupodman_deviceid:
image: docker.io/ubuntu:latest
command: ["nvidia-smi"]
deploy:
resources:
reservations:
devices:
- driver: cdi
device_ids: ['nvidia.com/gpu=all']
capabilities: [gpu]
Closes: containers#19338
Signed-off-by: Giuseppe Scrivano <[email protected]>
giuseppe
force-pushed
the
pass-device-requests-to-cli
branch
from
9223c57 to
18e2907
Compare
|
@Luap99 comments addressed |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: giuseppe, Luap99 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 |
|
/lgtm |
| @@ -0,0 +1,9 @@ | |||
| if is_rootless; then | |||
| reason=" - can't write to /etc/cdi" | |||
There was a problem hiding this comment.
Would using /var/run/cdi allow for these to run in rootless environments?
There was a problem hiding this comment.
AFAICS, we are not currently using that in Podman (probably we should).
There was a problem hiding this comment.
Yes but even when we add that a rootless user would still have no write privs there.
Someone should finish #21448 so the test can pick their own private dir for the configs.
There was a problem hiding this comment.
I can't remember if the handling of /etc/cdi and /var/run/cdi respect the relevant XDG_ envvars.
|
Is this a good 5.4 candidate? I think I'd like it in |
Disclaimer: I am not very familiar with all the details of CDI, so there might still be something missing, but the following patch seems to be enough for the test cases that were reported so I open a PR to gather some feedback.
Pass down the devices specifies in the resources block so that CDI devices in the compose file are honored.
Tested manually with the following compose file:
services:
testgpupodman_count:
image: ubuntu:latest
command: ["nvidia-smi"]
profiles: [gpu]
deploy:
resources:
reservations:
devices:
- driver: nvidia
count: 1
capabilities: [gpu]
testgpupodman_deviceid:
image: docker.io/ubuntu:latest
command: ["nvidia-smi"]
deploy:
resources:
reservations:
devices:
- driver: cdi
device_ids: ['nvidia.com/gpu=all']
capabilities: [gpu]
Does this PR introduce a user-facing change?