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_* modules: unify docker module version checks #47046

Merged
merged 3 commits into from
Oct 18, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
33 changes: 27 additions & 6 deletions lib/ansible/module_utils/docker_common.py
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,8 @@ def log(self, msg, pretty_print=False):
class AnsibleDockerClient(Client):

def __init__(self, argument_spec=None, supports_check_mode=False, mutually_exclusive=None,
required_together=None, required_if=None):
required_together=None, required_if=None, min_docker_version=MIN_DOCKER_VERSION,
min_docker_api_version=None):

merged_arg_spec = dict()
merged_arg_spec.update(DOCKER_COMMON_ARGS)
Expand All @@ -182,18 +183,33 @@ def __init__(self, argument_spec=None, supports_check_mode=False, mutually_exclu
required_together=required_together_params,
required_if=required_if)

NEEDS_DOCKER_PY2 = (LooseVersion(min_docker_version) < LooseVersion('2.0'))
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't NEEDS_DOCKER_PY2 be True if the required version is >=2.0 instead than <2.0?

Suggested change
NEEDS_DOCKER_PY2 = (LooseVersion(min_docker_version) < LooseVersion('2.0'))
NEEDS_DOCKER_PY2 = (LooseVersion(min_docker_version) >= LooseVersion('2.0'))

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, you are right! Thanks for spotting this. I'll create a new PR for fixing this (since this was already merged).


if HAS_DOCKER_MODELS and HAS_DOCKER_SSLADAPTER:
self.fail("Cannot have both the docker-py and docker python modules installed together as they use the same namespace and "
"cause a corrupt installation. Please uninstall both packages, and re-install only the docker-py or docker python "
"module. It is recommended to install the docker module if no support for Python 2.6 is required. "
"Please note that simply uninstalling one of the modules can leave the other module in a broken state.")

if not HAS_DOCKER_PY:
self.fail("Failed to import docker or docker-py - %s. Try `pip install docker` or `pip install docker-py` (Python 2.6)" % HAS_DOCKER_ERROR)

if LooseVersion(docker_version) < LooseVersion(MIN_DOCKER_VERSION):
self.fail("Error: docker / docker-py version is %s. Minimum version required is %s." % (docker_version,
MIN_DOCKER_VERSION))
if NEEDS_DOCKER_PY2:
msg = "Failed to import docker - %s. Try `pip install docker`"
else:
msg = "Failed to import docker or docker-py - %s. Try `pip install docker` or `pip install docker-py` (Python 2.6)"
Copy link
Contributor

Choose a reason for hiding this comment

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

Currently the module would suggest installing docker when min_docker_version < LooseVersion(2.0) and docker or docker-py otherwise.
These messages should be swapped, and would be by applying the suggestion above

self.fail(msg % HAS_DOCKER_ERROR)

if LooseVersion(docker_version) < LooseVersion(min_docker_version):
if NEEDS_DOCKER_PY2:
if docker_version < LooseVersion('2.0'):
msg = "Error: docker-py version is %s, while this module requires docker %s. Try `pip uninstall docker-py` and then `pip install docker`"
else:
msg = "Error: docker version is %s. Minimum version required is %s. Use `pip install --upgrade docker` to upgrade."
else:
# The minimal required version is < 2.0 (and the current version as well).
# Advertise docker (instead of docker-py) for non-Python-2.6 users.
msg = ("Error: docker / docker-py version is %s. Minimum version required is %s. "
"Hint: if you do not need Python 2.6 support, try `pip uninstall docker-py` followed by `pip install docker`")
self.fail(msg % (docker_version, min_docker_version))

