From 70d8f02db7c27cf17f24373150b4db64922eacd3 Mon Sep 17 00:00:00 2001 From: Hannes Ljungberg Date: Tue, 12 Feb 2019 03:06:58 -0500 Subject: [PATCH] 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 * Fix typo Co-Authored-By: hannseman * Handle empty env and env_files values --- ...rm_service-extend-env-and-add-env-file.yml | 3 + .../cloud/docker/docker_swarm_service.py | 72 ++++++++++++- .../docker_swarm_service/files/env-file-1 | 2 + .../docker_swarm_service/files/env-file-2 | 2 + .../tasks/tests/options.yml | 102 +++++++++++++++++- .../cloud/docker/test_docker_swarm_service.py | 53 +++++++-- 6 files changed, 221 insertions(+), 13 deletions(-) create mode 100644 changelogs/fragments/51762-docker_swarm_service-extend-env-and-add-env-file.yml create mode 100644 test/integration/targets/docker_swarm_service/files/env-file-1 create mode 100644 test/integration/targets/docker_swarm_service/files/env-file-2 diff --git a/changelogs/fragments/51762-docker_swarm_service-extend-env-and-add-env-file.yml b/changelogs/fragments/51762-docker_swarm_service-extend-env-and-add-env-file.yml new file mode 100644 index 00000000000..44d252b2d38 --- /dev/null +++ b/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." diff --git a/lib/ansible/modules/cloud/docker/docker_swarm_service.py b/lib/ansible/modules/cloud/docker/docker_swarm_service.py index 687872d1de3..9d21490c1d4 100644 --- a/lib/ansible/modules/cloud/docker/docker_swarm_service.py +++ b/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. - - Corresponds to the C(--env) option of C(docker service create). + - List of paths to files, present on the target, containing environment variables C(FOO=BAR). + - 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'), diff --git a/test/integration/targets/docker_swarm_service/files/env-file-1 b/test/integration/targets/docker_swarm_service/files/env-file-1 new file mode 100644 index 00000000000..b15f1b64cfa --- /dev/null +++ b/test/integration/targets/docker_swarm_service/files/env-file-1 @@ -0,0 +1,2 @@ +TEST3=val3 +TEST4=val4 diff --git a/test/integration/targets/docker_swarm_service/files/env-file-2 b/test/integration/targets/docker_swarm_service/files/env-file-2 new file mode 100644 index 00000000000..eff99aca79e --- /dev/null +++ b/test/integration/targets/docker_swarm_service/files/env-file-2 @@ -0,0 +1,2 @@ +TEST3=val5 +TEST5=val5 diff --git a/test/integration/targets/docker_swarm_service/tasks/tests/options.yml b/test/integration/targets/docker_swarm_service/tasks/tests/options.yml index 239746074c6..bef816f27dd 100644 --- a/test/integration/targets/docker_swarm_service/tasks/tests/options.yml +++ b/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" - - "TEST2=val2" + TEST1: val1 + 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 ################################################### diff --git a/test/units/modules/cloud/docker/test_docker_swarm_service.py b/test/units/modules/cloud/docker/test_docker_swarm_service.py index cdb0eeed63b..8542f5ee2f4 100644 --- a/test/units/modules/cloud/docker/test_docker_swarm_service.py +++ b/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( - message='', - response=None, - explanation='some error', - ) + side_effect=APIErrorMock(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 == []