docker_container: pass volumes only for anonymous volumes (#66600)

* Simplify code.

* Only pass anonymous volumes.

* Add changelog and update porting guide.

* Add integration tests.

changelogs/fragments/66600-docker_container-volumes.yml

@@ -0,0 +1,2 @@
+minor_changes:
+- "docker_container - only passes anonymous volumes to docker daemon as ``Volumes``. This increases compatibility with the ``docker`` CLI program. Note that if you specify ``volumes: strict`` in ``comparisons``, this could cause existing containers created with docker_container from Ansible 2.9 or earlier to restart."

docs/docsite/rst/porting_guides/porting_guide_2.10.rst

@@ -107,6 +107,7 @@ Noteworthy module changes
     * When the directory specified by ``paths`` does not exist or is a file, it will no longer fail and will just warn the user
     * Junction points are no longer reported as ``islnk``, use ``isjunction`` to properly report these files. This behaviour matches the :ref:`win_stat <win_stat_module>`
     * Directories no longer return a ``size``, this matches the ``stat`` and ``find`` behaviour and has been removed due to the difficulties in correctly reporting the size of a directory
+* :ref:`docker_container <docker_container_module>` no longer passes information on non-anonymous volumes or binds as ``Volumes`` to the Docker daemon. This increases compatibility with the ``docker`` CLI program. Note that if you specify ``volumes: strict`` in ``comparisons``, this could cause existing containers created with docker_container from Ansible 2.9 or earlier to restart.
 
 Plugins
 =======

lib/ansible/modules/cloud/docker/docker_container.py

@@ -1495,16 +1495,16 @@ class TaskParameters(DockerBaseClass):
         new_vols = []
         for vol in self.volumes:
             if ':' in vol:
-                if len(vol.split(':')) == 3:
+                parts = vol.split(':')
-                    host, container, mode = vol.split(':')
+                if len(parts) == 3:
+                    host, container, mode = parts
                     if not is_volume_permissions(mode):
                         self.fail('Found invalid volumes mode: {0}'.format(mode))
                     if re.match(r'[.~]', host):
                         host = os.path.abspath(os.path.expanduser(host))
                     new_vols.append("%s:%s:%s" % (host, container, mode))
                     continue
-                elif len(vol.split(':')) == 2:
+                elif len(parts) == 2:
-                    parts = vol.split(':')
                     if not is_volume_permissions(parts[1]) and re.match(r'[.~]', parts[0]):
                         host = os.path.abspath(os.path.expanduser(parts[0]))
                         new_vols.append("%s:%s:rw" % (host, parts[1]))
@@ -1520,15 +1520,13 @@ class TaskParameters(DockerBaseClass):
         result = []
         if self.volumes:
             for vol in self.volumes:
+                # Only pass anonymous volumes to create container
                 if ':' in vol:
-                    if len(vol.split(':')) == 3:
-                        dummy, container, dummy = vol.split(':')
-                        result.append(container)
-                        continue
-                    if len(vol.split(':')) == 2:
                     parts = vol.split(':')
+                    if len(parts) == 3:
+                        continue
+                    if len(parts) == 2:
                         if not is_volume_permissions(parts[1]):
-                            result.append(parts[1])
                             continue
                 result.append(vol)
         self.log("mounts:")
@@ -1698,7 +1696,7 @@ class TaskParameters(DockerBaseClass):
                             self.fail('Found invalid volumes mode: {0}'.format(mode))
                     elif len(parts) == 2:
                         if not is_volume_permissions(parts[1]):
-                            host, container, mode = (vol.split(':') + ['rw'])
+                            host, container, mode = (parts + ['rw'])
                 if host is not None:
                     result[host] = dict(
                         bind=container,
@@ -2445,14 +2443,14 @@ class Container(DockerBaseClass):
             for vol in self.parameters.volumes:
                 host = None
                 if ':' in vol:
-                    if len(vol.split(':')) == 3:
+                    parts = vol.split(':')
-                        host, container, mode = vol.split(':')
+                    if len(parts) == 3:
+                        host, container, mode = parts
                         if not is_volume_permissions(mode):
                             self.fail('Found invalid volumes mode: {0}'.format(mode))
-                    if len(vol.split(':')) == 2:
+                    if len(parts) == 2:
-                        parts = vol.split(':')
                         if not is_volume_permissions(parts[1]):
-                            host, container, mode = vol.split(':') + ['rw']
+                            host, container, mode = parts + ['rw']
                 if host:
                     param_vols.append("%s:%s:%s" % (host, container, mode))
         result = list(set(image_vols + param_vols))
@@ -2494,22 +2492,15 @@ class Container(DockerBaseClass):
 
         if self.parameters.volumes:
             for vol in self.parameters.volumes:
-                container = None
+                # We only expect anonymous volumes to show up in the list
                 if ':' in vol:
-                    if len(vol.split(':')) == 3:
-                        dummy, container, mode = vol.split(':')
-                        if not is_volume_permissions(mode):
-                            self.fail('Found invalid volumes mode: {0}'.format(mode))
-                    if len(vol.split(':')) == 2:
                     parts = vol.split(':')
+                    if len(parts) == 3:
+                        continue
+                    if len(parts) == 2:
                         if not is_volume_permissions(parts[1]):
-                            dummy, container, mode = vol.split(':') + ['rw']
+                            continue
-                new_vol = dict()
+                expected_vols[vol] = dict()
-                if container:
-                    new_vol[container] = dict()
-                else:
-                    new_vol[vol] = dict()
-                expected_vols.update(new_vol)
 
         if not expected_vols:
             expected_vols = None

test/integration/targets/docker_container/tasks/tests/mounts-volumes.yml

@@ -293,6 +293,7 @@
     volumes:
     - "/tmp:/tmp"
     - "/:/whatever:rw,z"
+    - "/anon:rw"
   register: volumes_1
 
 - name: volumes (idempotency)
@@ -304,6 +305,7 @@
     volumes:
     - "/:/whatever:rw,z"
     - "/tmp:/tmp"
+    - "/anon:rw"
   register: volumes_2
 
 - name: volumes (less volumes)
@@ -363,9 +365,12 @@
 - assert:
     that:
     - volumes_1 is changed
+    - volumes_1.container.Config.Volumes | length == 1
+    - volumes_1.container.Config.Volumes['/anon:rw'] | length == 0
     - volumes_2 is not changed
     - volumes_3 is not changed
     - volumes_4 is changed
+    - not volumes_4.container.Config.Volumes
     - volumes_5 is changed
     - volumes_6 is failed
     - "'The mount point \"/tmp\" appears twice in the volumes option' in volumes_6.msg"