docker_swarm_service: Extend env and add env_files support (#51762)

* Extend env and add env_files support

* Python 2.6 compat

* Handle lists passed as string

* Add changelog fragment

* Use correct link formatting

Co-Authored-By: hannseman <hannes@5monkeys.se>

* Fix typo

Co-Authored-By: hannseman <hannes@5monkeys.se>

* Handle empty env and env_files values

changelogs/fragments/51762-docker_swarm_service-extend-env-and-add-env-file.yml

@@ -0,0 +1,3 @@
+minor_changes:
+  - "docker_swarm_service - ``env`` parameter now supports setting values as a dict."
+  - "docker_swarm_service - Added support for ``env_files`` parameter."

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

@@ -124,10 +124,22 @@ options:
       - vip
       - dnsrr
   env:
+    type: raw
+    description:
+      - List or dictionary of the service environment variables.
+      - If passed a list each items need to be in the format of C(KEY=VALUE).
+      - If passed a dictionary values which might be parsed as numbers,
+        booleans or other types by the YAML parser must be quoted (e.g. C("true"))
+        in order to avoid data loss.
+      - Corresponds to the C(--env) option of C(docker service create).
+  env_files:
     type: list
     description:
-      - List of the service environment variables.
+      - List of paths to files, present on the target, containing environment variables C(FOO=BAR).
-      - Corresponds to the C(--env) option of C(docker service create).
+      - The order of the list is significant in determining the value assigned to a
+        variable that shows up more than once.
+      - If variable also present in I(env), then I(env) value will override.
+    version_added: "2.8"
   log_driver:
     type: str
     description:
@@ -568,13 +580,61 @@ from ansible.module_utils._text import to_text
 
 try:
     from docker import types
-    from docker.utils import parse_repository_tag
+    from docker.utils import (
+        parse_repository_tag,
+        parse_env_file,
+        format_environment
+    )
     from docker.errors import APIError, DockerException
 except ImportError:
     # missing docker-py handled in ansible.module_utils.docker.common
     pass
 
 
+def get_docker_environment(env, env_files):
+    """
+    Will return a list of "KEY=VALUE" items. Supplied env variable can
+    be either a list or a dictionary.
+
+    If environment files are combined with explicit environment variables,
+    the explicit environment variables take precedence.
+    """
+    env_dict = {}
+    if env_files:
+        for env_file in env_files:
+            parsed_env_file = parse_env_file(env_file)
+            for name, value in parsed_env_file.items():
+                env_dict[name] = str(value)
+    if env is not None and isinstance(env, string_types):
+        env = env.split(',')
+    if env is not None and isinstance(env, dict):
+        for name, value in env.items():
+            if not isinstance(value, string_types):
+                raise ValueError(
+                    'Non-string value found for env option. '
+                    'Ambiguous env options must be wrapped in quotes to avoid YAML parsing. Key: %s' % name
+                )
+            env_dict[name] = str(value)
+    elif env is not None and isinstance(env, list):
+        for item in env:
+            try:
+                name, value = item.split('=', 1)
+            except ValueError:
+                raise ValueError('Invalid environment variable found in list, needs to be in format KEY=VALUE.')
+            env_dict[name] = value
+    elif env is not None:
+        raise ValueError(
+            'Invalid type for env %s (%s). Only list or dict allowed.' % (env, type(env))
+        )
+    env_list = format_environment(env_dict)
+    if not env_list:
+        if env is not None or env_files is not None:
+            return []
+        else:
+            return None
+    return sorted(env_list)
+
+
 class DockerService(DockerBaseClass):
     def __init__(self):
         super(DockerService, self).__init__()
@@ -674,7 +734,6 @@ class DockerService(DockerBaseClass):
         s.dns_options = ap['dns_options']
         s.hostname = ap['hostname']
         s.tty = ap['tty']
-        s.env = ap['env']
         s.log_driver = ap['log_driver']
         s.log_driver_options = ap['log_driver_options']
         s.labels = ap['labels']
@@ -724,6 +783,8 @@ class DockerService(DockerBaseClass):
                 % (s.command, type(s.command))
             )
 
+        s.env = get_docker_environment(ap['env'], ap['env_files'])
+
         if ap['force_update']:
             s.force_update = int(str(time.time()).replace('.', ''))
 
@@ -1487,7 +1548,8 @@ def main():
         networks=dict(type='list'),
         command=dict(type='raw'),
         args=dict(type='list'),
-        env=dict(type='list'),
+        env=dict(type='raw'),
+        env_files=dict(type='list', elements='path'),
         force_update=dict(default=False, type='bool'),
         log_driver=dict(type='str'),
         log_driver_options=dict(type='dict'),

test/integration/targets/docker_swarm_service/files/env-file-1

@@ -0,0 +1,2 @@
+TEST3=val3
+TEST4=val4

test/integration/targets/docker_swarm_service/files/env-file-2

@@ -0,0 +1,2 @@
+TEST3=val5
+TEST5=val5

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

@@ -704,8 +704,8 @@
     image: alpine:3.8
     command: '/bin/sh -v -c "sleep 10m"'
     env:
-      - "TEST1=val1"
+      TEST1: val1
-      - "TEST2=val2"
+      TEST2: val2
   register: env_2
 
 - name: env (changes)
@@ -734,6 +734,25 @@
     env: []
   register: env_5
 
