Merge pull request #1718 from brenton/docker1

Containerized installs on RHEL were downgrading docker unnecessarily

13 files changed, 54 insertions, 24 deletions

diff --git a/roles/docker/tasks/main.yml b/roles/docker/tasks/main.yml
index 1d8493938..89648b321 100644
--- a/roles/docker/tasks/main.yml
+++ b/roles/docker/tasks/main.yml
@@ -7,22 +7,27 @@
   register: docker_version_result
   changed_when: false
 
+- stat: path=/etc/sysconfig/docker-storage
+  register: docker_storage_check
+
+- name: Remove deferred deletion for downgrades from 1.9
+  command: >
+    sed -i 's/--storage-opt dm.use_deferred_deletion=true//' /etc/sysconfig/docker-storage
+  when: docker_storage_check.stat.exists | bool and not docker_version_result | skipped and docker_version_result.stdout | default('0.0', True) | version_compare('1.9', '>=') and docker_version | version_compare('1.9', '<')
+
 - name: Downgrade docker if necessary
   command: "{{ ansible_pkg_mgr }} downgrade -y docker-{{ docker_version }}"
   register: docker_downgrade_result
   when: not docker_version_result | skipped and docker_version_result.stdout | default('0.0', True) | version_compare(docker_version, 'gt')
 
 - name: Install docker
-  action: "{{ ansible_pkg_mgr }} name=docker{{ '-' + docker_version if docker_version != '' else '' }} state=present"
-  when: not openshift.common.is_atomic | bool and not docker_downgrade_result | changed
+  action: "{{ ansible_pkg_mgr }} name=docker{{ '-' + docker_version if docker_version is defined and docker_version != '' else '' }} state=present"
+  when: not openshift.common.is_atomic | bool and not docker_version_result | skipped and docker_version_result.stdout | default('0.0', True) | version_compare(docker_version, 'lt')
 
-- stat: path=/etc/sysconfig/docker-storage
-  register: docker_storage_check
-
-- name: Remove deferred deletion for downgrades from 1.9
-  command: >
-    sed -i 's/--storage-opt dm.use_deferred_deletion=true//' /etc/sysconfig/docker-storage
-  when: docker_downgrade_result | changed and docker_storage_check.stat.exists | bool and docker_version_result.stdout | default('0.0', True) | version_compare('1.9', '>=') and docker_version | version_compare('1.9', '<')
+# We're getting ready to start docker.  This is a workaround for cases where it
+# seems a package install/upgrade/downgrade has rebooted docker and crashed it.
+- name: Reset docker service state
+  command: systemctl reset-failed docker.service
 
 - name: enable and start the docker service
   service:
diff --git a/roles/etcd/templates/etcd.docker.service b/roles/etcd/templates/etcd.docker.service
index b8dbefa64..cf957ede8 100644
--- a/roles/etcd/templates/etcd.docker.service
+++ b/roles/etcd/templates/etcd.docker.service
@@ -11,6 +11,7 @@ ExecStart=/usr/bin/docker run --name {{ etcd_service }} --rm -v /var/lib/etcd:/v
 ExecStop=/usr/bin/docker stop {{ etcd_service }}
 SyslogIdentifier=etcd_container
 Restart=always
+RestartSec=5s
 
 [Install]
 WantedBy=docker.service
diff --git a/roles/openshift_common/tasks/main.yml b/roles/openshift_common/tasks/main.yml
index 59ca659b5..eda43b9f8 100644
--- a/roles/openshift_common/tasks/main.yml
+++ b/roles/openshift_common/tasks/main.yml
@@ -33,7 +33,7 @@
 # versions or image tags.  openshift_common's usage requires that it be a RPM
 # version and openshift_cli expects it to be an image tag.
 - name: Install the base package for versioning
-  action: "{{ ansible_pkg_mgr }} name={{ openshift.common.service_type }}{{ openshift_version | default('') | oo_image_tag_to_rpm_version }} state=present"
+  action: "{{ ansible_pkg_mgr }} name={{ openshift.common.service_type }}{{ openshift_version | default('') | oo_image_tag_to_rpm_version(include_dash=True) }} state=present"
   when: not openshift.common.is_containerized | bool
 
 - name: Set version facts
diff --git a/roles/openshift_docker_facts/tasks/main.yml b/roles/openshift_docker_facts/tasks/main.yml
index eb762e33f..7ea359af1 100644
--- a/roles/openshift_docker_facts/tasks/main.yml
+++ b/roles/openshift_docker_facts/tasks/main.yml
@@ -46,10 +46,24 @@
   register: common_version
   failed_when: false
   changed_when: false
-  when: not openshift.common.is_atomic | bool
+  when: not openshift.common.is_containerized | bool
+
+- set_fact:
+    l_common_version: "{{ openshift.common.image_tag | default('0.0', True) | oo_image_tag_to_rpm_version }}"
+  when: openshift.common.is_containerized | bool
+
+- set_fact:
+    l_common_version: "{{ common_version.stdout | default('0.0', True) }}"
+  when: not openshift.common.is_containerized | bool
 
 - name: Set docker version to be installed
   set_fact:
     docker_version: "{{ '1.8.2' }}"
