From 3d043ae144b86071033101f2b0119e90ff526f7a Mon Sep 17 00:00:00 2001 From: Aurelien Lourot Date: Tue, 28 Sep 2021 10:44:26 +0200 Subject: [PATCH] Process subordinate releases packages map For principal - subordinate plugin type relations where the principal Python payload imports code from packages managed by a subordinate, upgrades can be problematic. This change will allow a subordinate charm that have opted into the feature to inform its principal about all implemented release - packages combinations ahead of time. With this information in place the principal can do the upgrade in one operation without risk of charm relation RPC type processing at a critical moment. Related-Bug: #1806111 Change-Id: Ic8ea4fe6109081814045adea7ce6688b3564c9e5 Co-authored-by: Aurelien Lourot --- bindep.txt | 4 +++ hooks/cinder_utils.py | 19 ++++++++---- unit_tests/test_cinder_utils.py | 52 ++++++++++++++++++++++++--------- 3 files changed, 57 insertions(+), 18 deletions(-) create mode 100644 bindep.txt diff --git a/bindep.txt b/bindep.txt new file mode 100644 index 00000000..ba2ccb4b --- /dev/null +++ b/bindep.txt @@ -0,0 +1,4 @@ +libxml2-dev [platform:dpkg test] +libxslt1-dev [platform:dpkg test] +build-essential [platform:dpkg test] +zlib1g-dev [platform:dpkg test] diff --git a/hooks/cinder_utils.py b/hooks/cinder_utils.py index 39d8897b..a6f60ae9 100644 --- a/hooks/cinder_utils.py +++ b/hooks/cinder_utils.py @@ -97,6 +97,7 @@ from charmhelpers.contrib.openstack import ( from charmhelpers.contrib.openstack.utils import ( configure_installation_source, get_os_codename_install_source, + get_subordinate_release_packages, os_release, reset_os_release, make_assess_status_func, @@ -389,13 +390,17 @@ def determine_packages(): pkgs.extend(token_cache_pkgs(source=config()['openstack-origin'])) - if CompareOpenStackReleases(os_release('cinder')) >= 'rocky': + release = os_release('cinder') + if CompareOpenStackReleases(release) >= 'rocky': pkgs = [p for p in pkgs if not p.startswith('python-')] pkgs.extend(PY3_PACKAGES) if service_enabled('api'): pkgs.extend(PY3_API_PACKAGES) - return pkgs + pkgs = set(pkgs).union(get_subordinate_release_packages( + release).install) + + return sorted(pkgs) def determine_purge_packages(): @@ -405,12 +410,16 @@ def determine_purge_packages(): :returns: list of package names ''' - if CompareOpenStackReleases(os_release('cinder')) >= 'rocky': + pkgs = [] + release = os_release('cinder') + if CompareOpenStackReleases(release) >= 'rocky': pkgs = [p for p in COMMON_PACKAGES if p.startswith('python-')] pkgs.append('python-cinder') pkgs.append('python-memcache') - return pkgs - return [] + + pkgs = set(pkgs).union( + get_subordinate_release_packages(release).purge) + return sorted(pkgs) def remove_old_packages(): diff --git a/unit_tests/test_cinder_utils.py b/unit_tests/test_cinder_utils.py index a717f003..bd900f14 100644 --- a/unit_tests/test_cinder_utils.py +++ b/unit_tests/test_cinder_utils.py @@ -127,21 +127,31 @@ class TestCinderUtils(CharmTestCase): self.test_config.set('enabled-services', 'api,scheduler') self.assertFalse(cinder_utils.service_enabled('volume')) - def test_determine_purge_packages(self): + @patch.object(cinder_utils, 'get_subordinate_release_packages') + def test_determine_purge_packages( + self, + mock_get_subordinate_release_packages): 'Ensure no packages are identified for purge prior to rocky' self.os_release.return_value = 'queens' self.assertEqual(cinder_utils.determine_purge_packages(), []) - def test_determine_purge_packages_rocky(self): + @patch.object(cinder_utils, 'get_subordinate_release_packages') + def test_determine_purge_packages_rocky( + self, + mock_get_subordinate_release_packages): 'Ensure python packages are identified for purge at rocky' self.os_release.return_value = 'rocky' self.assertEqual(cinder_utils.determine_purge_packages(), - [p for p in cinder_utils.COMMON_PACKAGES - if p.startswith('python-')] + - ['python-cinder', 'python-memcache']) + sorted(set([p for p in cinder_utils.COMMON_PACKAGES + if p.startswith('python-')] + + ['python-cinder', 'python-memcache']))) + @patch.object(cinder_utils, 'get_subordinate_release_packages') @patch('cinder_utils.service_enabled') - def test_determine_packages_all(self, service_enabled): + def test_determine_packages_all( + self, + service_enabled, + mock_get_subordinate_release_packages): 'It determines all packages required when all services enabled' service_enabled.return_value = True self.os_release.return_value = 'icehouse' @@ -152,8 +162,12 @@ class TestCinderUtils(CharmTestCase): cinder_utils.API_PACKAGES + cinder_utils.SCHEDULER_PACKAGES)) + @patch.object(cinder_utils, 'get_subordinate_release_packages') @patch('cinder_utils.service_enabled') - def test_determine_packages_all_rocky(self, service_enabled): + def test_determine_packages_all_rocky( + self, + service_enabled, + mock_get_subordinate_release_packages): 'Check python3 packages are installed @ rocky' service_enabled.return_value = True self.os_release.return_value = 'rocky' @@ -168,8 +182,10 @@ class TestCinderUtils(CharmTestCase): cinder_utils.PY3_PACKAGES + cinder_utils.PY3_API_PACKAGES)) + @patch.object(cinder_utils, 'get_subordinate_release_packages') @patch('cinder_utils.service_enabled') - def test_determine_packages_subset(self, service_enabled): + def test_determine_packages_subset(self, service_enabled, + mock_get_subordinate_release_packages): 'It determines packages required for a subset of enabled services' service_enabled.side_effect = self.svc_enabled self.test_config.set('openstack-origin', 'cloud:xenial-newton') @@ -771,13 +787,15 @@ class TestCinderUtils(CharmTestCase): out.write('env CEPH_ARGS="--id %s"\n' % service) """ + @patch.object(cinder_utils, 'get_subordinate_release_packages') @patch.object(cinder_utils, 'service_enabled') @patch.object(cinder_utils, 'register_configs') @patch.object(cinder_utils, 'services') @patch.object(cinder_utils, 'migrate_database') @patch.object(cinder_utils, 'determine_packages') def test_openstack_upgrade_leader(self, pkgs, migrate, services, - mock_register_configs, service_enabled): + mock_register_configs, service_enabled, + mock_get_subordinate_release_packages): pkgs.return_value = ['mypackage'] self.os_release.return_value = 'havana' self.config.side_effect = None @@ -796,14 +814,20 @@ class TestCinderUtils(CharmTestCase): configs.set_release.assert_called_with(openstack_release='havana') self.assertTrue(migrate.called) + @patch.object(cinder_utils, 'get_subordinate_release_packages') @patch.object(cinder_utils, 'service_enabled') @patch.object(cinder_utils, 'register_configs') @patch.object(cinder_utils, 'services') @patch.object(cinder_utils, 'migrate_database') @patch.object(cinder_utils, 'determine_packages') - def test_openstack_upgrade_not_leader(self, pkgs, migrate, services, - mock_register_configs, - service_enabled): + def test_openstack_upgrade_not_leader( + self, + pkgs, + migrate, + services, + mock_register_configs, + service_enabled, + mock_get_subordinate_release_packages): pkgs.return_value = ['mypackage'] self.os_release.return_value = 'havana' self.config.side_effect = None @@ -822,6 +846,7 @@ class TestCinderUtils(CharmTestCase): configs.set_release.assert_called_with(openstack_release='havana') self.assertFalse(migrate.called) + @patch.object(cinder_utils, 'get_subordinate_release_packages') @patch.object(cinder_utils, 'service_enabled') @patch.object(cinder_utils, 'register_configs') @patch.object(cinder_utils, 'services') @@ -829,7 +854,8 @@ class TestCinderUtils(CharmTestCase): @patch.object(cinder_utils, 'determine_packages') def test_openstack_upgrade_rocky(self, pkgs, migrate, services, mock_register_configs, - service_enabled): + service_enabled, + mock_get_subordinate_release_packages): pkgs.return_value = ['mypackage'] self.os_release.return_value = 'rocky' self.config.side_effect = None