Merge "Fix module-instances command"

releasenotes/notes/fix_mod_inst_cmd-3a46c7233e3.yaml

@@ -0,0 +1,8 @@
+---
+fixes:
+  - The module-instances command now returns a paginated
+    list of instances.  A --count_only flag was added to the
+    command to return a summary of the applied instances
+    based on the MD5 of the module (this is most useful
+    for live_update modules, to see which ones haven't been
+    updated). Bug 1554900

tools/trove-pylint.config

@@ -1137,6 +1137,78 @@
       "Module 'eventlet.green.subprocess' has no 'PIPE' member",
       "PgDump._execute_postgres_restore"
     ],
+    [
+      "trove/instance/models.py",
+      "E1101",
+      "Class 'DBInstance' has no 'cluster_id' member",
+      "module_instance_count"
+    ],
+    [
+      "trove/instance/models.py",
+      "E1101",
+      "Class 'DBInstance' has no 'id' member",
+      "Instances.load"
+    ],
+    [
+      "trove/instance/models.py",
+      "E1101",
+      "Class 'DBInstance' has no 'id' member",
+      "module_instance_count"
+    ],
+    [
+      "trove/instance/models.py",
+      "E1101",
+      "Class 'DBInstance' has no 'tenant_id' member",
+      "module_instance_count"
+    ],
+    [
+      "trove/instance/models.py",
+      "E1101",
+      "Class 'DBInstanceModule' has no 'deleted' member",
+      "module_instance_count"
+    ],
+    [
+      "trove/instance/models.py",
+      "E1101",
+      "Class 'DBInstanceModule' has no 'instance_id' member",
+      "module_instance_count"
+    ],
+    [
+      "trove/instance/models.py",
+      "E1101",
+      "Class 'DBInstanceModule' has no 'md5' member",
+      "module_instance_count"
+    ],
+    [
+      "trove/instance/models.py",
+      "E1101",
+      "Class 'DBInstanceModule' has no 'module_id' member",
+      "module_instance_count"
+    ],
+    [
+      "trove/instance/models.py",
+      "E1101",
+      "Class 'DBInstanceModule' has no 'updated' member",
+      "module_instance_count"
+    ],
+    [
+      "trove/instance/models.py",
+      "E1101",
+      "Class 'DBModule' has no 'id' member",
+      "module_instance_count"
+    ],
+    [
+      "trove/instance/models.py",
+      "E1101",
+      "Class 'DBModule' has no 'md5' member",
+      "module_instance_count"
+    ],
+    [
+      "trove/instance/models.py",
+      "E1101",
+      "Class 'DBModule' has no 'name' member",
+      "module_instance_count"
+    ],
     [
       "trove/instance/models.py",
       "E1101",
@@ -1149,6 +1221,78 @@
       "Instance of 'DBInstance' has no 'encrypted_key' member",
       "DBInstance.key"
     ],
