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
This commit is contained in:
@@ -58,7 +58,7 @@ class BaseTask(object):
|
|||||||
https://gist.github.com/jfwood/a8130265b0db3c793ec8
|
https://gist.github.com/jfwood/a8130265b0db3c793ec8
|
||||||
"""
|
"""
|
||||||
try:
|
try:
|
||||||
self.process(*args, **kwargs)
|
return self.process(*args, **kwargs)
|
||||||
except Exception:
|
except Exception:
|
||||||
LOG.exception(
|
LOG.exception(
|
||||||
u._LE(
|
u._LE(
|
||||||
|
|||||||
@@ -287,9 +287,11 @@ class WhenCallingTasksMethod(utils.BaseTestCase):
|
|||||||
super(WhenCallingTasksMethod, self).tearDown()
|
super(WhenCallingTasksMethod, self).tearDown()
|
||||||
self.is_server_side_patcher.stop()
|
self.is_server_side_patcher.stop()
|
||||||
|
|
||||||
|
@mock.patch('barbican.queue.server.schedule_order_retry_tasks')
|
||||||
@mock.patch('barbican.tasks.resources.BeginTypeOrder')
|
@mock.patch('barbican.tasks.resources.BeginTypeOrder')
|
||||||
def test_should_process_begin_order(self, mock_begin_order):
|
def test_should_process_begin_order(self, mock_begin_order, mock_schedule):
|
||||||
mock_begin_order.return_value.process.return_value = None
|
method = mock_begin_order.return_value.process_and_suppress_exceptions
|
||||||
|
method.return_value = 'result'
|
||||||
|
|
||||||
self.tasks.process_type_order(
|
self.tasks.process_type_order(
|
||||||
None, self.order_id, self.external_project_id)
|
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 = mock_begin_order.return_value
|
||||||
mock_process.process_and_suppress_exceptions.assert_called_with(
|
mock_process.process_and_suppress_exceptions.assert_called_with(
|
||||||
self.order_id, self.external_project_id)
|
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')
|
@mock.patch('barbican.tasks.resources.UpdateOrder')
|
||||||
def test_should_process_update_order(self, mock_update_order):
|
def test_should_process_update_order(
|
||||||
mock_update_order.return_value.process.return_value = None
|
self, mock_update_order, mock_schedule):
|
||||||
updated_meta = {}
|
method = mock_update_order.return_value.process_and_suppress_exceptions
|
||||||
|
method.return_value = 'result'
|
||||||
|
updated_meta = {'foo': 1}
|
||||||
|
|
||||||
self.tasks.update_order(
|
self.tasks.update_order(
|
||||||
None, self.order_id, self.external_project_id, updated_meta)
|
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(
|
mock_process.process_and_suppress_exceptions.assert_called_with(
|
||||||
self.order_id, self.external_project_id, updated_meta
|
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')
|
@mock.patch('barbican.tasks.resources.CheckCertificateStatusOrder')
|
||||||
def test_should_check_certificate_order(self, mock_check_cert_order):
|
def test_should_check_certificate_order(
|
||||||
mock_check_cert_order.return_value.process.return_value = None
|
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(
|
self.tasks.check_certificate_status(
|
||||||
None, self.order_id, self.external_project_id)
|
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(
|
mock_process.process_and_suppress_exceptions.assert_called_with(
|
||||||
self.order_id, self.external_project_id
|
self.order_id, self.external_project_id
|
||||||
)
|
)
|
||||||
|
mock_schedule.assert_called_with(
|
||||||
|
mock.ANY, 'result', None, 'order1234', 'keystone1234')
|
||||||
|
|
||||||
@mock.patch('barbican.tasks.resources.BeginTypeOrder')
|
@mock.patch('barbican.tasks.resources.BeginTypeOrder')
|
||||||
def test_process_order_catch_exception(self, mock_begin_order):
|
def test_process_order_catch_exception(self, mock_begin_order):
|
||||||
|
|||||||
@@ -272,7 +272,8 @@ class WhenBeginningCertificateTypeOrder(BaseOrderTestCase):
|
|||||||
self, mock_issue_cert_request):
|
self, mock_issue_cert_request):
|
||||||
mock_issue_cert_request.return_value = None
|
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(
|
self.order_repo.get.assert_called_once_with(
|
||||||
entity_id=self.order.id,
|
entity_id=self.order.id,
|
||||||
@@ -286,6 +287,7 @@ class WhenBeginningCertificateTypeOrder(BaseOrderTestCase):
|
|||||||
mock.ANY
|
mock.ANY
|
||||||
)
|
)
|
||||||
self.assertIsNone(self.order.container_id)
|
self.assertIsNone(self.order.container_id)
|
||||||
|
self.assertIsInstance(result, common.FollowOnProcessingStatusDTO)
|
||||||
|
|
||||||
@mock.patch(
|
@mock.patch(
|
||||||
'barbican.tasks.certificate_resources.issue_certificate_request')
|
'barbican.tasks.certificate_resources.issue_certificate_request')
|
||||||
@@ -293,7 +295,8 @@ class WhenBeginningCertificateTypeOrder(BaseOrderTestCase):
|
|||||||
self, mock_issue_cert_request):
|
self, mock_issue_cert_request):
|
||||||
mock_issue_cert_request.return_value = self.container
|
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(
|
self.order_repo.get.assert_called_once_with(
|
||||||
entity_id=self.order.id,
|
entity_id=self.order.id,
|
||||||
@@ -307,6 +310,7 @@ class WhenBeginningCertificateTypeOrder(BaseOrderTestCase):
|
|||||||
mock.ANY
|
mock.ANY
|
||||||
)
|
)
|
||||||
self.assertEqual(self.container.id, self.order.container_id)
|
self.assertEqual(self.container.id, self.order.container_id)
|
||||||
|
self.assertIsInstance(result, common.FollowOnProcessingStatusDTO)
|
||||||
|
|
||||||
|
|
||||||
class WhenUpdatingOrder(BaseOrderTestCase):
|
class WhenUpdatingOrder(BaseOrderTestCase):
|
||||||
@@ -323,7 +327,7 @@ class WhenUpdatingOrder(BaseOrderTestCase):
|
|||||||
def test_should_update_certificate_order(self, mock_modify_cert_request):
|
def test_should_update_certificate_order(self, mock_modify_cert_request):
|
||||||
self.order.type = models.OrderType.CERTIFICATE
|
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.order.id, self.external_project_id, self.updated_meta)
|
||||||
|
|
||||||
self.assertEqual(self.order.status, models.States.ACTIVE)
|
self.assertEqual(self.order.status, models.States.ACTIVE)
|
||||||
@@ -474,7 +478,8 @@ class WhenCheckingCertificateStatus(BaseOrderTestCase):
|
|||||||
self, mock_check_cert_request):
|
self, mock_check_cert_request):
|
||||||
mock_check_cert_request.return_value = None
|
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(
|
self.order_repo.get.assert_called_once_with(
|
||||||
entity_id=self.order.id,
|
entity_id=self.order.id,
|
||||||
@@ -488,6 +493,7 @@ class WhenCheckingCertificateStatus(BaseOrderTestCase):
|
|||||||
mock.ANY
|
mock.ANY
|
||||||
)
|
)
|
||||||
self.assertIsNone(self.order.container_id)
|
self.assertIsNone(self.order.container_id)
|
||||||
|
self.assertIsInstance(result, common.FollowOnProcessingStatusDTO)
|
||||||
|
|
||||||
@mock.patch(
|
@mock.patch(
|
||||||
'barbican.tasks.certificate_resources.check_certificate_request')
|
'barbican.tasks.certificate_resources.check_certificate_request')
|
||||||
|
|||||||
Reference in New Issue
Block a user