From faf984cbb5e62980e025191450a02d30aae8e02d Mon Sep 17 00:00:00 2001 From: Jason Dunsmore Date: Thu, 1 Aug 2013 12:37:29 -0500 Subject: [PATCH] Implement an "Action in progress" error. Raises an "Action in progress" error if an action is in progress when another action is issued. Fixes bug #1207032 Change-Id: I2b6fd92cfa4b93ac2531dd3d76bf2dfc01e58c50 --- heat/api/aws/exception.py | 14 ++++++++- heat/api/middleware/fault.py | 1 + heat/common/exception.py | 5 +++ heat/engine/service.py | 9 +++++- heat/tests/test_api_cfn_v1.py | 42 +++++++++++++++++++++++++ heat/tests/test_api_openstack_v1.py | 42 +++++++++++++++++++++++++ heat/tests/test_engine_service.py | 49 +++++++++++++++++++++++------ 7 files changed, 151 insertions(+), 11 deletions(-) diff --git a/heat/api/aws/exception.py b/heat/api/aws/exception.py index a0207b673..fc9281af7 100644 --- a/heat/api/aws/exception.py +++ b/heat/api/aws/exception.py @@ -246,6 +246,15 @@ class HeatAPINotImplementedError(HeatAPIException): err_type = "Server" +class HeatInvalidStateError(HeatAPIException): + ''' + Cannot perform action on stack in its current state + ''' + code = 400 + title = 'InvalidAction' + explanation = "Cannot perform action on stack in its current state" + + def map_remote_error(ex): """ Map rpc_common.RemoteError exceptions returned by the engine @@ -268,7 +277,8 @@ def map_remote_error(ex): 'UserParameterMissing', ) denied_errors = ('Forbidden', 'NotAuthorized') - already_exists_errors = ('StackExists') + already_exists_errors = ('StackExists',) + invalid_state_errors = ('ActionInProgress',) if ex.exc_type in inval_param_errors: return HeatInvalidParameterValueError(detail=ex.value) @@ -276,6 +286,8 @@ def map_remote_error(ex): return HeatAccessDeniedError(detail=ex.value) elif ex.exc_type in already_exists_errors: return AlreadyExistsError(detail=ex.value) + elif ex.exc_type in invalid_state_errors: + return HeatInvalidStateError(detail=ex.value) else: # Map everything else to internal server error for now return HeatInternalFailureError(detail=ex.value) diff --git a/heat/api/middleware/fault.py b/heat/api/middleware/fault.py index c8e77093f..5c1287102 100644 --- a/heat/api/middleware/fault.py +++ b/heat/api/middleware/fault.py @@ -55,6 +55,7 @@ class FaultWrapper(wsgi.Middleware): error_map = { 'AttributeError': webob.exc.HTTPBadRequest, + 'ActionInProgress': webob.exc.HTTPConflict, 'ValueError': webob.exc.HTTPBadRequest, 'StackNotFound': webob.exc.HTTPNotFound, 'ResourceNotFound': webob.exc.HTTPNotFound, diff --git a/heat/common/exception.py b/heat/common/exception.py index 4f0c08153..3492c3ac1 100644 --- a/heat/common/exception.py +++ b/heat/common/exception.py @@ -263,6 +263,11 @@ class WatchRuleNotFound(OpenstackException): message = _("The Watch Rule (%(watch_name)s) could not be found.") +class ActionInProgress(OpenstackException): + message = _("Stack %(stack_name)s already has an action (%(action)s) " + "in progress") + + class ResourceFailure(OpenstackException): message = _("%(exc_type)s: %(message)s") diff --git a/heat/engine/service.py b/heat/engine/service.py index 56eb5a17c..fed7d3bf1 100644 --- a/heat/engine/service.py +++ b/heat/engine/service.py @@ -277,6 +277,10 @@ class EngineService(service.Service): # Get the database representation of the existing stack db_stack = self._get_stack(cnxt, stack_identity) + if db_stack.status != parser.Stack.COMPLETE: + raise exception.ActionInProgress(stack_name=db_stack.name, + action=db_stack.action) + current_stack = parser.Stack.load(cnxt, stack=db_stack) # Now parse the template and any parameters for the updated @@ -378,8 +382,11 @@ class EngineService(service.Service): """ st = self._get_stack(cnxt, stack_identity) - logger.info('deleting stack %s' % st.name) + if st.status not in (parser.Stack.COMPLETE, parser.Stack.FAILED): + raise exception.ActionInProgress(stack_name=st.name, + action=st.action) + logger.info('deleting stack %s' % st.name) stack = parser.Stack.load(cnxt, stack=st) # Kill any pending threads by calling ThreadGroup.stop() diff --git a/heat/tests/test_api_cfn_v1.py b/heat/tests/test_api_cfn_v1.py index b302770e6..703a6d4bf 100644 --- a/heat/tests/test_api_cfn_v1.py +++ b/heat/tests/test_api_cfn_v1.py @@ -821,6 +821,48 @@ class CfnStackControllerTest(HeatTestCase): exception.AlreadyExistsError) self.m.VerifyAll() + def test_invalid_state_err(self): + ''' + Test that an ActionInProgress exception results in a + HeatInvalidStateError. + + ''' + # Format a dummy request + stack_name = "wordpress" + template = {u'Foo': u'bar'} + json_template = json.dumps(template) + params = {'Action': 'CreateStack', 'StackName': stack_name, + 'TemplateBody': '%s' % json_template, + 'TimeoutInMinutes': 30, + 'Parameters.member.1.ParameterKey': 'InstanceType', + 'Parameters.member.1.ParameterValue': 'm1.xlarge'} + engine_parms = {u'InstanceType': u'm1.xlarge'} + engine_args = {'timeout_mins': u'30'} + dummy_req = self._dummy_GET_request(params) + + # Insert an engine RPC error and ensure we map correctly to the + # heat exception type + self.m.StubOutWithMock(rpc, 'call') + + rpc.call(dummy_req.context, self.topic, + {'namespace': None, + 'method': 'create_stack', + 'args': {'stack_name': stack_name, + 'template': template, + 'params': engine_parms, + 'files': {}, + 'args': engine_args}, + 'version': self.api_version}, None + ).AndRaise(rpc_common.RemoteError("ActionInProgress")) + + self.m.ReplayAll() + + result = self.controller.create(dummy_req) + + self.assertEqual(type(result), + exception.HeatInvalidStateError) + self.m.VerifyAll() + def test_create_err_engine(self): # Format a dummy request stack_name = "wordpress" diff --git a/heat/tests/test_api_openstack_v1.py b/heat/tests/test_api_openstack_v1.py index 80d37a689..6639ec485 100644 --- a/heat/tests/test_api_openstack_v1.py +++ b/heat/tests/test_api_openstack_v1.py @@ -957,6 +957,48 @@ class StackControllerTest(ControllerTest, HeatTestCase): body=body) self.m.VerifyAll() + def test_update_in_progress_err(self): + ''' + Tests that the ActionInProgress exception results in an HTTPConflict. + + ''' + identity = identifier.HeatIdentifier(self.tenant, 'wordpress', '6') + template = {u'Foo': u'bar'} + parameters = {u'InstanceType': u'm1.xlarge'} + body = {'template': template, + 'parameters': parameters, + 'files': {}, + 'timeout_mins': 30} + + req = self._put('/stacks/%(stack_name)s/%(stack_id)s' % identity, + json.dumps(body)) + + self.m.StubOutWithMock(rpc, 'call') + error = to_remote_error(heat_exc.ActionInProgress(stack_name="foo", + action="UPDATE")) + rpc.call(req.context, self.topic, + {'namespace': None, + 'method': 'update_stack', + 'args': {'stack_identity': dict(identity), + 'template': template, + 'params': {'parameters': parameters}, + 'files': {}, + 'args': {'timeout_mins': 30}}, + 'version': self.api_version}, + None).AndRaise(error) + self.m.ReplayAll() + + resp = request_with_middleware(fault.FaultWrapper, + self.controller.update, + req, tenant_id=identity.tenant, + stack_name=identity.stack_name, + stack_id=identity.stack_id, + body=body) + self.assertEqual(resp.json['code'], 409) + self.assertEqual(resp.json['title'], 'Conflict') + self.assertEqual(resp.json['error']['type'], 'ActionInProgress') + self.m.VerifyAll() + def test_update_bad_name(self): identity = identifier.HeatIdentifier(self.tenant, 'wibble', '6') template = {u'Foo': u'bar'} diff --git a/heat/tests/test_engine_service.py b/heat/tests/test_engine_service.py index 069ed1fe6..d98671251 100644 --- a/heat/tests/test_engine_service.py +++ b/heat/tests/test_engine_service.py @@ -413,12 +413,8 @@ class StackServiceCreateUpdateDeleteTest(HeatTestCase): def test_stack_delete(self): stack_name = 'service_delete_test_stack' stack = get_wordpress_stack(stack_name, self.ctx) - sid = stack.store() - - s = db_api.stack_get(self.ctx, sid) - self.m.StubOutWithMock(parser.Stack, 'load') - - parser.Stack.load(self.ctx, stack=s).AndReturn(stack) + stack.status = "COMPLETE" + stack.store() self.man.tg = DummyThreadGroup() @@ -428,16 +424,29 @@ class StackServiceCreateUpdateDeleteTest(HeatTestCase): self.man.delete_stack(self.ctx, stack.identifier())) self.m.VerifyAll() + def test_stack_delete_action_in_progress_err(self): + ''' + Test that deleting a stack with an action in progress results in + an ActionInProgress exception. + + ''' + stack_name = 'service_delete_action_in_progress_err' + stack = get_wordpress_stack(stack_name, self.ctx) + stack.status = "IN_PROGRESS" + stack.store() + + self.assertRaises(exception.ActionInProgress, + self.man.delete_stack, + self.ctx, + stack.identifier()) + def test_stack_delete_nonexist(self): stack_name = 'service_delete_nonexist_test_stack' stack = get_wordpress_stack(stack_name, self.ctx) - self.m.ReplayAll() - self.assertRaises(exception.StackNotFound, self.man.delete_stack, self.ctx, stack.identifier()) - self.m.VerifyAll() def test_stack_update(self): stack_name = 'service_update_test_stack' @@ -445,6 +454,7 @@ class StackServiceCreateUpdateDeleteTest(HeatTestCase): template = '{ "Template": "data" }' old_stack = get_wordpress_stack(stack_name, self.ctx) + old_stack.status = 'COMPLETE' sid = old_stack.store() s = db_api.stack_get(self.ctx, sid) @@ -477,12 +487,33 @@ class StackServiceCreateUpdateDeleteTest(HeatTestCase): self.assertTrue(result['stack_id']) self.m.VerifyAll() + def test_stack_update_action_in_progress_err(self): + ''' + Test that updating a stack with an action in progress results + in an ActionInProgress exception. + + ''' + stack_name = 'service_update_action_in_progress_err_test_stack' + params = {'foo': 'bar'} + template = '{ "Template": "data" }' + + old_stack = get_wordpress_stack(stack_name, self.ctx) + old_stack.status = 'IN_PROGRESS' + old_stack.store() + + self.assertRaises( + exception.ActionInProgress, + self.man.update_stack, + self.ctx, old_stack.identifier(), + template, params, None, {}) + def test_stack_update_verify_err(self): stack_name = 'service_update_verify_err_test_stack' params = {'foo': 'bar'} template = '{ "Template": "data" }' old_stack = get_wordpress_stack(stack_name, self.ctx) + old_stack.status = 'COMPLETE' old_stack.store() sid = old_stack.store() s = db_api.stack_get(self.ctx, sid)