Implement container_runtime playbooks and changes

This commit refactors some duplicate code, removes
usage of set_fact where not needed, and reorganizes
container_runtime role to use include_role.

11 files changed, 188 insertions, 257 deletions

diff --git a/roles/container_runtime/tasks/common/atomic_proxy.yml b/roles/container_runtime/tasks/common/atomic_proxy.yml
new file mode 100644
index 000000000..dde099984
--- /dev/null
+++ b/roles/container_runtime/tasks/common/atomic_proxy.yml
@@ -0,0 +1,32 @@
+---
+# Set http_proxy, https_proxy, and no_proxy in /etc/atomic.conf
+# regexp: the line starts with or without #, followed by the string
+#         http_proxy, then either : or =
+- block:
+
+    - name: Add http_proxy to /etc/atomic.conf
+      lineinfile:
+        dest: /etc/atomic.conf
+        regexp: "^#?http_proxy[:=]{1}"
+        line: "http_proxy: {{ openshift.common.http_proxy | default('') }}"
+      when:
+        - openshift.common.http_proxy is defined
+        - openshift.common.http_proxy != ''
+
+    - name: Add https_proxy to /etc/atomic.conf
+      lineinfile:
+        dest: /etc/atomic.conf
+        regexp: "^#?https_proxy[:=]{1}"
+        line: "https_proxy: {{ openshift.common.https_proxy | default('') }}"
+      when:
+        - openshift.common.https_proxy is defined
+        - openshift.common.https_proxy != ''
+
+    - name: Add no_proxy to /etc/atomic.conf
+      lineinfile:
+        dest: /etc/atomic.conf
+        regexp: "^#?no_proxy[:=]{1}"
+        line: "no_proxy: {{ openshift.common.no_proxy | default('') }}"
+      when:
+        - openshift.common.no_proxy is defined
+        - openshift.common.no_proxy != ''
diff --git a/roles/container_runtime/tasks/common/post.yml b/roles/container_runtime/tasks/common/post.yml
new file mode 100644
index 000000000..d790eb2c0
--- /dev/null
+++ b/roles/container_runtime/tasks/common/post.yml
@@ -0,0 +1,26 @@
+---
+- name: Ensure /var/lib/containers exists
+  file:
+    path: /var/lib/containers
+    state: directory
+
+- name: Fix SELinux Permissions on /var/lib/containers
+  command: "restorecon -R /var/lib/containers/"
+  changed_when: false
+
+- meta: flush_handlers
+
+# This needs to run after docker is restarted to account for proxy settings.
+# registry_auth is called directly with include_role in some places, so we
+# have to put it in the root of the tasks/ directory.
+- include_tasks: ../registry_auth.yml
+
+- name: stat the docker data dir
+  stat:
+    path: "{{ docker_default_storage_path }}"
+  register: dockerstat
+
+- include_tasks: setup_docker_symlink.yml
+  when:
+    - openshift_use_crio
+    - dockerstat.stat.islnk is defined and not (dockerstat.stat.islnk | bool)
diff --git a/roles/container_runtime/tasks/common/pre.yml b/roles/container_runtime/tasks/common/pre.yml
new file mode 100644
index 000000000..990fe66da
--- /dev/null
+++ b/roles/container_runtime/tasks/common/pre.yml
@@ -0,0 +1,12 @@
+---
+- include_tasks: udev_workaround.yml
+  when: docker_udev_workaround | default(False) | bool
+
+- name: Add enterprise registry, if necessary
+  set_fact:
+    l2_docker_additional_registries: "{{ l2_docker_additional_registries + [openshift_docker_ent_reg] }}"
+  when:
+    - openshift.common.deployment_type == 'openshift-enterprise'
+    - openshift_docker_ent_reg != ''
+    - openshift_docker_ent_reg not in l2_docker_additional_registries
+    - not openshift_use_crio_only | bool
diff --git a/roles/container_runtime/tasks/common/setup_docker_symlink.yml b/roles/container_runtime/tasks/common/setup_docker_symlink.yml
new file mode 100644
index 000000000..d7aeb192e
--- /dev/null
+++ b/roles/container_runtime/tasks/common/setup_docker_symlink.yml
@@ -0,0 +1,38 @@
+---
+- block:
+    - name: stop the current running docker
+      systemd:
+        state: stopped
+        name: "{{ openshift_docker_service_name }}"
+
+    - name: copy "{{ docker_default_storage_path }}" to "{{ docker_alt_storage_path }}"
+      command: "cp -r {{ docker_default_storage_path }} {{ docker_alt_storage_path }}"
+      register: results
+      failed_when:
+        - results.rc != 0
+
+    - name: "Set the selinux context on {{ docker_alt_storage_path }}"
+      command: "semanage fcontext -a -e {{ docker_default_storage_path }} {{ docker_alt_storage_path }}"
+      register: results
+      failed_when:
+        - results.rc == 1
+        - "'already exists' not in results.stderr"
+
+    - name: "restorecon the {{ docker_alt_storage_path }}"
+      command: "restorecon -r {{ docker_alt_storage_path }}"
+
+    - name: Remove the old docker location
+      file:
+        state: absent
+        path: "{{ docker_default_storage_path }}"
+
+    - name: Setup the link
+      file:
+        state: link
+        src: "{{ docker_alt_storage_path }}"
+        path: "{{ docker_default_storage_path }}"
+
+    - name: start docker
+      systemd:
+        state: started
+        name: "{{ openshift_docker_service_name }}"
diff --git a/roles/container_runtime/tasks/common/syscontainer_packages.yml b/roles/container_runtime/tasks/common/syscontainer_packages.yml
new file mode 100644
index 000000000..715ed492d
--- /dev/null
+++ b/roles/container_runtime/tasks/common/syscontainer_packages.yml
@@ -0,0 +1,28 @@
+---
+
+- name: Ensure container-selinux is installed
+  package:
+    name: container-selinux
+    state: present
+  when: not openshift.common.is_atomic | bool
+  register: result
+  until: result | success
+
+# Used to pull and install the system container
+- name: Ensure atomic is installed
+  package:
+    name: atomic
+    state: present
+  when: not openshift.common.is_atomic | bool
+  register: result
+  until: result | success
+
+# At the time of writing the atomic command requires runc for it's own use. This
+# task is here in the even that the atomic package ever removes the dependency.
+- name: Ensure runc is installed
+  package:
+    name: runc
+    state: present
+  when: not openshift.common.is_atomic | bool
+  register: result
+  until: result | success
diff --git a/roles/container_runtime/tasks/udev_workaround.yml b/roles/container_runtime/tasks/common/udev_workaround.yml
index 257c3123d..257c3123d 100644
--- a/roles/container_runtime/tasks/udev_workaround.yml
+++ b/roles/container_runtime/tasks/common/udev_workaround.yml
diff --git a/roles/container_runtime/tasks/docker_sanity.yml b/roles/container_runtime/tasks/docker_sanity.yml
new file mode 100644
index 000000000..e62cf5505
--- /dev/null
+++ b/roles/container_runtime/tasks/docker_sanity.yml
@@ -0,0 +1,27 @@
+---
+# Sanity checks to ensure the role will complete and provide helpful error
+# messages for common problems.
+
+- name: Error out if Docker pre-installed but too old
+  fail:
+    msg: "Docker {{ curr_docker_version.stdout }} is installed, but >= 1.9.1 is required."
+  when: not curr_docker_version | skipped and curr_docker_version.stdout != '' and curr_docker_version.stdout | version_compare('1.9.1', '<') and not docker_version is defined
+
+- name: Error out if requested Docker is too old
+  fail:
+    msg: "Docker {{ docker_version }} requested, but >= 1.9.1 is required."
+  when: docker_version is defined and docker_version | version_compare('1.9.1', '<')
+
+# If a docker_version was requested, sanity check that we can install or upgrade to it, and
+# no downgrade is required.
+- name: Fail if Docker version requested but downgrade is required
+  fail:
+    msg: "Docker {{ curr_docker_version.stdout }} is installed, but version {{ docker_version }} was requested."
+  when: not curr_docker_version | skipped and curr_docker_version.stdout != '' and docker_version is defined and curr_docker_version.stdout | version_compare(docker_version, '>')
+
+# This involves an extremely slow migration process, users should instead run the
+# Docker 1.10 upgrade playbook to accomplish this.
+- name: Error out if attempting to upgrade Docker across the 1.10 boundary
+  fail:
+    msg: "Cannot upgrade Docker to >= 1.10, please upgrade or remove Docker manually, or use the Docker upgrade playbook if OpenShift is already installed."
+  when: not curr_docker_version | skipped and curr_docker_version.stdout != '' and curr_docker_version.stdout | version_compare('1.10', '<') and docker_version is defined and docker_version | version_compare('1.10', '>=')
diff --git a/roles/container_runtime/tasks/main.yml b/roles/container_runtime/tasks/main.yml
index 6d68082b1..96d8606c6 100644
--- a/roles/container_runtime/tasks/main.yml
+++ b/roles/container_runtime/tasks/main.yml
@@ -1,85 +1,2 @@
 ---
