From 0266de616c1ef45e7c089021fc2a4021d9191978 Mon Sep 17 00:00:00 2001 From: Dmitriy Uvarenkov Date: Wed, 28 Sep 2016 14:24:28 +0300 Subject: [PATCH] Add sahara constraints Add few constraints for better stack validation. Also, refactor old sahara constraint tests to make everything more compact. Change-Id: I7157fdc7077bddfda432a40f2597463ae23577bb --- heat/engine/clients/os/sahara.py | 94 +++++++++++++------ .../resources/openstack/sahara/cluster.py | 5 +- heat/engine/resources/openstack/sahara/job.py | 32 ++++--- heat/tests/clients/test_sahara_client.py | 91 +++++++++++------- heat/tests/openstack/sahara/test_job.py | 14 +-- setup.cfg | 5 + 6 files changed, 160 insertions(+), 81 deletions(-) diff --git a/heat/engine/clients/os/sahara.py b/heat/engine/clients/os/sahara.py index 9990ad29b5..7f82b2e29f 100644 --- a/heat/engine/clients/os/sahara.py +++ b/heat/engine/clients/os/sahara.py @@ -13,7 +13,6 @@ # See the License for the specific language governing permissions and # limitations under the License. -from oslo_utils import uuidutils from saharaclient.api import base as sahara_base from saharaclient import client as sahara_client import six @@ -72,6 +71,21 @@ class SaharaClientPlugin(client_plugin.ClientPlugin): ex.error_code == 400 and ex.error_name == 'IMAGE_NOT_REGISTERED') + def find_resource_by_name_or_id(self, resource_name, value): + """Return the ID for the specified name or identifier. + + :param resource_name: API name of entity + :param value: ID or name of entity + :returns: the id of the requested :value: + :raises: exception.EntityNotFound, + exception.PhysicalResourceNameAmbiguity + """ + try: + entity = getattr(self.client(), resource_name) + return entity.get(value).id + except sahara_base.APIException: + return self.find_resource_by_name(resource_name, value) + def get_image_id(self, image_identifier): """Return the ID for the specified image name or identifier. @@ -80,40 +94,37 @@ class SaharaClientPlugin(client_plugin.ClientPlugin): :raises: exception.EntityNotFound, exception.PhysicalResourceNameAmbiguity """ - if uuidutils.is_uuid_like(image_identifier): - try: - image_id = self.client().images.get(image_identifier).id - except sahara_base.APIException as ex: - if self.is_not_registered(ex): - image_id = self.get_image_id_by_name(image_identifier) - else: - image_id = self.get_image_id_by_name(image_identifier) - return image_id + # leave this method for backward compatibility + try: + return self.find_resource_by_name_or_id('images', image_identifier) + except exception.EntityNotFound: + raise exception.EntityNotFound(entity='Image', + name=image_identifier) - def get_image_id_by_name(self, image_identifier): - """Return the ID for the specified image name. + def find_resource_by_name(self, resource_name, value): + """Return the ID for the specified entity name. - :param image_identifier: image name - :returns: the id of the requested :image_identifier: :raises: exception.EntityNotFound, exception.PhysicalResourceNameAmbiguity """ try: - filters = {'name': image_identifier} - image_list = self.client().images.find(**filters) + filters = {'name': value} + obj = getattr(self.client(), resource_name) + obj_list = obj.find(**filters) except sahara_base.APIException as ex: raise exception.Error( - _("Error retrieving image list from sahara: " - "%s") % six.text_type(ex)) - num_matches = len(image_list) + _("Error retrieving %(entity)s list from sahara: " + "%(err)s") % dict(entity=resource_name, + err=six.text_type(ex))) + num_matches = len(obj_list) if num_matches == 0: - raise exception.EntityNotFound(entity='Image', - name=image_identifier) + raise exception.EntityNotFound(entity=resource_name or 'entity', + name=value) elif num_matches > 1: raise exception.PhysicalResourceNameAmbiguity( - name=image_identifier) + name=value) else: - return image_list[0].id + return obj_list[0].id def get_plugin_id(self, plugin_name): """Get the id for the specified plugin name. @@ -130,16 +141,41 @@ class SaharaClientPlugin(client_plugin.ClientPlugin): class SaharaBaseConstraint(constraints.BaseCustomConstraint): + expected_exceptions = (exception.EntityNotFound, + exception.PhysicalResourceNameAmbiguity,) + resource_name = None + + def validate_with_client(self, client, resource_id): + sahara_plugin = client.client_plugin(CLIENT_NAME) + sahara_plugin.find_resource_by_name_or_id(self.resource_name, + resource_id) + + +class PluginConstraint(constraints.BaseCustomConstraint): + # do not touch constraint for compatibility resource_client_name = CLIENT_NAME + resource_getter_name = 'get_plugin_id' class ImageConstraint(SaharaBaseConstraint): - - expected_exceptions = (exception.EntityNotFound, - exception.PhysicalResourceNameAmbiguity,) - resource_getter_name = 'get_image_id' + resource_name = 'images' -class PluginConstraint(SaharaBaseConstraint): +class JobBinaryConstraint(SaharaBaseConstraint): + resource_name = 'job_binaries' - resource_getter_name = 'get_plugin_id' + +class ClusterConstraint(SaharaBaseConstraint): + resource_name = 'clusters' + + +class DataSourceConstraint(SaharaBaseConstraint): + resource_name = 'data_sources' + + +class ClusterTemplateConstraint(SaharaBaseConstraint): + resource_name = 'cluster_templates' + + +class JobTypeConstraint(SaharaBaseConstraint): + resource_name = 'job_types' diff --git a/heat/engine/resources/openstack/sahara/cluster.py b/heat/engine/resources/openstack/sahara/cluster.py index 1852cf28bb..d88e995ed5 100644 --- a/heat/engine/resources/openstack/sahara/cluster.py +++ b/heat/engine/resources/openstack/sahara/cluster.py @@ -88,7 +88,10 @@ class SaharaCluster(resource.Resource): properties.Schema.STRING, _('ID of the Cluster Template used for ' 'Node Groups and configurations.'), - required=True, + constraints=[ + constraints.CustomConstraint('sahara.cluster_template') + ], + required=True ), KEY_NAME: properties.Schema( properties.Schema.STRING, diff --git a/heat/engine/resources/openstack/sahara/job.py b/heat/engine/resources/openstack/sahara/job.py index 784f6fcd17..16eb8076ab 100644 --- a/heat/engine/resources/openstack/sahara/job.py +++ b/heat/engine/resources/openstack/sahara/job.py @@ -58,9 +58,6 @@ class SaharaJob(signal_responder.SignalResponder, resource.Resource): 'executions', 'default_execution_url' ) - JOB_TYPES = ['Hive', 'Java', 'MapReduce', 'MapReduce.Streaming', - 'Pig', 'Shell', 'Spark', 'Storm', 'Storm.Pyleus'] - properties_schema = { NAME: properties.Schema( properties.Schema.STRING, @@ -74,7 +71,9 @@ class SaharaJob(signal_responder.SignalResponder, resource.Resource): TYPE: properties.Schema( properties.Schema.STRING, _("Type of the job."), - constraints=[constraints.AllowedValues(JOB_TYPES)], + constraints=[ + constraints.CustomConstraint('sahara.job_type') + ], required=True ), MAINS: properties.Schema( @@ -84,18 +83,20 @@ class SaharaJob(signal_responder.SignalResponder, resource.Resource): "one item."), schema=properties.Schema( properties.Schema.STRING, - _("ID of job's main job binary.") + _("ID of job's main job binary."), + constraints=[constraints.CustomConstraint('sahara.job_binary')] ), - constraints=[ - constraints.Length(max=1) - ], + constraints=[constraints.Length(max=1)], default=[] ), LIBS: properties.Schema( properties.Schema.LIST, _("IDs of job's lib job binaries."), schema=properties.Schema( - properties.Schema.STRING + properties.Schema.STRING, + constraints=[ + constraints.CustomConstraint('sahara.job_binary') + ] ), default=[] ), @@ -124,15 +125,24 @@ class SaharaJob(signal_responder.SignalResponder, resource.Resource): CLUSTER: properties.Schema( properties.Schema.STRING, _('ID of the cluster to run the job in.'), + constraints=[ + constraints.CustomConstraint('sahara.cluster') + ], required=True ), INPUT: properties.Schema( properties.Schema.STRING, - _('ID of the input data source.') + _('ID of the input data source.'), + constraints=[ + constraints.CustomConstraint('sahara.data_source') + ] ), OUTPUT: properties.Schema( properties.Schema.STRING, - _('ID of the output data source.') + _('ID of the output data source.'), + constraints=[ + constraints.CustomConstraint('sahara.data_source') + ] ), CONFIGS: properties.Schema( properties.Schema.MAP, diff --git a/heat/tests/clients/test_sahara_client.py b/heat/tests/clients/test_sahara_client.py index bf8f9f6313..33189a459a 100644 --- a/heat/tests/clients/test_sahara_client.py +++ b/heat/tests/clients/test_sahara_client.py @@ -42,7 +42,11 @@ class SaharaUtilsTest(common.HeatTestCase): img_name = 'myfakeimage' self.my_image.id = img_id self.my_image.name = img_name - self.sahara_client.images.get.return_value = self.my_image + self.sahara_client.images.get.side_effect = [ + self.my_image, + sahara_base.APIException(404), + sahara_base.APIException(404) + ] self.sahara_client.images.find.side_effect = [[self.my_image], []] self.assertEqual(img_id, self.sahara_plugin.get_image_id(img_id)) @@ -52,7 +56,6 @@ class SaharaUtilsTest(common.HeatTestCase): calls = [mock.call(name=img_name), mock.call(name='noimage')] - self.sahara_client.images.get.assert_called_once_with(img_id) self.sahara_client.images.find.assert_has_calls(calls) def test_get_image_id_by_name_in_uuid(self): @@ -78,10 +81,10 @@ class SaharaUtilsTest(common.HeatTestCase): self.sahara_client.images.find.side_effect = [ sahara_base.APIException(error_message="Error", error_code=404)] - expected_error = "Error retrieving image list from sahara: Error" + expected_error = "Error retrieving images list from sahara: Error" e = self.assertRaises(exception.Error, - self.sahara_plugin.get_image_id_by_name, - img_name) + self.sahara_plugin.find_resource_by_name, + 'images', img_name) self.assertEqual(expected_error, six.text_type(e)) self.sahara_client.images.find.assert_called_once_with(name=img_name) @@ -106,6 +109,7 @@ class SaharaUtilsTest(common.HeatTestCase): img_name = 'ambiguity_name' self.my_image.name = img_name + self.sahara_client.images.get.side_effect = sahara_base.APIException() self.sahara_client.images.find.return_value = [self.my_image, self.my_image] self.assertRaises(exception.PhysicalResourceNameAmbiguity, @@ -152,41 +156,60 @@ class SaharaUtilsTest(common.HeatTestCase): self.sahara_client.plugins.get.assert_has_calls(calls) -class ImageConstraintTest(common.HeatTestCase): +class SaharaConstraintsTest(common.HeatTestCase): + scenarios = [ + ('JobType', dict( + constraint=sahara.JobTypeConstraint(), + resource_name='job_types' + )), + ('ClusterTemplate', dict( + constraint=sahara.ClusterTemplateConstraint(), + resource_name='cluster_templates' + )), + ('DataSource', dict( + constraint=sahara.DataSourceConstraint(), + resource_name='data_sources' + )), + ('Cluster', dict( + constraint=sahara.ClusterConstraint(), + resource_name='clusters' + )), + ('JobBinary', dict( + constraint=sahara.JobBinaryConstraint(), + resource_name='job_binaries' + )), + ('Plugin', dict( + constraint=sahara.PluginConstraint(), + resource_name=None + )), + ('Image', dict( + constraint=sahara.ImageConstraint(), + resource_name='images' + )), + ] def setUp(self): - super(ImageConstraintTest, self).setUp() + super(SaharaConstraintsTest, self).setUp() self.ctx = utils.dummy_context() - self.mock_get_image = mock.Mock() - self.ctx.clients.client_plugin( - 'sahara').get_image_id = self.mock_get_image - self.constraint = sahara.ImageConstraint() + self.mock_get = mock.Mock() + cl_plgn = self.ctx.clients.client_plugin('sahara') + cl_plgn.find_resource_by_name_or_id = self.mock_get + cl_plgn.get_image_id = self.mock_get + cl_plgn.get_plugin_id = self.mock_get def test_validation(self): - self.mock_get_image.return_value = "id1" + self.mock_get.return_value = "fake_val" self.assertTrue(self.constraint.validate("foo", self.ctx)) + if self.resource_name is None: + self.mock_get.assert_called_once_with("foo") + else: + self.mock_get.assert_called_once_with(self.resource_name, "foo") def test_validation_error(self): - self.mock_get_image.side_effect = exception.EntityNotFound( - entity='Image', name='bar') - self.assertFalse(self.constraint.validate("bar", self.ctx)) - - -class PluginConstraintTest(common.HeatTestCase): - - def setUp(self): - super(PluginConstraintTest, self).setUp() - self.ctx = utils.dummy_context() - self.mock_get_plugin = mock.Mock() - self.ctx.clients.client_plugin( - 'sahara').get_plugin_id = self.mock_get_plugin - self.constraint = sahara.PluginConstraint() - - def test_validation(self): - self.mock_get_plugin.return_value = "id1" - self.assertTrue(self.constraint.validate("foo", self.ctx)) - - def test_validation_error(self): - self.mock_get_plugin.side_effect = exception.EntityNotFound( - entity='Plugin', name='bar') + self.mock_get.side_effect = exception.EntityNotFound( + entity='Fake entity', name='bar') self.assertFalse(self.constraint.validate("bar", self.ctx)) + if self.resource_name is None: + self.mock_get.assert_called_once_with("bar") + else: + self.mock_get.assert_called_once_with(self.resource_name, "bar") diff --git a/heat/tests/openstack/sahara/test_job.py b/heat/tests/openstack/sahara/test_job.py index 55440c10f8..45aeab2cf3 100644 --- a/heat/tests/openstack/sahara/test_job.py +++ b/heat/tests/openstack/sahara/test_job.py @@ -61,10 +61,16 @@ class SaharaJobTest(common.HeatTestCase): 'id': 'fake-execution-id'} self.client.job_executions.find.return_value = [fake_execution] - def _create_resource(self, name, snippet, stack): + def _create_resource(self, name, snippet, stack, without_name=False): jb = job.SaharaJob(name, snippet, stack) + if without_name: + self.client.jobs.create = mock.Mock(return_value='fake_rsrc_id') + jb.physical_resource_name = mock.Mock( + return_value='fake_phys_name') value = mock.MagicMock(id='fake-resource-id') self.client.jobs.create.return_value = value + mock_get_res = mock.Mock(return_value='some res') + jb.client_plugin().find_resource_by_name_or_id = mock_get_res scheduler.TaskRunner(jb.create)() return jb @@ -89,11 +95,7 @@ class SaharaJobTest(common.HeatTestCase): props = self.stack.t.t['resources']['job']['properties'] del props['name'] self.rsrc_defn = self.rsrc_defn.freeze(properties=props) - jb = job.SaharaJob('job', self.rsrc_defn, self.stack) - value = mock.MagicMock(id='fake-resource-id') - self.client.jobs.create.return_value = value - jb.physical_resource_name = mock.Mock(return_value='fake_phys_name') - scheduler.TaskRunner(jb.create)() + jb = self._create_resource('job', self.rsrc_defn, self.stack, True) args = self.client.jobs.create.call_args[1] expected_args = { 'name': 'fake_phys_name', diff --git a/setup.cfg b/setup.cfg index 1ec855a694..3ba30c180c 100644 --- a/setup.cfg +++ b/setup.cfg @@ -138,7 +138,12 @@ heat.constraints = nova.keypair = heat.engine.clients.os.nova:KeypairConstraint nova.network = heat.engine.clients.os.nova:NetworkConstraint nova.server = heat.engine.clients.os.nova:ServerConstraint + sahara.cluster = heat.engine.clients.os.sahara:ClusterConstraint + sahara.cluster_template = heat.engine.clients.os.sahara:ClusterTemplateConstraint + sahara.data_source = heat.engine.clients.os.sahara:DataSourceConstraint sahara.image = heat.engine.clients.os.sahara:ImageConstraint + sahara.job_binary = heat.engine.clients.os.sahara:JobBinaryConstraint + sahara.job_type = heat.engine.clients.os.sahara:JobTypeConstraint sahara.plugin = heat.engine.clients.os.sahara:PluginConstraint senlin.cluster = heat.engine.clients.os.senlin:ClusterConstraint senlin.policy_type = heat.engine.clients.os.senlin:PolicyTypeConstraint