From 3290b8343cd905182a9be2f184294256c1fd1d0c Mon Sep 17 00:00:00 2001 From: Felix Fontein Date: Fri, 9 Aug 2019 23:50:11 +0200 Subject: [PATCH] docker: fix sanity errors (#60047) * Remove sanity check errors. * More linting. * Forgot to update places. * Remove choices which aren't provided in argspec. --- contrib/inventory/docker.py | 6 +- lib/ansible/module_utils/docker/common.py | 58 ++++++++------- lib/ansible/module_utils/docker/swarm.py | 4 ++ .../modules/cloud/docker/docker_container.py | 72 ++++++++++--------- .../modules/cloud/docker/docker_network.py | 2 +- .../cloud/docker/docker_swarm_service.py | 6 -- lib/ansible/plugins/doc_fragments/docker.py | 3 + test/sanity/ignore.txt | 19 ----- test/units/module_utils/docker/test_common.py | 3 + .../cloud/docker/test_docker_container.py | 3 + .../cloud/docker/test_docker_network.py | 4 ++ .../cloud/docker/test_docker_swarm_service.py | 3 + .../cloud/docker/test_docker_volume.py | 4 ++ 13 files changed, 98 insertions(+), 89 deletions(-) diff --git a/contrib/inventory/docker.py b/contrib/inventory/docker.py index 1b92b4c527..489c820a23 100755 --- a/contrib/inventory/docker.py +++ b/contrib/inventory/docker.py @@ -20,6 +20,10 @@ # along with Ansible. If not, see . # +from __future__ import (absolute_import, division, print_function) +__metaclass__ = type + + DOCUMENTATION = ''' Docker Inventory Script @@ -383,7 +387,7 @@ except ImportError as exc: # 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: diff --git a/lib/ansible/module_utils/docker/common.py b/lib/ansible/module_utils/docker/common.py index ccf7e5e5db..5b747952ec 100644 --- a/lib/ansible/module_utils/docker/common.py +++ b/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 . +from __future__ import (absolute_import, division, print_function) +__metaclass__ = type + + import os import platform import re @@ -554,16 +558,16 @@ class AnsibleDockerClient(Client): 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 @@ class AnsibleDockerClient(Client): 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,7 +606,7 @@ class AnsibleDockerClient(Client): 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/' as the name of images pulled from docker hub registry, repo_name = auth.resolve_repository_name(name) if registry == 'docker.io': @@ -610,12 +614,12 @@ class AnsibleDockerClient(Client): # 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 @@ class AnsibleDockerClient(Client): 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 @@ class AnsibleDockerClient(Client): 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 @@ class AnsibleDockerClient(Client): 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,7 +787,7 @@ 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: @@ -791,9 +795,9 @@ def compare_generic(a, b, method, type): # 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: diff --git a/lib/ansible/module_utils/docker/swarm.py b/lib/ansible/module_utils/docker/swarm.py index acbc1731a0..ccaafe3a79 100644 --- a/lib/ansible/module_utils/docker/swarm.py +++ 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 diff --git a/lib/ansible/modules/cloud/docker/docker_container.py b/lib/ansible/modules/cloud/docker/docker_container.py index 4139b40f08..3991c410f8 100644 --- a/lib/ansible/modules/cloud/docker/docker_container.py +++ b/lib/ansible/modules/cloud/docker/docker_container.py @@ -1064,8 +1064,8 @@ REQUIRES_CONVERSION_TO_BYTES = [ ] -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 @@ class TaskParameters(DockerBaseClass): 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 @@ class TaskParameters(DockerBaseClass): 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 @@ class TaskParameters(DockerBaseClass): 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 @@ class Container(DockerBaseClass): 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 @@ class Container(DockerBaseClass): 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 @@ class ContainerManager(DockerBaseClass): 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 @@ class AnsibleDockerClientContainer(AnsibleDockerClient): 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', []): diff --git a/lib/ansible/modules/cloud/docker/docker_network.py b/lib/ansible/modules/cloud/docker/docker_network.py index fbeebd1fe8..b0601a2fd0 100644 --- a/lib/ansible/modules/cloud/docker/docker_network.py +++ b/lib/ansible/modules/cloud/docker/docker_network.py @@ -547,7 +547,7 @@ class DockerNetworkManager(object): 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 diff --git a/lib/ansible/modules/cloud/docker/docker_swarm_service.py b/lib/ansible/modules/cloud/docker/docker_swarm_service.py index 9188fe2489..a64b269f76 100644 --- a/lib/ansible/modules/cloud/docker/docker_swarm_service.py +++ b/lib/ansible/modules/cloud/docker/docker_swarm_service.py @@ -583,9 +583,6 @@ options: - Corresponds to the C(--rollback-order) option of C(docker service create). - Requires API version >= 1.29. type: str - choices: - - stop-first - - start-first type: dict version_added: "2.8" secrets: @@ -697,9 +694,6 @@ options: - Corresponds to the C(--update-order) option of C(docker service create). - Requires API version >= 1.29. type: str - choices: - - stop-first - - start-first type: dict version_added: "2.8" update_delay: diff --git a/lib/ansible/plugins/doc_fragments/docker.py b/lib/ansible/plugins/doc_fragments/docker.py index d11848749e..d72f52e2db 100644 --- a/lib/ansible/plugins/doc_fragments/docker.py +++ b/lib/ansible/plugins/doc_fragments/docker.py @@ -2,6 +2,9 @@ # 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 + class ModuleDocFragment(object): diff --git a/test/sanity/ignore.txt b/test/sanity/ignore.txt index d2f6657c7f..fef978e815 100644 --- a/test/sanity/ignore.txt +++ b/test/sanity/ignore.txt @@ -20,8 +20,6 @@ contrib/inventory/consul_io.py future-import-boilerplate contrib/inventory/consul_io.py metaclass-boilerplate contrib/inventory/digital_ocean.py future-import-boilerplate contrib/inventory/digital_ocean.py metaclass-boilerplate -contrib/inventory/docker.py future-import-boilerplate -contrib/inventory/docker.py metaclass-boilerplate contrib/inventory/ec2.py future-import-boilerplate contrib/inventory/ec2.py metaclass-boilerplate contrib/inventory/fleet.py future-import-boilerplate @@ -226,10 +224,6 @@ lib/ansible/module_utils/distro/_distro.py future-import-boilerplate # ignore bu lib/ansible/module_utils/distro/_distro.py metaclass-boilerplate # ignore bundled lib/ansible/module_utils/distro/_distro.py no-assert lib/ansible/module_utils/distro/_distro.py pep8!skip # bundled code we don't want to modify -lib/ansible/module_utils/docker/common.py future-import-boilerplate -lib/ansible/module_utils/docker/common.py metaclass-boilerplate -lib/ansible/module_utils/docker/swarm.py future-import-boilerplate -lib/ansible/module_utils/docker/swarm.py metaclass-boilerplate lib/ansible/module_utils/ec2.py future-import-boilerplate lib/ansible/module_utils/ec2.py metaclass-boilerplate lib/ansible/module_utils/exoscale.py future-import-boilerplate @@ -1438,7 +1432,6 @@ lib/ansible/modules/cloud/dimensiondata/dimensiondata_vlan.py validate-modules:E lib/ansible/modules/cloud/dimensiondata/dimensiondata_vlan.py validate-modules:E337 lib/ansible/modules/cloud/dimensiondata/dimensiondata_vlan.py validate-modules:E338 lib/ansible/modules/cloud/docker/docker_container.py use-argspec-type-path # uses colon-separated paths, can't use type=path -lib/ansible/modules/cloud/docker/docker_swarm_service.py validate-modules:E326 lib/ansible/modules/cloud/google/_gcdns_record.py validate-modules:E337 lib/ansible/modules/cloud/google/_gcdns_zone.py validate-modules:E337 lib/ansible/modules/cloud/google/_gce.py pylint:blacklisted-name @@ -6461,8 +6454,6 @@ lib/ansible/plugins/doc_fragments/dimensiondata.py future-import-boilerplate lib/ansible/plugins/doc_fragments/dimensiondata.py metaclass-boilerplate lib/ansible/plugins/doc_fragments/dimensiondata_wait.py future-import-boilerplate lib/ansible/plugins/doc_fragments/dimensiondata_wait.py metaclass-boilerplate -lib/ansible/plugins/doc_fragments/docker.py future-import-boilerplate -lib/ansible/plugins/doc_fragments/docker.py metaclass-boilerplate lib/ansible/plugins/doc_fragments/ec2.py future-import-boilerplate lib/ansible/plugins/doc_fragments/ec2.py metaclass-boilerplate lib/ansible/plugins/doc_fragments/emc.py future-import-boilerplate @@ -6944,8 +6935,6 @@ test/units/module_utils/common/test_dict_transformations.py future-import-boiler test/units/module_utils/common/test_dict_transformations.py metaclass-boilerplate test/units/module_utils/conftest.py future-import-boilerplate test/units/module_utils/conftest.py metaclass-boilerplate -test/units/module_utils/docker/test_common.py future-import-boilerplate -test/units/module_utils/docker/test_common.py metaclass-boilerplate test/units/module_utils/ec2/test_aws.py future-import-boilerplate test/units/module_utils/ec2/test_aws.py metaclass-boilerplate test/units/module_utils/facts/base.py future-import-boilerplate @@ -7053,14 +7042,6 @@ test/units/modules/cloud/amazon/test_s3_bucket_notification.py future-import-boi test/units/modules/cloud/amazon/test_s3_bucket_notification.py metaclass-boilerplate test/units/modules/cloud/cloudstack/test_cs_traffic_type.py future-import-boilerplate test/units/modules/cloud/cloudstack/test_cs_traffic_type.py metaclass-boilerplate -test/units/modules/cloud/docker/test_docker_container.py future-import-boilerplate -test/units/modules/cloud/docker/test_docker_container.py metaclass-boilerplate -test/units/modules/cloud/docker/test_docker_network.py future-import-boilerplate -test/units/modules/cloud/docker/test_docker_network.py metaclass-boilerplate -test/units/modules/cloud/docker/test_docker_swarm_service.py future-import-boilerplate -test/units/modules/cloud/docker/test_docker_swarm_service.py metaclass-boilerplate -test/units/modules/cloud/docker/test_docker_volume.py future-import-boilerplate -test/units/modules/cloud/docker/test_docker_volume.py metaclass-boilerplate test/units/modules/cloud/google/test_gce_tag.py future-import-boilerplate test/units/modules/cloud/google/test_gce_tag.py metaclass-boilerplate test/units/modules/cloud/google/test_gcp_forwarding_rule.py future-import-boilerplate diff --git a/test/units/module_utils/docker/test_common.py b/test/units/module_utils/docker/test_common.py index f0bae1d7c2..785ecf5efe 100644 --- a/test/units/module_utils/docker/test_common.py +++ b/test/units/module_utils/docker/test_common.py @@ -1,3 +1,6 @@ +from __future__ import (absolute_import, division, print_function) +__metaclass__ = type + import pytest from ansible.module_utils.docker.common import ( diff --git a/test/units/modules/cloud/docker/test_docker_container.py b/test/units/modules/cloud/docker/test_docker_container.py index ae81714799..a648ae8042 100644 --- a/test/units/modules/cloud/docker/test_docker_container.py +++ b/test/units/modules/cloud/docker/test_docker_container.py @@ -1,3 +1,6 @@ +from __future__ import (absolute_import, division, print_function) +__metaclass__ = type + import unittest from ansible.modules.cloud.docker.docker_container import TaskParameters diff --git a/test/units/modules/cloud/docker/test_docker_network.py b/test/units/modules/cloud/docker/test_docker_network.py index 2c385a71d2..4d67565ea6 100644 --- a/test/units/modules/cloud/docker/test_docker_network.py +++ b/test/units/modules/cloud/docker/test_docker_network.py @@ -1,4 +1,8 @@ """Unit tests for docker_network.""" + +from __future__ import (absolute_import, division, print_function) +__metaclass__ = type + import pytest from ansible.modules.cloud.docker.docker_network import get_ip_version diff --git a/test/units/modules/cloud/docker/test_docker_swarm_service.py b/test/units/modules/cloud/docker/test_docker_swarm_service.py index 3e0823a6b7..668f2837ea 100644 --- a/test/units/modules/cloud/docker/test_docker_swarm_service.py +++ b/test/units/modules/cloud/docker/test_docker_swarm_service.py @@ -1,3 +1,6 @@ +from __future__ import (absolute_import, division, print_function) +__metaclass__ = type + import pytest diff --git a/test/units/modules/cloud/docker/test_docker_volume.py b/test/units/modules/cloud/docker/test_docker_volume.py index 9a1b96cf7b..e291da1237 100644 --- a/test/units/modules/cloud/docker/test_docker_volume.py +++ b/test/units/modules/cloud/docker/test_docker_volume.py @@ -1,5 +1,9 @@ # Copyright (c) 2018 Ansible Project # 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 import pytest