diff --git a/changelogs/fragments/54361-docker_swarm_service-remove-secrets-configs-defaults.yaml b/changelogs/fragments/54361-docker_swarm_service-remove-secrets-configs-defaults.yaml new file mode 100644 index 0000000000..640317d8ac --- /dev/null +++ b/changelogs/fragments/54361-docker_swarm_service-remove-secrets-configs-defaults.yaml @@ -0,0 +1,2 @@ +bugfixes: + - "docker_swarm_service - Removed redundant defaults for ``uid``, ``gid``, and ``mode`` from ``configs`` and ``secrets``." diff --git a/lib/ansible/modules/cloud/docker/docker_swarm_service.py b/lib/ansible/modules/cloud/docker/docker_swarm_service.py index b940e0ce79..1ca046e207 100644 --- a/lib/ansible/modules/cloud/docker/docker_swarm_service.py +++ b/lib/ansible/modules/cloud/docker/docker_swarm_service.py @@ -59,17 +59,14 @@ options: description: - UID of the config file's owner. type: int - default: 0 gid: description: - GID of the config file's group. type: int - default: 0 mode: description: - File access mode inside the container. type: str - default: "0o444" constraints: description: - List of the service constraints. @@ -615,17 +612,14 @@ options: description: - UID of the secret file's owner. type: int - default: 0 gid: description: - GID of the secret file's group. type: int - default: 0 mode: description: - File access mode inside the container. type: int - default: 0o444 state: description: - Service state. @@ -1148,6 +1142,42 @@ def get_value(key, values, default=None): return value if value is not None else default +def has_dict_changed(new_dict, old_dict): + """ + Check if new_dict has differences compared to old_dict while + ignoring keys in old_dict which are None in new_dict. + """ + if new_dict is None: + return False + if not new_dict and old_dict: + return True + if not old_dict and new_dict: + return True + defined_options = dict( + (option, value) for option, value in new_dict.items() + if value is not None + ) + for option, value in defined_options.items(): + if value != old_dict.get(option): + return True + return False + + +def has_list_of_dicts_changed(new_list, old_list): + """ + Check two lists of dicts has differences. + """ + if new_list is None: + return False + old_list = old_list or [] + if len(new_list) != len(old_list): + return True + for new_item, old_item in zip(new_list, old_list): + if has_dict_changed(new_item, old_item): + return True + return False + + class DockerService(DockerBaseClass): def __init__(self): super(DockerService, self).__init__() @@ -1620,11 +1650,11 @@ class DockerService(DockerBaseClass): if self.mode != os.mode: needs_rebuild = True differences.add('mode', parameter=self.mode, active=os.mode) - if self.mounts is not None and self.mounts != (os.mounts or []): + if has_list_of_dicts_changed(self.mounts, os.mounts): differences.add('mounts', parameter=self.mounts, active=os.mounts) - if self.configs is not None and self.configs != (os.configs or []): + if has_list_of_dicts_changed(self.configs, os.configs): differences.add('configs', parameter=self.configs, active=os.configs) - if self.secrets is not None and self.secrets != (os.secrets or []): + if has_list_of_dicts_changed(self.secrets, os.secrets): differences.add('secrets', parameter=self.secrets, active=os.secrets) if self.networks is not None and self.networks != (os.networks or []): differences.add('networks', parameter=self.networks, active=os.networks) @@ -1669,7 +1699,7 @@ class DockerService(DockerBaseClass): differences.add('restart_policy_delay', parameter=self.restart_policy_delay, active=os.restart_policy_delay) if self.restart_policy_window is not None and self.restart_policy_window != os.restart_policy_window: differences.add('restart_policy_window', parameter=self.restart_policy_window, active=os.restart_policy_window) - if self.rollback_config is not None and self.has_rollback_config_changed(os.rollback_config): + if has_dict_changed(self.rollback_config, os.rollback_config): differences.add('rollback_config', parameter=self.rollback_config, active=os.rollback_config) if self.update_delay is not None and self.update_delay != os.update_delay: differences.add('update_delay', parameter=self.update_delay, active=os.update_delay) @@ -1744,18 +1774,6 @@ class DockerService(DockerBaseClass): old_image = old_image.split('@')[0] return self.image != old_image, old_image - def has_rollback_config_changed(self, old_rollback_config): - if old_rollback_config is None and self.rollback_config: - return True - defined_options = dict( - (option, value) for option, value in self.rollback_config.items() - if value is not None - ) - for option, value in defined_options.items(): - if value != old_rollback_config.get(option): - return True - return False - def __str__(self): return str({ 'mode': self.mode, @@ -1997,14 +2015,14 @@ class DockerService(DockerBaseClass): ports = {} for port in self.publish: if port.get('mode'): - ports[int(port['published_port'])] = ( - int(port['target_port']), + ports[port['published_port']] = ( + port['target_port'], port['protocol'], port['mode'], ) else: - ports[int(port['published_port'])] = ( - int(port['target_port']), + ports[port['published_port']] = ( + port['target_port'], port['protocol'], ) endpoint_spec_args['ports'] = ports @@ -2470,17 +2488,17 @@ def main(): config_id=dict(type='str', required=True), config_name=dict(type='str', required=True), filename=dict(type='str'), - uid=dict(type='int', default=0), - gid=dict(type='int', default=0), - mode=dict(type='int', default=0o444), + uid=dict(type='int'), + gid=dict(type='int'), + mode=dict(type='int'), )), secrets=dict(type='list', elements='dict', options=dict( secret_id=dict(type='str', required=True), secret_name=dict(type='str', required=True), filename=dict(type='str'), - uid=dict(type='int', default=0), - gid=dict(type='int', default=0), - mode=dict(type='int', default=0o444), + uid=dict(type='int'), + gid=dict(type='int'), + mode=dict(type='int'), )), networks=dict(type='list', elements='str'), command=dict(type='raw'), 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 9538d2c25a..4d2dac3a15 100644 --- a/test/integration/targets/docker_swarm_service/tasks/tests/options.yml +++ b/test/integration/targets/docker_swarm_service/tasks/tests/options.yml @@ -1062,7 +1062,7 @@ command: '/bin/sh -v -c "sleep 10m"' healthcheck: test: - - NONE + - NONE register: healthcheck_5 ignore_errors: yes 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 d7779c30fd..3e0823a6b7 100644 --- a/test/units/modules/cloud/docker/test_docker_swarm_service.py +++ b/test/units/modules/cloud/docker/test_docker_swarm_service.py @@ -112,3 +112,163 @@ def test_get_nanoseconds_from_raw_option(docker_swarm_service): with pytest.raises(ValueError): docker_swarm_service.get_nanoseconds_from_raw_option('test', []) + + +def test_has_dict_changed(docker_swarm_service): + assert not docker_swarm_service.has_dict_changed( + {"a": 1}, + {"a": 1}, + ) + assert not docker_swarm_service.has_dict_changed( + {"a": 1}, + {"a": 1, "b": 2} + ) + assert docker_swarm_service.has_dict_changed( + {"a": 1}, + {"a": 2, "b": 2} + ) + assert docker_swarm_service.has_dict_changed( + {"a": 1, "b": 1}, + {"a": 1} + ) + assert not docker_swarm_service.has_dict_changed( + None, + {"a": 2, "b": 2} + ) + assert docker_swarm_service.has_dict_changed( + {}, + {"a": 2, "b": 2} + ) + assert docker_swarm_service.has_dict_changed( + {"a": 1}, + {} + ) + assert docker_swarm_service.has_dict_changed( + {"a": 1}, + None + ) + assert not docker_swarm_service.has_dict_changed( + {}, + {} + ) + assert not docker_swarm_service.has_dict_changed( + None, + None + ) + assert not docker_swarm_service.has_dict_changed( + {}, + None + ) + assert not docker_swarm_service.has_dict_changed( + None, + {} + ) + + +def test_has_list_of_dicts_changed(docker_swarm_service): + assert docker_swarm_service.has_list_of_dicts_changed( + [ + {"a": 1}, + {"b": 1} + ], + [ + {"a": 1} + ] + ) + assert docker_swarm_service.has_list_of_dicts_changed( + [ + {"a": 1}, + ], + [ + {"a": 1}, + {"b": 1}, + ] + ) + assert not docker_swarm_service.has_list_of_dicts_changed( + [ + {"a": 1}, + {"b": 1}, + ], + [ + {"a": 1}, + {"b": 1} + ] + ) + assert not docker_swarm_service.has_list_of_dicts_changed( + None, + [ + {"b": 1}, + {"a": 1} + ] + ) + assert docker_swarm_service.has_list_of_dicts_changed( + [], + [ + {"b": 1}, + {"a": 1} + ] + ) + assert not docker_swarm_service.has_list_of_dicts_changed( + None, + None + ) + assert not docker_swarm_service.has_list_of_dicts_changed( + [], + None + ) + assert not docker_swarm_service.has_list_of_dicts_changed( + None, + [] + ) + assert not docker_swarm_service.has_list_of_dicts_changed( + [ + {"src": 1, "dst": 2}, + {"src": 1, "dst": 2, "protocol": "udp"}, + ], + [ + {"src": 1, "dst": 2, "protocol": "tcp"}, + {"src": 1, "dst": 2, "protocol": "udp"}, + ] + ) + assert not docker_swarm_service.has_list_of_dicts_changed( + [ + {"src": 1, "dst": 2, "protocol": "udp"}, + {"src": 1, "dst": 3, "protocol": "tcp"}, + ], + [ + {"src": 1, "dst": 2, "protocol": "udp"}, + {"src": 1, "dst": 3, "protocol": "tcp"}, + ] + ) + assert docker_swarm_service.has_list_of_dicts_changed( + [ + {"src": 1, "dst": 2, "protocol": "udp"}, + {"src": 1, "dst": 2}, + {"src": 3, "dst": 4}, + ], + [ + {"src": 1, "dst": 3, "protocol": "udp"}, + {"src": 1, "dst": 2, "protocol": "tcp"}, + {"src": 3, "dst": 4, "protocol": "tcp"}, + ] + ) + assert docker_swarm_service.has_list_of_dicts_changed( + [ + {"src": 1, "dst": 3, "protocol": "tcp"}, + {"src": 1, "dst": 2, "protocol": "udp"}, + ], + [ + {"src": 1, "dst": 2, "protocol": "tcp"}, + {"src": 1, "dst": 2, "protocol": "udp"}, + ] + ) + assert docker_swarm_service.has_list_of_dicts_changed( + [ + {"src": 1, "dst": 2, "protocol": "udp"}, + {"src": 1, "dst": 2, "protocol": "tcp", "extra": {"test": "foo"}}, + ], + [ + {"src": 1, "dst": 2, "protocol": "udp"}, + {"src": 1, "dst": 2, "protocol": "tcp"}, + ] + )