-
Notifications
You must be signed in to change notification settings - Fork 457
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
Add supoort for enabling GPU access #920
Conversation
8ecb17d
to
124c2cc
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, thanks.
Could you add unit tests? There's a bunch already in pytests/test_container_to_args.py. It should be easy to extend the test suite for the GPU options. The existing tests test container_to_args()
which I think is appropriate level of detail for the additional GPU tests.
@p12tic I looked at the existing unit tests and it was a bit difficult to understand. It seems to be output after execution. But Github Actions does not have a GPU and cannot be tested. |
@mokeyish Tests in pytests/test_container_to_args.py don't actually run any containers. The test structure is as follows:
With such tests any feature of podman-compose can be tested without any requirements from hardware. |
So in GPU tests I would expect tests that check whether correct GPU-related options are added to podman_args. |
Signed-off-by: YISH <mokeyish@hotmail.com>
@@ -325,3 +325,106 @@ async def test_env_file_obj_optional(self): | |||
"busybox", | |||
], | |||
) | |||
|
|||
async def test_gpu(self): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This test is actually 4 tests. Could it be split? It's ok if the preamble
c = create_compose_mock()
cnt = get_minimal_container()
cnt["command"] = ["nvidia-smi"]
cnt["deploy"] = {"resources": {"reservations": {"devices": [{}]}}}
repeats in each test.
closes #919