-- include_tasks: udev_workaround.yml
-  when: docker_udev_workaround | default(False) | bool
-
-- name: Add enterprise registry, if necessary
-  set_fact:
-    l2_docker_additional_registries: "{{ l2_docker_additional_registries + [openshift_docker_ent_reg] }}"
-  when:
-    - openshift.common.deployment_type == 'openshift-enterprise'
-    - openshift_docker_ent_reg != ''
-    - openshift_docker_ent_reg not in l2_docker_additional_registries
-    - not openshift_use_crio_only | bool
-
-- name: Use Package Docker if Requested
-  include_tasks: package_docker.yml
-  when:
-    - not openshift_docker_use_system_container
-    - not openshift_use_crio_only
-
-- name: Ensure /var/lib/containers exists
-  file:
-    path: /var/lib/containers
-    state: directory
-
-- name: Fix SELinux Permissions on /var/lib/containers
-  command: "restorecon -R /var/lib/containers/"
-  changed_when: false
-
-- name: Use System Container Docker if Requested
-  include_tasks: systemcontainer_docker.yml
-  when:
-    - openshift_docker_use_system_container
-    - not openshift_use_crio_only
-
-- name: Add CRI-O usage Requested
-  include_tasks: systemcontainer_crio.yml
-  when:
-    - openshift_use_crio
-    - openshift_docker_is_node_or_master | bool
-
-- name: stat the docker data dir
-  stat:
-    path: "{{ docker_default_storage_path }}"
-  register: dockerstat
-
-- when:
-    - openshift_use_crio
-    - dockerstat.stat.islnk is defined and not (dockerstat.stat.islnk | bool)
-  block:
-    - name: stop the current running docker
-      systemd:
-        state: stopped
-        name: "{{ openshift_docker_service_name }}"
-
-    - name: copy "{{ docker_default_storage_path }}" to "{{ docker_alt_storage_path }}"
-      command: "cp -r {{ docker_default_storage_path }} {{ docker_alt_storage_path }}"
-      register: results
-      failed_when:
-        - results.rc != 0
-
-    - name: "Set the selinux context on {{ docker_alt_storage_path }}"
-      command: "semanage fcontext -a -e {{ docker_default_storage_path }} {{ docker_alt_storage_path }}"
-      register: results
-      failed_when:
-        - results.rc == 1
-        - "'already exists' not in results.stderr"
-
-    - name: "restorecon the {{ docker_alt_storage_path }}"
-      command: "restorecon -r {{ docker_alt_storage_path }}"
-
-    - name: Remove the old docker location
-      file:
-        state: absent
-        path: "{{ docker_default_storage_path }}"
-
-    - name: Setup the link
-      file:
-        state: link
-        src: "{{ docker_alt_storage_path }}"
-        path: "{{ docker_default_storage_path }}"
-
-    - name: start docker
-      systemd:
-        state: started
-        name: "{{ openshift_docker_service_name }}"
+# This role is meant to be used with include_role and tasks_from.
diff --git a/roles/container_runtime/tasks/package_docker.yml b/roles/container_runtime/tasks/package_docker.yml
index 40ab75a25..89899c9cf 100644
--- a/roles/container_runtime/tasks/package_docker.yml
+++ b/roles/container_runtime/tasks/package_docker.yml
@@ -1,4 +1,6 @@
 ---
