avoid excessive database calls while loading events

list_events constructs Event objects through Event.load, which causes
every event record previously loaded into memory to be reloaded again.
Furthermore, Event.load will also reload the associated stack, thus
all its resources and template. Another bad thing with that is
reloading a stack will lead to many extra stack static resolution
(including calls to nova to resovle Fn::GetAZs if it's referenced in
the template).

Fixes bug #1195793

Change-Id: I4f14c9a2a4f990b753a6e1405e27eb44c53537c9

diff --git a/heat/engine/event.py b/heat/engine/event.py
index 50e85ff26127e869f7ea18b578d72241b0cf987d..8fe176145f7a9e48ad2719b920545866989fe369 100644 (file)
--- a/heat/engine/event.py
+++ b/heat/engine/event.py
@@ -48,25 +48,25 @@ class Event(object):
         self.id = id
 
     @classmethod
-    def load(cls, context, event_id):
+    def load(cls, context, event_id, event=None, stack=None):
         '''Retrieve an Event from the database.'''
         from heat.engine import parser
 
-        ev = db_api.event_get(context, event_id)
+        ev = event if event is not None else\
+            db_api.event_get(context, event_id)
         if ev is None:
             message = 'No event exists with id "%s"' % str(event_id)
             raise exception.NotFound(message)
 
-        stack = parser.Stack.load(context, ev.stack_id)
-        resource = stack[ev.logical_resource_id]
+        st = stack if stack is not None else\
+            parser.Stack.load(context, ev.stack_id)
+        resource = st[ev.logical_resource_id]
 
-        event = cls(context, stack, resource,
-                    ev.resource_action, ev.resource_status,
-                    ev.resource_status_reason,
-                    ev.physical_resource_id, ev.resource_properties,
-                    ev.created_at, ev.id)
-
-        return event
+        return cls(context, st, resource,
+                   ev.resource_action, ev.resource_status,
+                   ev.resource_status_reason,
+                   ev.physical_resource_id, ev.resource_properties,
+                   ev.created_at, ev.id)
 
     def store(self):
         '''Store the Event in the database.'''

