Add device requests

[Lucidiot]

Another take on #2395: while the Docker CLI adds a --gpus parameter, it is in fact syntatic sugar for the DeviceRequests argument in a container's host config. This PR adds support for any kind of device request, as it seems the API was designed to handle more in the future than just GPUs.

It would still be possible to support another gpus=... argument in a way similar to #2465, that could handle the previous nvidia runtime and set the environment variables itself or use the new device requests.

For example, with NVIDIA's test image, the equivalent of --gpus all would be:

client.containers.run(
    'nvidia/cuda:9.0-base',
    'nvidia-smi',
    device_requests=[
        docker.types.DeviceRequest(count=-1, capabilities=[['gpu']])
    ]
)

The equivalent of --gpus 2 would be DeviceRequest(count=2, capabilities=[['gpu']]).

And for something like --gpus device=GPU-c6e298ba-aa33-4083-8ae3-db4e27037475,driver=nvidia,capabilities="utility,display":

client.containers.run(
    'nvidia/cuda:9.0-base',
    'nvidia-smi',
    device_requests=[
        docker.types.DeviceRequest(
            driver='nvidia',
            device_ids=['GPU-c6e298ba-aa33-4083-8ae3-db4e27037475'],
            capabilities=[
                ['gpu', 'utility', 'display']
            ]
        )
    ]
)

[zajozorPho]

Seems to work.

For all those currently using this fork - do not forget to specify api version when instantiating the client like so:

docker_client = docker.from_env(version='1.40')
docker_client.containers.run(
        'nvidia/cuda:9.0-base',
        'nvidia-smi',
        device_requests=[
            DeviceRequest(count=-1, capabilities=[['gpu']])
        ]
    )

(the default version is 1.35 even if you have docker that is new enough installed..)
just a tip for others, because I spent an hour figuring out why my api version in docker-py is lower than what docker version says..

[felixfontein]

Looks good to me. Would be nice if a docker-py maintainer could take a look as well.

[Lucidiot]

A note on the version='1.40' requirement mentioned by @zajozorPho:

I would recommend using version='auto' here as this makes the client ask the daemon what its actual API version is, which can help with troubleshooting if the installed daemon is not up to date.

The client defaults to 1.35 because it is the value of DEFAULT_DOCKER_API_VERSION defined here—I do not know whether or not this default version should be bumped. The swarm address pool and subnet size parameters both require API version 1.39 but this constant was not updated.

[maximskripnik]

@Lucidiot Thanks a lot for this implementation, it solved a problem for us. I hope this PR gets merged soon, so that we don't have to build this fork manually as a part of our CI/CD pipeline

[EpicWink]

What's the hold-up on this PR? Christmas? The two failing tests in Jenkins?

Is there any way I can help?

[zajozorPho]

ping @shin- @ulyssessouza @aanand @bfirsh
any active maintainer please?

[AustinDeric]

+1 for the code, merge it! @shin- @ulyssessouza @aanand @bfirsh

[EpicWink]

Tests failing:

• test: tests/integration/api_container_test.py AttachContainerTest.test_attach_no_stream
  environment: Jenkins - py3.7_19.03.5
  error: AssertionError: assert b'' == b'hello\n'
  Not sure why this is failing. Is it flaky? I can't replicate it
• test: tests/unit/api_container_test.py CreateContainerTest.test_create_container_with_device_requests
  environment: Jenkins py3.7_19.03.5 and py2.7_19.03.5
  error: docker.errors.InvalidVersion: device_requests param is not supported in API versions < 1.40
  This test is marked to be skipped if the API version (specifically, the environment variable "DOCKER_TEST_API_VERSION") is less than 1.40, but it's not being skipped or the API version is invalid.

[mitar]

Tested it and it works great!

[EpicWink]

For an in-library patch until this is merged, I've created a module which you can use here: _docker_patch.py

[felixfontein]

@ulyssessouza could you please take a look at this PR? It would be really nice to be able to use this feature with Docker SDK for Python.

[Dmitry1987]

Ansible team desperately needs this merged for the ansible docker_container module functionality, please please please merge it! The world is waiting! 📟

[ghost]

Any updates on this? The docker-compose guys are waiting on this too docker/compose#6691 (much frustration is being felt by many)

[EpicWink]

I've made a new PR #2581 with the test fixed

[La0]

I work with @Lucidiot and merged your fix @EpicWink. Thanks a lot for your help !

Looks like Jenkins is happy now !
@ulyssessouza could you take a look now and consider merging this PR ? It seems it's needed by a lot of folks 😃

[ghost]

@ulyssessouza whats the ETA on this PR merging? Are there any blockers?

[hadim]

I would also be interested in this PR!

[sebastianhorwege]

Same Here!

[ulyssessouza]

LGTM

[La0]

@ulyssessouza When do you plan to merge this PR ? Thanks !

[La0]

@ulyssessouza or any maintainer : Could you provide us with an update ? Is there a blocker ?

I know it's summer in the northern hemisphere, and lots of folks are not working, but the lack of communication is pretty disturbing for such an important project 😢

[EpicWink]

Thanks @Lucidiot, @La0, @ulyssessouza, @aiordache and the rest of the Docker team for getting this merged in. So many of us are looking forward to using it in downstream projects.

[alexbegoon]

Thanks a lot! You are awesome! 🍺