+    [
+      "trove/instance/models.py",
+      "no-member",
+      "Class 'DBInstance' has no 'cluster_id' member",
+      "module_instance_count"
+    ],
+    [
+      "trove/instance/models.py",
+      "no-member",
+      "Class 'DBInstance' has no 'id' member",
+      "Instances.load"
+    ],
+    [
+      "trove/instance/models.py",
+      "no-member",
+      "Class 'DBInstance' has no 'id' member",
+      "module_instance_count"
+    ],
+    [
+      "trove/instance/models.py",
+      "no-member",
+      "Class 'DBInstance' has no 'tenant_id' member",
+      "module_instance_count"
+    ],
+    [
+      "trove/instance/models.py",
+      "no-member",
+      "Class 'DBInstanceModule' has no 'deleted' member",
+      "module_instance_count"
+    ],
+    [
+      "trove/instance/models.py",
+      "no-member",
+      "Class 'DBInstanceModule' has no 'instance_id' member",
+      "module_instance_count"
+    ],
+    [
+      "trove/instance/models.py",
+      "no-member",
+      "Class 'DBInstanceModule' has no 'md5' member",
+      "module_instance_count"
+    ],
+    [
+      "trove/instance/models.py",
+      "no-member",
+      "Class 'DBInstanceModule' has no 'module_id' member",
+      "module_instance_count"
+    ],
+    [
+      "trove/instance/models.py",
+      "no-member",
+      "Class 'DBInstanceModule' has no 'updated' member",
+      "module_instance_count"
+    ],
+    [
+      "trove/instance/models.py",
+      "no-member",
+      "Class 'DBModule' has no 'id' member",
+      "module_instance_count"
+    ],
+    [
+      "trove/instance/models.py",
+      "no-member",
+      "Class 'DBModule' has no 'md5' member",
+      "module_instance_count"
+    ],
+    [
+      "trove/instance/models.py",
+      "no-member",
+      "Class 'DBModule' has no 'name' member",
+      "module_instance_count"
+    ],
     [
       "trove/instance/models.py",
       "no-member",

trove/datastore/models.py

@@ -519,6 +519,38 @@ def get_datastore_version(type=None, version=None, return_inactive=False):
     return (datastore, datastore_version)
 
 
+def get_datastore_or_version(datastore=None, datastore_version=None):
+    """
+    Validate that the specified datastore/version exists, and return the
+    corresponding ids.  This differs from 'get_datastore_version' in that
+    you don't need to specify both - specifying only a datastore will
+    return 'None' in the ds_ver field.  Raises DatastoreNoVersion if
+    you pass in a ds_ver without a ds.  Originally designed for module
+    management.
+
+    :param datastore:           Datastore name or id
+    :param datastore_version:   Version name or id
+    :return:                    Tuple of ds_id, ds_ver_id if found
+    """
+
+    datastore_id = None
+    datastore_version_id = None
+    if datastore:
+        if datastore_version:
+            ds, ds_ver = get_datastore_version(
+                type=datastore, version=datastore_version)
+            datastore_id = ds.id
+            datastore_version_id = ds_ver.id
+        else:
+            ds = Datastore.load(datastore)
+            datastore_id = ds.id
+    elif datastore_version:
+        # Cannot specify version without datastore.
+        raise exception.DatastoreNoVersion(
+            datastore=datastore, version=datastore_version)
+    return datastore_id, datastore_version_id
+
+
 def update_datastore(name, default_version):
     db_api.configure_db(CONF)
     try:

trove/db/models.py

@@ -115,6 +115,10 @@ class DatabaseModelBase(models.ModelBase):
 
         return model
 
+    @classmethod
+    def find_by_filter(cls, **kwargs):
+        return db_query.find_by_filter(cls, **cls._process_conditions(kwargs))
+
     @classmethod
     def get_by(cls, **kwargs):
         return get_db_api().find_by(cls, **cls._process_conditions(kwargs))

trove/db/sqlalchemy/api.py

@@ -50,6 +50,11 @@ def find_by(model, **kwargs):
     return _query_by(model, **kwargs).first()
 
 
+def find_by_filter(model, **kwargs):
+    filters = kwargs.pop('filters', [])
+    return _query_by_filter(model, *filters, **kwargs)
+
+
 def save(model):
     try:
         db_session = session.get_session()
@@ -124,6 +129,13 @@ def _query_by(cls, **conditions):
     return query
 
 
+def _query_by_filter(cls, *filters, **conditions):
+    query = _query_by(cls, **conditions)
+    if filters:
+        query = query.filter(*filters)
+    return query
+
+
 def _limits(query_func, model, conditions, limit, marker, marker_column=None):
     query = query_func(model, **conditions)
     marker_column = marker_column or model.id

trove/instance/models.py

@@ -19,6 +19,7 @@ from datetime import datetime
 from datetime import timedelta
 import os.path
 import re
+from sqlalchemy import func
 
 from novaclient import exceptions as nova_exceptions
 from oslo_config.cfg import NoSuchOptError
@@ -569,26 +570,6 @@ def load_server_group_info(instance, context, compute_id):
         instance.locality = srv_grp.ServerGroup.get_locality(server_group)
 
 
-def validate_modules_for_apply(modules, datastore_id, datastore_version_id):
-    for module in modules:
-        if (module.datastore_id and
-                module.datastore_id != datastore_id):
-            reason = (_("Module '%(mod)s' cannot be applied "
-                        " (Wrong datastore '%(ds)s' - expected '%(ds2)s')")
-                      % {'mod': module.name, 'ds': module.datastore_id,
-                         'ds2': datastore_id})
-            raise exception.ModuleInvalid(reason=reason)
-        if (module.datastore_version_id and
-                module.datastore_version_id != datastore_version_id):
-            reason = (_("Module '%(mod)s' cannot be applied "
-                        " (Wrong datastore version '%(ver)s' "
-                        "- expected '%(ver2)s')")
-                      % {'mod': module.name,
-                         'ver': module.datastore_version_id,
-                         'ver2': datastore_version_id})
-            raise exception.ModuleInvalid(reason=reason)
-
-
 class BaseInstance(SimpleInstance):
     """Represents an instance.
     -----------
@@ -1013,8 +994,9 @@ class Instance(BuiltInstance):
         for aa_module in auto_apply_modules:
             if aa_module.id not in module_ids:
                 modules.append(aa_module)
-        validate_modules_for_apply(modules, datastore.id, datastore_version.id)
-        module_list = module_views.get_module_list(modules)
+        module_models.Modules.validate(
+            modules, datastore.id, datastore_version.id)
+        module_list = module_views.convert_modules_to_list(modules)
 
         def _create_resources():
 
@@ -1481,12 +1463,13 @@ class Instances(object):
                       'deleted': False}
         if not include_clustered:
             query_opts['cluster_id'] = None
-        if instance_ids and len(instance_ids) > 1:
-            raise exception.DatastoreOperationNotSupported(
-                operation='module-instances', datastore='current')
         if instance_ids:
-            query_opts['id'] = instance_ids[0]
-        db_infos = DBInstance.find_all(**query_opts)
+            if context.is_admin:
+                query_opts.pop('tenant_id')
+            filters = [DBInstance.id.in_(instance_ids)]
+            db_infos = DBInstance.find_by_filter(filters=filters, **query_opts)
+        else:
+            db_infos = DBInstance.find_all(**query_opts)
         limit = utils.pagination_limit(context.limit, Instances.DEFAULT_LIMIT)
         data_view = DBInstance.find_by_pagination('instances', db_infos, "foo",
                                                   limit=limit,
@@ -1672,6 +1655,40 @@ def get_instance_encryption_key(instance_id):
     return _instance_encryption_key[instance_id]
 
 
+def module_instance_count(context, module_id, include_clustered=False):
+    """Returns a summary of the instances that have applied a given
+    module.  We use the SQLAlchemy query object directly here as there's
+    functionality needed that's not exposed in the trove/db/__init__.py/Query
+    object.
+    """
+    columns = [module_models.DBModule.name,
+               module_models.DBInstanceModule.module_id,
+               module_models.DBInstanceModule.md5,
+               func.count(module_models.DBInstanceModule.md5),
+               (module_models.DBInstanceModule.md5 ==
+                module_models.DBModule.md5),
+               func.min(module_models.DBInstanceModule.updated),
+               func.max(module_models.DBInstanceModule.updated)]
+    filters = [module_models.DBInstanceModule.module_id == module_id,
+               module_models.DBInstanceModule.deleted == 0]
+    query = module_models.DBInstanceModule.query()
+    query = query.join(
+        module_models.DBModule,
+        module_models.DBInstanceModule.module_id == module_models.DBModule.id)
+    query = query.join(
+        DBInstance,
+        module_models.DBInstanceModule.instance_id == DBInstance.id)
+    if not include_clustered:
+        filters.append(DBInstance.cluster_id.is_(None))
+    if not context.is_admin:
+        filters.append(DBInstance.tenant_id == context.tenant)
+    query = query.group_by(module_models.DBInstanceModule.md5)
+    query = query.add_columns(*columns)
+    query = query.filter(*filters)
+    query = query.order_by(module_models.DBInstanceModule.updated)
+    return query.all()
+
+
 def persist_instance_fault(notification, event_qualifier):
     """This callback is registered to be fired whenever a
     notification is sent out.

trove/instance/service.py

@@ -536,9 +536,9 @@ class InstanceController(wsgi.Controller):
         self.authorize_instance_action(context, 'module_apply', instance)
         module_ids = [mod['id'] for mod in body.get('modules', [])]
         modules = module_models.Modules.load_by_ids(context, module_ids)
-        models.validate_modules_for_apply(
+        module_models.Modules.validate(
             modules, instance.datastore.id, instance.datastore_version.id)
-        module_list = module_views.get_module_list(modules)
+        module_list = module_views.convert_modules_to_list(modules)
         client = create_guest_client(context, id)
         result_list = client.module_apply(module_list)
         models.Instance.add_instance_modules(context, id, modules)

trove/instance/views.py

@@ -41,6 +41,8 @@ class InstanceView(object):
                           "version": self.instance.datastore_version.name},
             "region": self.instance.region_name
         }
