From 579731aeec384623acb9d88722cb5306a68f1290 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 9ea73dfda..d70a42839 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 42636cca8..2fe155fb7 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 c42c6e622..b7e79b2ce 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 2c115bb2b..516154a96 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 eeb85d3e4..f8cf04ae7 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):