Merge pull request #1742 from dgoodwin/docker-logging

Fixes For Docker Log Options

filter_plugins/oo_filters.py

@@ -259,8 +259,11 @@ class FilterModule(object):
 
     @staticmethod
     def oo_split(string, separator=','):
-        """ This splits the input string into a list
+        """ This splits the input string into a list. If the input string is
+        already a list we will return it as is.
         """
+        if isinstance(string, list):
+            return string
         return string.split(separator)
 
     @staticmethod

inventory/byo/hosts.aep.example

@@ -75,7 +75,7 @@ deployment_type=atomic-enterprise
 #openshift_docker_options="-l warn --ipv6=false"
 # Deprecated methods to set --log-driver and --log-opts flags, use openshift_docker_options instead
 #openshift_docker_log_driver=json
-#openshift_docker_log_options="tag=mailer"
+#openshift_docker_log_options=["tag=mailer"]
 
 # Alternate image format string. If you're not modifying the format string and
 # only need to inject your own registry you may want to consider

inventory/byo/hosts.origin.example

@@ -76,7 +76,7 @@ deployment_type=origin
 #openshift_docker_options="-l warn --ipv6=false"
 # Deprecated methods to set --log-driver and --log-opts flags, use openshift_docker_options instead
 #openshift_docker_log_driver=json
-#openshift_docker_log_options="tag=mailer"
+#openshift_docker_log_options=["tag=mailer"]
 
 # Alternate image format string. If you're not modifying the format string and
 # only need to inject your own registry you may want to consider

inventory/byo/hosts.ose.example

@@ -75,7 +75,7 @@ deployment_type=openshift-enterprise
 #openshift_docker_options="-l warn --ipv6=false"
 # Deprecated methods to set --log-driver and --log-opts flags, use openshift_docker_options instead
 #openshift_docker_log_driver=json
-#openshift_docker_log_options="tag=mailer"
+#openshift_docker_log_options=["tag=mailer"]
 
 
 # Alternate image format string. If you're not modifying the format string and

playbooks/common/openshift-cluster/config.yml

@@ -4,7 +4,7 @@
 - include: validate_hostnames.yml
 
 - name: Set oo_options
-  hosts: oo_hosts_to_config
+  hosts: oo_all_hosts
   tasks:
   - set_fact:
       openshift_docker_additional_registries: "{{ lookup('oo_option', 'docker_additional_registries') }}"

roles/openshift_facts/library/openshift_facts.py

@@ -56,6 +56,13 @@ def migrate_docker_facts(facts):
     if 'node' in facts and 'portal_net' in facts['node']:
         facts['docker']['hosted_registry_insecure'] = True
         facts['docker']['hosted_registry_network'] = facts['node'].pop('portal_net')
+
+    # log_options was originally meant to be a comma separated string, but
+    # we now prefer an actual list, with backward compatability:
+    if 'log_options' in facts['docker'] and \
+            isinstance(facts['docker']['log_options'], basestring):
+        facts['docker']['log_options'] = facts['docker']['log_options'].split(",")
+
     return facts
 
 # TODO: We should add a generic migration function that takes source and destination
@@ -1852,6 +1859,10 @@ class OpenShiftFacts(object):
                     if isinstance(val, basestring):
                         val = [x.strip() for x in val.split(',')]
                     new_local_facts['docker'][key] = list(set(val) - set(['']))
+            # Convert legacy log_options comma sep string to a list if present:
+            if 'log_options' in new_local_facts['docker'] and \
+                    isinstance(new_local_facts['docker']['log_options'], basestring):
+                new_local_facts['docker']['log_options'] = new_local_facts['docker']['log_options'].split(',')
 
         new_local_facts = self.remove_empty_facts(new_local_facts)
 
@@ -1875,7 +1886,7 @@ class OpenShiftFacts(object):
             if isinstance(facts[fact], dict):
                 facts[fact] = self.remove_empty_facts(facts[fact])
             else:
-                if value == "" or value is None:
+                if value == "" or value == [""] or value is None:
                     facts_to_remove.append(fact)
         for fact in facts_to_remove:
             del facts[fact]