+        if self.context.is_admin:
+            instance_dict['tenant_id'] = self.instance.tenant_id
         if self.instance.volume_support:
             instance_dict['volume'] = {'size': self.instance.volume_size}
 
@@ -212,3 +214,19 @@ class GuestLogsView(object):
 
     def data(self):
         return [GuestLogView(l).data() for l in self.guest_logs]
+
+
+def convert_instance_count_to_list(instance_count):
+    instance_list = []
+    for row in instance_count:
+        (_, name, id, md5, count, current, min_date, max_date) = row
+        instance_list.append(
+            {'module_name': name,
+             'module_id': id,
+             'module_md5': md5,
+             'instance_count': count,
+             'current': current,
+             'min_updated_date': min_date,
+             'max_updated_date': max_date
+             })
+    return instance_list

trove/module/models.py

@@ -127,6 +127,26 @@ class Modules(object):
             modules = db_info.all()
         return modules
 
+    @staticmethod
+    def validate(modules, datastore_id, datastore_version_id):
+        for module in modules:
+            if (module.datastore_id and
+                    module.datastore_id != datastore_id):
+                reason = (_("Module '%(mod)s' cannot be applied "
+                            " (Wrong datastore '%(ds)s' - expected '%(ds2)s')")
+                          % {'mod': module.name, 'ds': module.datastore_id,
+                             'ds2': datastore_id})
+                raise exception.ModuleInvalid(reason=reason)
+            if (module.datastore_version_id and
+                    module.datastore_version_id != datastore_version_id):
+                reason = (_("Module '%(mod)s' cannot be applied "
+                            " (Wrong datastore version '%(ver)s' "
+                            "- expected '%(ver2)s')")
+                          % {'mod': module.name,
+                             'ver': module.datastore_version_id,
+                             'ver2': datastore_version_id})
+                raise exception.ModuleInvalid(reason=reason)
+
 
 class Module(object):
 
