fixes

12 files changed, 104 insertions, 84 deletions

diff --git a/playbooks/common/openshift-docker/config.yml b/playbooks/common/openshift-docker/config.yml
index c3541e544..f838dd66c 100644
--- a/playbooks/common/openshift-docker/config.yml
+++ b/playbooks/common/openshift-docker/config.yml
@@ -5,4 +5,5 @@
     docker_insecure_registries: "{{ lookup('oo_option',  'docker_insecure_registries') | oo_split }}"
     docker_blocked_registries: "{{ lookup('oo_option', 'docker_blocked_registries') | oo_split }}"
   roles:
-  - openshift-docker
+  - openshift_facts
+  - openshift_docker
diff --git a/roles/docker/handlers/main.yml b/roles/docker/handlers/main.yml
index 7d60f1891..9f827417f 100644
--- a/roles/docker/handlers/main.yml
+++ b/roles/docker/handlers/main.yml
@@ -1,7 +1,10 @@
 ---
 
 - name: restart docker
-  service: name=docker state=restarted
+  service:
+    name: docker
+    state: restarted
+  when: not docker_service_status_changed | default(false)
 
 - name: restart udev
   service:
diff --git a/roles/etcd/tasks/main.yml b/roles/etcd/tasks/main.yml
index aef52886b..6906244e9 100644
--- a/roles/etcd/tasks/main.yml
+++ b/roles/etcd/tasks/main.yml
@@ -7,9 +7,6 @@
     msg: IPv4 address not found for {{ etcd_interface }}
   when: "'ipv4' not in hostvars[inventory_hostname]['ansible_' ~ etcd_interface] or 'address' not in hostvars[inventory_hostname]['ansible_' ~ etcd_interface].ipv4"
 
-- debug: var=openshift.common.is_containerized
-- debug: var=openshift.common.is_atomic
-
 - name: Install etcd
   action: "{{ ansible_pkg_mgr }} name=etcd-2.* state=present"
   when: not openshift.common.is_containerized | bool
@@ -25,7 +22,7 @@
     src: etcd.docker.service
   register: install_etcd_result
   when: openshift.common.is_containerized | bool
-  
+
 - name: Ensure etcd datadir exists
   when: openshift.common.is_containerized | bool
   file:
@@ -48,16 +45,16 @@
   file:
     path: "{{ etcd_conf_dir }}"
     state: directory
-    owner: etcd
-    group: etcd
+    owner: "{{ 'etcd' if not openshift.common.is_containerized | bool else omit }}"
+    group: "{{ 'etcd' if not openshift.common.is_containerized | bool else omit }}"
     mode: 0700
 
 - name: Validate permissions on certificate files
   file:
     path: "{{ item }}"
     mode: 0600
-    group: etcd
-    owner: etcd
+    owner: "{{ 'etcd' if not openshift.common.is_containerized | bool else omit }}"
+    group: "{{ 'etcd' if not openshift.common.is_containerized | bool else omit }}"
   when: etcd_url_scheme == 'https'
   with_items:
   - "{{ etcd_ca_file }}"
@@ -68,8 +65,8 @@
   file:
     path: "{{ item }}"
     mode: 0600
-    group: etcd
-    owner: etcd
+    owner: "{{ 'etcd' if not openshift.common.is_containerized | bool else omit }}"
+    group: "{{ 'etcd' if not openshift.common.is_containerized | bool else omit }}"
   when: etcd_peer_url_scheme == 'https'
   with_items:
   - "{{ etcd_peer_ca_file }}"
diff --git a/roles/openshift_docker/tasks/main.yml b/roles/openshift_docker/tasks/main.yml
index f63b3acd5..4f9fe1f97 100644
--- a/roles/openshift_docker/tasks/main.yml
+++ b/roles/openshift_docker/tasks/main.yml
@@ -1,27 +1,27 @@
 ---
 - openshift_facts:
-  - role: common
+    role: common
     local_facts:
       deployment_type: "{{ openshift_deployment_type }}"
