From b160972b08ba0c1c6aa90459c7c0da7da8452856 Mon Sep 17 00:00:00 2001 From: Thomas Spatzier Date: Mon, 23 Jun 2014 18:54:15 +0200 Subject: [PATCH] Only do property validation in validate() This patch changes resource property validation to only validate properties in the Properties.validate() method. So far, validation was done for each access to a property's value in the __getitem__ method of the Properties class. This causes a lot of un-necessary revalidation and especially with custom constraints for flavors, images etc. this can become very expensive. Doing validation in the validate() method should be sufficient since this invoked as part of the pre-processing of resource actions that receive new or updated properties (CREATE, UPDATE). As part of the change in properties.py the validate_data method of Property was also renamed to get_value with an optional validate flag, since to the most part this method is really used to get a sanitized representation of a property's value with only basic data type checking. Partial-Bug: #1324102 Change-Id: Ibe0aefbe8fa554c0cf227f16f971baedaa52cd8d --- heat/engine/properties.py | 58 +++++----- heat/tests/test_autoscaling.py | 23 ++-- heat/tests/test_autoscaling_update_policy.py | 15 ++- heat/tests/test_instance.py | 29 +++-- heat/tests/test_properties.py | 105 ++++++++++--------- heat/tests/test_server.py | 87 +++++++++++++-- 6 files changed, 214 insertions(+), 103 deletions(-) diff --git a/heat/engine/properties.py b/heat/engine/properties.py index a4861b0510..0d4d5f354a 100644 --- a/heat/engine/properties.py +++ b/heat/engine/properties.py @@ -194,7 +194,7 @@ class Property(object): def support_status(self): return self.schema.support_status - def _validate_integer(self, value): + def _get_integer(self, value): if value is None: value = self.has_default() and self.default() or 0 try: @@ -204,19 +204,19 @@ class Property(object): else: return value - def _validate_number(self, value): + def _get_number(self, value): if value is None: value = self.has_default() and self.default() or 0 return Schema.str_to_num(value) - def _validate_string(self, value): + def _get_string(self, value): if value is None: value = self.has_default() and self.default() or '' if not isinstance(value, basestring): raise ValueError(_('Value must be a string')) return value - def _validate_children(self, child_values, keys=None): + def _get_children(self, child_values, keys=None, validate=False): if self.schema.schema is not None: if keys is None: keys = list(self.schema.schema) @@ -224,30 +224,33 @@ class Property(object): properties = Properties(schemata, dict(child_values), parent_name=self.name, context=self.context) - properties.validate() + if validate: + properties.validate() + return ((k, properties[k]) for k in keys) else: return child_values - def _validate_map(self, value): + def _get_map(self, value, validate=False): if value is None: value = self.has_default() and self.default() or {} if not isinstance(value, collections.Mapping): raise TypeError(_('"%s" is not a map') % value) - return dict(self._validate_children(value.iteritems())) + return dict(self._get_children(value.iteritems(), validate=validate)) - def _validate_list(self, value): + def _get_list(self, value, validate=False): if value is None: value = self.has_default() and self.default() or [] if (not isinstance(value, collections.Sequence) or isinstance(value, basestring)): raise TypeError(_('"%s" is not a list') % repr(value)) - return [v[1] for v in self._validate_children(enumerate(value), - range(len(value)))] + return [v[1] for v in self._get_children(enumerate(value), + range(len(value)), + validate)] - def _validate_bool(self, value): + def _get_bool(self, value): if value is None: value = self.has_default() and self.default() or False if isinstance(value, bool): @@ -258,25 +261,27 @@ class Property(object): return normalised == 'true' - def _validate_data_type(self, value): + def get_value(self, value, validate=False): + """Get value from raw value and sanitize according to data type.""" + t = self.type() if t == Schema.STRING: - return self._validate_string(value) + _value = self._get_string(value) elif t == Schema.INTEGER: - return self._validate_integer(value) + _value = self._get_integer(value) elif t == Schema.NUMBER: - return self._validate_number(value) + _value = self._get_number(value) elif t == Schema.MAP: - return self._validate_map(value) + _value = self._get_map(value, validate) elif t == Schema.LIST: - return self._validate_list(value) + _value = self._get_list(value, validate) elif t == Schema.BOOLEAN: - return self._validate_bool(value) + _value = self._get_bool(value) - def validate_data(self, value): - value = self._validate_data_type(value) - self.schema.validate_constraints(value, self.context) - return value + if validate: + self.schema.validate_constraints(_value, self.context) + + return _value class Properties(collections.Mapping): @@ -311,7 +316,7 @@ class Properties(collections.Mapping): for (key, prop) in self.props.items(): if with_value: try: - self[key] + self._get_property_value(key, validate=True) except ValueError as e: msg = _("Property error : %s") % e raise exception.StackValidationFailed(message=msg) @@ -326,7 +331,7 @@ class Properties(collections.Mapping): msg = _("Unknown Property %s") % key raise exception.StackValidationFailed(message=msg) - def __getitem__(self, key): + def _get_property_value(self, key, validate=False): if key not in self: raise KeyError(_('%(prefix)sInvalid Property %(key)s') % {'prefix': self.error_prefix, 'key': key}) @@ -336,7 +341,7 @@ class Properties(collections.Mapping): if key in self.data: try: value = self.resolve(self.data[key]) - return prop.validate_data(value) + return prop.get_value(value, validate) # the resolver function could raise any number of exceptions, # so handle this generically except Exception as e: @@ -348,6 +353,9 @@ class Properties(collections.Mapping): raise ValueError(_('%(prefix)sProperty %(key)s not assigned') % {'prefix': self.error_prefix, 'key': key}) + def __getitem__(self, key): + return self._get_property_value(key) + def __len__(self): return len(self.props) diff --git a/heat/tests/test_autoscaling.py b/heat/tests/test_autoscaling.py index 7a8b4d617c..2e8555c7e4 100644 --- a/heat/tests/test_autoscaling.py +++ b/heat/tests/test_autoscaling.py @@ -169,10 +169,13 @@ class AutoScalingTest(HeatTestCase): instance.Instance.check_create_complete( cookie).MultipleTimes().AndReturn(True) - def _stub_delete(self, num): + def _stub_image_validate(self, num=1): self.m.StubOutWithMock(image.ImageConstraint, "validate") - image.ImageConstraint.validate( - mox.IgnoreArg(), mox.IgnoreArg()).MultipleTimes().AndReturn(True) + for x in range(num): + image.ImageConstraint.validate( + mox.IgnoreArg(), mox.IgnoreArg()).AndReturn(True) + + def _stub_delete(self, num): self.m.StubOutWithMock(instance.Instance, 'handle_delete') self.m.StubOutWithMock(instance.Instance, 'check_delete_complete') task = object() @@ -185,9 +188,6 @@ class AutoScalingTest(HeatTestCase): def _stub_suspend(self, cookies=[], with_error=None): self.m.StubOutWithMock(instance.Instance, 'handle_suspend') self.m.StubOutWithMock(instance.Instance, 'check_suspend_complete') - self.m.StubOutWithMock(image.ImageConstraint, "validate") - image.ImageConstraint.validate( - mox.IgnoreArg(), mox.IgnoreArg()).MultipleTimes().AndReturn(True) if with_error: instance.Instance.handle_suspend().AndRaise( exception.Error(with_error)) @@ -203,9 +203,6 @@ class AutoScalingTest(HeatTestCase): def _stub_resume(self, cookies=[], with_error=None): self.m.StubOutWithMock(instance.Instance, 'handle_resume') self.m.StubOutWithMock(instance.Instance, 'check_resume_complete') - self.m.StubOutWithMock(image.ImageConstraint, "validate") - image.ImageConstraint.validate( - mox.IgnoreArg(), mox.IgnoreArg()).MultipleTimes().AndReturn(True) if with_error: instance.Instance.handle_resume().AndRaise( exception.Error(with_error)) @@ -889,6 +886,7 @@ class AutoScalingTest(HeatTestCase): # reduce to 1 self._stub_lb_reload(1) self._stub_delete(2) + self._stub_image_validate() self._stub_meta_expected(now, 'ChangeInCapacity : -2') self._stub_scale_notification(adjust=-2, groupname=rsrc.FnGetRefId(), start_capacity=3, end_capacity=1) @@ -909,6 +907,7 @@ class AutoScalingTest(HeatTestCase): # set to 2 self._stub_lb_reload(2) self._stub_delete(1) + self._stub_image_validate(2) self._stub_meta_expected(now, 'ExactCapacity : 2') self._stub_scale_notification(adjust=2, groupname=rsrc.FnGetRefId(), adjust_type='ExactCapacity', @@ -974,6 +973,7 @@ class AutoScalingTest(HeatTestCase): # lower below the min self._stub_lb_reload(1) self._stub_delete(4) + self._stub_image_validate() self._stub_meta_expected(now, 'ChangeInCapacity : -5') self.m.ReplayAll() rsrc.adjust(-5) @@ -1008,6 +1008,7 @@ class AutoScalingTest(HeatTestCase): adjust = 'PercentChangeInCapacity : %d' % decrease self._stub_meta_expected(now, adjust) self._stub_delete(2 - lowest) + self._stub_image_validate() self.m.ReplayAll() rsrc.adjust(decrease, 'PercentChangeInCapacity') self.assertEqual(lowest, len(rsrc.get_instance_names())) @@ -1052,6 +1053,7 @@ class AutoScalingTest(HeatTestCase): # reduce by 50% self._stub_lb_reload(1) self._stub_delete(1) + self._stub_image_validate() self._stub_meta_expected(now, 'PercentChangeInCapacity : -50') self.m.ReplayAll() rsrc.adjust(-50, 'PercentChangeInCapacity') @@ -1103,6 +1105,7 @@ class AutoScalingTest(HeatTestCase): # reduce by 50% self._stub_lb_reload(1) self._stub_delete(1) + self._stub_image_validate() self._stub_meta_expected(now, 'PercentChangeInCapacity : -50') self.m.ReplayAll() rsrc.adjust(-50, 'PercentChangeInCapacity') @@ -1157,6 +1160,7 @@ class AutoScalingTest(HeatTestCase): self._stub_lb_reload(1) self._stub_meta_expected(now, 'PercentChangeInCapacity : -50') self._stub_delete(1) + self._stub_image_validate() self.m.ReplayAll() rsrc.adjust(-50, 'PercentChangeInCapacity') self.assertEqual(1, len(rsrc.get_instance_names())) @@ -1304,6 +1308,7 @@ class AutoScalingTest(HeatTestCase): # Scale down one self._stub_lb_reload(1) self._stub_delete(1) + self._stub_image_validate() self._stub_meta_expected(now, 'ChangeInCapacity : -1', 2) self.m.ReplayAll() diff --git a/heat/tests/test_autoscaling_update_policy.py b/heat/tests/test_autoscaling_update_policy.py index 782dd18b27..fc2028f2cd 100644 --- a/heat/tests/test_autoscaling_update_policy.py +++ b/heat/tests/test_autoscaling_update_policy.py @@ -217,10 +217,17 @@ class AutoScalingGroupTest(HeatTestCase): clients.OpenStackClients.glance().MultipleTimes().AndReturn( g_cli_mock) self.m.StubOutWithMock(glance_utils, 'get_image_id') - glance_utils.get_image_id(g_cli_mock, imageId_input).MultipleTimes().\ - AndReturn(imageId) - if update_image: - glance_utils.get_image_id(g_cli_mock, update_image).\ + + # If update_image is None (create case), validation for initial image + # imageId_input will be invoked multiple times (for each server). + # If update_image is set (update case), validation of the old property + # values and new property values will be done, but the order is not + # deterministic. Therefore, using mox.IgnoreArg() for the update case. + if update_image is None: + glance_utils.get_image_id(g_cli_mock, imageId_input).\ + MultipleTimes().AndReturn(imageId) + else: + glance_utils.get_image_id(g_cli_mock, mox.IgnoreArg()).\ MultipleTimes().AndReturn(imageId) def _stub_validate(self): diff --git a/heat/tests/test_instance.py b/heat/tests/test_instance.py index 48d4e0607b..76d29f32b6 100644 --- a/heat/tests/test_instance.py +++ b/heat/tests/test_instance.py @@ -187,12 +187,18 @@ class InstancesTest(HeatTestCase): instance = instances.Instance('instance_create_image_err', resource_defns['WebServer'], stack) - g_cli_moc = self.m.CreateMockAnything() - self.m.StubOutWithMock(clients.OpenStackClients, 'glance') - clients.OpenStackClients.glance().MultipleTimes().AndReturn(g_cli_moc) + self._mock_get_image_id_fail('Slackware', + exception.ImageNotFound( + image_name='Slackware')) self.m.ReplayAll() - self.assertRaises(ValueError, instance.handle_create) + create = scheduler.TaskRunner(instance.create) + error = self.assertRaises(exception.ResourceFailure, create) + self.assertEqual( + 'StackValidationFailed: Property error : WebServer: ' + 'ImageId Error validating value \'Slackware\': ' + 'The Image (Slackware) could not be found.', + str(error)) self.m.VerifyAll() @@ -213,7 +219,13 @@ class InstancesTest(HeatTestCase): self.m.ReplayAll() - self.assertRaises(ValueError, instance.handle_create) + create = scheduler.TaskRunner(instance.create) + error = self.assertRaises(exception.ResourceFailure, create) + self.assertEqual( + 'StackValidationFailed: Property error : WebServer: ' + 'ImageId Multiple physical resources were ' + 'found with name (CentOS 5.2).', + str(error)) self.m.VerifyAll() @@ -231,7 +243,12 @@ class InstancesTest(HeatTestCase): self.m.ReplayAll() - self.assertRaises(ValueError, instance.handle_create) + create = scheduler.TaskRunner(instance.create) + error = self.assertRaises(exception.ResourceFailure, create) + self.assertEqual( + 'StackValidationFailed: Property error : WebServer: ' + 'ImageId 404 (HTTP 404)', + str(error)) self.m.VerifyAll() diff --git a/heat/tests/test_properties.py b/heat/tests/test_properties.py index 0047f64617..b20c71de98 100644 --- a/heat/tests/test_properties.py +++ b/heat/tests/test_properties.py @@ -616,123 +616,126 @@ class PropertyTest(testtools.TestCase): schema = {'Type': 'String', 'AllowedPattern': '[a-z]*'} p = properties.Property(schema) - self.assertEqual('foo', p.validate_data('foo')) + self.assertEqual('foo', p.get_value('foo', True)) def test_string_pattern_bad_prefix(self): schema = {'Type': 'String', 'AllowedPattern': '[a-z]*'} p = properties.Property(schema) self.assertRaises(exception.StackValidationFailed, - p.validate_data, '1foo') + p.get_value, '1foo', True) def test_string_pattern_bad_suffix(self): schema = {'Type': 'String', 'AllowedPattern': '[a-z]*'} p = properties.Property(schema) self.assertRaises(exception.StackValidationFailed, - p.validate_data, 'foo1') + p.get_value, 'foo1', True) def test_string_value_list_good(self): schema = {'Type': 'String', 'AllowedValues': ['foo', 'bar', 'baz']} p = properties.Property(schema) - self.assertEqual('bar', p.validate_data('bar')) + self.assertEqual('bar', p.get_value('bar', True)) def test_string_value_list_bad(self): schema = {'Type': 'String', 'AllowedValues': ['foo', 'bar', 'baz']} p = properties.Property(schema) self.assertRaises(exception.StackValidationFailed, - p.validate_data, 'blarg') + p.get_value, 'blarg', True) def test_string_maxlength_good(self): schema = {'Type': 'String', 'MaxLength': '5'} p = properties.Property(schema) - self.assertEqual('abcd', p.validate_data('abcd')) + self.assertEqual('abcd', p.get_value('abcd', True)) def test_string_exceeded_maxlength(self): schema = {'Type': 'String', 'MaxLength': '5'} p = properties.Property(schema) self.assertRaises(exception.StackValidationFailed, - p.validate_data, 'abcdef') + p.get_value, 'abcdef', True) def test_string_length_in_range(self): schema = {'Type': 'String', 'MinLength': '5', 'MaxLength': '10'} p = properties.Property(schema) - self.assertEqual('abcdef', p.validate_data('abcdef')) + self.assertEqual('abcdef', p.get_value('abcdef', True)) def test_string_minlength_good(self): schema = {'Type': 'String', 'MinLength': '5'} p = properties.Property(schema) - self.assertEqual('abcde', p.validate_data('abcde')) + self.assertEqual('abcde', p.get_value('abcde', True)) def test_string_smaller_than_minlength(self): schema = {'Type': 'String', 'MinLength': '5'} p = properties.Property(schema) self.assertRaises(exception.StackValidationFailed, - p.validate_data, 'abcd') + p.get_value, 'abcd', True) def test_int_good(self): schema = {'Type': 'Integer', 'MinValue': 3, 'MaxValue': 3} p = properties.Property(schema) - self.assertEqual(3, p.validate_data(3)) + self.assertEqual(3, p.get_value(3, True)) def test_int_bad(self): schema = {'Type': 'Integer'} p = properties.Property(schema) - ex = self.assertRaises(TypeError, p.validate_data, [1]) + ex = self.assertRaises(TypeError, p.get_value, [1]) self.assertEqual("int() argument must be a string or a number, " "not 'list'", str(ex)) def test_int_from_str_good(self): schema = {'Type': 'Integer'} p = properties.Property(schema) - self.assertEqual(3, p.validate_data('3')) + self.assertEqual(3, p.get_value('3')) def test_int_from_str_bad(self): schema = {'Type': 'Integer'} p = properties.Property(schema) - ex = self.assertRaises(TypeError, p.validate_data, '3a') + ex = self.assertRaises(TypeError, p.get_value, '3a') self.assertEqual("Value '3a' is not an integer", str(ex)) def test_integer_low(self): schema = {'Type': 'Integer', 'MinValue': 4} p = properties.Property(schema) - self.assertRaises(exception.StackValidationFailed, p.validate_data, 3) + self.assertRaises(exception.StackValidationFailed, p.get_value, 3, + True) def test_integer_high(self): schema = {'Type': 'Integer', 'MaxValue': 2} p = properties.Property(schema) - self.assertRaises(exception.StackValidationFailed, p.validate_data, 3) + self.assertRaises(exception.StackValidationFailed, p.get_value, 3, + True) def test_integer_value_list_good(self): schema = {'Type': 'Integer', 'AllowedValues': [1, 3, 5]} p = properties.Property(schema) - self.assertEqual(5, p.validate_data(5)) + self.assertEqual(5, p.get_value(5), True) def test_integer_value_list_bad(self): schema = {'Type': 'Integer', 'AllowedValues': [1, 3, 5]} p = properties.Property(schema) - self.assertRaises(exception.StackValidationFailed, p.validate_data, 2) + self.assertRaises(exception.StackValidationFailed, p.get_value, 2, + True) def test_number_good(self): schema = {'Type': 'Number', 'MinValue': '3', 'MaxValue': '3'} p = properties.Property(schema) - self.assertEqual(3, p.validate_data(3)) + self.assertEqual(3, p.get_value(3, True)) def test_numbers_from_strings(self): """Numbers can be converted from strings.""" @@ -740,127 +743,127 @@ class PropertyTest(testtools.TestCase): 'MinValue': '3', 'MaxValue': '3'} p = properties.Property(schema) - self.assertEqual(3, p.validate_data('3')) + self.assertEqual(3, p.get_value('3')) def test_number_value_list_good(self): schema = {'Type': 'Number', 'AllowedValues': [1, 3, 5]} p = properties.Property(schema) - self.assertEqual(5, p.validate_data('5')) + self.assertEqual(5, p.get_value('5', True)) def test_number_value_list_bad(self): schema = {'Type': 'Number', 'AllowedValues': ['1', '3', '5']} p = properties.Property(schema) self.assertRaises(exception.StackValidationFailed, - p.validate_data, '2') + p.get_value, '2', True) def test_number_low(self): schema = {'Type': 'Number', 'MinValue': '4'} p = properties.Property(schema) self.assertRaises(exception.StackValidationFailed, - p.validate_data, '3') + p.get_value, '3', True) def test_number_high(self): schema = {'Type': 'Number', 'MaxValue': '2'} p = properties.Property(schema) self.assertRaises(exception.StackValidationFailed, - p.validate_data, '3') + p.get_value, '3', True) def test_boolean_true(self): p = properties.Property({'Type': 'Boolean'}) - self.assertIs(True, p.validate_data('True')) - self.assertIs(True, p.validate_data('true')) - self.assertIs(True, p.validate_data(True)) + self.assertIs(True, p.get_value('True')) + self.assertIs(True, p.get_value('true')) + self.assertIs(True, p.get_value(True)) def test_boolean_false(self): p = properties.Property({'Type': 'Boolean'}) - self.assertIs(False, p.validate_data('False')) - self.assertIs(False, p.validate_data('false')) - self.assertIs(False, p.validate_data(False)) + self.assertIs(False, p.get_value('False')) + self.assertIs(False, p.get_value('false')) + self.assertIs(False, p.get_value(False)) def test_boolean_invalid(self): p = properties.Property({'Type': 'Boolean'}) - self.assertRaises(ValueError, p.validate_data, 'fish') + self.assertRaises(ValueError, p.get_value, 'fish') def test_list_string(self): p = properties.Property({'Type': 'List'}) - self.assertRaises(TypeError, p.validate_data, 'foo') + self.assertRaises(TypeError, p.get_value, 'foo') def test_list_good(self): p = properties.Property({'Type': 'List'}) - self.assertEqual(['foo', 'bar'], p.validate_data(['foo', 'bar'])) + self.assertEqual(['foo', 'bar'], p.get_value(['foo', 'bar'])) def test_list_dict(self): p = properties.Property({'Type': 'List'}) - self.assertRaises(TypeError, p.validate_data, {'foo': 'bar'}) + self.assertRaises(TypeError, p.get_value, {'foo': 'bar'}) def test_list_maxlength_good(self): schema = {'Type': 'List', 'MaxLength': '3'} p = properties.Property(schema) - self.assertEqual(['1', '2'], p.validate_data(['1', '2'])) + self.assertEqual(['1', '2'], p.get_value(['1', '2'], True)) def test_list_exceeded_maxlength(self): schema = {'Type': 'List', 'MaxLength': '2'} p = properties.Property(schema) self.assertRaises(exception.StackValidationFailed, - p.validate_data, ['1', '2', '3']) + p.get_value, ['1', '2', '3'], True) def test_list_length_in_range(self): schema = {'Type': 'List', 'MinLength': '2', 'MaxLength': '4'} p = properties.Property(schema) - self.assertEqual(['1', '2', '3'], p.validate_data(['1', '2', '3'])) + self.assertEqual(['1', '2', '3'], p.get_value(['1', '2', '3'], True)) def test_list_minlength_good(self): schema = {'Type': 'List', 'MinLength': '3'} p = properties.Property(schema) - self.assertEqual(['1', '2', '3'], p.validate_data(['1', '2', '3'])) + self.assertEqual(['1', '2', '3'], p.get_value(['1', '2', '3'], True)) def test_list_smaller_than_minlength(self): schema = {'Type': 'List', 'MinLength': '4'} p = properties.Property(schema) self.assertRaises(exception.StackValidationFailed, - p.validate_data, ['1', '2', '3']) + p.get_value, ['1', '2', '3'], True) def test_map_string(self): p = properties.Property({'Type': 'Map'}) - self.assertRaises(TypeError, p.validate_data, 'foo') + self.assertRaises(TypeError, p.get_value, 'foo') def test_map_list(self): p = properties.Property({'Type': 'Map'}) - self.assertRaises(TypeError, p.validate_data, ['foo']) + self.assertRaises(TypeError, p.get_value, ['foo']) def test_map_schema_good(self): map_schema = {'valid': {'Type': 'Boolean'}} p = properties.Property({'Type': 'Map', 'Schema': map_schema}) - self.assertEqual({'valid': True}, p.validate_data({'valid': 'TRUE'})) + self.assertEqual({'valid': True}, p.get_value({'valid': 'TRUE'})) def test_map_schema_bad_data(self): map_schema = {'valid': {'Type': 'Boolean'}} p = properties.Property({'Type': 'Map', 'Schema': map_schema}) ex = self.assertRaises(exception.StackValidationFailed, - p.validate_data, {'valid': 'fish'}) + p.get_value, {'valid': 'fish'}, True) self.assertEqual('Property error : valid "fish" is not ' 'a valid boolean', str(ex)) def test_map_schema_missing_data(self): map_schema = {'valid': {'Type': 'Boolean'}} p = properties.Property({'Type': 'Map', 'Schema': map_schema}) - self.assertEqual({'valid': None}, p.validate_data({})) + self.assertEqual({'valid': None}, p.get_value({})) def test_map_schema_missing_required_data(self): map_schema = {'valid': {'Type': 'Boolean', 'Required': True}} p = properties.Property({'Type': 'Map', 'Schema': map_schema}) ex = self.assertRaises(exception.StackValidationFailed, - p.validate_data, {}) + p.get_value, {}, True) self.assertEqual('Property error : Property valid not assigned', str(ex)) @@ -870,29 +873,29 @@ class PropertyTest(testtools.TestCase): p = properties.Property({'Type': 'List', 'Schema': list_schema}) self.assertEqual([{'valid': True}, {'valid': False}], - p.validate_data([{'valid': 'TRUE'}, - {'valid': 'False'}])) + p.get_value([{'valid': 'TRUE'}, + {'valid': 'False'}])) def test_list_schema_bad_data(self): map_schema = {'valid': {'Type': 'Boolean'}} list_schema = {'Type': 'Map', 'Schema': map_schema} p = properties.Property({'Type': 'List', 'Schema': list_schema}) ex = self.assertRaises(exception.StackValidationFailed, - p.validate_data, - [{'valid': 'True'}, {'valid': 'fish'}]) + p.get_value, + [{'valid': 'True'}, {'valid': 'fish'}], True) self.assertEqual('Property error : 1 Property error : 1: valid ' '"fish" is not a valid boolean', str(ex)) def test_list_schema_int_good(self): list_schema = {'Type': 'Integer'} p = properties.Property({'Type': 'List', 'Schema': list_schema}) - self.assertEqual([1, 2, 3], p.validate_data([1, 2, 3])) + self.assertEqual([1, 2, 3], p.get_value([1, 2, 3])) def test_list_schema_int_bad_data(self): list_schema = {'Type': 'Integer'} p = properties.Property({'Type': 'List', 'Schema': list_schema}) ex = self.assertRaises(exception.StackValidationFailed, - p.validate_data, [42, 'fish']) + p.get_value, [42, 'fish'], True) self.assertEqual('Property error : 1 Value \'fish\' is not ' 'an integer', str(ex)) diff --git a/heat/tests/test_server.py b/heat/tests/test_server.py index 086c970e31..3c9e8b9a4e 100644 --- a/heat/tests/test_server.py +++ b/heat/tests/test_server.py @@ -184,6 +184,15 @@ class ServersTest(HeatTestCase): self.m.StubOutWithMock(glance_utils, 'get_image_id') glance_utils.get_image_id(g_cli_mock, image_id).AndRaise(exp) + def _mock_get_keypair_success(self, keypair_input, keypair): + n_cli_mock = self.m.CreateMockAnything() + self.m.StubOutWithMock(clients.OpenStackClients, 'nova') + clients.OpenStackClients.nova().MultipleTimes().AndReturn( + n_cli_mock) + self.m.StubOutWithMock(nova_utils, 'get_keypair') + nova_utils.get_keypair(n_cli_mock, keypair_input).MultipleTimes().\ + AndReturn(keypair) + def _server_validate_mock(self, server): self.m.StubOutWithMock(server, 'nova') server.nova().MultipleTimes().AndReturn(self.fc) @@ -340,12 +349,15 @@ class ServersTest(HeatTestCase): self._mock_get_image_id_fail('Slackware', exception.ImageNotFound( image_name='Slackware')) + self._mock_get_keypair_success('test', ('test', 'abc123')) self.m.ReplayAll() - error = self.assertRaises(ValueError, server.handle_create) + create = scheduler.TaskRunner(server.create) + error = self.assertRaises(exception.ResourceFailure, create) self.assertEqual( - 'WebServer: image Error validating value ' - '\'Slackware\': The Image (Slackware) could not be found.', + 'StackValidationFailed: Property error : WebServer: ' + 'image Error validating value \'Slackware\': ' + 'The Image (Slackware) could not be found.', str(error)) self.m.VerifyAll() @@ -363,11 +375,14 @@ class ServersTest(HeatTestCase): self._mock_get_image_id_fail('CentOS 5.2', exception.PhysicalResourceNameAmbiguity( name='CentOS 5.2')) + self._mock_get_keypair_success('test', ('test', 'abc123')) self.m.ReplayAll() - error = self.assertRaises(ValueError, server.handle_create) + create = scheduler.TaskRunner(server.create) + error = self.assertRaises(exception.ResourceFailure, create) self.assertEqual( - 'WebServer: image Multiple physical resources were ' + 'StackValidationFailed: Property error : WebServer: ' + 'image Multiple physical resources were ' 'found with name (CentOS 5.2).', str(error)) @@ -385,12 +400,14 @@ class ServersTest(HeatTestCase): self._mock_get_image_id_fail('1', exception.ImageNotFound(image_name='1')) - + self._mock_get_keypair_success('test', ('test', 'abc123')) self.m.ReplayAll() - error = self.assertRaises(ValueError, server.handle_create) + create = scheduler.TaskRunner(server.create) + error = self.assertRaises(exception.ResourceFailure, create) self.assertEqual( - 'WebServer: image Error validating value \'1\': ' + 'StackValidationFailed: Property error : WebServer: ' + 'image Error validating value \'1\': ' 'The Image (1) could not be found.', str(error)) @@ -2459,6 +2476,60 @@ class ServersTest(HeatTestCase): self.assertEqual((server.UPDATE, server.COMPLETE), server.state) self.m.VerifyAll() + def test_server_properties_validation_create_and_update(self): + return_server = self.fc.servers.list()[1] + + self.m.StubOutWithMock(image.ImageConstraint, "validate") + # verify that validate gets invoked exactly once for create + image.ImageConstraint.validate( + 'CentOS 5.2', mox.IgnoreArg()).AndReturn(True) + # verify that validate gets invoked exactly once for update + image.ImageConstraint.validate( + 'Update Image', mox.IgnoreArg()).AndReturn(True) + self.m.ReplayAll() + + # create + server = self._create_test_server(return_server, + 'my_server') + + update_template = copy.deepcopy(server.t) + update_template['Properties']['image'] = 'Update Image' + + #update + updater = scheduler.TaskRunner(server.update, update_template) + self.assertRaises(resource.UpdateReplace, updater) + + self.m.VerifyAll() + + def test_server_properties_validation_create_and_update_fail(self): + return_server = self.fc.servers.list()[1] + + self.m.StubOutWithMock(image.ImageConstraint, "validate") + # verify that validate gets invoked exactly once for create + image.ImageConstraint.validate( + 'CentOS 5.2', mox.IgnoreArg()).AndReturn(True) + # verify that validate gets invoked exactly once for update + ex = exception.ImageNotFound(image_name='Update Image') + image.ImageConstraint.validate('Update Image', + mox.IgnoreArg()).AndRaise(ex) + self.m.ReplayAll() + + # create + server = self._create_test_server(return_server, + 'my_server') + + update_template = copy.deepcopy(server.t) + update_template['Properties']['image'] = 'Update Image' + + #update + updater = scheduler.TaskRunner(server.update, update_template) + err = self.assertRaises(exception.ResourceFailure, updater) + self.assertEqual('StackValidationFailed: Property error : WebServer: ' + 'image The Image (Update Image) could not be found.', + str(err)) + + self.m.VerifyAll() + class FlavorConstraintTest(HeatTestCase):