Simplify double upgrade version logic

Currently, double upgrade process (3.7 -> 3.9)
for control plane attempts to run openshift_version
role twice to set the appropriate values for
upgrading each major version, 3.8 and 3.9.

This commit instructs openshift_version to
only inquire about the proper settings for 3.9,
and hard-sets the appropriate values for 3.8.

This allows a simplification of the
openshift_version role, allowing for easier
debugging.

Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1542368

8 files changed, 100 insertions, 45 deletions

diff --git a/playbooks/common/openshift-cluster/upgrades/pre/config.yml b/playbooks/common/openshift-cluster/upgrades/pre/config.yml
index 44af37b2d..4dcd4f61b 100644
--- a/playbooks/common/openshift-cluster/upgrades/pre/config.yml
+++ b/playbooks/common/openshift-cluster/upgrades/pre/config.yml
@@ -51,6 +51,10 @@
     # l_openshift_version_set_hosts is passed via upgrade_control_plane.yml
     # l_openshift_version_check_hosts is passed via upgrade_control_plane.yml
 
+# version_override will set various version-related variables during a double upgrade.
+- import_playbook: version_override.yml
+  when: l_double_upgrade_cp | default(False)
+
 - import_playbook: verify_cluster.yml
 
 # If we're only upgrading nodes, we need to ensure masters are already upgraded
diff --git a/playbooks/common/openshift-cluster/upgrades/pre/version_override.yml b/playbooks/common/openshift-cluster/upgrades/pre/version_override.yml
new file mode 100644
index 000000000..b2954397f
--- /dev/null
+++ b/playbooks/common/openshift-cluster/upgrades/pre/version_override.yml
@@ -0,0 +1,29 @@
+---
+# This playbook overrides normal version setting during double upgrades.
+
+- name: Set proper version values for upgrade
+  hosts: "{{ l_version_override_hosts | default('all:!all') }}"
+  tasks:
+    - set_fact:
+        # All of these will either have been set by openshift_version or
+        # provided by the user; we need to save these for later.
+        l_double_upgrade_saved_version: "{{ openshift_version }}"
+        l_double_upgrade_saved_release: "{{ openshift_release | default(openshift_upgrade_target) }}"
+        l_double_upgrade_saved_tag: "{{ openshift_image_tag }}"
+        l_double_upgrade_saved_pkgv: "{{ openshift_pkg_version }}"
+    - set_fact:
+        # We already ran openshift_version for the second of two upgrades;
+        # here we need to set some variables to enable the first upgrade.
+        # openshift_version, openshift_image_tag, and openshift_pkg_version
+        # will be modified by openshift_version; we want to ensure these
+        # are initially set to first versions to ensure no accidental usage of
+        # second versions (eg, 3.8 and 3.9 respectively) are used.
+        l_double_upgrade_cp_reset_version: True
+        openshift_version: "{{ l_double_upgrade_first_version }}"
+        openshift_release: "{{ l_double_upgrade_first_release }}"
+        openshift_upgrade_target: '3.8'
+        openshift_upgrade_min: '3.7'
+
+# Now that we have force-set a different version, we need to update a few things
+# to ensure we have settings that actually match what's in repos/registries.
+- import_playbook: ../../../../init/version.yml
diff --git a/playbooks/common/openshift-cluster/upgrades/v3_9/upgrade_control_plane.yml b/playbooks/common/openshift-cluster/upgrades/v3_9/upgrade_control_plane.yml
index 9c7677f1b..c21862dea 100644
--- a/playbooks/common/openshift-cluster/upgrades/v3_9/upgrade_control_plane.yml
+++ b/playbooks/common/openshift-cluster/upgrades/v3_9/upgrade_control_plane.yml
@@ -17,32 +17,32 @@
     l_init_fact_hosts: "oo_masters_to_config:oo_etcd_to_config:oo_lb_to_config"
     l_base_packages_hosts: "oo_masters_to_config:oo_etcd_to_config:oo_lb_to_config"
 
