From 9e007acc441d1eb18069cdb6c21102f49c315697 Mon Sep 17 00:00:00 2001 From: Steve Baker Date: Fri, 17 Jun 2016 15:49:21 +1200 Subject: [PATCH] Add context to stack lock function arguments Context is added as the first argument to stack_lock_* db functions but is currently ignored for getting the session used for stack lock operations. This is required later for bug 1479723 but is added in its own change here to ease reviewing load. Change-Id: Ieb3e4e2ee67150777cbe1e961d0d1806cf1f7e46 Related-Bug: #1479723 --- heat/db/api.py | 17 ++--- heat/db/sqlalchemy/api.py | 8 +-- heat/engine/resources/stack_resource.py | 3 +- heat/engine/stack_lock.py | 15 +++-- heat/objects/stack_lock.py | 16 ++--- heat/tests/db/test_sqlalchemy_api.py | 66 ++++++++++--------- .../tests/engine/service/test_stack_delete.py | 10 +-- heat/tests/test_stack_lock.py | 33 ++++++---- heat/tests/test_stack_resource.py | 1 + 9 files changed, 96 insertions(+), 73 deletions(-) diff --git a/heat/db/api.py b/heat/db/api.py index 21eb737409..73b4ecaade 100644 --- a/heat/db/api.py +++ b/heat/db/api.py @@ -206,20 +206,21 @@ def stack_delete(context, stack_id): return IMPL.stack_delete(context, stack_id) -def stack_lock_create(stack_id, engine_id): - return IMPL.stack_lock_create(stack_id, engine_id) +def stack_lock_create(context, stack_id, engine_id): + return IMPL.stack_lock_create(context, stack_id, engine_id) -def stack_lock_get_engine_id(stack_id): - return IMPL.stack_lock_get_engine_id(stack_id) +def stack_lock_get_engine_id(context, stack_id): + return IMPL.stack_lock_get_engine_id(context, stack_id) -def stack_lock_steal(stack_id, old_engine_id, new_engine_id): - return IMPL.stack_lock_steal(stack_id, old_engine_id, new_engine_id) +def stack_lock_steal(context, stack_id, old_engine_id, new_engine_id): + return IMPL.stack_lock_steal(context, stack_id, old_engine_id, + new_engine_id) -def stack_lock_release(stack_id, engine_id): - return IMPL.stack_lock_release(stack_id, engine_id) +def stack_lock_release(context, stack_id, engine_id): + return IMPL.stack_lock_release(context, stack_id, engine_id) def persist_state_and_release_lock(context, stack_id, engine_id, values): diff --git a/heat/db/sqlalchemy/api.py b/heat/db/sqlalchemy/api.py index 5a3e7a6bd6..66787b79c6 100644 --- a/heat/db/sqlalchemy/api.py +++ b/heat/db/sqlalchemy/api.py @@ -619,7 +619,7 @@ def stack_delete(context, stack_id): @oslo_db_api.wrap_db_retry(max_retries=3, retry_on_deadlock=True, retry_interval=0.5, inc_retry_interval=True) -def stack_lock_create(stack_id, engine_id): +def stack_lock_create(context, stack_id, engine_id): session = get_session() with session.begin(): lock = session.query(models.StackLock).get(stack_id) @@ -628,7 +628,7 @@ def stack_lock_create(stack_id, engine_id): session.add(models.StackLock(stack_id=stack_id, engine_id=engine_id)) -def stack_lock_get_engine_id(stack_id): +def stack_lock_get_engine_id(context, stack_id): session = get_session() with session.begin(): lock = session.query(models.StackLock).get(stack_id) @@ -652,7 +652,7 @@ def persist_state_and_release_lock(context, stack_id, engine_id, values): return True -def stack_lock_steal(stack_id, old_engine_id, new_engine_id): +def stack_lock_steal(context, stack_id, old_engine_id, new_engine_id): session = get_session() with session.begin(): lock = session.query(models.StackLock).get(stack_id) @@ -664,7 +664,7 @@ def stack_lock_steal(stack_id, old_engine_id, new_engine_id): return lock.engine_id if lock is not None else True -def stack_lock_release(stack_id, engine_id): +def stack_lock_release(context, stack_id, engine_id): session = get_session() with session.begin(): rows_affected = session.query( diff --git a/heat/engine/resources/stack_resource.py b/heat/engine/resources/stack_resource.py index 61228e9c64..e660d38664 100644 --- a/heat/engine/resources/stack_resource.py +++ b/heat/engine/resources/stack_resource.py @@ -407,7 +407,8 @@ class StackResource(resource.Resource): if status == self.IN_PROGRESS: return False elif status == self.COMPLETE: - ret = stack_lock.StackLock.get_engine_id(self.resource_id) is None + ret = stack_lock.StackLock.get_engine_id( + self.context, self.resource_id) is None if ret: # Reset nested, to indicate we changed status self._nested = None diff --git a/heat/engine/stack_lock.py b/heat/engine/stack_lock.py index 2f00018a82..4993206173 100644 --- a/heat/engine/stack_lock.py +++ b/heat/engine/stack_lock.py @@ -35,14 +35,16 @@ class StackLock(object): self.listener = None def get_engine_id(self): - return stack_lock_object.StackLock.get_engine_id(self.stack_id) + return stack_lock_object.StackLock.get_engine_id(self.context, + self.stack_id) def try_acquire(self): """Try to acquire a stack lock. Don't raise an ActionInProgress exception or try to steal lock. """ - return stack_lock_object.StackLock.create(self.stack_id, + return stack_lock_object.StackLock.create(self.context, + self.stack_id, self.engine_id) def acquire(self, retry=True): @@ -51,7 +53,8 @@ class StackLock(object): :param retry: When True, retry if lock was released while stealing. :type retry: boolean """ - lock_engine_id = stack_lock_object.StackLock.create(self.stack_id, + lock_engine_id = stack_lock_object.StackLock.create(self.context, + self.stack_id, self.engine_id) if lock_engine_id is None: LOG.debug("Engine %(engine)s acquired lock on stack " @@ -74,7 +77,8 @@ class StackLock(object): "%(engine)s will attempt to steal the lock"), {'stack': self.stack_id, 'engine': self.engine_id}) - result = stack_lock_object.StackLock.steal(self.stack_id, + result = stack_lock_object.StackLock.steal(self.context, + self.stack_id, lock_engine_id, self.engine_id) @@ -105,7 +109,8 @@ class StackLock(object): """Release a stack lock.""" # Only the engine that owns the lock will be releasing it. - result = stack_lock_object.StackLock.release(self.stack_id, + result = stack_lock_object.StackLock.release(self.context, + self.stack_id, self.engine_id) if result is True: LOG.warning(_LW("Lock was already released on stack %s!"), diff --git a/heat/objects/stack_lock.py b/heat/objects/stack_lock.py index ede67a78d3..21445b4239 100644 --- a/heat/objects/stack_lock.py +++ b/heat/objects/stack_lock.py @@ -34,19 +34,19 @@ class StackLock( } @classmethod - def create(cls, stack_id, engine_id): - return db_api.stack_lock_create(stack_id, engine_id) + def create(cls, context, stack_id, engine_id): + return db_api.stack_lock_create(context, stack_id, engine_id) @classmethod - def steal(cls, stack_id, old_engine_id, new_engine_id): - return db_api.stack_lock_steal(stack_id, + def steal(cls, context, stack_id, old_engine_id, new_engine_id): + return db_api.stack_lock_steal(context, stack_id, old_engine_id, new_engine_id) @classmethod - def release(cls, stack_id, engine_id): - return db_api.stack_lock_release(stack_id, engine_id) + def release(cls, context, stack_id, engine_id): + return db_api.stack_lock_release(context, stack_id, engine_id) @classmethod - def get_engine_id(cls, stack_id): - return db_api.stack_lock_get_engine_id(stack_id) + def get_engine_id(cls, context, stack_id): + return db_api.stack_lock_get_engine_id(context, stack_id) diff --git a/heat/tests/db/test_sqlalchemy_api.py b/heat/tests/db/test_sqlalchemy_api.py index 5114b99ec2..584aeb28e3 100644 --- a/heat/tests/db/test_sqlalchemy_api.py +++ b/heat/tests/db/test_sqlalchemy_api.py @@ -1776,7 +1776,7 @@ class DBAPIStackTest(common.HeatTestCase): 'timeout': '90', 'current_traversal': 'another-dummy-uuid', } - db_api.stack_lock_create(stack.id, UUID1) + db_api.stack_lock_create(self.ctx, stack.id, UUID1) observed = db_api.persist_state_and_release_lock(self.ctx, stack.id, UUID1, values) self.assertIsNone(observed) @@ -1798,7 +1798,7 @@ class DBAPIStackTest(common.HeatTestCase): 'timeout': '90', 'current_traversal': 'another-dummy-uuid', } - db_api.stack_lock_create(stack.id, UUID2) + db_api.stack_lock_create(self.ctx, stack.id, UUID2) observed = db_api.persist_state_and_release_lock(self.ctx, stack.id, UUID1, values) self.assertTrue(observed) @@ -1813,7 +1813,7 @@ class DBAPIStackTest(common.HeatTestCase): 'timeout': '90', 'current_traversal': 'another-dummy-uuid', } - db_api.stack_lock_create(stack.id, UUID1) + db_api.stack_lock_create(self.ctx, stack.id, UUID1) observed = db_api.persist_state_and_release_lock(self.ctx, UUID2, UUID1, values) self.assertTrue(observed) @@ -2356,63 +2356,66 @@ class DBAPIStackLockTest(common.HeatTestCase): self.stack = create_stack(self.ctx, self.template, self.user_creds) def test_stack_lock_create_success(self): - observed = db_api.stack_lock_create(self.stack.id, UUID1) + observed = db_api.stack_lock_create(self.ctx, self.stack.id, UUID1) self.assertIsNone(observed) def test_stack_lock_create_fail_double_same(self): - db_api.stack_lock_create(self.stack.id, UUID1) - observed = db_api.stack_lock_create(self.stack.id, UUID1) + db_api.stack_lock_create(self.ctx, self.stack.id, UUID1) + observed = db_api.stack_lock_create(self.ctx, self.stack.id, UUID1) self.assertEqual(UUID1, observed) def test_stack_lock_create_fail_double_different(self): - db_api.stack_lock_create(self.stack.id, UUID1) - observed = db_api.stack_lock_create(self.stack.id, UUID2) + db_api.stack_lock_create(self.ctx, self.stack.id, UUID1) + observed = db_api.stack_lock_create(self.ctx, self.stack.id, UUID2) self.assertEqual(UUID1, observed) def test_stack_lock_get_id_success(self): - db_api.stack_lock_create(self.stack.id, UUID1) - observed = db_api.stack_lock_get_engine_id(self.stack.id) + db_api.stack_lock_create(self.ctx, self.stack.id, UUID1) + observed = db_api.stack_lock_get_engine_id(self.ctx, self.stack.id) self.assertEqual(UUID1, observed) def test_stack_lock_get_id_return_none(self): - observed = db_api.stack_lock_get_engine_id(self.stack.id) + observed = db_api.stack_lock_get_engine_id(self.ctx, self.stack.id) self.assertIsNone(observed) def test_stack_lock_steal_success(self): - db_api.stack_lock_create(self.stack.id, UUID1) - observed = db_api.stack_lock_steal(self.stack.id, UUID1, UUID2) + db_api.stack_lock_create(self.ctx, self.stack.id, UUID1) + observed = db_api.stack_lock_steal(self.ctx, self.stack.id, + UUID1, UUID2) self.assertIsNone(observed) def test_stack_lock_steal_fail_gone(self): - db_api.stack_lock_create(self.stack.id, UUID1) - db_api.stack_lock_release(self.stack.id, UUID1) - observed = db_api.stack_lock_steal(self.stack.id, UUID1, UUID2) + db_api.stack_lock_create(self.ctx, self.stack.id, UUID1) + db_api.stack_lock_release(self.ctx, self.stack.id, UUID1) + observed = db_api.stack_lock_steal(self.ctx, self.stack.id, + UUID1, UUID2) self.assertTrue(observed) def test_stack_lock_steal_fail_stolen(self): - db_api.stack_lock_create(self.stack.id, UUID1) + db_api.stack_lock_create(self.ctx, self.stack.id, UUID1) # Simulate stolen lock - db_api.stack_lock_release(self.stack.id, UUID1) - db_api.stack_lock_create(self.stack.id, UUID2) + db_api.stack_lock_release(self.ctx, self.stack.id, UUID1) + db_api.stack_lock_create(self.ctx, self.stack.id, UUID2) - observed = db_api.stack_lock_steal(self.stack.id, UUID3, UUID2) + observed = db_api.stack_lock_steal(self.ctx, self.stack.id, + UUID3, UUID2) self.assertEqual(UUID2, observed) def test_stack_lock_release_success(self): - db_api.stack_lock_create(self.stack.id, UUID1) - observed = db_api.stack_lock_release(self.stack.id, UUID1) + db_api.stack_lock_create(self.ctx, self.stack.id, UUID1) + observed = db_api.stack_lock_release(self.ctx, self.stack.id, UUID1) self.assertIsNone(observed) def test_stack_lock_release_fail_double(self): - db_api.stack_lock_create(self.stack.id, UUID1) - db_api.stack_lock_release(self.stack.id, UUID1) - observed = db_api.stack_lock_release(self.stack.id, UUID1) + db_api.stack_lock_create(self.ctx, self.stack.id, UUID1) + db_api.stack_lock_release(self.ctx, self.stack.id, UUID1) + observed = db_api.stack_lock_release(self.ctx, self.stack.id, UUID1) self.assertTrue(observed) def test_stack_lock_release_fail_wrong_engine_id(self): - db_api.stack_lock_create(self.stack.id, UUID1) - observed = db_api.stack_lock_release(self.stack.id, UUID2) + db_api.stack_lock_create(self.ctx, self.stack.id, UUID1) + observed = db_api.stack_lock_release(self.ctx, self.stack.id, UUID2) self.assertTrue(observed) @mock.patch.object(time, 'sleep') @@ -2420,7 +2423,8 @@ class DBAPIStackLockTest(common.HeatTestCase): with mock.patch('sqlalchemy.orm.Session.add', side_effect=db_exception.DBDeadlock) as mock_add: self.assertRaises(db_exception.DBDeadlock, - db_api.stack_lock_create, self.stack.id, UUID1) + db_api.stack_lock_create, self.ctx, + self.stack.id, UUID1) self.assertEqual(4, mock_add.call_count) @@ -3493,12 +3497,14 @@ class ResetStackStatusTests(common.HeatTestCase): def test_status_reset(self): db_api.stack_update(self.ctx, self.stack.id, {'status': 'IN_PROGRESS'}) - db_api.stack_lock_create(self.stack.id, UUID1) + db_api.stack_lock_create(self.ctx, self.stack.id, UUID1) db_api.reset_stack_status(self.ctx, self.stack.id) self.assertEqual('FAILED', self.stack.status) self.assertEqual('Stack status manually reset', self.stack.status_reason) - self.assertEqual(True, db_api.stack_lock_release(self.stack.id, UUID1)) + self.assertEqual(True, db_api.stack_lock_release(self.ctx, + self.stack.id, + UUID1)) def test_resource_reset(self): resource_progress = create_resource(self.ctx, self.stack, diff --git a/heat/tests/engine/service/test_stack_delete.py b/heat/tests/engine/service/test_stack_delete.py index 88d2a5449f..6ca22e71f4 100644 --- a/heat/tests/engine/service/test_stack_delete.py +++ b/heat/tests/engine/service/test_stack_delete.py @@ -105,7 +105,8 @@ class StackDeleteTest(common.HeatTestCase): sid = stack.store() # Insert a fake lock into the db - stack_lock_object.StackLock.create(stack.id, self.man.engine_id) + stack_lock_object.StackLock.create( + self.ctx, stack.id, self.man.engine_id) # Create a fake ThreadGroup too self.man.thread_group_mgr.groups[stack.id] = tools.DummyThreadGroup() @@ -135,7 +136,7 @@ class StackDeleteTest(common.HeatTestCase): sid = stack.store() # Insert a fake lock into the db - stack_lock_object.StackLock.create(stack.id, OTHER_ENGINE) + stack_lock_object.StackLock.create(self.ctx, stack.id, OTHER_ENGINE) st = stack_object.Stack.get_by_id(self.ctx, sid) mock_load.return_value = stack @@ -170,7 +171,7 @@ class StackDeleteTest(common.HeatTestCase): sid = stack.store() # Insert a fake lock into the db - stack_lock_object.StackLock.create(stack.id, OTHER_ENGINE) + stack_lock_object.StackLock.create(self.ctx, stack.id, OTHER_ENGINE) st = stack_object.Stack.get_by_id(self.ctx, sid) mock_load.return_value = stack @@ -202,7 +203,8 @@ class StackDeleteTest(common.HeatTestCase): sid = stack.store() # Insert a fake lock into the db - stack_lock_object.StackLock.create(stack.id, "other-engine-fake-uuid") + stack_lock_object.StackLock.create( + self.ctx, stack.id, "other-engine-fake-uuid") st = stack_object.Stack.get_by_id(self.ctx, sid) mock_load.return_value = stack diff --git a/heat/tests/test_stack_lock.py b/heat/tests/test_stack_lock.py index ecdb2eb50b..a920f4698a 100644 --- a/heat/tests/test_stack_lock.py +++ b/heat/tests/test_stack_lock.py @@ -47,7 +47,8 @@ class StackLockTest(common.HeatTestCase): self.engine_id) slock.acquire() - mock_create.assert_called_once_with(self.stack_id, self.engine_id) + mock_create.assert_called_once_with( + self.context, self.stack_id, self.engine_id) def test_failed_acquire_existing_lock_current_engine(self): mock_create = self.patchobject(stack_lock_object.StackLock, @@ -63,7 +64,8 @@ class StackLockTest(common.HeatTestCase): self.stack_id, tenant_safe=False, show_deleted=True) - mock_create.assert_called_once_with(self.stack_id, self.engine_id) + mock_create.assert_called_once_with( + self.context, self.stack_id, self.engine_id) def test_successful_acquire_existing_lock_engine_dead(self): mock_create = self.patchobject(stack_lock_object.StackLock, @@ -78,9 +80,10 @@ class StackLockTest(common.HeatTestCase): self.patchobject(service_utils, 'engine_alive', return_value=False) slock.acquire() - mock_create.assert_called_once_with(self.stack_id, self.engine_id) - mock_steal.assert_called_once_with(self.stack_id, 'fake-engine-id', - self.engine_id) + mock_create.assert_called_once_with( + self.context, self.stack_id, self.engine_id) + mock_steal.assert_called_once_with( + self.context, self.stack_id, 'fake-engine-id', self.engine_id) def test_failed_acquire_existing_lock_engine_alive(self): mock_create = self.patchobject(stack_lock_object.StackLock, @@ -97,7 +100,8 @@ class StackLockTest(common.HeatTestCase): tenant_safe=False, show_deleted=True) - mock_create.assert_called_once_with(self.stack_id, self.engine_id) + mock_create.assert_called_once_with( + self.context, self.stack_id, self.engine_id) def test_failed_acquire_existing_lock_engine_dead(self): mock_create = self.patchobject(stack_lock_object.StackLock, @@ -117,9 +121,10 @@ class StackLockTest(common.HeatTestCase): tenant_safe=False, show_deleted=True) - mock_create.assert_called_once_with(self.stack_id, self.engine_id) - mock_steal.assert_called_once_with(self.stack_id, 'fake-engine-id', - self.engine_id) + mock_create.assert_called_once_with( + self.context, self.stack_id, self.engine_id) + mock_steal.assert_called_once_with( + self.context, self.stack_id, 'fake-engine-id', self.engine_id) def test_successful_acquire_with_retry(self): mock_create = self.patchobject(stack_lock_object.StackLock, @@ -135,9 +140,10 @@ class StackLockTest(common.HeatTestCase): slock.acquire() mock_create.assert_has_calls( - [mock.call(self.stack_id, self.engine_id)] * 2) + [mock.call(self.context, self.stack_id, self.engine_id)] * 2) mock_steal.assert_has_calls( - [mock.call(self.stack_id, 'fake-engine-id', self.engine_id)] * 2) + [mock.call(self.context, self.stack_id, + 'fake-engine-id', self.engine_id)] * 2) def test_failed_acquire_one_retry_only(self): mock_create = self.patchobject(stack_lock_object.StackLock, @@ -158,9 +164,10 @@ class StackLockTest(common.HeatTestCase): show_deleted=True) mock_create.assert_has_calls( - [mock.call(self.stack_id, self.engine_id)] * 2) + [mock.call(self.context, self.stack_id, self.engine_id)] * 2) mock_steal.assert_has_calls( - [mock.call(self.stack_id, 'fake-engine-id', self.engine_id)] * 2) + [mock.call(self.context, self.stack_id, + 'fake-engine-id', self.engine_id)] * 2) def test_context_mgr_exception(self): stack_lock_object.StackLock.create = mock.Mock(return_value=None) diff --git a/heat/tests/test_stack_resource.py b/heat/tests/test_stack_resource.py index 25db23f7fe..9e52ae98c1 100644 --- a/heat/tests/test_stack_resource.py +++ b/heat/tests/test_stack_resource.py @@ -745,6 +745,7 @@ class StackResourceCheckCompleteTest(StackResourceBaseTest): self.mock_status.assert_called_once_with( self.parent_resource.context, self.parent_resource.resource_id) self.mock_lock.assert_called_once_with( + self.parent_resource.context, self.parent_resource.resource_id) def test_state_err(self):