From edd2f8f0aca14f225e441dd3efbfa1cce7a859b4 Mon Sep 17 00:00:00 2001 From: Zane Bitter Date: Thu, 10 Sep 2015 10:58:52 -0400 Subject: [PATCH] Work around problems storing huge properties in events There is a maximum limit on the size of the resource properties we can store for an event in MySQL. To work around this, store an error instead of the largest property, as this is expected to cater for graceful-failure of the most common known case (large SoftwareConfig config properties), with a fallback of storing only the error when this also fails. Co-Authored-By: Steven Hardy Co-Authored-By: Marios Andreou Closes-Bug: #1493858 Change-Id: I668c7ed8ca6c063fd20bc5271d6afea941a5f277 (cherry picked from commit ab78bde75bb6aae2025815abd4613d8dabbbd4df) --- heat/engine/event.py | 19 ++++++- heat/tests/generic_resource.py | 9 ++++ heat/tests/test_event.py | 93 ++++++++++++++++++++++++++++++++-- 3 files changed, 117 insertions(+), 4 deletions(-) diff --git a/heat/engine/event.py b/heat/engine/event.py index 3d9752c2a4..3fdbc96721 100644 --- a/heat/engine/event.py +++ b/heat/engine/event.py @@ -13,6 +13,8 @@ import six +import oslo_db.exception + from heat.common import exception from heat.common.i18n import _ from heat.common import identifier @@ -84,7 +86,22 @@ class Event(object): if self.timestamp is not None: ev['created_at'] = self.timestamp - new_ev = event_object.Event.create(self.context, ev) + try: + new_ev = event_object.Event.create(self.context, ev) + except oslo_db.exception.DBError: + # Attempt do drop the largest key and re-store as we expect + # This to mostly happen with one large config blob property + max_key, max_val = max(ev['resource_properties'].items(), + key=lambda i: len(repr(i[1]))) + err = 'Resource properties are too large to store' + ev['resource_properties'].update({'Error': err}) + ev['resource_properties'][max_key] = '' + try: + new_ev = event_object.Event.create(self.context, ev) + except oslo_db.exception.DBError: + # Give up and drop all properties.. + ev['resource_properties'] = {'Error': err} + new_ev = event_object.Event.create(self.context, ev) self.id = new_ev.id return self.id diff --git a/heat/tests/generic_resource.py b/heat/tests/generic_resource.py index 456e67dab3..78ffa7a9c5 100644 --- a/heat/tests/generic_resource.py +++ b/heat/tests/generic_resource.py @@ -131,6 +131,15 @@ class ResourceWithRequiredProps(GenericResource): required=True)} +class ResourceWithMultipleRequiredProps(GenericResource): + properties_schema = {'Foo1': properties.Schema(properties.Schema.STRING, + required=True), + 'Foo2': properties.Schema(properties.Schema.STRING, + required=True), + 'Foo3': properties.Schema(properties.Schema.STRING, + required=True)} + + class SignalResource(signal_responder.SignalResponder): properties_schema = {} attributes_schema = {'AlarmUrl': attributes.Schema('Get a signed webhook')} diff --git a/heat/tests/test_event.py b/heat/tests/test_event.py index 4fb1320bb4..0d6f0657bc 100644 --- a/heat/tests/test_event.py +++ b/heat/tests/test_event.py @@ -11,7 +11,10 @@ # License for the specific language governing permissions and limitations # under the License. +import mock + from oslo_config import cfg +import oslo_db.exception from heat.engine import event from heat.engine import parser @@ -37,11 +40,25 @@ tmpl = { } } +tmpl_multiple = { + 'HeatTemplateFormatVersion': '2012-12-12', + 'Resources': { + 'EventTestResource': { + 'Type': 'ResourceWithMultipleRequiredProps', + 'Properties': {'Foo1': 'zoo', + 'Foo2': 'A0000000000', + 'Foo3': '99999'} + } + } +} -class EventTest(common.HeatTestCase): + +class EventCommon(common.HeatTestCase): def setUp(self): - super(EventTest, self).setUp() + super(EventCommon, self).setUp() + + def _setup_stack(self, the_tmpl): self.username = 'event_test_user' self.ctx = utils.dummy_context() @@ -50,15 +67,25 @@ class EventTest(common.HeatTestCase): resource._register_class('ResourceWithRequiredProps', generic_rsrc.ResourceWithRequiredProps) + resource._register_class( + 'ResourceWithMultipleRequiredProps', + generic_rsrc.ResourceWithMultipleRequiredProps) self.stack = parser.Stack(self.ctx, 'event_load_test_stack', - template.Template(tmpl)) + template.Template(the_tmpl)) self.stack.store() self.resource = self.stack['EventTestResource'] self.resource._store() self.addCleanup(stack_object.Stack.delete, self.ctx, self.stack.id) + +class EventTest(EventCommon): + + def setUp(self): + super(EventTest, self).setUp() + self._setup_stack(tmpl) + def test_load(self): self.resource.resource_id_set('resource_physical_id') @@ -157,3 +184,63 @@ class EventTest(common.HeatTestCase): e = event.Event(self.ctx, self.stack, 'TEST', 'IN_PROGRESS', 'Testing', 'wibble', res.properties, res.name, res.type()) self.assertIn('Error', e.resource_properties) + + +class EventTestProps(EventCommon): + + def setUp(self): + super(EventTestProps, self).setUp() + self._setup_stack(tmpl_multiple) + + def test_store_fail_all_props(self): + self.resource.resource_id_set('resource_physical_id') + + e = event.Event(self.ctx, self.stack, 'TEST', 'IN_PROGRESS', 'Testing', + 'alabama', self.resource.properties, + self.resource.name, self.resource.type()) + e.store() + self.assertIsNotNone(e.id) + ev = event_object.Event.get_by_id(self.ctx, e.id) + + errors = [oslo_db.exception.DBError, oslo_db.exception.DBError] + + def side_effect(*args): + try: + raise errors.pop() + except IndexError: + self.assertEqual( + {'Error': 'Resource properties are too large to store'}, + args[1]['resource_properties']) + return ev + + with mock.patch("heat.objects.event.Event") as mock_event: + mock_event.create.side_effect = side_effect + e.store() + + def test_store_fail_one_prop(self): + self.resource.resource_id_set('resource_physical_id') + + e = event.Event(self.ctx, self.stack, 'TEST', 'IN_PROGRESS', 'Testing', + 'alabama', self.resource.properties, + self.resource.name, self.resource.type()) + e.store() + self.assertIsNotNone(e.id) + ev = event_object.Event.get_by_id(self.ctx, e.id) + + errors = [oslo_db.exception.DBError] + + def side_effect(*args): + try: + raise errors.pop() + except IndexError: + self.assertEqual( + {'Foo1': 'zoo', + 'Foo2': '', + 'Foo3': '99999', + 'Error': 'Resource properties are too large to store'}, + args[1]['resource_properties']) + return ev + + with mock.patch("heat.objects.event.Event") as mock_event: + mock_event.create.side_effect = side_effect + e.store()