openstack/heat
Code Issues Proposed changes

Merge "Fix action (CREATE) in progress error for TemplateResource/RG"

Browse Source
This commit is contained in:
Jenkins 2015-10-04 15:57:23 +00:00 committed by Gerrit Code Review
commit 8e8b338f98
13 changed files with 143 additions and 27 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
heat/db/api.py
View File

@ -201,6 +201,11 @@ def stack_lock_release(stack_id, engine_id):
return IMPL.stack_lock_release(stack_id, engine_id)
def persist_state_and_release_lock(context, stack_id, engine_id, values):
return IMPL.persist_state_and_release_lock(context, stack_id,
engine_id, values)
def stack_get_root_id(context, stack_id):
return IMPL.stack_get_root_id(context, stack_id)

16
heat/db/sqlalchemy/api.py
View File

@ -562,6 +562,22 @@ def stack_lock_get_engine_id(stack_id):
return lock.engine_id
def persist_state_and_release_lock(context, stack_id, engine_id, values):
session = _session(context)
with session.begin():
rows_updated = (session.query(models.Stack)
.filter(models.Stack.id == stack_id)
.update(values, synchronize_session=False))
rows_affected = None
if rows_updated is not None and rows_updated > 0:
rows_affected = session.query(
models.StackLock
).filter_by(stack_id=stack_id, engine_id=engine_id).delete()
session.expire_all()
if not rows_affected:
return True
def stack_lock_steal(stack_id, old_engine_id, new_engine_id):
session = get_session()
with session.begin():

12
heat/engine/service.py
View File