diff --git a/heat/engine/service.py b/heat/engine/service.py
index 5abb939f13e2f94bfde3152d138397c12ce95a83..4a8596106b6b8005dbd62966e0799879ffbf6ce0 100644 (file)
--- a/heat/engine/service.py
+++ b/heat/engine/service.py
@@ -366,6 +366,7 @@ class EngineService(service.Service):
         arg1 -> RPC context.
         arg2 -> Name of the stack you want to get events for.
         """
+
         if stack_identity is not None:
             st = self._get_stack(cnxt, stack_identity)
 
@@ -373,7 +374,17 @@ class EngineService(service.Service):
         else:
             events = db_api.event_get_all_by_tenant(cnxt)
 
-        return [api.format_event(Event.load(cnxt, e.id)) for e in events]
+        stacks = {}
+
+        def get_stack(stack_id):
+            if stack_id not in stacks:
+                stacks[stack_id] = parser.Stack.load(cnxt, stack_id)
+            return stacks[stack_id]
+
+        return [api.format_event(Event.load(cnxt,
+                                            e.id, e,
+                                            get_stack(e.stack_id)))
+                for e in events]
 
     def _authorize_stack_user(self, cnxt, stack, resource_name):
         '''

diff --git a/heat/tests/test_engine_service.py b/heat/tests/test_engine_service.py
index 8a71eaeed02ccffb16257f5067c415c3f6a109b8..011b82d4c9e6906bff9ca393a7fb46def1ac74eb 100644 (file)
--- a/heat/tests/test_engine_service.py
+++ b/heat/tests/test_engine_service.py
@@ -18,6 +18,8 @@ import json
 import sys
 
 import mox
+from testtools import matchers
+
 from oslo.config import cfg
 
 from heat.common import config
@@ -221,9 +223,9 @@ class stackCreateTest(HeatTestCase):
         rsrc = stack.resources['WebServer']
         self.assertEqual((rsrc.DELETE, rsrc.COMPLETE), rsrc.state)
         self.assertEqual((stack.DELETE, stack.COMPLETE), rsrc.state)
-        self.assertEqual(db_api.stack_get(ctx, stack_id), None)
-        self.assertEqual(db_s.action, 'DELETE')
-        self.assertEqual(db_s.status, 'COMPLETE')
+        self.assertEqual(None, db_api.stack_get(ctx, stack_id))
+        self.assertEqual('DELETE', db_s.action)
+        self.assertEqual('COMPLETE', db_s.status, )
 
 
 class stackServiceCreateUpdateDeleteTest(HeatTestCase):
@@ -263,7 +265,7 @@ class stackServiceCreateUpdateDeleteTest(HeatTestCase):
 
         result = self.man.create_stack(self.ctx, stack_name,
                                        template, params, None, {})
-        self.assertEqual(result, stack.identifier())
+        self.assertEqual(stack.identifier(), result)
         self.assertTrue(isinstance(result, dict))
         self.assertTrue(result['stack_id'])
         self.m.VerifyAll()
@@ -360,8 +362,8 @@ class stackServiceCreateUpdateDeleteTest(HeatTestCase):
 
         self.m.ReplayAll()
 
-        self.assertEqual(self.man.delete_stack(self.ctx, stack.identifier()),
-                         None)
+        self.assertEqual(None,
+                         self.man.delete_stack(self.ctx, stack.identifier()))
         self.m.VerifyAll()
 
     def test_stack_delete_nonexist(self):
@@ -408,7 +410,7 @@ class stackServiceCreateUpdateDeleteTest(HeatTestCase):
 
         result = self.man.update_stack(self.ctx, old_stack.identifier(),
                                        template, params, None, {})
-        self.assertEqual(result, old_stack.identifier())
+        self.assertEqual(old_stack.identifier(), result)
         self.assertTrue(isinstance(result, dict))
         self.assertTrue(result['stack_id'])
         self.m.VerifyAll()
@@ -492,7 +494,7 @@ class stackServiceSuspendResumeTest(HeatTestCase):
         self.m.ReplayAll()
 
         result = self.man.stack_suspend(self.ctx, stack.identifier())
-        self.assertEqual(result, None)
+        self.assertEqual(None, result)
 
         self.m.VerifyAll()
 
@@ -509,7 +511,7 @@ class stackServiceSuspendResumeTest(HeatTestCase):
         self.m.ReplayAll()
 
         result = self.man.stack_resume(self.ctx, self.stack.identifier())
-        self.assertEqual(result, None)
+        self.assertEqual(None, result)
         self.m.VerifyAll()
 
     def test_stack_suspend_nonexist(self):
@@ -556,7 +558,7 @@ class stackServiceTest(HeatTestCase):
 
         self.m.ReplayAll()
         identity = self.eng.identify_stack(self.ctx, self.stack.name)
-        self.assertEqual(identity, self.stack.identifier())
+        self.assertEqual(self.stack.identifier(), identity)
 
         self.m.VerifyAll()
 
@@ -568,7 +570,7 @@ class stackServiceTest(HeatTestCase):
 
         self.m.ReplayAll()
         identity = self.eng.identify_stack(self.ctx, self.stack.id)
-        self.assertEqual(identity, self.stack.identifier())
+        self.assertEqual(self.stack.identifier(), identity)
 
         self.m.VerifyAll()
 
@@ -601,14 +603,14 @@ class stackServiceTest(HeatTestCase):
 
         events = self.eng.list_events(self.ctx, self.stack.identifier())
 
-        self.assertEqual(len(events), 2)
+        self.assertEqual(2, len(events))
         for ev in events:
             self.assertTrue('event_identity' in ev)
-            self.assertEqual(type(ev['event_identity']), dict)
+            self.assertEqual(dict, type(ev['event_identity']))
             self.assertTrue(ev['event_identity']['path'].rsplit('/', 1)[1])
 
             self.assertTrue('logical_resource_id' in ev)
-            self.assertEqual(ev['logical_resource_id'], 'WebServer')
+            self.assertEqual('WebServer', ev['logical_resource_id'])
 
             self.assertTrue('physical_resource_id' in ev)
 
@@ -617,30 +619,73 @@ class stackServiceTest(HeatTestCase):
             # a few times so this should work.
             user_data = ev['resource_properties']['UserData']
             self.assertNotEqual(user_data.find('wordpress'), -1)
-            self.assertEqual(ev['resource_properties']['ImageId'],
-                             'F17-x86_64-gold')
-            self.assertEqual(ev['resource_properties']['InstanceType'],
-                             'm1.large')
+            self.assertEqual('F17-x86_64-gold',
+                             ev['resource_properties']['ImageId'])
+            self.assertEqual('m1.large',
+                             ev['resource_properties']['InstanceType'])
 
-            self.assertEqual(ev['resource_action'], 'CREATE')
+            self.assertEqual('CREATE', ev['resource_action'])
             self.assertTrue(ev['resource_status'] in ('IN_PROGRESS',
                                                       'COMPLETE'))
 
             self.assertTrue('resource_status_reason' in ev)
-            self.assertEqual(ev['resource_status_reason'], 'state changed')
+            self.assertEqual('state changed', ev['resource_status_reason'])
 
             self.assertTrue('resource_type' in ev)
-            self.assertEqual(ev['resource_type'], 'AWS::EC2::Instance')
+            self.assertEqual('AWS::EC2::Instance', ev['resource_type'])
 
             self.assertTrue('stack_identity' in ev)
 
             self.assertTrue('stack_name' in ev)
-            self.assertEqual(ev['stack_name'], self.stack.name)
+            self.assertEqual(self.stack.name, ev['stack_name'])
 
             self.assertTrue('event_time' in ev)
 
         self.m.VerifyAll()
 
+    @stack_context('service_event_list_test_stack')
+    def test_stack_event_list_by_tenant(self):
+        events = self.eng.list_events(self.ctx, None)
+
+        self.assertEqual(2, len(events))
+        for ev in events:
+            self.assertIn('event_identity', ev)
+            self.assertThat(ev['event_identity'], matchers.IsInstance(dict))
+            self.assertTrue(ev['event_identity']['path'].rsplit('/', 1)[1])
+
+            self.assertTrue('logical_resource_id' in ev)
+            self.assertEqual('WebServer', ev['logical_resource_id'])
+
+            self.assertTrue('physical_resource_id' in ev)
+
+            self.assertTrue('resource_properties' in ev)
+            # Big long user data field.. it mentions 'wordpress'
+            # a few times so this should work.
+            user_data = ev['resource_properties']['UserData']
+            self.assertIn('wordpress', user_data)
+            self.assertEqual('F17-x86_64-gold',
+                             ev['resource_properties']['ImageId'])
+            self.assertEqual('m1.large',
+                             ev['resource_properties']['InstanceType'])
+
+            self.assertEqual('CREATE', ev['resource_action'])
+            self.assertIn(ev['resource_status'], ('IN_PROGRESS', 'COMPLETE'))
+
+            self.assertIn('resource_status_reason', ev)
+            self.assertEqual('state changed', ev['resource_status_reason'])
+
+            self.assertIn('resource_type', ev)
+            self.assertEqual('AWS::EC2::Instance', ev['resource_type'])
+
+            self.assertIn('stack_identity', ev)
+
+            self.assertIn('stack_name', ev)
+            self.assertEqual(self.stack.name, ev['stack_name'])
+
+            self.assertIn('event_time', ev)
+
+        self.m.VerifyAll()
+
     @stack_context('service_list_all_test_stack')
     def test_stack_list_all(self):
         self.m.StubOutWithMock(parser.Stack, 'load')
@@ -650,14 +695,14 @@ class stackServiceTest(HeatTestCase):
         self.m.ReplayAll()
         sl = self.eng.list_stacks(self.ctx)
 
-        self.assertEqual(len(sl), 1)
+        self.assertEqual(1, len(sl))
         for s in sl:
             self.assertTrue('creation_time' in s)
             self.assertTrue('updated_time' in s)
             self.assertTrue('stack_identity' in s)
             self.assertNotEqual(s['stack_identity'], None)
             self.assertTrue('stack_name' in s)
-            self.assertEqual(s['stack_name'], self.stack.name)
+            self.assertEqual(self.stack.name, s['stack_name'])
             self.assertTrue('stack_status' in s)
             self.assertTrue('stack_status_reason' in s)
             self.assertTrue('description' in s)
@@ -706,7 +751,7 @@ class stackServiceTest(HeatTestCase):
 
         sl = self.eng.show_stack(self.ctx, self.stack.identifier())
 
-        self.assertEqual(len(sl), 1)
+        self.assertEqual(1, len(sl))
 
         s = sl[0]
         self.assertTrue('creation_time' in s)
@@ -714,7 +759,7 @@ class stackServiceTest(HeatTestCase):
         self.assertTrue('stack_identity' in s)
         self.assertNotEqual(s['stack_identity'], None)
         self.assertTrue('stack_name' in s)
-        self.assertEqual(s['stack_name'], self.stack.name)
+        self.assertEqual(self.stack.name, s['stack_name'])
         self.assertTrue('stack_status' in s)
         self.assertTrue('stack_status_reason' in s)
         self.assertTrue('description' in s)
@@ -727,7 +772,7 @@ class stackServiceTest(HeatTestCase):
     def test_stack_describe_all(self):
         sl = self.eng.show_stack(self.ctx, None)
 
-        self.assertEqual(len(sl), 1)
+        self.assertEqual(1, len(sl))
 
         s = sl[0]
         self.assertTrue('creation_time' in s)
@@ -735,7 +780,7 @@ class stackServiceTest(HeatTestCase):
         self.assertTrue('stack_identity' in s)
         self.assertNotEqual(s['stack_identity'], None)
         self.assertTrue('stack_name' in s)
-        self.assertEqual(s['stack_name'], self.stack.name)
+        self.assertEqual(self.stack.name, s['stack_name'])
         self.assertTrue('stack_status' in s)
         self.assertTrue('stack_status_reason' in s)
         self.assertTrue('description' in s)
@@ -764,14 +809,14 @@ class stackServiceTest(HeatTestCase):
         self.assertTrue('stack_identity' in r)
         self.assertNotEqual(r['stack_identity'], None)
         self.assertTrue('stack_name' in r)
-        self.assertEqual(r['stack_name'], self.stack.name)
+        self.assertEqual(self.stack.name, r['stack_name'])
         self.assertTrue('metadata' in r)
         self.assertTrue('resource_status' in r)
         self.assertTrue('resource_status_reason' in r)
         self.assertTrue('resource_type' in r)
         self.assertTrue('physical_resource_id' in r)
         self.assertTrue('logical_resource_id' in r)
-        self.assertEqual(r['logical_resource_id'], 'WebServer')
+        self.assertEqual('WebServer', r['logical_resource_id'])
 
         self.m.VerifyAll()
 
@@ -858,7 +903,7 @@ class stackServiceTest(HeatTestCase):
                                                       self.stack.identifier(),
                                                       'WebServer')
 
-        self.assertEqual(len(resources), 1)
+        self.assertEqual(1, len(resources))
         r = resources[0]
         self.assertTrue('resource_identity' in r)
         self.assertTrue('description' in r)
@@ -866,13 +911,13 @@ class stackServiceTest(HeatTestCase):
         self.assertTrue('stack_identity' in r)
         self.assertNotEqual(r['stack_identity'], None)
         self.assertTrue('stack_name' in r)
-        self.assertEqual(r['stack_name'], self.stack.name)
+        self.assertEqual(self.stack.name, r['stack_name'])
         self.assertTrue('resource_status' in r)
         self.assertTrue('resource_status_reason' in r)
         self.assertTrue('resource_type' in r)
         self.assertTrue('physical_resource_id' in r)
         self.assertTrue('logical_resource_id' in r)
-        self.assertEqual(r['logical_resource_id'], 'WebServer')
+        self.assertEqual('WebServer', r['logical_resource_id'])
 
         self.m.VerifyAll()
 
@@ -887,10 +932,10 @@ class stackServiceTest(HeatTestCase):
                                                       self.stack.identifier(),
                                                       None)
 
-        self.assertEqual(len(resources), 1)
+        self.assertEqual(1, len(resources))
         r = resources[0]
         self.assertTrue('logical_resource_id' in r)
-        self.assertEqual(r['logical_resource_id'], 'WebServer')
+        self.assertEqual('WebServer', r['logical_resource_id'])
 
         self.m.VerifyAll()
 
@@ -924,8 +969,8 @@ class stackServiceTest(HeatTestCase):
         result = self.eng.find_physical_resource(self.ctx, phys_id)
         self.assertTrue(isinstance(result, dict))
         resource_identity = identifier.ResourceIdentifier(**result)
-        self.assertEqual(resource_identity.stack(), self.stack.identifier())
-        self.assertEqual(resource_identity.resource_name, 'WebServer')
+        self.assertEqual(self.stack.identifier(), resource_identity.stack())
+        self.assertEqual('WebServer', resource_identity.resource_name)
 
     def test_find_physical_resource_nonexist(self):
         self.assertRaises(exception.PhysicalResourceNotFound,
@@ -942,13 +987,13 @@ class stackServiceTest(HeatTestCase):
         resources = self.eng.list_stack_resources(self.ctx,
                                                   self.stack.identifier())
 
-        self.assertEqual(len(resources), 1)
+        self.assertEqual(1, len(resources))
         r = resources[0]
         self.assertTrue('resource_identity' in r)
         self.assertTrue('updated_time' in r)
         self.assertTrue('physical_resource_id' in r)
         self.assertTrue('logical_resource_id' in r)
-        self.assertEqual(r['logical_resource_id'], 'WebServer')
+        self.assertEqual('WebServer', r['logical_resource_id'])
         self.assertTrue('resource_status' in r)
         self.assertTrue('resource_status_reason' in r)
         self.assertTrue('resource_type' in r)
@@ -989,7 +1034,7 @@ class stackServiceTest(HeatTestCase):
                                           'WebServer', test_metadata)
         # metadata_update is a no-op for all resources except
         # WaitConditionHandle so we don't expect this to have changed
-        self.assertEqual(result, pre_update_meta)
+        self.assertEqual(pre_update_meta, result)
 
         self.m.VerifyAll()
 
@@ -1157,23 +1202,23 @@ class stackServiceTest(HeatTestCase):
         result = self.eng.set_watch_state(self.ctx,
                                           watch_name="OverrideAlarm",
                                           state=state)
-        self.assertEqual(result[engine_api.WATCH_STATE_VALUE], state)
-        self.assertEqual(self.eng.stg[self.stack.id].threads, [])
+        self.assertEqual(state, result[engine_api.WATCH_STATE_VALUE])
+        self.assertEqual([], self.eng.stg[self.stack.id].threads)
 
         state = watchrule.WatchRule.NORMAL
         result = self.eng.set_watch_state(self.ctx,
                                           watch_name="OverrideAlarm",
                                           state=state)
-        self.assertEqual(result[engine_api.WATCH_STATE_VALUE], state)
-        self.assertEqual(self.eng.stg[self.stack.id].threads, [])
+        self.assertEqual(state, result[engine_api.WATCH_STATE_VALUE])
+        self.assertEqual([], self.eng.stg[self.stack.id].threads)
 
         state = watchrule.WatchRule.ALARM
         result = self.eng.set_watch_state(self.ctx,
                                           watch_name="OverrideAlarm",
                                           state=state)
-        self.assertEqual(result[engine_api.WATCH_STATE_VALUE], state)
-        self.assertEqual(self.eng.stg[self.stack.id].threads,
-                         [DummyAction.alarm])
+        self.assertEqual(state, result[engine_api.WATCH_STATE_VALUE])
+        self.assertEqual([DummyAction.alarm],
+                         self.eng.stg[self.stack.id].threads)
 
         self.m.VerifyAll()
 
@@ -1228,9 +1273,9 @@ class stackServiceTest(HeatTestCase):
     def test_stack_list_all_empty(self):
         sl = self.eng.list_stacks(self.ctx)
 
-        self.assertEqual(len(sl), 0)
+        self.assertEqual(0, len(sl))
 
     def test_stack_describe_all_empty(self):
         sl = self.eng.show_stack(self.ctx, None)
 
-        self.assertEqual(len(sl), 0)
+        self.assertEqual(0, len(sl))

diff --git a/heat/tests/test_event.py b/heat/tests/test_event.py
index 160eba42edbabd0d7371372a2ea1639b800506f5..0cadb861daf0616c4b9ee469fdba07191aadc529 100644 (file)
--- a/heat/tests/test_event.py
+++ b/heat/tests/test_event.py
@@ -71,15 +71,39 @@ class EventTest(HeatTestCase):
 
         loaded_e = event.Event.load(self.ctx, e.id)
 
-        self.assertEqual(loaded_e.stack.id, self.stack.id)
-        self.assertEqual(loaded_e.resource.name, self.resource.name)
-        self.assertEqual(loaded_e.resource.id, self.resource.id)
-        self.assertEqual(loaded_e.physical_resource_id, 'wibble')
-        self.assertEqual(loaded_e.action, 'TEST')
-        self.assertEqual(loaded_e.status, 'IN_PROGRESS')
-        self.assertEqual(loaded_e.reason, 'Testing')
-        self.assertNotEqual(loaded_e.timestamp, None)
-        self.assertEqual(loaded_e.resource_properties, {'Foo': 'goo'})
+        self.assertEqual(self.stack.id, loaded_e.stack.id)
+        self.assertEqual(self.resource.name, loaded_e.resource.name)
+        self.assertEqual(self.resource.id, loaded_e.resource.id)
+        self.assertEqual('wibble', loaded_e.physical_resource_id)
+        self.assertEqual('TEST', loaded_e.action)
+        self.assertEqual('IN_PROGRESS', loaded_e.status)
+        self.assertEqual('Testing', loaded_e.reason)
+        self.assertNotEqual(None, loaded_e.timestamp)
+        self.assertEqual({'Foo': 'goo'}, loaded_e.resource_properties)
+
+    def test_load_given_stack_event(self):
+        self.resource.resource_id_set('resource_physical_id')
+
+        e = event.Event(self.ctx, self.stack, self.resource,
+                        'TEST', 'IN_PROGRESS', 'Testing',
+                        'wibble', self.resource.properties)
+
+        e.store()
+        self.assertNotEqual(e.id, None)
+
+        ev = db_api.event_get(self.ctx, e.id)
+
+        loaded_e = event.Event.load(self.ctx, e.id, stack=self.stack, event=ev)
+
+        self.assertEqual(self.stack.id, loaded_e.stack.id)
+        self.assertEqual(self.resource.name, loaded_e.resource.name)
+        self.assertEqual(self.resource.id, loaded_e.resource.id)
+        self.assertEqual('wibble', loaded_e.physical_resource_id)
+        self.assertEqual('TEST', loaded_e.action)
+        self.assertEqual('IN_PROGRESS', loaded_e.status)
+        self.assertEqual('Testing', loaded_e.reason)
+        self.assertNotEqual(None, loaded_e.timestamp)
+        self.assertEqual({'Foo': 'goo'}, loaded_e.resource_properties)
 
     def test_identifier(self):
         e = event.Event(self.ctx, self.stack, self.resource,
@@ -93,7 +117,7 @@ class EventTest(HeatTestCase):
             'tenant': self.ctx.tenant_id,
             'path': '/resources/EventTestResource/events/%s' % str(eid)
         }
-        self.assertEqual(e.identifier(), expected_identifier)
+        self.assertEqual(expected_identifier, e.identifier())
 
     def test_badprop(self):
         tmpl = {'Type': 'ResourceWithRequiredProps',