diff --git a/requirements.txt b/requirements.txt index 1a3b33cc7..992af69e8 100644 --- a/requirements.txt +++ b/requirements.txt @@ -50,7 +50,10 @@ python-novaclient python-octaviaclient python-swiftclient python-watcherclient -tenacity +# Due to https://github.com/jd/tenacity/pull/479 the strategy for mocking out tenacity +# waits/times/etc no longer works. Pin to 8.4.1 until it is solved. +# Bug in tenacity tracking issue: https://github.com/jd/tenacity/issues/482 +tenacity<8.4.2 paramiko # Documentation requirements diff --git a/unit_tests/utilities/test_utilities.py b/unit_tests/utilities/test_utilities.py index 6af6089b3..7eb99276d 100644 --- a/unit_tests/utilities/test_utilities.py +++ b/unit_tests/utilities/test_utilities.py @@ -113,6 +113,28 @@ def func(self): mock_sleep.assert_not_called() + @mock.patch("time.sleep") + def test_object_wrap_multilevel_with_exception(self, mock_sleep): + + class A: + + def func(self): + raise SomeException() + + class B: + + def __init__(self): + self.a = A() + + b = B() + # retry on a specific exception + wrapped_b = utilities.ObjectRetrierWraps( + b, num_retries=1, retry_exceptions=[SomeException]) + with self.assertRaises(SomeException): + wrapped_b.a.func() + + mock_sleep.assert_called_once_with(5) + @mock.patch("time.sleep") def test_log_called(self, mock_sleep): @@ -128,9 +150,7 @@ def func(self): with self.assertRaises(SomeException): wrapped_a.func() - # there should be two calls; one for the single retry and one for the - # failure. - self.assertEqual(mock_log.call_count, 2) + mock_log.assert_called() @mock.patch("time.sleep") def test_back_off_maximum(self, mock_sleep): diff --git a/unit_tests/utilities/test_zaza_utilities_openstack.py b/unit_tests/utilities/test_zaza_utilities_openstack.py index 358f15a64..57544c17b 100644 --- a/unit_tests/utilities/test_zaza_utilities_openstack.py +++ b/unit_tests/utilities/test_zaza_utilities_openstack.py @@ -99,6 +99,9 @@ def setUp(self): self.neutronclient.list_agents.return_value = self.agents self.neutronclient.list_bgp_speaker_on_dragent.return_value = \ self.bgp_speakers + self.patch("zaza.openstack.utilities.ObjectRetrierWraps", + name="_object_retrier_wraps", + new=lambda x, *_, **__: x) def test_create_port(self): self.patch_object(openstack_utils, "get_net_uuid") diff --git a/zaza/openstack/charm_tests/audit/__init__.py b/zaza/openstack/charm_tests/audit/__init__.py new file mode 100644 index 000000000..0c3a4c526 --- /dev/null +++ b/zaza/openstack/charm_tests/audit/__init__.py @@ -0,0 +1,20 @@ +# Copyright 2024 Canonical Ltd. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +""" +Keystone audit middleware. + +Collection of code for setting up and testing Keystone audit middleware +functionality. +""" diff --git a/zaza/openstack/charm_tests/audit/tests.py b/zaza/openstack/charm_tests/audit/tests.py new file mode 100644 index 000000000..b10c3837f --- /dev/null +++ b/zaza/openstack/charm_tests/audit/tests.py @@ -0,0 +1,119 @@ +#!/usr/bin/env python3 +# +# Copyright 2024 Canonical Ltd. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +""" +Keystone audit middleware API logging testing. + +These methods test the rendering of the charm api-paste.ini file to +ensure the appropriate sections are rendered or not rendered depending +on the state of the audit-middleware configuration option. +""" + +import textwrap +import logging +import zaza.model +import zaza.openstack.charm_tests.test_utils as test_utils + + +class KeystoneAuditMiddlewareTest(test_utils.OpenStackBaseTest): + """Keystone audit middleware test class.""" + + @classmethod + def setUpClass(cls): + """Run class setup for Keystone audit middleware tests.""" + super(KeystoneAuditMiddlewareTest, cls).setUpClass() + test_config = cls.test_config['tests_options']['audit-middleware'] + cls.service_name = test_config['service'] + + cls.application_name = test_config.get('application', cls.service_name) + logging.info('Using application name: %s', cls.application_name) + + cls.initial_audit_middleware = zaza.model.get_application_config( + cls.application_name)['audit-middleware']['value'] + + @classmethod + def tearDownClass(cls): + """Restore the audit-middleware configuration to its original state.""" + super(KeystoneAuditMiddlewareTest, cls).tearDownClass() + logging.info("Running teardown on %s" % cls.application_name) + zaza.model.set_application_config( + cls.application_name, + {'audit-middleware': str(cls.initial_audit_middleware)}, + model_name=cls.model_name + ) + zaza.model.wait_for_application_states( + states={cls.application_name: { + 'workload-status': 'active', + 'workload-status-message': 'Unit is ready'}}, + model_name=cls.model_name + ) + + def fetch_api_paste_content(self): + """Fetch content of api-paste.ini file.""" + api_paste_ini_path = f"/etc/{self.service_name}/api-paste.ini" + lead_unit = zaza.model.get_lead_unit_name( + self.application_name, + model_name=self.model_name + ) + try: + return zaza.model.file_contents( + lead_unit, + api_paste_ini_path, + ) + except zaza.model.CommandRunFailed as e: + self.fail("Error fetching api-paste.ini: %s" % e) + + def test_101_apipaste_includes_audit_section(self): + """Test api-paste.ini renders audit section when enabled.""" + expected_content = textwrap.dedent(f"""\ + [filter:audit] + paste.filter_factory = keystonemiddleware.audit:filter_factory + audit_map_file = /etc/{self.service_name}/api_audit_map.conf + service_name = {self.service_name} + """) + + set_default = {'audit-middleware': False} + set_alternate = {'audit-middleware': True} + + with self.config_change(default_config=set_default, + alternate_config=set_alternate, + application_name=self.application_name): + api_paste_content = self.fetch_api_paste_content() + self.assertIn(expected_content, api_paste_content) + + def test_102_apipaste_excludes_audit_section(self): + """Test api_paste.ini does not render audit section when disabled.""" + section_heading = '[filter:audit]' + set_default = {'audit-middleware': True} + set_alternate = {'audit-middleware': False} + + with self.config_change(default_config=set_default, + alternate_config=set_alternate, + application_name=self.application_name): + api_paste_content = self.fetch_api_paste_content() + self.assertNotIn(section_heading, api_paste_content) + + +class IronicAuditMiddlewareTest(KeystoneAuditMiddlewareTest): + """Ironic-API audit middleware test class.""" + + def test_101_apipaste_includes_audit_section(self): + """Test api-paste.ini renders audit section when enabled.""" + self.skipTest('ironic-api does not use an api-paste.ini file') + + def test_102_apipaste_excludes_audit_section(self): + """Test api_paste.ini does not render audit section when disabled.""" + self.skipTest('ironic-api does not use an api-paste.ini file') diff --git a/zaza/openstack/charm_tests/cinder_backup/tests.py b/zaza/openstack/charm_tests/cinder_backup/tests.py index 97b365808..258c7bb77 100644 --- a/zaza/openstack/charm_tests/cinder_backup/tests.py +++ b/zaza/openstack/charm_tests/cinder_backup/tests.py @@ -22,6 +22,7 @@ import zaza.model import zaza.openstack.charm_tests.test_utils as test_utils +from zaza.openstack.utilities import retry_on_connect_failure import zaza.openstack.utilities.ceph as ceph_utils import zaza.openstack.utilities.openstack as openstack_utils @@ -35,8 +36,9 @@ class CinderBackupTest(test_utils.OpenStackBaseTest): def setUpClass(cls): """Run class setup for running Cinder Backup tests.""" super(CinderBackupTest, cls).setUpClass() - cls.cinder_client = openstack_utils.get_cinder_session_client( - cls.keystone_session) + cls.cinder_client = retry_on_connect_failure( + openstack_utils.get_cinder_session_client(cls.keystone_session), + log=logging.warn) @property def services(self): @@ -101,7 +103,7 @@ def test_410_cinder_vol_create_backup_delete_restore_pool_inspect(self): self.cinder_client.volumes, cinder_vol.id, wait_iteration_max_time=180, - stop_after_attempt=15, + stop_after_attempt=30, expected_status='available', msg='ceph-backed cinder volume') @@ -123,7 +125,7 @@ def test_410_cinder_vol_create_backup_delete_restore_pool_inspect(self): self.cinder_client.backups, vol_backup.id, wait_iteration_max_time=180, - stop_after_attempt=15, + stop_after_attempt=30, expected_status='available', msg='Backup volume') diff --git a/zaza/openstack/charm_tests/designate/tests.py b/zaza/openstack/charm_tests/designate/tests.py index 73a434c4e..ed68a8c93 100644 --- a/zaza/openstack/charm_tests/designate/tests.py +++ b/zaza/openstack/charm_tests/designate/tests.py @@ -134,6 +134,12 @@ def wait(): def test_300_default_soa_config_options(self): """Configure default SOA options.""" + current_release = openstack_utils.get_os_release() + jammy_antelope = openstack_utils.get_os_release('jammy_antelope') + if current_release > jammy_antelope: + self.skipTest('changing default ttl is currently broken since ' + 'jammy_bobcat due to LP#2042944') + test_domain = "test_300_example.com." DEFAULT_TTL = 60 alternate_config = {'default-soa-minimum': 600, diff --git a/zaza/openstack/charm_tests/manila/tests.py b/zaza/openstack/charm_tests/manila/tests.py index ea4073891..3a3f54c00 100644 --- a/zaza/openstack/charm_tests/manila/tests.py +++ b/zaza/openstack/charm_tests/manila/tests.py @@ -359,7 +359,7 @@ def test_manila_share(self): self.manila_client.shares, share.id, wait_iteration_max_time=120, - stop_after_attempt=2, + stop_after_attempt=10, expected_status="available", msg="Waiting for a share to become available") diff --git a/zaza/openstack/charm_tests/mysql/tests.py b/zaza/openstack/charm_tests/mysql/tests.py index 710195541..7c574a919 100644 --- a/zaza/openstack/charm_tests/mysql/tests.py +++ b/zaza/openstack/charm_tests/mysql/tests.py @@ -1145,3 +1145,66 @@ def test_910_restart_on_config_change(self): {}, {}, self.services) logging.info("Passed restart on changed test.") + + def test_920_mysqlrouter_conf_broken(self): + """Checking conf broken case. + + Run the bootstrap when conf is broken + """ + # application_name on test is keystone-mysql-router + # using self.conf_file introduces error. + # instead of changing current self.conf_file, + # define one (it could introduce another issue) + config_file = ( + "/var/lib/mysql/{}/mysqlrouter.conf" + .format(self.application_name)) + + logging.info("Starting broken conf test") + + # put empty string to conf_file and make it wrong + logging.info("Breaking configuration file") + zaza.model.run_on_leader(self.application, + "echo '[DEFAULT]\n \ + [metadata_cache:[\\w$]+$] \ + ' > {}".format( + config_file)) + + logging.info("Getting configuration file") + recovered = zaza.model.run_on_leader(self.application, + "cat {}".format( + config_file))['Stdout'] + + # Checking conf file length, + # if file is broken it is around 250 + assert len(recovered) < 1000, ( + "Breaking mysqlrouter conf failed.") + + # verify it is in error state + for attempt in tenacity.Retrying( + reraise=True, + wait=tenacity.wait_fixed(10), + stop=tenacity.stop_after_attempt(30), + ): + with attempt: + # update status to make the status error + logging.info("Run update-status") + self.run_update_status_hooks(['keystone-mysql-router/0']) + + # get current status + unit_status = (zaza.model.get_status() + .applications + ['keystone-mysql-router']['status']) + logging.info("Status:{}".format(unit_status['status'])) + self.assertEqual(unit_status['status'], "active") + + logging.info("Getting configuration file") + recovered = zaza.model.run_on_leader(self.application, + "cat {}".format( + config_file))['Stdout'] + + # Checking conf file length, + # if file is broken it is around 250 + assert len(recovered) > 1000, ( + "Fixing mysqlrouter conf failed.") + + logging.info("Passed broken conf test.") diff --git a/zaza/openstack/charm_tests/nova/tests.py b/zaza/openstack/charm_tests/nova/tests.py index 34f02731c..2c0286e4c 100644 --- a/zaza/openstack/charm_tests/nova/tests.py +++ b/zaza/openstack/charm_tests/nova/tests.py @@ -19,6 +19,7 @@ import json import logging import os +import re import tempfile import tenacity import unittest @@ -470,6 +471,63 @@ def test_901_pause_resume(self): with self.pause_resume(['nova-compute']): logging.info("Testing pause resume") + def test_904_test_ceph_keys(self): + """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 = {} + + # The new and correct behavior is to have + # "nova-compute-ceph-auth-" named keyring + # and one other named after the charm app. Example: + # for a charm app named "nova-compute-kvm", + # it should have both nova-compute-kvm and + # nova-compute-ceph-auth- keyrings. + # For a charm app named "nova-compute", + # it should have both nova-compute and + # nova-compute-ceph-auth- keyrings. + + # Previous behaviors: + # The old behavior is to have only 1 keyring named after the charm app. + + def check_keyring(key_name): + """Check matching keyring name and different from existing ones.""" + keyring_file = ( + '/etc/ceph/ceph.client.{}.keyring'.format(key_name)) + data = str(generic_utils.get_file_contents( + unit, keyring_file)) + + result = regex.findall(data)[0] + + # Assert keyring file name matches intended name + self.assertEqual(2, len(result)) + self.assertEqual(result[0], key_name) + + # Confirm the keys are different from each other and the + # same across all units + 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] + + for unit in zaza.model.get_units( + self.application_name, model_name=self.model_name): + + # old key + check_keyring(self.application_name) + # new key + check_keyring('nova-compute-ceph-auth-c91ce26f') + def test_930_check_virsh_default_network(self): """Test default virt network is not present.""" for unit in zaza.model.get_units('nova-compute', diff --git a/zaza/openstack/charm_tests/swift/tests.py b/zaza/openstack/charm_tests/swift/tests.py index 605b275c1..93bcc9006 100644 --- a/zaza/openstack/charm_tests/swift/tests.py +++ b/zaza/openstack/charm_tests/swift/tests.py @@ -132,9 +132,15 @@ def test_904_set_weight_action_and_validate_rebalance(self): zaza.model.wait_for_agent_status() zaza.model.block_until_all_units_idle() - self.assertTrue( - swift_utils.is_ring_synced('swift-proxy', 'object', - self.TEST_EXPECTED_RING_HOSTS)) + for attempt in tenacity.Retrying( + wait=tenacity.wait_fixed(2), + retry=tenacity.retry_if_exception_type(AssertionError), + reraise=True, + stop=tenacity.stop_after_attempt(10)): + with attempt: + self.assertTrue( + swift_utils.is_ring_synced('swift-proxy', 'object', + self.TEST_EXPECTED_RING_HOSTS)) def test_905_remove_device_action_and_validate_rebalance(self): """Remove device from object ring.""" diff --git a/zaza/openstack/utilities/__init__.py b/zaza/openstack/utilities/__init__.py index 5798f2650..02826ff7f 100644 --- a/zaza/openstack/utilities/__init__.py +++ b/zaza/openstack/utilities/__init__.py @@ -15,10 +15,30 @@ """Collection of utilities to support zaza tests etc.""" +import logging import time from keystoneauth1.exceptions.connection import ConnectFailure +NEVER_RETRY_EXCEPTIONS = ( + AssertionError, + AttributeError, + ImportError, + IndexError, + KeyError, + NotImplementedError, + OverflowError, + RecursionError, + ReferenceError, + RuntimeError, + SyntaxError, + IndentationError, + SystemExit, + TypeError, + UnicodeError, + ZeroDivisionError, +) + class ObjectRetrierWraps(object): """An automatic retrier for an object. @@ -76,10 +96,19 @@ def __init__(self, obj, num_retries=3, initial_interval=5.0, backoff=1.0, If a list, then it will only retry if the exception is one of the ones in the list. :type retry_exceptions: List[Exception] + :param log: If False, disable logging; if None (the default) or True, + use logging.warn; otherwise use the passed param `log`. + :type param: None | Boolean | Callable """ # Note we use semi-private variable names that shouldn't clash with any # on the actual object. self.__obj = obj + if log in (None, True): + _log = logging.warning + elif log is False: + _log = lambda *_, **__: None # noqa + else: + _log = log self.__kwargs = { 'num_retries': num_retries, 'initial_interval': initial_interval, @@ -87,24 +116,19 @@ def __init__(self, obj, num_retries=3, initial_interval=5.0, backoff=1.0, 'max_interval': max_interval, 'total_wait': total_wait, 'retry_exceptions': retry_exceptions, - 'log': log or (lambda x: None), + 'log': _log, } + _log(f"ObjectRetrierWraps: wrapping {self.__obj}") def __getattr__(self, name): """Get attribute; delegates to wrapped object.""" - # Note the above may generate an attribute error; we expect this and - # will fail with an attribute error. - attr = getattr(self.__obj, name) - if callable(attr) or hasattr(attr, "__getattr__"): + obj = self.__obj + attr = getattr(obj, name) + if callable(attr): return ObjectRetrierWraps(attr, **self.__kwargs) - else: + if attr.__class__.__module__ == 'builtins': return attr - # TODO(ajkavanagh): Note detecting a property is a bit trickier. we - # can do isinstance(attr, property), but then the act of accessing it - # is what calls it. i.e. it would fail at the getattr(self.__obj, - # name) stage. The solution is to check first, and if it's a property, - # then treat it like the retrier. However, I think this is too - # complex for the first go, and to use manual retries in that instance. + return ObjectRetrierWraps(attr, **self.__kwargs) def __call__(self, *args, **kwargs): """Call the object; delegates to the wrapped object.""" @@ -126,16 +150,20 @@ def __call__(self, *args, **kwargs): # is not in the list of retries, then raise an exception # immediately. This means that if retry_exceptions is None, # then the method is always retried. + if isinstance(e, NEVER_RETRY_EXCEPTIONS): + log("ObjectRetrierWraps: error {} is never caught; raising" + .format(str(e))) + raise if (retry_exceptions is not None and type(e) not in retry_exceptions): raise retry += 1 if retry > num_retries: - log("{}: exceeded number of retries, so erroring out" - .format(str(obj))) + log("ObjectRetrierWraps: exceeded number of retries, " + "so erroring out") raise e - log("{}: call failed: retrying in {} seconds" - .format(str(obj), wait)) + log("ObjectRetrierWraps: call failed: retrying in {} " + "seconds" .format(wait)) time.sleep(wait) wait_so_far += wait if wait_so_far >= total_wait: diff --git a/zaza/openstack/utilities/openstack.py b/zaza/openstack/utilities/openstack.py index e548884d3..e3032d2c2 100644 --- a/zaza/openstack/utilities/openstack.py +++ b/zaza/openstack/utilities/openstack.py @@ -85,6 +85,7 @@ from zaza.openstack.utilities import ( exceptions, generic as generic_utils, + ObjectRetrierWraps, ) import zaza.utilities.networking as network_utils @@ -379,7 +380,8 @@ def get_nova_session_client(session, version=None): """ if not version: version = 2 - return novaclient_client.Client(version, session=session) + return ObjectRetrierWraps( + novaclient_client.Client(version, session=session)) def get_neutron_session_client(session):