From 336581efeb9a1c5d363c5d3ce631d9bad1a8a337 Mon Sep 17 00:00:00 2001 From: Felipe Reyes Date: Fri, 1 Feb 2019 11:39:27 -0300 Subject: [PATCH] Update cloud-archive.list when upgrading from Pike to Queens ceph-osd 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: I27c2d7648a1add6528924724a03682060d2d6007 Closes-Bug: 1778823 --- hooks/ceph_hooks.py | 21 +++++++--- lib/ceph/utils.py | 1 + unit_tests/test_upgrade.py | 82 ++++++++++++++++++++++++++++---------- unit_tests/test_utils.py | 30 ++++++++++++-- 4 files changed, 104 insertions(+), 30 deletions(-) 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 08e627c4..8d06328f 100644 --- a/lib/ceph/utils.py +++ b/lib/ceph/utils.py @@ -2563,6 +2563,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):