From 429c39890e3242c6a502037673943b38452c5811 Mon Sep 17 00:00:00 2001 From: Lingxian Kong Date: Thu, 16 Apr 2020 17:50:55 +1200 Subject: [PATCH] Support ip address type for instances Include address type in getting instance response. * Deprecate confip option network_label_regex as we don't reply on Nova to get addresses, network names don't make any sense. * Add 'addresses' in instance API response, keep 'ip' as is but mark it deprecated in API doc, python-troveclient shouldn't break. Story: 2007562 Task: 39445 Change-Id: Ia0458b5ddae8959ce29c17e444e1a51a026283cd --- api-ref/source/instances.inc | 81 +++++++---- api-ref/source/parameters.yaml | 8 +- .../instance-list-detail-response.json | 10 ++ .../samples/instance-mgmt-list-response.json | 10 ++ .../samples/instance-mgmt-show-response.json | 10 ++ .../samples/instance-show-response.json | 10 ++ devstack/plugin.sh | 1 - doc/source/user/set-up-clustering.rst | 5 + etc/trove/trove.conf.sample | 7 - etc/trove/trove.conf.test | 1 - .../ussuri-add-ip-addresses-for-instance.yaml | 8 ++ trove/cluster/views.py | 1 - trove/common/cfg.py | 3 +- trove/instance/models.py | 136 ++++++++++-------- trove/instance/views.py | 9 +- trove/taskmanager/manager.py | 2 + trove/taskmanager/models.py | 21 +-- trove/tests/api/instances.py | 6 +- .../unittests/cluster/test_cluster_views.py | 14 +- .../instance/test_instance_models.py | 81 +++-------- .../unittests/instance/test_instance_views.py | 7 +- .../unittests/taskmanager/test_models.py | 2 +- 22 files changed, 253 insertions(+), 180 deletions(-) create mode 100644 releasenotes/notes/ussuri-add-ip-addresses-for-instance.yaml diff --git a/api-ref/source/instances.inc b/api-ref/source/instances.inc index 62da0653b9..2be3519bb6 100644 --- a/api-ref/source/instances.inc +++ b/api-ref/source/instances.inc @@ -161,39 +161,14 @@ Response Parameters - datastore: datastore2 - datastore.type: datastore_type - datastore.version: datastore_version1 - - region: region_name2 - - tenant_id: tenant_id - volume: volume - volume.size: volume_size - volume.used: volume_used - - hostname: instance_hostname - - ip: instance_ip_address - created: created - updated: updated - service_status_updated: service_status_updated - - fault: instance_fault - - fault.message: instance_fault_message - - fault.created: instance_fault_created - - fault.details: instance_fault_details - - replicas: instance_replicas - - replicas.id: instance_replica_id - - replicas.links: instance_replica_links - - replicas.links.href: instance_replica_link_href - - replicas.links.rel: instance_replica_link_rel - - configuration: configuration1 - - configuration.id: configuration_id - - configuration.name: configuration_name - - configuration.links: configuration_links - - configuration.links.href: configuration_link_href - - configuration.links.rel: configuration_link_rel - locality: locality - - local_storage_used: local_storage_used - password: root_password - - cluster_id: cluster_id - - shard_id: shard_id - - server_id: server_id - - volume_id: volume_id - - encrypted_rpc_messaging: encrypted_rpc_messaging - instance: instance @@ -231,6 +206,62 @@ Request - instanceId: instanceId +Response Parameters +------------------- + +.. rest_parameters:: parameters.yaml + + - instance: instance + - id: instanceId1 + - name: instanceName1 + - status: instance_status + - links: instance_links + - links.href: instance_link_href + - links.rel: instance_link_rel + - flavor: flavor + - flavor.id: flavorId1 + - flavor.links: flavor_links + - flavor.links.href: flavor_link_href + - flavor.links.rel: flavor_link_rel + - datastore: datastore2 + - datastore.type: datastore_type + - datastore.version: datastore_version1 + - region: region_name2 + - tenant_id: tenant_id + - volume: volume + - volume.size: volume_size + - volume.used: volume_used + - hostname: instance_hostname + - ip: instance_ip_address + - addresses: instance_ip_addresses + - created: created + - updated: updated + - service_status_updated: service_status_updated + - fault: instance_fault + - fault.message: instance_fault_message + - fault.created: instance_fault_created + - fault.details: instance_fault_details + - replicas: instance_replicas + - replicas.id: instance_replica_id + - replicas.links: instance_replica_links + - replicas.links.href: instance_replica_link_href + - replicas.links.rel: instance_replica_link_rel + - configuration: configuration1 + - configuration.id: configuration_id + - configuration.name: configuration_name + - configuration.links: configuration_links + - configuration.links.href: configuration_link_href + - configuration.links.rel: configuration_link_rel + - locality: locality + - local_storage_used: local_storage_used + - password: root_password + - cluster_id: cluster_id + - shard_id: shard_id + - server_id: server_id + - volume_id: volume_id + - encrypted_rpc_messaging: encrypted_rpc_messaging + + Response Example ---------------- diff --git a/api-ref/source/parameters.yaml b/api-ref/source/parameters.yaml index 610628396e..c6279686b4 100755 --- a/api-ref/source/parameters.yaml +++ b/api-ref/source/parameters.yaml @@ -419,10 +419,16 @@ instance_hostname: type: string instance_ip_address: description: | - The IP address of an instance. + The IP address of an instance(deprecated). in: body require: false type: string +instance_ip_addresses: + description: | + The IP addresses of an instance, including the address type and IP. + in: body + require: false + type: array instance_link_href: description: | The ``href`` attribute of an instance link. diff --git a/api-ref/source/samples/instance-list-detail-response.json b/api-ref/source/samples/instance-list-detail-response.json index fd785b99bc..619c87bf9a 100644 --- a/api-ref/source/samples/instance-list-detail-response.json +++ b/api-ref/source/samples/instance-list-detail-response.json @@ -1,6 +1,16 @@ { "instances": [ { + "addresses": [ + { + "address": "10.1.0.62", + "type": "private" + }, + { + "address": "172.24.5.114", + "type": "public" + } + ], "created": "2019-12-23T20:58:35", "datastore": { "type": "mysql", diff --git a/api-ref/source/samples/instance-mgmt-list-response.json b/api-ref/source/samples/instance-mgmt-list-response.json index 706fdf79e3..f67e63253a 100644 --- a/api-ref/source/samples/instance-mgmt-list-response.json +++ b/api-ref/source/samples/instance-mgmt-list-response.json @@ -1,6 +1,16 @@ { "instances": [ { + "addresses": [ + { + "address": "10.1.0.62", + "type": "private" + }, + { + "address": "172.24.5.114", + "type": "public" + } + ], "created": "2019-12-23T20:58:35", "datastore": { "type": "mysql", diff --git a/api-ref/source/samples/instance-mgmt-show-response.json b/api-ref/source/samples/instance-mgmt-show-response.json index 649f0b582d..eea90bc9bb 100644 --- a/api-ref/source/samples/instance-mgmt-show-response.json +++ b/api-ref/source/samples/instance-mgmt-show-response.json @@ -1,5 +1,15 @@ { "instance": { + "addresses": [ + { + "address": "10.1.0.62", + "type": "private" + }, + { + "address": "172.24.5.114", + "type": "public" + } + ], "created": "2019-12-23T20:58:35", "datastore": { "type": "mysql", diff --git a/api-ref/source/samples/instance-show-response.json b/api-ref/source/samples/instance-show-response.json index 4a162db3c4..32fa7f8be1 100644 --- a/api-ref/source/samples/instance-show-response.json +++ b/api-ref/source/samples/instance-show-response.json @@ -1,5 +1,15 @@ { "instance": { + "addresses": [ + { + "address": "10.1.0.62", + "type": "private" + }, + { + "address": "172.24.5.114", + "type": "public" + } + ], "created": "2019-12-23T20:58:35", "datastore": { "type": "mysql", diff --git a/devstack/plugin.sh b/devstack/plugin.sh index 39f34b6399..4de219e46b 100644 --- a/devstack/plugin.sh +++ b/devstack/plugin.sh @@ -556,7 +556,6 @@ function config_trove_network { sudo ip route # Now make sure the conf settings are right - iniset $TROVE_CONF DEFAULT network_label_regex "" iniset $TROVE_CONF DEFAULT ip_regex "" iniset $TROVE_CONF DEFAULT black_list_regex "" iniset $TROVE_CONF DEFAULT management_networks ${mgmt_net_id} diff --git a/doc/source/user/set-up-clustering.rst b/doc/source/user/set-up-clustering.rst index e5a0caa3d8..e487519ee1 100644 --- a/doc/source/user/set-up-clustering.rst +++ b/doc/source/user/set-up-clustering.rst @@ -2,6 +2,11 @@ Set up database clustering ========================== +.. caution:: + + Database clustering function is still in experimental, should not be used + in production environment. + You can store data across multiple machines by setting up MongoDB sharded clusters. diff --git a/etc/trove/trove.conf.sample b/etc/trove/trove.conf.sample index 75dffc8a72..e4fdd98cba 100644 --- a/etc/trove/trove.conf.sample +++ b/etc/trove/trove.conf.sample @@ -58,13 +58,6 @@ trove_auth_url = http://0.0.0.0/identity/v2.0 # Service type to use when searching catalog. #neutron_service_type = network -# Config option for showing the IP address that nova doles out -# For nova-network, set this to the appropriate network label defined in nova -# For neutron, set this to .* since users can specify custom network labels -# You can also optionally specify regex'es to match the actual IP addresses -# ip_regex (white-list) is applied before black_list_regex in the filter chain -network_label_regex = ^private$ -#network_label_regex = .* #with neutron enabled #ip_regex = ^(15.|123.) #black_list_regex = ^10.0.0. diff --git a/etc/trove/trove.conf.test b/etc/trove/trove.conf.test index 09a9bac7aa..92f07d7e27 100644 --- a/etc/trove/trove.conf.test +++ b/etc/trove/trove.conf.test @@ -50,7 +50,6 @@ nova_compute_service_type = compute nova_service_name = Compute Service # Config option for showing the IP address that nova doles out -network_label_regex = ^private$ ip_regex = ^(15.|123.) black_list_regex = ^(10.0.0.) diff --git a/releasenotes/notes/ussuri-add-ip-addresses-for-instance.yaml b/releasenotes/notes/ussuri-add-ip-addresses-for-instance.yaml new file mode 100644 index 0000000000..af2a559907 --- /dev/null +++ b/releasenotes/notes/ussuri-add-ip-addresses-for-instance.yaml @@ -0,0 +1,8 @@ +--- +features: + - Added a new field named ``addresses`` in the instance API + response which including the IP address and type, either 'private' or + 'public'. +deprecations: + - The 'ip' field of getting instance response is deprecated and will be + removed in W release. diff --git a/trove/cluster/views.py b/trove/cluster/views.py index ce669ade0c..174ac14fed 100644 --- a/trove/cluster/views.py +++ b/trove/cluster/views.py @@ -93,7 +93,6 @@ class ClusterView(object): ip_list.extend(instance_ips) if instance.type in instance_dict_to_be_published_for: instances.append(instance_dict) - ip_list.sort() return instances, ip_list def build_instances(self): diff --git a/trove/common/cfg.py b/trove/common/cfg.py index a97623f6a5..becfe393a2 100644 --- a/trove/common/cfg.py +++ b/trove/common/cfg.py @@ -373,7 +373,8 @@ common_opts = [ 'mariadb': '7a4f82cc-10d2-4bc6-aadc-d9aacc2a3cb5'}, help='Unique ID to tag notification events.'), cfg.StrOpt('network_label_regex', default='^private$', - help='Regular expression to match Trove network labels.'), + help='Regular expression to match Trove network labels.', + deprecated_for_removal=True), cfg.StrOpt('ip_regex', default=None, help='List IP addresses that match this regular expression.'), cfg.StrOpt('black_list_regex', default=None, diff --git a/trove/instance/models.py b/trove/instance/models.py index eda59e9889..f2c5cbaf0d 100644 --- a/trove/instance/models.py +++ b/trove/instance/models.py @@ -25,16 +25,12 @@ from novaclient import exceptions as nova_exceptions from oslo_config.cfg import NoSuchOptError from oslo_log import log as logging from oslo_utils import encodeutils +from oslo_utils import netutils from sqlalchemy import func from trove.backup.models import Backup from trove.common import cfg -from trove.common.clients import create_cinder_client -from trove.common.clients import create_dns_client -from trove.common.clients import create_glance_client -from trove.common.clients import create_guest_client -from trove.common.clients import create_neutron_client -from trove.common.clients import create_nova_client +from trove.common import clients from trove.common import crypto_utils as cu from trove.common import exception from trove.common.i18n import _ @@ -63,13 +59,16 @@ from trove.taskmanager import api as task_api CONF = cfg.CONF LOG = logging.getLogger(__name__) +# Invalid states to contact the agent +AGENT_INVALID_STATUSES = ["BUILD", "REBOOT", "RESIZE", "PROMOTE", "EJECT", + "UPGRADE"] -def filter_ips(ips, white_list_regex, black_list_regex): - """Return IPs matching white_list_regex and - Filter out IPs matching black_list_regex. - """ - return [ip for ip in ips if re.search(white_list_regex, ip) - and not re.search(black_list_regex, ip)] + +def ip_visible(ip, white_list_regex, black_list_regex): + if re.search(white_list_regex, ip) and not re.search(black_list_regex, ip): + return True + + return False def load_server(context, instance_id, server_id, region_name): @@ -84,7 +83,7 @@ def load_server(context, instance_id, server_id, region_name): :type server_id: unicode :rtype: novaclient.v2.servers.Server """ - client = create_nova_client(context, region_name=region_name) + client = clients.create_nova_client(context, region_name=region_name) try: server = client.servers.get(server_id) except nova_exceptions.NotFound: @@ -129,21 +128,42 @@ def load_simple_instance_server_status(context, db_info): """Loads a server or raises an exception.""" if 'BUILDING' == db_info.task_status.action: db_info.server_status = "BUILD" - db_info.addresses = {} else: - client = create_nova_client(context, db_info.region_id) + client = clients.create_nova_client(context, db_info.region_id) try: server = client.servers.get(db_info.compute_instance_id) db_info.server_status = server.status - db_info.addresses = server.addresses except nova_exceptions.NotFound: db_info.server_status = "SHUTDOWN" - db_info.addresses = {} -# Invalid states to contact the agent -AGENT_INVALID_STATUSES = ["BUILD", "REBOOT", "RESIZE", "PROMOTE", "EJECT", - "UPGRADE"] +def load_simple_instance_addresses(context, db_info): + """Get addresses of the instance from Neutron.""" + if 'BUILDING' == db_info.task_status.action: + db_info.addresses = [] + return + + addresses = [] + client = clients.create_neutron_client(context, db_info.region_id) + ports = client.list_ports(device_id=db_info.compute_instance_id)['ports'] + for port in ports: + if 'Management port' not in port['description']: + LOG.debug('Found user port %s for instance %s', port['id'], + db_info.id) + for ip in port['fixed_ips']: + # TODO(lxkong): IPv6 is not supported + if netutils.is_valid_ipv4(ip.get('ip_address')): + addresses.append( + {'address': ip['ip_address'], 'type': 'private'}) + + fips = client.list_floatingips(port_id=port['id']) + if len(fips['floatingips']) == 0: + continue + fip = fips['floatingips'][0] + addresses.append( + {'address': fip['floating_ip_address'], 'type': 'public'}) + + db_info.addresses = addresses class SimpleInstance(object): @@ -196,13 +216,6 @@ class SimpleInstance(object): @property def addresses(self): - # TODO(tim.simpson): This code attaches two parts of the Nova server to - # db_info: "status" and "addresses". The idea - # originally was to listen to events to update this - # data and store it in the Trove database. - # However, it may have been unwise as a year and a - # half later we still have to load the server anyway - # and this makes the code confusing. if hasattr(self.db_info, 'addresses'): return self.db_info.addresses else: @@ -217,6 +230,7 @@ class SimpleInstance(object): """Returns the IP address to be used with DNS.""" ips = self.get_visible_ip_addresses() if ips: + # FIXME return ips[0] @property @@ -234,20 +248,14 @@ class SimpleInstance(object): return None IPs = [] - mgmt_networks = neutron.get_management_networks(self.context) - for label in self.addresses: - if label in mgmt_networks: - continue - if (CONF.network_label_regex and - not re.search(CONF.network_label_regex, label)): - continue + for addr_info in self.addresses: + if CONF.ip_regex and CONF.black_list_regex: + if not ip_visible(addr_info['address'], CONF.ip_regex, + CONF.black_list_regex): + continue - IPs.extend([addr.get('addr') for addr in self.addresses[label]]) - - # Includes ip addresses that match the regexp pattern - if CONF.ip_regex and CONF.black_list_regex: - IPs = filter_ips(IPs, CONF.ip_regex, CONF.black_list_regex) + IPs.append(addr_info) return IPs @@ -550,15 +558,16 @@ def load_instance(cls, context, id, needs_server=False, # necessary and instead we'll just use the server_status field from # the instance table. load_simple_instance_server_status(context, db_info) + load_simple_instance_addresses(context, db_info) server = None else: try: server = load_server(context, db_info.id, db_info.compute_instance_id, region_name=db_info.region_id) - # TODO(tim.simpson): Remove this hack when we have notifications! db_info.server_status = server.status - db_info.addresses = server.addresses + + load_simple_instance_addresses(context, db_info) except exception.ComputeInstanceNotFound: LOG.error("Could not load compute instance %s.", db_info.compute_instance_id) @@ -568,24 +577,32 @@ def load_instance(cls, context, id, needs_server=False, service_status = InstanceServiceStatus.find_by(instance_id=id) LOG.debug("Instance %(instance_id)s service status is %(service_status)s.", {'instance_id': id, 'service_status': service_status.status}) + return cls(context, db_info, server, service_status) def load_instance_with_info(cls, context, id, cluster_id=None): db_info = get_db_info(context, id, cluster_id) + load_simple_instance_server_status(context, db_info) + + load_simple_instance_addresses(context, db_info) + service_status = InstanceServiceStatus.find_by(instance_id=id) LOG.debug("Instance %(instance_id)s service status is %(service_status)s.", {'instance_id': id, 'service_status': service_status.status}) instance = cls(context, db_info, service_status) + load_guest_info(instance, context, id) + load_server_group_info(instance, context) + return instance def load_guest_info(instance, context, id): if instance.status not in AGENT_INVALID_STATUSES: - guest = create_guest_client(context, id) + guest = clients.create_guest_client(context, id) try: volume_info = guest.get_volume_info() instance.volume_used = volume_info['used'] @@ -646,7 +663,7 @@ class BaseInstance(SimpleInstance): self._server_group_loaded = False def get_guest(self): - return create_guest_client(self.context, self.db_info.id) + return clients.create_guest_client(self.context, self.db_info.id) def delete(self): def _delete_resources(): @@ -754,7 +771,7 @@ class BaseInstance(SimpleInstance): try: dns_support = CONF.trove_dns_support if dns_support: - dns_api = create_dns_client(self.context) + dns_api = clients.create_dns_client(self.context) dns_api.delete_instance_entry(instance_id=self.id) except Exception as e: LOG.warning("Failed to delete dns entry of instance %s, error: %s", @@ -839,7 +856,7 @@ class BaseInstance(SimpleInstance): @property def nova_client(self): if not self._nova_client: - self._nova_client = create_nova_client( + self._nova_client = clients.create_nova_client( self.context, region_name=self.db_info.region_id) return self._nova_client @@ -866,14 +883,14 @@ class BaseInstance(SimpleInstance): @property def volume_client(self): if not self._volume_client: - self._volume_client = create_cinder_client( + self._volume_client = clients.create_cinder_client( self.context, region_name=self.db_info.region_id) return self._volume_client @property def neutron_client(self): if not self._neutron_client: - self._neutron_client = create_neutron_client( + self._neutron_client = clients.create_neutron_client( self.context, region_name=self.db_info.region_id) return self._neutron_client @@ -966,8 +983,8 @@ class Instance(BuiltInstance): @classmethod def _validate_remote_datastore(cls, context, region_name, flavor, datastore, datastore_version): - remote_nova_client = create_nova_client(context, - region_name=region_name) + remote_nova_client = clients.create_nova_client( + context, region_name=region_name) try: remote_flavor = remote_nova_client.flavors.get(flavor.id) if (flavor.ram != remote_flavor.ram or @@ -1000,9 +1017,9 @@ class Instance(BuiltInstance): "Datastore Version %(dsv)s not found in region %(remote)s." % {'dsv': datastore_version.name, 'remote': region_name}) - glance_client = create_glance_client(context) + glance_client = clients.create_glance_client(context) local_image = glance_client.images.get(datastore_version.image) - remote_glance_client = create_glance_client( + remote_glance_client = clients.create_glance_client( context, region_name=region_name) remote_image = remote_glance_client.images.get( remote_ds_ver.image) @@ -1050,7 +1067,7 @@ class Instance(BuiltInstance): flavor_id=flavor_id) datastore_cfg = CONF.get(datastore_version.manager) - client = create_nova_client(context) + client = clients.create_nova_client(context) try: flavor = client.flavors.get(flavor_id) except nova_exceptions.NotFound: @@ -1242,7 +1259,7 @@ class Instance(BuiltInstance): status=tr_instance.ServiceStatuses.NEW) if CONF.trove_dns_support: - dns_client = create_dns_client(context) + dns_client = clients.create_dns_client(context) hostname = dns_client.determine_hostname(instance_id) db_info.hostname = hostname db_info.save() @@ -1659,7 +1676,7 @@ class Instances(object): if context is None: raise TypeError(_("Argument context not defined.")) - client = create_nova_client(context) + client = clients.create_nova_client(context) servers = client.servers.list() query_opts = {'tenant_id': context.project_id, 'deleted': False} @@ -1711,26 +1728,25 @@ class Instances(object): for db in db_items: server = None try: - # TODO(tim.simpson): Delete when we get notifications working! if InstanceTasks.BUILDING == db.task_status: db.server_status = "BUILD" - db.addresses = {} + db.addresses = [] else: try: region = CONF.service_credentials.region_name if (not db.region_id or db.region_id == region): server = find_server(db.id, db.compute_instance_id) else: - nova_client = create_nova_client( + nova_client = clients.create_nova_client( context, region_name=db.region_id) server = nova_client.servers.get( db.compute_instance_id) db.server_status = server.status - db.addresses = server.addresses + + load_simple_instance_addresses(context, db) except exception.ComputeInstanceNotFound: db.server_status = "SHUTDOWN" # Fake it... - db.addresses = {} - # TODO(tim.simpson): End of hack. + db.addresses = [] # volumes = find_volumes(server.id) datastore_status = InstanceServiceStatus.find_by( diff --git a/trove/instance/views.py b/trove/instance/views.py index a79b27b8d2..92fd0a58e3 100644 --- a/trove/instance/views.py +++ b/trove/instance/views.py @@ -53,9 +53,12 @@ class InstanceView(object): if self.instance.hostname: instance_dict['hostname'] = self.instance.hostname else: - ip = self.instance.get_visible_ip_addresses() - if ip: - instance_dict['ip'] = ip + addresses = self.instance.get_visible_ip_addresses() + if addresses: + # NOTE(lxkong): 'ip' is deprecated in stable/ussuri and should + # be removed in W. + instance_dict['ip'] = [addr['address'] for addr in addresses] + instance_dict['addresses'] = addresses if self.instance.slave_of_id is not None: instance_dict['replica_of'] = self._build_master_info() diff --git a/trove/taskmanager/manager.py b/trove/taskmanager/manager.py index b103704b16..ce1bcff809 100644 --- a/trove/taskmanager/manager.py +++ b/trove/taskmanager/manager.py @@ -344,6 +344,8 @@ class Manager(periodic_task.PeriodicTasks): snapshot = instance_tasks.get_replication_master_snapshot( context, slave_of_id, flavor, replica_backup_id, replica_number=replica_number) + LOG.info('Snapshot info for creating replica of %s: %s', + slave_of_id, snapshot) replica_backup_id = snapshot['dataset']['snapshot_id'] replica_backup_created = (replica_backup_id is not None) diff --git a/trove/taskmanager/models.py b/trove/taskmanager/models.py index da3ad45ad1..6eaa5e04c0 100755 --- a/trove/taskmanager/models.py +++ b/trove/taskmanager/models.py @@ -198,7 +198,7 @@ class ClusterTasks(Cluster): @classmethod def get_ip(cls, instance): - return instance.get_visible_ip_addresses()[0] + return instance.get_visible_ip_addresses()[0].get('address') def _all_instances_ready(self, instance_ids, cluster_id, shard_id=None): @@ -1170,19 +1170,22 @@ class BuiltInstanceTasks(BuiltInstance, NotifyMixin, ConfigurationMixin): return floating_ips def detach_public_ips(self): - LOG.debug("Begin detach_public_ips for instance %s", self.id) + LOG.info("Begin detach_public_ips for instance %s", self.id) removed_ips = [] floating_ips = self._get_floating_ips() - for ip in self.get_visible_ip_addresses(): - if ip in floating_ips: - fip_id = floating_ips[ip] - self.neutron_client.update_floatingip( - fip_id, {'floatingip': {'port_id': None}}) - removed_ips.append(fip_id) + + for item in self.get_visible_ip_addresses(): + if item['type'] == 'public': + ip = item['address'] + if ip in floating_ips: + fip_id = floating_ips[ip] + self.neutron_client.update_floatingip( + fip_id, {'floatingip': {'port_id': None}}) + removed_ips.append(fip_id) return removed_ips def attach_public_ips(self, ips): - LOG.debug("Begin attach_public_ips for instance %s", self.id) + LOG.info("Begin attach_public_ips for instance %s", self.id) server_id = self.db_info.compute_instance_id # NOTE(zhaochao): in Nova's addFloatingIp, the new floating ip will diff --git a/trove/tests/api/instances.py b/trove/tests/api/instances.py index d6be7bddca..998529f8ca 100644 --- a/trove/tests/api/instances.py +++ b/trove/tests/api/instances.py @@ -919,7 +919,7 @@ class TestGetInstances(object): def test_index_list(self): allowed_attrs = ['id', 'links', 'name', 'status', 'flavor', 'datastore', 'ip', 'hostname', 'replica_of', - 'region'] + 'region', 'addresses'] if VOLUME_SUPPORT: allowed_attrs.append('volume') instances = dbaas.instances.list() @@ -941,7 +941,7 @@ class TestGetInstances(object): allowed_attrs = ['created', 'databases', 'flavor', 'hostname', 'id', 'links', 'name', 'status', 'updated', 'ip', 'datastore', 'fault', 'region', - 'service_status_updated'] + 'service_status_updated', 'addresses'] if VOLUME_SUPPORT: allowed_attrs.append('volume') instances = dbaas.instances.list(detailed=True) @@ -961,7 +961,7 @@ class TestGetInstances(object): allowed_attrs = ['created', 'databases', 'flavor', 'hostname', 'id', 'links', 'name', 'status', 'updated', 'ip', 'datastore', 'fault', 'region', - 'service_status_updated'] + 'service_status_updated', 'addresses'] if VOLUME_SUPPORT: allowed_attrs.append('volume') else: diff --git a/trove/tests/unittests/cluster/test_cluster_views.py b/trove/tests/unittests/cluster/test_cluster_views.py index 1479bae26c..b1a5ef5d99 100644 --- a/trove/tests/unittests/cluster/test_cluster_views.py +++ b/trove/tests/unittests/cluster/test_cluster_views.py @@ -82,13 +82,16 @@ class ClusterViewTest(trove_testtools.TestCase): cluster.instances.append(Mock()) cluster.instances.append(Mock()) cluster.instances[0].type = 'configsvr' - cluster.instances[0].get_visible_ip_addresses = lambda: ['1.2.3.4'] + cluster.instances[0].get_visible_ip_addresses.return_value = [ + {'type': 'private', 'address': '1.2.3.4'}] cluster.instances[0].datastore_version.manager = 'mongodb' cluster.instances[1].type = 'query_router' - cluster.instances[1].get_visible_ip_addresses = lambda: ['1.2.3.4'] + cluster.instances[1].get_visible_ip_addresses.return_value = [ + {'type': 'private', 'address': '1.2.3.4'}] cluster.instances[1].datastore_version.manager = 'mongodb' cluster.instances[2].type = 'member' - cluster.instances[2].get_visible_ip_addresses = lambda: ['1.2.3.4'] + cluster.instances[2].get_visible_ip_addresses.return_value = [ + {'type': 'private', 'address': '1.2.3.4'}] cluster.instances[2].datastore_version.manager = 'mongodb' def test_case(ip_to_be_published_for, @@ -124,7 +127,8 @@ class ClusterInstanceDetailViewTest(trove_testtools.TestCase): self.instance.addresses = {"private": [{"addr": self.ip}]} self.instance.volume_used = '3' self.instance.root_password = 'iloveyou' - self.instance.get_visible_ip_addresses = lambda: ["1.2.3.4"] + self.instance.get_visible_ip_addresses.return_value = [ + {'type': 'private', 'address': '1.2.3.4'}] self.instance.slave_of_id = None self.instance.slaves = None self.context = trove_testtools.TroveTestContext(self) @@ -162,3 +166,5 @@ class ClusterInstanceDetailViewTest(trove_testtools.TestCase): result['instance']['datastore']['version']) self.assertNotIn('hostname', result['instance']) self.assertEqual([self.ip], result['instance']['ip']) + self.assertEqual(self.ip, + result['instance']['addresses'][0]['address']) diff --git a/trove/tests/unittests/instance/test_instance_models.py b/trove/tests/unittests/instance/test_instance_models.py index 714e72dafa..7c4ca2822a 100644 --- a/trove/tests/unittests/instance/test_instance_models.py +++ b/trove/tests/unittests/instance/test_instance_models.py @@ -17,6 +17,7 @@ from mock import Mock, patch from trove.backup import models as backup_models from trove.common import cfg +from trove.common import clients from trove.common import exception from trove.common.instance import ServiceStatuses from trove.common import neutron @@ -24,7 +25,6 @@ from trove.datastore import models as datastore_models from trove.instance import models from trove.instance.models import DBInstance from trove.instance.models import DBInstanceFault -from trove.instance.models import filter_ips from trove.instance.models import Instance from trove.instance.models import instance_encryption_key_cache from trove.instance.models import InstanceServiceStatus @@ -50,16 +50,16 @@ class SimpleInstanceTest(trove_testtools.TestCase): ServiceStatuses.BUILDING), ds_version=Mock(), ds=Mock(), locality='affinity') self.instance.context = self.context - db_info.addresses = {"private": [{"addr": "123.123.123.123"}], - "internal": [{"addr": "10.123.123.123"}], - "public": [{"addr": "15.123.123.123"}]} - self.orig_conf = CONF.network_label_regex + db_info.addresses = [ + {'type': 'private', 'address': '123.123.123.123'}, + {'type': 'private', 'address': '10.123.123.123'}, + {'type': 'public', 'address': '15.123.123.123'}, + ] self.orig_ip_regex = CONF.ip_regex self.orig_black_list_regex = CONF.black_list_regex def tearDown(self): super(SimpleInstanceTest, self).tearDown() - CONF.network_label_regex = self.orig_conf CONF.ip_start = None CONF.management_networks = [] CONF.ip_regex = self.orig_ip_regex @@ -73,64 +73,22 @@ class SimpleInstanceTest(trove_testtools.TestCase): self.assertFalse(root_on_create_val) def test_filter_ips_white_list(self): - CONF.network_label_regex = '.*' CONF.ip_regex = '^(15.|123.)' CONF.black_list_regex = '^10.123.123.*' ip = self.instance.get_visible_ip_addresses() - ip = filter_ips( - ip, CONF.ip_regex, CONF.black_list_regex) + ip = [addr['address'] for addr in ip] self.assertEqual(2, len(ip)) self.assertIn('123.123.123.123', ip) self.assertIn('15.123.123.123', ip) def test_filter_ips_black_list(self): - CONF.network_label_regex = '.*' CONF.ip_regex = '.*' CONF.black_list_regex = '^10.123.123.*' ip = self.instance.get_visible_ip_addresses() - ip = filter_ips( - ip, CONF.ip_regex, CONF.black_list_regex) + ip = [addr['address'] for addr in ip] self.assertEqual(2, len(ip)) self.assertNotIn('10.123.123.123', ip) - def test_one_network_label(self): - CONF.network_label_regex = 'public' - ip = self.instance.get_visible_ip_addresses() - self.assertEqual(['15.123.123.123'], ip) - - def test_two_network_labels(self): - CONF.network_label_regex = '^(private|public)$' - ip = self.instance.get_visible_ip_addresses() - self.assertEqual(2, len(ip)) - self.assertIn('123.123.123.123', ip) - self.assertIn('15.123.123.123', ip) - - def test_all_network_labels(self): - CONF.network_label_regex = '.*' - ip = self.instance.get_visible_ip_addresses() - self.assertEqual(3, len(ip)) - self.assertIn('10.123.123.123', ip) - self.assertIn('123.123.123.123', ip) - self.assertIn('15.123.123.123', ip) - - @patch('trove.common.clients.create_neutron_client') - def test_filter_management_ip_addresses(self, mock_neutron_client): - CONF.network_label_regex = '' - CONF.management_networks = ['fake-net-id'] - - neutron_client = Mock() - neutron_client.show_network.return_value = { - 'network': {'name': 'public'} - } - mock_neutron_client.return_value = neutron_client - - ip = self.instance.get_visible_ip_addresses() - - neutron_client.show_network.assert_called_once_with('fake-net-id') - self.assertEqual(2, len(ip)) - self.assertIn('123.123.123.123', ip) - self.assertIn('10.123.123.123', ip) - def test_locality(self): self.assertEqual('affinity', self.instance.locality) @@ -208,8 +166,10 @@ class CreateInstanceTest(trove_testtools.TestCase): deleted=False ) self.backup_id = self.backup.id - self.orig_client = models.create_nova_client - models.create_nova_client = nova.fake_create_nova_client + + self.orig_client = clients.create_nova_client + clients.create_nova_client = nova.fake_create_nova_client + self.orig_api = task_api.API(self.context).create_instance task_api.API(self.context).create_instance = Mock() self.run_with_quotas = models.run_with_quotas @@ -232,7 +192,7 @@ class CreateInstanceTest(trove_testtools.TestCase): self.backup.delete() self.datastore.delete() self.datastore_version.delete() - models.create_nova_client = self.orig_client + clients.create_nova_client = self.orig_client task_api.API(self.context).create_instance = self.orig_api models.run_with_quotas = self.run_with_quotas backup_models.DBBackup.check_swift_object_exist = self.check @@ -310,21 +270,22 @@ class TestInstanceUpgrade(trove_testtools.TestCase): manager='test', active=1) - self.safe_nova_client = models.create_nova_client - models.create_nova_client = nova.fake_create_nova_client + self.safe_nova_client = clients.create_nova_client + clients.create_nova_client = nova.fake_create_nova_client super(TestInstanceUpgrade, self).setUp() def tearDown(self): self.datastore.delete() self.datastore_version1.delete() self.datastore_version2.delete() - models.create_nova_client = self.safe_nova_client + clients.create_nova_client = self.safe_nova_client super(TestInstanceUpgrade, self).tearDown() + @patch('trove.common.clients.create_neutron_client') @patch.object(task_api.API, 'get_client', Mock(return_value=Mock())) @patch.object(task_api.API, 'upgrade') @patch('trove.tests.fakes.nova.LOG') - def test_upgrade(self, mock_logging, task_upgrade): + def test_upgrade(self, mock_logging, task_upgrade, mock_neutron_client): instance_model = DBInstance( InstanceTasks.NONE, id=str(uuid.uuid4()), @@ -391,8 +352,8 @@ class TestReplication(trove_testtools.TestCase): instance_id=self.master.id) self.master_status.save() - self.safe_nova_client = models.create_nova_client - models.create_nova_client = nova.fake_create_nova_client + self.safe_nova_client = clients.create_nova_client + clients.create_nova_client = nova.fake_create_nova_client self.swift_verify_patch = patch.object(models.Backup, 'verify_swift_auth_token') @@ -406,7 +367,7 @@ class TestReplication(trove_testtools.TestCase): self.master_status.delete() self.datastore.delete() self.datastore_version.delete() - models.create_nova_client = self.safe_nova_client + clients.create_nova_client = self.safe_nova_client super(TestReplication, self).tearDown() @patch('trove.instance.models.LOG') diff --git a/trove/tests/unittests/instance/test_instance_views.py b/trove/tests/unittests/instance/test_instance_views.py index e41836d8e6..8d5923b029 100644 --- a/trove/tests/unittests/instance/test_instance_views.py +++ b/trove/tests/unittests/instance/test_instance_views.py @@ -29,12 +29,10 @@ class InstanceViewsTest(trove_testtools.TestCase): self.addresses = {"private": [{"addr": "123.123.123.123"}], "internal": [{"addr": "10.123.123.123"}], "public": [{"addr": "15.123.123.123"}]} - self.orig_label_regex = CONF.network_label_regex self.orig_ip_regex = CONF.ip_regex def tearDown(self): super(InstanceViewsTest, self).tearDown() - CONF.network_label_regex = self.orig_label_regex CONF.ip_regex = self.orig_ip_regex @@ -59,7 +57,8 @@ class InstanceDetailViewTest(trove_testtools.TestCase): self.instance.addresses = {"private": [{"addr": self.ip}]} self.instance.volume_used = '3' self.instance.root_password = 'iloveyou' - self.instance.get_visible_ip_addresses = lambda: ["1.2.3.4"] + self.instance.get_visible_ip_addresses.return_value = [ + {'type': 'private', 'address': '1.2.3.4'}] self.instance.slave_of_id = None self.instance.slaves = [] self.instance.locality = 'affinity' @@ -104,6 +103,8 @@ class InstanceDetailViewTest(trove_testtools.TestCase): result['instance']['datastore']['version']) self.assertNotIn('hostname', result['instance']) self.assertEqual([self.ip], result['instance']['ip']) + self.assertEqual(self.ip, + result['instance']['addresses'][0]['address']) def test_locality(self): self.instance.hostname = None diff --git a/trove/tests/unittests/taskmanager/test_models.py b/trove/tests/unittests/taskmanager/test_models.py index 9ac99a691e..721a9b4477 100644 --- a/trove/tests/unittests/taskmanager/test_models.py +++ b/trove/tests/unittests/taskmanager/test_models.py @@ -932,7 +932,7 @@ class BuiltInstanceTasksTest(trove_testtools.TestCase): floating_ips) @patch.object(BaseInstance, 'get_visible_ip_addresses', - return_value=['192.168.10.1']) + return_value=[{'address': '192.168.10.1', 'type': 'public'}]) def test_detach_public_ips(self, mock_address): removed_ips = self.instance_task.detach_public_ips() self.assertEqual(['fake-floatingip-id'], removed_ips)