@@ -145,8 +165,9 @@ class Module(object):
         Module.validate_action(
             context, 'create', tenant_id, auto_apply, visible, priority_apply,
             full_access)
-        datastore_id, datastore_version_id = Module.validate_datastore(
-            datastore, datastore_version)
+        datastore_id, datastore_version_id = (
+            datastore_models.get_datastore_or_version(
+                datastore, datastore_version))
         if Module.key_exists(
                 name, module_type, tenant_id,
                 datastore_id, datastore_version_id):
@@ -203,24 +224,6 @@ class Module(object):
                 action=action_str, options=admin_options_str)
         return admin_options_str
 
-    @staticmethod
-    def validate_datastore(datastore, datastore_version):
-        datastore_id = None
-        datastore_version_id = None
-        if datastore:
-            if datastore_version:
-                ds, ds_ver = datastore_models.get_datastore_version(
-                    type=datastore, version=datastore_version)
-                datastore_id = ds.id
-                datastore_version_id = ds_ver.id
-            else:
-                ds = datastore_models.Datastore.load(datastore)
-                datastore_id = ds.id
-        elif datastore_version:
-            msg = _("Cannot specify version without datastore")
-            raise exception.BadRequest(message=msg)
-        return datastore_id, datastore_version_id
-
     @staticmethod
     def key_exists(name, module_type, tenant_id, datastore_id,
                    datastore_version_id):
@@ -323,7 +326,7 @@ class Module(object):
         # but we turn it on/off if full_access is specified
         if full_access is not None:
             module.is_admin = 0 if full_access else 1
