diff --git a/releasenotes/notes/fix_mod_inst_cmd-3a46c7233e3.yaml b/releasenotes/notes/fix_mod_inst_cmd-3a46c7233e3.yaml new file mode 100644 index 0000000000..43b2a3dd12 --- /dev/null +++ b/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 diff --git a/tools/trove-pylint.config b/tools/trove-pylint.config index 01ced46f51..06031e795d 100644 --- a/tools/trove-pylint.config +++ b/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", diff --git a/trove/datastore/models.py b/trove/datastore/models.py index 07d70554d3..e2af26c376 100644 --- a/trove/datastore/models.py +++ b/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: diff --git a/trove/db/models.py b/trove/db/models.py index 90dc4800d3..0af007488f 100644 --- a/trove/db/models.py +++ b/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)) diff --git a/trove/db/sqlalchemy/api.py b/trove/db/sqlalchemy/api.py index 324dcd32c4..3172e896e5 100644 --- a/trove/db/sqlalchemy/api.py +++ b/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 diff --git a/trove/instance/models.py b/trove/instance/models.py index 0621c2cc3f..f7e4cd776c 100644 --- a/trove/instance/models.py +++ b/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. diff --git a/trove/instance/service.py b/trove/instance/service.py index e0cd404b45..378f661b18 100644 --- a/trove/instance/service.py +++ b/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) diff --git a/trove/instance/views.py b/trove/instance/views.py index 30c045c794..aeeb807be9 100644 --- a/trove/instance/views.py +++ b/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 diff --git a/trove/module/models.py b/trove/module/models.py index 0ee40dd59c..7308b63b1b 100644 --- a/trove/module/models.py +++ b/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) diff --git a/trove/module/service.py b/trove/module/service.py index b75108ec9a..1a13911cbd 100644 --- a/trove/module/service.py +++ b/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) diff --git a/trove/module/views.py b/trove/module/views.py index 5a747caf92..20134a8167 100644 --- a/trove/module/views.py +++ b/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 diff --git a/trove/tests/scenario/groups/module_group.py b/trove/tests/scenario/groups/module_group.py index 49fc3eab6b..2490cd3378 100644 --- a/trove/tests/scenario/groups/module_group.py +++ b/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.""" diff --git a/trove/tests/scenario/runners/module_runners.py b/trove/tests/scenario/runners/module_runners.py index e3302ee0bb..f8c1fbfe35 100644 --- a/trove/tests/scenario/runners/module_runners.py +++ b/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( diff --git a/trove/tests/unittests/datastore/test_datastore.py b/trove/tests/unittests/datastore/test_datastore.py index 47a64a37e2..a54a6768a9 100644 --- a/trove/tests/unittests/datastore/test_datastore.py +++ b/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) diff --git a/trove/tests/unittests/instance/test_instance_models.py b/trove/tests/unittests/instance/test_instance_models.py index c089daaad4..88de37ed2f 100644 --- a/trove/tests/unittests/instance/test_instance_models.py +++ b/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 diff --git a/trove/tests/unittests/module/test_module_models.py b/trove/tests/unittests/module/test_module_models.py index 20ce5c2096..330694e0b7 100644 --- a/trove/tests/unittests/module/test_module_models.py +++ b/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)