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):