From 8938a26d6b5f747717649f55c761775e3cc52ad3 Mon Sep 17 00:00:00 2001 From: Davanum Srinivas Date: Thu, 15 Sep 2016 10:27:12 -0400 Subject: [PATCH 01/17] Update .gitreview for stable/newton Change-Id: I54e0df289a6f999ae1086fd8fef6284d908d95b0 --- .gitreview | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitreview b/.gitreview index 73add9f8c..8143a89bd 100644 --- a/.gitreview +++ b/.gitreview @@ -2,3 +2,4 @@ host=review.openstack.org port=29418 project=openstack/heat.git +defaultbranch=stable/newton From 4a57772a74f002273c46172277b5087acf0effa3 Mon Sep 17 00:00:00 2001 From: Zane Bitter Date: Thu, 15 Sep 2016 10:16:54 -0400 Subject: [PATCH 02/17] Don't log locked resource at ERROR level It's expected that during a convergence traversal, we may encounter a resource that is still locked by a previous traversal. Don't log an ERROR-level message about what is a normal condition. Instead, log at INFO level describing what is happening, with more details at DEBUG level. Change-Id: I645c2a173b828d4a983ba874037d059ee645955f Related-Bug: #1607814 (cherry picked from commit 7f5bd76f7a14ad67b1c8e38061b316dfc509d881) --- heat/engine/resource.py | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/heat/engine/resource.py b/heat/engine/resource.py index edacbf17b..2946c60d7 100644 --- a/heat/engine/resource.py +++ b/heat/engine/resource.py @@ -1813,14 +1813,15 @@ class Resource(object): raise if not updated_ok: - ex = exception.UpdateInProgress(self.name) - LOG.error(_LE( - 'Error acquiring lock for resource id:%(resource_id)s with ' - 'atomic_key:%(atomic_key)s, ' - 'engine_id:%(rs_engine_id)s/%(engine_id)s') % { - 'resource_id': rs.id, 'atomic_key': rs.atomic_key, - 'rs_engine_id': rs.engine_id, 'engine_id': engine_id}) - raise ex + LOG.info(_LI('Resource %s is locked for update; deferring'), + six.text_type(self)) + LOG.debug(('Resource id:%(resource_id)s with ' + 'atomic_key:%(atomic_key)s, locked ' + 'by engine_id:%(rs_engine_id)s/%(engine_id)s') % { + 'resource_id': rs.id, 'atomic_key': rs.atomic_key, + 'rs_engine_id': rs.engine_id, + 'engine_id': engine_id}) + raise exception.UpdateInProgress(self.name) def _release(self, engine_id): rs = None From 5443453fbbaab761a42023c556ee7ed4c55d60ba Mon Sep 17 00:00:00 2001 From: Sergey Kraynev Date: Thu, 15 Sep 2016 04:08:20 -0400 Subject: [PATCH 03/17] Allow update inplace for allowed_address_pairs This patch allows to do update inplace for allowed_address_pairs properties. Scenario mentioned in bug works correct now. Also add couple fixes to related test: - Add explicit translation name to string, otherwise it returns objects, that raise error during resolving Property name, which should be a string. - Add check, that update of any of mentioned properties does not cause replace. Change-Id: I913fd36012179f2fdd602f2cca06a89e3fa896f3 Closes-Bug: #1623821 (cherry picked from commit 353e7319dbbff7f10e7cdf664f017a01e15b5168) --- heat/engine/resources/openstack/neutron/port.py | 3 ++- heat/tests/openstack/neutron/test_neutron_port.py | 11 ++++++++++- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/heat/engine/resources/openstack/neutron/port.py b/heat/engine/resources/openstack/neutron/port.py index 687554b1b..2728a14d8 100644 --- a/heat/engine/resources/openstack/neutron/port.py +++ b/heat/engine/resources/openstack/neutron/port.py @@ -250,7 +250,8 @@ class Port(neutron.NeutronResource): ] ), }, - ) + ), + update_allowed=True, ), VNIC_TYPE: properties.Schema( properties.Schema.STRING, diff --git a/heat/tests/openstack/neutron/test_neutron_port.py b/heat/tests/openstack/neutron/test_neutron_port.py index 348f96bcc..cb2013eb2 100644 --- a/heat/tests/openstack/neutron/test_neutron_port.py +++ b/heat/tests/openstack/neutron/test_neutron_port.py @@ -933,7 +933,7 @@ class UpdatePortTest(common.HeatTestCase): return_value=fake_groups_list) props = {'network_id': u'net1234', - 'name': utils.PhysName(stack.name, 'port'), + 'name': str(utils.PhysName(stack.name, 'port')), 'admin_state_up': True, 'device_owner': u'network:dhcp'} @@ -970,6 +970,15 @@ class UpdatePortTest(common.HeatTestCase): update_props)()) update_port.assset_called_once_with(update_dict) + + # check, that update does not cause of Update Replace + create_snippet = rsrc_defn.ResourceDefinition(port.name, port.type(), + props) + after_props, before_props = port._prepare_update_props(update_snippet, + create_snippet) + self.assertIsNotNone( + port.update_template_diff_properties(after_props, before_props)) + # update with empty prop_diff scheduler.TaskRunner(port.handle_update, update_snippet, {}, {})() self.assertEqual(1, update_port.call_count) From 4695f47f68031195b23857a1fce827c39c626faa Mon Sep 17 00:00:00 2001 From: Zane Bitter Date: Fri, 16 Sep 2016 17:05:25 -0400 Subject: [PATCH 04/17] Don't acquire the lock when cancelling a stack We used to try to acquire the stack lock in order to find out which engine to cancel a running update on, in the misguided belief that it could never succeed. Accordingly, we never released the lock. Since it is entirely possible to encounter a race where the lock has already been released, use the get_engine_id() method instead to look up the ID of the engine holding the lock without attempting to acquire it. Change-Id: I1d026f8c67dddcf840ccbc2f3f1537693dc266fb Closes-Bug: #1624538 (cherry picked from commit e2ba3390cda4d7bfcf8397da097ac5358ead1ca5) --- heat/engine/service.py | 15 ++++++++---- heat/engine/stack_lock.py | 4 ++++ .../tests/engine/service/test_stack_update.py | 23 +++++++++++++++++-- 3 files changed, 36 insertions(+), 6 deletions(-) diff --git a/heat/engine/service.py b/heat/engine/service.py index 05dd1a561..443333e8d 100644 --- a/heat/engine/service.py +++ b/heat/engine/service.py @@ -1127,12 +1127,13 @@ class EngineService(service.Service): self.thread_group_mgr.start(current_stack.id, func) return - # stop the running update and take the lock - # as we cancel only running update, the acquire_result is - # always some engine_id, not None lock = stack_lock.StackLock(cnxt, current_stack.id, self.engine_id) - engine_id = lock.try_acquire() + engine_id = lock.get_engine_id() + + if engine_id is None: + LOG.debug('No lock found on stack %s', db_stack.name) + return if cancel_with_rollback: cancel_message = rpc_api.THREAD_CANCEL_WITH_ROLLBACK @@ -1156,6 +1157,12 @@ class EngineService(service.Service): raise exception.EventSendFailed(stack_name=current_stack.name, engine_id=engine_id) + else: + LOG.warning(_('Cannot cancel stack %(stack_name)s: lock held by ' + 'unknown engine %(engine_id)s') % { + 'stack_name': db_stack.name, + 'engine_id': engine_id}) + @context.request_context def validate_template(self, cnxt, template, params=None, files=None, environment_files=None, show_nested=False, diff --git a/heat/engine/stack_lock.py b/heat/engine/stack_lock.py index 2cb19c064..3d8822735 100644 --- a/heat/engine/stack_lock.py +++ b/heat/engine/stack_lock.py @@ -35,6 +35,10 @@ class StackLock(object): self.listener = None def get_engine_id(self): + """Return the ID of the engine which currently holds the lock. + + Returns None if there is no lock held on the stack. + """ return stack_lock_object.StackLock.get_engine_id(self.context, self.stack_id) diff --git a/heat/tests/engine/service/test_stack_update.py b/heat/tests/engine/service/test_stack_update.py index 708e95e43..fe09ddf04 100644 --- a/heat/tests/engine/service/test_stack_update.py +++ b/heat/tests/engine/service/test_stack_update.py @@ -482,8 +482,10 @@ resources: stk.disable_rollback = False stk.store() + self.man.engine_id = service_utils.generate_engine_id() + self.patchobject(stack.Stack, 'load', return_value=stk) - self.patchobject(stack_lock.StackLock, 'try_acquire', + self.patchobject(stack_lock.StackLock, 'get_engine_id', return_value=self.man.engine_id) self.patchobject(self.man.thread_group_mgr, 'send') @@ -500,7 +502,7 @@ resources: stk.disable_rollback = False stk.store() self.patchobject(stack.Stack, 'load', return_value=stk) - self.patchobject(stack_lock.StackLock, 'try_acquire', + self.patchobject(stack_lock.StackLock, 'get_engine_id', return_value=str(uuid.uuid4())) self.patchobject(service_utils, 'engine_alive', return_value=True) @@ -514,6 +516,23 @@ resources: self.man.stack_cancel_update, self.ctx, stk.identifier()) + def test_stack_cancel_update_no_lock(self): + stack_name = 'service_update_stack_test_cancel_same_engine' + stk = tools.get_stack(stack_name, self.ctx) + stk.state_set(stk.UPDATE, stk.IN_PROGRESS, 'test_override') + stk.disable_rollback = False + stk.store() + + self.patchobject(stack.Stack, 'load', return_value=stk) + self.patchobject(stack_lock.StackLock, 'get_engine_id', + return_value=None) + self.patchobject(self.man.thread_group_mgr, 'send') + + self.man.stack_cancel_update(self.ctx, stk.identifier(), + cancel_with_rollback=False) + + self.assertFalse(self.man.thread_group_mgr.send.called) + def test_stack_cancel_update_wrong_state_fails(self): stack_name = 'service_update_cancel_test_stack' stk = tools.get_stack(stack_name, self.ctx) From 80081db9f8fa89d8984855ad219848555a3c76ed Mon Sep 17 00:00:00 2001 From: Davanum Srinivas Date: Thu, 15 Sep 2016 10:27:17 -0400 Subject: [PATCH 05/17] Update UPPER_CONSTRAINTS_FILE for stable/newton Change-Id: Iab0150ac0a611de8c8ae0138c76083cacb383068 --- tox.ini | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tox.ini b/tox.ini index 38b442bae..6995d956e 100644 --- a/tox.ini +++ b/tox.ini @@ -8,7 +8,7 @@ setenv = VIRTUAL_ENV={envdir} OS_TEST_PATH=heat/tests TESTR_START_DIR=heat/tests usedevelop = True -install_command = pip install -c{env:UPPER_CONSTRAINTS_FILE:https://git.openstack.org/cgit/openstack/requirements/plain/upper-constraints.txt} {opts} {packages} +install_command = pip install -c{env:UPPER_CONSTRAINTS_FILE:https://git.openstack.org/cgit/openstack/requirements/plain/upper-constraints.txt?h=stable/newton} {opts} {packages} deps = -r{toxinidir}/requirements.txt -r{toxinidir}/test-requirements.txt commands = From 14a90d67a6444db8c519b4d0b9a3eadebe08287c Mon Sep 17 00:00:00 2001 From: Zane Bitter Date: Fri, 16 Sep 2016 09:22:01 -0400 Subject: [PATCH 06/17] Use correct schema for SoftwareDeploymentGroup rolling_update Change-Id: Ie5f1df4862fcbe7fc2949b3516f91e193461b2a0 Closes-Bug: #1624387 (cherry picked from commit 676281dc8c6708edf7e0b5a6c0a508f0b031d497) --- .../resources/openstack/heat/software_deployment.py | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/heat/engine/resources/openstack/heat/software_deployment.py b/heat/engine/resources/openstack/heat/software_deployment.py index 43d80c85f..43dabc1ba 100644 --- a/heat/engine/resources/openstack/heat/software_deployment.py +++ b/heat/engine/resources/openstack/heat/software_deployment.py @@ -642,6 +642,19 @@ class SoftwareDeploymentGroup(resource_group.ResourceGroup): default=0), } + update_policy_schema = { + resource_group.ResourceGroup.ROLLING_UPDATE: properties.Schema( + properties.Schema.MAP, + schema=rolling_update_schema, + support_status=support.SupportStatus(version='7.0.0') + ), + resource_group.ResourceGroup.BATCH_CREATE: properties.Schema( + properties.Schema.MAP, + schema=resource_group.ResourceGroup.batch_create_schema, + support_status=support.SupportStatus(version='7.0.0') + ) + } + def get_size(self): return len(self.properties[self.SERVERS]) From c7fe0ba5ee46dd97c5d07f8c45df6c382f24306c Mon Sep 17 00:00:00 2001 From: Anant Patil Date: Wed, 14 Sep 2016 16:59:55 +0530 Subject: [PATCH 07/17] Fix sync point delete When a resource failed, the stack state was set to FAILED and current traversal was set to emoty string. The actual traversal was lost and there was no way to delete the sync points belonging to the actual traversal. This change keeps the current traversal when you do a state set, so that later you can delete the sync points belonging to it. Also, the current traversal is set to empty when the stack has failed and there is no need to rollback. Closes-Bug: #1618155 Change-Id: Iec3922af92b70b0628fb94b7b2d597247e6d42c4 (cherry picked from commit 2e281df4280de5fdf5a56827d38845090e720392) --- heat/engine/stack.py | 15 ++++++------ heat/engine/worker.py | 31 +++++++++++++++++++++---- heat/tests/engine/test_engine_worker.py | 11 +++++++++ heat/tests/test_convg_stack.py | 3 ++- 4 files changed, 46 insertions(+), 14 deletions(-) diff --git a/heat/engine/stack.py b/heat/engine/stack.py index 7780e8e4c..e32428422 100644 --- a/heat/engine/stack.py +++ b/heat/engine/stack.py @@ -927,14 +927,9 @@ class Stack(collections.Mapping): self._send_notification_and_add_event() if self.convergence: # do things differently for convergence - exp_trvsl = self.current_traversal - if self.status == self.FAILED: - self.current_traversal = '' - values['current_traversal'] = self.current_traversal - updated = stack_object.Stack.select_and_update( self.context, self.id, values, - exp_trvsl=exp_trvsl) + exp_trvsl=self.current_traversal) return updated @@ -2024,13 +2019,17 @@ class Stack(collections.Mapping): """ resource_objects.Resource.purge_deleted(self.context, self.id) + exp_trvsl = self.current_traversal + if self.status == self.FAILED: + self.current_traversal = '' + prev_tmpl_id = None if (self.prev_raw_template_id is not None and self.status != self.FAILED): prev_tmpl_id = self.prev_raw_template_id self.prev_raw_template_id = None - stack_id = self.store() + stack_id = self.store(exp_trvsl=exp_trvsl) if stack_id is None: # Failed concurrent update LOG.warning(_LW("Failed to store stack %(name)s with traversal ID " @@ -2042,7 +2041,7 @@ class Stack(collections.Mapping): if prev_tmpl_id is not None: raw_template_object.RawTemplate.delete(self.context, prev_tmpl_id) - sync_point.delete_all(self.context, self.id, self.current_traversal) + sync_point.delete_all(self.context, self.id, exp_trvsl) if (self.action, self.status) == (self.DELETE, self.COMPLETE): if not self.owner_id: diff --git a/heat/engine/worker.py b/heat/engine/worker.py index 70fea6bfe..b1e821204 100644 --- a/heat/engine/worker.py +++ b/heat/engine/worker.py @@ -18,6 +18,7 @@ import eventlet.queue from oslo_log import log as logging import oslo_messaging from oslo_service import service +from oslo_utils import uuidutils from osprofiler import profiler from heat.common import context @@ -28,6 +29,7 @@ from heat.common import messaging as rpc_messaging from heat.db import api as db_api from heat.engine import check_resource from heat.engine import sync_point +from heat.objects import stack as stack_objects from heat.rpc import api as rpc_api from heat.rpc import worker_client as rpc_client @@ -102,14 +104,24 @@ class WorkerService(service.Service): in_progress resources to complete normally; no worker is stopped abruptly. """ - reason = 'User cancelled stack %s ' % stack.action - # state_set will update the current traversal to '' for FAILED state old_trvsl = stack.current_traversal + updated = _update_current_traversal(stack) + if not updated: + LOG.warning(_LW("Failed to update stack %(name)s with new " + "traversal, aborting stack cancel"), + {'name': stack.name}) + return + + reason = 'User cancelled stack %s ' % stack.action updated = stack.state_set(stack.action, stack.FAILED, reason) if not updated: - LOG.warning(_LW("Failed to stop traversal %(trvsl)s of stack " - "%(name)s while cancelling the operation."), - {'name': stack.name, 'trvsl': old_trvsl}) + LOG.warning(_LW("Failed to update stack %(name)s status" + " to %(action)_%(state)"), + {'name': stack.name, 'action': stack.action, + 'state': stack.FAILED}) + return + + sync_point.delete_all(stack.context, stack.id, old_trvsl) def stop_all_workers(self, stack): # stop the traversal @@ -172,6 +184,15 @@ class WorkerService(service.Service): _cancel_check_resource(stack_id, self.engine_id, self.thread_group_mgr) +def _update_current_traversal(stack): + previous_traversal = stack.current_traversal + stack.current_traversal = uuidutils.generate_uuid() + values = {'current_traversal': stack.current_traversal} + return stack_objects.Stack.select_and_update( + stack.context, stack.id, values, + exp_trvsl=previous_traversal) + + def _cancel_check_resource(stack_id, engine_id, tgm): LOG.debug('Cancelling workers for stack [%s] in engine [%s]', stack_id, engine_id) diff --git a/heat/tests/engine/test_engine_worker.py b/heat/tests/engine/test_engine_worker.py index 421cf4981..f02c18ab4 100644 --- a/heat/tests/engine/test_engine_worker.py +++ b/heat/tests/engine/test_engine_worker.py @@ -18,6 +18,7 @@ import mock from heat.db import api as db_api from heat.engine import check_resource from heat.engine import worker +from heat.objects import stack as stack_objects from heat.rpc import worker_client as wc from heat.tests import common from heat.tests import utils @@ -220,3 +221,13 @@ class WorkerServiceTest(common.HeatTestCase): mock_cw.assert_called_with(stack, mock_tgm, 'engine-001', _worker._rpc_client) self.assertFalse(stack.rollback.called) + + @mock.patch.object(stack_objects.Stack, 'select_and_update') + def test_update_current_traversal(self, mock_sau): + stack = mock.MagicMock() + stack.current_traversal = 'some-thing' + old_trvsl = stack.current_traversal + worker._update_current_traversal(stack) + self.assertNotEqual(old_trvsl, stack.current_traversal) + mock_sau.assert_called_once_with(mock.ANY, stack.id, mock.ANY, + exp_trvsl=old_trvsl) diff --git a/heat/tests/test_convg_stack.py b/heat/tests/test_convg_stack.py index d296cf611..11b79670d 100644 --- a/heat/tests/test_convg_stack.py +++ b/heat/tests/test_convg_stack.py @@ -375,8 +375,9 @@ class StackConvergenceCreateUpdateDeleteTest(common.HeatTestCase): stack = tools.get_stack('test_stack', utils.dummy_context(), template=tools.string_template_five, convergence=True) + stack.status = stack.FAILED stack.store() - stack.state_set(stack.action, stack.FAILED, 'test-reason') + stack.purge_db() self.assertEqual('', stack.current_traversal) @mock.patch.object(raw_template_object.RawTemplate, 'delete') From d26c90389af4ac4afa86e23e358856fcda779e78 Mon Sep 17 00:00:00 2001 From: huangtianhua Date: Wed, 14 Sep 2016 14:41:42 +0800 Subject: [PATCH 08/17] Correct the response info of template-version-list Correct HeatTemplateFormatVersion.2012-12-12 as cfn template. Closes-Bug: #1623340 Change-Id: I5dd0131c60ab5786ad667e4e61c3eeeb7fae047d (cherry picked from commit bf5ce6af12d04a782edcadf1faef721c38a34686) --- heat/engine/service.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/heat/engine/service.py b/heat/engine/service.py index 05dd1a561..cca3c3148 100644 --- a/heat/engine/service.py +++ b/heat/engine/service.py @@ -1482,7 +1482,7 @@ class EngineService(service.Service): for name in mgr.names()] versions = [] for t in sorted(_template_classes): # Sort to ensure dates come first - if issubclass(t[1], cfntemplate.CfnTemplate): + if issubclass(t[1], cfntemplate.CfnTemplateBase): type = 'cfn' else: type = 'hot' From 25fbedc726aebcb8179faa5c4a3d85de2a9067c2 Mon Sep 17 00:00:00 2001 From: Anant Patil Date: Wed, 14 Sep 2016 18:59:36 +0530 Subject: [PATCH 09/17] Cancel traversal of nested stack The stack cancel update would halt the parent stack from propagating but the nested stacks kept on going till they either failed or completed. This is not desired, the cancel update should stop all the nested stacks from moving further, albeit, it shouldn't abruptly stop the currently running workers. Change-Id: I3e1c58bbe4f92e2d2bfea539f3d0e861a3a7cef1 Co-Authored-By: Zane Bitter Closes-Bug: #1623201 (cherry picked from commit bc2e136fe34e25185f344ebce4fb171243ae7dc9) --- heat/engine/worker.py | 47 ++++++++++++++++--------- heat/tests/engine/test_engine_worker.py | 22 ++++++++++++ 2 files changed, 53 insertions(+), 16 deletions(-) diff --git a/heat/engine/worker.py b/heat/engine/worker.py index b1e821204..1a544a794 100644 --- a/heat/engine/worker.py +++ b/heat/engine/worker.py @@ -28,6 +28,7 @@ from heat.common.i18n import _LW from heat.common import messaging as rpc_messaging from heat.db import api as db_api from heat.engine import check_resource +from heat.engine import stack as parser from heat.engine import sync_point from heat.objects import stack as stack_objects from heat.rpc import api as rpc_api @@ -104,24 +105,17 @@ class WorkerService(service.Service): in_progress resources to complete normally; no worker is stopped abruptly. """ - old_trvsl = stack.current_traversal - updated = _update_current_traversal(stack) - if not updated: - LOG.warning(_LW("Failed to update stack %(name)s with new " - "traversal, aborting stack cancel"), - {'name': stack.name}) - return + _stop_traversal(stack) - reason = 'User cancelled stack %s ' % stack.action - updated = stack.state_set(stack.action, stack.FAILED, reason) - if not updated: - LOG.warning(_LW("Failed to update stack %(name)s status" - " to %(action)_%(state)"), - {'name': stack.name, 'action': stack.action, - 'state': stack.FAILED}) - return + db_child_stacks = stack_objects.Stack.get_all_by_root_owner_id( + stack.context, stack.id) - sync_point.delete_all(stack.context, stack.id, old_trvsl) + for db_child in db_child_stacks: + if db_child.status == parser.Stack.IN_PROGRESS: + child = parser.Stack.load(stack.context, + stack_id=db_child.id, + stack=db_child) + _stop_traversal(child) def stop_all_workers(self, stack): # stop the traversal @@ -184,6 +178,27 @@ class WorkerService(service.Service): _cancel_check_resource(stack_id, self.engine_id, self.thread_group_mgr) +def _stop_traversal(stack): + old_trvsl = stack.current_traversal + updated = _update_current_traversal(stack) + if not updated: + LOG.warning(_LW("Failed to update stack %(name)s with new " + "traversal, aborting stack cancel"), + {'name': stack.name}) + return + + reason = 'Stack %(action)s cancelled' % {'action': stack.action} + updated = stack.state_set(stack.action, stack.FAILED, reason) + if not updated: + LOG.warning(_LW("Failed to update stack %(name)s status" + " to %(action)_%(state)"), + {'name': stack.name, 'action': stack.action, + 'state': stack.FAILED}) + return + + sync_point.delete_all(stack.context, stack.id, old_trvsl) + + def _update_current_traversal(stack): previous_traversal = stack.current_traversal stack.current_traversal = uuidutils.generate_uuid() diff --git a/heat/tests/engine/test_engine_worker.py b/heat/tests/engine/test_engine_worker.py index f02c18ab4..175691694 100644 --- a/heat/tests/engine/test_engine_worker.py +++ b/heat/tests/engine/test_engine_worker.py @@ -17,6 +17,8 @@ import mock from heat.db import api as db_api from heat.engine import check_resource +from heat.engine import stack as parser +from heat.engine import template as templatem from heat.engine import worker from heat.objects import stack as stack_objects from heat.rpc import worker_client as wc @@ -178,6 +180,26 @@ class WorkerServiceTest(common.HeatTestCase): mock_ccr.assert_has_calls(calls, any_order=True) self.assertTrue(mock_wc.called) + @mock.patch.object(worker, '_stop_traversal') + def test_stop_traversal_stops_nested_stack(self, mock_st): + mock_tgm = mock.Mock() + ctx = utils.dummy_context() + tmpl = templatem.Template.create_empty_template() + stack1 = parser.Stack(ctx, 'stack1', tmpl, + current_traversal='123') + stack1.store() + stack2 = parser.Stack(ctx, 'stack2', tmpl, + owner_id=stack1.id, current_traversal='456') + stack2.store() + _worker = worker.WorkerService('host-1', 'topic-1', 'engine-001', + mock_tgm) + _worker.stop_traversal(stack1) + self.assertEqual(2, mock_st.call_count) + call1, call2 = mock_st.call_args_list + call_args1, call_args2 = call1[0][0], call2[0][0] + self.assertEqual('stack1', call_args1.name) + self.assertEqual('stack2', call_args2.name) + @mock.patch.object(worker, '_cancel_workers') @mock.patch.object(worker.WorkerService, 'stop_traversal') def test_stop_all_workers_when_stack_in_progress(self, mock_st, mock_cw): From 9ea43edfade947c03f8980c761687c458107478d Mon Sep 17 00:00:00 2001 From: OpenStack Proposal Bot Date: Tue, 20 Sep 2016 13:11:06 +0000 Subject: [PATCH 10/17] Updated from global requirements Change-Id: I0ebaeff953abe61f8d08b03ab93d11005ea1821a --- requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements.txt b/requirements.txt index 8b61d36ff..11ac45d5c 100644 --- a/requirements.txt +++ b/requirements.txt @@ -12,7 +12,7 @@ greenlet>=0.3.2 # MIT keystoneauth1>=2.10.0 # Apache-2.0 keystonemiddleware!=4.1.0,!=4.5.0,>=4.0.0 # Apache-2.0 lxml>=2.3 # BSD -netaddr!=0.7.16,>=0.7.12 # BSD +netaddr!=0.7.16,>=0.7.13 # BSD oslo.cache>=1.5.0 # Apache-2.0 oslo.config>=3.14.0 # Apache-2.0 oslo.concurrency>=3.8.0 # Apache-2.0 From 2dd44db1b9cf4b789d8a083df6f97ae1fb5e22d5 Mon Sep 17 00:00:00 2001 From: Steve Baker Date: Fri, 16 Sep 2016 03:29:59 +0000 Subject: [PATCH 11/17] Legacy delete attempt thread cancel before stop The error messages 'Command Out of Sync' are due to the threads being stopped in the middle of the database operations. This happens in the legacy action when delete is requested during a stack create. We have the thread cancel message but that was not being used in this case. Thread cancel should provide a more graceful way of ensuring the stack is in a FAILED state before the delete is attempted. This changes does the following in the delete_stack service method for legace engine: - if the stack is still locked, send thread cancel message - in a subthread wait for the lock to be released, or until a timeout based on the 4 minute cancel grace period - if the stack is still locked, do a thread stop as before Closes-Bug: #1499669 Closes-Bug: #1546431 Closes-Bug: #1536451 Change-Id: I4cd613681f07d295955c4d8a06505d72d83728a0 (cherry picked from commit 3000f904080d8dcd841d913dcd2ae658fb526c1a) --- heat/engine/service.py | 75 ++++++++++++++----- .../tests/engine/service/test_stack_delete.py | 61 ++++++++++----- 2 files changed, 98 insertions(+), 38 deletions(-) diff --git a/heat/engine/service.py b/heat/engine/service.py index cca3c3148..0a99e216a 100644 --- a/heat/engine/service.py +++ b/heat/engine/service.py @@ -1380,31 +1380,68 @@ class EngineService(service.Service): if acquire_result == self.engine_id: # give threads which are almost complete an opportunity to # finish naturally before force stopping them - eventlet.sleep(0.2) - self.thread_group_mgr.stop(stack.id) + self.thread_group_mgr.send(stack.id, rpc_api.THREAD_CANCEL) # Another active engine has the lock elif service_utils.engine_alive(cnxt, acquire_result): - stop_result = self._remote_call( - cnxt, acquire_result, self.listener.STOP_STACK, - stack_identity=stack_identity) - if stop_result is None: - LOG.debug("Successfully stopped remote task on engine %s" - % acquire_result) + cancel_result = self._remote_call( + cnxt, acquire_result, self.listener.SEND, + stack_identity=stack_identity, message=rpc_api.THREAD_CANCEL) + if cancel_result is None: + LOG.debug("Successfully sent %(msg)s message " + "to remote task on engine %(eng)s" % { + 'eng': acquire_result, + 'msg': rpc_api.THREAD_CANCEL}) else: - raise exception.StopActionFailed(stack_name=stack.name, - engine_id=acquire_result) + raise exception.EventSendFailed(stack_name=stack.name, + engine_id=acquire_result) - # There may be additional resources that we don't know about - # if an update was in-progress when the stack was stopped, so - # reload the stack from the database. - st = self._get_stack(cnxt, stack_identity) - stack = parser.Stack.load(cnxt, stack=st) - self.resource_enforcer.enforce_stack(stack) + def reload(): + st = self._get_stack(cnxt, stack_identity) + stack = parser.Stack.load(cnxt, stack=st) + self.resource_enforcer.enforce_stack(stack) + return stack - self.thread_group_mgr.start_with_lock(cnxt, stack, self.engine_id, - stack.delete) - return + def wait_then_delete(stack): + watch = timeutils.StopWatch(cfg.CONF.error_wait_time + 10) + watch.start() + + while not watch.expired(): + LOG.debug('Waiting for stack cancel to complete: %s' % + stack.name) + with lock.try_thread_lock() as acquire_result: + + if acquire_result is None: + stack = reload() + # do the actual delete with the aquired lock + self.thread_group_mgr.start_with_acquired_lock( + stack, lock, stack.delete) + return + eventlet.sleep(1.0) + + if acquire_result == self.engine_id: + # cancel didn't finish in time, attempt a stop instead + self.thread_group_mgr.stop(stack.id) + elif service_utils.engine_alive(cnxt, acquire_result): + # Another active engine has the lock + stop_result = self._remote_call( + cnxt, acquire_result, self.listener.STOP_STACK, + stack_identity=stack_identity) + if stop_result is None: + LOG.debug("Successfully stopped remote task " + "on engine %s" % acquire_result) + else: + raise exception.StopActionFailed( + stack_name=stack.name, engine_id=acquire_result) + + stack = reload() + # do the actual delete in a locked task + self.thread_group_mgr.start_with_lock(cnxt, stack, self.engine_id, + stack.delete) + + # Cancelling the stack could take some time, so do it in a task + self.thread_group_mgr.start(stack.id, wait_then_delete, + stack) @context.request_context def export_stack(self, cnxt, stack_identity): diff --git a/heat/tests/engine/service/test_stack_delete.py b/heat/tests/engine/service/test_stack_delete.py index 6ca22e71f..ab0dd282d 100644 --- a/heat/tests/engine/service/test_stack_delete.py +++ b/heat/tests/engine/service/test_stack_delete.py @@ -11,7 +11,9 @@ # under the License. import mock +from oslo_config import cfg from oslo_messaging.rpc import dispatcher +from oslo_utils import timeutils from heat.common import exception from heat.common import service_utils @@ -97,8 +99,11 @@ class StackDeleteTest(common.HeatTestCase): @mock.patch.object(parser.Stack, 'load') @mock.patch.object(stack_lock.StackLock, 'try_acquire') @mock.patch.object(stack_lock.StackLock, 'acquire') - def test_stack_delete_current_engine_active_lock(self, mock_acquire, - mock_try, mock_load): + @mock.patch.object(timeutils.StopWatch, 'expired') + def test_stack_delete_current_engine_active_lock(self, mock_expired, + mock_acquire, mock_try, + mock_load): + cfg.CONF.set_override('error_wait_time', 0) self.man.start() stack_name = 'service_delete_test_stack_current_active_lock' stack = tools.get_stack(stack_name, self.ctx) @@ -108,27 +113,32 @@ class StackDeleteTest(common.HeatTestCase): 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() st = stack_object.Stack.get_by_id(self.ctx, sid) mock_load.return_value = stack mock_try.return_value = self.man.engine_id mock_stop = self.patchobject(self.man.thread_group_mgr, 'stop') + mock_send = self.patchobject(self.man.thread_group_mgr, 'send') + mock_expired.side_effect = [False, True] self.assertIsNone(self.man.delete_stack(self.ctx, stack.identifier())) + self.man.thread_group_mgr.groups[sid].wait() mock_load.assert_called_with(self.ctx, stack=st) - self.assertEqual(2, len(mock_load.mock_calls)) - mock_try.assert_called_once_with() - mock_acquire.assert_called_once_with(True) + mock_send.assert_called_once_with(stack.id, 'cancel') mock_stop.assert_called_once_with(stack.id) + self.assertEqual(2, len(mock_load.mock_calls)) + mock_try.assert_called_with() + mock_acquire.assert_called_once_with(True) @mock.patch.object(parser.Stack, 'load') @mock.patch.object(stack_lock.StackLock, 'try_acquire') @mock.patch.object(service_utils, 'engine_alive') - def test_stack_delete_other_engine_active_lock_failed(self, mock_alive, - mock_try, mock_load): + @mock.patch.object(timeutils.StopWatch, 'expired') + def test_stack_delete_other_engine_active_lock_failed(self, mock_expired, + mock_alive, mock_try, + mock_load): + cfg.CONF.set_override('error_wait_time', 0) OTHER_ENGINE = "other-engine-fake-uuid" self.man.start() stack_name = 'service_delete_test_stack_other_engine_lock_fail' @@ -142,6 +152,7 @@ class StackDeleteTest(common.HeatTestCase): mock_load.return_value = stack mock_try.return_value = OTHER_ENGINE mock_alive.return_value = True + mock_expired.side_effect = [False, True] mock_call = self.patchobject(self.man, '_remote_call', return_value=False) @@ -149,20 +160,23 @@ class StackDeleteTest(common.HeatTestCase): ex = self.assertRaises(dispatcher.ExpectedException, self.man.delete_stack, self.ctx, stack.identifier()) - self.assertEqual(exception.StopActionFailed, ex.exc_info[0]) + self.assertEqual(exception.EventSendFailed, ex.exc_info[0]) mock_load.assert_called_once_with(self.ctx, stack=st) mock_try.assert_called_once_with() mock_alive.assert_called_once_with(self.ctx, OTHER_ENGINE) - mock_call.assert_called_once_with(self.ctx, OTHER_ENGINE, "stop_stack", + mock_call.assert_called_once_with(self.ctx, OTHER_ENGINE, "send", + message='cancel', stack_identity=mock.ANY) @mock.patch.object(parser.Stack, 'load') @mock.patch.object(stack_lock.StackLock, 'try_acquire') @mock.patch.object(service_utils, 'engine_alive') @mock.patch.object(stack_lock.StackLock, 'acquire') + @mock.patch.object(timeutils.StopWatch, 'expired') def test_stack_delete_other_engine_active_lock_succeeded( - self, mock_acquire, mock_alive, mock_try, mock_load): + self, mock_expired, mock_acquire, mock_alive, mock_try, mock_load): + cfg.CONF.set_override('error_wait_time', 0) OTHER_ENGINE = "other-engine-fake-uuid" self.man.start() @@ -177,6 +191,7 @@ class StackDeleteTest(common.HeatTestCase): mock_load.return_value = stack mock_try.return_value = OTHER_ENGINE mock_alive.return_value = True + mock_expired.side_effect = [False, True] mock_call = self.patchobject(self.man, '_remote_call', return_value=None) @@ -185,18 +200,25 @@ class StackDeleteTest(common.HeatTestCase): self.assertEqual(2, len(mock_load.mock_calls)) mock_load.assert_called_with(self.ctx, stack=st) - mock_try.assert_called_once_with() - mock_alive.assert_called_once_with(self.ctx, OTHER_ENGINE) - mock_call.assert_called_once_with(self.ctx, OTHER_ENGINE, "stop_stack", - stack_identity=mock.ANY) + mock_try.assert_called_with() + mock_alive.assert_called_with(self.ctx, OTHER_ENGINE) + mock_call.assert_has_calls([ + mock.call(self.ctx, OTHER_ENGINE, "send", + message='cancel', + stack_identity=mock.ANY), + mock.call(self.ctx, OTHER_ENGINE, "stop_stack", + stack_identity=mock.ANY) + ]) mock_acquire.assert_called_once_with(True) @mock.patch.object(parser.Stack, 'load') @mock.patch.object(stack_lock.StackLock, 'try_acquire') @mock.patch.object(service_utils, 'engine_alive') @mock.patch.object(stack_lock.StackLock, 'acquire') + @mock.patch.object(timeutils.StopWatch, 'expired') def test_stack_delete_other_dead_engine_active_lock( - self, mock_acquire, mock_alive, mock_try, mock_load): + self, mock_expired, mock_acquire, mock_alive, mock_try, mock_load): + cfg.CONF.set_override('error_wait_time', 0) OTHER_ENGINE = "other-engine-fake-uuid" stack_name = 'service_delete_test_stack_other_dead_engine' stack = tools.get_stack(stack_name, self.ctx) @@ -210,11 +232,12 @@ class StackDeleteTest(common.HeatTestCase): mock_load.return_value = stack mock_try.return_value = OTHER_ENGINE mock_alive.return_value = False + mock_expired.side_effect = [False, True] self.assertIsNone(self.man.delete_stack(self.ctx, stack.identifier())) self.man.thread_group_mgr.groups[sid].wait() mock_load.assert_called_with(self.ctx, stack=st) - mock_try.assert_called_once_with() + mock_try.assert_called_with() mock_acquire.assert_called_once_with(True) - mock_alive.assert_called_once_with(self.ctx, OTHER_ENGINE) + mock_alive.assert_called_with(self.ctx, OTHER_ENGINE) From c6bc3fef71a9c46a4109d6abe4d4d4923c7bdae9 Mon Sep 17 00:00:00 2001 From: Anant Patil Date: Fri, 16 Sep 2016 14:13:57 +0000 Subject: [PATCH 12/17] Re-trigger on update-replace It is found that the inter-leaving of lock when a update-replace of a resource is needed is the reason for new traversal not being triggered. Consider the order of events below: 1. A server is being updated. The worker locks the server resource. 2. A rollback is triggered because some one cancelled the stack. 3. As part of rollback, new update using old template is started. 4. The new update tries to take the lock but it has been already acquired in (1). The new update now expects that the when the old resource is done, it will re-trigger the new traversal. 5. The old update decides to create a new resource for replacement. The replacement resource is initiated for creation, a check_resource RPC call is made for new resource. 6. A worker, possibly in another engine, receives the call and then it bails out when it finds that there is a new traversal initiated (from 2). Now, there is no progress from here because it is expected (from 4) that there will be a re-trigger when the old resource is done. This change takes care of re-triggering the new traversal from worker when it finds that there is a new traversal and an update-replace. Note that this issue will not be seen when there is no update-replace because the old resource will finish (either fail or complete) and in the same thread it will find the new traversal and trigger it. Closes-Bug: #1625073 Change-Id: Icea5ba498ef8ca45cd85a9721937da2f4ac304e0 (cherry picked from commit 99b055b42357e2fae6006fe150c3c47c30dab1c0) --- heat/engine/check_resource.py | 10 +++---- heat/engine/worker.py | 37 ++++++++++++++++++------ heat/tests/engine/test_check_resource.py | 20 ++++++------- 3 files changed, 43 insertions(+), 24 deletions(-) diff --git a/heat/engine/check_resource.py b/heat/engine/check_resource.py index 1cd0bcaa7..4164e73a9 100644 --- a/heat/engine/check_resource.py +++ b/heat/engine/check_resource.py @@ -94,8 +94,8 @@ class CheckResource(object): latest_stack = parser.Stack.load(cnxt, stack_id=stack.id, force_reload=True) if traversal != latest_stack.current_traversal: - self._retrigger_check_resource(cnxt, is_update, rsrc_id, - latest_stack) + self.retrigger_check_resource(cnxt, is_update, rsrc_id, + latest_stack) def _handle_stack_timeout(self, cnxt, stack): failure_reason = u'Timed out' @@ -158,7 +158,7 @@ class CheckResource(object): return False - def _retrigger_check_resource(self, cnxt, is_update, resource_id, stack): + def retrigger_check_resource(self, cnxt, is_update, resource_id, stack): current_traversal = stack.current_traversal graph = stack.convergence_dependencies.graph() key = (resource_id, is_update) @@ -239,8 +239,8 @@ class CheckResource(object): current_traversal) return - self._retrigger_check_resource(cnxt, is_update, - resource_id, stack) + self.retrigger_check_resource(cnxt, is_update, + resource_id, stack) else: raise diff --git a/heat/engine/worker.py b/heat/engine/worker.py index 1a544a794..0d2d27d60 100644 --- a/heat/engine/worker.py +++ b/heat/engine/worker.py @@ -137,6 +137,23 @@ class WorkerService(service.Service): return True + def _retrigger_replaced(self, is_update, rsrc, stack, msg_queue): + graph = stack.convergence_dependencies.graph() + key = (rsrc.id, is_update) + if key not in graph and rsrc.replaces is not None: + # This resource replaces old one and is not needed in + # current traversal. You need to mark the resource as + # DELETED so that it gets cleaned up in purge_db. + values = {'action': rsrc.DELETE} + db_api.resource_update_and_save(stack.context, rsrc.id, values) + # The old resource might be in the graph (a rollback case); + # just re-trigger it. + key = (rsrc.replaces, is_update) + cr = check_resource.CheckResource(self.engine_id, self._rpc_client, + self.thread_group_mgr, msg_queue) + cr.retrigger_check_resource(stack.context, is_update, key[0], + stack) + @context.request_context def check_resource(self, cnxt, resource_id, current_traversal, data, is_update, adopt_stack_data): @@ -152,18 +169,20 @@ class WorkerService(service.Service): if rsrc is None: return - if current_traversal != stack.current_traversal: - LOG.debug('[%s] Traversal cancelled; stopping.', current_traversal) - return - msg_queue = eventlet.queue.LightQueue() try: self.thread_group_mgr.add_msg_queue(stack.id, msg_queue) - cr = check_resource.CheckResource(self.engine_id, self._rpc_client, - self.thread_group_mgr, msg_queue) - - cr.check(cnxt, resource_id, current_traversal, resource_data, - is_update, adopt_stack_data, rsrc, stack) + if current_traversal != stack.current_traversal: + LOG.debug('[%s] Traversal cancelled; re-trigerring.', + current_traversal) + self._retrigger_replaced(is_update, rsrc, stack, msg_queue) + else: + cr = check_resource.CheckResource(self.engine_id, + self._rpc_client, + self.thread_group_mgr, + msg_queue) + cr.check(cnxt, resource_id, current_traversal, resource_data, + is_update, adopt_stack_data, rsrc, stack) finally: self.thread_group_mgr.remove_msg_queue(None, stack.id, msg_queue) diff --git a/heat/tests/engine/test_check_resource.py b/heat/tests/engine/test_check_resource.py index bfe405398..5d8054df5 100644 --- a/heat/tests/engine/test_check_resource.py +++ b/heat/tests/engine/test_check_resource.py @@ -77,12 +77,12 @@ class CheckWorkflowUpdateTest(common.HeatTestCase): for mocked in [mock_cru, mock_crc, mock_pcr, mock_csc, mock_cid]: self.assertFalse(mocked.called) + @mock.patch.object(worker.WorkerService, '_retrigger_replaced') def test_stale_traversal( - self, mock_cru, mock_crc, mock_pcr, mock_csc, mock_cid): + self, mock_rnt, mock_cru, mock_crc, mock_pcr, mock_csc, mock_cid): self.worker.check_resource(self.ctx, self.resource.id, 'stale-traversal', {}, True, None) - for mocked in [mock_cru, mock_crc, mock_pcr, mock_csc, mock_cid]: - self.assertFalse(mocked.called) + self.assertTrue(mock_rnt.called) def test_is_update_traversal( self, mock_cru, mock_crc, mock_pcr, mock_csc, mock_cid): @@ -320,7 +320,7 @@ class CheckWorkflowUpdateTest(common.HeatTestCase): self.assertTrue(self.stack.purge_db.called) @mock.patch.object(check_resource.CheckResource, - '_retrigger_check_resource') + 'retrigger_check_resource') @mock.patch.object(stack.Stack, 'load') def test_initiate_propagate_rsrc_retriggers_check_rsrc_on_new_stack_update( self, mock_stack_load, mock_rcr, mock_cru, mock_crc, mock_pcr, @@ -368,8 +368,8 @@ class CheckWorkflowUpdateTest(common.HeatTestCase): # A, B are predecessors to C when is_update is True expected_predecessors = {(self.stack['A'].id, True), (self.stack['B'].id, True)} - self.cr._retrigger_check_resource(self.ctx, self.is_update, - resC.id, self.stack) + self.cr.retrigger_check_resource(self.ctx, self.is_update, + resC.id, self.stack) mock_pcr.assert_called_once_with(self.ctx, mock.ANY, resC.id, self.stack.current_traversal, mock.ANY, (resC.id, True), None, @@ -386,7 +386,7 @@ class CheckWorkflowUpdateTest(common.HeatTestCase): [(1, False), (1, True)], [(2, False), None]]) # simulate rsrc 2 completing its update for old traversal # and calling rcr - self.cr._retrigger_check_resource(self.ctx, True, 2, self.stack) + self.cr.retrigger_check_resource(self.ctx, True, 2, self.stack) # Ensure that pcr was called with proper delete traversal mock_pcr.assert_called_once_with(self.ctx, mock.ANY, 2, self.stack.current_traversal, @@ -401,7 +401,7 @@ class CheckWorkflowUpdateTest(common.HeatTestCase): [(1, False), (1, True)], [(2, False), (2, True)]]) # simulate rsrc 2 completing its delete for old traversal # and calling rcr - self.cr._retrigger_check_resource(self.ctx, False, 2, self.stack) + self.cr.retrigger_check_resource(self.ctx, False, 2, self.stack) # Ensure that pcr was called with proper delete traversal mock_pcr.assert_called_once_with(self.ctx, mock.ANY, 2, self.stack.current_traversal, @@ -426,7 +426,7 @@ class CheckWorkflowUpdateTest(common.HeatTestCase): @mock.patch.object(stack.Stack, 'purge_db') @mock.patch.object(stack.Stack, 'state_set') @mock.patch.object(check_resource.CheckResource, - '_retrigger_check_resource') + 'retrigger_check_resource') @mock.patch.object(check_resource.CheckResource, '_trigger_rollback') def test_handle_rsrc_failure_when_update_fails( self, mock_tr, mock_rcr, mock_ss, mock_pdb, mock_cru, mock_crc, @@ -444,7 +444,7 @@ class CheckWorkflowUpdateTest(common.HeatTestCase): @mock.patch.object(stack.Stack, 'purge_db') @mock.patch.object(stack.Stack, 'state_set') @mock.patch.object(check_resource.CheckResource, - '_retrigger_check_resource') + 'retrigger_check_resource') @mock.patch.object(check_resource.CheckResource, '_trigger_rollback') def test_handle_rsrc_failure_when_update_fails_different_traversal( self, mock_tr, mock_rcr, mock_ss, mock_pdb, mock_cru, mock_crc, From 9ece778613d67feff35342aeaa6618aaa5f6fc85 Mon Sep 17 00:00:00 2001 From: Oleksii Chuprykov Date: Fri, 29 Apr 2016 17:03:17 +0300 Subject: [PATCH 13/17] Fix cancel update for nova server with defined port This particular patch fixes a behaviour of cancel update for nova server with defined port, so there are no ports manageable by nova. We have these issues while restoring ports after rollback: 1) We doesn't detach any ports from current server, because we doesn't save them to resoruce data. (we store this data after succesfull create of the server) 2) Detaching an interface from current server will fail, if the server will be in building state, so we need to wait until server will be in active or in error state. Refresh ports list to solve problem (1). Wait until nova moves to active/error state to solve (2). A functional test to prove the fix was added. Note, that this test is skipped for convergence engine tests until cancel update will work properly in convergence mode (see bug 1533176). Partial-Bug: #1570908 Change-Id: If6fd916068a425eea6dc795192f286cb5ffcb794 (cherry picked from commit 584efe3329143e28fdb42b5d9496977f5cdf275a) --- .../openstack/nova/server_network_mixin.py | 116 +++++++++--------- heat/tests/openstack/nova/test_server.py | 21 +++- heat_integrationtests/common/test.py | 19 +++ .../functional/test_cancel_update.py | 63 ++++++++++ 4 files changed, 159 insertions(+), 60 deletions(-) create mode 100644 heat_integrationtests/functional/test_cancel_update.py diff --git a/heat/engine/resources/openstack/nova/server_network_mixin.py b/heat/engine/resources/openstack/nova/server_network_mixin.py index 9def1a221..87b5cb42b 100644 --- a/heat/engine/resources/openstack/nova/server_network_mixin.py +++ b/heat/engine/resources/openstack/nova/server_network_mixin.py @@ -13,6 +13,7 @@ import itertools +import eventlet from oslo_log import log as logging from oslo_serialization import jsonutils from oslo_utils import netutils @@ -21,9 +22,7 @@ import retrying from heat.common import exception from heat.common.i18n import _ from heat.common.i18n import _LI - from heat.engine import resource - from heat.engine.resources.openstack.neutron import port as neutron_port LOG = logging.getLogger(__name__) @@ -413,6 +412,46 @@ class ServerNetworkMixin(object): elif not self.is_using_neutron(): self._floating_ip_nova_associate(floating_ip) + @staticmethod + def get_all_ports(server): + return itertools.chain( + server._data_get_ports(), + server._data_get_ports('external_ports') + ) + + def detach_ports(self, server): + existing_server_id = server.resource_id + for port in self.get_all_ports(server): + self.client_plugin().interface_detach( + existing_server_id, port['id']) + try: + if self.client_plugin().check_interface_detach( + existing_server_id, port['id']): + LOG.info(_LI('Detach interface %(port)s successful from ' + 'server %(server)s.') + % {'port': port['id'], + 'server': existing_server_id}) + except retrying.RetryError: + raise exception.InterfaceDetachFailed( + port=port['id'], server=existing_server_id) + + def attach_ports(self, server): + prev_server_id = server.resource_id + + for port in self.get_all_ports(server): + self.client_plugin().interface_attach(prev_server_id, + port['id']) + try: + if self.client_plugin().check_interface_attach( + prev_server_id, port['id']): + LOG.info(_LI('Attach interface %(port)s successful to ' + 'server %(server)s') + % {'port': port['id'], + 'server': prev_server_id}) + except retrying.RetryError: + raise exception.InterfaceAttachFailed( + port=port['id'], server=prev_server_id) + def prepare_ports_for_replace(self): if not self.is_using_neutron(): return @@ -426,21 +465,7 @@ class ServerNetworkMixin(object): for port_type, port in port_data: data[port_type].append({'id': port['id']}) - # detach the ports from the server - server_id = self.resource_id - for port_type, port in port_data: - self.client_plugin().interface_detach(server_id, port['id']) - try: - if self.client_plugin().check_interface_detach( - server_id, port['id']): - LOG.info(_LI('Detach interface %(port)s successful ' - 'from server %(server)s when prepare ' - 'for replace.') - % {'port': port['id'], - 'server': server_id}) - except retrying.RetryError: - raise exception.InterfaceDetachFailed( - port=port['id'], server=server_id) + self.detach_ports(self) def restore_ports_after_rollback(self, convergence): if not self.is_using_neutron(): @@ -460,46 +485,23 @@ class ServerNetworkMixin(object): else: existing_server = self - port_data = itertools.chain( - existing_server._data_get_ports(), - existing_server._data_get_ports('external_ports') - ) - - existing_server_id = existing_server.resource_id - for port in port_data: - # detach the ports from current resource - self.client_plugin().interface_detach( - existing_server_id, port['id']) + # Wait until server will move to active state. We can't + # detach interfaces from server in BUILDING state. + # In case of convergence, the replacement resource may be + # created but never have been worked on because the rollback was + # trigerred or new update was trigerred. + if existing_server.resource_id is not None: try: - if self.client_plugin().check_interface_detach( - existing_server_id, port['id']): - LOG.info(_LI('Detach interface %(port)s successful from ' - 'server %(server)s when restore after ' - 'rollback.') - % {'port': port['id'], - 'server': existing_server_id}) - except retrying.RetryError: - raise exception.InterfaceDetachFailed( - port=port['id'], server=existing_server_id) + while True: + active = self.client_plugin()._check_active( + existing_server.resource_id) + if active: + break + eventlet.sleep(1) + except exception.ResourceInError: + pass - # attach the ports for old resource - prev_port_data = itertools.chain( - prev_server._data_get_ports(), - prev_server._data_get_ports('external_ports')) + self.store_external_ports() + self.detach_ports(existing_server) - prev_server_id = prev_server.resource_id - - for port in prev_port_data: - self.client_plugin().interface_attach(prev_server_id, - port['id']) - try: - if self.client_plugin().check_interface_attach( - prev_server_id, port['id']): - LOG.info(_LI('Attach interface %(port)s successful to ' - 'server %(server)s when restore after ' - 'rollback.') - % {'port': port['id'], - 'server': prev_server_id}) - except retrying.RetryError: - raise exception.InterfaceAttachFailed( - port=port['id'], server=prev_server_id) + self.attach_ports(prev_server) diff --git a/heat/tests/openstack/nova/test_server.py b/heat/tests/openstack/nova/test_server.py index a0461df65..97258f87e 100644 --- a/heat/tests/openstack/nova/test_server.py +++ b/heat/tests/openstack/nova/test_server.py @@ -35,6 +35,7 @@ from heat.engine.clients.os import zaqar from heat.engine import environment from heat.engine import resource from heat.engine.resources.openstack.nova import server as servers +from heat.engine.resources.openstack.nova import server_network_mixin from heat.engine.resources import scheduler_hints as sh from heat.engine import scheduler from heat.engine import stack as parser @@ -4395,7 +4396,9 @@ class ServerInternalPortTest(common.HeatTestCase): mock.call('test_server', 3344), mock.call('test_server', 5566)]) - def test_restore_ports_after_rollback(self): + @mock.patch.object(server_network_mixin.ServerNetworkMixin, + 'store_external_ports') + def test_restore_ports_after_rollback(self, store_ports): t, stack, server = self._return_template_stack_and_rsrc_defn( 'test', tmpl_server_with_network_id) server.resource_id = 'existing_server' @@ -4403,6 +4406,8 @@ class ServerInternalPortTest(common.HeatTestCase): external_port_ids = [{'id': 5566}] server._data = {"internal_ports": jsonutils.dumps(port_ids), "external_ports": jsonutils.dumps(external_port_ids)} + self.patchobject(nova.NovaClientPlugin, '_check_active') + nova.NovaClientPlugin._check_active.side_effect = [False, True] # add data to old server in backup stack old_server = mock.Mock() @@ -4420,6 +4425,8 @@ class ServerInternalPortTest(common.HeatTestCase): server.restore_prev_rsrc() + self.assertEqual(2, nova.NovaClientPlugin._check_active.call_count) + # check, that ports were detached from new server nova.NovaClientPlugin.interface_detach.assert_has_calls([ mock.call('existing_server', 1122), @@ -4432,12 +4439,16 @@ class ServerInternalPortTest(common.HeatTestCase): mock.call('old_server', 3344), mock.call('old_server', 5566)]) - def test_restore_ports_after_rollback_attach_failed(self): + @mock.patch.object(server_network_mixin.ServerNetworkMixin, + 'store_external_ports') + def test_restore_ports_after_rollback_attach_failed(self, store_ports): t, stack, server = self._return_template_stack_and_rsrc_defn( 'test', tmpl_server_with_network_id) server.resource_id = 'existing_server' port_ids = [{'id': 1122}, {'id': 3344}] server._data = {"internal_ports": jsonutils.dumps(port_ids)} + self.patchobject(nova.NovaClientPlugin, '_check_active') + nova.NovaClientPlugin._check_active.return_value = True # add data to old server in backup stack old_server = mock.Mock() @@ -4465,10 +4476,14 @@ class ServerInternalPortTest(common.HeatTestCase): '(old_server)', six.text_type(exc)) - def test_restore_ports_after_rollback_convergence(self): + @mock.patch.object(server_network_mixin.ServerNetworkMixin, + 'store_external_ports') + def test_restore_ports_after_rollback_convergence(self, store_ports): t = template_format.parse(tmpl_server_with_network_id) stack = utils.parse_stack(t) stack.store() + self.patchobject(nova.NovaClientPlugin, '_check_active') + nova.NovaClientPlugin._check_active.return_value = True # mock resource from previous template prev_rsrc = stack['server'] diff --git a/heat_integrationtests/common/test.py b/heat_integrationtests/common/test.py index a7aec6c47..b9bb0362c 100644 --- a/heat_integrationtests/common/test.py +++ b/heat_integrationtests/common/test.py @@ -411,6 +411,25 @@ class HeatIntegrationTest(testscenarios.WithScenarios, self._wait_for_stack_status(**kwargs) + def cancel_update_stack(self, stack_identifier, + expected_status='ROLLBACK_COMPLETE'): + + stack_name = stack_identifier.split('/')[0] + + self.updated_time[stack_identifier] = self.client.stacks.get( + stack_identifier, resolve_outputs=False).updated_time + + self.client.actions.cancel_update(stack_name) + + kwargs = {'stack_identifier': stack_identifier, + 'status': expected_status} + if expected_status in ['ROLLBACK_COMPLETE']: + # To trigger rollback you would intentionally fail the stack + # Hence check for rollback failures + kwargs['failure_pattern'] = '^ROLLBACK_FAILED$' + + self._wait_for_stack_status(**kwargs) + def preview_update_stack(self, stack_identifier, template, environment=None, files=None, parameters=None, tags=None, disable_rollback=True, diff --git a/heat_integrationtests/functional/test_cancel_update.py b/heat_integrationtests/functional/test_cancel_update.py new file mode 100644 index 000000000..c40aeb0a0 --- /dev/null +++ b/heat_integrationtests/functional/test_cancel_update.py @@ -0,0 +1,63 @@ +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +from heat_integrationtests.functional import functional_base + + +class CancelUpdateTest(functional_base.FunctionalTestsBase): + + template = ''' +heat_template_version: '2013-05-23' +parameters: + InstanceType: + type: string + ImageId: + type: string + network: + type: string +resources: + port: + type: OS::Neutron::Port + properties: + network: {get_param: network} + Server: + type: OS::Nova::Server + properties: + flavor_update_policy: REPLACE + image: {get_param: ImageId} + flavor: {get_param: InstanceType} + networks: + - port: {get_resource: port} +''' + + def setUp(self): + super(CancelUpdateTest, self).setUp() + if not self.conf.image_ref: + raise self.skipException("No image configured to test.") + if not self.conf.instance_type: + raise self.skipException("No flavor configured to test.") + if not self.conf.minimal_instance_type: + raise self.skipException("No minimal flavor configured to test.") + + def test_cancel_update_server_with_port(self): + parameters = {'InstanceType': self.conf.minimal_instance_type, + 'ImageId': self.conf.image_ref, + 'network': self.conf.fixed_network_name} + + stack_identifier = self.stack_create(template=self.template, + parameters=parameters) + parameters['InstanceType'] = 'm1.large' + self.update_stack(stack_identifier, self.template, + parameters=parameters, + expected_status='UPDATE_IN_PROGRESS') + + self.cancel_update_stack(stack_identifier) From a96d89b2005dcef2f06b6ae55260fdb0f358abab Mon Sep 17 00:00:00 2001 From: Zane Bitter Date: Wed, 21 Sep 2016 18:37:04 -0400 Subject: [PATCH 14/17] Don't use cast() to do StackResource delete If an exception was raised in delete_stack when deleting a nested stack, the parent stack would never hear about it because we were accidentally using cast() instead of call() to do the stack delete. This meant the parent resource would remain DELETE_IN_PROGRESS until timeout when the nested stack had already failed and raised an exception. In the case of bug 1499669, the exception being missed was StopActionFailed. Change-Id: I039eb8f6c6a262653c1e9edc8173e5680d81e31b Partial-Bug: #1499669 (cherry picked from commit e5cec71e52c3fed0ffb4385990758db8ebf367da) --- heat/engine/resources/stack_resource.py | 5 +++-- heat/tests/test_nested_stack.py | 2 +- heat/tests/test_provider_template.py | 3 ++- heat/tests/test_stack_resource.py | 2 +- 4 files changed, 7 insertions(+), 5 deletions(-) diff --git a/heat/engine/resources/stack_resource.py b/heat/engine/resources/stack_resource.py index 751c6c159..2978879a9 100644 --- a/heat/engine/resources/stack_resource.py +++ b/heat/engine/resources/stack_resource.py @@ -524,9 +524,10 @@ class StackResource(resource.Resource): if self.abandon_in_progress: self.rpc_client().abandon_stack(self.context, stack_identity) else: - self.rpc_client().delete_stack(self.context, stack_identity) + self.rpc_client().delete_stack(self.context, stack_identity, + cast=False) except Exception as ex: - self.rpc_client().ignore_error_named(ex, 'NotFound') + self.rpc_client().ignore_error_named(ex, 'EntityNotFound') def handle_delete(self): return self.delete_nested() diff --git a/heat/tests/test_nested_stack.py b/heat/tests/test_nested_stack.py index 77d5d4081..24c8eaf12 100644 --- a/heat/tests/test_nested_stack.py +++ b/heat/tests/test_nested_stack.py @@ -413,4 +413,4 @@ Outputs: self.res.nested().identifier.return_value = stack_identity self.res.handle_delete() self.res.rpc_client.return_value.delete_stack.assert_called_once_with( - self.ctx, self.res.nested().identifier()) + self.ctx, self.res.nested().identifier(), cast=False) diff --git a/heat/tests/test_provider_template.py b/heat/tests/test_provider_template.py index 7e48f341c..6148681c6 100644 --- a/heat/tests/test_provider_template.py +++ b/heat/tests/test_provider_template.py @@ -1021,4 +1021,5 @@ class TemplateResourceCrudTest(common.HeatTestCase): rpcc = self.res.rpc_client.return_value rpcc.delete_stack.assert_called_once_with( self.ctx, - self.res.nested().identifier()) + self.res.nested().identifier(), + cast=False) diff --git a/heat/tests/test_stack_resource.py b/heat/tests/test_stack_resource.py index f8e506ce8..c809d27d8 100644 --- a/heat/tests/test_stack_resource.py +++ b/heat/tests/test_stack_resource.py @@ -514,7 +514,7 @@ class StackResourceTest(StackResourceBaseTest): side_effect=exception.NotFound()) self.assertIsNone(self.parent_resource.delete_nested()) rpcc.return_value.delete_stack.assert_called_once_with( - self.parent_resource.context, mock.ANY) + self.parent_resource.context, mock.ANY, cast=False) def test_need_update_for_nested_resource(self): """Test the resource with nested stack should need update. From fb26ec9da5ee6fd051d3d0b39588972ecc86f1d9 Mon Sep 17 00:00:00 2001 From: Crag Wolfe Date: Wed, 21 Sep 2016 19:06:36 -0400 Subject: [PATCH 15/17] Do not attempt deletion of a DELETE_COMPLETE stack in service api A stack may be in transient state where it is DELETE_COMPLETE, but has has not actually been soft-deleted yet. For the purposes of delete_stack in service.py, consider a DELETE_COMPLETE stack as equivalent to a soft-deleted one (it soon will be), thereby avoiding a race where we would have attempted to update the stack, running into a foreign-key constraint issue for a non-existing user_cred. Change-Id: Iec021e6a0df262d447fdf9ee1789603c7a1c55f8 Closes-Bug: #1626173 Closes-Bug: #1626107 (cherry picked from commit e1f161a19a9eef3edb05612aad905e2e7ae7c674) --- heat/engine/service.py | 4 ++++ heat/tests/test_engine_service.py | 11 +++++++++++ 2 files changed, 15 insertions(+) diff --git a/heat/engine/service.py b/heat/engine/service.py index 8bee77aec..bd3c807c6 100644 --- a/heat/engine/service.py +++ b/heat/engine/service.py @@ -1358,6 +1358,10 @@ class EngineService(service.Service): """ st = self._get_stack(cnxt, stack_identity) + if (st.status == parser.Stack.COMPLETE and + st.action == parser.Stack.DELETE): + raise exception.EntityNotFound(entity='Stack', name=st.name) + LOG.info(_LI('Deleting stack %s'), st.name) stack = parser.Stack.load(cnxt, stack=st) self.resource_enforcer.enforce_stack(stack) diff --git a/heat/tests/test_engine_service.py b/heat/tests/test_engine_service.py index 118683bae..e58d03dad 100644 --- a/heat/tests/test_engine_service.py +++ b/heat/tests/test_engine_service.py @@ -975,6 +975,17 @@ class StackServiceTest(common.HeatTestCase): outputs = self.eng.list_outputs(self.ctx, mock.ANY) self.assertEqual([], outputs) + def test_stack_delete_complete_is_not_found(self): + mock_get_stack = self.patchobject(self.eng, '_get_stack') + mock_get_stack.return_value = mock.MagicMock() + mock_get_stack.return_value.status = parser.Stack.COMPLETE + mock_get_stack.return_value.action = parser.Stack.DELETE + ex = self.assertRaises(dispatcher.ExpectedException, + self.eng.delete_stack, + 'irrelevant', + 'irrelevant') + self.assertEqual(exception.EntityNotFound, ex.exc_info[0]) + def test_get_environment(self): # Setup t = template_format.parse(tools.wp_template) From ee86435e44065f9d59425023b1b8220826a6e0f2 Mon Sep 17 00:00:00 2001 From: Zane Bitter Date: Thu, 22 Sep 2016 09:44:56 -0400 Subject: [PATCH 16/17] Increase the timeout for the stop_stack message Previously, the stop_stack message accidentally used the engine_life_check_timeout (by default, 2s). But unlike other messages sent using that timeout, stop_stack needs to synchronously kill all running threads operating on the stack. For a very large stack, this can easily take much longer than a couple of seconds. This patch increases the timeout to give a better chance of being able to start the delete. Change-Id: I4b36ed7f1025b6439aeab63d71041bb2000363a0 Closes-Bug: #1499669 (cherry picked from commit e56fc689e19d92b5a3d23736d472c9a1fc698537) --- heat/engine/service.py | 16 ++++--- .../tests/engine/service/test_stack_delete.py | 7 ++-- .../functional/test_delete.py | 42 +++++++++++++++++++ 3 files changed, 57 insertions(+), 8 deletions(-) create mode 100644 heat_integrationtests/functional/test_delete.py diff --git a/heat/engine/service.py b/heat/engine/service.py index bd3c807c6..bcd7801b5 100644 --- a/heat/engine/service.py +++ b/heat/engine/service.py @@ -79,6 +79,10 @@ cfg.CONF.import_opt('enable_stack_abandon', 'heat.common.config') cfg.CONF.import_opt('enable_stack_adopt', 'heat.common.config') cfg.CONF.import_opt('convergence_engine', 'heat.common.config') +# Time to wait for a stack to stop when cancelling running threads, before +# giving up on being able to start a delete. +STOP_STACK_TIMEOUT = 30 + LOG = logging.getLogger(__name__) @@ -1147,7 +1151,8 @@ class EngineService(service.Service): # Another active engine has the lock elif service_utils.engine_alive(cnxt, engine_id): cancel_result = self._remote_call( - cnxt, engine_id, self.listener.SEND, + cnxt, engine_id, cfg.CONF.engine_life_check_timeout, + self.listener.SEND, stack_identity=stack_identity, message=cancel_message) if cancel_result is None: LOG.debug("Successfully sent %(msg)s message " @@ -1337,8 +1342,7 @@ class EngineService(service.Service): return api.format_stack_output(outputs[output_key]) - def _remote_call(self, cnxt, lock_engine_id, call, **kwargs): - timeout = cfg.CONF.engine_life_check_timeout + def _remote_call(self, cnxt, lock_engine_id, timeout, call, **kwargs): self.cctxt = self._client.prepare( version='1.0', timeout=timeout, @@ -1396,7 +1400,8 @@ class EngineService(service.Service): # Another active engine has the lock elif service_utils.engine_alive(cnxt, acquire_result): cancel_result = self._remote_call( - cnxt, acquire_result, self.listener.SEND, + cnxt, acquire_result, cfg.CONF.engine_life_check_timeout, + self.listener.SEND, stack_identity=stack_identity, message=rpc_api.THREAD_CANCEL) if cancel_result is None: LOG.debug("Successfully sent %(msg)s message " @@ -1436,7 +1441,8 @@ class EngineService(service.Service): elif service_utils.engine_alive(cnxt, acquire_result): # Another active engine has the lock stop_result = self._remote_call( - cnxt, acquire_result, self.listener.STOP_STACK, + cnxt, acquire_result, STOP_STACK_TIMEOUT, + self.listener.STOP_STACK, stack_identity=stack_identity) if stop_result is None: LOG.debug("Successfully stopped remote task " diff --git a/heat/tests/engine/service/test_stack_delete.py b/heat/tests/engine/service/test_stack_delete.py index ab0dd282d..418c9018e 100644 --- a/heat/tests/engine/service/test_stack_delete.py +++ b/heat/tests/engine/service/test_stack_delete.py @@ -165,7 +165,8 @@ class StackDeleteTest(common.HeatTestCase): mock_load.assert_called_once_with(self.ctx, stack=st) mock_try.assert_called_once_with() mock_alive.assert_called_once_with(self.ctx, OTHER_ENGINE) - mock_call.assert_called_once_with(self.ctx, OTHER_ENGINE, "send", + mock_call.assert_called_once_with(self.ctx, OTHER_ENGINE, mock.ANY, + "send", message='cancel', stack_identity=mock.ANY) @@ -203,10 +204,10 @@ class StackDeleteTest(common.HeatTestCase): mock_try.assert_called_with() mock_alive.assert_called_with(self.ctx, OTHER_ENGINE) mock_call.assert_has_calls([ - mock.call(self.ctx, OTHER_ENGINE, "send", + mock.call(self.ctx, OTHER_ENGINE, mock.ANY, "send", message='cancel', stack_identity=mock.ANY), - mock.call(self.ctx, OTHER_ENGINE, "stop_stack", + mock.call(self.ctx, OTHER_ENGINE, mock.ANY, "stop_stack", stack_identity=mock.ANY) ]) mock_acquire.assert_called_once_with(True) diff --git a/heat_integrationtests/functional/test_delete.py b/heat_integrationtests/functional/test_delete.py new file mode 100644 index 000000000..92b1c7451 --- /dev/null +++ b/heat_integrationtests/functional/test_delete.py @@ -0,0 +1,42 @@ +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +import time + +from heat_integrationtests.functional import functional_base + + +class DeleteInProgressTest(functional_base.FunctionalTestsBase): + + root_template = ''' +heat_template_version: 2013-05-23 +resources: + rg: + type: OS::Heat::ResourceGroup + properties: + count: 125 + resource_def: + type: empty.yaml +''' + + empty_template = ''' +heat_template_version: 2013-05-23 +resources: +''' + + def test_delete_nested_stacks_create_in_progress(self): + files = {'empty.yaml': self.empty_template} + identifier = self.stack_create(template=self.root_template, + files=files, + expected_status='CREATE_IN_PROGRESS') + time.sleep(20) + self._stack_delete(identifier) From e59f9275625e6757bc619161caf6b3e9339e8887 Mon Sep 17 00:00:00 2001 From: Zane Bitter Date: Thu, 22 Sep 2016 17:16:26 -0400 Subject: [PATCH 17/17] Get rid of circular reference in Event class This would have been causing the entire stack to remain in memory until garbage-collected. We only need the identifier, so store that instead. Change-Id: If965b4415d7640b93edd153f2893a7e0c04bc8d6 Partial-Bug: #1626675 (cherry picked from commit 82b8fd8c17d94e5c18ca3bdbca94e60978d1952b) --- heat/engine/event.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/heat/engine/event.py b/heat/engine/event.py index 1aedfa511..35b5f2040 100644 --- a/heat/engine/event.py +++ b/heat/engine/event.py @@ -37,7 +37,7 @@ class Event(object): already in the database. """ self.context = context - self.stack = stack + self._stack_identifier = stack.identifier() self.action = action self.status = status self.reason = reason @@ -57,7 +57,7 @@ class Event(object): ev = { 'resource_name': self.resource_name, 'physical_resource_id': self.physical_resource_id, - 'stack_id': self.stack.id, + 'stack_id': self._stack_identifier.stack_id, 'resource_action': self.action, 'resource_status': self.status, 'resource_status_reason': self.reason, @@ -114,7 +114,7 @@ class Event(object): return None res_id = identifier.ResourceIdentifier( - resource_name=self.resource_name, **self.stack.identifier()) + resource_name=self.resource_name, **self._stack_identifier) return identifier.EventIdentifier(event_id=str(self.uuid), **res_id) @@ -127,7 +127,7 @@ class Event(object): 'payload': { 'resource_name': self.resource_name, 'physical_resource_id': self.physical_resource_id, - 'stack_id': self.stack.id, + 'stack_id': self._stack_identifier.stack_id, 'resource_action': self.action, 'resource_status': self.status, 'resource_status_reason': self.reason,