From 219e485792525cd9146c14e91a8f78b0fde02235 Mon Sep 17 00:00:00 2001 From: jfwood Date: Mon, 1 Jun 2015 21:09:27 -0500 Subject: [PATCH] Fix cert tasks not being scheduled for retry Certificate tasks that are waiting for a CA to generate certificates were not being retried. This CR fixes that bug, introduced as part of properly handling exceptions raised during the worker service processes. This CR also beefs up unit testing around this issue so that this issue is not re-introduced later. Change-Id: I0a506c67be7881cf8c781c534f332f1f8122161e Closes-Bug: #1460873 --- barbican/tasks/resources.py | 2 +- barbican/tests/queue/test_server.py | 31 +++++++++++++++++++------- barbican/tests/tasks/test_resources.py | 14 ++++++++---- 3 files changed, 34 insertions(+), 13 deletions(-) diff --git a/barbican/tasks/resources.py b/barbican/tasks/resources.py index 7d8e69dc9..d6d4260d8 100644 --- a/barbican/tasks/resources.py +++ b/barbican/tasks/resources.py @@ -58,7 +58,7 @@ class BaseTask(object): https://gist.github.com/jfwood/a8130265b0db3c793ec8 """ try: - self.process(*args, **kwargs) + return self.process(*args, **kwargs) except Exception: LOG.exception( u._LE( diff --git a/barbican/tests/queue/test_server.py b/barbican/tests/queue/test_server.py index d1f74f856..502aac1cd 100644 --- a/barbican/tests/queue/test_server.py +++ b/barbican/tests/queue/test_server.py @@ -287,9 +287,11 @@ class WhenCallingTasksMethod(utils.BaseTestCase): super(WhenCallingTasksMethod, self).tearDown() self.is_server_side_patcher.stop() + @mock.patch('barbican.queue.server.schedule_order_retry_tasks') @mock.patch('barbican.tasks.resources.BeginTypeOrder') - def test_should_process_begin_order(self, mock_begin_order): - mock_begin_order.return_value.process.return_value = None + def test_should_process_begin_order(self, mock_begin_order, mock_schedule): + method = mock_begin_order.return_value.process_and_suppress_exceptions + method.return_value = 'result' self.tasks.process_type_order( None, self.order_id, self.external_project_id) @@ -297,11 +299,16 @@ class WhenCallingTasksMethod(utils.BaseTestCase): mock_process = mock_begin_order.return_value mock_process.process_and_suppress_exceptions.assert_called_with( self.order_id, self.external_project_id) + mock_schedule.assert_called_with( + mock.ANY, 'result', None, 'order1234', 'keystone1234') + @mock.patch('barbican.queue.server.schedule_order_retry_tasks') @mock.patch('barbican.tasks.resources.UpdateOrder') - def test_should_process_update_order(self, mock_update_order): - mock_update_order.return_value.process.return_value = None - updated_meta = {} + def test_should_process_update_order( + self, mock_update_order, mock_schedule): + method = mock_update_order.return_value.process_and_suppress_exceptions + method.return_value = 'result' + updated_meta = {'foo': 1} self.tasks.update_order( None, self.order_id, self.external_project_id, updated_meta) @@ -310,18 +317,26 @@ class WhenCallingTasksMethod(utils.BaseTestCase): mock_process.process_and_suppress_exceptions.assert_called_with( self.order_id, self.external_project_id, updated_meta ) + mock_schedule.assert_called_with( + mock.ANY, 'result', None, + 'order1234', 'keystone1234', updated_meta) + @mock.patch('barbican.queue.server.schedule_order_retry_tasks') @mock.patch('barbican.tasks.resources.CheckCertificateStatusOrder') - def test_should_check_certificate_order(self, mock_check_cert_order): - mock_check_cert_order.return_value.process.return_value = None + def test_should_check_certificate_order( + self, mock_check_cert, mock_schedule): + method = mock_check_cert.return_value.process_and_suppress_exceptions + method.return_value = 'result' self.tasks.check_certificate_status( None, self.order_id, self.external_project_id) - mock_process = mock_check_cert_order.return_value + mock_process = mock_check_cert.return_value mock_process.process_and_suppress_exceptions.assert_called_with( self.order_id, self.external_project_id ) + mock_schedule.assert_called_with( + mock.ANY, 'result', None, 'order1234', 'keystone1234') @mock.patch('barbican.tasks.resources.BeginTypeOrder') def test_process_order_catch_exception(self, mock_begin_order): diff --git a/barbican/tests/tasks/test_resources.py b/barbican/tests/tasks/test_resources.py index 60d7fe497..f4a5cca7f 100644 --- a/barbican/tests/tasks/test_resources.py +++ b/barbican/tests/tasks/test_resources.py @@ -272,7 +272,8 @@ class WhenBeginningCertificateTypeOrder(BaseOrderTestCase): self, mock_issue_cert_request): mock_issue_cert_request.return_value = None - self.resource.process(self.order.id, self.external_project_id) + result = self.resource.process_and_suppress_exceptions( + self.order.id, self.external_project_id) self.order_repo.get.assert_called_once_with( entity_id=self.order.id, @@ -286,6 +287,7 @@ class WhenBeginningCertificateTypeOrder(BaseOrderTestCase): mock.ANY ) self.assertIsNone(self.order.container_id) + self.assertIsInstance(result, common.FollowOnProcessingStatusDTO) @mock.patch( 'barbican.tasks.certificate_resources.issue_certificate_request') @@ -293,7 +295,8 @@ class WhenBeginningCertificateTypeOrder(BaseOrderTestCase): self, mock_issue_cert_request): mock_issue_cert_request.return_value = self.container - self.resource.process(self.order.id, self.external_project_id) + result = self.resource.process( + self.order.id, self.external_project_id) self.order_repo.get.assert_called_once_with( entity_id=self.order.id, @@ -307,6 +310,7 @@ class WhenBeginningCertificateTypeOrder(BaseOrderTestCase): mock.ANY ) self.assertEqual(self.container.id, self.order.container_id) + self.assertIsInstance(result, common.FollowOnProcessingStatusDTO) class WhenUpdatingOrder(BaseOrderTestCase): @@ -323,7 +327,7 @@ class WhenUpdatingOrder(BaseOrderTestCase): def test_should_update_certificate_order(self, mock_modify_cert_request): self.order.type = models.OrderType.CERTIFICATE - self.resource.process( + self.resource.process_and_suppress_exceptions( self.order.id, self.external_project_id, self.updated_meta) self.assertEqual(self.order.status, models.States.ACTIVE) @@ -474,7 +478,8 @@ class WhenCheckingCertificateStatus(BaseOrderTestCase): self, mock_check_cert_request): mock_check_cert_request.return_value = None - self.resource.process(self.order.id, self.external_project_id) + result = self.resource.process_and_suppress_exceptions( + self.order.id, self.external_project_id) self.order_repo.get.assert_called_once_with( entity_id=self.order.id, @@ -488,6 +493,7 @@ class WhenCheckingCertificateStatus(BaseOrderTestCase): mock.ANY ) self.assertIsNone(self.order.container_id) + self.assertIsInstance(result, common.FollowOnProcessingStatusDTO) @mock.patch( 'barbican.tasks.certificate_resources.check_certificate_request')