openstack
/
heat
Code Issues Proposed changes

Avoid creating new resource with old template 

If a traversal is interrupted by a fresh update before a particular
resource is created, then the resource is left stored in the DB with the
old template ID. While an update always uses the new template, a create
assumes that the template ID in the DB is correct. Since the resource has
never been created, the new traversal will create it using the old
template.

To resolve this, detect the case where the resource has not been created
yet and we are about to create it and the traversal ID is still current,
and always use the new resource definition in that case.

Change-Id: Ifa0ce9e1e08f86b30df00d92488301ea05b45b14
Closes-Bug: #1663745
This commit is contained in:
Zane Bitter 2017-06-05 23:14:19 -04:00
parent b9dd705fe1
commit 5681e237c5
8 changed files with 57 additions and 23 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
heat/engine/check_resource.py
View File

@ -287,9 +287,10 @@ class CheckResource(object):
rsrc, stack)
def load_resource(cnxt, resource_id, resource_data, is_update):
def load_resource(cnxt, resource_id, resource_data,
current_traversal, is_update):
try:
return resource.Resource.load(cnxt, resource_id,
return resource.Resource.load(cnxt, resource_id, current_traversal,
is_update, resource_data)
except (exception.ResourceNotFound, exception.NotFound):
# can be ignored

7
heat/engine/resource.py
View File

@ -322,14 +322,17 @@ class Resource(status.ResourceStatus):
self._stackref = weakref.ref(stack)
@classmethod
def load(cls, context, resource_id, is_update, data):
def load(cls, context, resource_id, current_traversal, is_update, data):
from heat.engine import stack as stack_mod
db_res = resource_objects.Resource.get_obj(context, resource_id)
curr_stack = stack_mod.Stack.load(context, stack_id=db_res.stack_id,
cache_data=data)
resource_owning_stack = curr_stack
if db_res.current_template_id != curr_stack.t.id:
if (db_res.current_template_id != curr_stack.t.id and
(db_res.action != cls.INIT or
not is_update or
current_traversal != curr_stack.current_traversal)):
# load stack with template owning the resource
db_stack = stack_objects.Stack.get_by_id(context, db_res.stack_id)
db_stack.raw_template = None

16
heat/engine/resources/openstack/neutron/port.py
View File

@ -558,23 +558,25 @@ class Port(neutron.NeutronResource):
def restore_prev_rsrc(self, convergence=False):
# In case of convergence, during rollback, the previous rsrc is
# already selected and is being acted upon.
backup_stack = self.stack._backup_stack()
backup_res = backup_stack.resources.get(self.name)
prev_port = self if convergence else backup_res
fixed_ips = prev_port.data().get('port_fip', [])
props = {'fixed_ips': []}
if convergence:
prev_port = self
existing_port, rsrc_owning_stack, stack = resource.Resource.load(
prev_port.context, prev_port.replaced_by, True,
prev_port.context, prev_port.replaced_by,
prev_port.stack.current_traversal, True,
prev_port.stack.cache_data
)
existing_port_id = existing_port.resource_id
else:
backup_stack = self.stack._backup_stack()
prev_port = backup_stack.resources.get(self.name)
existing_port_id = self.resource_id
if existing_port_id:
# reset fixed_ips to [] for new resource
props = {'fixed_ips': []}
self.client().update_port(existing_port_id, {'port': props})
fixed_ips = prev_port.data().get('port_fip', [])
if fixed_ips and prev_port.resource_id:
# restore ip for old port
prev_port_props = {'fixed_ips': jsonutils.loads(fixed_ips)}

10
heat/engine/resources/openstack/nova/server_network_mixin.py
View File

@ -522,17 +522,17 @@ class ServerNetworkMixin(object):
def restore_ports_after_rollback(self, convergence):
# In case of convergence, during rollback, the previous rsrc is
# already selected and is being acted upon.
backup_stack = self.stack._backup_stack()
backup_res = backup_stack.resources.get(self.name)
prev_server = self if convergence else backup_res
if convergence:
prev_server = self
rsrc, rsrc_owning_stack, stack = resource.Resource.load(
prev_server.context, prev_server.replaced_by, True,
prev_server.context, prev_server.replaced_by,
prev_server.stack.current_traversal, True,
prev_server.stack.cache_data
)
existing_server = rsrc
else:
backup_stack = self.stack._backup_stack()
prev_server = backup_stack.resources.get(self.name)
existing_server = self
# Wait until server will move to active state. We can't

2
heat/engine/worker.py
View File

@ -158,7 +158,7 @@ class WorkerService(object):
resource_data = node_data.load_resources_data(in_data if is_update
else {})
rsrc, rsrc_owning_stack, stack = check_resource.load_resource(
cnxt, resource_id, resource_data, is_update)
cnxt, resource_id, resource_data, current_traversal, is_update)
if rsrc is None:
return

