Browse Source

Don't pass input_data to Resource.delete_convergence()

In the original prototype for convergence, we passed the input_data from
the SyncPoint to the resource when calling the equivalent of
convergence_delete(), so that we could clear and needed_by references that
no longer exist. This is pointless for a few reasons:

* It's implemented incorrectly - it *sets* the referenced resources into
  needed_by instead of clearing them from it.
* We don't actually pass any input data - in WorkerService.check_resource()
  it's always set to an empty dict for cleanup nodes, regardless of what
  came in on the wire.
* We don't store the result to the database unless we're deleting the
  resource anyway - in which case it doesn't matter.
* It turns out that even in the prototype, the whole needed_by mechanism
  isn't actually used for anything:
  c74aac1f07

Rather than pretend that we're doing something useful with the input_data
here, just set the needed_by to an empty list, which is what was happening
anyway.

Change-Id: I73f6cf1646584dc4a83497f5a583c2c8973e8aba
changes/68/486268/10
Zane Bitter 5 years ago
parent
commit
812055ba44
  1. 7
      heat/engine/check_resource.py
  2. 5
      heat/engine/resource.py
  3. 6
      heat/tests/engine/test_check_resource.py
  4. 15
      heat/tests/test_resource.py

7
heat/engine/check_resource.py

@ -149,8 +149,7 @@ class CheckResource(object):
return False
else:
check_resource_cleanup(rsrc, tmpl.id, resource_data,
self.engine_id,
check_resource_cleanup(rsrc, tmpl.id, self.engine_id,
stack.time_remaining(), self.msg_queue)
return True
@ -401,9 +400,9 @@ def check_resource_update(rsrc, template_id, resource_data, engine_id,
check_message)
def check_resource_cleanup(rsrc, template_id, resource_data, engine_id,
def check_resource_cleanup(rsrc, template_id, engine_id,
timeout, msg_queue):
"""Delete the Resource if appropriate."""
check_message = functools.partial(_check_for_message, msg_queue)
rsrc.delete_convergence(template_id, resource_data, engine_id, timeout,
rsrc.delete_convergence(template_id, engine_id, timeout,
check_message)

5
heat/engine/resource.py

@ -1915,7 +1915,7 @@ class Resource(status.ResourceStatus):
expected_engine_id=None):
self._incr_atomic_key(self._atomic_key)
def delete_convergence(self, template_id, input_data, engine_id, timeout,
def delete_convergence(self, template_id, engine_id, timeout,
progress_callback=None):
"""Destroys the resource if it doesn't belong to given template.
@ -1928,8 +1928,7 @@ class Resource(status.ResourceStatus):
the replacement resource's needed_by and replaces fields.
"""
self._calling_engine_id = engine_id
self.needed_by = list(set(v for v in input_data.values()
if v is not None))
self.needed_by = []
if self.current_template_id != template_id:
# just delete the resources in INIT state

6
heat/tests/engine/test_check_resource.py

@ -563,7 +563,7 @@ class CheckWorkflowCleanupTest(common.HeatTestCase):
self.assertFalse(mock_cru.called)
mock_crc.assert_called_once_with(
self.resource, self.resource.stack.t.id,
{}, self.worker.engine_id,
self.worker.engine_id,
tr(), mock.ANY)
@mock.patch.object(stack.Stack, 'time_remaining')
@ -576,7 +576,7 @@ class CheckWorkflowCleanupTest(common.HeatTestCase):
self.is_update, None)
mock_crc.assert_called_once_with(self.resource,
self.resource.stack.t.id,
{}, self.worker.engine_id,
self.worker.engine_id,
tr(), mock.ANY)
self.assertFalse(mock_cru.called)
self.assertFalse(mock_pcr.called)
@ -708,7 +708,7 @@ class MiscMethodsTest(common.HeatTestCase):
def test_check_resource_cleanup_delete(self, mock_delete):
self.resource.current_template_id = 'new-template-id'
check_resource.check_resource_cleanup(
self.resource, self.resource.stack.t.id, {}, 'engine-id',
self.resource, self.resource.stack.t.id, 'engine-id',
self.stack.timeout_secs(), None)
self.assertTrue(mock_delete.called)

15
heat/tests/test_resource.py

@ -2472,7 +2472,7 @@ class ResourceTest(common.HeatTestCase):
self._assert_resource_lock(res.id, None, None)
pcb = mock.Mock()
with mock.patch.object(resource.Resource, 'delete') as mock_delete:
tr = scheduler.TaskRunner(res.delete_convergence, 2, {},
tr = scheduler.TaskRunner(res.delete_convergence, 2,
'engine-007', 20, pcb)
tr()
self.assertTrue(mock_delete.called)
@ -2485,7 +2485,7 @@ class ResourceTest(common.HeatTestCase):
res.current_template_id = 'same-template'
res.store()
res.delete = mock.Mock()
tr = scheduler.TaskRunner(res.delete_convergence, 'same-template', {},
tr = scheduler.TaskRunner(res.delete_convergence, 'same-template',
'engine-007', self.dummy_timeout,
self.dummy_event)
tr()
@ -2502,7 +2502,7 @@ class ResourceTest(common.HeatTestCase):
res.handle_delete = mock.Mock(side_effect=ValueError('test'))
self._assert_resource_lock(res.id, None, None)
res.stack.convergence = True
tr = scheduler.TaskRunner(res.delete_convergence, 2, {}, 'engine-007',
tr = scheduler.TaskRunner(res.delete_convergence, 2, 'engine-007',
self.dummy_timeout, self.dummy_event)
self.assertRaises(exception.ResourceFailure, tr)
self.assertTrue(res.handle_delete.called)
@ -2540,12 +2540,11 @@ class ResourceTest(common.HeatTestCase):
res.action = res.CREATE
res.store()
res.destroy = mock.Mock()
input_data = {(1, False): 4, (2, False): 5} # needed_by resource ids
self._assert_resource_lock(res.id, None, None)
scheduler.TaskRunner(res.delete_convergence, 1, input_data,
scheduler.TaskRunner(res.delete_convergence, 1,
'engine-007', self.dummy_timeout,
self.dummy_event)()
self.assertItemsEqual([4, 5], res.needed_by)
self.assertItemsEqual([], res.needed_by)
@mock.patch.object(resource_objects.Resource, 'get_obj')
def test_update_replacement_data(self, mock_get_obj):
@ -2691,7 +2690,7 @@ class ResourceTest(common.HeatTestCase):
res.store()
with mock.patch.object(resource_objects.Resource,
'delete') as resource_del:
tr = scheduler.TaskRunner(res.delete_convergence, 1, {},
tr = scheduler.TaskRunner(res.delete_convergence, 1,
'engine-007', 1, self.dummy_event)
tr()
resource_del.assert_called_once_with(res.context, res.id)
@ -2702,7 +2701,7 @@ class ResourceTest(common.HeatTestCase):
res.action = res.CREATE
res.store()
timeout = -1 # to emulate timeout
tr = scheduler.TaskRunner(res.delete_convergence, 1, {}, 'engine-007',
tr = scheduler.TaskRunner(res.delete_convergence, 1, 'engine-007',
timeout, self.dummy_event)
self.assertRaises(scheduler.Timeout, tr)

Loading…
Cancel
Save