-        ds_id, ds_ver_id = Module.validate_datastore(
+        ds_id, ds_ver_id = datastore_models.get_datastore_or_version(
             module.datastore_id, module.datastore_version_id)
         if module.contents != original_module.contents:
             md5, processed_contents = Module.process_contents(module.contents)

trove/module/service.py

@@ -62,7 +62,7 @@ class ModuleController(wsgi.Controller):
         return wsgi.Result(view.data(), 200)
 
     def show(self, req, tenant_id, id):
-        LOG.info(_("Showing module %s") % id)
+        LOG.info(_("Showing module %s.") % id)
 
         context = req.environ[wsgi.CONTEXT_KEY]
         module = models.Module.load(context, id)
@@ -104,7 +104,7 @@ class ModuleController(wsgi.Controller):
         return wsgi.Result(view_data.data(), 200)
 
     def delete(self, req, tenant_id, id):
-        LOG.info(_("Deleting module %s") % id)
+        LOG.info(_("Deleting module %s.") % id)
 
         context = req.environ[wsgi.CONTEXT_KEY]
         module = models.Module.load(context, id)
@@ -113,7 +113,7 @@ class ModuleController(wsgi.Controller):
         return wsgi.Result(None, 200)
 
     def update(self, req, body, tenant_id, id):
-        LOG.info(_("Updating module %s") % id)
+        LOG.info(_("Updating module %s.") % id)
 
         context = req.environ[wsgi.CONTEXT_KEY]
         module = models.Module.load(context, id)
@@ -171,26 +171,34 @@ class ModuleController(wsgi.Controller):
         return wsgi.Result(view_data.data(), 200)
 
     def instances(self, req, tenant_id, id):
-        LOG.info(_("Getting instances for module %s") % id)
+        LOG.info(_("Getting instances for module %s.") % id)
 
         context = req.environ[wsgi.CONTEXT_KEY]
 
         module = models.Module.load(context, id)
         self.authorize_module_action(context, 'instances', module)
 
-        instance_modules, marker = models.InstanceModules.load(
-            context, module_id=id)
-        if instance_modules:
-            instance_ids = [inst_mod.instance_id
-                            for inst_mod in instance_modules]
-            include_clustered = (
-                req.GET.get('include_clustered', '').lower() == 'true')
-            instances, marker = instance_models.Instances.load(
-                context, include_clustered, instance_ids=instance_ids)
+        count_only = req.GET.get('count_only', '').lower() == 'true'
+        include_clustered = (
+            req.GET.get('include_clustered', '').lower() == 'true')
+        if count_only:
+            instance_count = instance_models.module_instance_count(
+                context, id, include_clustered=include_clustered)
+            result_list = {
+                'instances':
+                instance_views.convert_instance_count_to_list(instance_count)}
         else:
-            instances = []
-            marker = None
-        view = instance_views.InstancesView(instances, req=req)
-        paged = pagination.SimplePaginatedDataView(req.url, 'instances',
-                                                   view, marker)
-        return wsgi.Result(paged.data(), 200)
+            instance_modules, marker = models.InstanceModules.load(
+                context, module_id=id)
+            if instance_modules:
+                instance_ids = [inst_mod.instance_id
+                                for inst_mod in instance_modules]
+                instances, marker = instance_models.Instances.load(
+                    context, include_clustered, instance_ids=instance_ids)
+            else:
+                instances = []
+                marker = None
+            view = instance_views.InstancesView(instances, req=req)
+            result_list = pagination.SimplePaginatedDataView(
+                req.url, 'instances', view, marker).data()
+        return wsgi.Result(result_list, 200)

trove/module/views.py

@@ -108,10 +108,9 @@ class DetailedModuleView(ModuleView):
         return {"module": module_dict}
 
 
-def get_module_list(modules):
+def convert_modules_to_list(modules):
     module_list = []
     for module in modules:
-        module_info = DetailedModuleView(module).data(
-            include_contents=True)
+        module_info = DetailedModuleView(module).data(include_contents=True)
         module_list.append(module_info)
     return module_list

trove/tests/scenario/groups/module_group.py

@@ -361,6 +361,11 @@ class ModuleInstCreateGroup(TestGroup):
         self.test_runner.run_module_instances_empty()
 
     @test(runs_after=[module_instances_empty])
+    def module_instance_count_empty(self):
+        """Check that no instance count exists."""
+        self.test_runner.run_module_instance_count_empty()
+
+    @test(runs_after=[module_instance_count_empty])
     def module_query_empty(self):
         """Check that the instance has no modules applied."""
         self.test_runner.run_module_query_empty()
@@ -380,7 +385,17 @@ class ModuleInstCreateGroup(TestGroup):
         """Check that the instance has one module associated."""
         self.test_runner.run_module_list_instance_after_apply()
 
-    @test(depends_on=[module_apply])
+    @test(runs_after=[module_list_instance_after_apply])
+    def module_instances_after_apply(self):
+        """Check that the instance shows up in the list."""
+        self.test_runner.run_module_instances_after_apply()
+
+    @test(runs_after=[module_instances_after_apply])
+    def module_instance_count_after_apply(self):
+        """Check that the instance count is right after apply."""
+        self.test_runner.run_module_instance_count_after_apply()
+
+    @test(runs_after=[module_instance_count_after_apply])
     def module_query_after_apply(self):
         """Check that module-query works."""
         self.test_runner.run_module_query_after_apply()
@@ -395,6 +410,16 @@ class ModuleInstCreateGroup(TestGroup):
         """Check that the instance has one module associated."""
         self.test_runner.run_module_list_instance_after_apply_another()
 
+    @test(runs_after=[module_list_instance_after_apply_another])
+    def module_instances_after_apply_another(self):
+        """Check that the instance shows up in the list still."""
+        self.test_runner.run_module_instances_after_apply_another()
+
+    @test(runs_after=[module_instances_after_apply_another])
+    def module_instance_count_after_apply_another(self):
+        """Check that the instance count is right after another apply."""
+        self.test_runner.run_module_instance_count_after_apply_another()
+
     @test(depends_on=[module_apply_another])
     def module_query_after_apply_another(self):
         """Check that module-query works after another apply."""

trove/tests/scenario/runners/module_runners.py

@@ -892,6 +892,24 @@ class ModuleRunner(TestRunner):
         self.assert_equal(expected_count, count,
                           "Wrong number of instances applied from module")
 
+    def run_module_instance_count_empty(self):
+        self.assert_module_instance_count(
+            self.auth_client, self.main_test_module.id, 0)
+
+    def assert_module_instance_count(self, client, module_id,
+                                     expected_rows, expected_count=None,
+                                     expected_http_code=200):
+        instance_count_list = client.modules.instances(module_id,
+                                                       count_only=True)
+        self.assert_client_code(client, expected_http_code)
+        rowcount = len(instance_count_list)
+        self.assert_equal(expected_rows, rowcount,
+                          "Wrong number of instance count records from module")
+        if expected_rows == 1:
+            self.assert_equal(expected_count,
+                              instance_count_list[0].instance_count,
+                              "Wrong count in record from module instances")
+
     def run_module_query_empty(self):
         self.assert_module_query(
             self.auth_client, self.instance_info.id,
@@ -1044,6 +1062,14 @@ class ModuleRunner(TestRunner):
             datastore_version=self.instance_info.dbaas_datastore_version,
             contents=contents)
 
+    def run_module_instances_after_apply(self):
+        self.assert_module_instances(
+            self.auth_client, self.main_test_module.id, 1)
+
+    def run_module_instance_count_after_apply(self):
+        self.assert_module_instance_count(
+            self.auth_client, self.main_test_module.id, 1, 1)
+
     def run_module_query_after_apply(self):
         expected_count = self.module_auto_apply_count_prior_to_create + 1
         expected_results = self.create_default_query_expected_results(
@@ -1078,6 +1104,14 @@ class ModuleRunner(TestRunner):
             }
         return expected_results
 
+    def run_module_instances_after_apply_another(self):
+        self.assert_module_instances(
+            self.auth_client, self.main_test_module.id, 1)
+
+    def run_module_instance_count_after_apply_another(self):
+        self.assert_module_instance_count(
+            self.auth_client, self.main_test_module.id, 1, 1)
+
     def run_module_query_after_apply_another(self):
         expected_count = self.module_auto_apply_count_prior_to_create + 2
         expected_results = self.create_default_query_expected_results(

trove/tests/unittests/datastore/test_datastore.py

@@ -13,6 +13,7 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
+from mock import Mock
 from mock import patch
 
 from trove.common import exception
@@ -42,3 +43,41 @@ class TestDatastore(TestDatastoreBase):
                                 "Datastore 'my_ds' cannot be found",
                                 datastore_models.get_datastore_version,
                                 'my_ds')
+
+    def test_get_datastore_or_version(self):
+        # datastore, datastore_version, valid, exception
+        data = [
+            [None, None, True],
+            ['ds', None, True],
+            ['ds', 'ds_ver', True],
+            [None, 'ds_ver', False, exception.DatastoreNoVersion],
+        ]
+        for datum in data:
+            ds_id = datum[0]
+            ds_ver_id = datum[1]
+            valid = datum[2]
+            expected_exception = None
+            if not valid:
+                expected_exception = datum[3]
+            ds = Mock()
+            ds.id = ds_id
+            ds.name = ds_id
+            ds_ver = Mock()
+            ds_ver.id = ds_ver_id
+            ds_ver.name = ds_ver_id
+            ds_ver.datastore_id = ds_id
+            with patch.object(datastore_models.Datastore, 'load',
+                              return_value=ds):
+                with patch.object(datastore_models.DatastoreVersion, 'load',
+                                  return_value=ds_ver):
+                    if valid:
+                        (get_ds_id, get_ds_ver_id) = (
+                            datastore_models.get_datastore_or_version(
+                                ds_id, ds_ver_id))
+                        self.assertEqual(ds_id, get_ds_id)
+                        self.assertEqual(ds_ver_id, get_ds_ver_id)
+                    else:
+                        self.assertRaises(
+                            expected_exception,
+                            datastore_models.get_datastore_or_version,
+                            ds_id, ds_ver_id)

trove/tests/unittests/instance/test_instance_models.py

@@ -17,7 +17,6 @@ from mock import Mock, patch
 
 from trove.backup import models as backup_models
 from trove.common import cfg
-from trove.common import crypto_utils
 from trove.common import exception
 from trove.common.instance import ServiceStatuses
 from trove.datastore import models as datastore_models
@@ -407,71 +406,6 @@ class TestReplication(trove_testtools.TestCase):
                           None, slave_of_id=self.replica_info.id)
 
 
