Skip to content

Commit

Permalink
Don't destroy a resource before creating its replacement
Browse files Browse the repository at this point in the history
During an update, where a resource must be replaced in its entirety, create
the replacement resource before deleting the old resource.

Fixes bug #1176142

Change-Id: Id89654bad297815bdbcc86f666367772889b5df4
  • Loading branch information
zaneb committed Jun 18, 2013
1 parent a3e3f7a commit 9dd0703
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 22 deletions.
3 changes: 2 additions & 1 deletion heat/db/sqlalchemy/api.py
Expand Up @@ -70,7 +70,8 @@ def resource_get(context, resource_id):
def resource_get_by_name_and_stack(context, resource_name, stack_id):
result = model_query(context, models.Resource).\
filter_by(name=resource_name).\
filter_by(stack_id=stack_id).first()
filter_by(stack_id=stack_id).\
order_by('created_at').first()

return result

Expand Down
29 changes: 25 additions & 4 deletions heat/engine/update.py
Expand Up @@ -53,9 +53,14 @@ def __call__(self):
update = scheduler.DependencyTaskGroup(new_deps,
self._update_resource)

yield cleanup()
yield create_new()
yield update()
try:
yield update()
finally:
prev_deps = self.previous_stack._get_dependencies(
self.previous_stack.resources.itervalues())
self.previous_stack.dependencies = prev_deps
yield cleanup()

@scheduler.wrappertask
def _remove_old_resource(self, existing_res):
Expand All @@ -79,10 +84,26 @@ def _create_new_resource(self, new_res):
@scheduler.wrappertask
def _replace_resource(self, new_res):
res_name = new_res.name
yield self.existing_stack[res_name].destroy()

existing_res = self.existing_stack[res_name]
existing_res.stack = self.previous_stack
self.previous_stack[res_name] = existing_res

new_res.stack = self.existing_stack
self.existing_stack[res_name] = new_res
yield new_res.create()
if new_res.state is None:
yield new_res.create()
else:
new_res.state_set(new_res.UPDATE_COMPLETE)

# Remove from rollback cache
previous = resource.Resource(res_name,
self.previous_stack[res_name].t,
self.previous_stack)
self.previous_stack[res_name] = previous

existing_res.stack = self.existing_stack
yield existing_res.destroy()

@scheduler.wrappertask
def _update_resource(self, new_res):
Expand Down
21 changes: 4 additions & 17 deletions heat/tests/test_parser.py
Expand Up @@ -721,7 +721,6 @@ def test_update_modify_update_failed(self):
'Properties': {'Foo': 'abc'}}}}

self.m.StubOutWithMock(scheduler.TaskRunner, '_sleep')
scheduler.TaskRunner._sleep(mox.IsA(int)).MultipleTimes()
mox.Replay(scheduler.TaskRunner._sleep)

self.stack = parser.Stack(self.ctx, 'update_test_stack',
Expand Down Expand Up @@ -806,7 +805,6 @@ def test_update_modify_replace_failed_create(self):
'Properties': {'Foo': 'abc'}}}}

self.m.StubOutWithMock(scheduler.TaskRunner, '_sleep')
scheduler.TaskRunner._sleep(mox.IsA(int)).MultipleTimes()
mox.Replay(scheduler.TaskRunner._sleep)

self.stack = parser.Stack(self.ctx, 'update_test_stack',
Expand Down Expand Up @@ -900,10 +898,9 @@ def test_update_rollback(self):
# key/property in update_allowed_keys/update_allowed_properties

# patch in a dummy handle_create making the replace fail when creating
# the replacement rsrc, but succeed the second call (rollback)
# the replacement rsrc
self.m.StubOutWithMock(generic_rsrc.GenericResource, 'handle_create')
generic_rsrc.GenericResource.handle_create().AndRaise(Exception)
generic_rsrc.GenericResource.handle_create().AndReturn(None)
self.m.ReplayAll()

self.stack.update(updated_stack)
Expand All @@ -921,7 +918,6 @@ def test_update_rollback_fail(self):
'Properties': {'Foo': 'abc'}}}}

self.m.StubOutWithMock(scheduler.TaskRunner, '_sleep')
scheduler.TaskRunner._sleep(mox.IsA(int)).MultipleTimes()
mox.Replay(scheduler.TaskRunner._sleep)

self.stack = parser.Stack(self.ctx, 'update_test_stack',
Expand All @@ -944,8 +940,9 @@ def test_update_rollback_fail(self):
# patch in a dummy handle_create making the replace fail when creating
# the replacement rsrc, and again on the second call (rollback)
self.m.StubOutWithMock(generic_rsrc.GenericResource, 'handle_create')
self.m.StubOutWithMock(generic_rsrc.GenericResource, 'handle_delete')
generic_rsrc.GenericResource.handle_create().AndRaise(Exception)
generic_rsrc.GenericResource.handle_create().AndRaise(Exception)
generic_rsrc.GenericResource.handle_delete().AndRaise(Exception)
self.m.ReplayAll()

self.stack.update(updated_stack)
Expand Down Expand Up @@ -1124,16 +1121,12 @@ def test_update_by_reference_and_rollback_1(self):
# resource.UpdateReplace because we've not specified the modified
# key/property in update_allowed_keys/update_allowed_properties

generic_rsrc.GenericResource.FnGetRefId().AndReturn(
generic_rsrc.GenericResource.FnGetRefId().MultipleTimes().AndReturn(
'AResource')

# mock to make the replace fail when creating the replacement resource
generic_rsrc.GenericResource.handle_create().AndRaise(Exception)

generic_rsrc.GenericResource.handle_create().AndReturn(None)
generic_rsrc.GenericResource.FnGetRefId().MultipleTimes().AndReturn(
'AResource')

self.m.ReplayAll()

updated_stack = parser.Stack(self.ctx, 'updated_stack',
Expand Down Expand Up @@ -1205,12 +1198,6 @@ def handle_create(self):
# replacement resource
generic_rsrc.GenericResource.handle_create().AndRaise(Exception)

# Calls to GenericResource.handle_update will raise
# resource.UpdateReplace because we've not specified the modified
# key/property in update_allowed_keys/update_allowed_properties

generic_rsrc.GenericResource.handle_create().AndReturn(None)

self.m.ReplayAll()

updated_stack = parser.Stack(self.ctx, 'updated_stack',
Expand Down

0 comments on commit 9dd0703

Please sign in to comment.