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

@@ -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):
+        # leave this method for backward compatibility
-            try:
+        try:
-                image_id = self.client().images.get(image_identifier).id
+            return self.find_resource_by_name_or_id('images', image_identifier)
-            except sahara_base.APIException as ex:
+        except exception.EntityNotFound:
-                if self.is_not_registered(ex):
+            raise exception.EntityNotFound(entity='Image',
-                    image_id = self.get_image_id_by_name(image_identifier)
+                                           name=image_identifier)
-        else:
-            image_id = self.get_image_id_by_name(image_identifier)
-        return image_id
 
-    def get_image_id_by_name(self, image_identifier):
+    def find_resource_by_name(self, resource_name, value):
-        """Return the ID for the specified image name.
+        """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}
+            filters = {'name': value}
-            image_list = self.client().images.find(**filters)
+            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: "
+                _("Error retrieving %(entity)s list from sahara: "
-                  "%s") % six.text_type(ex))
+                  "%(err)s") % dict(entity=resource_name,
-        num_matches = len(image_list)
+                                    err=six.text_type(ex)))
+        num_matches = len(obj_list)
         if num_matches == 0:
-            raise exception.EntityNotFound(entity='Image',
+            raise exception.EntityNotFound(entity=resource_name or 'entity',
-                                           name=image_identifier)
+                                           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):
-
+    resource_name = 'images'
-    expected_exceptions = (exception.EntityNotFound,
-                           exception.PhysicalResourceNameAmbiguity,)
-    resource_getter_name = 'get_image_id'
 
 
-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'

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,

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=[constraints.Length(max=1)],
-                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,

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,
+                              self.sahara_plugin.find_resource_by_name,
-                              img_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.mock_get = mock.Mock()
-        self.ctx.clients.client_plugin(
+        cl_plgn = self.ctx.clients.client_plugin('sahara')
-            'sahara').get_image_id = self.mock_get_image
+        cl_plgn.find_resource_by_name_or_id = self.mock_get
-        self.constraint = sahara.ImageConstraint()
+        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(
+        self.mock_get.side_effect = exception.EntityNotFound(
-            entity='Image', name='bar')
+            entity='Fake entity', 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.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")

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)
+        jb = self._create_resource('job', self.rsrc_defn, self.stack, True)
-        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)()
         args = self.client.jobs.create.call_args[1]
         expected_args = {
             'name': 'fake_phys_name',

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