From 0206a46e1d52d4366cfc632310677ea2c921c62b Mon Sep 17 00:00:00 2001
From: Felix Fontein <felix@fontein.de>
Date: Mon, 12 Nov 2018 12:33:42 +0100
Subject: [PATCH] docker_* modules: updating argument_spec (#48491)

* Updating argument_spec for docker_* modules.

* Adjust docker_network to work with new recursive argument_spec.

* Adjust device IO limits to recursive argument_spec.

* Improve test (test Ansible's cast from str to int).

* Adjust healthcheck options construction.

* Remove superfluous check.

* Make flake8 happy.

* Simplify comparison.
---
 .../modules/cloud/docker/docker_container.py  | 230 ++++++++++++------
 .../modules/cloud/docker/docker_image.py      |   7 +-
 .../cloud/docker/docker_image_facts.py        |   2 +-
 .../modules/cloud/docker/docker_network.py    |  43 +++-
 .../modules/cloud/docker/docker_service.py    |   4 +-
 .../modules/cloud/docker/docker_swarm.py      |   2 +-
 .../docker_container/tasks/tests/options.yml  |   2 +-
 7 files changed, 210 insertions(+), 80 deletions(-)

diff --git a/lib/ansible/modules/cloud/docker/docker_container.py b/lib/ansible/modules/cloud/docker/docker_container.py
index 858277efaf8..688d7a4043b 100644
--- a/lib/ansible/modules/cloud/docker/docker_container.py
+++ b/lib/ansible/modules/cloud/docker/docker_container.py
@@ -101,34 +101,74 @@ options:
   device_read_bps:
     description:
       - "List of device path and read rate (bytes per second) from device."
-      - "I(path) - device path in the container."
-      - "I(rate) - device read limit (format: <number>[<unit>])"
-      - "Number is a positive integer. Unit can be one of C(B) (byte), C(K) (kibibyte, 1024B), C(M) (mebibyte), C(G) (gibibyte),
-        C(T) (tebibyte), or C(P) (pebibyte)"
-      - "Omitting the unit defaults to bytes."
+    type: list
+    suboptions:
+      path:
+        type: str
+        required: true
+        description:
+        - Device path in the container.
+      rate:
+        type: str
+        required: true
+        description:
+        - "Device read limit. Format: <number>[<unit>]"
+        - "Number is a positive integer. Unit can be one of C(B) (byte), C(K) (kibibyte, 1024B), C(M) (mebibyte), C(G) (gibibyte),
+          C(T) (tebibyte), or C(P) (pebibyte)"
+        - "Omitting the unit defaults to bytes."
     version_added: "2.8"
   device_write_bps:
     description:
       - "List of device and write rate (bytes per second) to device."
-      - "I(path) - device path in the container."
-      - "I(rate) - device write limit (format: <number>[<unit>])"
-      - "Number is a positive integer. Unit can be one of C(B) (byte), C(K) (kibibyte, 1024B), C(M) (mebibyte), C(G) (gibibyte),
-        C(T) (tebibyte), or C(P) (pebibyte)"
-      - "Omitting the unit defaults to bytes."
+    type: list
+    suboptions:
+      path:
+        type: str
+        required: true
+        description:
+        - Device path in the container.
+      rate:
+        type: str
+        required: true
+        description:
+        - "Device read limit. Format: <number>[<unit>]"
+        - "Number is a positive integer. Unit can be one of C(B) (byte), C(K) (kibibyte, 1024B), C(M) (mebibyte), C(G) (gibibyte),
+          C(T) (tebibyte), or C(P) (pebibyte)"
+        - "Omitting the unit defaults to bytes."
     version_added: "2.8"
   device_read_iops:
     description:
       - "List of device and read rate (IO per second) from device."
-      - "I(path) - device path in the container."
-      - "I(rate) - device read limit (format: <number>)"
-      - "Number is a positive integer."
+    type: list
+    suboptions:
+      path:
+        type: str
+        required: true
+        description:
+        - Device path in the container.
+      rate:
+        type: int
+        required: true
+        description:
+        - "Device read limit."
+        - "Must be a positive integer."
     version_added: "2.8"
   device_write_iops:
     description:
       - "List of device and write rate (IO per second) to device."
-      - "I(path) - device path in the container."
-      - "I(rate) - device write limit (format: <number>)"
-      - "Number is a positive integer."
+    type: list
+    suboptions:
+      path:
+        type: str
+        required: true
+        description:
+        - Device path in the container.
+      rate:
+        type: int
+        required: true
+        description:
+        - "Device read limit."
+        - "Must be a positive integer."
     version_added: "2.8"
   dns_opts:
     description:
@@ -184,14 +224,29 @@ options:
       - 'Configure a check that is run to determine whether or not containers for this service are "healthy".
         See the docs for the L(HEALTHCHECK Dockerfile instruction,https://docs.docker.com/engine/reference/builder/#healthcheck)
         for details on how healthchecks work.'
-      - 'I(test) - Command to run to check health. C(test) must be either a string or a list. If it is a list, the first item must
-        be one of C(NONE), C(CMD) or C(CMD-SHELL).'
-      - 'I(interval) - Time between running the check. (default: 30s)'
-      - 'I(timeout) - Maximum time to allow one check to run. (default: 30s)'
-      - 'I(retries) - Consecutive failures needed to report unhealthy. It accept integer value. (default: 3)'
-      - 'I(start_period) - Start period for the container to initialize before starting health-retries countdown. (default: 0s)'
-      - 'C(interval), C(timeout) and C(start_period) are specified as durations. They accept duration as a string in a format
+      - 'I(interval), I(timeout) and I(start_period) are specified as durations. They accept duration as a string in a format
         that look like: C(5h34m56s), C(1m30s) etc. The supported units are C(us), C(ms), C(s), C(m) and C(h)'
+    suboptions:
+      test:
+        description:
+          - Command to run to check health.
+          - Must be either a string or a list. If it is a list, the first item must be one of C(NONE), C(CMD) or C(CMD-SHELL).
+      interval:
+        description:
+          - 'Time between running the check. (default: 30s)'
+        type: str
+      timeout:
+        description:
+          - 'Maximum time to allow one check to run. (default: 30s)'
+        type: str
+      retries:
+        description:
+          - 'Consecutive failures needed to report unhealthy. It accept integer value. (default: 3)'
+        type: int
+      start_period:
+        description:
+          - 'Start period for the container to initialize before starting health-retries countdown. (default: 0s)'
+        type: str
   hostname:
     description:
       - Container hostname.
@@ -300,15 +355,36 @@ options:
   networks:
      description:
        - List of networks the container belongs to.
-       - Each network is a dict with keys C(name), C(ipv4_address), C(ipv6_address), C(links), C(aliases).
-       - For each network C(name) is required, all other keys are optional.
-       - If included, C(links) or C(aliases) are lists.
        - For examples of the data structure and usage see EXAMPLES below.
        - To remove a container from one or more networks, use the C(purge_networks) option.
        - Note that as opposed to C(docker run ...), M(docker_container) does not remove the default
          network if C(networks) is specified. You need to explicity use C(purge_networks) to enforce
          the removal of the default network (and all other networks not explicitly mentioned in C(networks)).
      version_added: "2.2"
+     type: list
+     suboptions:
+        name:
+           type: str
+           required: true
+           description:
+             - The network's name.
+        ipv4_address:
+           type: str
+           description:
+             - The container's IPv4 address in this network.
+        ipv6_address:
+           type: str
+           description:
+             - The container's IPv6 address in this network.
+        links:
+           type: list
+           description:
+             - A list of containers to link to.
+        aliases:
+           type: list
+           description:
+             - List of aliases for this container in this network. These names
+               can be used in the network to reach this container.
   oom_killer:
     description:
       - Whether or not to disable OOM Killer for the container.
@@ -1015,8 +1091,6 @@ class TaskParameters(DockerBaseClass):
 
         if self.networks:
             for network in self.networks:
-                if not network.get('name'):
-                    self.fail("Parameter error: network must have a name attribute.")
                 network['id'] = self._get_network_id(network['name'])
                 if not network['id']:
                     self.fail("Parameter error: network named %s could not be found. Does it exist?" % network['name'])
@@ -1441,6 +1515,10 @@ class TaskParameters(DockerBaseClass):
 
         for (key, value) in options.items():
             if value in self.healthcheck:
+                if self.healthcheck.get(value) is None:
+                    # due to recursive argument_spec, all keys are always present
+                    # (but have default value None if not specified)
+                    continue
                 if value in duration_options:
                     time = self._convert_duration_to_nanosecond(self.healthcheck.get(value))
                     if time:
@@ -1539,7 +1617,7 @@ class TaskParameters(DockerBaseClass):
         devices_list = []
         for v in getattr(self, option):
             device_dict = dict((x.title(), y) for x, y in v.items())
-            device_dict['Rate'] = human_to_bytes(device_dict.get('Rate', 0))
+            device_dict['Rate'] = human_to_bytes(device_dict['Rate'])
             devices_list.append(device_dict)
 
         setattr(self, option, devices_list)
@@ -1550,12 +1628,8 @@ class TaskParameters(DockerBaseClass):
         """
         devices_list = []
         for v in getattr(self, option):
-            try:
-                device_dict = dict((x.title(), y) for x, y in v.items())
-                device_dict['Rate'] = int(device_dict.get('Rate', 0))
-                devices_list.append(device_dict)
-            except ValueError:
-                self.fail("Invalid device iops value: '{0}'. Must be a positive integer.".format(device_dict.get('Rate')))
+            device_dict = dict((x.title(), y) for x, y in v.items())
+            devices_list.append(device_dict)
 
         setattr(self, option, devices_list)
 
@@ -1866,21 +1940,15 @@ class Container(DockerBaseClass):
                     diff = True
                 if network.get('ipv6_address') and network['ipv6_address'] != connected_networks[network['name']].get('GlobalIPv6Address'):
                     diff = True
-                if network.get('aliases') and not connected_networks[network['name']].get('Aliases'):
-                    diff = True
-                if network.get('aliases') and connected_networks[network['name']].get('Aliases'):
-                    for alias in network.get('aliases'):
-                        if alias not in connected_networks[network['name']].get('Aliases', []):
-                            diff = True
-                if network.get('links') and not connected_networks[network['name']].get('Links'):
-                    diff = True
-                if network.get('links') and connected_networks[network['name']].get('Links'):
+                if network.get('aliases'):
+                    if not compare_generic(network['aliases'], connected_networks[network['name']].get('Aliases'), 'allow_more_present', 'set'):
+                        diff = True
+                if network.get('links'):
                     expected_links = []
                     for link, alias in network['links']:
                         expected_links.append("%s:%s" % (link, alias))
-                    for link in expected_links:
-                        if link not in connected_networks[network['name']].get('Links', []):
-                            diff = True
+                    if not compare_generic(expected_links, connected_networks[network['name']].get('Links'), 'allow_more_present', 'set'):
+                        diff = True
                 if diff:
                     different = True
                     differences.append(dict(
@@ -2723,8 +2791,8 @@ def main():
     argument_spec = dict(
         auto_remove=dict(type='bool', default=False),
         blkio_weight=dict(type='int'),
-        capabilities=dict(type='list'),
-        cap_drop=dict(type='list'),
+        capabilities=dict(type='list', elements='str'),
+        cap_drop=dict(type='list', elements='str'),
         cleanup=dict(type='bool', default=False),
         command=dict(type='raw'),
         comparisons=dict(type='dict'),
@@ -2734,23 +2802,41 @@ def main():
         cpuset_mems=dict(type='str'),
         cpu_shares=dict(type='int'),
         detach=dict(type='bool', default=True),
-        devices=dict(type='list'),
-        device_read_bps=dict(type='list'),
-        device_write_bps=dict(type='list'),
-        device_read_iops=dict(type='list'),
-        device_write_iops=dict(type='list'),
-        dns_servers=dict(type='list'),
-        dns_opts=dict(type='list'),
-        dns_search_domains=dict(type='list'),
+        devices=dict(type='list', elements='str'),
+        device_read_bps=dict(type='list', elements='dict', options=dict(
+            path=dict(required=True, type='str'),
+            rate=dict(required=True, type='str'),
+        )),
+        device_write_bps=dict(type='list', elements='dict', options=dict(
+            path=dict(required=True, type='str'),
+            rate=dict(required=True, type='str'),
+        )),
+        device_read_iops=dict(type='list', elements='dict', options=dict(
+            path=dict(required=True, type='str'),
+            rate=dict(required=True, type='int'),
+        )),
+        device_write_iops=dict(type='list', elements='dict', options=dict(
+            path=dict(required=True, type='str'),
+            rate=dict(required=True, type='int'),
+        )),
+        dns_servers=dict(type='list', elements='str'),
+        dns_opts=dict(type='list', elements='str'),
+        dns_search_domains=dict(type='list', elements='str'),
         domainname=dict(type='str'),
-        entrypoint=dict(type='list'),
+        entrypoint=dict(type='list', elements='str'),
         env=dict(type='dict'),
         env_file=dict(type='path'),
         etc_hosts=dict(type='dict'),
-        exposed_ports=dict(type='list', aliases=['exposed', 'expose']),
+        exposed_ports=dict(type='list', aliases=['exposed', 'expose'], elements='str'),
         force_kill=dict(type='bool', default=False, aliases=['forcekill']),
-        groups=dict(type='list'),
-        healthcheck=dict(type='dict'),
+        groups=dict(type='list', elements='str'),
+        healthcheck=dict(type='dict', options=dict(
+            test=dict(type='raw'),
+            interval=dict(type='str'),
+            timeout=dict(type='str'),
+            start_period=dict(type='str'),
+            retries=dict(type='int'),
+        )),
         hostname=dict(type='str'),
         ignore_image=dict(type='bool', default=False),
         image=dict(type='str'),
@@ -2761,7 +2847,7 @@ def main():
         kernel_memory=dict(type='str'),
         kill_signal=dict(type='str'),
         labels=dict(type='dict'),
-        links=dict(type='list'),
+        links=dict(type='list', elements='str'),
         log_driver=dict(type='str'),
         log_options=dict(type='dict', aliases=['log_opt']),
         mac_address=dict(type='str'),
@@ -2771,14 +2857,20 @@ def main():
         memory_swappiness=dict(type='int'),
         name=dict(type='str', required=True),
         network_mode=dict(type='str'),
-        networks=dict(type='list'),
+        networks=dict(type='list', elements='dict', options=dict(
+            name=dict(required=True, type='str'),
+            ipv4_address=dict(type='str'),
+            ipv6_address=dict(type='str'),
+            aliases=dict(type='list', elements='str'),
+            links=dict(type='list', elements='str'),
+        )),
         oom_killer=dict(type='bool'),
         oom_score_adj=dict(type='int'),
         output_logs=dict(type='bool', default=False),
         paused=dict(type='bool', default=False),
         pid_mode=dict(type='str'),
         privileged=dict(type='bool', default=False),
-        published_ports=dict(type='list', aliases=['ports']),
+        published_ports=dict(type='list', aliases=['ports'], elements='str'),
         pull=dict(type='bool', default=False),
         purge_networks=dict(type='bool', default=False),
         read_only=dict(type='bool', default=False),
@@ -2787,22 +2879,22 @@ def main():
         restart_policy=dict(type='str', choices=['no', 'on-failure', 'always', 'unless-stopped']),
         restart_retries=dict(type='int', default=None),
         runtime=dict(type='str', default=None),
-        security_opts=dict(type='list'),
+        security_opts=dict(type='list', elements='str'),
         shm_size=dict(type='str'),
         state=dict(type='str', choices=['absent', 'present', 'started', 'stopped'], default='started'),
         stop_signal=dict(type='str'),
         stop_timeout=dict(type='int'),
         sysctls=dict(type='dict'),
-        tmpfs=dict(type='list'),
+        tmpfs=dict(type='list', elements='str'),
         trust_image_content=dict(type='bool', default=False),
         tty=dict(type='bool', default=False),
-        ulimits=dict(type='list'),
+        ulimits=dict(type='list', elements='str'),
         user=dict(type='str'),
         userns_mode=dict(type='str'),
         uts=dict(type='str'),
         volume_driver=dict(type='str'),
-        volumes=dict(type='list'),
-        volumes_from=dict(type='list'),
+        volumes=dict(type='list', elements='str'),
+        volumes_from=dict(type='list', elements='str'),
         working_dir=dict(type='str'),
     )
 
diff --git a/lib/ansible/modules/cloud/docker/docker_image.py b/lib/ansible/modules/cloud/docker/docker_image.py
index b876c031ad0..911e5525a02 100644
--- a/lib/ansible/modules/cloud/docker/docker_image.py
+++ b/lib/ansible/modules/cloud/docker/docker_image.py
@@ -583,7 +583,12 @@ class ImageManager(DockerBaseClass):
 def main():
     argument_spec = dict(
         archive_path=dict(type='path'),
-        container_limits=dict(type='dict'),
+        container_limits=dict(type='dict', options=dict(
+            memory=dict(type='int'),
+            memswap=dict(type='int'),
+            cpushares=dict(type='int'),
+            cpusetcpus=dict(type='str'),
+        )),
         dockerfile=dict(type='str'),
         force=dict(type='bool', default=False),
         http_timeout=dict(type='int'),
diff --git a/lib/ansible/modules/cloud/docker/docker_image_facts.py b/lib/ansible/modules/cloud/docker/docker_image_facts.py
index d12b79f213a..380e6dddfdd 100644
--- a/lib/ansible/modules/cloud/docker/docker_image_facts.py
+++ b/lib/ansible/modules/cloud/docker/docker_image_facts.py
@@ -227,7 +227,7 @@ class ImageManager(DockerBaseClass):
 
 def main():
     argument_spec = dict(
-        name=dict(type='list'),
+        name=dict(type='list', elements='str'),
     )
 
     client = AnsibleDockerClient(
diff --git a/lib/ansible/modules/cloud/docker/docker_network.py b/lib/ansible/modules/cloud/docker/docker_network.py
index 2a30e7b9d81..f64654bbd29 100644
--- a/lib/ansible/modules/cloud/docker/docker_network.py
+++ b/lib/ansible/modules/cloud/docker/docker_network.py
@@ -77,7 +77,7 @@ options:
   ipam_options:
     description:
       - Dictionary of IPAM options.
-      - Deprecated in 2.8, will be removed in 2.12. Use parameter ``ipam_config`` instead. In Docker 1.10.0, IPAM
+      - Deprecated in 2.8, will be removed in 2.12. Use parameter C(ipam_config) instead. In Docker 1.10.0, IPAM
         options were introduced (see L(here,https://github.com/moby/moby/pull/17316)). This module parameter addresses
         the IPAM config not the newly introduced IPAM options.
 
@@ -86,9 +86,27 @@ options:
     description:
       - List of IPAM config blocks. Consult
         L(Docker docs,https://docs.docker.com/compose/compose-file/compose-file-v2/#ipam) for valid options and values.
+        Note that I(iprange) is spelled differently here (we use the notation from the Docker Python SDK).
     type: list
     default: null
     required: false
+    suboptions:
+      subnet:
+        description:
+          - IP subset in CIDR notation.
+        type: str
+      iprange:
+        description:
+          - IP address range in CIDR notation.
+        type: str
+      gateway:
+        description:
+          - IP gateway address.
+        type: str
+      aux_addresses:
+        description:
+          - Auxiliary IP addresses used by Network driver, as a mapping from hostname to IP.
+        type: dict
 
   state:
     description:
@@ -300,7 +318,8 @@ class DockerNetworkManager(object):
         if not self.parameters.connected and self.existing_network:
             self.parameters.connected = container_names_in_network(self.existing_network)
 
-        if self.parameters.ipam_options:
+        if (self.parameters.ipam_options['subnet'] or self.parameters.ipam_options['iprange'] or
+                self.parameters.ipam_options['gateway'] or self.parameters.ipam_options['aux_addresses']):
             self.parameters.ipam_config = [self.parameters.ipam_options]
 
         if self.parameters.driver_options:
@@ -362,6 +381,10 @@ class DockerNetworkManager(object):
                         self.client.fail(str(e))
 
                     for key, value in ipam_config.items():
+                        if value is None:
+                            # due to recursive argument_spec, all keys are always present
+                            # (but have default value None if not specified)
+                            continue
                         camelkey = None
                         for net_key in net_config:
                             if key == net_key.lower():
@@ -493,15 +516,25 @@ class DockerNetworkManager(object):
 def main():
     argument_spec = dict(
         network_name=dict(type='str', required=True, aliases=['name']),
-        connected=dict(type='list', default=[], aliases=['containers']),
+        connected=dict(type='list', default=[], aliases=['containers'], elements='str'),
         state=dict(type='str', default='present', choices=['present', 'absent']),
         driver=dict(type='str', default='bridge'),
         driver_options=dict(type='dict', default={}),
         force=dict(type='bool', default=False),
         appends=dict(type='bool', default=False, aliases=['incremental']),
         ipam_driver=dict(type='str'),
-        ipam_options=dict(type='dict', default={}, removed_in_version='2.12'),
-        ipam_config=dict(type='list', elements='dict'),
+        ipam_options=dict(type='dict', default={}, removed_in_version='2.12', options=dict(
+            subnet=dict(type='str'),
+            iprange=dict(type='str'),
+            gateway=dict(type='str'),
+            aux_addresses=dict(type='dict'),
+        )),
+        ipam_config=dict(type='list', elements='dict', options=dict(
+            subnet=dict(type='str'),
+            iprange=dict(type='str'),
+            gateway=dict(type='str'),
+            aux_addresses=dict(type='dict'),
+        )),
         enable_ipv6=dict(type='bool'),
         internal=dict(type='bool'),
         debug=dict(type='bool', default=False)
diff --git a/lib/ansible/modules/cloud/docker/docker_service.py b/lib/ansible/modules/cloud/docker/docker_service.py
index 902d72da365..79f8003558b 100644
--- a/lib/ansible/modules/cloud/docker/docker_service.py
+++ b/lib/ansible/modules/cloud/docker/docker_service.py
@@ -1033,7 +1033,7 @@ def main():
     argument_spec = dict(
         project_src=dict(type='path'),
         project_name=dict(type='str',),
-        files=dict(type='list'),
+        files=dict(type='list', elements='path'),
         state=dict(type='str', choices=['absent', 'present'], default='present'),
         definition=dict(type='dict'),
         hostname_check=dict(type='bool', default=False),
@@ -1045,7 +1045,7 @@ def main():
         stopped=dict(type='bool', default=False),
         restarted=dict(type='bool', default=False),
         scale=dict(type='dict'),
-        services=dict(type='list'),
+        services=dict(type='list', elements='str'),
         dependencies=dict(type='bool', default=True),
         pull=dict(type='bool', default=False),
         nocache=dict(type='bool', default=False),
diff --git a/lib/ansible/modules/cloud/docker/docker_swarm.py b/lib/ansible/modules/cloud/docker/docker_swarm.py
index d349740b361..a4dbc6beb8f 100644
--- a/lib/ansible/modules/cloud/docker/docker_swarm.py
+++ b/lib/ansible/modules/cloud/docker/docker_swarm.py
@@ -475,7 +475,7 @@ def main():
         state=dict(type='str', choices=['present', 'join', 'absent', 'remove', 'inspect'], default='present'),
         force=dict(type='bool', default=False),
         listen_addr=dict(type='str', default='0.0.0.0:2377'),
-        remote_addrs=dict(type='list'),
+        remote_addrs=dict(type='list', elements='str'),
         join_token=dict(type='str'),
         snapshot_interval=dict(type='int'),
         task_history_retention_limit=dict(type='int'),
diff --git a/test/integration/targets/docker_container/tasks/tests/options.yml b/test/integration/targets/docker_container/tasks/tests/options.yml
index e7b01c24a26..2ad08ef36a3 100644
--- a/test/integration/targets/docker_container/tasks/tests/options.yml
+++ b/test/integration/targets/docker_container/tasks/tests/options.yml
@@ -709,7 +709,7 @@
     state: started
     device_read_iops:
       - path: /dev/urandom
-        rate: 20
+        rate: "20"
       - path: /dev/random
         rate: 10
   register: device_read_iops_2