Make docker ver checks issue failures rather than silently ignoring

Also:
* make client version checks robust for two digit version pieces and
  alpha versions
* consolidate version checking code

lib/ansible/modules/cloud/docker/docker.py

@@ -409,9 +409,60 @@ def get_split_image_tag(image):
 
     return resource, tag
 
+def get_docker_py_versioninfo():
+    if hasattr(docker, '__version__'):
+        # a '__version__' attribute was added to the module but not until
+        # after 0.3.0 was pushed to pypi. If it's there, use it.
+        version = []
+        for part in docker.__version__.split('.'):
+            try:
+                version.append(int(part))
+            except ValueError:
+                for idx, char in enumerate(part):
+                    if not char.isdigit():
+                        nondigit = part[idx:]
+                        digit = part[:idx]
+                if digit:
+                    version.append(int(digit))
+                if nondigit:
+                    version.append(nondigit)
+    elif hasattr(docker.Client, '_get_raw_response_socket'):
+        # HACK: if '__version__' isn't there, we check for the existence of
+        # `_get_raw_response_socket` in the docker.Client class, which was
+        # added in 0.3.0
+        version = (0, 3, 0)
+    else:
+        # This is untrue but this module does not function with a version less
+        # than 0.3.0 so it's okay to lie here.
+        version = (0,)
+
+    return version
+
+def check_dependencies(module):
+    """
+    Ensure `docker-py` >= 0.3.0 is installed, and call module.fail_json with a
+    helpful error message if it isn't.
+    """
+    if not HAS_DOCKER_PY:
+        module.fail_json(msg="`docker-py` doesn't seem to be installed, but is required for the Ansible Docker module.")
+    else:
+        versioninfo = get_docker_py_versioninfo()
+        if versioninfo < (0, 3, 0):
+            module.fail_json(msg="The Ansible Docker module requires `docker-py` >= 0.3.0.")
+
+
 class DockerManager:
 
     counters = {'created':0, 'started':0, 'stopped':0, 'killed':0, 'removed':0, 'restarted':0, 'pull':0}
+    _capabilities = set()
+    # Map optional parameters to minimum (docker-py version, server APIVersion)
+    # docker-py version is a tuple of ints because we have to compare them
+    # server APIVersion is passed to a docker-py function that takes strings
+    _cap_ver_req = {
+            'dns': ((0, 3, 0), '1.10'),
+            'volume_from': ((0, 3, 0), '1.10'),
+            'restart_policy': ((0, 5, 0), '1.14'),
+            }
 
     def __init__(self, module):
         self.module = module
@@ -466,6 +517,39 @@ class DockerManager:
         docker_api_version = module.params.get('docker_api_version')
         self.client = docker.Client(base_url=docker_url.geturl(), version=docker_api_version)
 
