From 855e5f0d3452341706c07c861050941923cf316d Mon Sep 17 00:00:00 2001
From: Stan Lagun <slagun@mirantis.com>
Date: Thu, 31 Mar 2016 02:06:59 +0300
Subject: [PATCH] Destroy orphan objects

Murano has a garbage collector that destroys objects
which were deleted in API after last deployment.
However if objects were removed from object model
during deployment (action execution) or were created
and not saved there they left unnoticed by the engine
causing resource leak.

With this change all objects that are presented in object store
but were not serialized to result object model are get destroyed.

Change-Id: I107b34e3ae4e1b5835645e116f9445535dfb7636
Closes-Bug: #1562804
---
 murano/common/engine.py                    |  7 +++-
 murano/dsl/executor.py                     | 37 +++++++++++++++++-----
 murano/dsl/object_store.py                 |  6 ++++
 murano/dsl/serializer.py                   |  5 +--
 murano/tests/unit/dsl/foundation/runner.py |  2 +-
 5 files changed, 45 insertions(+), 12 deletions(-)

diff --git a/murano/common/engine.py b/murano/common/engine.py
index 9ea73dfd..d70a4283 100755
--- a/murano/common/engine.py
+++ b/murano/common/engine.py
@@ -210,7 +210,12 @@ class TaskExecutor(object):
                     action_result = self._invoke(executor)
                 finally:
                     try:
-                        self._model = serializer.serialize_model(obj, executor)
+                        self._model, alive_object_ids = \
+                            serializer.serialize_model(obj, executor)
+                        LOG.debug('Cleaning up orphan objects')
+                        n = executor.cleanup_orphans(alive_object_ids)
+                        LOG.debug('{} orphan objects were destroyed'.format(n))
+
                     except Exception as e:
                         return self.exception_result(e, None, '<model>')
             except Exception as e:
diff --git a/murano/dsl/executor.py b/murano/dsl/executor.py
index 42636cca..2fe155fb 100644
--- a/murano/dsl/executor.py
+++ b/murano/dsl/executor.py
@@ -242,14 +242,35 @@ class MuranoDslExecutor(object):
                 objects_to_clean.append(obj)
         if objects_to_clean:
             for obj in objects_to_clean:
-                methods = obj.type.find_methods(lambda m: m.name == '.destroy')
-                for method in methods:
-                    try:
-                        method.invoke(self, obj, (), {}, None)
-                    except Exception as e:
-                        LOG.warning(_LW(
-                            'Muted exception during execution of .destroy '
-                            'on {0}: {1}').format(obj, e), exc_info=True)
+                self._destroy_object(obj)
+
+    def cleanup_orphans(self, alive_object_ids):
+        with helpers.execution_session(self._session):
+            orphan_ids = self._collect_orphans(alive_object_ids)
+            self._destroy_orphans(orphan_ids)
+            return len(orphan_ids)
+
+    def _collect_orphans(self, alive_object_ids):
+        orphan_ids = []
+        for obj_id in self._object_store.iterate():
+            if obj_id not in alive_object_ids:
+                orphan_ids.append(obj_id)
+        return orphan_ids
+
+    def _destroy_orphans(self, orphan_ids):
+        for obj_id in orphan_ids:
+            self._destroy_object(self._object_store.get(obj_id))
+            self._object_store.remove(obj_id)
+
+    def _destroy_object(self, obj):
+        methods = obj.type.find_methods(lambda m: m.name == '.destroy')
+        for method in methods:
+            try:
+                method.invoke(self, obj, (), {}, None)
+            except Exception as e:
+                LOG.warning(_LW(
+                    'Muted exception during execution of .destroy '
+                    'on {0}: {1}').format(obj, e), exc_info=True)
 
     def _list_potential_object_ids(self, data):
         if isinstance(data, dict):
diff --git a/murano/dsl/object_store.py b/murano/dsl/object_store.py
index c42c6e62..b7e79b2c 100644
--- a/murano/dsl/object_store.py
+++ b/murano/dsl/object_store.py
@@ -52,6 +52,12 @@ class ObjectStore(object):
     def put(self, murano_object):
         self._store[murano_object.object_id] = murano_object
 
+    def iterate(self):
+        return six.iterkeys(self._store)
+
+    def remove(self, object_id):
+        self._store.pop(object_id)
+
     def load(self, value, owner, context=None):
         if value is None:
             return None
diff --git a/murano/dsl/serializer.py b/murano/dsl/serializer.py
index 2c115bb2..516154a9 100644
--- a/murano/dsl/serializer.py
+++ b/murano/dsl/serializer.py
@@ -26,7 +26,7 @@ class ObjRef(object):
 
 
 def serialize(obj):
-    return serialize_model(obj, None, True)['Objects']
+    return serialize_model(obj, None, True)[0]['Objects']
 
 
 def _serialize_object(root_object, designer_attributes, allow_refs):
@@ -53,6 +53,7 @@ def serialize_model(root_object, executor, allow_refs=False):
         tree = None
         tree_copy = None
         attributes = []
+        serialized_objects = set()
     else:
         tree, serialized_objects = _serialize_object(
             root_object, designer_attributes, allow_refs)
@@ -66,7 +67,7 @@ def serialize_model(root_object, executor, allow_refs=False):
         'Objects': tree,
         'ObjectsCopy': tree_copy,
         'Attributes': attributes
-    }
+    }, serialized_objects
 
 
 def _serialize_available_action(obj, current_actions):
diff --git a/murano/tests/unit/dsl/foundation/runner.py b/murano/tests/unit/dsl/foundation/runner.py
index eeb85d3e..f8cf04ae 100644
--- a/murano/tests/unit/dsl/foundation/runner.py
+++ b/murano/tests/unit/dsl/foundation/runner.py
@@ -124,7 +124,7 @@ class Runner(object):
 
     @property
     def serialized_model(self):
-        return serializer.serialize_model(self._root, self.executor)
+        return serializer.serialize_model(self._root, self.executor)[0]
 
     @property
     def preserve_exception(self):