-## Check to see if they're running 3.7 and if so upgrade them to 3.8 on control plan
-## If they've specified pkg_version or image_tag preserve that for later use
-- name: Configure the upgrade target for the common upgrade tasks 3.8
+- name: Configure the initial upgrade target for the common upgrade tasks
   hosts: oo_masters_to_config:oo_etcd_to_config:oo_lb_to_config
   tasks:
   - set_fact:
-      openshift_upgrade_target: '3.8'
+      # We use 3.9 here so when we run openshift_version we can get
+      # correct values for 3.9, 3.8 we will hard-code the values in
+      # ../pre/version_override.yml, if necessary.
+      openshift_upgrade_target: '3.9'
       openshift_upgrade_min: '3.7'
-      openshift_release: '3.8'
-      _requested_pkg_version: "{{ openshift_pkg_version if openshift_pkg_version is defined else omit }}"
-      openshift_pkg_version: ''
-      _requested_image_tag: "{{ openshift_image_tag if openshift_image_tag is defined else omit }}"
+
+## Check to see if we need to double upgrade (3.7 -> 3.8 -> 3.9)
+- name: Configure variables for double upgrade
+  hosts: oo_masters_to_config:oo_etcd_to_config
+  tasks:
+  - set_fact:
       l_double_upgrade_cp: True
+      l_version_override_hosts: "oo_masters_to_config:oo_etcd_to_config"
+      l_double_upgrade_first_version: "3.8"
+      l_double_upgrade_first_release: "3.8"
     when: hostvars[groups.oo_first_master.0].openshift_currently_installed_version | version_compare('3.8','<')
 
-  - name: set l_force_image_tag_to_version = True
-    set_fact:
-      # Need to set this during 3.8 upgrade to ensure image_tag is set correctly
-      # to match 3.8 version
-      l_force_image_tag_to_version: True
-    when: _requested_image_tag is defined
-
 - import_playbook: ../pre/config.yml
   # These vars a meant to exclude oo_nodes from plays that would otherwise include
   # them by default.
   vars:
+    l_version_override_hosts: "oo_masters_to_config:oo_etcd_to_config"
     l_openshift_version_set_hosts: "oo_etcd_to_config:oo_masters_to_config:!oo_first_master"
     l_openshift_version_check_hosts: "oo_masters_to_config:!oo_first_master"
     l_upgrade_repo_hosts: "oo_masters_to_config:oo_etcd_to_config:oo_lb_to_config"
@@ -52,46 +52,48 @@
     l_upgrade_docker_target_hosts: "oo_masters_to_config:oo_etcd_to_config"
     l_upgrade_excluder_hosts: "oo_masters_to_config"
     openshift_protect_installed_version: False
-  when: hostvars[groups.oo_first_master.0].openshift_currently_installed_version | version_compare('3.8','<')
+  when: l_double_upgrade_cp | default(False)
 
 - name: Flag pre-upgrade checks complete for hosts without errors 3.8
   hosts: oo_masters_to_config:oo_etcd_to_config
   tasks:
   - set_fact:
       pre_upgrade_complete: True
-    when: hostvars[groups.oo_first_master.0].openshift_currently_installed_version | version_compare('3.8','<')
+    when: l_double_upgrade_cp | default(False)
 
 # Pre-upgrade completed
 
 - name: Intermediate 3.8 Upgrade
   import_playbook: ../upgrade_control_plane.yml
-  when: hostvars[groups.oo_first_master.0].openshift_currently_installed_version | version_compare('3.8','<')
+  when: l_double_upgrade_cp | default(False)
+
+- name: Restore 3.9 version variables
+  hosts: oo_masters_to_config:oo_etcd_to_config
+  tasks:
+  - set_fact:
+      # all:!all == 0 hosts
+      l_version_override_hosts: "all:!all"
+      openshift_version: "{{ l_double_upgrade_saved_version }}"
+      openshift_release: "{{ l_double_upgrade_saved_release }}"
+      openshift_image_tag: "{{ l_double_upgrade_saved_tag }}"
+      openshift_pkg_version: "{{ l_double_upgrade_saved_pkgv }}"
+    when: l_double_upgrade_cp | default(False)
 
 ## 3.8 upgrade complete we should now be able to upgrade to 3.9