+- include_tasks: common/pre.yml
+
 - name: Get current installed Docker version
   command: "{{ repoquery_installed }} --qf '%{version}' docker"
   when: not openshift.common.is_atomic | bool
@@ -7,35 +9,16 @@
   until: curr_docker_version | succeeded
   changed_when: false
 
-- name: Error out if Docker pre-installed but too old
-  fail:
-    msg: "Docker {{ curr_docker_version.stdout }} is installed, but >= 1.9.1 is required."
-  when: not curr_docker_version | skipped and curr_docker_version.stdout != '' and curr_docker_version.stdout | version_compare('1.9.1', '<') and not docker_version is defined
-
-- name: Error out if requested Docker is too old
-  fail:
-    msg: "Docker {{ docker_version }} requested, but >= 1.9.1 is required."
-  when: docker_version is defined and docker_version | version_compare('1.9.1', '<')
-
-# If a docker_version was requested, sanity check that we can install or upgrade to it, and
-# no downgrade is required.
-- name: Fail if Docker version requested but downgrade is required
-  fail:
-    msg: "Docker {{ curr_docker_version.stdout }} is installed, but version {{ docker_version }} was requested."
-  when: not curr_docker_version | skipped and curr_docker_version.stdout != '' and docker_version is defined and curr_docker_version.stdout | version_compare(docker_version, '>')
-
-# This involves an extremely slow migration process, users should instead run the
-# Docker 1.10 upgrade playbook to accomplish this.
-- name: Error out if attempting to upgrade Docker across the 1.10 boundary
-  fail:
-    msg: "Cannot upgrade Docker to >= 1.10, please upgrade or remove Docker manually, or use the Docker upgrade playbook if OpenShift is already installed."
-  when: not curr_docker_version | skipped and curr_docker_version.stdout != '' and curr_docker_version.stdout | version_compare('1.10', '<') and docker_version is defined and docker_version | version_compare('1.10', '>=')
+# Some basic checks to ensure the role will complete
+- include_tasks: docker_sanity.yml
 
 # Make sure Docker is installed, but does not update a running version.
 # Docker upgrades are handled by a separate playbook.
 # Note: The curr_docker_version.stdout check can be removed when https://github.com/ansible/ansible/issues/33187 gets fixed.
 - name: Install Docker
