Skip to content
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

docker_container: add device_requests option #1119

Conversation

felixfontein
Copy link
Collaborator

SUMMARY

Fixes ansible/ansible#65748.

Unfortunately it is hard to test this in CI, since we don't have special hardware and docker runtimes installed. The tests are therefore very minimalistic and don't really test something.

ISSUE TYPE
  • Feature Pull Request
COMPONENT NAME

docker_container

@ansibullbot ansibullbot added affects_2.10 cloud community_review docker feature This issue/PR relates to a feature request integration tests/integration module module needs_triage plugins plugin (any type) tests tests labels Oct 18, 2020
@felixfontein
Copy link
Collaborator Author

CC @ei-grad @ssidorenko @mbigras @pete0emerson @Samh57 @xywsxp @tizz98 @bingzhangdai @pommedeterresautee since you all liked ansible/ansible#65748 or ansible/ansible#65748 (comment). Please test this, since I don't really have the ability to do so, and I guess you have :)

Copy link
Contributor

@Andersson007 Andersson007 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be also good to have:

  1. examples for new options
  2. check_mode: yes in the tests

plugins/modules/cloud/docker/docker_container.py Outdated Show resolved Hide resolved
@felixfontein
Copy link
Collaborator Author

I've added an example. I won't add check_mode tests for this option, since we don't do it for any other option either, as it would put a too large strain on CI resources. The docker_container tests are one of the most extensive tests already (over 650 tests) and run for 5-7 minutes (not counting setup). Also, the way the module is implemented makes additional check_mode tests for every option superfluous :)

Copy link
Contributor

@Andersson007 Andersson007 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the fragment, example, and tests LGTM:)

@felixfontein felixfontein added backport-1 check-before-release PR will be looked at again shortly before release and merged if possible. and removed check-before-release PR will be looked at again shortly before release and merged if possible. labels Oct 21, 2020
@felixfontein felixfontein merged commit 8670eff into ansible-collections:main Oct 27, 2020
@felixfontein felixfontein deleted the docker_container-gpu-support branch October 27, 2020 04:52
patchback bot pushed a commit that referenced this pull request Oct 27, 2020
* docker_container: add device_requests option.

* Fix copy'n'paste mistake.

* Fix failure test.

* Added example.

* Adjust tense.

(cherry picked from commit 8670eff)
felixfontein added a commit that referenced this pull request Oct 27, 2020
* docker_container: add device_requests option.

* Fix copy'n'paste mistake.

* Fix failure test.

* Added example.

* Adjust tense.

(cherry picked from commit 8670eff)

Co-authored-by: Felix Fontein <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cloud community_review docker feature This issue/PR relates to a feature request integration tests/integration module module needs_triage plugins plugin (any type) tests tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

docker_container module: GPU support
3 participants