-      docker_additional_registries: "{{ docker_additional_registries | oo_split() }}"
-      docker_insecure_registries: "{{ docker_insecure_registries | oo_split() }}"
-      docker_blocked_registries: "{{ docker_blocked_registries | oo_split() }}"
+      docker_additional_registries: "{{ docker_additional_registries }}"
+      docker_insecure_registries: "{{ docker_insecure_registries }}"
+      docker_blocked_registries: "{{ docker_blocked_registries }}"
 
 - name: Set registry params
   lineinfile:
     dest: /etc/sysconfig/docker
-    regexp: '^{{ reg_conf_var }}=.*$'
-    line: "{{ reg_conf_var }}='{{ reg_fact_val | oo_prepend_strings_in_list(reg_flag ~ ' ') | join(' ') }}'"
+    regexp: '^{{ item.reg_conf_var }}=.*$'
+    line: "{{ item.reg_conf_var }}='{{ item.reg_fact_val | oo_prepend_strings_in_list(item.reg_flag ~ ' ') | join(' ') }}'"
   when: "'docker_additional_registries' in openshift.common"
   with_items:
   - reg_conf_var: ADD_REGISTRY
-    reg_fact_val: {{ openshift.common.docker_additional_registries }}
+    reg_fact_val: "{{ openshift.common.docker_additional_registries }}"
     reg_flag: --add-registry
   - reg_conf_var: BLOCK_REGISTRY
-    reg_fact_val: {{ openshift.common.docker_blocked_registries }}
+    reg_fact_val: "{{ openshift.common.docker_blocked_registries }}"
     reg_flag: --block-registry
   - reg_conf_var: INSECURE_REGISTRY
-    reg_fact_val: {{ openshift.common.docker_insecure_registries }}
+    reg_fact_val: "{{ openshift.common.docker_insecure_registries }}"
     reg_flag: --insecure-registry
   notify:
   - restart docker
diff --git a/roles/openshift_examples/defaults/main.yml b/roles/openshift_examples/defaults/main.yml
index d318d17e3..e27f4e0a6 100644
--- a/roles/openshift_examples/defaults/main.yml
+++ b/roles/openshift_examples/defaults/main.yml
@@ -1,14 +1,14 @@
 ---
 # By default install rhel and xpaas streams on enterprise installs
-openshift_examples_load_centos: "{{ openshift_deployment_type not in ['enterprise','openshift-enterprise','atomic-enterprise','online'] }}"
-openshift_examples_load_rhel: "{{ openshift_deployment_type in ['enterprise','openshift-enterprise','atomic-enterprise','online'] }}"
+openshift_examples_load_centos: "{{ openshift_deployment_type == 'origin' }}"
+openshift_examples_load_rhel: "{{ openshift_deployment_type != 'origin' }}"
 openshift_examples_load_db_templates: true
-openshift_examples_load_xpaas: "{{ openshift_deployment_type in ['enterprise','openshift-enterprise','atomic-enterprise','online']  }}"
+openshift_examples_load_xpaas: "{{ openshift_deployment_type != 'origin' }}"
 openshift_examples_load_quickstarts: true
 
 content_version: "{{ 'v1.1' if openshift.common.version_greater_than_3_1_or_1_1 else 'v1.0' }}"
 
-examples_base: "{% if openshift.common.is_atomic %}{{ openshift.common.config_base }}{% else %}/usr/share/openshift{% endif %}/examples"
+examples_base: "{{ openshift.common.config_base if openshift.common.is_containerized else '/usr/share/openshift' }}/examples"
 image_streams_base: "{{ examples_base }}/image-streams"
 centos_image_streams: "{{ image_streams_base}}/image-streams-centos7.json"
 rhel_image_streams: "{{ image_streams_base}}/image-streams-rhel7.json"