-class TestModules(trove_testtools.TestCase):
-
-    def setUp(self):
-        super(TestModules, self).setUp()
-
-    def tearDown(self):
-        super(TestModules, self).tearDown()
-
-    def _build_module(self, ds_id, ds_ver_id):
-        module = Mock()
-        module.datastore_id = ds_id
-        module.datastore_version_id = ds_ver_id
-        module.contents = crypto_utils.encode_data(
-            crypto_utils.encrypt_data(
-                'VGhpc2lzbXlkYXRhc3RyaW5n',
-                'thisismylongkeytouse'))
-        return module
-
-    def test_validate_modules_for_apply(self):
-        data = [
-            [[self._build_module('ds', 'ds_ver')], 'ds', 'ds_ver', True],
-            [[self._build_module('ds', None)], 'ds', 'ds_ver', True],
-            [[self._build_module(None, None)], 'ds', 'ds_ver', True],
-
-            [[self._build_module('ds', 'ds_ver')], 'ds', 'ds2_ver', False,
-             exception.TroveError],
-            [[self._build_module('ds', 'ds_ver')], 'ds2', 'ds_ver', False,
-             exception.TroveError],
-            [[self._build_module('ds', 'ds_ver')], 'ds2', 'ds2_ver', False,
-             exception.TroveError],
-            [[self._build_module('ds', None)], 'ds2', 'ds2_ver', False,
-             exception.TroveError],
-            [[self._build_module(None, None)], 'ds2', 'ds2_ver', True],
-
-            [[self._build_module(None, 'ds_ver')], 'ds2', 'ds_ver', True],
-        ]
-        for datum in data:
-            modules = datum[0]
-            ds_id = datum[1]
-            ds_ver_id = datum[2]
-            match = datum[3]
-            expected_exception = None
-            if not match:
-                expected_exception = datum[4]
-            ds = Mock()
-            ds.id = ds_id
-            ds.name = ds_id
-            ds_ver = Mock()
-            ds_ver.id = ds_ver_id
-            ds_ver.name = ds_ver_id
-            ds_ver.datastore_id = ds_id
-            with patch.object(datastore_models.Datastore, 'load',
-                              return_value=ds):
-                with patch.object(datastore_models.DatastoreVersion, 'load',
-                                  return_value=ds_ver):
-                    if match:
-                        models.validate_modules_for_apply(
-                            modules, ds_id, ds_ver_id)
-                    else:
-                        self.assertRaises(
-                            expected_exception,
-                            models.validate_modules_for_apply,
-                            modules, ds_id, ds_ver_id)
-
-
 def trivial_key_function(id):
     return id * id
 

