-
Notifications
You must be signed in to change notification settings - Fork 23.7k
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_network] Adding scope
and attachable
flags
#49562
[docker_network] Adding scope
and attachable
flags
#49562
Conversation
@@ -294,6 +316,7 @@ def get_ip_version(cidr): | |||
|
|||
|
|||
def get_driver_options(driver_options): | |||
# TODO: Move this and the same from docker_prune.py to docker_common.py |
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.
Created #49563 to track
@@ -310,6 +333,59 @@ def get_driver_options(driver_options): | |||
|
|||
class DockerNetworkManager(object): | |||
|
|||
def _get_minimal_versions(self): | |||
# TODO: Move this and the same from docker_container.py to docker_common.py |
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.
Created #49564 to track
Added comment to the test to indicate why it doesn't strictly do what it says it does (sigh). ready_for_review |
test/integration/targets/docker_network/tasks/tests/options.yml
Outdated
Show resolved
Hide resolved
The current CI failure is unrelated (pip fails installing requirements). |
BTW, instead of making trivial changes, you can also close and reopen the PR to trigger a new CI run. |
The test
The test
The test
|
Good to know on the closing/opening trick to get the tests to re-run, especially since trailing whitespace causes the above issues. I've updated the minimum versions and added the support for old version testing. ready_for_review |
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.
shipit
I've ran the integration tests with various docker-py versions (1.10.6, 2.0.0, 2.5.0 and 3.6.0), everything looks good!
@felixfontein Thanks for the reviews! |
Thank you @felixfontein! |
Incorporating the abandoned work from PRs ansible#35288 and ansible#45552. Also adding in the version checking from `docker_container.py`, which should be abstracted out to `docker_common.py`.
SUMMARY
Incorporating the abandoned work from PRs #35288 and #45552. Also adding in
the version checking from
docker_container.py
, which should be abstractedout to
docker_common.py
.Part of the work for ansible/community#408
ISSUE TYPE
COMPONENT NAME
docker_network
ADDITIONAL INFORMATION
Integration tests passing with
ansible-test integration -v docker_network --docker ubuntu1604