From a14800daa13ffcb5608e6619a88a7a5da0af69f9 Mon Sep 17 00:00:00 2001
From: Edward Hope-Morley <edward.hope-morley@canonical.com>
Date: Tue, 29 Sep 2020 10:23:35 +0100
Subject: [PATCH] Upgrade mon packages immediately

Instead of deferring the upgrading of packages
when doing a release upgrade, do them immediately
and only defer daemon restarts. This is to avoid
any api incompatibilities between newer mons and
older clients.

Change-Id: Idf77871561fa14ad61e0c7901eeb333254e37012
Closes-Bug: #1897594
---
 charms_ceph/utils.py                | 25 ++++++++++++++++++++-----
 unit_tests/test_mon_upgrade_roll.py |  5 +++++
 2 files changed, 25 insertions(+), 5 deletions(-)

diff --git a/charms_ceph/utils.py b/charms_ceph/utils.py
index 9da4dc1..52d380b 100644
--- a/charms_ceph/utils.py
+++ b/charms_ceph/utils.py
@@ -2141,6 +2141,8 @@ def roll_monitor_cluster(new_version, upgrade_key):
     # A sorted list of osd unit names
     mon_sorted_list = sorted(monitor_list)
 
+    # Install packages immediately but defer restarts to when it's our time.
+    upgrade_monitor(new_version, restart_daemons=False)
     try:
         position = mon_sorted_list.index(my_name)
         log("upgrade position: {}".format(position))
@@ -2182,7 +2184,7 @@ def noop():
     pass
 
 
-def upgrade_monitor(new_version, kick_function=None):
+def upgrade_monitor(new_version, kick_function=None, restart_daemons=True):
     """Upgrade the current ceph monitor to the new version
 
     :param new_version: String version to upgrade to.
@@ -2207,6 +2209,22 @@ def upgrade_monitor(new_version, kick_function=None):
         status_set("blocked", "Upgrade to {} failed".format(new_version))
         sys.exit(1)
     kick_function()
+
+    try:
+        apt_install(packages=determine_packages(), fatal=True)
+        rm_packages = determine_packages_to_remove()
+        if rm_packages:
+            apt_purge(packages=rm_packages, fatal=True)
+    except subprocess.CalledProcessError as err:
+        log("Upgrading packages failed "
+            "with message: {}".format(err))
+        status_set("blocked", "Upgrade to {} failed".format(new_version))
+        sys.exit(1)
+
+    if not restart_daemons:
+        log("Packages upgraded but not restarting daemons yet.")
+        return
+
     try:
         if systemd():
             service_stop('ceph-mon')
@@ -2216,10 +2234,7 @@ def upgrade_monitor(new_version, kick_function=None):
                 service_stop('ceph-mgr.target')
         else:
             service_stop('ceph-mon-all')
-        apt_install(packages=determine_packages(), fatal=True)
-        rm_packages = determine_packages_to_remove()
-        if rm_packages:
-            apt_purge(packages=rm_packages, fatal=True)
+
         kick_function()
 
         owner = ceph_user()
diff --git a/unit_tests/test_mon_upgrade_roll.py b/unit_tests/test_mon_upgrade_roll.py
index 61ef3df..23ced4e 100644
--- a/unit_tests/test_mon_upgrade_roll.py
+++ b/unit_tests/test_mon_upgrade_roll.py
@@ -252,6 +252,7 @@ class UpgradeRollingTestCase(unittest.TestCase):
                                  group='ceph',
                                  perms=0o755)
 
+    @patch.object(charms_ceph.utils, 'upgrade_monitor')
     @patch.object(charms_ceph.utils, 'bootstrap_manager')
     @patch.object(charms_ceph.utils, 'wait_for_all_monitors_to_upgrade')
     @patch.object(charms_ceph.utils, 'status_set')
@@ -267,6 +268,7 @@ class UpgradeRollingTestCase(unittest.TestCase):
                                    status_set,
                                    wait_for_all_monitors_to_upgrade,
                                    bootstrap_manager,
+                                   upgrade_monitor,
                                    new_version):
         socket.gethostname.return_value = "ip-192-168-1-3"
         get_mon_map.return_value = {
@@ -307,6 +309,9 @@ class UpgradeRollingTestCase(unittest.TestCase):
             wait_for_all_monitors_to_upgrade.assert_not_called()
             bootstrap_manager.assert_not_called()
 
+        upgrade_monitor.assert_has_calls([
+            call(new_version, restart_daemons=False)])
+
     def test_roll_monitor_cluster_luminous(self):
         self._test_roll_monitor_cluster(new_version='luminous')