-  package: name=docker{{ '-' + docker_version if docker_version is defined else '' }} state=present
+  package:
+    name: "docker{{ '-' + docker_version if docker_version is defined else '' }}"
+    state: present
   when: not openshift.common.is_atomic | bool and not curr_docker_version | skipped and not curr_docker_version.stdout != ''
   register: result
   until: result | success
@@ -161,7 +144,4 @@
 - set_fact:
     docker_service_status_changed: "{{ (r_docker_package_docker_start_result | changed) and (r_docker_already_running_result.stdout != 'ActiveState=active' ) }}"
 
-- meta: flush_handlers
-
-# This needs to run after docker is restarted to account for proxy settings.
-- include_tasks: registry_auth.yml
+- include_tasks: common/post.yml
diff --git a/roles/container_runtime/tasks/systemcontainer_crio.yml b/roles/container_runtime/tasks/systemcontainer_crio.yml
index 8dcfe60ef..d583547b2 100644
--- a/roles/container_runtime/tasks/systemcontainer_crio.yml
+++ b/roles/container_runtime/tasks/systemcontainer_crio.yml
@@ -1,39 +1,14 @@
 ---
 # TODO: Much of this file is shared with container engine tasks
-
-- name: Ensure container-selinux is installed
-  package:
-    name: container-selinux
-    state: present
-  when: not openshift.common.is_atomic | bool
-  register: result
-  until: result | success
-
 - name: Check we are not using node as a Docker container with CRI-O
   fail: msg='Cannot use CRI-O with node configured as a Docker container'
   when:
     - openshift.common.is_containerized | bool
     - not openshift.common.is_node_system_container | bool
 
