From 2cfb795490fb5985c24a11741ebb2b40b5d9ac28 Mon Sep 17 00:00:00 2001 From: Corey Bryant Date: Thu, 17 Oct 2019 19:25:47 +0000 Subject: [PATCH] Ensure placement charm related before Train upgrade As of OpenStack Train, the placement charm manages the placement API, and it is no longer managed by nova-cloud-controller. This requires the placement charm to be deployed and related to nova-cloud-controller prior to upgrading nova-cloud-controller to Train. This patch ensures that if an attempt is made to upgrade nova-cloud-controller from Stein to Train, and placement is not yet related, it will block and prevent the upgrade. Change-Id: I217adfb59aed2e509a56b6559a528ae4c0adaa48 Closes-Bug: 1848529 --- actions/openstack_upgrade.py | 7 ++++ hooks/nova_cc_hooks.py | 3 +- hooks/nova_cc_utils.py | 28 +++++++++++++++- unit_tests/test_actions_openstack_upgrade.py | 24 ++++++++++++-- unit_tests/test_nova_cc_hooks.py | 8 +++++ unit_tests/test_nova_cc_utils.py | 35 ++++++++++++++++---- 6 files changed, 94 insertions(+), 11 deletions(-) diff --git a/actions/openstack_upgrade.py b/actions/openstack_upgrade.py index ee0c702e..25754926 100755 --- a/actions/openstack_upgrade.py +++ b/actions/openstack_upgrade.py @@ -41,6 +41,13 @@ def openstack_upgrade(): code to run, otherwise a full service level upgrade will fire on config-changed.""" + # If attempting to upgrade from Stein->Train, block until Placement + # charm is related. Status is set in check_optional_relations(). + release = ch_utils.os_release('nova-common') + cmp_os_release = ch_utils.CompareOpenStackReleases(release) + if (cmp_os_release == 'stein' and not hookenv.relation_ids('placement')): + return + if (ch_utils.do_action_openstack_upgrade('nova-common', utils.do_openstack_upgrade, hooks.CONFIGS)): diff --git a/hooks/nova_cc_hooks.py b/hooks/nova_cc_hooks.py index 02995326..9711fcc5 100755 --- a/hooks/nova_cc_hooks.py +++ b/hooks/nova_cc_hooks.py @@ -252,7 +252,8 @@ def config_changed(): if not hookenv.config('action-managed-upgrade'): if ch_utils.openstack_upgrade_available('nova-common'): hookenv.status_set('maintenance', 'Running openstack upgrade') - ncc_utils.do_openstack_upgrade(CONFIGS) + if not ncc_utils.do_openstack_upgrade(CONFIGS): + return for rid in hookenv.relation_ids('neutron-api'): neutron_api_relation_joined(rid=rid, remote_restart=True) # NOTE(jamespage): Force re-fire of shared-db joined hook diff --git a/hooks/nova_cc_utils.py b/hooks/nova_cc_utils.py index a31c5e48..80015a44 100644 --- a/hooks/nova_cc_utils.py +++ b/hooks/nova_cc_utils.py @@ -706,6 +706,13 @@ def database_setup(prefix): def do_openstack_upgrade(configs): + # If attempting to upgrade from Stein->Train, block until Placement + # charm is related. Status is set in check_optional_relations(). + release = ch_utils.os_release('nova-common') + cmp_os_release = ch_utils.CompareOpenStackReleases(release) + if (cmp_os_release == 'stein' and not hookenv.relation_ids('placement')): + return None + new_src = hookenv.config('openstack-origin') step_src = get_step_upgrade_source(new_src) @@ -1648,7 +1655,12 @@ def get_optional_interfaces(): def check_optional_relations(configs): - """Check that if we have a relation_id for high availability that we can + """Check optional relations and set status + + If attempting to upgrade from Stein->Train, block until Placement + charm is related. + + Also check that if we have a relation_id for high availability that we can get the hacluster config. If we can't then we are blocked. This function is called from assess_status/set_os_workload_status as the @@ -1658,6 +1670,19 @@ def check_optional_relations(configs): :param configs: an OSConfigRender() instance. :return 2-tuple: (string, string) = (status, message) """ + cur_os_rel = ch_utils.os_release('nova-common') + cmp_cur_os_rel = ch_utils.CompareOpenStackReleases(cur_os_rel) + new_src = hookenv.config('openstack-origin') + new_os_rel = ch_utils.get_os_codename_install_source(new_src) + cmp_new_os_rel = ch_utils.CompareOpenStackReleases(new_os_rel) + + if (cmp_cur_os_rel == 'stein' and + cmp_new_os_rel == 'train' and + not hookenv.relation_ids('placement')): + return ('blocked', + 'placement charm must be related prior to ' + 'upgrading to OpenStack Train') + if hookenv.relation_ids('ha'): try: ch_cluster.get_hacluster_config() @@ -1665,6 +1690,7 @@ def check_optional_relations(configs): return ('blocked', 'hacluster missing configuration: ' 'vip, vip_iface, vip_cidr') + # return 'unknown' as the lowest priority to not clobber an existing # status. return "unknown", "" diff --git a/unit_tests/test_actions_openstack_upgrade.py b/unit_tests/test_actions_openstack_upgrade.py index d2520322..3092e8d2 100644 --- a/unit_tests/test_actions_openstack_upgrade.py +++ b/unit_tests/test_actions_openstack_upgrade.py @@ -33,13 +33,19 @@ class TestNovaCCUpgradeActions(CharmTestCase): super(TestNovaCCUpgradeActions, self).setUp(openstack_upgrade, TO_PATCH) + @patch('charmhelpers.contrib.openstack.utils.CompareOpenStackReleases') + @patch('charmhelpers.contrib.openstack.utils.os_release') @patch('charmhelpers.contrib.openstack.utils.config') @patch('charmhelpers.contrib.openstack.utils.action_set') @patch('charmhelpers.contrib.openstack.utils.openstack_upgrade_available') - def test_openstack_upgrade_true(self, upgrade_avail, - action_set, config): + def test_openstack_upgrade_true(self, upgrade_avail, action_set, + config, os_release, compare_releases): upgrade_avail.return_value = True config.return_value = True + + # upgrade from stein with placement related + os_release.return_value = 'stein' + compare_releases.return_value = 'stein' self.relation_ids.return_value = ['relid1'] openstack_upgrade.openstack_upgrade() @@ -50,13 +56,25 @@ class TestNovaCCUpgradeActions(CharmTestCase): self.db_joined.assert_called_with(relation_id='relid1') self.assertTrue(self.config_changed.called) + # upgrade from stein without placement related + os_release.return_value = 'stein' + compare_releases.return_value = 'stein' + self.relation_ids.return_value = [] + self.do_openstack_upgrade.reset_mock() + + openstack_upgrade.openstack_upgrade() + + self.assertFalse(self.do_openstack_upgrade.called) + + @patch('charmhelpers.contrib.openstack.utils.os_release') @patch('charmhelpers.contrib.openstack.utils.config') @patch('charmhelpers.contrib.openstack.utils.action_set') @patch('charmhelpers.contrib.openstack.utils.openstack_upgrade_available') def test_openstack_upgrade_false(self, upgrade_avail, - action_set, config): + action_set, config, os_release): upgrade_avail.return_value = True config.return_value = False + os_release.return_value = 'stein' openstack_upgrade.openstack_upgrade() diff --git a/unit_tests/test_nova_cc_hooks.py b/unit_tests/test_nova_cc_hooks.py index 58c7e98b..badf87b1 100644 --- a/unit_tests/test_nova_cc_hooks.py +++ b/unit_tests/test_nova_cc_hooks.py @@ -312,6 +312,14 @@ class NovaCCHooksTests(CharmTestCase): self.assertTrue(mock_quantum_joined.called) self.assertTrue(mock_update_aws_compat_services.called) + # test upgrade from stein->train without placement related + self.do_openstack_upgrade.return_value = None + self.os_release.return_value = 'stein' + self.save_script_rc.reset_mock() + hooks.config_changed() + self.assertTrue(self.do_openstack_upgrade.called) + self.assertFalse(self.save_script_rc.called) + @patch.object(utils, 'set_shared_metadatasecret') @patch.object(utils, 'get_shared_metadatasecret') @patch.object(hooks, 'update_nrpe_config') diff --git a/unit_tests/test_nova_cc_utils.py b/unit_tests/test_nova_cc_utils.py index faec57b6..5ac721d1 100644 --- a/unit_tests/test_nova_cc_utils.py +++ b/unit_tests/test_nova_cc_utils.py @@ -52,7 +52,9 @@ TO_PATCH = [ 'charmhelpers.core.host.service_running', 'charmhelpers.core.host.service_start', 'charmhelpers.core.host.service_stop', + 'charmhelpers.fetch.apt_autoremove', 'charmhelpers.fetch.apt_install', + 'charmhelpers.fetch.apt_purge', 'charmhelpers.fetch.apt_update', 'charmhelpers.fetch.apt_upgrade', 'disable_policy_rcd', @@ -1038,12 +1040,12 @@ class NovaCCUtilsTests(CharmTestCase): @patch.object(utils, 'get_step_upgrade_source') @patch.object(utils, 'migrate_nova_databases') @patch.object(utils, 'determine_packages') - def test_upgrade_queens_rocky(self, determine_packages, - migrate_nova_databases, - get_step_upgrade_source, - database_setup, - disable_package_apache_site, - online_data_migrations_if_needed): + def test_upgrade_to_rocky_and_to_train(self, determine_packages, + migrate_nova_databases, + get_step_upgrade_source, + database_setup, + disable_package_apache_site, + online_data_migrations_if_needed): "Simulate a call to do_openstack_upgrade() for queens->rocky" self.test_config.set('openstack-origin', 'cloud:bionic-queens') get_step_upgrade_source.return_value = None @@ -1052,7 +1054,9 @@ class NovaCCUtilsTests(CharmTestCase): self.is_leader.return_value = True self.relation_ids.return_value = [] database_setup.return_value = False + utils.do_openstack_upgrade(self.register_configs()) + self.apt_update.assert_called_with(fatal=True) self.apt_upgrade.assert_called_with(options=DPKG_OPTS, fatal=True, dist=True) @@ -1063,6 +1067,25 @@ class NovaCCUtilsTests(CharmTestCase): online_data_migrations_if_needed.assert_called_once() disable_package_apache_site.assert_called_once() + # test upgrade from stein->train without placement related + self.os_release.return_value = 'stein' + self.get_os_codename_install_source.return_value = 'train' + self.apt_update.reset_mock() + + utils.do_openstack_upgrade(self.register_configs()) + + self.assertFalse(self.apt_update.called) + + # test upgrade from stein->train with placement related + self.os_release.return_value = 'stein' + self.get_os_codename_install_source.return_value = 'train' + self.relation_ids.return_value = ['placement-id'] + self.apt_update.reset_mock() + + utils.do_openstack_upgrade(self.register_configs()) + + self.assertTrue(self.apt_update.called) + def test_guard_map_nova(self): self.relation_ids.return_value = [] self.os_release.return_value = 'icehouse'