From 7ceb2ac95a899a42b49c9ea46f2f93e3fb071608 Mon Sep 17 00:00:00 2001 From: Hannes Ljungberg Date: Mon, 21 Jan 2019 20:02:08 +0100 Subject: [PATCH] docker_swarm_service: Fix publish idempotency when mode is None (#50882) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Fix publish idempotency when mode is None * Add changelog fragment * Python 2.6 compat * Use self.publish * Check length of publish before comparing content * Sort publish lists before zipping * Enable publish tests * python3 compat * Don’t sort by mode as it is not safe * Document publish suboptions and add them to args * Add type to publish documentation * Add choices to publish argument_spec suboptions * Make tcp the default protocol * Make documentation reflect protocol default * Simplify setting mode * Remove redundant string quoting * Test order of publish * Add comment about publish change detection --- ..._swarm_service-fix-publish-idempotency.yml | 2 + .../cloud/docker/docker_swarm_service.py | 70 ++++++++++++-- .../tasks/tests/options.yml | 95 ++++++++++++------- 3 files changed, 123 insertions(+), 44 deletions(-) create mode 100644 changelogs/fragments/50882-docker_swarm_service-fix-publish-idempotency.yml diff --git a/changelogs/fragments/50882-docker_swarm_service-fix-publish-idempotency.yml b/changelogs/fragments/50882-docker_swarm_service-fix-publish-idempotency.yml new file mode 100644 index 0000000000..e773ead4ae --- /dev/null +++ b/changelogs/fragments/50882-docker_swarm_service-fix-publish-idempotency.yml @@ -0,0 +1,2 @@ +bugfixes: +- "docker_swarm_service - fixing falsely reporting ``publish`` as changed when ``publish.mode`` is not set." diff --git a/lib/ansible/modules/cloud/docker/docker_swarm_service.py b/lib/ansible/modules/cloud/docker/docker_swarm_service.py index c2ceb5e28a..bcd5ac5156 100644 --- a/lib/ansible/modules/cloud/docker/docker_swarm_service.py +++ b/lib/ansible/modules/cloud/docker/docker_swarm_service.py @@ -196,13 +196,41 @@ options: - List of the service networks names. - Maps docker service --network option. publish: - default: [] + type: list required: false + default: [] description: - List of dictionaries describing the service published ports. - - Every item must be a dictionary exposing the keys published_port, target_port, protocol (defaults to 'tcp') - Only used with api_version >= 1.25 - - If API version >= 1.32 and docker python library >= 3.0.0 attribute 'mode' can be set to 'ingress' or 'host' (default 'ingress'). + suboptions: + published_port: + type: int + required: true + description: + - The port to make externally available. + target_port: + type: int + required: true + description: + - The port inside the container to expose. + protocol: + type: str + required: false + default: tcp + description: + - What protocol to use. + choices: + - tcp + - udp + mode: + type: str + required: false + description: + - What publish mode to use. + - Requires API version >= 1.32 and docker python library >= 3.0.0 + choices: + - ingress + - host replicas: required: false default: -1 @@ -470,6 +498,7 @@ EXAMPLES = ''' ''' import time +import operator from ansible.module_utils.docker_common import ( DockerBaseClass, AnsibleDockerClient, @@ -478,7 +507,6 @@ from ansible.module_utils.docker_common import ( 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 types @@ -623,8 +651,8 @@ class DockerService(DockerBaseClass): s.publish = [] for param_p in ap['publish']: service_p = {} - service_p['protocol'] = param_p.get('protocol', 'tcp') - service_p['mode'] = param_p.get('mode', None) + service_p['protocol'] = param_p['protocol'] + service_p['mode'] = param_p['mode'] service_p['published_port'] = int(param_p['published_port']) service_p['target_port'] = int(param_p['target_port']) if service_p['protocol'] not in ['tcp', 'udp']: @@ -710,7 +738,7 @@ class DockerService(DockerBaseClass): differences.add('reserve_memory', parameter=self.reserve_memory, active=os.reserve_memory) if self.container_labels != os.container_labels: differences.add('container_labels', parameter=self.container_labels, active=os.container_labels) - if self.publish != os.publish: + if self.has_publish_changed(os.publish): differences.add('publish', parameter=self.publish, active=os.publish) if self.restart_policy != os.restart_policy: differences.add('restart_policy', parameter=self.restart_policy, active=os.restart_policy) @@ -750,6 +778,27 @@ class DockerService(DockerBaseClass): force_update = True return not differences.empty or force_update, differences, needs_rebuild, force_update + def has_publish_changed(self, old_publish): + if len(self.publish) != len(old_publish): + return True + publish_sorter = operator.itemgetter('published_port', 'target_port', 'protocol') + publish = sorted(self.publish, key=publish_sorter) + old_publish = sorted(old_publish, key=publish_sorter) + for publish_item, old_publish_item in zip(publish, old_publish): + ignored_keys = set() + if not publish_item.get('mode'): + ignored_keys.add('mode') + # Create copies of publish_item dicts where keys specified in ignored_keys are left out + filtered_old_publish_item = dict( + (k, v) for k, v in old_publish_item.items() if k not in ignored_keys + ) + filtered_publish_item = dict( + (k, v) for k, v in publish_item.items() if k not in ignored_keys + ) + if filtered_publish_item != filtered_old_publish_item: + return True + return False + def __str__(self): return str({ 'mode': self.mode, @@ -1137,7 +1186,12 @@ def main(): force_update=dict(default=False, type='bool'), log_driver=dict(default="json-file", type='str'), log_driver_options=dict(default={}, type='dict'), - publish=dict(default=[], type='list'), + publish=dict(default=[], type='list', elements='dict', options=dict( + published_port=dict(type='int', required=True), + target_port=dict(type='int', required=True), + protocol=dict(default='tcp', type='str', required=False, choices=('tcp', 'udp')), + mode=dict(type='str', required=False, choices=('ingress', 'host')), + )), constraints=dict(default=[], type='list'), tty=dict(default=False, type='bool'), dns=dict(default=[], type='list'), diff --git a/test/integration/targets/docker_swarm_service/tasks/tests/options.yml b/test/integration/targets/docker_swarm_service/tasks/tests/options.yml index 9450720347..3cfd2dbac0 100644 --- a/test/integration/targets/docker_swarm_service/tasks/tests/options.yml +++ b/test/integration/targets/docker_swarm_service/tasks/tests/options.yml @@ -954,14 +954,6 @@ ## publish ######################################################### #################################################################### -# FIXME: publish_2 is not marked as changed -#fatal: [testhost]: FAILED! => { -# "assertion": "publish_2 is not changed", -# "changed": false, -# "evaluated_to": false, -# "msg": "Assertion failed" -#} - - name: publish docker_swarm_service: name: "{{ service_name }}" @@ -975,37 +967,68 @@ target_port: 60002 register: publish_1 -#- name: publish (idempotency) -# docker_swarm_service: -# name: "{{ service_name }}" -# image: alpine:3.8 -# publish: -# - protocol: tcp -# published_port: 60001 -# target_port: 60001 -# - protocol: udp -# published_port: 60001 -# target_port: 60001 -# register: publish_2 -# -#- name: publish (change) -# docker_swarm_service: -# name: "{{ service_name }}" -# image: alpine:3.8 -# publish: -# - protocol: tcp -# published_port: 60002 -# target_port: 60001 -# - protocol: udp -# published_port: 60001 -# target_port: 60001 -# register: publish_3 -# +- name: publish (idempotency) + docker_swarm_service: + name: "{{ service_name }}" + image: alpine:3.8 + publish: + - protocol: udp + published_port: 60002 + target_port: 60002 + - published_port: 60001 + target_port: 60001 + register: publish_2 + +- name: publish (change) + docker_swarm_service: + name: "{{ service_name }}" + image: alpine:3.8 + publish: + - protocol: tcp + published_port: 60002 + target_port: 60003 + - protocol: udp + published_port: 60001 + target_port: 60001 + register: publish_3 + +- name: publish (mode) + docker_swarm_service: + name: "{{ service_name }}" + image: alpine:3.8 + publish: + - protocol: tcp + published_port: 60002 + target_port: 60003 + mode: host + - protocol: udp + published_port: 60001 + target_port: 60001 + mode: host + register: publish_4 + +- name: publish (mode idempotency) + docker_swarm_service: + name: "{{ service_name }}" + image: alpine:3.8 + publish: + - protocol: udp + published_port: 60001 + target_port: 60001 + mode: host + - protocol: tcp + published_port: 60002 + target_port: 60003 + mode: host + register: publish_5 + - assert: that: - publish_1 is changed -# - publish_2 is not changed -# - publish_3 is changed + - publish_2 is not changed + - publish_3 is changed + - publish_4 is changed + - publish_5 is not changed when: docker_api_version is version('1.25', '>=') - assert: that: