docker_container: fix env_file option (#452)

* Add better tests for env and env_file.

* Make sure that non-container options are also passed to preprocessing code.

* Add changelog fragment.

* Add env_file override test.

changelogs/fragments/452-docker_container-env_file.yml

@@ -0,0 +1,2 @@
+bugfixes:
+  - "docker_container - fix handling of ``env_file`` (https://github.com/ansible-collections/community.docker/issues/451, https://github.com/ansible-collections/community.docker/pull/452)."

plugins/module_utils/module_container/base.py

@@ -123,6 +123,7 @@ class OptionGroup(object):
                 return values
         self.preprocess = preprocess
         self.options = []
+        self.all_options = []
         self.engines = {}
         self.ansible_mutually_exclusive = ansible_mutually_exclusive or []
         self.ansible_required_together = ansible_required_together or []
@@ -135,6 +136,7 @@ class OptionGroup(object):
         option = Option(*args, owner=self, **kwargs)
         if not option.not_a_container_option:
             self.options.append(option)
+        self.all_options.append(option)
         if not option.not_an_ansible_option:
             ansible_option = {
                 'type': option.ansible_type,

plugins/module_utils/module_container/module.py

@@ -258,7 +258,7 @@ class ContainerManager(DockerBaseClass):
         for options in active_options:
             values = {}
             engine = options.get_engine(self.engine_driver.name)
-            for option in options.options:
+            for option in options.all_options:
                 if not option.not_an_ansible_option and self.module.params[option.name] is not None:
                     values[option.name] = self.module.params[option.name]
             values = options.preprocess(self.module, values)

tests/integration/targets/docker_container/tasks/tests/options.yml

@@ -1589,8 +1589,16 @@
 - assert:
     that:
     - env_1 is changed
+    - "'TEST1=val1' in env_1.container.Config.Env"
+    - "'TEST2=val2' in env_1.container.Config.Env"
+    - "'TEST3=False' in env_1.container.Config.Env"
+    - "'TEST4=true' in env_1.container.Config.Env"
+    - "'TEST5=yes' in env_1.container.Config.Env"
     - env_2 is not changed
     - env_3 is not changed
+    - "'TEST1=val1' in env_4.container.Config.Env"
+    - "'TEST2=val2' not in env_4.container.Config.Env"
+    - "'TEST3=val3' in env_4.container.Config.Env"
     - env_4 is changed
     - env_5 is failed
     - "('Non-string value found for env option.') in env_5.msg"
@@ -1622,6 +1630,54 @@
     env_file: "{{ remote_tmp_dir }}/env-file"
   register: env_file_2
 
+- name: env_file (with env, idempotent)
+  docker_container:
+    image: "{{ docker_test_image_alpine }}"
+    command: '/bin/sh -c "sleep 10m"'
+    name: "{{ cname }}"
+    state: started
+    env_file: "{{ remote_tmp_dir }}/env-file"
+    env:
+      TEST3: val3
+  register: env_file_3
+
+- name: env_file (with env)
+  docker_container:
+    image: "{{ docker_test_image_alpine }}"
+    command: '/bin/sh -c "sleep 10m"'
+    name: "{{ cname }}"
+    state: started
+    env_file: "{{ remote_tmp_dir }}/env-file"
+    env:
+      TEST1: val1
+      TEST3: val3
+    force_kill: yes
+  register: env_file_4
+
+- name: env_file (with env, idempotent)
+  docker_container:
+    image: "{{ docker_test_image_alpine }}"
+    command: '/bin/sh -c "sleep 10m"'
+    name: "{{ cname }}"
+    state: started
+    env_file: "{{ remote_tmp_dir }}/env-file"
+    env:
+      TEST1: val1
+  register: env_file_5
+
+- name: env_file (with env, override)
+  docker_container:
+    image: "{{ docker_test_image_alpine }}"
+    command: '/bin/sh -c "sleep 10m"'
+    name: "{{ cname }}"
+    state: started
+    env_file: "{{ remote_tmp_dir }}/env-file"
+    env:
+      TEST2: val2
+      TEST4: val4alt
+    force_kill: yes
+  register: env_file_6
+
 - name: cleanup
   docker_container:
     name: "{{ cname }}"
@@ -1632,7 +1688,19 @@
 - assert:
     that:
     - env_file_1 is changed
+    - "'TEST3=val3' in env_file_1.container.Config.Env"
+    - "'TEST4=val4' in env_file_1.container.Config.Env"
     - env_file_2 is not changed
+    - env_file_3 is not changed
+    - env_file_4 is changed
+    - "'TEST1=val1' in env_file_4.container.Config.Env"
+    - "'TEST3=val3' in env_file_4.container.Config.Env"
+    - "'TEST4=val4' in env_file_4.container.Config.Env"
+    - env_file_5 is not changed
+    - env_file_6 is changed
+    - "'TEST2=val2' in env_file_6.container.Config.Env"
+    - "'TEST3=val3' in env_file_6.container.Config.Env"
+    - "'TEST4=val4alt' in env_file_6.container.Config.Env"
 
 ####################################################################
 ## etc_hosts #######################################################