From 891687284faae66f52343bb921b001d089967915 Mon Sep 17 00:00:00 2001 From: Felix Fontein Date: Mon, 19 Nov 2018 10:59:54 +0100 Subject: docker_* modules: improve diff (#48546) * Add difference tracking tool * Improve --diff mode for docker_container. * Improve diffs of sets by ordering the sets. * Rewrite imports, get rid of HAS_DOCKER_PY_x variables and use docker_version instead. * Rename container -> active (more generic). * Add --diff for docker_volume. Change old diff output. * Add --diff for docker_network. Change old diff output. * Add --diff for docker_swarm_service. * Add changelog. * Add entry for porting guide on docker_network and docker_volume. --- changelogs/fragments/48546-docker-diff.yml | 7 ++ .../rst/porting_guides/porting_guide_2.8.rst | 4 + lib/ansible/module_utils/docker_common.py | 56 ++++++++++- .../modules/cloud/docker/docker_container.py | 70 +++++++++---- lib/ansible/modules/cloud/docker/docker_network.py | 85 ++++++++++------ .../modules/cloud/docker/docker_swarm_service.py | 108 ++++++++++++--------- lib/ansible/modules/cloud/docker/docker_volume.py | 38 ++++++-- .../targets/docker_container/tasks/main.yml | 2 + .../docker_container/tasks/tests/comparisons.yml | 6 ++ .../docker_container/tasks/tests/image-ids.yml | 1 + .../docker_container/tasks/tests/options.yml | 70 +++++++++++++ .../targets/docker_container/tasks/tests/ports.yml | 3 + .../targets/docker_network/tasks/tests/ipam.yml | 22 ++--- 13 files changed, 357 insertions(+), 115 deletions(-) create mode 100644 changelogs/fragments/48546-docker-diff.yml diff --git a/changelogs/fragments/48546-docker-diff.yml b/changelogs/fragments/48546-docker-diff.yml new file mode 100644 index 0000000000..2151b0febb --- /dev/null +++ b/changelogs/fragments/48546-docker-diff.yml @@ -0,0 +1,7 @@ +minor_changes: +- "docker_container - improved ``diff`` mode to show output." +- "docker_swarm_service - added ``diff`` mode." +- "docker_network - changed return value ``diff`` from ``list`` to ``dict``; the original list is contained in ``diff.differences``." +- "docker_network - improved ``diff`` mode to show output." +- "docker_volume - changed return value ``diff`` from ``list`` to ``dict``; the original list is contained in ``diff.differences``." +- "docker_volume - improved ``diff`` mode to show output." diff --git a/docs/docsite/rst/porting_guides/porting_guide_2.8.rst b/docs/docsite/rst/porting_guides/porting_guide_2.8.rst index bf31684ef8..fd56b239c9 100644 --- a/docs/docsite/rst/porting_guides/porting_guide_2.8.rst +++ b/docs/docsite/rst/porting_guides/porting_guide_2.8.rst @@ -110,6 +110,10 @@ Noteworthy module changes * The ``vmware_local_role_facts`` module now returns a list of dicts instead of a dict of dicts for role information. +* If ``docker_network`` or ``docker_volume`` were called with ``diff: yes``, ``check_mode: yes`` or ``debug: yes``, + a return value called ``diff`` was returned of type ``list``. To enable proper diff output, this was changed to + type ``dict``; the original ``list`` is returned as ``diff.differences``. + Plugins ======= diff --git a/lib/ansible/module_utils/docker_common.py b/lib/ansible/module_utils/docker_common.py index 32c1f20c04..d66c5b592b 100644 --- a/lib/ansible/module_utils/docker_common.py +++ b/lib/ansible/module_utils/docker_common.py @@ -20,7 +20,7 @@ import os import re from distutils.version import LooseVersion -from ansible.module_utils.basic import AnsibleModule, env_fallback +from ansible.module_utils.basic import AnsibleModule, env_fallback, jsonify from ansible.module_utils.six.moves.urllib.parse import urlparse from ansible.module_utils.parsing.convert_bool import BOOLEANS_TRUE, BOOLEANS_FALSE @@ -651,3 +651,57 @@ def compare_generic(a, b, method, type): if not found: return False return True + + +class DifferenceTracker(object): + def __init__(self): + self._diff = [] + + def add(self, name, parameter=None, active=None): + self._diff.append(dict( + name=name, + parameter=parameter, + active=active, + )) + + def merge(self, other_tracker): + self._diff.extend(other_tracker._diff) + + @property + def empty(self): + return len(self._diff) == 0 + + def get_before_after(self): + ''' + Return texts ``before`` and ``after``. + ''' + before = dict() + after = dict() + for item in self._diff: + before[item['name']] = item['active'] + after[item['name']] = item['parameter'] + return ( + jsonify(before, sort_keys=True, indent=2), + jsonify(after, sort_keys=True, indent=2), + ) + + def get_legacy_docker_container_diffs(self): + ''' + Return differences in the docker_container legacy format. + ''' + result = [] + for entry in self._diff: + item = dict() + item[entry['name']] = dict( + parameter=entry['parameter'], + container=entry['active'], + ) + result.append(item) + return result + + def get_legacy_docker_diffs(self): + ''' + Return differences in the docker_container legacy format. + ''' + result = [entry['name'] for entry in self._diff] + return result diff --git a/lib/ansible/modules/cloud/docker/docker_container.py b/lib/ansible/modules/cloud/docker/docker_container.py index 559aa47f9e..2b6c42dc33 100644 --- a/lib/ansible/modules/cloud/docker/docker_container.py +++ b/lib/ansible/modules/cloud/docker/docker_container.py @@ -868,7 +868,7 @@ from ansible.module_utils.basic import human_to_bytes from ansible.module_utils.docker_common import ( AnsibleDockerClient, DockerBaseClass, sanitize_result, is_image_name_id, - compare_generic, + compare_generic, DifferenceTracker, ) from ansible.module_utils.six import string_types @@ -1852,7 +1852,7 @@ class Container(DockerBaseClass): memory_swap=host_config.get('MemorySwap'), )) - differences = [] + differences = DifferenceTracker() for key, value in config_mapping.items(): compare = self.parameters.client.comparisons[self.parameters_map.get(key, key)] self.log('check differences %s %s vs %s (%s)' % (key, getattr(self.parameters, key), str(value), compare)) @@ -1861,14 +1861,24 @@ class Container(DockerBaseClass): if not match: # no match. record the differences - item = dict() - item[key] = dict( - parameter=getattr(self.parameters, key), - container=value - ) - differences.append(item) - - has_differences = True if len(differences) > 0 else False + p = getattr(self.parameters, key) + c = value + if compare['type'] == 'set': + # Since the order does not matter, sort so that the diff output is better. + if p is not None: + p = sorted(p) + if c is not None: + c = sorted(c) + elif compare['type'] == 'set(dict)': + # Since the order does not matter, sort so that the diff output is better. + # We sort the list of dictionaries by using the sorted items of a dict as its key. + if p is not None: + p = sorted(p, key=lambda x: sorted(x.items())) + if c is not None: + c = sorted(c, key=lambda x: sorted(x.items())) + differences.add(key, parameter=p, active=c) + + has_differences = not differences.empty return has_differences, differences def has_different_resource_limits(self): @@ -1896,7 +1906,7 @@ class Container(DockerBaseClass): memory_swap=host_config.get('MemorySwap'), ) - differences = [] + differences = DifferenceTracker() for key, value in config_mapping.items(): if getattr(self.parameters, key, None): compare = self.parameters.client.comparisons[self.parameters_map.get(key, key)] @@ -1904,13 +1914,8 @@ class Container(DockerBaseClass): if not match: # no match. record the differences - item = dict() - item[key] = dict( - parameter=getattr(self.parameters, key), - container=value - ) - differences.append(item) - different = (len(differences) > 0) + differences.add(key, parameter=getattr(self.parameters, key), active=value) + different = not differences.empty return different, differences def has_network_differences(self): @@ -2233,6 +2238,7 @@ class ContainerManager(DockerBaseClass): self.check_mode = self.client.check_mode self.results = {'changed': False, 'actions': []} self.diff = {} + self.diff_tracker = DifferenceTracker() self.facts = {} state = self.parameters.state @@ -2245,6 +2251,7 @@ class ContainerManager(DockerBaseClass): self.results.pop('actions') if self.client.module._diff or self.parameters.debug: + self.diff['before'], self.diff['after'] = self.diff_tracker.get_before_after() self.results['diff'] = self.diff if self.facts: @@ -2252,6 +2259,8 @@ class ContainerManager(DockerBaseClass): def present(self, state): container = self._get_container(self.parameters.name) + was_running = container.running + was_paused = container.paused # If the image parameter was passed then we need to deal with the image # version comparison. Otherwise we handle this depending on whether @@ -2265,6 +2274,7 @@ class ContainerManager(DockerBaseClass): self.log('No container found') if not self.parameters.image: self.fail('Cannot create container when image is not specified!') + self.diff_tracker.add('exists', parameter=True, active=False) new_container = self.container_create(self.parameters.image, self.parameters.create_parameters) if new_container: container = new_container @@ -2275,7 +2285,8 @@ class ContainerManager(DockerBaseClass): if self.parameters.comparisons['image']['comparison'] == 'strict': image_different = self._image_is_different(image, container) if image_different or different or self.parameters.recreate: - self.diff['differences'] = differences + self.diff_tracker.merge(differences) + self.diff['differences'] = differences.get_legacy_docker_container_diffs() if image_different: self.diff['image_different'] = True self.log("differences") @@ -2297,15 +2308,19 @@ class ContainerManager(DockerBaseClass): container = self.update_networks(container) if state == 'started' and not container.running: + self.diff_tracker.add('running', parameter=True, active=was_running) container = self.container_start(container.Id) elif state == 'started' and self.parameters.restart: + self.diff_tracker.add('running', parameter=True, active=was_running) self.container_stop(container.Id) container = self.container_start(container.Id) elif state == 'stopped' and container.running: + self.diff_tracker.add('running', parameter=False, active=was_running) self.container_stop(container.Id) container = self._get_container(container.Id) if state == 'started' and container.paused != self.parameters.paused: + self.diff_tracker.add('paused', parameter=self.parameters.paused, active=was_paused) if not self.check_mode: try: if self.parameters.paused: @@ -2326,7 +2341,9 @@ class ContainerManager(DockerBaseClass): container = self._get_container(self.parameters.name) if container.exists: if container.running: + self.diff_tracker.add('running', parameter=False, active=True) self.container_stop(container.Id) + self.diff_tracker.add('exists', parameter=False, active=True) self.container_remove(container.Id) def fail(self, msg, **kwargs): @@ -2369,6 +2386,7 @@ class ContainerManager(DockerBaseClass): if image and image.get('Id'): if container and container.Image: if image.get('Id') != container.Image: + self.diff_tracker.add('image', parameter=image.get('Id'), active=container.Image) return True return False @@ -2376,7 +2394,8 @@ class ContainerManager(DockerBaseClass): limits_differ, different_limits = container.has_different_resource_limits() if limits_differ: self.log("limit differences:") - self.log(different_limits, pretty_print=True) + self.log(different_limits.get_legacy_docker_container_diffs(), pretty_print=True) + self.diff_tracker.merge(different_limits) if limits_differ and not self.check_mode: self.container_update(container.Id, self.parameters.update_parameters) return self._get_container(container.Id) @@ -2390,6 +2409,12 @@ class ContainerManager(DockerBaseClass): self.diff['differences'].append(dict(network_differences=network_differences)) else: self.diff['differences'] = [dict(network_differences=network_differences)] + for netdiff in network_differences: + self.diff_tracker.add( + 'network.{0}'.format(netdiff['parameter']['name']), + parameter=netdiff['parameter'], + active=netdiff['container'] + ) self.results['changed'] = True updated_container = self._add_networks(container, network_differences) @@ -2400,6 +2425,11 @@ class ContainerManager(DockerBaseClass): self.diff['differences'].append(dict(purge_networks=extra_networks)) else: self.diff['differences'] = [dict(purge_networks=extra_networks)] + for extra_network in extra_networks: + self.diff_tracker.add( + 'network.{0}'.format(extra_network['name']), + active=extra_network + ) self.results['changed'] = True updated_container = self._purge_networks(container, extra_networks) return updated_container diff --git a/lib/ansible/modules/cloud/docker/docker_network.py b/lib/ansible/modules/cloud/docker/docker_network.py index e8eef59109..1c245bb147 100644 --- a/lib/ansible/modules/cloud/docker/docker_network.py +++ b/lib/ansible/modules/cloud/docker/docker_network.py @@ -228,12 +228,19 @@ facts: import re -from ansible.module_utils.docker_common import AnsibleDockerClient, DockerBaseClass, HAS_DOCKER_PY_2, HAS_DOCKER_PY_3 +from distutils.version import LooseVersion + +from ansible.module_utils.docker_common import ( + AnsibleDockerClient, + DockerBaseClass, + docker_version, + DifferenceTracker, +) try: from docker import utils from docker.errors import NotFound - if HAS_DOCKER_PY_2 or HAS_DOCKER_PY_3: + if LooseVersion(docker_version) >= LooseVersion('2.0.0'): from docker.types import IPAMPool, IPAMConfig except Exception as dummy: # missing docker-py handled in ansible.module_utils.docker_common @@ -312,6 +319,8 @@ class DockerNetworkManager(object): u'actions': [] } self.diff = self.client.module._diff + self.diff_tracker = DifferenceTracker() + self.diff_result = dict() self.existing_network = self.get_existing_network() @@ -331,6 +340,11 @@ class DockerNetworkManager(object): elif state == 'absent': self.absent() + if self.diff or self.check_mode or self.parameters.debug: + if self.diff: + self.diff_result['before'], self.diff_result['after'] = self.diff_tracker.get_before_after() + self.results['diff'] = self.diff_result + def get_existing_network(self): try: return self.client.inspect_network(self.parameters.network_name) @@ -345,30 +359,34 @@ class DockerNetworkManager(object): :param net: the inspection output for an existing network :return: (bool, list) ''' - different = False - differences = [] + differences = DifferenceTracker() if self.parameters.driver and self.parameters.driver != net['Driver']: - different = True - differences.append('driver') + differences.add('driver', + parameter=self.parameters.driver, + active=net['Driver']) if self.parameters.driver_options: if not net.get('Options'): - different = True - differences.append('driver_options') + differences.add('driver_options', + parameter=self.parameters.driver_options, + active=net.get('Options')) else: for key, value in self.parameters.driver_options.items(): if not (key in net['Options']) or value != net['Options'][key]: - different = True - differences.append('driver_options.%s' % key) + differences.add('driver_options.%s' % key, + parameter=value, + active=net['Options'].get(key)) if self.parameters.ipam_driver: if not net.get('IPAM') or net['IPAM']['Driver'] != self.parameters.ipam_driver: - different = True - differences.append('ipam_driver') + differences.add('ipam_driver', + parameter=self.parameters.ipam_driver, + active=net.get('IPAM')) if self.parameters.ipam_config is not None and self.parameters.ipam_config: if not net.get('IPAM') or not net['IPAM']['Config']: - different = True - differences.append('ipam_config') + differences.add('ipam_config', + parameter=self.parameters.ipam_config, + active=net.get('IPAM', {}).get('Config')) else: for idx, ipam_config in enumerate(self.parameters.ipam_config): net_config = dict() @@ -391,23 +409,27 @@ class DockerNetworkManager(object): camelkey = net_key break if not camelkey or net_config.get(camelkey) != value: - different = True - differences.append('ipam_config[%s].%s' % (idx, key)) + differences.add('ipam_config[%s].%s' % (idx, key), + parameter=value, + active=net_config.get(camelkey) if camelkey else None) if self.parameters.enable_ipv6 is not None and self.parameters.enable_ipv6 != net.get('EnableIPv6', False): - different = True - differences.append('enable_ipv6') + differences.add('enable_ipv6', + parameter=self.parameters.enable_ipv6, + active=net.get('EnableIPv6', False)) if self.parameters.internal is not None: if self.parameters.internal: if not net.get('Internal'): - different = True - differences.append('internal') + differences.add('internal', + parameter=self.parameters.internal, + active=net.get('Internal')) else: if net.get('Internal'): - different = True - differences.append('internal') - return different, differences + differences.add('internal', + parameter=self.parameters.internal, + active=net.get('Internal')) + return not differences.empty, differences def create_network(self): if not self.existing_network: @@ -419,7 +441,7 @@ class DockerNetworkManager(object): ipam_pools = [] if self.parameters.ipam_config: for ipam_pool in self.parameters.ipam_config: - if HAS_DOCKER_PY_2 or HAS_DOCKER_PY_3: + if LooseVersion(docker_version) >= LooseVersion('2.0.0'): ipam_pools.append(IPAMPool(**ipam_pool)) else: ipam_pools.append(utils.create_ipam_pool(**ipam_pool)) @@ -429,7 +451,7 @@ class DockerNetworkManager(object): # were specified. Leaving this parameter away can significantly speed up # creation; on my machine creation with this option needs ~15 seconds, # and without just a few seconds. - if HAS_DOCKER_PY_2 or HAS_DOCKER_PY_3: + if LooseVersion(docker_version) >= LooseVersion('2.0.0'): params['ipam'] = IPAMConfig(driver=self.parameters.ipam_driver, pool_configs=ipam_pools) else: @@ -466,6 +488,9 @@ class DockerNetworkManager(object): self.client.connect_container_to_network(name, self.parameters.network_name) self.results['actions'].append("Connected container %s" % (name,)) self.results['changed'] = True + self.diff_tracker.add('connected.{0}'.format(name), + parameter=True, + active=False) def disconnect_missing(self): if not self.existing_network: @@ -490,13 +515,17 @@ class DockerNetworkManager(object): self.client.disconnect_container_from_network(container_name, self.parameters.network_name) self.results['actions'].append("Disconnected container %s" % (container_name,)) self.results['changed'] = True + self.diff_tracker.add('connected.{0}'.format(container_name), + parameter=False, + active=True) def present(self): different = False - differences = [] + differences = DifferenceTracker() if self.existing_network: different, differences = self.has_different_config(self.existing_network) + self.diff_tracker.add('exists', parameter=True, active=self.existing_network is not None) if self.parameters.force or different: self.remove_network() self.existing_network = None @@ -507,7 +536,8 @@ class DockerNetworkManager(object): self.disconnect_missing() if self.diff or self.check_mode or self.parameters.debug: - self.results['diff'] = differences + self.diff_result['differences'] = differences.get_legacy_docker_diffs() + self.diff_tracker.merge(differences) if not self.check_mode and not self.parameters.debug: self.results.pop('actions') @@ -515,6 +545,7 @@ class DockerNetworkManager(object): self.results['ansible_facts'] = {u'docker_network': self.get_existing_network()} def absent(self): + self.diff_tracker.add('exists', parameter=False, active=self.existing_network is not None) self.remove_network() diff --git a/lib/ansible/modules/cloud/docker/docker_swarm_service.py b/lib/ansible/modules/cloud/docker/docker_swarm_service.py index f28ce760d8..e7d37cd8ae 100644 --- a/lib/ansible/modules/cloud/docker/docker_swarm_service.py +++ b/lib/ansible/modules/cloud/docker/docker_swarm_service.py @@ -455,16 +455,18 @@ EXAMPLES = ''' ''' import time -from ansible.module_utils.docker_common import DockerBaseClass -from ansible.module_utils.docker_common import AnsibleDockerClient -from ansible.module_utils.docker_common import docker_version +from ansible.module_utils.docker_common import ( + DockerBaseClass, + AnsibleDockerClient, + docker_version, + DifferenceTracker, +) from ansible.module_utils.basic import human_to_bytes from ansible.module_utils._text import to_text try: from distutils.version import LooseVersion - from docker import utils from docker import types except Exception as dummy: # missing docker-py handled in ansible.module_utils.docker @@ -653,87 +655,86 @@ class DockerService(DockerBaseClass): return s def compare(self, os): - differences = [] + differences = DifferenceTracker() needs_rebuild = False force_update = False if self.endpoint_mode != os.endpoint_mode: - differences.append('endpoint_mode') + differences.add('endpoint_mode', parameter=self.endpoint_mode, active=os.endpoint_mode) if self.env != os.env: - differences.append('env') + differences.add('env', parameter=self.env, active=os.env) if self.log_driver != os.log_driver: - differences.append('log_driver') + differences.add('log_driver', parameter=self.log_driver, active=os.log_driver) if self.log_driver_options != os.log_driver_options: - differences.append('log_opt') + differences.add('log_opt', parameter=self.log_driver_options, active=os.log_driver_options) if self.mode != os.mode: needs_rebuild = True - differences.append('mode') + differences.add('mode', parameter=self.mode, active=os.mode) if self.mounts != os.mounts: - differences.append('mounts') + differences.add('mounts', parameter=self.mounts, active=os.mounts) if self.configs != os.configs: - differences.append('configs') + differences.add('configs', parameter=self.configs, active=os.configs) if self.secrets != os.secrets: - differences.append('secrets') + differences.add('secrets', parameter=self.secrets, active=os.secrets) if self.networks != os.networks: - differences.append('networks') + differences.add('networks', parameter=self.networks, active=os.networks) needs_rebuild = True if self.replicas != os.replicas: - differences.append('replicas') + differences.add('replicas', parameter=self.replicas, active=os.replicas) if self.args != os.args: - differences.append('args') + differences.add('args', parameter=self.args, active=os.args) if self.constraints != os.constraints: - differences.append('constraints') + differences.add('constraints', parameter=self.constraints, active=os.constraints) if self.labels != os.labels: - differences.append('labels') + differences.add('labels', parameter=self.labels, active=os.labels) if self.limit_cpu != os.limit_cpu: - differences.append('limit_cpu') + differences.add('limit_cpu', parameter=self.limit_cpu, active=os.limit_cpu) if self.limit_memory != os.limit_memory: - differences.append('limit_memory') + differences.add('limit_memory', parameter=self.limit_memory, active=os.limit_memory) if self.reserve_cpu != os.reserve_cpu: - differences.append('reserve_cpu') + differences.add('reserve_cpu', parameter=self.reserve_cpu, active=os.reserve_cpu) if self.reserve_memory != os.reserve_memory: - differences.append('reserve_memory') + differences.add('reserve_memory', parameter=self.reserve_memory, active=os.reserve_memory) if self.container_labels != os.container_labels: - differences.append('container_labels') + differences.add('container_labels', parameter=self.container_labels, active=os.container_labels) if self.publish != os.publish: - differences.append('publish') + differences.add('publish', parameter=self.publish, active=os.publish) if self.restart_policy != os.restart_policy: - differences.append('restart_policy') + differences.add('restart_policy', parameter=self.restart_policy, active=os.restart_policy) if self.restart_policy_attempts != os.restart_policy_attempts: - differences.append('restart_policy_attempts') + differences.add('restart_policy_attempts', parameter=self.restart_policy_attempts, active=os.restart_policy_attempts) if self.restart_policy_delay != os.restart_policy_delay: - differences.append('restart_policy_delay') + differences.add('restart_policy_delay', parameter=self.restart_policy_delay, active=os.restart_policy_delay) if self.restart_policy_window != os.restart_policy_window: - differences.append('restart_policy_window') + differences.add('restart_policy_window', parameter=self.restart_policy_window, active=os.restart_policy_window) if self.update_delay != os.update_delay: - differences.append('update_delay') + differences.add('update_delay', parameter=self.update_delay, active=os.update_delay) if self.update_parallelism != os.update_parallelism: - differences.append('update_parallelism') + differences.add('update_parallelism', parameter=self.update_parallelism, active=os.update_parallelism) if self.update_failure_action != os.update_failure_action: - differences.append('update_failure_action') + differences.add('update_failure_action', parameter=self.update_failure_action, active=os.update_failure_action) if self.update_monitor != os.update_monitor: - differences.append('update_monitor') + differences.add('update_monitor', parameter=self.update_monitor, active=os.update_monitor) if self.update_max_failure_ratio != os.update_max_failure_ratio: - differences.append('update_max_failure_ratio') + differences.add('update_max_failure_ratio', parameter=self.update_max_failure_ratio, active=os.update_max_failure_ratio) if self.update_order != os.update_order: - differences.append('update_order') + differences.add('update_order', parameter=self.update_order, active=os.update_order) if self.image != os.image.split('@')[0]: - differences.append('image') + differences.add('image', parameter=self.image, active=os.image.split('@')[0]) if self.user != os.user: - differences.append('user') + differences.add('user', parameter=self.user, active=os.user) if self.dns != os.dns: - differences.append('dns') + differences.add('dns', parameter=self.dns, active=os.dns) if self.dns_search != os.dns_search: - differences.append('dns_search') + differences.add('dns_search', parameter=self.dns_search, active=os.dns_search) if self.dns_options != os.dns_options: - differences.append('dns_options') + differences.add('dns_options', parameter=self.dns_options, active=os.dns_options) if self.hostname != os.hostname: - differences.append('hostname') + differences.add('hostname', parameter=self.hostname, active=os.hostname) if self.tty != os.tty: - differences.append('tty') + differences.add('tty', parameter=self.tty, active=os.tty) if self.force_update: - differences.append('force_update') force_update = True - return len(differences) > 0, differences, needs_rebuild, force_update + return not differences.empty or force_update, differences, needs_rebuild, force_update def __str__(self): return str({ @@ -1020,6 +1021,7 @@ class DockerServiceManager(): def __init__(self, client): self.client = client + self.diff_tracker = DifferenceTracker() def test_parameter_versions(self): parameters_versions = [ @@ -1067,7 +1069,7 @@ class DockerServiceManager(): changed = False msg = 'noop' rebuilt = False - changes = [] + differences = DifferenceTracker() facts = {} if current_service: @@ -1077,8 +1079,9 @@ class DockerServiceManager(): msg = 'Service removed' changed = True else: - changed, changes, need_rebuild, force_update = new_service.compare(current_service) + changed, differences, need_rebuild, force_update = new_service.compare(current_service) if changed: + self.diff_tracker.merge(differences) if need_rebuild: if not module.check_mode: self.remove_service(module.params['name']) @@ -1116,7 +1119,7 @@ class DockerServiceManager(): changed = True facts = new_service.get_facts() - return msg, changed, rebuilt, changes, facts + return msg, changed, rebuilt, differences.get_legacy_docker_diffs(), facts def main(): @@ -1173,7 +1176,18 @@ def main(): dsm = DockerServiceManager(client) msg, changed, rebuilt, changes, facts = dsm.run() - client.module.exit_json(msg=msg, changed=changed, rebuilt=rebuilt, changes=changes, ansible_docker_service=facts) + results = dict( + msg=msg, + changed=changed, + rebuilt=rebuilt, + changes=changes, + ansible_docker_service=facts, + ) + if client.module._diff: + before, after = dsm.diff_tracker.get_before_after() + results['diff'] = dict(before=before, after=after) + + client.module.exit_json(**results) if __name__ == '__main__': diff --git a/lib/ansible/modules/cloud/docker/docker_volume.py b/lib/ansible/modules/cloud/docker/docker_volume.py index a39d9cf522..bf99334ca6 100644 --- a/lib/ansible/modules/cloud/docker/docker_volume.py +++ b/lib/ansible/modules/cloud/docker/docker_volume.py @@ -115,7 +115,11 @@ except ImportError: # missing docker-py handled in ansible.module_utils.docker_common pass -from ansible.module_utils.docker_common import DockerBaseClass, AnsibleDockerClient +from ansible.module_utils.docker_common import ( + DockerBaseClass, + AnsibleDockerClient, + DifferenceTracker, +) from ansible.module_utils.six import iteritems, text_type @@ -146,6 +150,8 @@ class DockerVolumeManager(object): u'actions': [] } self.diff = self.client.module._diff + self.diff_tracker = DifferenceTracker() + self.diff_result = dict() self.existing_volume = self.get_existing_volume() @@ -155,6 +161,11 @@ class DockerVolumeManager(object): elif state == 'absent': self.absent() + if self.diff or self.check_mode or self.parameters.debug: + if self.diff: + self.diff_result['before'], self.diff_result['after'] = self.diff_tracker.get_before_after() + self.results['diff'] = self.diff_result + def get_existing_volume(self): try: volumes = self.client.volumes() @@ -176,22 +187,28 @@ class DockerVolumeManager(object): :return: list of options that differ """ - differences = [] + differences = DifferenceTracker() if self.parameters.driver and self.parameters.driver != self.existing_volume['Driver']: - differences.append('driver') + differences.add('driver', parameter=self.parameters.driver, active=self.existing_volume['Driver']) if self.parameters.driver_options: if not self.existing_volume.get('Options'): - differences.append('driver_options') + differences.add('driver_options', + parameter=self.parameters.driver_options, + active=self.existing_volume.get('Options')) else: for key, value in iteritems(self.parameters.driver_options): if (not self.existing_volume['Options'].get(key) or value != self.existing_volume['Options'][key]): - differences.append('driver_options.%s' % key) + differences.add('driver_options.%s' % key, + parameter=value, + active=self.existing_volume['Options'].get(key)) if self.parameters.labels: existing_labels = self.existing_volume.get('Labels', {}) for label in self.parameters.labels: if existing_labels.get(label) != self.parameters.labels.get(label): - differences.append('labels.%s' % label) + differences.add('labels.%s' % label, + parameter=self.parameters.labels.get(label), + active=existing_labels.get(label)) return differences @@ -222,18 +239,20 @@ class DockerVolumeManager(object): self.results['changed'] = True def present(self): - differences = [] + differences = DifferenceTracker() if self.existing_volume: differences = self.has_different_config() - if differences or self.parameters.force: + self.diff_tracker.add('exists', parameter=True, active=self.existing_volume is not None) + if not differences.empty or self.parameters.force: self.remove_volume() self.existing_volume = None self.create_volume() if self.diff or self.check_mode or self.parameters.debug: - self.results['diff'] = differences + self.diff_result['differences'] = differences.get_legacy_docker_diffs() + self.diff_tracker.merge(differences) if not self.check_mode and not self.parameters.debug: self.results.pop('actions') @@ -241,6 +260,7 @@ class DockerVolumeManager(object): self.results['ansible_facts'] = {u'docker_volume': self.get_existing_volume()} def absent(self): + self.diff_tracker.add('exists', parameter=False, active=self.existing_volume is not None) self.remove_volume() diff --git a/test/integration/targets/docker_container/tasks/main.yml b/test/integration/targets/docker_container/tasks/main.yml index 04fb166b6c..051377dd7f 100644 --- a/test/integration/targets/docker_container/tasks/main.yml +++ b/test/integration/targets/docker_container/tasks/main.yml @@ -22,6 +22,7 @@ state: absent force_kill: yes with_items: "{{ cnames }}" + diff: no - name: "Make sure all networks are removed" docker_network: name: "{{ item }}" @@ -29,5 +30,6 @@ force: yes with_items: "{{ dnetworks }}" when: docker_py_version is version('1.10.0', '>=') + diff: no when: docker_py_version is version('1.8.0', '>=') and docker_api_version is version('1.20', '>=') diff --git a/test/integration/targets/docker_container/tasks/tests/comparisons.yml b/test/integration/targets/docker_container/tasks/tests/comparisons.yml index 92d8c74dd6..d5ee8d5458 100644 --- a/test/integration/targets/docker_container/tasks/tests/comparisons.yml +++ b/test/integration/targets/docker_container/tasks/tests/comparisons.yml @@ -48,6 +48,7 @@ name: "{{ cname }}" state: absent force_kill: yes + diff: no - assert: that: @@ -101,6 +102,7 @@ name: "{{ cname }}" state: absent force_kill: yes + diff: no - assert: that: @@ -186,6 +188,7 @@ name: "{{ cname }}" state: absent force_kill: yes + diff: no - assert: that: @@ -273,6 +276,7 @@ name: "{{ cname }}" state: absent force_kill: yes + diff: no - assert: that: @@ -360,6 +364,7 @@ name: "{{ cname }}" state: absent force_kill: yes + diff: no - assert: that: @@ -448,6 +453,7 @@ name: "{{ cname }}" state: absent force_kill: yes + diff: no - assert: that: diff --git a/test/integration/targets/docker_container/tasks/tests/image-ids.yml b/test/integration/targets/docker_container/tasks/tests/image-ids.yml index 402c69521d..6201f59ae9 100644 --- a/test/integration/targets/docker_container/tasks/tests/image-ids.yml +++ b/test/integration/targets/docker_container/tasks/tests/image-ids.yml @@ -66,6 +66,7 @@ name: "{{ cname }}" state: absent force_kill: yes + diff: no - assert: that: diff --git a/test/integration/targets/docker_container/tasks/tests/options.yml b/test/integration/targets/docker_container/tasks/tests/options.yml index 2e794f843f..63013ad442 100644 --- a/test/integration/targets/docker_container/tasks/tests/options.yml +++ b/test/integration/targets/docker_container/tasks/tests/options.yml @@ -96,6 +96,7 @@ name: "{{ cname }}" state: absent force_kill: yes + diff: no - assert: that: @@ -160,6 +161,7 @@ name: "{{ cname }}" state: absent force_kill: yes + diff: no - assert: that: @@ -202,6 +204,7 @@ name: "{{ cname }}" state: absent force_kill: yes + diff: no - assert: that: @@ -246,6 +249,7 @@ name: "{{ cname }}" state: absent force_kill: yes + diff: no - assert: that: @@ -290,6 +294,7 @@ name: "{{ cname }}" state: absent force_kill: yes + diff: no - assert: that: @@ -334,6 +339,7 @@ name: "{{ cname }}" state: absent force_kill: yes + diff: no - assert: that: @@ -381,6 +387,7 @@ name: "{{ cname }}" state: absent force_kill: yes + diff: no - assert: that: @@ -428,6 +435,7 @@ name: "{{ cname }}" state: absent force_kill: yes + diff: no - assert: that: @@ -495,6 +503,7 @@ name: "{{ cname }}" state: absent register: detach_no_cleanup_cleanup + diff: no - name: detach with cleanup docker_container: @@ -509,6 +518,7 @@ name: "{{ cname }}" state: absent register: detach_cleanup_cleanup + diff: no - name: detach with auto_remove and cleanup docker_container: @@ -525,6 +535,7 @@ name: "{{ cname }}" state: absent register: detach_auto_remove_cleanup + diff: no - assert: that: @@ -597,6 +608,7 @@ name: "{{ cname }}" state: absent force_kill: yes + diff: no - assert: that: @@ -669,6 +681,7 @@ name: "{{ cname }}" state: absent force_kill: yes + diff: no - assert: that: @@ -747,6 +760,7 @@ name: "{{ cname }}" state: absent force_kill: yes + diff: no - assert: that: @@ -816,6 +830,7 @@ name: "{{ cname }}" state: absent force_kill: yes + diff: no - assert: that: @@ -886,6 +901,7 @@ name: "{{ cname }}" state: absent force_kill: yes + diff: no - assert: that: @@ -954,6 +970,7 @@ name: "{{ cname }}" state: absent force_kill: yes + diff: no - assert: that: @@ -1016,6 +1033,7 @@ name: "{{ cname }}" state: absent force_kill: yes + diff: no - assert: that: @@ -1061,6 +1079,7 @@ name: "{{ cname }}" state: absent force_kill: yes + diff: no - assert: that: @@ -1137,6 +1156,7 @@ name: "{{ cname }}" state: absent force_kill: yes + diff: no - assert: that: @@ -1199,6 +1219,7 @@ name: "{{ cname }}" state: absent force_kill: yes + diff: no - assert: that: @@ -1234,6 +1255,7 @@ name: "{{ cname }}" state: absent force_kill: yes + diff: no - assert: that: @@ -1293,6 +1315,7 @@ name: "{{ cname }}" state: absent force_kill: yes + diff: no - assert: that: @@ -1354,6 +1377,7 @@ name: "{{ cname }}" state: absent force_kill: yes + diff: no - assert: that: @@ -1421,6 +1445,7 @@ name: "{{ cname }}" state: absent force_kill: yes + diff: no - assert: that: @@ -1552,6 +1577,7 @@ name: "{{ cname }}" state: absent force_kill: yes + diff: no - assert: that: @@ -1607,6 +1633,7 @@ name: "{{ cname }}" state: absent force_kill: yes + diff: no - assert: that: @@ -1654,6 +1681,7 @@ name: "{{ cname }}" state: absent force_kill: yes + diff: no - assert: that: @@ -1704,6 +1732,7 @@ name: "{{ cname }}" state: absent force_kill: yes + diff: no - assert: that: @@ -1758,6 +1787,7 @@ name: "{{ cname }}" state: absent force_kill: yes + diff: no - assert: that: @@ -1821,6 +1851,7 @@ - "{{ cname_h1 }}" loop_control: loop_var: container_name + diff: no - assert: that: @@ -1865,6 +1896,7 @@ name: "{{ cname }}" state: absent force_kill: yes + diff: no - assert: that: @@ -1931,6 +1963,7 @@ name: "{{ cname }}" state: absent force_kill: yes + diff: no - assert: that: @@ -2012,6 +2045,7 @@ - "{{ cname_h3 }}" loop_control: loop_var: container_name + diff: no - assert: that: @@ -2057,6 +2091,7 @@ name: "{{ cname }}" state: absent force_kill: yes + diff: no - assert: that: @@ -2121,6 +2156,7 @@ name: "{{ cname }}" state: absent force_kill: yes + diff: no - assert: that: @@ -2166,6 +2202,7 @@ name: "{{ cname }}" state: absent force_kill: yes + diff: no - assert: that: @@ -2210,6 +2247,7 @@ name: "{{ cname }}" state: absent force_kill: yes + diff: no - assert: that: @@ -2254,6 +2292,7 @@ name: "{{ cname }}" state: absent force_kill: yes + diff: no - assert: that: @@ -2307,6 +2346,7 @@ name: "{{ cname }}" state: absent force_kill: yes + diff: no - assert: that: @@ -2360,6 +2400,7 @@ name: "{{ cname }}" state: absent force_kill: yes + diff: no - assert: that: @@ -2404,6 +2445,7 @@ name: "{{ cname }}" state: absent force_kill: yes + diff: no - assert: that: @@ -2479,6 +2521,7 @@ name: "{{ cname }}" state: absent force_kill: yes + diff: no - assert: that: @@ -2530,6 +2573,7 @@ name: "{{ cname }}" state: absent force_kill: yes + diff: no - assert: that: @@ -2583,6 +2627,7 @@ name: "{{ cname }}" state: absent force_kill: yes + diff: no - assert: that: @@ -2649,6 +2694,7 @@ name: "{{ cname }}" state: absent force_kill: yes + diff: no - assert: that: @@ -2712,6 +2758,7 @@ - "{{ cname_h1 }}" loop_control: loop_var: container_name + diff: no - assert: that: @@ -2763,6 +2810,7 @@ name: "{{ cname }}" state: absent force_kill: yes + diff: no - assert: that: @@ -2823,6 +2871,7 @@ name: "{{ cname }}" state: absent force_kill: yes + diff: no - assert: that: @@ -2874,6 +2923,7 @@ name: "{{ cname }}" state: absent force_kill: yes + diff: no - assert: that: @@ -2928,6 +2978,7 @@ name: "{{ cname }}" state: absent force_kill: yes + diff: no - debug: var=recreate_1 - debug: var=recreate_2 @@ -2972,6 +3023,7 @@ name: "{{ cname }}" state: absent force_kill: yes + diff: no - debug: var=restart_1 - debug: var=restart_2 @@ -3019,6 +3071,7 @@ name: "{{ cname }}" state: absent force_kill: yes + diff: no - assert: that: @@ -3066,6 +3119,7 @@ name: "{{ cname }}" state: absent force_kill: yes + diff: no - assert: that: @@ -3102,6 +3156,7 @@ name: "{{ cname }}" state: absent force_kill: yes + diff: no - assert: that: @@ -3183,6 +3238,7 @@ name: "{{ cname }}" state: absent force_kill: yes + diff: no - assert: that: @@ -3228,6 +3284,7 @@ name: "{{ cname }}" state: absent force_kill: yes + diff: no - assert: that: @@ -3272,6 +3329,7 @@ name: "{{ cname }}" state: absent force_kill: yes + diff: no - assert: that: @@ -3316,6 +3374,7 @@ name: "{{ cname }}" state: absent force_kill: yes + diff: no - assert: that: @@ -3390,6 +3449,7 @@ name: "{{ cname }}" state: absent force_kill: yes + diff: no - assert: that: @@ -3457,6 +3517,7 @@ name: "{{ cname }}" state: absent force_kill: yes + diff: no - assert: that: @@ -3508,6 +3569,7 @@ name: "{{ cname }}" state: absent force_kill: yes + diff: no - assert: that: @@ -3568,6 +3630,7 @@ name: "{{ cname }}" state: absent force_kill: yes + diff: no - assert: that: @@ -3613,6 +3676,7 @@ name: "{{ cname }}" state: absent force_kill: yes + diff: no - assert: that: @@ -3660,6 +3724,7 @@ name: "{{ cname }}" state: absent force_kill: yes + diff: no - assert: that: @@ -3713,6 +3778,7 @@ name: "{{ cname }}" state: absent force_kill: yes + diff: no - assert: that: @@ -3769,6 +3835,7 @@ name: "{{ cname }}" state: absent force_kill: yes + diff: no - assert: that: @@ -3841,6 +3908,7 @@ name: "{{ cname }}" state: absent force_kill: yes + diff: no - assert: that: @@ -3907,6 +3975,7 @@ - "{{ cname_h2 }}" loop_control: loop_var: container_name + diff: no - assert: that: @@ -3951,6 +4020,7 @@ name: "{{ cname }}" state: absent force_kill: yes + diff: no - assert: that: diff --git a/test/integration/targets/docker_container/tasks/tests/ports.yml b/test/integration/targets/docker_container/tasks/tests/ports.yml index 5a3eaf5197..ac5553c617 100644 --- a/test/integration/targets/docker_container/tasks/tests/ports.yml +++ b/test/integration/targets/docker_container/tasks/tests/ports.yml @@ -84,6 +84,7 @@ name: "{{ cname }}" state: absent force_kill: yes + diff: no - assert: that: @@ -147,6 +148,7 @@ name: "{{ cname }}" state: absent force_kill: yes + diff: no - assert: that: @@ -208,6 +210,7 @@ name: "{{ cname }}" state: absent force_kill: yes + diff: no - assert: that: diff --git a/test/integration/targets/docker_network/tasks/tests/ipam.yml b/test/integration/targets/docker_network/tasks/tests/ipam.yml index 993635f770..9b0b7b5288 100644 --- a/test/integration/targets/docker_network/tasks/tests/ipam.yml +++ b/test/integration/targets/docker_network/tasks/tests/ipam.yml @@ -50,8 +50,8 @@ - assert: that: - network is changed - - network['diff'] | length == 1 - - network['diff'][0] == "ipam_config[0].subnet" + - network.diff.differences | length == 1 + - network.diff.differences[0] == "ipam_config[0].subnet" - name: Cleanup network with ipam_config and deprecated ipam_options docker_network: @@ -91,11 +91,11 @@ - assert: that: - network is changed - - network['diff'] | length == 4 - - '"ipam_config[0].subnet" in network["diff"]' - - '"ipam_config[0].gateway" in network["diff"]' - - '"ipam_config[0].iprange" in network["diff"]' - - '"ipam_config[0].aux_addresses" in network["diff"]' + - network.diff.differences | length == 4 + - '"ipam_config[0].subnet" in network.diff.differences' + - '"ipam_config[0].gateway" in network.diff.differences' + - '"ipam_config[0].iprange" in network.diff.differences' + - '"ipam_config[0].aux_addresses" in network.diff.differences' - name: Remove gateway and iprange of network with custom IPAM config docker_network: @@ -140,8 +140,8 @@ - assert: that: - network is changed - - network['diff'] | length == 1 - - network['diff'][0] == "ipam_config[0].subnet" + - network.diff.differences | length == 1 + - network.diff.differences[0] == "ipam_config[0].subnet" - name: Change subnet of network with IPv6 IPAM config docker_network: @@ -203,8 +203,8 @@ - assert: that: - network is changed - - network['diff'] | length == 1 - - network['diff'][0] == "enable_ipv6" + - network.diff.differences | length == 1 + - network.diff.differences[0] == "enable_ipv6" - name: Cleanup network with IPv6 and custom IPv4 IPAM config docker_network: -- cgit v1.2.1