+- name: Clear some values now that we're done with double upgrades.
+  hosts: oo_masters_to_config:oo_etcd_to_config
+  tasks:
+  - set_fact:
+      l_double_upgrade_cp: False
+      l_double_upgrade_cp_reset_version: False
 
-- name: Configure the upgrade target for the common upgrade tasks 3.9
+# We should be on 3.8 at this point, need to set upgrade_target to 3.9
+- name: Configure the upgrade target for second upgrade
   hosts: oo_masters_to_config:oo_etcd_to_config:oo_lb_to_config
   tasks:
-  - meta: clear_facts
   - set_fact:
       openshift_upgrade_target: '3.9'
       openshift_upgrade_min: '3.8'
-      openshift_release: '3.9'
-      openshift_pkg_version: "{{ _requested_pkg_version if _requested_pkg_version is defined else '' }}"
-  # Set the user's specified image_tag for 3.9 upgrade if it was provided.
-  - set_fact:
-      openshift_image_tag: "{{ _requested_image_tag }}"
-      l_force_image_tag_to_version: False
-    when: _requested_image_tag is defined
-  # If the user didn't specify an image_tag, we need to force update image_tag
-  # because it will have already been set during 3.8.  If we aren't running
-  # a double upgrade, then we can preserve image_tag because it will still
-  # be the user provided value.
-  - set_fact:
-      l_force_image_tag_to_version: True
-    when:
-    - l_double_upgrade_cp is defined and l_double_upgrade_cp
-    - _requested_image_tag is not defined
 
 - import_playbook: ../pre/config.yml
   # These vars a meant to exclude oo_nodes from plays that would otherwise include
@@ -106,7 +108,6 @@
     l_upgrade_docker_target_hosts: "oo_masters_to_config:oo_etcd_to_config"
     l_upgrade_excluder_hosts: "oo_masters_to_config"
     openshift_protect_installed_version: False
-    openshift_version_reinit: True
 
 - name: Flag pre-upgrade checks complete for hosts without errors
   hosts: oo_masters_to_config:oo_etcd_to_config
diff --git a/playbooks/openshift-master/private/validate_restart.yml b/playbooks/openshift-master/private/validate_restart.yml
index 60b0e5bb6..40aaa653c 100644
--- a/playbooks/openshift-master/private/validate_restart.yml
+++ b/playbooks/openshift-master/private/validate_restart.yml
@@ -33,6 +33,7 @@
   - stat: path="{{ hostvars.localhost.mktemp.stdout }}"
     register: exists
     changed_when: false
+    when: "'stdout' in hostvars.localhost.mktemp"
 
 - name: Cleanup temp file on localhost
   hosts: localhost
@@ -41,6 +42,7 @@
   tasks:
   - file: path="{{ hostvars.localhost.mktemp.stdout }}" state=absent
     changed_when: false
+    when: "'stdout' in hostvars.localhost.mktemp"
 
 - name: Warn if restarting the system where ansible is running
   hosts: oo_masters_to_config
@@ -54,7 +56,9 @@
         must be verified manually. To only restart services, set
         openshift_master_rolling_restart_mode=services in host
         inventory and relaunch the playbook.
-    when: exists.stat.exists and openshift.common.rolling_restart_mode == 'system'
+    when:
+    - "'stat' in exists"
+    - exists.stat.exists and openshift.common.rolling_restart_mode == 'system'
   - set_fact:
       current_host: "{{ exists.stat.exists }}"
     when: openshift.common.rolling_restart_mode == 'system'
