Raise exception on call of method of destroyed object

In theory using destruction dependencies one can obtain a new
reference to an object that is about to be destroyed. In this
case any attempt to call a method on such object should result
in exception being raised. This guarantees that the nothing can
be done on the object after its .destroy method was executed.

Also because the HeatStack instance is owned by the CloudRegion
it is going to be destroyed prior to it. But CloudRegion needs
to call $stack.delete() in its .destroy method and this will cause
error. As a solution HeatStack made not to be owned by the
region and instead be an independent object with destruction
dependency on it which guarantees that it will be destroyed
after the CloudRegion.

Targets-blueprint: dependency-driven-resource-deallocation
Change-Id: I5650ea672d5f121be69669f27dd5f513fbdd5c44
This commit is contained in:
Stan Lagun 2016-09-06 23:40:54 -07:00
parent c47269128b
commit dcbf5e1899
8 changed files with 46 additions and 23 deletions

View File

@ -42,9 +42,11 @@ Methods:
- $.setAttr(generatedStackName, $generatedStackName)
- $this.agentListener: new(sys:AgentListener, $this, name => $generatedStackName)
- $stackDescriptionFormat: 'This stack was generated by Murano for environment {0} (ID: {1}) - region {2}'
- $this.stack: new(sys:HeatStack, $this,
- $this.stack: new(sys:HeatStack,
regionName => $.name,
name => 'murano-' + $generatedStackName,
description => $stackDescriptionFormat.format($._environment.name, id($._environment), $.name))
- sys:GC.subscribeDestruction($this, $this.stack)
- $this.securityGroupManager:
coalesce($.defaultNetworks.environment, $.defaultNetworks.flat)?.

View File

@ -119,10 +119,6 @@ Methods:
Body:
- $.applications.pselect($.deploy())
destroy:
Body:
- $.stack.delete()
_assignRegions:
Body:
- If: $.region = null

View File

@ -174,3 +174,9 @@ class InvalidLhsTargetError(Exception):
class InvalidInheritanceError(Exception):
pass
class ObjectDestroyedError(Exception):
def __init__(self, obj):
super(ObjectDestroyedError, self).__init__(
'Object {0} is already destroyed'.format(obj))

View File

@ -110,6 +110,8 @@ class MuranoDslExecutor(object):
context = self.create_method_context(obj_context, method)
if isinstance(this, dsl_types.MuranoObject):
if this.destroyed:
raise dsl_exceptions.ObjectDestroyedError(this)
this = this.real_this
if method.arguments_scheme is not None:

View File

@ -41,7 +41,7 @@ class HeatStackError(Exception):
@dsl.name('io.murano.system.HeatStack')
class HeatStack(object):
def __init__(self, this, name, description=None, region_name=None):
def __init__(self, name, description=None, region_name=None):
self._name = name
self._template = None
self._parameters = {}
@ -51,7 +51,6 @@ class HeatStack(object):
self._description = description
self._last_stack_timestamps = (None, None)
self._tags = ''
self._owner = this.find_owner('io.murano.CloudRegion')
self._region_name = region_name
@staticmethod
@ -63,9 +62,7 @@ class HeatStack(object):
@property
def _client(self):
region = self._region_name or (
None if self._owner is None else self._owner['name'])
return self._get_client(region)
return self._get_client(self._region_name)
@staticmethod
@session_local_storage.execution_session_memoize
@ -75,9 +72,7 @@ class HeatStack(object):
def _get_token_client(self):
ks_session = auth_utils.get_token_client_session(conf=CONF.heat)
region = self._region_name or (
None if self._owner is None else self._owner['name'])
return self._create_client(ks_session, region)
return self._create_client(ks_session, self._region_name)
def current(self):
if self._template is not None:

View File

@ -60,3 +60,20 @@ Methods:
Contract: $.class(TestGCNode).notNull()
Body:
- trace('Destroy ' + $obj.value)
testCallOnDestroyedObject:
Body:
- $val: new(TestGCNode, value => X)
- sys:GC.subscribeDestruction($val, $this, _handler2)
- $val: null
- sys:GC.collect()
- $this.destroyed.foo()
_handler2:
Arguments:
- obj:
Contract: $.class(TestGCNode).notNull()
Body:
- $obj.foo()
- $this.destroyed: $obj

View File

@ -12,6 +12,8 @@
# License for the specific language governing permissions and limitations
# under the License.
from murano.dsl import exceptions
from murano.engine.system import garbage_collector
from murano.tests.unit.dsl.foundation import object_model as om
from murano.tests.unit.dsl.foundation import test_case
@ -56,3 +58,9 @@ class TestGC(test_case.DslTestCase):
self.assertEqual(
['Destroy A', 'Destroy B', 'Destruction of B', 'B', 'A'],
self.traces)
def test_call_on_destroyed_object(self):
self.assertRaises(
exceptions.ObjectDestroyedError,
self.runner.testCallOnDestroyedObject)
self.assertEqual(['foo', 'X'], self.traces)

View File

@ -36,8 +36,6 @@ class TestHeatStack(base.MuranoTestCase):
return_value=self.heat_client_mock)
heat_stack.HeatStack._get_client = mock.Mock(
return_value=self.heat_client_mock)
self._this = mock.MagicMock()
self._this.owner = None
@mock.patch(CLS_NAME + '._wait_state')
@mock.patch(CLS_NAME + '._get_status')
@ -46,8 +44,7 @@ class TestHeatStack(base.MuranoTestCase):
status_get.return_value = 'NOT_FOUND'
wait_st.return_value = {}
hs = heat_stack.HeatStack(
self._this, 'test-stack', 'Generated by TestHeatStack')
hs = heat_stack.HeatStack('test-stack', 'Generated by TestHeatStack')
hs._template = {'resources': {'test': 1}}
hs._files = {}
hs._hot_environment = ''
@ -56,7 +53,7 @@ class TestHeatStack(base.MuranoTestCase):
hs.push()
hs = heat_stack.HeatStack(
self._this, 'test-stack', 'Generated by TestHeatStack')
'test-stack', 'Generated by TestHeatStack')
hs._template = {'resources': {'test': 1}}
hs._files = {}
hs._parameters = {}
@ -86,7 +83,7 @@ class TestHeatStack(base.MuranoTestCase):
status_get.return_value = 'NOT_FOUND'
wait_st.return_value = {}
hs = heat_stack.HeatStack(self._this, 'test-stack', None)
hs = heat_stack.HeatStack('test-stack', None)
hs._template = {'resources': {'test': 1}}
hs._files = {}
hs._hot_environment = ''
@ -116,7 +113,7 @@ class TestHeatStack(base.MuranoTestCase):
status_get.return_value = 'NOT_FOUND'
wait_st.return_value = {}
hs = heat_stack.HeatStack(self._this, 'test-stack', None)
hs = heat_stack.HeatStack('test-stack', None)
hs._description = None
hs._template = {'resources': {'test': 1}}
hs._files = {"heatFile": "file"}
@ -147,7 +144,7 @@ class TestHeatStack(base.MuranoTestCase):
status_get.return_value = 'NOT_FOUND'
wait_st.return_value = {}
hs = heat_stack.HeatStack(self._this, 'test-stack', None)
hs = heat_stack.HeatStack('test-stack', None)
hs._description = None
hs._template = {'resources': {'test': 1}}
hs._files = {"heatFile": "file"}
@ -176,7 +173,7 @@ class TestHeatStack(base.MuranoTestCase):
"""Template version other than expected should cause error."""
hs = heat_stack.HeatStack(
self._this, 'test-stack', 'Generated by TestHeatStack')
'test-stack', 'Generated by TestHeatStack')
hs._template = {'resources': {'test': 1}}
invalid_template = {
@ -215,7 +212,7 @@ class TestHeatStack(base.MuranoTestCase):
wait_st.return_value = {}
CONF.set_override('stack_tags', ['test-murano', 'murano-tag'], 'heat',
enforce_type=True)
hs = heat_stack.HeatStack(self._this, 'test-stack', None)
hs = heat_stack.HeatStack('test-stack', None)
hs._description = None
hs._template = {'resources': {'test': 1}}
hs._files = {}