@ -160,8 +160,16 @@ class ThreadGroupManager(object):
"""
def release(gt):
"""Callback function that will be passed to GreenThread.link()."""
lock.release()
"""Callback function that will be passed to GreenThread.link().
Persist the stack state to COMPLETE and FAILED close to
releasing the lock to avoid race condtitions.
"""
if stack is not None and stack.action not in (
stack.DELETE, stack.ROLLBACK):
stack.persist_state_and_release_lock(lock.engine_id)
else:
lock.release()
th = self.start(stack.id, func, *args, **kwargs)
th.link(release)

51
heat/engine/stack.py
View File

@ -706,7 +706,7 @@ class Stack(collections.Mapping):
@profiler.trace('Stack.state_set', hide_args=False)
def state_set(self, action, status, reason):
"""Update the stack state in the database."""
"""Update the stack state."""
if action not in self.ACTIONS:
raise ValueError(_("Invalid action %s") % action)
@ -717,22 +717,47 @@ class Stack(collections.Mapping):
self.status = status
self.status_reason = reason
# Persist state to db only if status == IN_PROGRESS
# or action == self.DELETE/self.ROLLBACK. Else, it would
# be done before releasing the stack lock.
if status == self.IN_PROGRESS or action in (
self.DELETE, self.ROLLBACK):
self._persist_state()
def _persist_state(self):
"""Persist stack state to database"""
if self.id is None:
return
stack = stack_object.Stack.get_by_id(self.context, self.id)
if stack is not None:
notification.send(self)
self._add_event(action, status, reason)
LOG.info(_LI('Stack %(action)s %(status)s (%(name)s): '
'%(reason)s'),
{'action': action,
'status': status,
'name': self.name,
'reason': reason})
stack.update_and_save({'action': action,
'status': status,
'status_reason': reason})
values = {'action': self.action,
'status': self.status,
'status_reason': self.status_reason}
self._send_notification_and_add_event()
stack.update_and_save(values)
def _send_notification_and_add_event(self):
notification.send(self)
self._add_event(self.action, self.status, self.status_reason)
LOG.info(_LI('Stack %(action)s %(status)s (%(name)s): '
'%(reason)s'),
{'action': self.action,
'status': self.status,
'name': self.name,
'reason': self.status_reason})
def persist_state_and_release_lock(self, engine_id):
"""Persist stack state to database and release stack lock"""
if self.id is None:
return
stack = stack_object.Stack.get_by_id(self.context, self.id)
if stack is not None:
values = {'action': self.action,
'status': self.status,
'status_reason': self.status_reason}
self._send_notification_and_add_event()
stack.persist_state_and_release_lock(self.context, self.id,
engine_id, values)
@property
def state(self):

6
heat/objects/stack.py
View File

@ -169,6 +169,12 @@ class Stack(
return db_api.stack_update(context, stack_id, values,
exp_trvsl=exp_trvsl)
@classmethod
def persist_state_and_release_lock(cls, context, stack_id,
engine_id, values):
return db_api.persist_state_and_release_lock(context, stack_id,
engine_id, values)
@classmethod
def delete(cls, context, stack_id):
db_api.stack_delete(context, stack_id)

56
heat/tests/db/test_sqlalchemy_api.py
View File

@ -778,6 +778,7 @@ class SqlAlchemyTest(common.HeatTestCase):
self._mock_create(self.m)
self.m.ReplayAll()
stack.create()
stack._persist_state()
self.m.UnsetStubs()
events = db_api.event_get_all_by_stack(self.ctx, UUID1)
@ -869,6 +870,7 @@ class SqlAlchemyTest(common.HeatTestCase):
self._mock_create(self.m)
self.m.ReplayAll()
stack.create()
stack._persist_state()
self.m.UnsetStubs()
num_events = db_api.event_count_all_by_stack(self.ctx, UUID1)
@ -889,6 +891,7 @@ class SqlAlchemyTest(common.HeatTestCase):
self._mock_create(self.m)
self.m.ReplayAll()
[s.create() for s in stacks]
[s._persist_state() for s in stacks]
self.m.UnsetStubs()
events = db_api.event_get_all_by_tenant(self.ctx)
@ -909,6 +912,7 @@ class SqlAlchemyTest(common.HeatTestCase):
self._mock_create(self.m)
self.m.ReplayAll()
[s.create() for s in stacks]
[s._persist_state() for s in stacks]
self.m.UnsetStubs()
events = db_api.event_get_all(self.ctx)
@ -1657,6 +1661,58 @@ class DBAPIStackTest(common.HeatTestCase):
exp_trvsl=diff_uuid)
self.assertFalse(updated)
def test_stack_set_status_release_lock(self):
stack = create_stack(self.ctx, self.template, self.user_creds)
values = {
'name': 'db_test_stack_name2',
'action': 'update',
'status': 'failed',
'status_reason': "update_failed",
'timeout': '90',
'current_traversal': 'another-dummy-uuid',
}
db_api.stack_lock_create(stack.id, UUID1)
observed = db_api.persist_state_and_release_lock(self.ctx, stack.id,
UUID1, values)
self.assertIsNone(observed)
stack = db_api.stack_get(self.ctx, stack.id)
self.assertEqual('db_test_stack_name2', stack.name)
self.assertEqual('update', stack.action)
self.assertEqual('failed', stack.status)
self.assertEqual('update_failed', stack.status_reason)
self.assertEqual(90, stack.timeout)
self.assertEqual('another-dummy-uuid', stack.current_traversal)
def test_stack_set_status_release_lock_failed(self):
stack = create_stack(self.ctx, self.template, self.user_creds)
values = {
'name': 'db_test_stack_name2',
'action': 'update',
'status': 'failed',
'status_reason': "update_failed",
'timeout': '90',
'current_traversal': 'another-dummy-uuid',
}
db_api.stack_lock_create(stack.id, UUID2)
observed = db_api.persist_state_and_release_lock(self.ctx, stack.id,
UUID1, values)
self.assertTrue(observed)
def test_stack_set_status_failed_release_lock(self):
stack = create_stack(self.ctx, self.template, self.user_creds)
values = {
'name': 'db_test_stack_name2',
'action': 'update',
'status': 'failed',
'status_reason': "update_failed",
'timeout': '90',
'current_traversal': 'another-dummy-uuid',
}
db_api.stack_lock_create(stack.id, UUID1)
observed = db_api.persist_state_and_release_lock(self.ctx, UUID2,
UUID1, values)
self.assertTrue(observed)
def test_stack_get_returns_a_stack(self):
stack = create_stack(self.ctx, self.template, self.user_creds)
ret_stack = db_api.stack_get(self.ctx, stack.id, show_deleted=False)

2
heat/tests/engine/service/test_stack_adopt.py
View File

@ -92,7 +92,7 @@ class StackServiceAdoptTest(common.HeatTestCase):
{'adopt_stack_data': str(adopt_data)})
stack = stack_object.Stack.get_by_id(self.ctx, result['stack_id'])
self.assertEqual((parser.Stack.ADOPT, parser.Stack.COMPLETE),
self.assertEqual((parser.Stack.ADOPT, parser.Stack.IN_PROGRESS),
(stack.action, stack.status))
def test_stack_adopt_disabled(self):

3
heat/tests/engine/service/test_stack_events.py
View File

@ -109,6 +109,7 @@ class StackEventTest(common.HeatTestCase):
result = self.eng.update_stack(self.ctx, self.stack.identifier(),
new_tmpl, None, None, {})
self.stack._persist_state()
# The self.stack reference needs to be updated. Since the underlying
# stack is updated in update_stack, the original reference is now
@ -120,7 +121,7 @@ class StackEventTest(common.HeatTestCase):
self.assertTrue(result['stack_id'])
events = self.eng.list_events(self.ctx, self.stack.identifier())
self.assertEqual(10, len(events))
self.assertEqual(11, len(events))
for ev in events:
self.assertIn('event_identity', ev)

2
heat/tests/engine/service/test_stack_snapshot.py
View File

@ -57,12 +57,14 @@ class SnapshotServiceTest(common.HeatTestCase):
def test_show_snapshot_not_belong_to_stack(self):
stk1 = self._create_stack('stack_snaphot_not_belong_to_stack_1')
stk1._persist_state()
snapshot1 = self.engine.stack_snapshot(
self.ctx, stk1.identifier(), 'snap1')
self.engine.thread_group_mgr.groups[stk1.id].wait()
snapshot_id = snapshot1['id']
stk2 = self._create_stack('stack_snaphot_not_belong_to_stack_2')
stk2._persist_state()
ex = self.assertRaises(dispatcher.ExpectedException,
self.engine.show_snapshot,
self.ctx, stk2.identifier(),

4
heat/tests/engine/service/test_stack_update.py
View File

@ -439,13 +439,12 @@ class ServiceStackUpdateTest(common.HeatTestCase):
}
}
template = templatem.Template(tpl)
create_stack = stack.Stack(self.ctx, stack_name, template)
sid = create_stack.store()
create_stack.create()
self.assertEqual((create_stack.CREATE, create_stack.COMPLETE),
create_stack.state)
create_stack._persist_state()
s = stack_object.Stack.get_by_id(self.ctx, sid)
old_stack = stack.Stack.load(self.ctx, stack=s)
@ -460,6 +459,7 @@ class ServiceStackUpdateTest(common.HeatTestCase):
result = self.man.update_stack(self.ctx, create_stack.identifier(),
tpl, {}, None, {})
old_stack._persist_state()
self.assertEqual((old_stack.UPDATE, old_stack.COMPLETE),
old_stack.state)
self.assertEqual(create_stack.identifier(), result)

1
heat/tests/engine/tools.py
View File

@ -229,6 +229,7 @@ def setup_stack(stack_name, ctx, create_res=True):
setup_mocks(m, stack)
m.ReplayAll()
stack.create()
stack._persist_state()
m.UnsetStubs()
return stack

9
heat/tests/test_stack.py
View File

@ -196,17 +196,10 @@ class StackTest(common.HeatTestCase):
status=stack.Stack.IN_PROGRESS)
self.stack.id = '1234'
# Simulate a deleted stack
self.m.StubOutWithMock(stack_object.Stack, 'get_by_id')
stack_object.Stack.get_by_id(self.stack.context,
self.stack.id).AndReturn(None)
self.m.ReplayAll()
self.stack.delete()
self.assertIsNone(self.stack.state_set(stack.Stack.CREATE,
stack.Stack.COMPLETE,
'test'))
self.m.VerifyAll()
def test_state_bad(self):
self.stack = stack.Stack(self.ctx, 'test_stack', self.tmpl,

3
heat/tests/test_stack_update.py
View File

@ -269,6 +269,7 @@ class StackUpdateTest(common.HeatTestCase):
self.stack.store()
stack_id = self.stack.id
self.stack.create()
self.stack._persist_state()
self.assertEqual((stack.Stack.CREATE, stack.Stack.COMPLETE),
self.stack.state)
@ -292,6 +293,7 @@ class StackUpdateTest(common.HeatTestCase):
template.Template(tmpl2))
self.stack.update(updated_stack)
self.stack._persist_state()
self.assertEqual((stack.Stack.UPDATE, stack.Stack.COMPLETE),
self.stack.state)
mock_upd.assert_called_once_with(mock.ANY, mock.ANY, prop_diff1)
@ -309,6 +311,7 @@ class StackUpdateTest(common.HeatTestCase):
template.Template(tmpl3))
self.stack.update(updated_stack)
self.stack._persist_state()
self.assertEqual((stack.Stack.UPDATE, stack.Stack.COMPLETE),
self.stack.state)