-# Used to pull and install the system container
-- name: Ensure atomic is installed
-  package:
-    name: atomic
-    state: present
-  when: not openshift.common.is_atomic | bool
-  register: result
-  until: result | success
-
-# At the time of writing the atomic command requires runc for it's own use. This
-# task is here in the even that the atomic package ever removes the dependency.
-- name: Ensure runc is installed
-  package:
-    name: runc
-    state: present
-  when: not openshift.common.is_atomic | bool
-  register: result
-  until: result | success
+- include_tasks: common/pre.yml
 
+- include_tasks: common/syscontainer_packages.yml
 
 - name: Check that overlay is in the kernel
   shell: lsmod | grep overlay
@@ -60,50 +35,11 @@
         state: restarted
 
 - name: Ensure proxies are in the atomic.conf
-  include_role:
-    name: openshift_atomic
-    tasks_from: proxy
-
-- block:
-
-    - name: Set CRI-O image defaults
-      set_fact:
-        l_crio_image_prepend: "docker.io/gscrivano"
-        l_crio_image_name: "cri-o-fedora"
-        l_crio_image_tag: "latest"
-
-    - name: Use Centos based image when distribution is CentOS
-      set_fact:
-        l_crio_image_name: "cri-o-centos"
-      when: ansible_distribution == "CentOS"
-
-    - name: Set CRI-O image tag
-      set_fact:
-        l_crio_image_tag: "{{ l_openshift_image_tag }}"
-      when:
-        - openshift_deployment_type == 'openshift-enterprise'
-
-    - name: Use RHEL based image when distribution is Red Hat
-      set_fact:
-        l_crio_image_prepend: "registry.access.redhat.com/openshift3"
-        l_crio_image_name: "cri-o"
-      when: ansible_distribution == "RedHat"
-
-    - name: Set the full image name
-      set_fact:
-        l_crio_image: "{{ l_crio_image_prepend }}/{{ l_crio_image_name }}:{{ l_crio_image_tag }}"
-
-    # For https://github.com/openshift/aos-cd-jobs/pull/624#pullrequestreview-61816548
-    - name: Use a specific image if requested
-      set_fact:
-        l_crio_image: "{{ openshift_crio_systemcontainer_image_override }}"
-      when:
-        - openshift_crio_systemcontainer_image_override is defined
-        - openshift_crio_systemcontainer_image_override != ""
-
-    # Be nice and let the user see the variable result
-    - debug:
-        var: l_crio_image
+  include_tasks: common/atomic_proxy.yml
+
+# Be nice and let the user see the variable result
+- debug:
+    var: l_crio_image
 
 # NOTE: no_proxy added as a workaround until https://github.com/projectatomic/atomic/pull/999 is released
 - name: Pre-pull CRI-O System Container image
@@ -112,7 +48,6 @@
   environment:
     NO_PROXY: "{{ openshift.common.no_proxy | default('') }}"
 
-
 - name: Install CRI-O System Container
   oc_atomic_container:
     name: "cri-o"
@@ -155,10 +90,8 @@
     daemon_reload: yes
   register: start_result
 
-- meta: flush_handlers
-
 # If we are using crio only, docker.service might not be available for
 # 'docker login'
