From a15832695ca4c5db5cd2fcb858a527f93d882277 Mon Sep 17 00:00:00 2001 From: Luciano Lo Giudice Date: Mon, 22 Apr 2024 14:43:46 -0300 Subject: [PATCH 1/5] Implement key rotation tests for OSD units This PR adds OSD tests to the key rotation class for the ceph-mon charm. It will be tested in the following merge requests: https://review.opendev.org/c/openstack/charm-ceph-mon/+/915925 https://review.opendev.org/c/openstack/charm-ceph-osd/+/915926 --- zaza/openstack/charm_tests/ceph/tests.py | 1 + 1 file changed, 1 insertion(+) diff --git a/zaza/openstack/charm_tests/ceph/tests.py b/zaza/openstack/charm_tests/ceph/tests.py index 85de53dcf..10294d6ac 100644 --- a/zaza/openstack/charm_tests/ceph/tests.py +++ b/zaza/openstack/charm_tests/ceph/tests.py @@ -1802,6 +1802,7 @@ def test_key_rotate(self): """Test that rotating the keys actually changes them.""" unit = 'ceph-mon/0' self._check_key_rotation('mgr', unit) + self._check_key_rotation('osd.0', unit) try: zaza_model.get_application('ceph-radosgw') From 3cf2ed6d1519b450ca02c42b55383b0a31821e74 Mon Sep 17 00:00:00 2001 From: Luciano Lo Giudice Date: Mon, 6 May 2024 12:24:02 -0300 Subject: [PATCH 2/5] Retry on key rotation --- zaza/openstack/charm_tests/ceph/tests.py | 21 ++++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) diff --git a/zaza/openstack/charm_tests/ceph/tests.py b/zaza/openstack/charm_tests/ceph/tests.py index 4762d5eb3..b4bf1dc85 100644 --- a/zaza/openstack/charm_tests/ceph/tests.py +++ b/zaza/openstack/charm_tests/ceph/tests.py @@ -1795,9 +1795,24 @@ def entity_filter(name): action_params={'entity': entity} ) zaza_utils.assertActionRanOK(action_obj) - zaza_model.wait_for_application_states(states=self.app_states) - new_keys = self._get_all_keys(unit, entity_filter) - self.assertNotEqual(old_keys, new_keys) + # NOTE(lmlg): There's a nasty race going on here. Essentially, + # since this action involves 2 different applications, what + # happens is as follows: + # (1) (2) (3) (4) + # ceph-mon rotates key | (idle) | remote-unit rotates key | (idle) + # Between (2) and (3), there's a window where all units are + # idle, _but_ the key hasn't been rotated in the other unit. + # As such, we retry the checks a couple of times instead of + # using the `wait_for_application_states` interface. + + for attempt in tenacity.Retrying( + wait=tenacity.wait_exponential(multiplier=2, max=32), + reraise=True, stop=tenacity.stop_after_attempt(10), + retry=tenacity.retry_if_exception_type(AssertionError) + ): + new_keys = self._get_all_keys(unit, entity_filter) + self.assertNotEqual(old_keys, new_keys) + diff = new_keys - old_keys self.assertEqual(len(diff), 1) first = next(iter(diff)) From 9d931d373e670c90231724444a5a07150f31499a Mon Sep 17 00:00:00 2001 From: Luciano Lo Giudice Date: Mon, 6 May 2024 15:09:31 -0300 Subject: [PATCH 3/5] Sleep instead of retrying --- zaza/openstack/charm_tests/ceph/tests.py | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/zaza/openstack/charm_tests/ceph/tests.py b/zaza/openstack/charm_tests/ceph/tests.py index b4bf1dc85..66940b154 100644 --- a/zaza/openstack/charm_tests/ceph/tests.py +++ b/zaza/openstack/charm_tests/ceph/tests.py @@ -28,6 +28,7 @@ import urllib3 import tenacity +import time import zaza.charm_lifecycle.utils as lifecycle_utils import zaza.openstack.charm_tests.test_utils as test_utils @@ -1802,16 +1803,12 @@ def entity_filter(name): # ceph-mon rotates key | (idle) | remote-unit rotates key | (idle) # Between (2) and (3), there's a window where all units are # idle, _but_ the key hasn't been rotated in the other unit. - # As such, we retry the checks a couple of times instead of - # using the `wait_for_application_states` interface. + # As such, we sleep for a short time instead of using the + # `wait_for_application_states` interface. - for attempt in tenacity.Retrying( - wait=tenacity.wait_exponential(multiplier=2, max=32), - reraise=True, stop=tenacity.stop_after_attempt(10), - retry=tenacity.retry_if_exception_type(AssertionError) - ): - new_keys = self._get_all_keys(unit, entity_filter) - self.assertNotEqual(old_keys, new_keys) + time.sleep(10) + new_keys = self._get_all_keys(unit, entity_filter) + self.assertNotEqual(old_keys, new_keys) diff = new_keys - old_keys self.assertEqual(len(diff), 1) From 817534af7ece20b7974a0fd30b151d3ace345015 Mon Sep 17 00:00:00 2001 From: Luciano Lo Giudice Date: Mon, 6 May 2024 18:00:12 -0300 Subject: [PATCH 4/5] Fix tenacity usage --- zaza/openstack/charm_tests/ceph/tests.py | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/zaza/openstack/charm_tests/ceph/tests.py b/zaza/openstack/charm_tests/ceph/tests.py index 66940b154..7cc4aa846 100644 --- a/zaza/openstack/charm_tests/ceph/tests.py +++ b/zaza/openstack/charm_tests/ceph/tests.py @@ -28,7 +28,6 @@ import urllib3 import tenacity -import time import zaza.charm_lifecycle.utils as lifecycle_utils import zaza.openstack.charm_tests.test_utils as test_utils @@ -1803,12 +1802,17 @@ def entity_filter(name): # ceph-mon rotates key | (idle) | remote-unit rotates key | (idle) # Between (2) and (3), there's a window where all units are # idle, _but_ the key hasn't been rotated in the other unit. - # As such, we sleep for a short time instead of using the + # As such, we retry a few times instead of using the # `wait_for_application_states` interface. - time.sleep(10) - new_keys = self._get_all_keys(unit, entity_filter) - self.assertNotEqual(old_keys, new_keys) + for attempt in tenacity.Retrying( + wait=tenacity.wait_exponential(multiplier=2, max=32), + reraise=True, stop=tenacity.stop_after_attempt(10), + retry=tenacity.retry_if_exception_type(AssertionError) + ): + with attempt: + new_keys = self._get_all_keys(unit, entity_filter) + self.assertNotEqual(old_keys, new_keys) diff = new_keys - old_keys self.assertEqual(len(diff), 1) @@ -1834,7 +1838,6 @@ def _get_fs_client(self, unit): def test_key_rotate(self): """Test that rotating the keys actually changes them.""" unit = 'ceph-mon/0' - self._check_key_rotation('mgr', unit) self._check_key_rotation('osd.0', unit) try: From 239d028fdcc98e8e571089a9f24ec06d3e038fd4 Mon Sep 17 00:00:00 2001 From: Luciano Lo Giudice Date: Mon, 6 May 2024 20:26:21 -0300 Subject: [PATCH 5/5] Increase retries --- zaza/openstack/charm_tests/ceph/tests.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/zaza/openstack/charm_tests/ceph/tests.py b/zaza/openstack/charm_tests/ceph/tests.py index 7cc4aa846..58719c216 100644 --- a/zaza/openstack/charm_tests/ceph/tests.py +++ b/zaza/openstack/charm_tests/ceph/tests.py @@ -1807,7 +1807,7 @@ def entity_filter(name): for attempt in tenacity.Retrying( wait=tenacity.wait_exponential(multiplier=2, max=32), - reraise=True, stop=tenacity.stop_after_attempt(10), + reraise=True, stop=tenacity.stop_after_attempt(20), retry=tenacity.retry_if_exception_type(AssertionError) ): with attempt: