docker: fix sanity errors (ansible#60047)

* Remove sanity check errors.

* More linting.

* Forgot to update places.

* Remove choices which aren't provided in argspec.

contrib/inventory/docker.py

@@ -20,6 +20,10 @@
 # along with Ansible.  If not, see <http://www.gnu.org/licenses/>.
 #
 
+from __future__ import (absolute_import, division, print_function)
+__metaclass__ = type
+
+
 DOCUMENTATION = '''
 
 Docker Inventory Script
@@ -383,7 +387,7 @@
 # Client has recently been split into DockerClient and APIClient
 try:
     from docker import Client
-except ImportError as exc:
+except ImportError as dummy:
     try:
         from docker import APIClient as Client
     except ImportError as exc:

lib/ansible/module_utils/docker/common.py

@@ -16,6 +16,10 @@
 # You should have received a copy of the GNU General Public License
 # along with Ansible.  If not, see <http://www.gnu.org/licenses/>.
 
+from __future__ import (absolute_import, division, print_function)
+__metaclass__ = type
+
+
 import os
 import platform
 import re
@@ -554,16 +558,16 @@ def get_container(self, name=None):
 
         return result
 
-    def get_network(self, name=None, id=None):
+    def get_network(self, name=None, network_id=None):
         '''
         Lookup a network and return the inspection results.
         '''
-        if name is None and id is None:
+        if name is None and network_id is None:
             return None
 
         result = None
 
-        if id is None:
+        if network_id is None:
             try:
                 for network in self.networks():
                     self.log("testing network: %s" % (network['Name']))
@@ -579,12 +583,12 @@ def get_network(self, name=None, id=None):
                 self.fail("Error retrieving network list: %s" % exc)
 
         if result is not None:
-            id = result['Id']
+            network_id = result['Id']
 
-        if id is not None:
+        if network_id is not None:
             try:
-                self.log("Inspecting network Id %s" % id)
-                result = self.inspect_network(id)
+                self.log("Inspecting network Id %s" % network_id)
+                result = self.inspect_network(network_id)
                 self.log("Completed network inspection")
             except NotFound as dummy:
                 return None
@@ -602,20 +606,20 @@ def find_image(self, name, tag):
 
         self.log("Find image %s:%s" % (name, tag))
         images = self._image_lookup(name, tag)
-        if len(images) == 0:
+        if not images:
             # In API <= 1.20 seeing 'docker.io/<name>' as the name of images pulled from docker hub
             registry, repo_name = auth.resolve_repository_name(name)
             if registry == 'docker.io':
                 # If docker.io is explicitly there in name, the image
                 # isn't found in some cases (#41509)
                 self.log("Check for docker.io image: %s" % repo_name)
                 images = self._image_lookup(repo_name, tag)
-                if len(images) == 0 and repo_name.startswith('library/'):
+                if not images and repo_name.startswith('library/'):
                     # Sometimes library/xxx images are not found
                     lookup = repo_name[len('library/'):]
                     self.log("Check for docker.io image: %s" % lookup)
                     images = self._image_lookup(lookup, tag)
-                if len(images) == 0:
+                if not images:
                     # Last case: if docker.io wasn't there, it can be that
                     # the image wasn't found either (#15586)
                     lookup = "%s/%s" % (registry, repo_name)
@@ -635,18 +639,18 @@ def find_image(self, name, tag):
         self.log("Image %s:%s not found." % (name, tag))
         return None
 
-    def find_image_by_id(self, id):
+    def find_image_by_id(self, image_id):
         '''
         Lookup an image (by ID) and return the inspection results.
         '''
-        if not id:
+        if not image_id:
             return None
 
-        self.log("Find image %s (by ID)" % id)
+        self.log("Find image %s (by ID)" % image_id)
         try:
-            inspection = self.inspect_image(id)
+            inspection = self.inspect_image(image_id)
         except Exception as exc:
-            self.fail("Error inspecting image ID %s - %s" % (id, str(exc)))
+            self.fail("Error inspecting image ID %s - %s" % (image_id, str(exc)))
         return inspection
 
     def _image_lookup(self, name, tag):
@@ -721,7 +725,7 @@ def report_warnings(self, result, warnings_key=None):
         elif isinstance(result, string_types) and result:
             self.module.warn('Docker warning: {0}'.format(result))
 
-    def inspect_distribution(self, image):
+    def inspect_distribution(self, image, **kwargs):
         '''
         Get image digest by directly calling the Docker API when running Docker SDK < 4.0.0
         since prior versions did not support accessing private repositories.
@@ -734,7 +738,7 @@ def inspect_distribution(self, image):
                     self._url('/distribution/{0}/json', image),
                     headers={'X-Registry-Auth': header}
                 ), json=True)
-        return super(AnsibleDockerClient, self).inspect_distribution(image)
+        return super(AnsibleDockerClient, self).inspect_distribution(image, **kwargs)
 
 
 def compare_dict_allow_more_present(av, bv):
