diff --git a/releasenotes/notes/module_reapply_update_values-1fb88dc58701368d.yaml b/releasenotes/notes/module_reapply_update_values-1fb88dc58701368d.yaml new file mode 100644 index 0000000000..1616bd5b7d --- /dev/null +++ b/releasenotes/notes/module_reapply_update_values-1fb88dc58701368d.yaml @@ -0,0 +1,9 @@ +--- +fixes: + - Applying a module again will now relect the + update name, type, datastore and datastore_version + values. + Bug 1611525 + - Updating a module with all_datastores and + all_datastore_versions now works correctly. + Bug 1612430 diff --git a/trove/common/apischema.py b/trove/common/apischema.py index 8eaa90df03..08dd549c98 100644 --- a/trove/common/apischema.py +++ b/trove/common/apischema.py @@ -606,6 +606,8 @@ module = { }, "auto_apply": boolean_string, "all_tenants": boolean_string, + "all_datastores": boolean_string, + "all_datastore_versions": boolean_string, "visible": boolean_string, "live_update": boolean_string, } diff --git a/trove/common/exception.py b/trove/common/exception.py index 9fe4cc3ae4..3db5bcfb8f 100644 --- a/trove/common/exception.py +++ b/trove/common/exception.py @@ -530,7 +530,7 @@ class ModuleAccessForbidden(Forbidden): class ModuleInvalid(Forbidden): - message = _("The module you are applying is invalid: %(reason)s") + message = _("The module is invalid: %(reason)s") class ClusterNotFound(NotFound): diff --git a/trove/guestagent/module/module_manager.py b/trove/guestagent/module/module_manager.py index c41e00da39..37aa9da7a8 100644 --- a/trove/guestagent/module/module_manager.py +++ b/trove/guestagent/module/module_manager.py @@ -78,6 +78,10 @@ class ModuleManager(object): result['updated'] = now result['id'] = module_id result['md5'] = md5 + result['type'] = module_type + result['name'] = name + result['datastore'] = datastore + result['datastore_version'] = ds_version result['tenant'] = tenant result['auto_apply'] = auto_apply result['visible'] = visible diff --git a/trove/module/service.py b/trove/module/service.py index 3f5b87a003..f609454e38 100644 --- a/trove/module/service.py +++ b/trove/module/service.py @@ -20,6 +20,7 @@ from oslo_log import log as logging import trove.common.apischema as apischema from trove.common import cfg +from trove.common import exception from trove.common.i18n import _ from trove.common import pagination from trove.common import wsgi @@ -110,12 +111,28 @@ class ModuleController(wsgi.Controller): if 'all_tenants' in body['module']: module.tenant_id = (None if body['module']['all_tenants'] else tenant_id) + ds_changed = False + ds_ver_changed = False if 'datastore' in body['module']: if 'type' in body['module']['datastore']: module.datastore_id = body['module']['datastore']['type'] + ds_changed = True if 'version' in body['module']['datastore']: module.datastore_version_id = ( body['module']['datastore']['version']) + ds_ver_changed = True + if 'all_datastores' in body['module']: + if ds_changed: + raise exception.ModuleInvalid( + reason=_('You cannot set a datastore and specify ' + '--all_datastores')) + module.datastore_id = None + if 'all_datastore_versions' in body['module']: + if ds_ver_changed: + raise exception.ModuleInvalid( + reason=_('You cannot set a datastore version and specify ' + '--all_datastore_versions')) + module.datastore_version_id = None if 'auto_apply' in body['module']: module.auto_apply = body['module']['auto_apply'] if 'visible' in body['module']: diff --git a/trove/tests/scenario/groups/module_group.py b/trove/tests/scenario/groups/module_group.py index e116c6fa1e..3a744f1de7 100644 --- a/trove/tests/scenario/groups/module_group.py +++ b/trove/tests/scenario/groups/module_group.py @@ -83,12 +83,22 @@ class ModuleCreateGroup(TestGroup): """Check that create module works.""" self.test_runner.run_module_create() + @test(runs_after=[module_create]) + def module_create_for_update(self): + """Check that create module for update works.""" + self.test_runner.run_module_create_for_update() + @test(depends_on=[module_create]) def module_create_dupe(self): """Ensure create with duplicate info fails.""" self.test_runner.run_module_create_dupe() - @test(runs_after=[module_create]) + @test(depends_on=[module_create_for_update]) + def module_update_missing_datastore(self): + """Ensure update module with missing datastore fails.""" + self.test_runner.run_module_update_missing_datastore() + + @test(runs_after=[module_create_for_update]) def module_create_bin(self): """Check that create module with binary contents works.""" self.test_runner.run_module_create_bin() @@ -324,8 +334,23 @@ class ModuleInstCreateGroup(TestGroup): """Check that module-query works.""" self.test_runner.run_module_query_after_apply() + @test(runs_after=[module_query_after_apply]) + def module_apply_another(self): + """Check that module-apply works for another module.""" + self.test_runner.run_module_apply_another() + + @test(depends_on=[module_apply_another]) + def module_list_instance_after_apply_another(self): + """Check that the instance has one module associated.""" + self.test_runner.run_module_list_instance_after_apply_another() + + @test(depends_on=[module_apply_another]) + def module_query_after_apply_another(self): + """Check that module-query works after another apply.""" + self.test_runner.run_module_query_after_apply_another() + @test(depends_on=[module_apply], - runs_after=[module_query_after_apply]) + runs_after=[module_query_after_apply_another]) def create_inst_with_mods(self): """Check that creating an instance with modules works.""" self.test_runner.run_create_inst_with_mods() @@ -343,32 +368,44 @@ class ModuleInstCreateGroup(TestGroup): self.test_runner.run_module_remove() @test(depends_on=[module_remove]) - def module_query_empty_after(self): - """Check that the instance has no modules applied after remove.""" - self.test_runner.run_module_query_empty() + def module_query_after_remove(self): + """Check that the instance has one module applied after remove.""" + self.test_runner.run_module_query_after_remove() @test(depends_on=[module_remove], - runs_after=[module_query_empty_after]) - def module_apply_again(self): - """Check that module-apply works a second time.""" - self.test_runner.run_module_apply() + runs_after=[module_query_after_remove]) + def module_update_after_remove(self): + """Check that update module after remove works.""" + self.test_runner.run_module_update_after_remove() + + @test(depends_on=[module_remove], + runs_after=[module_update_after_remove]) + def module_apply_another_again(self): + """Check that module-apply another works a second time.""" + self.test_runner.run_module_apply_another() @test(depends_on=[module_apply], - runs_after=[module_apply_again]) - def module_query_after_apply_again(self): + runs_after=[module_apply_another_again]) + def module_query_after_apply_another2(self): """Check that module-query works after second apply.""" - self.test_runner.run_module_query_after_apply() + self.test_runner.run_module_query_after_apply_another() - @test(depends_on=[module_apply_again], - runs_after=[module_query_after_apply_again]) + @test(depends_on=[module_apply_another_again], + runs_after=[module_query_after_apply_another2]) def module_remove_again(self): """Check that module-remove works again.""" self.test_runner.run_module_remove() @test(depends_on=[module_remove_again]) def module_query_empty_after_again(self): - """Check that the instance has no modules applied after remove.""" - self.test_runner.run_module_query_empty() + """Check that the inst has one mod applied after 2nd remove.""" + self.test_runner.run_module_query_after_remove() + + @test(depends_on=[module_remove_again], + runs_after=[module_query_empty_after_again]) + def module_update_after_remove_again(self): + """Check that update module after remove again works.""" + self.test_runner.run_module_update_after_remove_again() @test(depends_on_groups=[groups.MODULE_INST_CREATE], diff --git a/trove/tests/scenario/runners/module_runners.py b/trove/tests/scenario/runners/module_runners.py index df9cb22f68..faa33e74a9 100644 --- a/trove/tests/scenario/runners/module_runners.py +++ b/trove/tests/scenario/runners/module_runners.py @@ -69,11 +69,18 @@ class ModuleRunner(TestRunner): self._module_type = self.test_helper.get_valid_module_type() return self._module_type + def _get_test_module(self, index): + if not self.test_modules or len(self.test_modules) < (index + 1): + raise SkipTest("Requested module not created") + return self.test_modules[index] + @property def main_test_module(self): - if not self.test_modules or not self.test_modules[0]: - SkipTest("No main module created") - return self.test_modules[0] + return self._get_test_module(0) + + @property + def update_test_module(self): + return self._get_test_module(1) def build_module_args(self, extra=None): extra = extra or '' @@ -269,7 +276,7 @@ class ModuleRunner(TestRunner): expected_tenant=tenant, expected_tenant_id=tenant_id, expected_datastore=datastore, - expected_ds_version=datastore_version, + expected_datastore_version=datastore_version, expected_auto_apply=auto_apply, expected_contents=contents) @@ -281,8 +288,10 @@ class ModuleRunner(TestRunner): expected_tenant_id=None, expected_datastore=None, expected_datastore_id=None, - expected_ds_version=None, - expected_ds_version_id=None, + expected_all_datastores=None, + expected_datastore_version=None, + expected_datastore_version_id=None, + expected_all_datastore_versions=None, expected_all_tenants=None, expected_auto_apply=None, expected_live_update=None, @@ -291,6 +300,12 @@ class ModuleRunner(TestRunner): if expected_all_tenants: expected_tenant = expected_tenant or models.Modules.MATCH_ALL_NAME + if expected_all_datastores: + expected_datastore = models.Modules.MATCH_ALL_NAME + expected_datastore_id = None + if expected_all_datastore_versions: + expected_datastore_version = models.Modules.MATCH_ALL_NAME + expected_datastore_version_id = None if expected_name: self.assert_equal(expected_name, module.name, 'Unexpected module name') @@ -309,8 +324,8 @@ class ModuleRunner(TestRunner): if expected_datastore: self.assert_equal(expected_datastore, module.datastore, 'Unexpected datastore') - if expected_ds_version: - self.assert_equal(expected_ds_version, + if expected_datastore_version: + self.assert_equal(expected_datastore_version, module.datastore_version, 'Unexpected datastore version') if expected_auto_apply is not None: @@ -320,8 +335,8 @@ class ModuleRunner(TestRunner): if expected_datastore_id: self.assert_equal(expected_datastore_id, module.datastore_id, 'Unexpected datastore id') - if expected_ds_version_id: - self.assert_equal(expected_ds_version_id, + if expected_datastore_version_id: + self.assert_equal(expected_datastore_version_id, module.datastore_version_id, 'Unexpected datastore version id') if expected_live_update is not None: @@ -331,6 +346,15 @@ class ModuleRunner(TestRunner): self.assert_equal(expected_visible, module.visible, 'Unexpected visible') + def run_module_create_for_update(self): + name, description, contents = self.build_module_args('_for_update') + self.assert_module_create( + self.auth_client, + name=name, + module_type=self.module_type, + contents=contents, + description=description) + def run_module_create_dupe( self, expected_exception=exceptions.BadRequest, expected_http_code=400): @@ -339,6 +363,15 @@ class ModuleRunner(TestRunner): self.auth_client.modules.create, self.MODULE_NAME, self.module_type, self.MODULE_NEG_CONTENTS) + def run_module_update_missing_datastore( + self, expected_exception=exceptions.BadRequest, + expected_http_code=400): + self.assert_raises( + expected_exception, expected_http_code, + self.auth_client.modules.update, + self.update_test_module.id, + datastore_version=self.instance_info.dbaas_datastore_version) + def run_module_create_bin(self): name, description, contents = self.build_module_args( self.MODULE_BINARY_SUFFIX) @@ -373,7 +406,7 @@ class ModuleRunner(TestRunner): expected_description=test_module.description, expected_tenant=test_module.tenant, expected_datastore=test_module.datastore, - expected_ds_version=test_module.datastore_version, + expected_datastore_version=test_module.datastore_version, expected_auto_apply=test_module.auto_apply, expected_live_update=False, expected_visible=True) @@ -414,7 +447,7 @@ class ModuleRunner(TestRunner): expected_description=test_module.description, expected_tenant=test_module.tenant, expected_datastore=test_module.datastore, - expected_ds_version=test_module.datastore_version, + expected_datastore_version=test_module.datastore_version, expected_auto_apply=test_module.auto_apply) def run_module_list_unauth_user(self): @@ -707,8 +740,14 @@ class ModuleRunner(TestRunner): "Wrong number of instances applied from module") def run_module_query_empty(self): - self.assert_module_query(self.auth_client, self.instance_info.id, - self.module_auto_apply_count_prior_to_create) + self.assert_module_query( + self.auth_client, self.instance_info.id, + self.module_auto_apply_count_prior_to_create) + + def run_module_query_after_remove(self): + self.assert_module_query( + self.auth_client, self.instance_info.id, + self.module_auto_apply_count_prior_to_create + 1) def assert_module_query(self, client, instance_id, expected_count, expected_http_code=200, expected_results=None): @@ -748,7 +787,7 @@ class ModuleRunner(TestRunner): expected_name=module.name, expected_module_type=module.type, expected_datastore=module.datastore, - expected_ds_version=module.datastore_version, + expected_datastore_version=module.datastore_version, expected_auto_apply=module.auto_apply, expected_visible=module.visible, expected_admin_only=admin_only, @@ -760,7 +799,7 @@ class ModuleRunner(TestRunner): expected_name=None, expected_module_type=None, expected_datastore=None, - expected_ds_version=None, + expected_datastore_version=None, expected_auto_apply=None, expected_visible=None, expected_admin_only=None, @@ -778,8 +817,8 @@ class ModuleRunner(TestRunner): if expected_datastore: self.assert_equal(expected_datastore, module_apply.datastore, '%s Unexpected datastore' % prefix) - if expected_ds_version: - self.assert_equal(expected_ds_version, + if expected_datastore_version: + self.assert_equal(expected_datastore_version, module_apply.datastore_version, '%s Unexpected datastore version' % prefix) if expected_auto_apply is not None: @@ -807,6 +846,24 @@ class ModuleRunner(TestRunner): self.assert_module_list_instance( self.auth_client, self.instance_info.id, 1) + def run_module_apply_another(self): + self.assert_module_apply(self.auth_client, self.instance_info.id, + self.update_test_module) + + def run_module_list_instance_after_apply_another(self): + self.assert_module_list_instance( + self.auth_client, self.instance_info.id, 2) + + def run_module_update_after_remove(self): + name, description, contents = self.build_module_args('_updated') + self.assert_module_update( + self.auth_client, + self.update_test_module.id, + name=name, + datastore=self.instance_info.dbaas_datastore, + datastore_version=self.instance_info.dbaas_datastore_version, + contents=contents) + 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( @@ -841,6 +898,22 @@ class ModuleRunner(TestRunner): } return expected_results + 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( + [self.main_test_module, self.update_test_module]) + self.assert_module_query(self.auth_client, self.instance_info.id, + expected_count=expected_count, + expected_results=expected_results) + + def run_module_update_after_remove_again(self): + self.assert_module_update( + self.auth_client, + self.update_test_module.id, + name=self.MODULE_NAME + '_updated_back', + all_datastores=True, + all_datastore_versions=True) + def run_create_inst_with_mods(self, expected_http_code=200): self.mod_inst_id = self.assert_inst_mod_create( self.main_test_module.id, '_module', expected_http_code) @@ -868,7 +941,7 @@ class ModuleRunner(TestRunner): def run_module_remove(self): self.assert_module_remove(self.auth_client, self.instance_info.id, - self.main_test_module.id) + self.update_test_module.id) def assert_module_remove(self, client, instance_id, module_id, expected_http_code=200):