+- name: env (fail unwrapped values)
+  docker_swarm_service:
+    name: "{{ service_name }}"
+    image: alpine:3.8
+    env:
+      TEST1: true
+  register: env_6
+  ignore_errors: yes
+
+- name: env (fail invalid formatted string)
+  docker_swarm_service:
+    name: "{{ service_name }}"
+    image: alpine:3.8
+    env:
+      - "TEST1=val3"
+      - "TEST2"
+  register: env_7
+  ignore_errors: yes
+
 - name: cleanup
   docker_swarm_service:
     name: "{{ service_name }}"
@@ -747,6 +766,85 @@
       - env_3 is changed
       - env_4 is changed
       - env_5 is not changed
+      - env_6 is failed
+      - env_7 is failed
+
+####################################################################
+## env_files #######################################################
+####################################################################
+
+- name: env_files
+  docker_swarm_service:
+    name: "{{ service_name }}"
+    image: alpine:3.8
+    env_files:
+      - "{{ role_path }}/files/env-file-1"
+  register: env_file_1
+
+- name: env_files (idempotency)
+  docker_swarm_service:
+    name: "{{ service_name }}"
+    image: alpine:3.8
+    env_files:
+      - "{{ role_path }}/files/env-file-1"
+  register: env_file_2
+
+- name: env_files (more items)
+  docker_swarm_service:
+    name: "{{ service_name }}"
+    image: alpine:3.8
+    env_files:
+      - "{{ role_path }}/files/env-file-1"
+      - "{{ role_path }}/files/env-file-2"
+  register: env_file_3
+
+- name: env_files (order)
+  docker_swarm_service:
+    name: "{{ service_name }}"
+    image: alpine:3.8
+    env_files:
+      - "{{ role_path }}/files/env-file-2"
+      - "{{ role_path }}/files/env-file-1"
+  register: env_file_4
+
+- name: env_files (multiple idempotency)
+  docker_swarm_service:
+    name: "{{ service_name }}"
+    image: alpine:3.8
+    env_files:
+      - "{{ role_path }}/files/env-file-2"
+      - "{{ role_path }}/files/env-file-1"
+  register: env_file_5
+
+- name: env_files (empty)
+  docker_swarm_service:
+    name: "{{ service_name }}"
+    image: alpine:3.8
+    env_files: []
+  register: env_file_6
+
+- name: env_files (empty idempotency)
+  docker_swarm_service:
+    name: "{{ service_name }}"
+    image: alpine:3.8
+    env_files: []
+  register: env_file_7
+
+- name: cleanup
+  docker_swarm_service:
+    name: "{{ service_name }}"
+    state: absent
+  diff: no
+
+- assert:
+    that:
+    - env_file_1 is changed
+    - env_file_2 is not changed
+    - env_file_3 is changed
+    - env_file_4 is changed
+    - env_file_5 is not changed
+    - env_file_6 is changed
+    - env_file_7 is not changed
 
 ###################################################################
 ## force_update ###################################################

test/units/modules/cloud/docker/test_docker_swarm_service.py

@@ -2,7 +2,6 @@ import pytest
 
 
 class APIErrorMock(Exception):
-
     def __init__(self, message, response=None, explanation=None):
         self.message = message
         self.response = response
@@ -26,6 +25,7 @@ def docker_module_mock(mocker):
 @pytest.fixture(autouse=True)
 def docker_swarm_service():
     from ansible.modules.cloud.docker import docker_swarm_service
+
     return docker_swarm_service
 
 
@@ -46,14 +46,55 @@ def test_retry_on_out_of_sequence_error(mocker, docker_swarm_service):
 
 def test_no_retry_on_general_api_error(mocker, docker_swarm_service):
     run_mock = mocker.MagicMock(
-        side_effect=APIErrorMock(
+        side_effect=APIErrorMock(message='', response=None, explanation='some error')
-            message='',
-            response=None,
-            explanation='some error',
-        )
     )
     manager = docker_swarm_service.DockerServiceManager(client=None)
     manager.run = run_mock
     with pytest.raises(APIErrorMock):
         manager.run_safe()
     assert run_mock.call_count == 1
+
+
+def test_get_docker_environment(mocker, docker_swarm_service):
+    env_file_result = {'TEST1': 'A', 'TEST2': 'B', 'TEST3': 'C'}
+    env_dict = {'TEST3': 'CC', 'TEST4': 'D'}
+    env_string = "TEST3=CC,TEST4=D"
+
+    env_list = ['TEST3=CC', 'TEST4=D']
+    expected_result = sorted(['TEST1=A', 'TEST2=B', 'TEST3=CC', 'TEST4=D'])
+    mocker.patch.object(
+        docker_swarm_service, 'parse_env_file', return_value=env_file_result
+    )
+    mocker.patch.object(
+        docker_swarm_service,
+        'format_environment',
+        side_effect=lambda d: ['{0}={1}'.format(key, value) for key, value in d.items()],
+    )
+    # Test with env dict and file
+    result = docker_swarm_service.get_docker_environment(
+        env_dict, env_files=['dummypath']
+    )
+    assert result == expected_result
+    # Test with env list and file
+    result = docker_swarm_service.get_docker_environment(
+        env_list,
+        env_files=['dummypath']
+    )
+    assert result == expected_result
+    # Test with env string and file
+    result = docker_swarm_service.get_docker_environment(
+        env_string, env_files=['dummypath']
+    )
+    assert result == expected_result
+
+    assert result == expected_result
+    # Test with empty env
+    result = docker_swarm_service.get_docker_environment(
+        [], env_files=None
+    )
+    assert result == []
+    # Test with empty env_files
+    result = docker_swarm_service.get_docker_environment(
+        None, env_files=[]
+    )
+    assert result == []