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: cgroup_parent #59

Conversation

felixfontein
Copy link
Collaborator

SUMMARY

Fixes #6.

Unfortunately cgroup_parent is pretty much undocumented, the docs do not seem to mention it at all. The feature was added in Docker SDK for Python 1.4.0 (docker/docker-py@3caaa00), so we can use it with all supported versions of the Docker SKD for Python (we require >= 1.8.0). It has already been mentioned in the API docs for 1.19 (https://docs.docker.com/engine/api/v1.19/#operation/ContainerCreate), so it should be available for all Docker API versions we support (>= 1.20).

This PR also mentions in all Docker SDK for Python based modules that they use the Docker SDK for Python to communicate with the docker daemon, since that is apparently unclear according to #6 ("Missing documentation how docker-container creates containers").

ISSUE TYPE
  • Feature Pull Request
COMPONENT NAME

docker_container

@felixfontein
Copy link
Collaborator Author

@WojciechowskiPiotr if you have time, can you take a quick look at this PR and #58?

@@ -52,6 +52,11 @@
- List of capabilities to drop from the container.
type: list
elements: str
cgroup_parent:
description:
- Specify the parent cgroup for the container.

Choose a reason for hiding this comment

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

I see in the tests you added alongside you pass an empty string, what does it do? perhaps it should be added this is supported and the behaviour ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Well, the test doesn't really do anything :) The problem with testing this properly in integration tests is that you need to know which cgroups are around on the systems you run the tests on, which is pretty much impossible since they can run on any system (if you run ansible-test integration --docker xxx locally, your own docker daemon will be used, and thus it depends on your local system). That's why I put in an empty string, which by the module is treated as "value specified by user", but by the Docker daemon as "no value supplied" resp. "user does not care whether/which cgroup parent to use". This test mainly makes sure that all related code is run through at least once.

@felixfontein felixfontein added the docker-plain plain Docker (no swarm, no compose, no stack) label Jan 2, 2021
@felixfontein felixfontein merged commit c7a3d9f into ansible-collections:main Jan 3, 2021
@felixfontein felixfontein deleted the docker_container-cgroups-parent branch January 3, 2021 12:30
@felixfontein
Copy link
Collaborator Author

@bmillemathias @tadeboro thanks a lot for reviewing this!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docker-plain plain Docker (no swarm, no compose, no stack)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add support of cgroup_parent in docker_container
3 participants