trove/tests/unittests/module/test_module_models.py

@@ -17,6 +17,7 @@
 import copy
 from mock import Mock, patch
 
+from trove.common import crypto_utils
 from trove.common import exception
 from trove.datastore import models as datastore_models
 from trove.module import models
@@ -106,22 +107,42 @@ class CreateModuleTest(trove_testtools.TestCase):
                         context, 'action', tenant, auto_apply, visible,
                         priority_apply, full_access)
 
-    def test_validate_datastore(self):
-        # datastore, datastore_version, valid, exception
+    def _build_module(self, ds_id, ds_ver_id):
+        module = Mock()
+        module.datastore_id = ds_id
+        module.datastore_version_id = ds_ver_id
+        module.contents = crypto_utils.encode_data(
+            crypto_utils.encrypt_data(
+                'VGhpc2lzbXlkYXRhc3RyaW5n',
+                'thisismylongkeytouse'))
+        return module
+
+    def test_validate(self):
         data = [
-            [None, None, True],
-            ['ds', None, True],
-            ['ds', 'ds_ver', True],
-            [None, 'ds_ver', False,
-             exception.BadRequest],
+            [[self._build_module('ds', 'ds_ver')], 'ds', 'ds_ver', True],
+            [[self._build_module('ds', None)], 'ds', 'ds_ver', True],
+            [[self._build_module(None, None)], 'ds', 'ds_ver', True],
+
+            [[self._build_module('ds', 'ds_ver')], 'ds', 'ds2_ver', False,
+             exception.TroveError],
+            [[self._build_module('ds', 'ds_ver')], 'ds2', 'ds_ver', False,
+             exception.TroveError],
+            [[self._build_module('ds', 'ds_ver')], 'ds2', 'ds2_ver', False,
+             exception.TroveError],
+            [[self._build_module('ds', None)], 'ds2', 'ds2_ver', False,
+             exception.TroveError],
+            [[self._build_module(None, None)], 'ds2', 'ds2_ver', True],
+
+            [[self._build_module(None, 'ds_ver')], 'ds2', 'ds_ver', True],
         ]
         for datum in data:
-            ds_id = datum[0]
-            ds_ver_id = datum[1]
-            valid = datum[2]
+            modules = datum[0]
+            ds_id = datum[1]
+            ds_ver_id = datum[2]
+            match = datum[3]
             expected_exception = None
-            if not valid:
-                expected_exception = datum[3]
+            if not match:
+                expected_exception = datum[4]
             ds = Mock()
             ds.id = ds_id
             ds.name = ds_id
@@ -133,9 +154,10 @@ class CreateModuleTest(trove_testtools.TestCase):
                               return_value=ds):
                 with patch.object(datastore_models.DatastoreVersion, 'load',
                                   return_value=ds_ver):
-                    if valid:
-                        models.Module.validate_datastore(ds_id, ds_ver_id)
+                    if match:
+                        models.Modules.validate(modules, ds_id, ds_ver_id)
                     else:
                         self.assertRaises(
                             expected_exception,
-                            models.Module.validate_datastore, ds_id, ds_ver_id)
+                            models.Modules.validate,
+                            modules, ds_id, ds_ver_id)