From 908f0b4a3b059583db62790099b683558fea31b7 Mon Sep 17 00:00:00 2001 From: Liam Young Date: Tue, 14 Dec 2021 08:06:58 +0000 Subject: [PATCH 1/4] Search for message in rabbit tests The current rabbit tests post a message on one unit and then consumes the first message on another unit. If the two do not match then the test fails. This means that a single pre-existing message can break all these tests as the consumer always gets the wrong message. This change updates the tests to search for the target message rather than assuming that the first message is the target message. Messages that are not the target message are not reposted so any pre-existing messages are consumed and in effect thrown away but given these are all test messages in a test queue and the tests do not run in parallel this should be fine. --- .../charm_tests/rabbitmq_server/tests.py | 59 ++++++++++++++----- 1 file changed, 45 insertions(+), 14 deletions(-) diff --git a/zaza/openstack/charm_tests/rabbitmq_server/tests.py b/zaza/openstack/charm_tests/rabbitmq_server/tests.py index 6ee156e..9901270 100644 --- a/zaza/openstack/charm_tests/rabbitmq_server/tests.py +++ b/zaza/openstack/charm_tests/rabbitmq_server/tests.py @@ -57,6 +57,41 @@ class RmqTests(test_utils.OpenStackBaseTest): ssl=ssl, port=port) + @tenacity.retry(reraise=True, stop=tenacity.stop_after_attempt(30), + retry=tenacity.retry_if_exception_type(AssertionError)) + def _search_for_message(self, amqp_msg, check_unit, ssl, port, + amqp_msg_counter): + """Search for message in message queue. + + WARNING: This will consume messages until it finds the target message. + + :param amqp_msg: Message to search for + :type amqp_msg: string + :param check_unit: Unit to retrieve messages from + :type check_unit: juju.unit.Unit + :param ssl: Whether to use SSL when connecting to rabbit + :type ssl: bool + :param port: Port to use when connecting to rabbit + :type port: Union[int, None] + :param amqp_msg_counter: Number in test sequence of this message. + :type amqp_msg: int + :raises: tenacity.RetryError + """ + amqp_msg_rcvd = self._retry_get_amqp_message( + check_unit, + ssl=ssl, + port=port) + + try: + logging.info('Looking for message {}'.format(amqp_msg)) + # Validate amqp message content + assert amqp_msg == amqp_msg_rcvd + logging.info('Message {} received OK.'.format(amqp_msg_counter)) + except AssertionError as err: + logging.info('Expected: {}'.format(amqp_msg)) + logging.info('Actual: {}'.format(amqp_msg_rcvd)) + raise err + def _test_rmq_amqp_messages_all_units(self, units, ssl=False, port=None): """Reusable test to send/check amqp messages to every listed rmq unit. @@ -111,25 +146,21 @@ class RmqTests(test_utils.OpenStackBaseTest): port=port) # Get amqp message - logging.info('Get message from: {} ' + logging.info('Get messages from: {} ' '({} {})'.format(check_unit_host, check_unit_name, check_unit_host_name)) - amqp_msg_rcvd = self._retry_get_amqp_message(check_unit, - ssl=ssl, - port=port) - - # Validate amqp message content - if amqp_msg == amqp_msg_rcvd: - logging.info('Message {} received ' - 'OK.'.format(amqp_msg_counter)) - else: - logging.error('Expected: {}'.format(amqp_msg)) - logging.error('Actual: {}'.format(amqp_msg_rcvd)) - msg = 'Message {} mismatch.'.format(amqp_msg_counter) + try: + self._search_for_message( + amqp_msg, + check_unit, + ssl, + port, + amqp_msg_counter) + except tenacity.RetryError: + msg = 'Message {} not found.'.format(amqp_msg_counter) raise Exception(msg) - amqp_msg_counter += 1 # Delete the test user From 87c72a510e88280c7ccf43e12c5326e7c178b566 Mon Sep 17 00:00:00 2001 From: Liam Young Date: Tue, 14 Dec 2021 09:02:16 +0000 Subject: [PATCH 2/4] Fix bug when no message is present --- zaza/openstack/charm_tests/rabbitmq_server/utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/zaza/openstack/charm_tests/rabbitmq_server/utils.py b/zaza/openstack/charm_tests/rabbitmq_server/utils.py index d00f288..503e80b 100644 --- a/zaza/openstack/charm_tests/rabbitmq_server/utils.py +++ b/zaza/openstack/charm_tests/rabbitmq_server/utils.py @@ -506,9 +506,9 @@ def get_amqp_message_by_unit(unit, queue="test", password=password) channel = connection.channel() method_frame, _, body = channel.basic_get(queue) - body = body.decode() if method_frame: + body = body.decode() logging.debug('Retreived message from {} queue:\n{}'.format(queue, body)) channel.basic_ack(method_frame.delivery_tag) From ce18b4a2a0f9a7dab002ea77e7f78bc26c01b6d6 Mon Sep 17 00:00:00 2001 From: Liam Young Date: Tue, 14 Dec 2021 11:09:41 +0000 Subject: [PATCH 3/4] Fix reraising for RmqNoMessageException --- zaza/openstack/charm_tests/rabbitmq_server/tests.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/zaza/openstack/charm_tests/rabbitmq_server/tests.py b/zaza/openstack/charm_tests/rabbitmq_server/tests.py index 9901270..b35817f 100644 --- a/zaza/openstack/charm_tests/rabbitmq_server/tests.py +++ b/zaza/openstack/charm_tests/rabbitmq_server/tests.py @@ -49,6 +49,7 @@ class RmqTests(test_utils.OpenStackBaseTest): return '[{}-{}]'.format(uuid.uuid4(), time.time()) @tenacity.retry( + reraise=True, retry=tenacity.retry_if_exception_type(RmqNoMessageException), wait=tenacity.wait_fixed(10), stop=tenacity.stop_after_attempt(2)) @@ -75,7 +76,7 @@ class RmqTests(test_utils.OpenStackBaseTest): :type port: Union[int, None] :param amqp_msg_counter: Number in test sequence of this message. :type amqp_msg: int - :raises: tenacity.RetryError + :raises: RmqNoMessageException """ amqp_msg_rcvd = self._retry_get_amqp_message( check_unit, @@ -158,7 +159,7 @@ class RmqTests(test_utils.OpenStackBaseTest): ssl, port, amqp_msg_counter) - except tenacity.RetryError: + except RmqNoMessageException: msg = 'Message {} not found.'.format(amqp_msg_counter) raise Exception(msg) amqp_msg_counter += 1 From e83591033babf602f3aa62ad0359d2c84f8d68ed Mon Sep 17 00:00:00 2001 From: Liam Young Date: Tue, 14 Dec 2021 11:53:06 +0000 Subject: [PATCH 4/4] Remove tenacity around message search --- .../charm_tests/rabbitmq_server/tests.py | 34 +++++++++---------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/zaza/openstack/charm_tests/rabbitmq_server/tests.py b/zaza/openstack/charm_tests/rabbitmq_server/tests.py index b35817f..22bdc2f 100644 --- a/zaza/openstack/charm_tests/rabbitmq_server/tests.py +++ b/zaza/openstack/charm_tests/rabbitmq_server/tests.py @@ -58,8 +58,6 @@ class RmqTests(test_utils.OpenStackBaseTest): ssl=ssl, port=port) - @tenacity.retry(reraise=True, stop=tenacity.stop_after_attempt(30), - retry=tenacity.retry_if_exception_type(AssertionError)) def _search_for_message(self, amqp_msg, check_unit, ssl, port, amqp_msg_counter): """Search for message in message queue. @@ -78,20 +76,21 @@ class RmqTests(test_utils.OpenStackBaseTest): :type amqp_msg: int :raises: RmqNoMessageException """ - amqp_msg_rcvd = self._retry_get_amqp_message( - check_unit, - ssl=ssl, - port=port) - - try: - logging.info('Looking for message {}'.format(amqp_msg)) - # Validate amqp message content - assert amqp_msg == amqp_msg_rcvd - logging.info('Message {} received OK.'.format(amqp_msg_counter)) - except AssertionError as err: - logging.info('Expected: {}'.format(amqp_msg)) - logging.info('Actual: {}'.format(amqp_msg_rcvd)) - raise err + for i in range(100): + amqp_msg_rcvd = self._retry_get_amqp_message( + check_unit, + ssl=ssl, + port=port) + if amqp_msg == amqp_msg_rcvd: + logging.info( + 'Message {} received OK.'.format(amqp_msg_counter)) + break + else: + logging.info('Expected: {}'.format(amqp_msg)) + logging.info('Actual: {}'.format(amqp_msg_rcvd)) + else: + msg = 'Message {} not found.'.format(amqp_msg_counter) + raise RmqNoMessageException(msg) def _test_rmq_amqp_messages_all_units(self, units, ssl=False, port=None): @@ -160,7 +159,8 @@ class RmqTests(test_utils.OpenStackBaseTest): port, amqp_msg_counter) except RmqNoMessageException: - msg = 'Message {} not found.'.format(amqp_msg_counter) + msg = 'Failed to retrieve message {}.'.format( + amqp_msg_counter) raise Exception(msg) amqp_msg_counter += 1