22
heat/tests/convergence/scenarios/update_interrupt_create.py Normal file
View File

@ -0,0 +1,22 @@
def check_resource_count(expected_count):
test.assertEqual(expected_count, len(reality.all_resources()))
example_template = Template({
'A': RsrcDef({}, []),
'B': RsrcDef({'a': '4alpha'}, ['A']),
'C': RsrcDef({'a': 'foo'}, ['B']),
'D': RsrcDef({'a': 'bar'}, ['C']),
})
engine.create_stack('foo', example_template)
engine.noop(1)
example_template2 = Template({
'A': RsrcDef({}, []),
'B': RsrcDef({'a': '4alpha'}, ['A']),
'C': RsrcDef({'a': 'blarg'}, ['B']),
'D': RsrcDef({'a': 'wibble'}, ['C']),
})
engine.update_stack('foo', example_template2)
engine.call(check_resource_count, 2)
engine.noop(11)
engine.call(verify, example_template2)

6
heat/tests/engine/test_check_resource.py
View File

@ -345,18 +345,20 @@ class CheckWorkflowUpdateTest(common.HeatTestCase):
resC = self.stack['C']
# lets say C is update-replaced
is_update = True
trav_id = self.stack.current_traversal
replacementC_id = resC.make_replacement(self.stack.t.id)
replacementC, stack, _ = resource.Resource.load(self.ctx,
replacementC_id,
trav_id,
is_update, {})
self.cr._initiate_propagate_resource(self.ctx, replacementC_id,
self.stack.current_traversal,
trav_id,
is_update, replacementC,
self.stack)
# check_stack_complete should be called with resC.id not
# replacementC.id
mock_csc.assert_called_once_with(self.ctx, self.stack,
self.stack.current_traversal,
trav_id,
resC.id, mock.ANY,
is_update)

12
heat/tests/test_resource.py
View File

@ -109,7 +109,8 @@ class ResourceTest(common.HeatTestCase):
res.state_set('CREATE', 'IN_PROGRESS')
self.stack.add_resource(res)
loaded_res, res_owning_stack, stack = resource.Resource.load(
self.stack.context, res.id, True, {})
self.stack.context, res.id,
self.stack.current_traversal, True, {})
self.assertEqual(loaded_res.id, res.id)
self.assertEqual(self.stack.t, stack.t)
@ -133,7 +134,8 @@ class ResourceTest(common.HeatTestCase):
res.state_set('CREATE', 'IN_PROGRESS')
self.old_stack.add_resource(res)
loaded_res, res_owning_stack, stack = resource.Resource.load(
self.old_stack.context, res.id, False, {})
self.old_stack.context, res.id,
self.stack.current_traversal, False, {})
self.assertEqual(loaded_res.id, res.id)
self.assertEqual(self.old_stack.t, stack.t)
self.assertNotEqual(self.new_stack.t, stack.t)
@ -158,7 +160,8 @@ class ResourceTest(common.HeatTestCase):
self.stack._resources = None
loaded_res, res_owning_stack, stack = resource.Resource.load(
self.stack.context, res.id, False, {})
self.stack.context, res.id,
self.stack.current_traversal, False, {})
self.assertEqual(origin_resources, stack.resources)
self.assertEqual(loaded_res.id, res.id)
self.assertEqual(self.stack.t, stack.t)
@ -2663,7 +2666,8 @@ class ResourceTest(common.HeatTestCase):
res.store()
data = {'bar': {'atrr1': 'baz', 'attr2': 'baz2'}}
mock_stack_load.return_value = stack
resource.Resource.load(stack.context, res.id, True, data)
resource.Resource.load(stack.context, res.id,
stack.current_traversal, True, data)
self.assertTrue(mock_stack_load.called)
mock_stack_load.assert_called_with(stack.context,
stack_id=stack.id,