diff --git a/roles/openshift_facts/library/openshift_facts.py b/roles/openshift_facts/library/openshift_facts.py
index 8e733a3a2..b5454dd81 100755
--- a/roles/openshift_facts/library/openshift_facts.py
+++ b/roles/openshift_facts/library/openshift_facts.py
@@ -647,14 +647,14 @@ def set_deployment_facts_if_unset(facts):
         for cat in  ['additional', 'blocked', 'insecure']:
             key = 'docker_{0}_registries'.format(cat)
             if key in facts['common']:
-                facts['common'][key] = set(facts['common'][key]) - set([''])
+                facts['common'][key] = list(set(facts['common'][key]) - set(['']))
 
 
         if deployment_type in ['enterprise', 'atomic-enterprise', 'openshift-enterprise']:
-            addtl_regs = facts['common']['docker_additional_registries']:
-            ent_reg = 'registry.access.redhat.com'
-            if ent_reg not in addtl_regs
-                facts['common']['docker_additional_registries'].append(ent_reg)
+            addtl_regs = facts['common'].get('docker_additional_registries', [])
+            ent_reg = ['registry.access.redhat.com']
+            if ent_reg not in addtl_regs:
+                facts['common']['docker_additional_registries'] = addtl_regs + ent_reg
 
 
     for role in ('master', 'node'):
@@ -934,6 +934,7 @@ def save_local_facts(filename, facts):
             os.makedirs(fact_dir)
         with open(filename, 'w') as fact_file:
             fact_file.write(module.jsonify(facts))
