From 4de40cb5144cfd8cdc4b270f23acfdbd3eafa5be Mon Sep 17 00:00:00 2001 From: Lingxian Kong Date: Mon, 10 Aug 2020 19:48:16 +1200 Subject: [PATCH] Support to update instance access Change-Id: I640cd8b50fd0e0f80a1a45399b8bfdac437ae2b9 --- api-ref/source/instances.inc | 38 +++++++++++- .../instance-update-access-request.json | 8 +++ .../victoria-show-update-instance-access.yaml | 3 + trove/common/apischema.py | 35 +++++++++++ trove/common/neutron.py | 59 +++++++++++++++---- trove/instance/models.py | 4 ++ trove/instance/service.py | 41 +++++++++++-- trove/instance/tasks.py | 4 ++ trove/taskmanager/api.py | 7 +++ trove/taskmanager/manager.py | 10 ++++ trove/taskmanager/models.py | 54 +++++++++++++++++ .../instance/test_instance_controller.py | 20 ++----- 12 files changed, 247 insertions(+), 36 deletions(-) create mode 100644 api-ref/source/samples/instance-update-access-request.json create mode 100644 releasenotes/notes/victoria-show-update-instance-access.yaml diff --git a/api-ref/source/instances.inc b/api-ref/source/instances.inc index 38c388649a..1d635fd537 100644 --- a/api-ref/source/instances.inc +++ b/api-ref/source/instances.inc @@ -334,7 +334,7 @@ Request Example Update instance name ~~~~~~~~~~~~~~~~~~~~ -.. rest_method:: PATCH /v1.0/{project_id}/instances/{instanceId} +.. rest_method:: PUT /v1.0/{project_id}/instances/{instanceId} Update the instance name. @@ -362,7 +362,7 @@ Request Example Upgrade datastore version for instance ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ -.. rest_method:: PATCH /v1.0/{project_id}/instances/{instanceId} +.. rest_method:: PUT /v1.0/{project_id}/instances/{instanceId} Upgrade datastore version. @@ -394,7 +394,7 @@ Request Example Detach replica ~~~~~~~~~~~~~~ -.. rest_method:: PATCH /v1.0/{project_id}/instances/{instanceId} +.. rest_method:: PUT /v1.0/{project_id}/instances/{instanceId} Detaches a replica from its replication source. @@ -426,6 +426,38 @@ Request Example +Update instance accessbility +~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +.. rest_method:: PUT /v1.0/{project_id}/instances/{instanceId} + +The following operations are supported: + +* If the instance should be exposed to public or not. Not providing + ``is_public`` means private. +* The list of CIDRs that are allowed to access the database service. Not + providing ``allowed_cidrs`` means allowing everything. + +Normal response codes: 202 + +Request +------- + +.. rest_parameters:: parameters.yaml + + - project_id: project_id + - instanceId: instanceId + - instance: instance + - access: access + - access.is_public: access_is_public + - access.allowed_cidrs: access_allowed_cidrs + +Request Example +--------------- + +.. literalinclude:: samples/instance-update-access-request.json + :language: javascript + Delete database instance ~~~~~~~~~~~~~~~~~~~~~~~~ diff --git a/api-ref/source/samples/instance-update-access-request.json b/api-ref/source/samples/instance-update-access-request.json new file mode 100644 index 0000000000..d19f65de2e --- /dev/null +++ b/api-ref/source/samples/instance-update-access-request.json @@ -0,0 +1,8 @@ +{ + "instance": { + "access": { + "is_public": true, + "allowed_cidrs": ["10.0.0.0/24"] + } + } +} diff --git a/releasenotes/notes/victoria-show-update-instance-access.yaml b/releasenotes/notes/victoria-show-update-instance-access.yaml new file mode 100644 index 0000000000..d7394162fa --- /dev/null +++ b/releasenotes/notes/victoria-show-update-instance-access.yaml @@ -0,0 +1,3 @@ +--- +features: + - Added support to show and update the access configuration for the instance. diff --git a/trove/common/apischema.py b/trove/common/apischema.py index 5b7594f167..2956120aa9 100644 --- a/trove/common/apischema.py +++ b/trove/common/apischema.py @@ -456,6 +456,41 @@ instance = { } } }, + "update": { + "name": "instance:update", + "type": "object", + "required": ["instance"], + "properties": { + "instance": { + "type": "object", + "required": [], + "additionalProperties": False, + "properties": { + "name": non_empty_string, + "replica_of": {}, + "configuration": configuration_id, + "datastore_version": non_empty_string, + "access": { + "type": "object", + "additionalProperties": False, + "properties": { + "is_public": {"type": "boolean"}, + "allowed_cidrs": { + "type": "array", + "uniqueItems": True, + "items": { + "type": "string", + "pattern": "^([0-9]{1,3}\\.){3}[0-9]{1,3}" + "(\\/([0-9]|[1-2][0-9]|3[0-2]))" + "?$" + } + } + } + } + } + } + } + }, "action": { "resize": { "volume": { diff --git a/trove/common/neutron.py b/trove/common/neutron.py index 5e08a51d59..70df7da225 100644 --- a/trove/common/neutron.py +++ b/trove/common/neutron.py @@ -77,17 +77,7 @@ def create_port(client, name, description, network_id, security_groups, port_id = port['port']['id'] if is_public: - public_network_id = get_public_network(client) - if not public_network_id: - raise exception.PublicNetworkNotFound() - - fip_body = { - "floatingip": { - 'floating_network_id': public_network_id, - 'port_id': port_id, - } - } - client.create_floatingip(fip_body) + make_port_public(client, port_id) return port_id @@ -107,6 +97,26 @@ def delete_port(client, id): client.delete_port(id) +def make_port_public(client, port_id): + public_network_id = get_public_network(client) + if not public_network_id: + raise exception.PublicNetworkNotFound() + + fip_body = { + "floatingip": { + 'floating_network_id': public_network_id, + 'port_id': port_id, + } + } + + try: + client.create_floatingip(fip_body) + except Exception as e: + LOG.error(f"Failed to associate public IP with port {port_id}: " + f"{str(e)}") + raise exception.TroveError('Failed to expose instance port to public.') + + def get_public_network(client): """Get public network ID. @@ -125,6 +135,24 @@ def get_public_network(client): return ret['networks'][0].get('id') +def ensure_port_access(client, port_id, is_public): + fips = client.list_floatingips(port_id=port_id)["floatingips"] + + if is_public and not fips: + # Associate floating IP + LOG.debug(f"Associate public IP with port {port_id}") + make_port_public(client, port_id) + return + + if not is_public and fips: + # Disassociate floating IP + for fip in fips: + LOG.debug(f"Disassociate public IP {fip['floating_ip_address']} " + f"from port {port_id}") + client.delete_floatingip(fip["id"]) + return + + def create_security_group(client, name, instance_id): body = { 'security_group': { @@ -159,6 +187,15 @@ def create_security_group_rule(client, sg_id, protocol, ports, remote_ips): client.create_security_group_rule(body) +def clear_ingress_security_group_rules(client, sg_id): + rules = client.list_security_group_rules( + security_group_id=sg_id)['security_group_rules'] + + for rule in rules: + if rule['direction'] == 'ingress': + client.delete_security_group_rule(rule['id']) + + def get_subnet_cidrs(client, network_id=None, subnet_id=None): cidrs = [] diff --git a/trove/instance/models.py b/trove/instance/models.py index 6875d0b480..bd4814d8da 100644 --- a/trove/instance/models.py +++ b/trove/instance/models.py @@ -1698,6 +1698,10 @@ class Instance(BuiltInstance): self.update_db(task_status=InstanceTasks.BUILDING) task_api.API(self.context).rebuild(self.id, image_id) + def update_access(self, access): + self.update_db(task_status=InstanceTasks.UPDATING) + task_api.API(self.context).update_access(self.id, access) + def create_server_list_matcher(server_list): # Returns a method which finds a server from the given list. diff --git a/trove/instance/service.py b/trove/instance/service.py index 1ed9ad41b5..4fae31e7ca 100644 --- a/trove/instance/service.py +++ b/trove/instance/service.py @@ -502,7 +502,9 @@ class InstanceController(wsgi.Controller): context, request=req) with StartNotification(context, instance_id=instance.id): instance.detach_replica() - if 'configuration_id' in kwargs: + + instance.update_db(**kwargs) + elif 'configuration_id' in kwargs: if kwargs['configuration_id']: context.notification = ( notification.DBaaSInstanceAttachConfiguration(context, @@ -517,7 +519,9 @@ class InstanceController(wsgi.Controller): request=req)) with StartNotification(context, instance_id=instance.id): instance.detach_configuration() - if 'datastore_version' in kwargs: + + instance.update_db(**kwargs) + elif 'datastore_version' in kwargs: datastore_version = ds_models.DatastoreVersion.load( instance.datastore, kwargs['datastore_version']) context.notification = ( @@ -525,11 +529,17 @@ class InstanceController(wsgi.Controller): with StartNotification(context, instance_id=instance.id, datastore_version_id=datastore_version.id): instance.upgrade(datastore_version) - if kwargs: + instance.update_db(**kwargs) + elif 'access' in kwargs: + instance.update_access(kwargs['access']) def update(self, req, id, body, tenant_id): - """Updates the instance to attach/detach configuration.""" + """Updates the instance. + + - attach/detach configuration. + - access information. + """ LOG.info("Updating database instance '%(instance_id)s' for tenant " "'%(tenant_id)s'", {'instance_id': id, 'tenant_id': tenant_id}) @@ -537,12 +547,31 @@ class InstanceController(wsgi.Controller): LOG.debug("body: %s", body) context = req.environ[wsgi.CONTEXT_KEY] + name = body['instance'].get('name') + if ((name and len(body['instance']) != 2) or + (not name and len(body['instance']) == 2)): + raise exception.BadRequest("Only one attribute (except 'name') is " + "allowed to update.") + instance = models.Instance.load(context, id) self.authorize_instance_action(context, 'update', instance) - # Make sure args contains a 'configuration_id' argument, args = {} - args['configuration_id'] = self._configuration_parse(context, body) + if name: + instance.update_db(name=name) + + detach_replica = ('replica_of' in body['instance'] or + 'slave_of' in body['instance']) + if detach_replica: + args['detach_replica'] = detach_replica + + configuration_id = self._configuration_parse(context, body) + if configuration_id: + args['configuration_id'] = configuration_id + + if 'access' in body['instance']: + args['access'] = body['instance']['access'] + self._modify_instance(context, req, instance, **args) return wsgi.Result(None, 202) diff --git a/trove/instance/tasks.py b/trove/instance/tasks.py index 46960fa802..17d699ff3c 100644 --- a/trove/instance/tasks.py +++ b/trove/instance/tasks.py @@ -82,6 +82,7 @@ class InstanceTasks(object): LOGGING = InstanceTask(0x0a, 'LOGGING', 'Transferring guest logs.') DETACHING = InstanceTask(0x0b, 'DETACHING', 'Detaching the instance from replica source.') + UPDATING = InstanceTask(0x0c, 'UPDATING', 'Updating the instance.') BUILDING_ERROR_DNS = InstanceTask(0x50, 'BUILDING', 'Build error: DNS.', is_error=True) @@ -121,6 +122,9 @@ class InstanceTasks(object): BUILDING_ERROR_PORT = InstanceTask(0x5c, 'BUILDING', 'Build error: Port.', is_error=True) + UPDATING_ERROR_ACCESS = InstanceTask(0x5d, 'UPDATING', + 'Update error: Access.', + is_error=True) # Dissuade further additions at run-time. diff --git a/trove/taskmanager/api.py b/trove/taskmanager/api.py index 61f71f4a76..3ffe8d6ea4 100644 --- a/trove/taskmanager/api.py +++ b/trove/taskmanager/api.py @@ -179,6 +179,13 @@ class API(object): self._cast("delete_instance", version=version, instance_id=instance_id) + def update_access(self, instance_id, access): + LOG.debug(f"Making async call to update instance: {instance_id}") + version = self.API_BASE_VERSION + + self._cast("update_access", version=version, + instance_id=instance_id, access=access) + def create_backup(self, backup_info, instance_id): LOG.debug("Making async call to create a backup for instance: %s", instance_id) diff --git a/trove/taskmanager/manager.py b/trove/taskmanager/manager.py index 2ac5de8734..6259863d8a 100644 --- a/trove/taskmanager/manager.py +++ b/trove/taskmanager/manager.py @@ -458,6 +458,16 @@ class Manager(periodic_task.PeriodicTasks): with EndNotification(context): instance_tasks.upgrade(datastore_version) + def update_access(self, context, instance_id, access): + instance_tasks = models.BuiltInstanceTasks.load(context, instance_id) + + try: + instance_tasks.update_access(access) + except Exception as e: + LOG.error(f"Failed to update access configuration for " + f"{instance_id}: {str(e)}") + self.update_db(task_status=InstanceTasks.UPDATING_ERROR_ACCESS) + def create_cluster(self, context, cluster_id): with EndNotification(context, cluster_id=cluster_id): cluster_tasks = models.load_cluster_tasks(context, cluster_id) diff --git a/trove/taskmanager/models.py b/trove/taskmanager/models.py index 6200744557..517e4cf7a4 100755 --- a/trove/taskmanager/models.py +++ b/trove/taskmanager/models.py @@ -1348,6 +1348,60 @@ class BuiltInstanceTasks(BuiltInstance, NotifyMixin, ConfigurationMixin): else: return "/dev/%s" % device + def update_access(self, access): + LOG.info(f"Updating access for instance {self.id}, access {access}") + + new_is_public = access.get('is_public', False) + new_allowed_cidrs = access.get('allowed_cidrs', []) + is_public = (self.access.get('is_public', False) if self.access + else None) + allowed_cidrs = (self.access.get('allowed_cidrs', []) if self.access + else None) + + ports = self.neutron_client.list_ports( + name='trove-%s' % self.id)['ports'] + + if is_public != new_is_public: + for port in ports: + if 'User port' in port['description']: + LOG.debug(f"Updating port {port['id']}, is_public: " + f"{new_is_public}") + neutron.ensure_port_access(self.neutron_client, port['id'], + new_is_public) + + if CONF.trove_security_groups_support: + if allowed_cidrs != new_allowed_cidrs: + name = f"{CONF.trove_security_group_name_prefix}-{self.id}" + sgs = self.neutron_client.list_security_groups( + name=name)['security_groups'] + + LOG.debug(f"Updating security group rules for instance " + f"{self.id}") + for sg in sgs: + neutron.clear_ingress_security_group_rules( + self.neutron_client, + sg['id']) + + if new_allowed_cidrs: + tcp_ports = CONF.get(self.datastore.name).tcp_ports + udp_ports = CONF.get(self.datastore.name).udp_ports + + neutron.create_security_group_rule( + self.neutron_client, sg['id'], 'tcp', tcp_ports, + new_allowed_cidrs) + neutron.create_security_group_rule( + self.neutron_client, sg['id'], 'udp', udp_ports, + new_allowed_cidrs) + else: + LOG.warning('Security group not supported.') + + LOG.info(f"Finished to update access for instance {self.id}") + self.update_db( + task_status=InstanceTasks.NONE, + access={'is_public': new_is_public, + 'allowed_cidrs': new_allowed_cidrs} + ) + class BackupTasks(object): @classmethod diff --git a/trove/tests/unittests/instance/test_instance_controller.py b/trove/tests/unittests/instance/test_instance_controller.py index f222ddac82..8be487f123 100644 --- a/trove/tests/unittests/instance/test_instance_controller.py +++ b/trove/tests/unittests/instance/test_instance_controller.py @@ -304,6 +304,7 @@ class TestInstanceController(trove_testtools.TestCase): instance.attach_configuration = Mock() instance.detach_configuration = Mock() instance.update_db = Mock() + instance.update_access = Mock() return instance def test_modify_instance_with_empty_args(self): @@ -318,16 +319,6 @@ class TestInstanceController(trove_testtools.TestCase): self.assertEqual(0, instance.attach_configuration.call_count) self.assertEqual(0, instance.update_db.call_count) - def test_modify_instance_with_nonempty_args_calls_update_db(self): - instance = self._setup_modify_instance_mocks() - args = {} - args['any'] = 'anything' - - self.controller._modify_instance(self.context, self.req, - instance, **args) - - instance.update_db.assert_called_once_with(**args) - def test_modify_instance_with_False_detach_replica_arg(self): instance = self._setup_modify_instance_mocks() args = {} @@ -368,15 +359,12 @@ class TestInstanceController(trove_testtools.TestCase): self.assertEqual(1, instance.detach_configuration.call_count) - def test_modify_instance_with_all_args(self): + def test_modify_instance_with_access(self): instance = self._setup_modify_instance_mocks() args = {} - args['detach_replica'] = True - args['configuration_id'] = 'some_id' + args['access'] = {'is_public': True} self.controller._modify_instance(self.context, self.req, instance, **args) - self.assertEqual(1, instance.detach_replica.call_count) - self.assertEqual(1, instance.attach_configuration.call_count) - instance.update_db.assert_called_once_with(**args) + instance.update_access.assert_called_once_with({'is_public': True})