From a15573625a90426fd1081af7d9dc3b92712e15f0 Mon Sep 17 00:00:00 2001 From: Rodrigo Barbieri Date: Tue, 3 Oct 2023 10:31:52 -0300 Subject: [PATCH] review feedback --- zaza/openstack/charm_tests/nova/tests.py | 87 +++++++++++++++-------- zaza/openstack/charm_tests/vault/setup.py | 11 ++- zaza/openstack/utilities/openstack.py | 14 +--- 3 files changed, 68 insertions(+), 44 deletions(-) diff --git a/zaza/openstack/charm_tests/nova/tests.py b/zaza/openstack/charm_tests/nova/tests.py index 81f1352b1..946ab1ac1 100644 --- a/zaza/openstack/charm_tests/nova/tests.py +++ b/zaza/openstack/charm_tests/nova/tests.py @@ -367,6 +367,9 @@ def wait_for_nova_compute_count(expected_count): if service_count < 1: self.fail("Unit '{}' has no nova-compute services registered in" " nova-cloud-controller".format(unit_to_remove.name)) + + # This elif would be removed in a future code when we have support + # for more than 1 nova-compute node in the CI elif service_count > 1: self.fail("Unexpected number of nova-compute services registered" " in nova-cloud controller. Expecting: 1, found: " @@ -380,7 +383,7 @@ def wait_for_nova_compute_count(expected_count): # Wait for nova-compute service to be removed from the # nova-cloud-controller - if not wait_for_nova_compute_count(service_count-1): + if not wait_for_nova_compute_count(service_count - 1): self.fail("nova-compute service was not unregistered from the " "nova-cloud-controller as expected.") @@ -482,43 +485,67 @@ def test_901_pause_resume(self): logging.info("Testing pause resume") def test_904_test_ceph_keys(self): - """Run pause and resume tests. - - Pause service and check services are stopped then resume and check - they are started - """ - # Expected default and alternate values + """Test if the ceph keys in /etc/ceph are correct.""" + # only run if configured as rbd with ceph image backend if zaza.model.get_application_config( self.application_name)['libvirt-image-backend'].get( 'value') != 'rbd': return + # Regex for + # [client.nova-compute] + # key = AQBm5xJl8CSnFxAACB9GVr2llNO0G8zWZuZnjQ == regex = re.compile(r"^\[client.(.+)\]\n\tkey = (.+)$") key_dict = {} - def check_keyring(app_name_dict_list): - for app_name_dict in app_name_dict_list: - for app_name, key_list in app_name_dict.items(): - for unit in zaza.model.get_units( - app_name, model_name=self.model_name): - for key_app_name in key_list: - keyring_file = ( - '/etc/ceph/ceph.client.{}.keyring'.format( - key_app_name)) - data = str(generic_utils.get_file_contents( - unit, keyring_file)) - - result = regex.findall(data) - self.assertEqual(2, len(result)) - self.assertEqual(result[0], key_app_name) - for k, v in key_dict.items(): - if k == result[0]: - self.assertEqual(v, result[1]) - else: - self.assertNotEqual(v, result[1]) - key_dict[result[0]] = result[1] - - check_keyring([{self.application_name: [self.application_name]}]) + # The new and correct behavior is to have "nova-compute" named keyring + # and one other named after the charm app, IF the charm app name is + # different than "nova-compute". Example: + # for a charm app named "nova-compute-kvm", + # it should have both nova-compute-kvm and nova-compute keyrings. + # for a charm app named "nova-compute", + # it should have only nova-compute keyring. + + # Previous behaviors: + # The old behavior is to have only 1 keyring named after the charm app. + # the intermediary behavior with the partial fix is to always have only + # the "nova-compute" keyring. + + nova_compute_apps = openstack_utils.get_app_names_for_charm( + 'nova-compute') + + def check_keyring(app_name): + """Check matching keyring name and different from existing ones.""" + keyring_file = ( + '/etc/ceph/ceph.client.{}.keyring'.format(app_name)) + data = str(generic_utils.get_file_contents( + unit, keyring_file)) + + result = regex.findall(data) + + # Assert keyring file name matches intended name + self.assertEqual(2, len(result)) + self.assertEqual(result[0], app_name) + + # Confirm the keys are different from each already checked + # nova-compute charm app + for k, v in key_dict.items(): + if k == result[0]: + self.assertEqual(v, result[1]) + else: + self.assertNotEqual(v, result[1]) + key_dict[result[0]] = result[1] + + # We typically only have 1 nova-compute charm app in the CI, + # so this code is future-proofing + for app_name in nova_compute_apps: + for unit in zaza.model.get_units( + app_name, model_name=self.model_name): + + check_keyring('nova-compute') + # extra future code for the new fixed version + # if app_name != 'nova-compute': + # check_keyring(app_name) def test_930_check_virsh_default_network(self): """Test default virt network is not present.""" diff --git a/zaza/openstack/charm_tests/vault/setup.py b/zaza/openstack/charm_tests/vault/setup.py index 93ed718d4..912282cbc 100644 --- a/zaza/openstack/charm_tests/vault/setup.py +++ b/zaza/openstack/charm_tests/vault/setup.py @@ -188,13 +188,18 @@ def auto_initialize(cacert=None, validation_application='keystone', wait=True, validate_ca(cacertificate, application=validation_application) # Once validation has completed restart nova-compute to work around # bug #1826382 - nova_compute_app_name = get_app_names_for_charm('nova-compute')[0] + nova_compute_app_names = get_app_names_for_charm('nova-compute') + charm_apps_to_restart_svcs = (nova_compute_app_names + + ['nova-cloud-controller']) + cmd_map = { 'nova-cloud-controller': ('systemctl restart ' 'nova-scheduler nova-conductor'), - nova_compute_app_name: 'systemctl restart nova-compute', } - for app in (nova_compute_app_name, 'nova-cloud-controller',): + for nova_compute_app_name in nova_compute_app_names: + cmd_map[nova_compute_app_name] = 'systemctl restart nova-compute' + + for app in charm_apps_to_restart_svcs: try: for unit in zaza.model.get_units(app): result = zaza.model.run_on_unit( diff --git a/zaza/openstack/utilities/openstack.py b/zaza/openstack/utilities/openstack.py index 6ad2d33b4..a43d3ae4e 100644 --- a/zaza/openstack/utilities/openstack.py +++ b/zaza/openstack/utilities/openstack.py @@ -2702,17 +2702,9 @@ def get_app_names_for_charm(charm_name, model_name=None): """ status = juju_utils.get_full_juju_status(model_name=model_name) result = [] - try: - for app_name, app_data in status['applications'].items(): - try: - if charm_name == app_data['charm-name']: - result.append(app_name) - except KeyError: - # Older juju versions may not have the field "charm-name" - if charm_name in app_data['charm']: - result.append(app_name) - except KeyError: - pass + for app_name, app_data in status['applications'].items(): + if charm_name == app_data['charm-name']: + result.append(app_name) return result