diff --git a/roles/openshift_version/defaults/main.yml b/roles/openshift_version/defaults/main.yml
index e2e6538c9..c807df9d3 100644
--- a/roles/openshift_version/defaults/main.yml
+++ b/roles/openshift_version/defaults/main.yml
@@ -10,3 +10,6 @@ openshift_service_type: "{{ openshift_service_type_dict[openshift_deployment_typ
 openshift_use_crio_only: False
 
 l_first_master_version_task_file: "{{ openshift_is_containerized | ternary('first_master_containerized_version.yml', 'first_master_rpm_version.yml') }}"
+
+# Used during double control plane upgrades.
+l_double_upgrade_cp_reset_version: False
diff --git a/roles/openshift_version/tasks/first_master.yml b/roles/openshift_version/tasks/first_master.yml
index 374725086..022ac30fc 100644
--- a/roles/openshift_version/tasks/first_master.yml
+++ b/roles/openshift_version/tasks/first_master.yml
@@ -13,18 +13,30 @@
 
 - include_tasks: "{{ l_first_master_version_task_file }}"
 
+# When double upgrade is in process, we want to set everything to match
+# openshift_verison.
 - block:
   - debug:
       msg: "openshift_pkg_version was not defined. Falling back to -{{ openshift_version }}"
   - set_fact:
-      openshift_pkg_version: -{{ openshift_version }}
+      openshift_pkg_version: "-{{ openshift_version }}"
   when:
-  - openshift_pkg_version is not defined
-  - openshift_upgrade_target is not defined
+  - openshift_pkg_version is not defined or l_double_upgrade_cp_reset_version
 
+# When double upgrade is in process, we want to set everything to match
+# openshift_verison.
 - block:
   - debug:
       msg: "openshift_image_tag was not defined. Falling back to v{{ openshift_version }}"
   - set_fact:
-      openshift_image_tag: v{{ openshift_version }}
-  when: openshift_image_tag is not defined
+      openshift_image_tag: "v{{ openshift_version }}"
+  when: openshift_image_tag is not defined or l_double_upgrade_cp_reset_version
+
+# The end result of these three variables is quite important so make sure they are displayed and logged:
+- debug: var=openshift_release
+
+- debug: var=openshift_image_tag
+
+- debug: var=openshift_pkg_version
+
+- debug: var=openshift_version
diff --git a/roles/openshift_version/tasks/first_master_rpm_version.yml b/roles/openshift_version/tasks/first_master_rpm_version.yml
index 264baca65..9a5f0c568 100644
--- a/roles/openshift_version/tasks/first_master_rpm_version.yml
+++ b/roles/openshift_version/tasks/first_master_rpm_version.yml
@@ -11,6 +11,8 @@
 - name: Set openshift_version for rpm installation
   include_tasks: check_available_rpms.yml
 
+# If double upgrade is in process, we want to set openshift_version to whatever
+# rpm package is available.
 - set_fact:
     openshift_version: "{{ rpm_results.results.versions.available_versions.0 }}"
-  when: openshift_version is not defined
+  when: openshift_version is not defined or l_double_upgrade_cp_reset_version
diff --git a/roles/openshift_version/tasks/masters_and_nodes.yml b/roles/openshift_version/tasks/masters_and_nodes.yml
index fbeb22d8b..c4dbc2a5f 100644
--- a/roles/openshift_version/tasks/masters_and_nodes.yml
+++ b/roles/openshift_version/tasks/masters_and_nodes.yml
@@ -6,7 +6,7 @@
     include_tasks: check_available_rpms.yml
   - name: Fail if rpm version and docker image version are different
     fail:
-      msg: "OCP rpm version {{ openshift_rpm_version }} is different from OCP image version {{ openshift_version }}"
+      msg: "OCP rpm version {{ rpm_results.results.versions.available_versions.0 }} is different from OCP image version {{ openshift_version }}"
     # Both versions have the same string representation
     when: rpm_results.results.versions.available_versions.0 != openshift_version
   # block when