-  when: " ( common_version.stdout | default('0.0', True) | version_compare('3.2','<') and openshift.common.service_type == 'atomic-openshift' ) or
-          ( common_version.stdout | default('0.0', True) | version_compare('1.1.4','<') and openshift.common.service_type == 'origin' )"
+  when: " ( l_common_version | version_compare('3.2','<') and openshift.common.service_type == 'atomic-openshift' ) or
+          ( l_common_version | version_compare('1.1.4','<') and openshift.common.service_type == 'origin' )"
+
+- name: Set docker version to be installed
+  set_fact:
+    docker_version: "{{ '1.9.1' }}"
+  when: " ( l_common_version | version_compare('3.2','>') and openshift.common.service_type == 'atomic-openshift' ) or
+          ( l_common_version | version_compare('1.2','>') and openshift.common.service_type == 'origin' )"
diff --git a/roles/openshift_facts/library/openshift_facts.py b/roles/openshift_facts/library/openshift_facts.py
index 92d650550..2b68f27b7 100755
--- a/roles/openshift_facts/library/openshift_facts.py
+++ b/roles/openshift_facts/library/openshift_facts.py
@@ -1080,13 +1080,9 @@ def get_openshift_version(facts, cli_image=None):
         elif 'node' in facts:
             container = facts['common']['service_type'] + '-node'
 
-        if container is not None:
-            exit_code, output, _ = module.run_command(['docker', 'exec', container, 'openshift', 'version'])
-            # if for some reason the container is installed but not running
-            # we'll fall back to using docker run later in this method.
-            if exit_code == 0:
-                version = parse_openshift_version(output)
-
+	# Try to get the version fromthe available cli image _before_ resorting
+	# to exec'ing in to the running container.  This is to be more fault
+	# tolerant in environments where the container is not running.
         if version is None and cli_image is not None:
             # Assume we haven't installed the environment yet and we need
             # to query the latest image, but only if docker is installed
@@ -1094,6 +1090,13 @@ def get_openshift_version(facts, cli_image=None):
                 exit_code, output, _ = module.run_command(['docker', 'run', '--rm', cli_image, 'version'])
                 version = parse_openshift_version(output)
 
+        if version is None and container is not None:
+            exit_code, output, _ = module.run_command(['docker', 'exec', container, 'openshift', 'version'])
+            # if for some reason the container is installed but not running
+            # we'll fall back to using docker run later in this method.
+            if exit_code == 0:
+                version = parse_openshift_version(output)
+
     return version
 
 def parse_openshift_version(output):
@@ -1351,7 +1354,7 @@ def set_container_facts_if_unset(facts):
         facts['common']['admin_binary'] = '/usr/local/bin/oadm'
         facts['common']['client_binary'] = '/usr/local/bin/oc'
         openshift_version = get_openshift_version(facts, cli_image)
-        if openshift_version is not None:
+        if openshift_version is not None and openshift_version is not "":
             base_version = openshift_version.split('-')[0]
             facts['common']['image_tag'] = "v" + base_version
 
diff --git a/roles/openshift_master/handlers/main.yml b/roles/openshift_master/handlers/main.yml
index 70c7ef4e4..f7dfb11f7 100644
--- a/roles/openshift_master/handlers/main.yml
+++ b/roles/openshift_master/handlers/main.yml
@@ -1,17 +1,17 @@
 ---
 - name: restart master
   service: name={{ openshift.common.service_type }}-master state=restarted
-  when: (openshift.master.ha is defined and not openshift.master.ha | bool) and (not (master_service_status_changed | default(false) | bool))
+  when: (openshift.master.ha is not defined or not openshift.master.ha | bool) and (not (master_service_status_changed | default(false) | bool))
   notify: Verify API Server
 
 - name: restart master api
   service: name={{ openshift.common.service_type }}-master-api state=restarted
-  when: (openshift.master.ha is defined and openshift_master_ha | bool) and (not (master_api_service_status_changed | default(false) | bool)) and openshift.master.cluster_method == 'native'
+  when: (openshift.master.ha is defined and openshift.master.ha | bool) and (not (master_api_service_status_changed | default(false) | bool)) and openshift.master.cluster_method == 'native'
   notify: Verify API Server
 
 - name: restart master controllers
   service: name={{ openshift.common.service_type }}-master-controllers state=restarted
-  when: (openshift.master.ha is defined and openshift_master_ha | bool) and (not (master_controllers_service_status_changed | default(false) | bool)) and openshift.master.cluster_method == 'native'
+  when: (openshift.master.ha is defined and openshift.master.ha | bool) and (not (master_controllers_service_status_changed | default(false) | bool)) and openshift.master.cluster_method == 'native'
 
 - name: Verify API Server
   # Using curl here since the uri module requires python-httplib2 and
