From 8afe46dc02b35d55d0df5dac65249561ee454df0 Mon Sep 17 00:00:00 2001
From: Felix Fontein <felix@fontein.de>
Date: Mon, 8 Oct 2018 09:23:23 +0200
Subject: [PATCH] docker_container: improve publish all ports functionality
 (#46594)

* Improve handling of published_ports: all.

* Add changelog.
---
 ...594-docker_container-publish-all-ports.yml |  2 +
 .../modules/cloud/docker/docker_container.py  |  7 +-
 .../docker_container/tasks/tests/ports.yml    | 94 +++++++++++++++++++
 3 files changed, 101 insertions(+), 2 deletions(-)
 create mode 100644 changelogs/fragments/46594-docker_container-publish-all-ports.yml
 create mode 100644 test/integration/targets/docker_container/tasks/tests/ports.yml

diff --git a/changelogs/fragments/46594-docker_container-publish-all-ports.yml b/changelogs/fragments/46594-docker_container-publish-all-ports.yml
new file mode 100644
index 0000000000..6b9ff8d715
--- /dev/null
+++ b/changelogs/fragments/46594-docker_container-publish-all-ports.yml
@@ -0,0 +1,2 @@
+bugfixes:
+- "docker_container - ``publish_ports: all`` was not used correctly when checking idempotency."
diff --git a/lib/ansible/modules/cloud/docker/docker_container.py b/lib/ansible/modules/cloud/docker/docker_container.py
index ca9127b89a..ae70076353 100644
--- a/lib/ansible/modules/cloud/docker/docker_container.py
+++ b/lib/ansible/modules/cloud/docker/docker_container.py
@@ -302,7 +302,7 @@ options:
       - "Use docker CLI syntax: C(8000), C(9000:8000), or C(0.0.0.0:9000:8000), where 8000 is a
         container port, 9000 is a host port, and 0.0.0.0 is a host interface."
       - Container ports must be exposed either in the Dockerfile or via the C(expose) option.
-      - A value of all will publish all exposed container ports to random host ports, ignoring
+      - A value of C(all) will publish all exposed container ports to random host ports, ignoring
         any other mappings.
       - If C(networks) parameter is provided, will inspect each network to see if there exists
         a bridge network with optional parameter com.docker.network.bridge.host_binding_ipv4.
@@ -1500,7 +1500,8 @@ class Container(DockerBaseClass):
             expected_volumes=config.get('Volumes'),
             expected_binds=host_config.get('Binds'),
             volumes_from=host_config.get('VolumesFrom'),
-            working_dir=config.get('WorkingDir')
+            working_dir=config.get('WorkingDir'),
+            publish_all_ports=host_config.get('PublishAllPorts'),
         )
         if self.parameters.restart_policy:
             config_mapping['restart_retries'] = restart_policy.get('MaximumRetryCount')
@@ -2291,6 +2292,8 @@ class AnsibleDockerClientContainer(AnsibleDockerClient):
                     comparisons[key_main]['comparison'] = value
                 else:
                     self.fail("Unknown comparison mode '%s'!" % value)
+        # Add implicit options
+        comparisons['publish_all_ports'] = dict(type='value', comparison='strict', name='published_ports')
         # Check legacy values
         if self.module.params['ignore_image'] and comparisons['image']['comparison'] != 'ignore':
             self.module.warn('The ignore_image option has been overridden by the comparisons option!')
diff --git a/test/integration/targets/docker_container/tasks/tests/ports.yml b/test/integration/targets/docker_container/tasks/tests/ports.yml
new file mode 100644
index 0000000000..cc1af9d3ec
--- /dev/null
+++ b/test/integration/targets/docker_container/tasks/tests/ports.yml
@@ -0,0 +1,94 @@
+---
+- name: Registering container name
+  set_fact:
+    cname: "{{ cname_prefix ~ '-options' }}"
+
+####################################################################
+## published_ports: all ############################################
+####################################################################
+
+- name: published_ports -- all
+  docker_container:
+    image: alpine:3.8
+    command: '/bin/sh -c "sleep 10m"'
+    name: "{{ cname }}"
+    state: started
+    exposed_ports:
+    - 8080
+    - 8081
+    published_ports:
+    - all
+    stop_timeout: 1
+  register: published_ports_1
+
+- name: published_ports -- all (idempotency)
+  docker_container:
+    image: alpine:3.8
+    command: '/bin/sh -c "sleep 10m"'
+    name: "{{ cname }}"
+    state: started
+    exposed_ports:
+    - 8080
+    - 8081
+    published_ports:
+    - all
+    stop_timeout: 1
+  register: published_ports_2
+
+- name: published_ports -- all (writing out 'all')
+  docker_container:
+    image: alpine:3.8
+    command: '/bin/sh -c "sleep 10m"'
+    name: "{{ cname }}"
+    state: started
+    exposed_ports:
+    - 8080
+    - 8081
+    published_ports:
+    - 8080
+    - 8081
+    stop_timeout: 1
+  register: published_ports_3
+
+- name: published_ports -- all (idempotency 2)
+  docker_container:
+    image: alpine:3.8
+    command: '/bin/sh -c "sleep 10m"'
+    name: "{{ cname }}"
+    state: started
+    exposed_ports:
+    - 8080
+    - 8081
+    published_ports:
+    - 8080
+    - 8081
+    stop_timeout: 1
+  register: published_ports_4
+
+- name: published_ports -- all (switching back to 'all')
+  docker_container:
+    image: alpine:3.8
+    command: '/bin/sh -c "sleep 10m"'
+    name: "{{ cname }}"
+    state: started
+    exposed_ports:
+    - 8080
+    - 8081
+    published_ports:
+    - all
+    stop_timeout: 1
+  register: published_ports_5
+
+- name: cleanup
+  docker_container:
+    name: "{{ cname }}"
+    state: absent
+    stop_timeout: 1
+
+- assert:
+    that:
+    - published_ports_1 is changed
+    - published_ports_2 is not changed
+    - published_ports_3 is changed
+    - published_ports_4 is not changed
+    - published_ports_5 is changed