diff --git a/hooks/ceph_hooks.py b/hooks/ceph_hooks.py index 7828613f..21885dcc 100755 --- a/hooks/ceph_hooks.py +++ b/hooks/ceph_hooks.py @@ -99,6 +99,7 @@ from charmhelpers.contrib.hardening.harden import harden from charmhelpers.contrib.openstack.utils import ( clear_unit_paused, clear_unit_upgrading, + get_os_codename_install_source, is_unit_paused_set, is_unit_upgrading_set, set_unit_paused, @@ -117,6 +118,7 @@ CRON_CEPH_CHECK_FILE = '/etc/cron.d/check-osd-services' def check_for_upgrade(): + if not os.path.exists(ceph._upgrade_keyring): log("Ceph upgrade keyring not detected, skipping upgrade checks.") return @@ -129,14 +131,14 @@ def check_for_upgrade(): 'distro') log('new_version: {}'.format(new_version)) + old_version_os = get_os_codename_install_source(c.previous('source') or + 'distro') + new_version_os = get_os_codename_install_source(hookenv.config('source')) + # May be in a previous upgrade that was failed if the directories # still need an ownership update. Check this condition. resuming_upgrade = ceph.dirs_need_ownership_update('osd') - if old_version == new_version and not resuming_upgrade: - log("No new ceph version detected, skipping upgrade.", DEBUG) - return - if (ceph.UPGRADE_PATHS.get(old_version) == new_version) or\ resuming_upgrade: if old_version == new_version: @@ -150,12 +152,21 @@ def check_for_upgrade(): ceph.roll_osd_cluster(new_version=new_version, upgrade_key='osd-upgrade') emit_cephconf(upgrading=False) + 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) def tune_network_adapters(): diff --git a/lib/ceph/utils.py b/lib/ceph/utils.py index 5bff375d..83388c9b 100644 --- a/lib/ceph/utils.py +++ b/lib/ceph/utils.py @@ -2568,6 +2568,7 @@ UCA_CODENAME_MAP = { 'pike': 'luminous', 'queens': 'luminous', 'rocky': 'mimic', + 'stein': 'mimic', } diff --git a/unit_tests/test_upgrade.py b/unit_tests/test_upgrade.py index ad876214..fa47ff71 100644 --- a/unit_tests/test_upgrade.py +++ b/unit_tests/test_upgrade.py @@ -1,22 +1,12 @@ -import unittest - -__author__ = 'Chris Holcombe ' - -from mock import call, patch, MagicMock - +from mock import call, patch +from test_utils import CharmTestCase from ceph_hooks import check_for_upgrade -def config_side_effect(*args): - if args[0] == 'source': - return 'cloud:trusty-kilo' - elif args[0] == 'key': - return 'key' - elif args[0] == 'release-version': - return 'cloud:trusty-kilo' +__author__ = 'Chris Holcombe ' -class UpgradeRollingTestCase(unittest.TestCase): +class UpgradeRollingTestCase(CharmTestCase): @patch('ceph_hooks.ceph.dirs_need_ownership_update') @patch('ceph_hooks.os.path.exists') @@ -30,10 +20,12 @@ class UpgradeRollingTestCase(unittest.TestCase): dirs_need_ownership_update.return_value = False exists.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') + self.test_config.set('key', 'key') + + hookenv.config.side_effect = self.test_config check_for_upgrade() roll_osd_cluster.assert_called_with(new_version='hammer', @@ -75,12 +67,58 @@ class UpgradeRollingTestCase(unittest.TestCase): version, exists): exists.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') + self.test_config.set('key', 'key') + + hookenv.config.side_effect = self.test_config check_for_upgrade() roll_monitor_cluster.assert_not_called() exists.assert_called_with( "/var/lib/ceph/osd/ceph.client.osd-upgrade.keyring") + + @patch('ceph_hooks.os.path.exists') + @patch('ceph_hooks.ceph.dirs_need_ownership_update') + @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, + dirs_need_ownership_update, + exists): + exists.return_value = True + dirs_need_ownership_update.return_value = False + 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.os.path.exists') + @patch('ceph_hooks.ceph.dirs_need_ownership_update') + @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, + dirs_need_ownership_update, + exists): + exists.return_value = True + dirs_need_ownership_update.return_value = False + 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 90db851e..639552e2 100644 --- a/unit_tests/test_utils.py +++ b/unit_tests/test_utils.py @@ -67,10 +67,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.patch_all() @@ -90,6 +90,18 @@ class TestConfig(object): def __init__(self): 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 __getitem__(self, item): + return self.config[item] def get(self, attr=None): if not attr: @@ -107,6 +119,18 @@ class TestConfig(object): raise KeyError self.config[attr] = value + def changed(self, attr): + return self.config_changed[attr] + + 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):