docker_container - adding publish_all_ports option (#162)

* Initial commit

* Adding changelog fragment

* Updating deprecation notice

* Adding integration test

* Applying second round of review suggestions

* Updating docs and cleaning up integration tests

* Updating test loop logic

162-docker_container_publish_all_option.yml

@@ -0,0 +1,4 @@
+---
+minor_changes:
+  - docker_container - added ``publish_all_ports`` option to publish all exposed ports to random ports except those
+    explicitly bound with ``published_ports`` (https://github.com/ansible-collections/community.docker/pull/162).

plugins/modules/docker_container.py

@@ -663,6 +663,12 @@
       - If I(container_default_behavior) is set to C(compatiblity) (the default value), this
         option has a default of C(no).
     type: bool
+  publish_all_ports:
+    description:
+      - Publish all ports to the host.
+      - Any specified port bindings from I(published_ports) will remain intact when C(true).
+    type: bool
+    version_added: 1.8.0
   published_ports:
     description:
       - List of ports to publish from the container to the host.
@@ -677,7 +683,7 @@
         is different from the C(docker) command line utility. Use the L(dig lookup,../lookup/dig.html)
         to resolve hostnames."
       - A value of C(all) will publish all exposed container ports to random host ports, ignoring
-        any other mappings.
+        any other mappings. Use I(publish_all_ports) instead as the use of C(all) will be deprecated in version 2.0.0.
       - If I(networks) parameter is provided, will inspect each network to see if there exists
         a bridge network with optional parameter C(com.docker.network.bridge.host_binding_ipv4).
         If such a network is found, then published ports where no host IP address is specified
@@ -1424,11 +1430,14 @@ def __init__(self, client):
                     except ValueError as exc:
                         self.fail("Failed to convert %s to bytes: %s" % (param_name, to_native(exc)))
 
-        self.publish_all_ports = False
         self.published_ports = self._parse_publish_ports()
+
         if self.published_ports == 'all':
-            self.publish_all_ports = True
-            self.published_ports = None
+            if self.publish_all_ports is None:
+                self.publish_all_ports = True
+                self.published_ports = None
+            else:
+                self.fail('"all" is not a valid value for "published_ports" when "publish_all_ports" is specified')
 
         self.ports = self._parse_exposed_ports(self.published_ports)
         self.log("expose ports:")
@@ -1750,10 +1759,9 @@ def _parse_publish_ports(self):
             if len(self.published_ports) > 1:
                 self.client.module.deprecate(
                     'Specifying "all" in published_ports together with port mappings is not properly '
-                    'supported by the module. The port mappings are currently ignored. Please specify '
-                    'only port mappings, or the value "all". The behavior for mixed usage will either '
-                    'be forbidden in version 2.0.0, or properly handled. In any case, the way you '
-                    'currently use the module will change in a breaking way',
+                    'supported by the module. The port mappings are currently ignored. Set publish_all_ports '
+                    'to "true" to randomly assign port mappings for those not specified by published_ports. '
+                    'The use of "all" in published_ports next to other values will be removed in version 2.0.0.',
                     collection_name='community.docker', version='2.0.0')
             return 'all'
 
@@ -3558,6 +3566,7 @@ def main():
         pid_mode=dict(type='str'),
         pids_limit=dict(type='int'),
         privileged=dict(type='bool'),
+        publish_all_ports=dict(type='bool'),
         published_ports=dict(type='list', elements='str', aliases=['ports']),
         pull=dict(type='bool', default=False),
         purge_networks=dict(type='bool', default=False),

tests/integration/targets/docker_container/tasks/tests/ports.yml

@@ -83,6 +83,19 @@
     force_kill: yes
   register: published_ports_5
 
+- name: published_ports -- all equivalence with publish_all_ports
+  docker_container:
+    image: "{{ docker_test_image_alpine }}"
+    command: '/bin/sh -c "sleep 10m"'
+    name: "{{ cname }}"
+    state: started
+    exposed_ports:
+      - "9001"
+      - "9002"
+    publish_all_ports: true
+    force_kill: yes
+  register: published_ports_6
+
 - name: cleanup
   docker_container:
     name: "{{ cname }}"
@@ -97,6 +110,7 @@
     - published_ports_3 is changed
     - published_ports_4 is not changed
     - published_ports_5 is changed
+    - published_ports_6 is not changed
 
 ####################################################################
 ## published_ports: port range #####################################
@@ -284,3 +298,84 @@
     - published_ports_2 is not changed
     - published_ports_3 is changed
     - published_ports_4 is failed
+
+####################################################################
+## publish_all_ports ###############################################
+####################################################################
+
+- set_fact:
+    publish_all_ports_test_cases:
+      - test_name: no_options
+        changed: true
+      - test_name: null_to_true
+        publish_all_ports_value: true
+        changed: true
+      - test_name: true_idempotency
+        publish_all_ports_value: true
+        changed: false
+      - test_name: true_to_null
+        changed: false
+      - test_name: null_to_true_2
+        publish_all_ports_value: true
+        changed: false
+      - test_name: true_to_false
+        publish_all_ports_value: false
+        changed: true
+      - test_name: false_idempotency
+        publish_all_ports_value: false
+        changed: false
+      - test_name: false_to_null
+        changed: false
+      - test_name: null_with_published_ports
+        published_ports_value: &ports
+          - "9001:9001"
+          - "9010-9050:9010-9050"
+        changed: true
+      - test_name: null_to_true_with_published_ports
+        publish_all_ports_value: true
+        published_ports_value: *ports
+        changed: true
+      - test_name: true_idempotency_with_published_ports
+        publish_all_ports_value: true
+        published_ports_value: *ports
+        changed: false
+      - test_name: true_to_null_with_published_ports
+        published_ports_value: *ports
+        changed: false
+      - test_name: null_to_true_2_with_published_ports
+        publish_all_ports_value: true
+        published_ports_value: *ports
+        changed: false
+      - test_name: true_to_false_with_published_ports
+        publish_all_ports_value: false
+        published_ports_value: *ports
+        changed: true
+      - test_name: false_idempotency_with_published_ports
+        publish_all_ports_value: false
+        published_ports_value: *ports
+        changed: false
+      - test_name: false_to_null_with_published_ports
+        published_ports_value: *ports
+        changed: false
+
+- name: publish_all_ports ({{ test_case.test_name }})
+  docker_container:
+    image: "{{ docker_test_image_alpine }}"
+    command: '/bin/sh -c "sleep 10m"'
+    name: "{{ cname }}"
+    state: started
+    publish_all_ports: "{{ test_case.publish_all_ports_value | default(omit) }}"
+    published_ports: "{{ test_case.published_ports_value | default(omit) }}"
+    force_kill: yes
+  register: publish_all_ports
+  loop_control:
+    loop_var: test_case
+  loop: "{{ publish_all_ports_test_cases }}"
+
+- assert:
+    that:
+      - publish_all_ports.results[index].changed == test_case.changed
+  loop: "{{ publish_all_ports_test_cases }}"
+  loop_control:
+    index_var: index
+    loop_var: test_case