diff --git a/roles/openshift_master/templates/docker-cluster/atomic-openshift-master-api.service.j2 b/roles/openshift_master/templates/docker-cluster/atomic-openshift-master-api.service.j2
index 66d76978e..df1dbb85e 100644
--- a/roles/openshift_master/templates/docker-cluster/atomic-openshift-master-api.service.j2
+++ b/roles/openshift_master/templates/docker-cluster/atomic-openshift-master-api.service.j2
@@ -20,6 +20,7 @@ LimitCORE=infinity
 WorkingDirectory={{ openshift.common.data_dir }}
 SyslogIdentifier={{ openshift.common.service_type }}-master-api
 Restart=always
+RestartSec=5s
 
 [Install]
 WantedBy=docker.service
diff --git a/roles/openshift_master/templates/docker-cluster/atomic-openshift-master-controllers.service.j2 b/roles/openshift_master/templates/docker-cluster/atomic-openshift-master-controllers.service.j2
index 75759c133..5ff2edae4 100644
--- a/roles/openshift_master/templates/docker-cluster/atomic-openshift-master-controllers.service.j2
+++ b/roles/openshift_master/templates/docker-cluster/atomic-openshift-master-controllers.service.j2
@@ -19,6 +19,7 @@ LimitCORE=infinity
 WorkingDirectory={{ openshift.common.data_dir }}
 SyslogIdentifier={{ openshift.common.service_type }}-master-controllers
 Restart=on-failure
+RestartSec=5s
 
 [Install]
 WantedBy=docker.service
diff --git a/roles/openshift_master/templates/docker/master.docker.service.j2 b/roles/openshift_master/templates/docker/master.docker.service.j2
index d02fc5342..97f698b68 100644
--- a/roles/openshift_master/templates/docker/master.docker.service.j2
+++ b/roles/openshift_master/templates/docker/master.docker.service.j2
@@ -12,6 +12,7 @@ ExecStart=/usr/bin/docker run --rm --privileged --net=host --name {{ openshift.c
 ExecStartPost=/usr/bin/sleep 10
 ExecStop=/usr/bin/docker stop {{ openshift.common.service_type }}-master
 Restart=always
+RestartSec=5s
 
 [Install]
 WantedBy=docker.service
diff --git a/roles/openshift_master/templates/native-cluster/atomic-openshift-master-api.service.j2 b/roles/openshift_master/templates/native-cluster/atomic-openshift-master-api.service.j2
index e61418eb6..1f50fdce1 100644
--- a/roles/openshift_master/templates/native-cluster/atomic-openshift-master-api.service.j2
+++ b/roles/openshift_master/templates/native-cluster/atomic-openshift-master-api.service.j2
@@ -15,6 +15,7 @@ LimitNOFILE=131072
 LimitCORE=infinity
 WorkingDirectory={{ openshift.common.data_dir }}
 SyslogIdentifier=atomic-openshift-master-api
+RestartSec=5s
 
 [Install]
 WantedBy=multi-user.target
diff --git a/roles/openshift_master/templates/native-cluster/atomic-openshift-master-controllers.service.j2 b/roles/openshift_master/templates/native-cluster/atomic-openshift-master-controllers.service.j2
index ba4c1e0e8..bf62696f0 100644
--- a/roles/openshift_master/templates/native-cluster/atomic-openshift-master-controllers.service.j2
+++ b/roles/openshift_master/templates/native-cluster/atomic-openshift-master-controllers.service.j2
@@ -20,6 +20,7 @@ LimitCORE=infinity
 WorkingDirectory={{ openshift.common.data_dir }}
 SyslogIdentifier={{ openshift.common.service_type }}-master-controllers
 Restart=on-failure
+RestartSec=5s
 
 [Install]
 WantedBy=multi-user.target
diff --git a/roles/openshift_node/templates/openshift.docker.node.service b/roles/openshift_node/templates/openshift.docker.node.service
index ff5a97fe0..30f09b250 100644
--- a/roles/openshift_node/templates/openshift.docker.node.service
+++ b/roles/openshift_node/templates/openshift.docker.node.service
@@ -17,6 +17,7 @@ ExecStartPost=/usr/bin/sleep 10
 ExecStop=/usr/bin/docker stop {{ openshift.common.service_type }}-node
 SyslogIdentifier={{ openshift.common.service_type }}-node
 Restart=always
+RestartSec=5s
 
 [Install]
 WantedBy=docker.service
diff --git a/roles/openshift_node/templates/openvswitch.docker.service b/roles/openshift_node/templates/openvswitch.docker.service
index 8052a3a39..1e1f8967d 100644
--- a/roles/openshift_node/templates/openvswitch.docker.service
+++ b/roles/openshift_node/templates/openvswitch.docker.service
@@ -11,6 +11,7 @@ ExecStartPost=/usr/bin/sleep 5
 ExecStop=/usr/bin/docker stop openvswitch
 SyslogIdentifier=openvswitch
 Restart=always
+RestartSec=5s
 
 [Install]
 WantedBy=docker.service