From 5ed4956f2bc594d42abf6fd2fa64dcddc1b86da5 Mon Sep 17 00:00:00 2001 From: Felipe Reyes Date: Tue, 11 Dec 2018 12:25:35 -0300 Subject: [PATCH] Update cloud-archive.list when upgrading from Pike to Queens ceph-mon charm only upgrades when the ceph version changes, for the case of upgrading from Pike to Queens the charm is skipping any upgrades, because the Cloud Archive has Luminous for those 2 releases. This patch checks if the requested ceph version is luminous and if the 'source' changed from pike to queens to then upgrade /etc/apt/sources.list.d/cloud-archive.list via add_source() Change-Id: I05b7d722e45d3a02a97866903a67bd9b16d4f552 Closes-Bug: 1778823 --- hooks/ceph_hooks.py | 19 +++++++++- lib/ceph/utils.py | 1 + unit_tests/test_ceph_hooks.py | 5 ++- unit_tests/test_upgrade.py | 65 ++++++++++++++++++++++++----------- unit_tests/test_utils.py | 19 ++++++++-- 5 files changed, 84 insertions(+), 25 deletions(-) diff --git a/hooks/ceph_hooks.py b/hooks/ceph_hooks.py index 8b70ce71..df438832 100755 --- a/hooks/ceph_hooks.py +++ b/hooks/ceph_hooks.py @@ -30,6 +30,8 @@ from charmhelpers.core import hookenv from charmhelpers.core.hookenv import ( log, DEBUG, + ERROR, + INFO, config, relation_ids, related_units, @@ -64,6 +66,7 @@ from charmhelpers.contrib.openstack.alternatives import install_alternative from charmhelpers.contrib.openstack.utils import ( clear_unit_paused, clear_unit_upgrading, + get_os_codename_install_source, is_unit_upgrading_set, set_unit_paused, set_unit_upgrading, @@ -105,6 +108,11 @@ def check_for_upgrade(): log('old_version: {}'.format(old_version)) # Strip all whitespace new_version = ceph.resolve_ceph_version(hookenv.config('source')) + + old_version_os = get_os_codename_install_source(c.previous('source') or + 'distro') + new_version_os = get_os_codename_install_source(hookenv.config('source')) + log('new_version: {}'.format(new_version)) if (old_version in ceph.UPGRADE_PATHS and @@ -113,12 +121,21 @@ def check_for_upgrade(): old_version, new_version)) ceph.roll_monitor_cluster(new_version=new_version, upgrade_key='admin') + elif (old_version == new_version and + old_version_os < new_version_os): + # See LP: #1778823 + add_source(hookenv.config('source'), hookenv.config('key')) + log(("The installation source has changed yet there is no new major " + "version of Ceph in this new source. As a result no package " + "upgrade will take effect. Please upgrade manually if you need " + "to."), level=INFO) else: # Log a helpful error message log("Invalid upgrade path from {} to {}. " "Valid paths are: {}".format(old_version, new_version, - ceph.pretty_print_upgrade_paths())) + ceph.pretty_print_upgrade_paths()), + level=ERROR) @hooks.hook('install.real') diff --git a/lib/ceph/utils.py b/lib/ceph/utils.py index 4fbf0fbc..a1cfbdc6 100644 --- a/lib/ceph/utils.py +++ b/lib/ceph/utils.py @@ -2564,6 +2564,7 @@ UCA_CODENAME_MAP = { 'pike': 'luminous', 'queens': 'luminous', 'rocky': 'mimic', + 'stein': 'mimic', } diff --git a/unit_tests/test_ceph_hooks.py b/unit_tests/test_ceph_hooks.py index 0a8b4393..a7fb43af 100644 --- a/unit_tests/test_ceph_hooks.py +++ b/unit_tests/test_ceph_hooks.py @@ -187,6 +187,7 @@ class CephHooksTestCase(unittest.TestCase): mocks["apt_install"].assert_called_once_with( ["python-dbus", "lockfile-progs"]) + @patch.object(ceph_hooks, 'service_pause') @patch.object(ceph_hooks, 'notify_radosgws') @patch.object(ceph_hooks, 'ceph') @patch.object(ceph_hooks, 'notify_client') @@ -196,7 +197,8 @@ class CephHooksTestCase(unittest.TestCase): mock_config, mock_notify_client, mock_ceph, - mock_notify_radosgws): + mock_notify_radosgws, + mock_service_pause): config = copy.deepcopy(CHARM_CONFIG) mock_config.side_effect = lambda key: config[key] with patch.multiple( @@ -217,6 +219,7 @@ class CephHooksTestCase(unittest.TestCase): mock_notify_client.assert_called_once_with() mock_notify_radosgws.assert_called_once_with() mock_ceph.update_monfs.assert_called_once_with() + mock_service_pause.assert_called_with('ceph-create-keys') class RelatedUnitsTestCase(unittest.TestCase): diff --git a/unit_tests/test_upgrade.py b/unit_tests/test_upgrade.py index f860f61e..f784f7cb 100644 --- a/unit_tests/test_upgrade.py +++ b/unit_tests/test_upgrade.py @@ -1,10 +1,10 @@ -import unittest +from mock import patch +from ceph_hooks import check_for_upgrade +from test_utils import CharmTestCase + __author__ = 'Chris Holcombe ' -from mock import patch, MagicMock -from ceph_hooks import check_for_upgrade - def config_side_effect(*args): if args[0] == 'source': @@ -15,20 +15,17 @@ def config_side_effect(*args): return 'cloud:trusty-kilo' -class UpgradeRollingTestCase(unittest.TestCase): +class UpgradeRollingTestCase(CharmTestCase): @patch('ceph_hooks.ceph.is_bootstrapped') - @patch('ceph_hooks.ceph.resolve_ceph_version') @patch('ceph_hooks.hookenv') @patch('ceph_hooks.ceph.roll_monitor_cluster') def test_check_for_upgrade(self, roll_monitor_cluster, hookenv, - version, is_bootstrapped): + is_bootstrapped): is_bootstrapped.return_value = True - version.side_effect = ['firefly', 'hammer'] - previous_mock = MagicMock().return_value - previous_mock.previous.return_value = "cloud:trusty-juno" - hookenv.config.side_effect = [previous_mock, - config_side_effect('source')] + self.test_config.set_previous('source', 'cloud:trusty-juno') + self.test_config.set('source', 'cloud:trusty-kilo') + hookenv.config.side_effect = self.test_config check_for_upgrade() roll_monitor_cluster.assert_called_with( @@ -36,18 +33,46 @@ class UpgradeRollingTestCase(unittest.TestCase): upgrade_key='admin') @patch('ceph_hooks.ceph.is_bootstrapped') - @patch('ceph_hooks.ceph.resolve_ceph_version') @patch('ceph_hooks.hookenv') @patch('ceph_hooks.ceph.roll_monitor_cluster') def test_check_for_upgrade_not_bootstrapped(self, roll_monitor_cluster, - hookenv, - version, is_bootstrapped): + hookenv, is_bootstrapped): is_bootstrapped.return_value = False - version.side_effect = ['firefly', 'hammer'] - previous_mock = MagicMock().return_value - previous_mock.previous.return_value = "cloud:trusty-juno" - hookenv.config.side_effect = [previous_mock, - config_side_effect('source')] + self.test_config.set_previous('source', 'cloud:trusty-juno') + self.test_config.set('source', 'cloud:trusty-kilo') + hookenv.config.side_effect = self.test_config check_for_upgrade() roll_monitor_cluster.assert_not_called() + + @patch('ceph_hooks.add_source') + @patch('ceph_hooks.ceph.is_bootstrapped') + @patch('ceph_hooks.hookenv') + @patch('ceph_hooks.ceph.roll_monitor_cluster') + def test_check_for_upgrade_from_pike_to_queens(self, roll_monitor_cluster, + hookenv, is_bootstrapped, + add_source): + is_bootstrapped.return_value = True + hookenv.config.side_effect = self.test_config + self.test_config.set('key', 'some-key') + self.test_config.set_previous('source', 'cloud:xenial-pike') + self.test_config.set('source', 'cloud:xenial-queens') + check_for_upgrade() + roll_monitor_cluster.assert_not_called() + add_source.assert_called_with('cloud:xenial-queens', 'some-key') + + @patch('ceph_hooks.add_source') + @patch('ceph_hooks.ceph.is_bootstrapped') + @patch('ceph_hooks.hookenv') + @patch('ceph_hooks.ceph.roll_monitor_cluster') + def test_check_for_upgrade_from_rocky_to_stein(self, roll_monitor_cluster, + hookenv, is_bootstrapped, + add_source): + is_bootstrapped.return_value = True + hookenv.config.side_effect = self.test_config + self.test_config.set('key', 'some-key') + self.test_config.set_previous('source', 'cloud:bionic-rocky') + self.test_config.set('source', 'cloud:bionic-stein') + check_for_upgrade() + roll_monitor_cluster.assert_not_called() + add_source.assert_called_with('cloud:bionic-stein', 'some-key') diff --git a/unit_tests/test_utils.py b/unit_tests/test_utils.py index 8539d8ec..83fe5ae2 100644 --- a/unit_tests/test_utils.py +++ b/unit_tests/test_utils.py @@ -59,10 +59,10 @@ def get_default_config(): class CharmTestCase(unittest.TestCase): - def setUp(self, obj, patches): + def setUp(self, obj=None, patches=None): super(CharmTestCase, self).setUp() - self.patches = patches - self.obj = obj + self.patches = patches or [] + self.obj = obj or [] self.test_config = TestConfig() self.test_relation = TestRelation() self.test_leader_settings = TestLeaderSettings() @@ -85,6 +85,13 @@ class TestConfig(object): self.config = get_default_config() self.config_changed = {} self.config_changed.setdefault(False) + self._previous = get_default_config() + + def __call__(self, key=None): + if key: + return self[key] + else: + return self def get(self, attr=None): if not attr: @@ -113,6 +120,12 @@ class TestConfig(object): def set_changed(self, attr, changed=True): self.config_changed[attr] = changed + def set_previous(self, key, value): + self._previous[key] = value + + def previous(self, key): + return self._previous[key] + class TestRelation(object):