docker_swarm_service: Don’t remove service when networks change (#52634)

* Don’t remove service when networks change

* Add changelog fragment

* Some more network integration tests

* Add hannseman as author

* Remove return on self.client.fail

changelogs/fragments/52634-docker_swarm_service-dont_remove_service_on_network.yml

@@ -0,0 +1,2 @@
+bugfixes:
+  - "docker_swarm_service - Don't recreate service when ``networks`` parameter changes when running Docker API >= 1.29."

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

@@ -12,7 +12,10 @@ ANSIBLE_METADATA = {'status': ['preview'],
 DOCUMENTATION = '''
 ---
 module: docker_swarm_service
-author: "Dario Zanzico (@dariko), Jason Witkowski (@jwitko)"
+author:
+  - "Dario Zanzico (@dariko)"
+  - "Jason Witkowski (@jwitko)"
+  - "Hannes Ljungberg (@hannseman)"
 short_description: docker swarm service
 description:
   - Manages docker services via a swarm manager node.
@@ -331,6 +334,8 @@ options:
   networks:
     description:
       - List of the service networks names.
+      - Prior to API version 1.29, updating and removing networks is not supported.
+        If changes are made the service will then be removed and recreated.
       - Corresponds to the C(--network) option of C(docker service create).
     type: list
   stop_signal:
@@ -367,6 +372,7 @@ options:
       mode:
         description:
           - What publish mode to use.
+          - Service will be removed and recreated when changed.
           - Requires API version >= 1.32.
         type: str
         choices:
@@ -757,6 +763,7 @@ class DockerService(DockerBaseClass):
         self.update_max_failure_ratio = None
         self.update_order = None
         self.working_dir = None
+        self.can_update_networks = None
 
     def get_facts(self):
         return {
@@ -805,9 +812,10 @@ class DockerService(DockerBaseClass):
         }
 
     @staticmethod
-    def from_ansible_params(ap, old_service, image_digest):
+    def from_ansible_params(ap, old_service, image_digest, can_update_networks):
         s = DockerService()
         s.image = image_digest
+        s.can_update_networks = can_update_networks
         s.constraints = ap['constraints']
         s.placement_preferences = ap['placement_preferences']
         s.args = ap['args']
@@ -962,7 +970,7 @@ class DockerService(DockerBaseClass):
             differences.add('secrets', parameter=self.secrets, active=os.secrets)
         if self.networks is not None and self.networks != (os.networks or []):
             differences.add('networks', parameter=self.networks, active=os.networks)
-            needs_rebuild = True
+            needs_rebuild = not self.can_update_networks
         if self.replicas != os.replicas:
             differences.add('replicas', parameter=self.replicas, active=os.replicas)
         if self.command is not None and self.command != (os.command or []):
@@ -1520,6 +1528,10 @@ class DockerServiceManager(object):
         digest = distribution_data['Descriptor']['digest']
         return '%s@%s' % (name, digest)
 
+    def can_update_networks(self):
+        # Before Docker API 1.29 adding/removing networks was not supported
+        return self.client.docker_api_version >= LooseVersion('1.29')
+
     def run(self):
         self.diff_tracker = DifferenceTracker()
         module = self.client.module
@@ -1532,21 +1544,29 @@ class DockerServiceManager(object):
             )
         except DockerException as e:
             self.client.fail(
-                "Error looking for an image named %s: %s" % (image, e))
+                'Error looking for an image named %s: %s'
+                % (image, e)
+            )
+
         try:
             current_service = self.get_service(module.params['name'])
         except Exception as e:
             self.client.fail(
-                "Error looking for service named %s: %s" % (module.params['name'], e))
+                'Error looking for service named %s: %s'
+                % (module.params['name'], e)
+            )
         try:
+            can_update_networks = self.can_update_networks()
             new_service = DockerService.from_ansible_params(
                 module.params,
                 current_service,
-                image_digest
+                image_digest,
+                can_update_networks
             )
         except Exception as e:
-            self.client.fail(
+            return self.client.fail(
-                "Error parsing module parameters: %s" % e)
+                'Error parsing module parameters: %s' % e
+            )
 
         changed = False
         msg = 'noop'

test/integration/targets/docker_swarm_service/tasks/tests/options.yml

@@ -1551,13 +1551,51 @@
       - "{{ network_name_1 }}"
   register: networks_2
 
+- name: networks (change more)
+  docker_swarm_service:
+    name: "{{ service_name }}"
+    image: alpine:3.8
+    command: '/bin/sh -v -c "sleep 10m"'
+    networks:
+      - "{{ network_name_1 }}"
+      - "{{ network_name_2 }}"
+  register: networks_3
+
+- name: networks (change more idempotency)
+  docker_swarm_service:
+    name: "{{ service_name }}"
+    image: alpine:3.8
+    command: '/bin/sh -v -c "sleep 10m"'
+    networks:
+      - "{{ network_name_1 }}"
+      - "{{ network_name_2 }}"
+  register: networks_4
+
+- name: networks (change less)
+  docker_swarm_service:
+    name: "{{ service_name }}"
+    image: alpine:3.8
+    command: '/bin/sh -v -c "sleep 10m"'
+    networks:
+      - "{{ network_name_2 }}"
+  register: networks_5
+
+- name: networks (change less idempotency)
+  docker_swarm_service:
+    name: "{{ service_name }}"
+    image: alpine:3.8
+    command: '/bin/sh -v -c "sleep 10m"'
+    networks:
+      - "{{ network_name_2 }}"
+  register: networks_6
+
 - name: networks (empty)
   docker_swarm_service:
     name: "{{ service_name }}"
     image: alpine:3.8
     command: '/bin/sh -v -c "sleep 10m"'
     networks: []
-  register: networks_3
+  register: networks_7
 
 - name: networks (empty idempotency)
   docker_swarm_service:
@@ -1565,7 +1603,7 @@
     image: alpine:3.8
     command: '/bin/sh -v -c "sleep 10m"'
     networks: []
-  register: networks_4
+  register: networks_8
 
 - name: cleanup
   docker_swarm_service:
@@ -1579,6 +1617,22 @@
       - networks_2 is not changed
       - networks_3 is changed
       - networks_4 is not changed
+      - networks_5 is changed
+      - networks_6 is not changed
+      - networks_7 is changed
+      - networks_8 is not changed
+
+- assert:
+    that:
+    - networks_3.rebuilt == false
+    - networks_5.rebuilt == false
+  when: docker_api_version is version('1.29', '>=')
+
+- assert:
+    that:
+    - networks_3.rebuilt == true
+    - networks_5.rebuilt == true
+  when: docker_api_version is version('1.29', '<')
 
 ####################################################################
 ## stop_signal #####################################################