@@ -749,22 +753,22 @@ def compare_dict_allow_more_present(av, bv):
     return True
 
 
-def compare_generic(a, b, method, type):
+def compare_generic(a, b, method, datatype):
     '''
-    Compare values a and b as described by method and type.
+    Compare values a and b as described by method and datatype.
 
     Returns ``True`` if the values compare equal, and ``False`` if not.
 
     ``a`` is usually the module's parameter, while ``b`` is a property
     of the current object. ``a`` must not be ``None`` (except for
-    ``type == 'value'``).
+    ``datatype == 'value'``).
 
     Valid values for ``method`` are:
     - ``ignore`` (always compare as equal);
     - ``strict`` (only compare if really equal)
     - ``allow_more_present`` (allow b to have elements which a does not have).
 
-    Valid values for ``type`` are:
+    Valid values for ``datatype`` are:
     - ``value``: for simple values (strings, numbers, ...);
     - ``list``: for ``list``s or ``tuple``s where order matters;
     - ``set``: for ``list``s, ``tuple``s or ``set``s where order does not
@@ -783,17 +787,17 @@ def compare_generic(a, b, method, type):
             return True
         # Otherwise, not equal for values, and equal
         # if the other is empty for set/list/dict
-        if type == 'value':
+        if datatype == 'value':
             return False
         # For allow_more_present, allow a to be None
         if method == 'allow_more_present' and a is None:
             return True
         # Otherwise, the iterable object which is not None must have length 0
         return len(b if a is None else a) == 0
     # Do proper comparison (both objects not None)
-    if type == 'value':
+    if datatype == 'value':
         return a == b
-    elif type == 'list':
+    elif datatype == 'list':
         if method == 'strict':
             return a == b
         else:
@@ -805,19 +809,19 @@ def compare_generic(a, b, method, type):
                     return False
                 i += 1
             return True
-    elif type == 'dict':
+    elif datatype == 'dict':
         if method == 'strict':
             return a == b
         else:
             return compare_dict_allow_more_present(a, b)
-    elif type == 'set':
+    elif datatype == 'set':
         set_a = set(a)
         set_b = set(b)
         if method == 'strict':
             return set_a == set_b
         else:
             return set_b >= set_a
-    elif type == 'set(dict)':
+    elif datatype == 'set(dict)':
         for av in a:
             found = False
             for bv in b:

lib/ansible/module_utils/docker/swarm.py

@@ -2,6 +2,10 @@
 # (c) Thierry Bouvet (@tbouvet)
 # GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt)
 
+from __future__ import (absolute_import, division, print_function)
+__metaclass__ = type
+
+
 import json
 from time import sleep
 

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

@@ -1064,8 +1064,8 @@
 ]
 
 
-def is_volume_permissions(input):
-    for part in input.split(','):
+def is_volume_permissions(mode):
+    for part in mode.split(','):
         if part not in ('rw', 'ro', 'z', 'Z', 'consistent', 'delegated', 'cached', 'rprivate', 'private', 'rshared', 'shared', 'rslave', 'slave', 'nocopy'):
             return False
     return True
@@ -1086,30 +1086,30 @@ def parse_port_range(range_or_port, client):
         return [int(range_or_port)]
 
 
-def split_colon_ipv6(input, client):
+def split_colon_ipv6(text, client):
     '''
     Split string by ':', while keeping IPv6 addresses in square brackets in one component.
     '''
-    if '[' not in input:
-        return input.split(':')
+    if '[' not in text:
+        return text.split(':')
     start = 0
     result = []
-    while start < len(input):
-        i = input.find('[', start)
+    while start < len(text):
+        i = text.find('[', start)
         if i < 0:
-            result.extend(input[start:].split(':'))
+            result.extend(text[start:].split(':'))
             break
-        j = input.find(']', i)
+        j = text.find(']', i)
         if j < 0:
-            client.fail('Cannot find closing "]" in input "{0}" for opening "[" at index {1}!'.format(input, i + 1))
-        result.extend(input[start:i].split(':'))
-        k = input.find(':', j)
+            client.fail('Cannot find closing "]" in input "{0}" for opening "[" at index {1}!'.format(text, i + 1))
+        result.extend(text[start:i].split(':'))
+        k = text.find(':', j)
         if k < 0:
-            result[-1] += input[i:]
-            start = len(input)
+            result[-1] += text[i:]
+            start = len(text)
         else:
-            result[-1] += input[i:k]
-            if k == len(input):
+            result[-1] += text[i:k]
+            if k == len(text):
                 result.append('')
                 break
             start = k + 1
@@ -1409,7 +1409,7 @@ def _get_mounts(self):
             for vol in self.volumes:
                 if ':' in vol:
                     if len(vol.split(':')) == 3:
-                        host, container, dummy = vol.split(':')
+                        dummy, container, dummy = vol.split(':')
                         result.append(container)
                         continue
                     if len(vol.split(':')) == 2:
@@ -1756,11 +1756,11 @@ def _process_mounts(self):
         mounts_expected = []
         for mount in self.mounts:
             target = mount['target']
-            type = mount['type']
+            datatype = mount['type']
             mount_dict = dict(mount)
             # Sanity checks (so we don't wait for docker-py to barf on input)
-            if mount_dict.get('source') is None and type != 'tmpfs':
-                self.client.fail('source must be specified for mount "{0}" of type "{1}"'.format(target, type))
+            if mount_dict.get('source') is None and datatype != 'tmpfs':
+                self.client.fail('source must be specified for mount "{0}" of type "{1}"'.format(target, datatype))
             mount_option_types = dict(
                 volume_driver='volume',
                 volume_options='volume',
@@ -1770,9 +1770,9 @@ def _process_mounts(self):
                 tmpfs_size='tmpfs',
                 tmpfs_mode='tmpfs',
             )
-            for option, req_type in mount_option_types.items():
-                if mount_dict.get(option) is not None and type != req_type:
-                    self.client.fail('{0} cannot be specified for mount "{1}" of type "{2}" (needs type "{3}")'.format(option, target, type, req_type))
+            for option, req_datatype in mount_option_types.items():
+                if mount_dict.get(option) is not None and datatype != req_datatype:
+                    self.client.fail('{0} cannot be specified for mount "{1}" of type "{2}" (needs type "{3}")'.format(option, target, datatype, req_datatype))
             # Handle volume_driver and volume_options
             volume_driver = mount_dict.pop('volume_driver')
             volume_options = mount_dict.pop('volume_options')
@@ -2090,12 +2090,14 @@ def has_different_configuration(self, image):
                             c = sorted(c)
                     elif compare['type'] == 'set(dict)':
                         # Since the order does not matter, sort so that the diff output is better.
-                        def sort_key_fn(x):
+                        if key == 'expected_mounts':
                             # For selected values, use one entry as key
-                            if key == 'expected_mounts':
+                            def sort_key_fn(x):
                                 return x['target']
+                        else:
                             # We sort the list of dictionaries by using the sorted items of a dict as its key.
-                            return sorted((a, str(b)) for a, b in x.items())
+                            def sort_key_fn(x):
+                                return sorted((a, str(b)) for a, b in x.items())
                         if p is not None:
                             p = sorted(p, key=sort_key_fn)
                         if c is not None:
@@ -2343,13 +2345,13 @@ def _get_expected_volumes(self, image):
                 container = None
                 if ':' in vol:
                     if len(vol.split(':')) == 3:
-                        host, container, mode = vol.split(':')
+                        dummy, container, mode = vol.split(':')
                         if not is_volume_permissions(mode):
                             self.fail('Found invalid volumes mode: {0}'.format(mode))
                     if len(vol.split(':')) == 2:
                         parts = vol.split(':')
                         if not is_volume_permissions(parts[1]):
-                            host, container, mode = vol.split(':') + ['rw']
+                            dummy, container, mode = vol.split(':') + ['rw']
                 new_vol = dict()
                 if container:
                     new_vol[container] = dict()
@@ -2740,7 +2742,7 @@ def container_start(self, container_id):
                     config = self.client.inspect_container(container_id)
                     logging_driver = config['HostConfig']['LogConfig']['Type']
 
-                    if logging_driver == 'json-file' or logging_driver == 'journald':
+                    if logging_driver in ('json-file', 'journald'):
                         output = self.client.logs(container_id, stdout=True, stderr=True, stream=False, timestamps=False)
                         if self.parameters.output_logs:
                             self._output_logs(msg=output)
@@ -2925,21 +2927,21 @@ def _parse_comparisons(self):
                 continue
             # Determine option type
             if option in explicit_types:
-                type = explicit_types[option]
+                datatype = explicit_types[option]
             elif data['type'] == 'list':
-                type = 'set'
+                datatype = 'set'
             elif data['type'] == 'dict':
-                type = 'dict'
+                datatype = 'dict'
             else:
-                type = 'value'
+                datatype = 'value'
             # Determine comparison type
             if option in default_values:
                 comparison = default_values[option]
-            elif type in ('list', 'value'):
+            elif datatype in ('list', 'value'):
                 comparison = 'strict'
             else:
                 comparison = 'allow_more_present'
-            comparisons[option] = dict(type=type, comparison=comparison, name=option)
+            comparisons[option] = dict(type=datatype, comparison=comparison, name=option)
             # Keep track of aliases
             comp_aliases[option] = option
             for alias in data.get('aliases', []):

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

@@ -547,7 +547,7 @@ def create_network(self):
             if not self.check_mode:
                 resp = self.client.create_network(self.parameters.name, **params)
                 self.client.report_warnings(resp, ['Warning'])
-                self.existing_network = self.client.get_network(id=resp['Id'])
+                self.existing_network = self.client.get_network(network_id=resp['Id'])
             self.results['actions'].append("Created network %s with driver %s" % (self.parameters.name, self.parameters.driver))
             self.results['changed'] = True