From 73640a41907c56a9802c28a8305eec7b48b6437c Mon Sep 17 00:00:00 2001 From: Dave Bendit Date: Thu, 6 Dec 2018 12:50:45 -0600 Subject: [PATCH] [docker_network] Adding `scope` and `attachable` flags (#49562) Incorporating the abandoned work from PRs #35288 and #45552. Also adding in the version checking from `docker_container.py`, which should be abstracted out to `docker_common.py`. --- ...ork-adding-scope-and-attachable-flags.yaml | 4 + .../docker_network-requirements.yaml | 1 + .../modules/cloud/docker/docker_network.py | 115 ++++++++++++++++-- .../modules/cloud/docker/docker_swarm.py | 2 +- .../docker_network/tasks/tests/options.yml | 97 +++++++++++++++ .../targets/docker_swarm/tasks/test_swarm.yml | 2 +- 6 files changed, 206 insertions(+), 15 deletions(-) create mode 100644 changelogs/fragments/docker_network-adding-scope-and-attachable-flags.yaml diff --git a/changelogs/fragments/docker_network-adding-scope-and-attachable-flags.yaml b/changelogs/fragments/docker_network-adding-scope-and-attachable-flags.yaml new file mode 100644 index 00000000000..6cde8d08ba2 --- /dev/null +++ b/changelogs/fragments/docker_network-adding-scope-and-attachable-flags.yaml @@ -0,0 +1,4 @@ +--- +minor_changes: + - "docker_network - ``scope`` is now used to set the ``Scope`` property of the docker network during creation." + - "docker_network - ``attachable`` is now used to set the ``Attachable`` property of the docker network during creation." diff --git a/changelogs/fragments/docker_network-requirements.yaml b/changelogs/fragments/docker_network-requirements.yaml index c8c2df88356..fae7c924d4c 100644 --- a/changelogs/fragments/docker_network-requirements.yaml +++ b/changelogs/fragments/docker_network-requirements.yaml @@ -2,3 +2,4 @@ minor_changes: - "docker_network - Minimum docker-py version increased from ``1.8.0`` to ``1.10.0``." - "docker_network - Minimum docker server version increased from ``1.9.0`` to ``1.10.0``." + - "docker_network - Minimum docker API version explcitly set to ``1.22``." diff --git a/lib/ansible/modules/cloud/docker/docker_network.py b/lib/ansible/modules/cloud/docker/docker_network.py index 1c245bb1474..804b502ea99 100644 --- a/lib/ansible/modules/cloud/docker/docker_network.py +++ b/lib/ansible/modules/cloud/docker/docker_network.py @@ -132,6 +132,26 @@ options: default: null required: false + scope: + version_added: 2.8 + description: + - Specify the network's scope. + type: str + default: null + required: false + choices: + - local + - global + - swarm + + attachable: + version_added: 2.8 + description: + - If enabled, and the network is in the global scope, non-service containers on worker nodes will be able to connect to the network. + type: bool + default: null + required: false + extends_documentation_fragment: - docker @@ -264,6 +284,8 @@ class TaskParameters(DockerBaseClass): self.internal = None self.debug = None self.enable_ipv6 = None + self.scope = None + self.attachable = None for key, value in client.module.params.items(): setattr(self, key, value) @@ -294,6 +316,7 @@ def get_ip_version(cidr): def get_driver_options(driver_options): + # TODO: Move this and the same from docker_prune.py to docker_common.py result = dict() if driver_options is not None: for k, v in driver_options.items(): @@ -310,6 +333,59 @@ def get_driver_options(driver_options): class DockerNetworkManager(object): + def _get_minimal_versions(self): + # TODO: Move this and the same from docker_container.py to docker_common.py + self.option_minimal_versions = dict() + for option, data in self.client.module.argument_spec.items(): + self.option_minimal_versions[option] = dict() + self.option_minimal_versions.update(dict( + scope=dict(docker_py_version='2.6.0', docker_api_version='1.30'), + attachable=dict(docker_py_version='2.0.0', docker_api_version='1.26'), + )) + + for option, data in self.option_minimal_versions.items(): + # Test whether option is supported, and store result + support_docker_py = True + support_docker_api = True + if 'docker_py_version' in data: + support_docker_py = self.client.docker_py_version >= LooseVersion(data['docker_py_version']) + if 'docker_api_version' in data: + support_docker_api = self.client.docker_api_version >= LooseVersion(data['docker_api_version']) + data['supported'] = support_docker_py and support_docker_api + # Fail if option is not supported but used + if not data['supported']: + # Test whether option is specified + if 'detect_usage' in data: + used = data['detect_usage']() + else: + used = self.client.module.params.get(option) is not None + if used and 'default' in self.client.module.argument_spec[option]: + used = self.client.module.params[option] != self.client.module.argument_spec[option]['default'] + if used: + # If the option is used, compose error message. + if 'usage_msg' in data: + usg = data['usage_msg'] + else: + usg = 'set %s option' % (option, ) + if not support_docker_api: + msg = 'docker API version is %s. Minimum version required is %s to %s.' + msg = msg % (self.client.docker_api_version_str, data['docker_api_version'], usg) + elif not support_docker_py: + if LooseVersion(data['docker_py_version']) < LooseVersion('2.0.0'): + msg = ("docker-py version is %s. Minimum version required is %s to %s. " + "Consider switching to the 'docker' package if you do not require Python 2.6 support.") + elif self.client.docker_py_version < LooseVersion('2.0.0'): + msg = ("docker-py version is %s. Minimum version required is %s to %s. " + "You have to switch to the Python 'docker' package. First uninstall 'docker-py' before " + "installing 'docker' to avoid a broken installation.") + else: + msg = "docker version is %s. Minimum version required is %s to %s." + msg = msg % (docker_version, data['docker_py_version'], usg) + else: + # should not happen + msg = 'Cannot %s with your configuration.' % (usg, ) + self.client.fail(msg) + def __init__(self, client): self.client = client self.parameters = TaskParameters(client) @@ -322,6 +398,8 @@ class DockerNetworkManager(object): self.diff_tracker = DifferenceTracker() self.diff_result = dict() + self._get_minimal_versions() + self.existing_network = self.get_existing_network() if not self.parameters.connected and self.existing_network: @@ -418,17 +496,21 @@ class DockerNetworkManager(object): 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'): - differences.add('internal', - parameter=self.parameters.internal, - active=net.get('Internal')) - else: - if net.get('Internal'): - differences.add('internal', - parameter=self.parameters.internal, - active=net.get('Internal')) + if self.parameters.internal is not None and self.parameters.internal != net.get('Internal', False): + differences.add('internal', + parameter=self.parameters.internal, + active=net.get('Internal')) + + if self.parameters.scope is not None and self.parameters.scope != net.get('Scope'): + differences.add('scope', + parameter=self.parameters.scope, + active=net.get('Scope')) + + if self.parameters.attachable is not None and self.parameters.attachable != net.get('Attachable', False): + differences.add('attachable', + parameter=self.parameters.attachable, + active=net.get('Attachable')) + return not differences.empty, differences def create_network(self): @@ -462,6 +544,10 @@ class DockerNetworkManager(object): params['enable_ipv6'] = self.parameters.enable_ipv6 if self.parameters.internal is not None: params['internal'] = self.parameters.internal + if self.parameters.scope is not None: + params['scope'] = self.parameters.scope + if self.parameters.attachable is not None: + params['attachable'] = self.parameters.attachable if not self.check_mode: resp = self.client.create_network(self.parameters.network_name, **params) @@ -573,7 +659,9 @@ def main(): )), enable_ipv6=dict(type='bool'), internal=dict(type='bool'), - debug=dict(type='bool', default=False) + debug=dict(type='bool', default=False), + scope=dict(type='str', choices=['local', 'global', 'swarm']), + attachable=dict(type='bool'), ) mutually_exclusive = [ @@ -584,7 +672,8 @@ def main(): argument_spec=argument_spec, mutually_exclusive=mutually_exclusive, supports_check_mode=True, - min_docker_version='1.10.0' + min_docker_version='1.10.0', + min_docker_api_version='1.22' # "The docker server >= 1.10.0" ) diff --git a/lib/ansible/modules/cloud/docker/docker_swarm.py b/lib/ansible/modules/cloud/docker/docker_swarm.py index a4dbc6beb8f..9fd0c3ba64b 100644 --- a/lib/ansible/modules/cloud/docker/docker_swarm.py +++ b/lib/ansible/modules/cloud/docker/docker_swarm.py @@ -429,7 +429,7 @@ class SwarmManager(DockerBaseClass): self.client.leave_swarm(force=self.parameters.force) except APIError as exc: self.client.fail(msg="This node can not leave the Swarm Cluster: %s" % to_native(exc)) - self.results['actions'].append("Node has leaved the swarm cluster") + self.results['actions'].append("Node has left the swarm cluster") self.results['changed'] = True def __get_node_info(self): diff --git a/test/integration/targets/docker_network/tasks/tests/options.yml b/test/integration/targets/docker_network/tasks/tests/options.yml index d4ac563308a..b3ac57b416b 100644 --- a/test/integration/targets/docker_network/tasks/tests/options.yml +++ b/test/integration/targets/docker_network/tasks/tests/options.yml @@ -92,3 +92,100 @@ - driver_options_3 is not changed - driver_options_4 is changed - driver_options_5 is not changed + +#################################################################### +## scope ########################################################### +#################################################################### + +- block: + - name: scope + docker_network: + name: "{{ nname_1 }}" + driver: bridge + scope: local + register: scope_1 + + - name: scope (idempotency) + docker_network: + name: "{{ nname_1 }}" + driver: bridge + scope: local + register: scope_2 + + - name: swarm + docker_swarm: + state: present + advertise_addr: "{{ansible_default_ipv4.address}}" + + # Driver change alongside scope is intentional - bridge doesn't appear to support anything but local, and overlay can't downgrade to local. Additionally, overlay reports as swarm for swarm OR global, so no change is reported in that case. + # Test output indicates that the scope is altered, at least, so manual inspection will be required to verify this going forward, unless we come up with a test driver that supports multiple scopes. + - name: scope (change) + docker_network: + name: "{{ nname_1 }}" + driver: overlay + scope: swarm + register: scope_3 + + - name: cleanup network + docker_network: + name: "{{ nname_1 }}" + state: absent + force: yes + + - assert: + that: + - scope_1 is changed + - scope_2 is not changed + - scope_3 is changed + + always: + - name: cleanup swarm + docker_swarm: + state: absent + force: yes + + # Requirements for docker_swarm + when: docker_py_version is version('2.6.0', '>=') and docker_api_version is version('1.35', '>=') + +#################################################################### +## attachable ###################################################### +#################################################################### + +- name: attachable + docker_network: + name: "{{ nname_1 }}" + attachable: true + register: attachable_1 + ignore_errors: yes + +- name: attachable (idempotency) + docker_network: + name: "{{ nname_1 }}" + attachable: true + register: attachable_2 + ignore_errors: yes + +- name: attachable (change) + docker_network: + name: "{{ nname_1 }}" + attachable: false + register: attachable_3 + ignore_errors: yes + +- name: cleanup + docker_network: + name: "{{ nname_1 }}" + state: absent + force: yes + +- assert: + that: + - attachable_1 is changed + - attachable_2 is not changed + - attachable_3 is changed + when: docker_py_version is version('2.0.0', '>=') +- assert: + that: + - attachable_1 is failed + - "('version is ' ~ docker_py_version ~'. Minimum version required is 2.0.0') in attachable_1.msg" + when: docker_py_version is version('2.0.0', '<') diff --git a/test/integration/targets/docker_swarm/tasks/test_swarm.yml b/test/integration/targets/docker_swarm/tasks/test_swarm.yml index eca815f1a45..f683de239fa 100644 --- a/test/integration/targets/docker_swarm/tasks/test_swarm.yml +++ b/test/integration/targets/docker_swarm/tasks/test_swarm.yml @@ -52,7 +52,7 @@ assert: that: - 'output.changed' - - 'output.actions[0] == "Node has leaved the swarm cluster"' + - 'output.actions[0] == "Node has left the swarm cluster"' always: - name: Cleanup