From 2fe147c66e9a64abda15b5a6e162a6d4b3a04554 Mon Sep 17 00:00:00 2001 From: Iccha Sethi Date: Wed, 7 May 2014 09:15:56 -0500 Subject: [PATCH] Allow users the ability to update an instance name Currently the users do not have the ability to rename an instance. The current PUT call, does not allow the user to modify individual attributes of the instance. Hence this review introduces a new PATCH call for updating instance name and/or configurations. The long term plan is to deprecate PUT in favor of PATCH. Implements: blueprint Allow users the ability to update the instance name Author: Iccha Sethi Co-Authored-By: Theron Voran Co-Authored-By: Riddhi Shah Change-Id: I25c3b0906191afdc587f5ddd5777b3185dc080d4 --- trove/common/apischema.py | 2 + trove/instance/models.py | 1 - trove/instance/service.py | 34 ++++++-- trove/taskmanager/models.py | 1 - trove/tests/api/configurations.py | 131 +++++++++++++++++++++++++++--- trove/tests/api/instances.py | 34 ++++++++ trove/tests/config.py | 4 +- 7 files changed, 188 insertions(+), 19 deletions(-) diff --git a/trove/common/apischema.py b/trove/common/apischema.py index 01258b1816..69589ca048 100644 --- a/trove/common/apischema.py +++ b/trove/common/apischema.py @@ -303,6 +303,8 @@ instance = { "required": [], "properties": { "slave_of": {}, + "name": non_empty_string, + "configuration_id": configuration_id, } } } diff --git a/trove/instance/models.py b/trove/instance/models.py index 7dae8305c4..cfb41188f1 100644 --- a/trove/instance/models.py +++ b/trove/instance/models.py @@ -969,7 +969,6 @@ class Instance(BuiltInstance): config = Configuration(self.context, configuration.id) LOG.debug("Config config is %s." % config) - self.update_db(configuration_id=configuration.id) self.update_overrides(config) def update_overrides(self, config): diff --git a/trove/instance/service.py b/trove/instance/service.py index fa56a8cab7..730137f109 100644 --- a/trove/instance/service.py +++ b/trove/instance/service.py @@ -250,17 +250,20 @@ class InstanceController(wsgi.Controller): instance = models.Instance.load(context, id) - # if configuration is set, then we will update the instance to use - # the new configuration. If configuration is empty, we want to - # disassociate the instance from the configuration group and remove the - # active overrides file. + # If configuration is set, then we will update the instance to use the + # new configuration. If configuration is empty, we want to disassociate + # the instance from the configuration group and remove the active + # overrides file. + update_args = {} configuration_id = self._configuration_parse(context, body) - if configuration_id: instance.assign_configuration(configuration_id) else: instance.unassign_configuration() + + update_args['configuration_id'] = configuration_id + instance.update_db(**update_args) return wsgi.Result(None, 202) def edit(self, req, id, body, tenant_id): @@ -278,6 +281,27 @@ class InstanceController(wsgi.Controller): LOG.debug("Detaching replica from source.") instance.detach_replica() + # If configuration is set, then we will update the instance to + # use the new configuration. If configuration is empty, we + # want to disassociate the instance from the configuration + # group and remove the active overrides file. + # If instance name is set, then we will update the instance name. + + edit_args = {} + if 'configuration' in body['instance']: + configuration_id = self._configuration_parse(context, body) + if configuration_id: + instance.assign_configuration(configuration_id) + else: + instance.unassign_configuration() + edit_args['configuration_id'] = configuration_id + + if 'name' in body['instance']: + edit_args['name'] = body['instance']['name'] + + if edit_args: + instance.update_db(**edit_args) + return wsgi.Result(None, 202) def configuration(self, req, tenant_id, id): diff --git a/trove/taskmanager/models.py b/trove/taskmanager/models.py index 6f0edf9a80..4aed496ae5 100755 --- a/trove/taskmanager/models.py +++ b/trove/taskmanager/models.py @@ -1075,7 +1075,6 @@ class BuiltInstanceTasks(BuiltInstance, NotifyMixin, ConfigurationMixin): overrides[item.configuration_key] = _convert_value(val) LOG.debug("setting the default variables in dict: %s" % overrides) self.update_overrides(overrides, remove=True) - self.update_db(configuration_id=None) def refresh_compute_server_info(self): """Refreshes the compute server field.""" diff --git a/trove/tests/api/configurations.py b/trove/tests/api/configurations.py index 1d20aa66db..e17ed77215 100644 --- a/trove/tests/api/configurations.py +++ b/trove/tests/api/configurations.py @@ -312,6 +312,32 @@ class AfterConfigurationsCreation(ConfigurationsTestBase): resp, body = instance_info.dbaas.client.last_response assert_equal(resp.status, 202) + @test + def test_assign_name_to_instance_using_patch(self): + # test assigning a name to an instance + new_name = 'new_name_1' + report = CONFIG.get_report() + report.log("instance_info.id: %s" % instance_info.id) + report.log("instance name:%s" % instance_info.name) + report.log("instance new name:%s" % new_name) + instance_info.dbaas.instances.edit(instance_info.id, name=new_name) + assert_equal(202, instance_info.dbaas.last_http_code) + check = instance_info.dbaas.instances.get(instance_info.id) + assert_equal(200, instance_info.dbaas.last_http_code) + assert_equal(check.name, new_name) + # Restore instance name + instance_info.dbaas.instances.edit(instance_info.id, + name=instance_info.name) + assert_equal(202, instance_info.dbaas.last_http_code) + + @test + def test_assign_configuration_to_invalid_instance_using_patch(self): + # test assign config group to an invalid instance + invalid_id = "invalid-inst-id" + assert_raises(exceptions.NotFound, + instance_info.dbaas.instances.edit, + invalid_id, configuration=configuration_info.id) + @test(depends_on=[test_assign_configuration_to_valid_instance]) def test_assign_configuration_to_instance_with_config(self): # test assigning a configuration to an instance that @@ -324,14 +350,15 @@ class AfterConfigurationsCreation(ConfigurationsTestBase): @test(depends_on=[test_assign_configuration_to_valid_instance]) @time_out(30) def test_get_configuration_details_from_instance_validation(self): - # validate that the configuraiton was applied correctly to the instance + # validate that the configuration was applied correctly to the instance + print("instance_info.id: %s" % instance_info.id) inst = instance_info.dbaas.instances.get(instance_info.id) configuration_id = inst.configuration['id'] + print("configuration_info: %s" % configuration_id) assert_not_equal(None, inst.configuration['id']) _test_configuration_is_applied_to_instance(instance_info, configuration_id) - @test def test_configurations_get(self): # test that the instance shows up on the assigned configuration result = instance_info.dbaas.configurations.get(configuration_info.id) @@ -664,14 +691,6 @@ class DeleteConfigurations(ConfigurationsTestBase): print(configuration_instance.id) print(instance_info.id) - @test(depends_on=[test_no_instances_on_configuration]) - def test_delete_unassigned_configuration(self): - # test that we can delete the configuration after no instances are - # assigned to it any longer - instance_info.dbaas.configurations.delete(configuration_info.id) - resp, body = instance_info.dbaas.client.last_response - assert_equal(resp.status, 202) - @test(depends_on=[test_unassign_configuration_from_instances]) @time_out(120) def test_restart_service_after_unassign_return_active(self): @@ -711,6 +730,98 @@ class DeleteConfigurations(ConfigurationsTestBase): return False poll_until(result_is_active) + @test(depends_on=[test_restart_service_should_return_active]) + def test_assign_config_and_name_to_instance_using_patch(self): + # test assigning a configuration and name to an instance + new_name = 'new_name' + report = CONFIG.get_report() + report.log("instance_info.id: %s" % instance_info.id) + report.log("configuration_info: %s" % configuration_info) + report.log("configuration_info.id: %s" % configuration_info.id) + report.log("instance name:%s" % instance_info.name) + report.log("instance new name:%s" % new_name) + saved_name = instance_info.name + config_id = configuration_info.id + instance_info.dbaas.instances.edit(instance_info.id, + configuration=config_id, + name=new_name) + assert_equal(202, instance_info.dbaas.last_http_code) + check = instance_info.dbaas.instances.get(instance_info.id) + assert_equal(200, instance_info.dbaas.last_http_code) + assert_equal(check.name, new_name) + + # restore instance name + instance_info.dbaas.instances.edit(instance_info.id, + name=saved_name) + assert_equal(202, instance_info.dbaas.last_http_code) + + instance = instance_info.dbaas.instances.get(instance_info.id) + assert_equal('RESTART_REQUIRED', instance.status) + # restart to be sure configuration is applied + instance_info.dbaas.instances.restart(instance_info.id) + assert_equal(202, instance_info.dbaas.last_http_code) + sleep(2) + + def result_is_active(): + instance = instance_info.dbaas.instances.get( + instance_info.id) + if instance.status == "ACTIVE": + return True + else: + assert_equal("REBOOT", instance.status) + return False + poll_until(result_is_active) + # test assigning a configuration to an instance that + # already has an assigned configuration with patch + config_id = configuration_info.id + assert_raises(exceptions.BadRequest, + instance_info.dbaas.instances.edit, + instance_info.id, configuration=config_id) + + @test(runs_after=[test_assign_config_and_name_to_instance_using_patch]) + def test_unassign_configuration_after_patch(self): + # remove the configuration from the instance + instance_info.dbaas.instances.edit(instance_info.id, + remove_configuration=True) + assert_equal(202, instance_info.dbaas.last_http_code) + instance = instance_info.dbaas.instances.get(instance_info.id) + assert_equal('RESTART_REQUIRED', instance.status) + # restart to be sure configuration has been unassigned + instance_info.dbaas.instances.restart(instance_info.id) + assert_equal(202, instance_info.dbaas.last_http_code) + sleep(2) + + def result_is_active(): + instance = instance_info.dbaas.instances.get( + instance_info.id) + if instance.status == "ACTIVE": + return True + else: + assert_equal("REBOOT", instance.status) + return False + poll_until(result_is_active) + result = instance_info.dbaas.configurations.get(configuration_info.id) + assert_equal(result.instance_count, 0) + + @test + def test_unassign_configuration_from_invalid_instance_using_patch(self): + # test unassign config group from an invalid instance + invalid_id = "invalid-inst-id" + try: + instance_info.dbaas.instances.edit(invalid_id, + remove_configuration=True) + except exceptions.NotFound: + resp, body = instance_info.dbaas.client.last_response + assert_equal(resp.status, 404) + + @test(runs_after=[test_unassign_configuration_after_patch]) + def test_delete_unassigned_configuration(self): + # test that we can delete the configuration after no instances are + # assigned to it any longer + instance_info.dbaas.configurations.delete(configuration_info.id) + resp, body = instance_info.dbaas.client.last_response + assert_equal(resp.status, 202) + @test(depends_on=[test_delete_unassigned_configuration]) @time_out(TIMEOUT_INSTANCE_DELETE) def test_delete_configuration_instance(self): diff --git a/trove/tests/api/instances.py b/trove/tests/api/instances.py index b124cfbd68..d6a90b9edf 100644 --- a/trove/tests/api/instances.py +++ b/trove/tests/api/instances.py @@ -1175,6 +1175,40 @@ class TestInstanceListing(object): check.volume_mgmt() +@test(depends_on_classes=[WaitForGuestInstallationToFinish], + groups=[GROUP, GROUP_START, GROUP_START_SIMPLE, "dbaas.update"]) +class TestInstanceUpdate(object): + """Test the updation of the instance information.""" + + @before_class + def setUp(self): + reqs = Requirements(is_admin=False) + self.other_user = CONFIG.users.find_user( + reqs, + black_list=[instance_info.user.auth_user]) + self.other_client = create_dbaas_client(self.other_user) + + @test + def test_update_name(self): + new_name = 'new-name' + result = dbaas.instances.edit(instance_info.id, name=new_name) + assert_equal(202, dbaas.last_http_code) + result = dbaas.instances.get(instance_info.id) + assert_equal(200, dbaas.last_http_code) + assert_equal(new_name, result.name) + # Restore instance name because other tests depend on it + dbaas.instances.edit(instance_info.id, name=instance_info.name) + assert_equal(202, dbaas.last_http_code) + + @test + def test_update_name_to_invalid_instance(self): + # test assigning to an instance that does not exist + invalid_id = "invalid-inst-id" + assert_raises(exceptions.NotFound, instance_info.dbaas.instances.edit, + invalid_id, name='name') + assert_equal(404, instance_info.dbaas.last_http_code) + + @test(depends_on_classes=[WaitForGuestInstallationToFinish], groups=[GROUP, 'dbaas.usage']) class TestCreateNotification(object): diff --git a/trove/tests/config.py b/trove/tests/config.py index c07b5e80ee..2dffd7e957 100644 --- a/trove/tests/config.py +++ b/trove/tests/config.py @@ -95,12 +95,12 @@ class TestConfig(object): "configurations": { "valid_values": { "connect_timeout": 120, - "local_infile": True, + "local_infile": 0, "collation_server": "latin1_swedish_ci" }, "appending_values": { "join_buffer_size": 1048576, - "connect_timeout": 60 + "connect_timeout": 15 }, "nondynamic_parameter": { "join_buffer_size": 1048576,