self.debug = self.module.params.get('debug')
self.check_mode = self.module.check_mode
Expand All @@ -206,6 +222,11 @@ def __init__(self, argument_spec=None, supports_check_mode=False, mutually_exclu
except Exception as exc:
self.fail("Error connecting: %s" % exc)

if min_docker_api_version is not None:
docker_api_version = self.version()['ApiVersion']
if LooseVersion(docker_api_version) < LooseVersion(min_docker_api_version):
self.fail('docker API version is %s. Minimum version required is %s.' % (docker_api_version, min_docker_api_version))

def log(self, msg, pretty_print=False):
pass
# if self.debug:
Expand Down
3 changes: 2 additions & 1 deletion lib/ansible/modules/cloud/docker/docker_container.py
Original file line number Diff line number Diff line change
Expand Up @@ -2522,7 +2522,8 @@ def main():
client = AnsibleDockerClientContainer(
argument_spec=argument_spec,
required_if=required_if,
supports_check_mode=True
supports_check_mode=True,
min_docker_api_version='1.20',
)

cm = ContainerManager(client)
Expand Down
1 change: 1 addition & 0 deletions lib/ansible/modules/cloud/docker/docker_image.py
Original file line number Diff line number Diff line change
Expand Up @@ -598,6 +598,7 @@ def main():
client = AnsibleDockerClient(
argument_spec=argument_spec,
supports_check_mode=True,
min_docker_api_version='1.20',
)

results = dict(
Expand Down
3 changes: 2 additions & 1 deletion lib/ansible/modules/cloud/docker/docker_image_facts.py
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,8 @@ def main():

client = AnsibleDockerClient(
argument_spec=argument_spec,
supports_check_mode=True
supports_check_mode=True,
min_docker_api_version='1.20',
)

results = dict(
Expand Down
3 changes: 2 additions & 1 deletion lib/ansible/modules/cloud/docker/docker_login.py
Original file line number Diff line number Diff line change
Expand Up @@ -311,7 +311,8 @@ def main():
client = AnsibleDockerClient(
argument_spec=argument_spec,
supports_check_mode=True,
required_if=required_if
required_if=required_if,
min_docker_api_version='1.20',
)

results = dict(
Expand Down
3 changes: 2 additions & 1 deletion lib/ansible/modules/cloud/docker/docker_network.py
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@
from docker import utils
if HAS_DOCKER_PY_2 or HAS_DOCKER_PY_3:
from docker.types import IPAMPool, IPAMConfig
except:
except Exception as dummy:
# missing docker-py handled in ansible.module_utils.docker_common
pass

Expand Down Expand Up @@ -384,6 +384,7 @@ def main():
client = AnsibleDockerClient(
argument_spec=argument_spec,
supports_check_mode=True
# "The docker server >= 1.9.0"
)

cm = DockerNetworkManager(client)
Expand Down
4 changes: 3 additions & 1 deletion lib/ansible/modules/cloud/docker/docker_secret.py
Original file line number Diff line number Diff line change
Expand Up @@ -272,7 +272,9 @@ def main():
client = AnsibleDockerClient(
argument_spec=argument_spec,
supports_check_mode=True,
required_if=required_if
required_if=required_if,
min_docker_version='2.1.0',
min_docker_api_version='1.25',
)

results = dict(
Expand Down
3 changes: 2 additions & 1 deletion lib/ansible/modules/cloud/docker/docker_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -1053,7 +1053,8 @@ def main():
client = AnsibleDockerClient(
argument_spec=argument_spec,
mutually_exclusive=mutually_exclusive,
supports_check_mode=True
supports_check_mode=True,
min_docker_api_version='1.20',
)

result = ContainerManager(client).exec_module()
Expand Down
4 changes: 3 additions & 1 deletion lib/ansible/modules/cloud/docker/docker_swarm.py
Original file line number Diff line number Diff line change
Expand Up @@ -508,7 +508,9 @@ def main():
client = AnsibleDockerClient(
argument_spec=argument_spec,
supports_check_mode=True,
required_if=required_if
required_if=required_if,
min_docker_version='2.6.0',
min_docker_api_version='1.35',
)

results = dict(
Expand Down
23 changes: 8 additions & 15 deletions lib/ansible/modules/cloud/docker/docker_swarm_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -277,6 +277,10 @@
- docker
requirements:
- "docker-py >= 2.0"
- "Please note that the L(docker-py,https://pypi.org/project/docker-py/) Python
module has been superseded by L(docker,https://pypi.org/project/docker/)
(see L(here,https://github.com/docker/docker-py/issues/1310) for details).
Version 2.1.0 or newer is only available with the C(docker) module."
'''

RETURN = '''
Expand Down Expand Up @@ -462,13 +466,7 @@
from distutils.version import LooseVersion
from docker import utils
from docker import types
from docker import __version__ as docker_version
if LooseVersion(docker_version) >= LooseVersion('2.0.0'):
from docker.types import Ulimit, LogConfig
HAS_DOCKER_PY_2 = True
else:
from docker.utils.types import Ulimit, LogConfig
except:
except Exception as dummy:
# missing docker-py handled in ansible.module_utils.docker
pass

Expand Down Expand Up @@ -839,7 +837,7 @@ def generate_docker_py_service_description(self, name, docker_networks):
network_id = None
try:
network_id = list(filter(lambda n: n['name'] == network_name, docker_networks))[0]['id']
except:
except Exception as dummy:
pass
if network_id:
networks.append({'Target': network_id})
Expand Down Expand Up @@ -1159,15 +1157,10 @@ def main():
client = AnsibleDockerClient(
argument_spec=argument_spec,
required_if=required_if,
supports_check_mode=True
supports_check_mode=True,
min_docker_version='2.0.0',
)

if not HAS_DOCKER_PY_2:
client.module.fail_json(
msg=("docker python library version is %s. " +
"this module requires version 2.0.0 or greater")
% docker_version)

dsm = DockerServiceManager(client)
msg, changed, rebuilt, changes, facts = dsm.run()

Expand Down
4 changes: 3 additions & 1 deletion lib/ansible/modules/cloud/docker/docker_volume.py
Original file line number Diff line number Diff line change
Expand Up @@ -254,7 +254,9 @@ def main():

client = AnsibleDockerClient(
argument_spec=argument_spec,
supports_check_mode=True
supports_check_mode=True,
min_docker_version='1.10.0',
# "The docker server >= 1.9.0"
)

cm = DockerVolumeManager(client)
Expand Down