From d64b17731d0a0d39b47ae16b54e7b744063aa42c Mon Sep 17 00:00:00 2001
From: Felix Fontein <felix@fontein.de>
Date: Sat, 13 Apr 2019 00:36:11 +0200
Subject: [PATCH] docker_container: improve log_options idempotency by
 converting to string (#54955)

* Warn when log_options values are not strings.

* Add changelog.

* Improve message.

* Improve formatting and formulation of other messages.

* Add test for warning.

* Trying double escaping.
---
 ...955-docker_container-log_options-strings.yml |  2 ++
 .../modules/cloud/docker/docker_container.py    | 17 ++++++++++++-----
 .../docker_container/tasks/tests/options.yml    |  6 +++++-
 3 files changed, 19 insertions(+), 6 deletions(-)
 create mode 100644 changelogs/fragments/54955-docker_container-log_options-strings.yml

diff --git a/changelogs/fragments/54955-docker_container-log_options-strings.yml b/changelogs/fragments/54955-docker_container-log_options-strings.yml
new file mode 100644
index 0000000000..7774530ad1
--- /dev/null
+++ b/changelogs/fragments/54955-docker_container-log_options-strings.yml
@@ -0,0 +1,2 @@
+bugfixes:
+- "docker_container - fix idempotency of ``log_options`` when non-string values are used. Also warn user that this is the case."
diff --git a/lib/ansible/modules/cloud/docker/docker_container.py b/lib/ansible/modules/cloud/docker/docker_container.py
index af4007fb05..f5a1905c37 100644
--- a/lib/ansible/modules/cloud/docker/docker_container.py
+++ b/lib/ansible/modules/cloud/docker/docker_container.py
@@ -1586,7 +1586,14 @@ class TaskParameters(DockerBaseClass):
         if self.log_options is not None:
             options['Config'] = dict()
             for k, v in self.log_options.items():
-                options['Config'][k] = str(v)
+                if not isinstance(v, string_types):
+                    self.client.module.warn(
+                        "Non-string value found for log_options option '%s'. The value is automatically converted to '%s'. "
+                        "If this is not correct, or you want to avoid such warnings, please quote the value." % (k, str(v))
+                    )
+                v = str(v)
+                self.log_options[k] = v
+                options['Config'][k] = v
 
         try:
             return LogConfig(**options)
@@ -1622,8 +1629,8 @@ class TaskParameters(DockerBaseClass):
         if self.env:
             for name, value in self.env.items():
                 if not isinstance(value, string_types):
-                    self.fail("Non-string value found for env option. "
-                              "Ambiguous env options must be wrapped in quotes to avoid YAML parsing. Key: %s" % (name, ))
+                    self.fail("Non-string value found for env option. Ambiguous env options must be "
+                              "wrapped in quotes to avoid them being interpreted. Key: %s" % (name, ))
                 final_env[name] = str(value)
         return final_env
 
@@ -2761,8 +2768,8 @@ class AnsibleDockerClientContainer(AnsibleDockerClient):
                 key_main = comp_aliases.get(key)
                 if key_main is None:
                     if key_main in all_options:
-                        self.fail(("The module option '%s' cannot be specified in the comparisons dict," +
-                                   " since it does not correspond to container's state!") % key)
+                        self.fail("The module option '%s' cannot be specified in the comparisons dict, "
+                                  "since it does not correspond to container's state!" % key)
                     self.fail("Unknown module option '%s' in comparisons dict!" % key)
                 if key_main in comp_aliases_used:
                     self.fail("Both '%s' and '%s' (aliases of %s) are specified in comparisons dict!" % (key, comp_aliases_used[key_main], key_main))
diff --git a/test/integration/targets/docker_container/tasks/tests/options.yml b/test/integration/targets/docker_container/tasks/tests/options.yml
index 0f07f06277..7808c06b66 100644
--- a/test/integration/targets/docker_container/tasks/tests/options.yml
+++ b/test/integration/targets/docker_container/tasks/tests/options.yml
@@ -2125,6 +2125,7 @@
     log_options:
       labels: production_status
       env: os,customer
+      max-file: 5
   register: log_options_1
 
 - name: log_options (idempotency)
@@ -2137,6 +2138,7 @@
     log_options:
       env: os,customer
       labels: production_status
+      max-file: 5
   register: log_options_2
 
 - name: log_options (less log options)
@@ -2159,7 +2161,7 @@
     log_driver: json-file
     log_options:
       labels: production_status
-      max-file: 1
+      max-size: 10m
     force_kill: yes
   register: log_options_4
 
@@ -2174,6 +2176,8 @@
     that:
     - log_options_1 is changed
     - log_options_2 is not changed
+    - "'Non-string value found for log_options option \\'max-file\\'. The value is automatically converted to \\'5\\'. If this is not correct, or you want to
+avoid such warnings, please quote the value.' in log_options_2.warnings"
     - log_options_3 is not changed
     - log_options_4 is changed