+        self.docker_py_versioninfo = get_docker_py_versioninfo()
+
+    def _check_capabilties(self):
+        """
+        Create a list of available capabilities
+        """
+        api_version = self.client.version()['ApiVersion']
+        for cap, req_vers in self._cap_ver_req.items():
+            if (self.docker_py_versioninfo >= req_vers[0] and
+                    docker.utils.compare_version(req_vers[1], api_version) >= 0):
+                self._capabilities.add(cap)
+
+    def ensure_capability(self, capability):
+        """
+        Some of the functionality this ansible module implements are only
+        available in newer versions of docker.  Ensure that the capability
+        is available here.
+        """
+        if not self._capabilities:
+            self._check_capabilties()
+
+        if capability in self._capabilities:
+            return True
+
+        api_version = self.client.version()['ApiVersion']
+        self.module.fail_json(msg='Specifying the `%s` parameter requires'
+                ' docker-py: %s, docker server apiversion %s; found'
+                ' docker-py: %s, server: %s' % (
+                    capability,
+                    '.'.join(self._cap_ver_req[capability][0]),
+                    self._cap_ver_req[capability][1],
+                    '.'.join(self.docker_py_versioninfo),
+                    api_version))
 
     def get_links(self, links):
         """
@@ -628,9 +712,11 @@ class DockerManager:
                   'tty':          self.module.params.get('tty'),
                   }
 
-        if docker.utils.compare_version('1.10', self.client.version()['ApiVersion']) < 0:
+        if params['dns'] is not None:
-            params['dns'] = self.module.params.get('dns')
+            self.ensure_capability('dns')
-            params['volumes_from'] = self.module.params.get('volumes_from')
+
+        if params['volumes_from'] is not None:
+            self.ensure_capability('volumes_from')
 
         def do_create(count, params):
             results = []
@@ -675,15 +761,24 @@ class DockerManager:
             'links': self.links,
             'network_mode': self.module.params.get('net'),
         }
-        if docker.utils.compare_version('1.10', self.client.version()['ApiVersion']) >= 0 and hasattr(docker, '__version__') and docker.__version__ > '0.3.0':
-            params['dns'] = self.module.params.get('dns')
-            params['volumes_from'] = self.module.params.get('volumes_from')
 
-        if docker.utils.compare_version('1.14', self.client.version()['ApiVersion']) >= 0 and hasattr(docker, '__version__') and docker.__version__ >= '0.5.0':
+        optionals = []
-            if self.module.params.get('restart_policy') is not None:
+        for optional_param in ('dns', 'volumes_from', 'restart_policy', 'restart_policy_retry'):
-                params['restart_policy'] = { 'Name': self.module.params.get('restart_policy') }
+            optionals[optional_param] = self.module.params.get(optional_param)
-                if params['restart_policy']['Name'] == 'on-failure':
+
-                    params['restart_policy']['MaximumRetryCount'] = self.module.params.get('restart_policy_retry')
+        if optionals['dns'] is not None:
+            self.ensure_capability('dns')
+            params['dns'] = optionals['dns']
+
+        if optionals['volumes_from'] is not None:
+            self.ensure_capability('volumes_from')
+            params['volumes_from'] = optionals['volumes_from']
+
+        if optionals['restart_policy'] is not None:
+            self.ensure_capability('restart_policy')
+            params['restart_policy'] = { 'Name': optionals['restart_policy'] }
+            if params['restart_policy']['Name'] == 'on-failure':
+                params['restart_policy']['MaximumRetryCount'] = optionals['restart_policy_retry']
 
         for i in containers:
             self.client.start(i['Id'], **params)
@@ -712,31 +807,6 @@ class DockerManager:
             self.increment_counter('restarted')
 
 
-def check_dependencies(module):
-    """
-    Ensure `docker-py` >= 0.3.0 is installed, and call module.fail_json with a
-    helpful error message if it isn't.
-    """
-    if not HAS_DOCKER_PY:
-        module.fail_json(msg="`docker-py` doesn't seem to be installed, but is required for the Ansible Docker module.")
-    else:
-        HAS_NEW_ENOUGH_DOCKER_PY = False
-        if hasattr(docker, '__version__'):
-            # a '__version__' attribute was added to the module but not until
-            # after 0.3.0 was added pushed to pip. If it's there, use it.
-            if docker.__version__ >= '0.3.0':
-                HAS_NEW_ENOUGH_DOCKER_PY = True
-        else:
-            # HACK: if '__version__' isn't there, we check for the existence of
-            # `_get_raw_response_socket` in the docker.Client class, which was
-            # added in 0.3.0
-            if hasattr(docker.Client, '_get_raw_response_socket'):
-                HAS_NEW_ENOUGH_DOCKER_PY = True
-
-        if not HAS_NEW_ENOUGH_DOCKER_PY:
-            module.fail_json(msg="The Ansible Docker module requires `docker-py` >= 0.3.0.")
-
-
 def main():
     module = AnsibleModule(
         argument_spec = dict(