+        os.chmod(filename, 0o600)
     except (IOError, OSError) as ex:
         raise OpenShiftFactsFileWriteError(
             "Could not create fact file: %s, error: %s" % (filename, ex)
@@ -969,6 +970,69 @@ def get_local_facts_from_file(filename):
     return local_facts
 
 
+def set_container_facts_if_unset(facts):
+    """ Set containerized facts.
+
+        Args:
+            facts (dict): existing facts
+        Returns:
+            dict: the facts dict updated with the generated containerization
+            facts
+    """
+    deployment_type = facts['common']['deployment_type']
+    if deployment_type in ['enterprise', 'openshift-enterprise']:
+        master_image = 'openshift3/ose'
+        cli_image = master_image
+        node_image = 'openshift3/node'
+        ovs_image = 'openshift3/openvswitch'
+        etcd_image = 'registry.access.redhat.com/rhel7/etcd'
+    elif deployment_type == 'atomic-enterprise':
+        master_image = 'aep3_beta/aep'
+        cli_image = master_image
+        node_image = 'aep3_beta/node'
+        ovs_image = 'aep3_beta/openvswitch'
+        etcd_image = 'registry.access.redhat.com/rhel7/etcd'
+    else:
+        master_image = 'openshift/origin'
+        cli_image = master_image
+        node_image = 'openshift/node'
+        ovs_image = 'openshift/openvswitch'
+        etcd_image = 'registry.access.redhat.com/rhel7/etcd'
+
+    facts['common']['is_atomic'] = os.path.isfile('/run/ostree-booted')
+    if 'is_containerized' not in facts['common']:
+        facts['common']['is_containerized'] = facts['common']['is_atomic']
+    if 'cli_image' not in facts['common']:
+        facts['common']['cli_image'] = cli_image
+    if 'etcd' in facts and 'etcd_image' not in facts['etcd']:
+        facts['etcd']['etcd_image'] = etcd_image
+    if 'master' in facts and 'master_image' not in facts['master']:
+        facts['master']['master_image'] = master_image
+    if 'node' in facts:
+        if 'node_image' not in facts['node']:
+            facts['node']['node_image'] = node_image
+        if 'ovs_image' not in facts['node']:
+            facts['node']['ovs_image'] = ovs_image
+
+    # shared /tmp/openshift vol is for file exchange with ansible
+    # --privileged is required to read the config dir
+    # --net host to access openshift from the container
+    # maybe -v /var/run/docker.sock:/var/run/docker.sock is required as well
+    runner = ("docker run --rm --privileged --net host -v "
+              "/tmp/openshift:/tmp/openshift -v {datadir}:{datadir} "
+              "-v {confdir}:{confdir} "
+              "-e KUBECONFIG={confdir}/master/admin.kubeconfig "
+              "{image}").format(confdir=facts['common']['config_base'],
+                                datadir=facts['common']['data_dir'],
+                                image=facts['common']['cli_image'])
+
+    if facts['common']['is_containerized']:
+        facts['common']['client_binary'] = '%s cli' % runner
+        facts['common']['admin_binary'] = '%s admin' % runner
+
+    return facts
+
+
 class OpenShiftFactsUnsupportedRoleError(Exception):
     """Origin Facts Unsupported Role Error"""
     pass
@@ -1046,7 +1110,7 @@ class OpenShiftFacts(object):
         facts = set_version_facts_if_unset(facts)
         facts = set_aggregate_facts(facts)
         facts = set_etcd_facts_if_unset(facts)
-        facts = self.set_containerized_facts_if_unset(facts)
+        facts = set_container_facts_if_unset(facts)
         return dict(openshift=facts)
 
     def get_defaults(self, roles):
@@ -1213,56 +1277,6 @@ class OpenShiftFacts(object):
         self.changed = changed
         return new_local_facts
 
-    def set_containerized_facts_if_unset(self, facts):
-        deployment_type = facts['common']['deployment_type']
-        if deployment_type in ['enterprise','openshift-enterprise']:
-            master_image = 'openshift3/ose'
-            cli_image = master_image
-            node_image = 'openshift3/node'
-            ovs_image = 'openshift3/openvswitch'
-            etcd_image = 'registry.access.redhat.com/rhel7/etcd'
-        elif deployment_type == 'atomic-enterprise':
-            master_image = 'aep3_beta/aep'
-            cli_image = master_image
-            node_image = 'aep3_beta/node'
-            ovs_image = 'aep3_beta/openvswitch'
-            etcd_image = 'registry.access.redhat.com/rhel7/etcd'
-        else:
-            master_image = 'openshift/origin'
-            cli_image = master_image
-            node_image = 'openshift/node'
-            ovs_image = 'openshift/openvswitch'
-            etcd_image = 'registry.access.redhat.com/rhel7/etcd'
-
-        facts['common']['is_atomic'] = os.path.isfile('/run/ostree-booted')
-        if 'is_containerized' not in facts['common']:
-            facts['common']['is_containerized'] = facts['common']['is_atomic']
-        if 'cli_image' not in facts['common']:
-            facts['common']['cli_image'] = cli_image
-        if 'master' in facts:
-            if 'master_image' not in facts['master']:
-                facts['master']['master_image'] = master_image
-        if 'node' in facts:
-            if 'node_image' not in facts ['node']:
-                facts['node']['node_image'] = node_image
-            if 'ovs_image' not in facts ['node']:
-                facts['node']['ovs_image'] = ovs_image
-        if 'etcd' in facts:
-            if 'etcd_image' not in facts['etcd']:
-                facts['etcd']['etcd_image'] = etcd_image
-
-        # shared /tmp/openshift vol is for file exchange with ansible
-        # --privileged is required to read the config dir
-        # --net host to access openshift from the container
-        # maybe -v /var/run/docker.sock:/var/run/docker.sock is required as well
-        runner = "docker run --rm --privileged --net host -v /tmp/openshift:/tmp/openshift -v {datadir}:{datadir} -v {confdir}:{confdir} -e KUBECONFIG={confdir}/master/admin.kubeconfig {image}".format(confdir=facts['common']['config_base'], datadir=facts['common']['data_dir'], image=facts['common']['cli_image'])
-
-        if facts['common']['is_containerized']:
-            facts['common']['client_binary'] = '%s cli' % runner
-            facts['common']['admin_binary'] = '%s admin' % runner
-
-        return facts
-
 
 def main():
     """ main """
diff --git a/roles/openshift_facts/tasks/main.yml b/roles/openshift_facts/tasks/main.yml
index 832f7ad84..55071436f 100644
--- a/roles/openshift_facts/tasks/main.yml
+++ b/roles/openshift_facts/tasks/main.yml
@@ -9,5 +9,8 @@
 - name: Ensure PyYaml is installed
   action: "{{ ansible_pkg_mgr }} name=PyYAML state=present"
 
-- name: Gather Cluster facts
+- name: Gather Cluster facts and set is_containerized if needed
   openshift_facts:
+    role: common
+    local_facts:
+      is_containerized: "{{ openshift_containerized | default(None) }}"
diff --git a/roles/openshift_manage_node/tasks/main.yml b/roles/openshift_manage_node/tasks/main.yml
index 637e494ea..06f12053a 100644
--- a/roles/openshift_manage_node/tasks/main.yml
+++ b/roles/openshift_manage_node/tasks/main.yml
@@ -5,6 +5,7 @@
   until: omd_get_node.rc == 0
   retries: 20
   delay: 5
+  changed_when: false
   with_items: openshift_nodes
 
 - name: Set node schedulability
diff --git a/roles/openshift_master/tasks/main.yml b/roles/openshift_master/tasks/main.yml
index e9e77d231..0c7718299 100644
--- a/roles/openshift_master/tasks/main.yml
+++ b/roles/openshift_master/tasks/main.yml
@@ -314,6 +314,7 @@
 
 - name: Lookup default group for ansible_ssh_user
   command: "/usr/bin/id -g {{ ansible_ssh_user }}"
+  changed_when: false
   register: _ansible_ssh_user_gid
 
 - name: Create the client config dir(s)
diff --git a/roles/openshift_master/templates/master.docker.service.j2 b/roles/openshift_master/templates/master.docker.service.j2
index 8b23d70b1..23781a313 100644
--- a/roles/openshift_master/templates/master.docker.service.j2
+++ b/roles/openshift_master/templates/master.docker.service.j2
@@ -6,7 +6,7 @@ PartOf=docker.service
 
 [Service]
 EnvironmentFile=/etc/sysconfig/{{ openshift.common.service_type }}-master
-ExecStartPre=-/usr/bin/docker rm -f {{ openshift.common.service_type}}-master
+ExecStartPre=-/usr/bin/docker rm -f {{ openshift.common.service_type }}-master
 ExecStart=/usr/bin/docker run --rm --privileged --net=host --name {{ openshift.common.service_type }}-master -v {{ openshift.common.data_dir }}:{{ openshift.common.data_dir }} -v /var/run/docker.sock:/var/run/docker.sock -v {{ openshift.common.config_base }}:{{ openshift.common.config_base }} {{ openshift.master.master_image }} start master --config=${CONFIG_FILE} $OPTIONS
 ExecStartPost=/usr/bin/sleep 10
 ExecStop=/usr/bin/docker stop {{ openshift.common.service_type }}-master
diff --git a/roles/openshift_node/meta/main.yml b/roles/openshift_node/meta/main.yml
index 9d40ae3b3..c92008a77 100644
--- a/roles/openshift_node/meta/main.yml
+++ b/roles/openshift_node/meta/main.yml
@@ -13,4 +13,3 @@ galaxy_info:
   - cloud
 dependencies:
 - { role: openshift_common }
-- { role: docker }
diff --git a/roles/openshift_serviceaccounts/tasks/main.yml b/roles/openshift_serviceaccounts/tasks/main.yml
index eb9c9b2c9..4c7faa6fe 100644
--- a/roles/openshift_serviceaccounts/tasks/main.yml
+++ b/roles/openshift_serviceaccounts/tasks/main.yml
@@ -23,6 +23,7 @@
   shell: >
     {{ openshift.common.client_binary }} get scc privileged -o yaml
     --output-version=v1 > /tmp/openshift/scc.yaml
+  changed_when: false
 
 - name: Add security context constraint for {{ item }}
   lineinfile: