Calculate the requires list in check_resource

Obtain the list of required resources in one place in check_resource,
instead of in both Resource.create_convergence() and
Resource.update_convergence(). Since these require knowledge about how the
dependencies are encoded in the RPC message, they never really belonged in
the Resource class anyway.

Change-Id: I030c6287acddcd91dfe5fecba72c276fec52829b
This commit is contained in:
Zane Bitter 2017-07-21 21:32:46 -04:00
parent 812055ba44
commit 8edccc98b1
4 changed files with 46 additions and 66 deletions

View File

@ -137,8 +137,10 @@ class CheckResource(object):
is_update, rsrc, stack, adopt_stack_data):
try:
if is_update:
requires = set(d.primary_key for d in resource_data.values()
if d is not None)
try:
check_resource_update(rsrc, tmpl.id, resource_data,
check_resource_update(rsrc, tmpl.id, requires,
self.engine_id,
stack, self.msg_queue)
except resource.UpdateReplace:
@ -387,15 +389,15 @@ def _check_for_message(msg_queue):
LOG.error('Unknown message "%s" received', message)
def check_resource_update(rsrc, template_id, resource_data, engine_id,
def check_resource_update(rsrc, template_id, requires, engine_id,
stack, msg_queue):
"""Create or update the Resource if appropriate."""
check_message = functools.partial(_check_for_message, msg_queue)
if rsrc.action == resource.Resource.INIT:
rsrc.create_convergence(template_id, resource_data, engine_id,
rsrc.create_convergence(template_id, requires, engine_id,
stack.time_remaining(), check_message)
else:
rsrc.update_convergence(template_id, resource_data, engine_id,
rsrc.update_convergence(template_id, requires, engine_id,
stack.time_remaining(), stack,
check_message)

View File

@ -1154,14 +1154,11 @@ class Resource(status.ResourceStatus):
"""
return self
def create_convergence(self, template_id, resource_data, engine_id,
def create_convergence(self, template_id, requires, engine_id,
timeout, progress_callback=None):
"""Creates the resource by invoking the scheduler TaskRunner."""
self._calling_engine_id = engine_id
self.requires = list(
set(data.primary_key for data in resource_data.values()
if data is not None)
)
self.requires = list(requires)
self.current_template_id = template_id
if self.stack.adopt_stack_data is None:
runner = scheduler.TaskRunner(self.create)
@ -1412,7 +1409,7 @@ class Resource(status.ResourceStatus):
else:
raise UpdateReplace(self.name)
def update_convergence(self, template_id, resource_data, engine_id,
def update_convergence(self, template_id, new_requires, engine_id,
timeout, new_stack, progress_callback=None):
"""Update the resource synchronously.
@ -1422,8 +1419,6 @@ class Resource(status.ResourceStatus):
resource by invoking the scheduler TaskRunner.
"""
self._calling_engine_id = engine_id
new_requires = set(data.primary_key for data in resource_data.values()
if data is not None)
# Check that the resource type matches. If the type has changed by a
# legitimate substitution, the load()ed resource will already be of

View File

@ -90,7 +90,7 @@ class CheckWorkflowUpdateTest(common.HeatTestCase):
self.is_update, None)
mock_cru.assert_called_once_with(self.resource,
self.resource.stack.t.id,
{}, self.worker.engine_id,
set(), self.worker.engine_id,
mock.ANY, mock.ANY)
self.assertFalse(mock_crc.called)
@ -121,7 +121,7 @@ class CheckWorkflowUpdateTest(common.HeatTestCase):
self.is_update, None)
mock_cru.assert_called_once_with(self.resource,
self.resource.stack.t.id,
{}, self.worker.engine_id,
set(), self.worker.engine_id,
mock.ANY, mock.ANY)
self.assertTrue(mock_mr.called)
self.assertFalse(mock_crc.called)
@ -143,7 +143,7 @@ class CheckWorkflowUpdateTest(common.HeatTestCase):
self.is_update, None)
mock_cru.assert_called_once_with(self.resource,
self.resource.stack.t.id,
{}, self.worker.engine_id,
set(), self.worker.engine_id,
mock.ANY, mock.ANY)
mock_ss.assert_called_once_with(self.resource.action,
resource.Resource.FAILED,
@ -523,6 +523,20 @@ class CheckWorkflowUpdateTest(common.HeatTestCase):
self.assertFalse(mock_pcr.called)
self.assertFalse(mock_csc.called)
@mock.patch.object(resource.Resource, 'load')
def test_requires(self, mock_load, mock_cru, mock_crc, mock_pcr, mock_csc):
mock_load.return_value = self.resource, self.stack, self.stack
res_data = {(1, True): {u'id': 5, u'name': 'A', 'attrs': {}},
(2, True): {u'id': 3, u'name': 'B', 'attrs': {}}}
self.worker.check_resource(self.ctx, self.resource.id,
self.stack.current_traversal,
sync_point.serialize_input_data(res_data),
self.is_update, {})
mock_cru.assert_called_once_with(
self.resource, self.resource.stack.t.id,
{5, 3}, self.worker.engine_id,
self.stack, mock.ANY)
@mock.patch.object(check_resource, 'check_stack_complete')
@mock.patch.object(check_resource, 'propagate_check_resource')
@ -677,7 +691,7 @@ class MiscMethodsTest(common.HeatTestCase):
def test_check_resource_update_init_action(self, mock_update, mock_create):
self.resource.action = 'INIT'
check_resource.check_resource_update(
self.resource, self.resource.stack.t.id, {}, 'engine-id',
self.resource, self.resource.stack.t.id, set(), 'engine-id',
self.stack, None)
self.assertTrue(mock_create.called)
self.assertFalse(mock_update.called)
@ -688,7 +702,7 @@ class MiscMethodsTest(common.HeatTestCase):
self, mock_update, mock_create):
self.resource.action = 'CREATE'
check_resource.check_resource_update(
self.resource, self.resource.stack.t.id, {}, 'engine-id',
self.resource, self.resource.stack.t.id, set(), 'engine-id',
self.stack, None)
self.assertFalse(mock_create.called)
self.assertTrue(mock_update.called)
@ -699,7 +713,7 @@ class MiscMethodsTest(common.HeatTestCase):
self, mock_update, mock_create):
self.resource.action = 'UPDATE'
check_resource.check_resource_update(
self.resource, self.resource.stack.t.id, {}, 'engine-id',
self.resource, self.resource.stack.t.id, set(), 'engine-id',
self.stack, None)
self.assertFalse(mock_create.called)
self.assertTrue(mock_update.called)

View File

@ -2089,13 +2089,10 @@ class ResourceTest(common.HeatTestCase):
res.action = res.CREATE
res.store()
self._assert_resource_lock(res.id, None, None)
res_data = {(1, True): {u'id': 1, u'name': 'A', 'attrs': {}},
(2, True): {u'id': 3, u'name': 'B', 'attrs': {}}}
res_data = node_data.load_resources_data(res_data)
pcb = mock.Mock()
with mock.patch.object(resource.Resource, 'create') as mock_create:
res.create_convergence(self.stack.t.id, res_data, 'engine-007',
res.create_convergence(self.stack.t.id, {1, 3}, 'engine-007',
-1, pcb)
self.assertTrue(mock_create.called)
@ -2107,13 +2104,10 @@ class ResourceTest(common.HeatTestCase):
res = generic_rsrc.GenericResource('test_res', tmpl, self.stack)
res.action = res.CREATE
res.store()
res_data = {(1, True): {u'id': 1, u'name': 'A', 'attrs': {}},
(2, True): {u'id': 3, u'name': 'B', 'attrs': {}}}
res_data = node_data.load_resources_data(res_data)
pcb = mock.Mock()
self.assertRaises(scheduler.Timeout, res.create_convergence,
self.stack.t.id, res_data, 'engine-007', -1, pcb)
self.stack.t.id, {1, 3}, 'engine-007', -1, pcb)
def test_create_convergence_sets_requires_for_failure(self):
"""Ensure that requires are computed correctly.
@ -2127,11 +2121,8 @@ class ResourceTest(common.HeatTestCase):
dummy_ex = exception.ResourceNotAvailable(resource_name=res.name)
res.create = mock.Mock(side_effect=dummy_ex)
self._assert_resource_lock(res.id, None, None)
res_data = {(1, True): {u'id': 5, u'name': 'A', 'attrs': {}},
(2, True): {u'id': 3, u'name': 'B', 'attrs': {}}}
res_data = node_data.load_resources_data(res_data)
self.assertRaises(exception.ResourceNotAvailable,
res.create_convergence, self.stack.t.id, res_data,
res.create_convergence, self.stack.t.id, {5, 3},
'engine-007', self.dummy_timeout, self.dummy_event)
self.assertItemsEqual([5, 3], res.requires)
# The locking happens in create which we mocked out
@ -2146,11 +2137,8 @@ class ResourceTest(common.HeatTestCase):
self.stack.adopt_stack_data = {'resources': {'test_res': {
'resource_id': 'fluffy'}}}
self._assert_resource_lock(res.id, None, None)
res_data = {(1, True): {u'id': 5, u'name': 'A', 'attrs': {}},
(2, True): {u'id': 3, u'name': 'B', 'attrs': {}}}
res_data = node_data.load_resources_data(res_data)
tr = scheduler.TaskRunner(res.create_convergence, self.stack.t.id,
res_data, 'engine-007', self.dummy_timeout,
{5, 3}, 'engine-007', self.dummy_timeout,
self.dummy_event)
tr()
mock_adopt.assert_called_once_with(
@ -2165,11 +2153,8 @@ class ResourceTest(common.HeatTestCase):
res.store()
self.stack.adopt_stack_data = {'resources': {}}
self._assert_resource_lock(res.id, None, None)
res_data = {(1, True): {u'id': 5, u'name': 'A', 'attrs': {}},
(2, True): {u'id': 3, u'name': 'B', 'attrs': {}}}
res_data = node_data.load_resources_data(res_data)
tr = scheduler.TaskRunner(res.create_convergence, self.stack.t.id,
res_data, 'engine-007', self.dummy_timeout,
{5, 3}, 'engine-007', self.dummy_timeout,
self.dummy_event)
exc = self.assertRaises(exception.ResourceFailure, tr)
self.assertIn('Resource ID was not provided', six.text_type(exc))
@ -2203,11 +2188,8 @@ class ResourceTest(common.HeatTestCase):
new_temp, stack_id=self.stack.id)
res.stack.convergence = True
res_data = {(1, True): {u'id': 4, u'name': 'A', 'attrs': {}},
(2, True): {u'id': 3, u'name': 'B', 'attrs': {}}}
res_data = node_data.load_resources_data(res_data)
tr = scheduler.TaskRunner(res.update_convergence, new_temp.id,
res_data, 'engine-007', 120, new_stack)
{4, 3}, 'engine-007', 120, new_stack)
tr()
self.assertItemsEqual([3, 4], res.requires)
@ -2238,9 +2220,8 @@ class ResourceTest(common.HeatTestCase):
new_stack = parser.Stack(utils.dummy_context(), 'test_stack',
new_temp, stack_id=self.stack.id)
res_data = {}
tr = scheduler.TaskRunner(res.update_convergence, new_temp.id,
res_data, 'engine-007', -1, new_stack,
set(), 'engine-007', -1, new_stack,
self.dummy_event)
self.assertRaises(scheduler.Timeout, tr)
@ -2260,9 +2241,8 @@ class ResourceTest(common.HeatTestCase):
new_stack = parser.Stack(utils.dummy_context(), 'test_stack',
new_temp, stack_id=self.stack.id)
res_data = {}
self.assertRaises(resource.UpdateReplace, res.update_convergence,
new_temp.id, res_data, 'engine-007',
new_temp.id, set(), 'engine-007',
-1, new_stack)
def test_update_convergence_checks_resource_class(self):
@ -2282,9 +2262,8 @@ class ResourceTest(common.HeatTestCase):
new_stack = parser.Stack(ctx, 'test_stack',
new_temp, stack_id=self.stack.id)
res_data = {}
tr = scheduler.TaskRunner(res.update_convergence, new_temp.id,
res_data, 'engine-007', -1, new_stack,
set(), 'engine-007', -1, new_stack,
self.dummy_event)
self.assertRaises(resource.UpdateReplace, tr)
@ -2301,9 +2280,6 @@ class ResourceTest(common.HeatTestCase):
res.stack.convergence = True
res_data = {(1, True): {u'id': 4, u'name': 'A', 'attrs': {}},
(2, True): {u'id': 3, u'name': 'B', 'attrs': {}}}
res_data = node_data.load_resources_data(res_data)
tmpl = template.Template({
'HeatTemplateFormatVersion': '2012-12-12',
'Resources': {
@ -2312,7 +2288,7 @@ class ResourceTest(common.HeatTestCase):
new_stack = parser.Stack(utils.dummy_context(), 'test_stack',
tmpl, stack_id=self.stack.id)
tr = scheduler.TaskRunner(res.update_convergence, 'template_key',
res_data, 'engine-007', self.dummy_timeout,
{4, 3}, 'engine-007', self.dummy_timeout,
new_stack)
ex = self.assertRaises(exception.UpdateInProgress, tr)
msg = ("The resource %s is already being updated." %
@ -2349,9 +2325,6 @@ class ResourceTest(common.HeatTestCase):
}}, env=self.env)
new_temp.store(stack.context)
res_data = {(1, True): {u'id': 4, u'name': 'A', 'attrs': {}},
(2, True): {u'id': 3, u'name': 'B', 'attrs': {}}}
res_data = node_data.load_resources_data(res_data)
new_stack = parser.Stack(utils.dummy_context(), 'test_stack',
new_temp, stack_id=self.stack.id)
@ -2359,7 +2332,7 @@ class ResourceTest(common.HeatTestCase):
res._calling_engine_id = 'engine-9'
tr = scheduler.TaskRunner(res.update_convergence, new_temp.id,
res_data, 'engine-007', 120, new_stack,
{4, 3}, 'engine-007', 120, new_stack,
self.dummy_event)
self.assertRaises(exception.ResourceFailure, tr)
@ -2395,13 +2368,10 @@ class ResourceTest(common.HeatTestCase):
res.stack.convergence = True
res_data = {(1, True): {u'id': 4, u'name': 'A', 'attrs': {}},
(2, True): {u'id': 3, u'name': 'B', 'attrs': {}}}
res_data = node_data.load_resources_data(res_data)
new_stack = parser.Stack(utils.dummy_context(), 'test_stack',
new_temp, stack_id=self.stack.id)
tr = scheduler.TaskRunner(res.update_convergence, new_temp.id,
res_data, 'engine-007', 120, new_stack,
{4, 3}, 'engine-007', 120, new_stack,
self.dummy_event)
self.assertRaises(resource.UpdateReplace, tr)
@ -2429,7 +2399,7 @@ class ResourceTest(common.HeatTestCase):
self.stack.state_set(self.stack.ROLLBACK, self.stack.IN_PROGRESS,
'Simulate rollback')
res.restore_prev_rsrc = mock.Mock()
tr = scheduler.TaskRunner(res.update_convergence, 'new_tmpl_id', {},
tr = scheduler.TaskRunner(res.update_convergence, 'new_tmpl_id', set(),
'engine-007', self.dummy_timeout,
new_stack, self.dummy_event)
self.assertRaises(resource.UpdateReplace, tr)
@ -2453,7 +2423,7 @@ class ResourceTest(common.HeatTestCase):
self.stack.state_set(self.stack.ROLLBACK, self.stack.IN_PROGRESS,
'Simulate rollback')
res.restore_prev_rsrc = mock.Mock(side_effect=Exception)
tr = scheduler.TaskRunner(res.update_convergence, 'new_tmpl_id', {},
tr = scheduler.TaskRunner(res.update_convergence, 'new_tmpl_id', set(),
'engine-007', self.dummy_timeout, new_stack,
self.dummy_event)
self.assertRaises(exception.ResourceFailure, tr)
@ -4330,11 +4300,10 @@ class ResourceUpdateRestrictionTest(common.HeatTestCase):
self.stack = parser.Stack(utils.dummy_context(), 'test_stack',
template.Template(self.tmpl, env=self.env),
stack_id=str(uuid.uuid4()))
res_data = {}
res = self.stack['bar']
pcb = mock.Mock()
self.patchobject(res, 'lock')
res.create_convergence(self.stack.t.id, res_data, 'engine-007',
res.create_convergence(self.stack.t.id, set(), 'engine-007',
self.dummy_timeout, pcb)
return res
@ -4456,7 +4425,7 @@ class ResourceUpdateRestrictionTest(common.HeatTestCase):
error = self.assertRaises(exception.ResourceFailure,
scheduler.TaskRunner(res.update_convergence,
self.stack.t.id,
{},
set(),
'engine-007',
self.dummy_timeout,
self.new_stack,
@ -4488,7 +4457,7 @@ class ResourceUpdateRestrictionTest(common.HeatTestCase):
error = self.assertRaises(resource.UpdateReplace,
scheduler.TaskRunner(res.update_convergence,
self.stack.t.id,
{},
set(),
'engine-007',
self.dummy_timeout,
self.new_stack,