-- include_tasks: registry_auth.yml
+- include_tasks: common/post.yml
   vars:
     openshift_docker_alternative_creds: "{{ openshift_use_crio_only }}"
diff --git a/roles/container_runtime/tasks/systemcontainer_docker.yml b/roles/container_runtime/tasks/systemcontainer_docker.yml
index 84217e50c..10570fe34 100644
--- a/roles/container_runtime/tasks/systemcontainer_docker.yml
+++ b/roles/container_runtime/tasks/systemcontainer_docker.yml
@@ -11,32 +11,9 @@
       traditional docker package install. Otherwise, comment out openshift_docker_options
       in your inventory file.
 
-- name: Ensure container-selinux is installed
-  package:
-    name: container-selinux
-    state: present
-  when: not openshift.common.is_atomic | bool
-  register: result
-  until: result | success
-
-# Used to pull and install the system container
-- name: Ensure atomic is installed
-  package:
-    name: atomic
-    state: present
-  when: not openshift.common.is_atomic | bool
-  register: result
-  until: result | success
+- include_tasks: common/pre.yml
 
-# At the time of writing the atomic command requires runc for it's own use. This
-# task is here in the even that the atomic package ever removes the dependency.
-- name: Ensure runc is installed
-  package:
-    name: runc
-    state: present
-  when: not openshift.common.is_atomic | bool
-  register: result
-  until: result | success
+- include_tasks: common/syscontainer_packages.yml
 
 # Make sure Docker is installed so we are able to use the client
 - name: Install Docker so we can use the client
@@ -59,48 +36,11 @@
   delay: 30
 
 - name: Ensure proxies are in the atomic.conf
-  include_role:
-    name: openshift_atomic
-    tasks_from: proxy
-
-- block:
-
-    - name: Set to default prepend
-      set_fact:
-        l_docker_image_prepend: "gscrivano"
-        l_docker_image_tag: "latest"
-
-    - name: Set container engine image tag
-      set_fact:
-        l_docker_image_tag: "{{ l_openshift_image_tag }}"
-      when:
-        - openshift_deployment_type == 'openshift-enterprise'
-
-    - name: Use Red Hat Registry for image when distribution is Red Hat
-      set_fact:
-        l_docker_image_prepend: "registry.access.redhat.com/openshift3"
-      when: ansible_distribution == 'RedHat'
-
-    - name: Use Fedora Registry for image when distribution is Fedora
-      set_fact:
-        l_docker_image_prepend: "registry.fedoraproject.org/f25"
-      when: ansible_distribution == 'Fedora'
-
-    - name: Set the full image name
-      set_fact:
-        l_docker_image: "{{ l_docker_image_prepend }}/{{ openshift_docker_service_name }}:{{ l_docker_image_tag }}"
-
-    # For https://github.com/openshift/openshift-ansible/pull/5354#issuecomment-328552959
-    - name: Use a specific image if requested
-      set_fact:
-        l_docker_image: "{{ openshift_docker_systemcontainer_image_override }}"
-      when:
-        - openshift_docker_systemcontainer_image_override is defined
-        - openshift_docker_systemcontainer_image_override != ""
-
-    # Be nice and let the user see the variable result
-    - debug:
-        var: l_docker_image
+  include_tasks: common/atomic_proxy.yml
+
+# Be nice and let the user see the variable result
+- debug:
+    var: l_docker_image
 
 # NOTE: no_proxy added as a workaround until https://github.com/projectatomic/atomic/pull/999 is released
 - name: Pre-pull Container Engine System Container image
@@ -154,10 +94,8 @@
 - set_fact:
     docker_service_status_changed: "{{ r_docker_systemcontainer_docker_start_result | changed }}"
 
-- meta: flush_handlers
-
 # Since docker is running as a system container, docker login will fail to create
 # credentials.  Use alternate method if requiring authenticated registries.
-- include_tasks: registry_auth.yml
+- include_tasks: common/post.yml
   vars:
     openshift_docker_alternative_creds: True