From b70dce2aab5b1c496786d4258a93f3c7ea3dc267 Mon Sep 17 00:00:00 2001 From: Salvatore Orlando Date: Sun, 27 Oct 2013 14:09:57 -0700 Subject: [PATCH] Add DB mappings with NSX logical routers This patch introduces DB mappings between neutron and NSX router, thus not requiring anymore the Neutron router ID to be equal to the NSX one. This change is needed for enabling asynchronous operations in the NSX plugin. This patch also performs NVP/NSX renaming where appropriate, and fixes delete router logic causing a 500 HTTP error to be returned when a Neutron internal error occurs. Related to blueprint nvp-async-backend-communication Related to blueprint nicira-plugin-renaming Change-Id: Ib0e9ed0f58e7fa3497a93e9cd3baa9cb81ad797b --- .../4ca36cfc898c_nsx_router_mappings.py | 62 +++++ neutron/plugins/nicira/NeutronPlugin.py | 230 +++++++++++------- .../plugins/nicira/NeutronServicePlugin.py | 53 ++-- neutron/plugins/nicira/common/nsx_utils.py | 55 ++++- neutron/plugins/nicira/common/sync.py | 4 +- neutron/plugins/nicira/dbexts/nicira_db.py | 31 ++- .../plugins/nicira/dbexts/nicira_models.py | 9 + neutron/plugins/nicira/nvplib.py | 42 ++-- .../tests/unit/nicira/test_nicira_plugin.py | 5 +- neutron/tests/unit/nicira/test_nsx_utils.py | 58 ++++- neutron/tests/unit/nicira/test_nvp_sync.py | 12 +- neutron/tests/unit/nicira/test_nvplib.py | 90 +++++-- 12 files changed, 476 insertions(+), 175 deletions(-) create mode 100644 neutron/db/migration/alembic_migrations/versions/4ca36cfc898c_nsx_router_mappings.py diff --git a/neutron/db/migration/alembic_migrations/versions/4ca36cfc898c_nsx_router_mappings.py b/neutron/db/migration/alembic_migrations/versions/4ca36cfc898c_nsx_router_mappings.py new file mode 100644 index 000000000..f259161a5 --- /dev/null +++ b/neutron/db/migration/alembic_migrations/versions/4ca36cfc898c_nsx_router_mappings.py @@ -0,0 +1,62 @@ +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. +# + +"""nsx_router_mappings + +Revision ID: 4ca36cfc898c +Revises: 3d3cb89d84ee +Create Date: 2014-01-08 10:41:43.373031 + +""" + +# revision identifiers, used by Alembic. +revision = '4ca36cfc898c' +down_revision = '3d3cb89d84ee' + +# Change to ['*'] if this migration applies to all plugins + +migration_for_plugins = [ + 'neutron.plugins.nicira.NeutronPlugin.NvpPluginV2', + 'neutron.plugins.nicira.NeutronServicePlugin.NvpAdvancedPlugin' +] + +from alembic import op +import sqlalchemy as sa + +from neutron.db import migration + + +def upgrade(active_plugins=None, options=None): + if not migration.should_run(active_plugins, migration_for_plugins): + return + + # Create table for router/lrouter mappings + op.create_table( + 'neutron_nsx_router_mappings', + sa.Column('neutron_id', sa.String(length=36), nullable=False), + sa.Column('nsx_id', sa.String(length=36), nullable=True), + sa.ForeignKeyConstraint(['neutron_id'], ['routers.id'], + ondelete='CASCADE'), + sa.PrimaryKeyConstraint('neutron_id'), + ) + # Execute statement to a record in nsx_router_mappings for + # each record in routers + op.execute("INSERT INTO neutron_nsx_router_mappings SELECT id,id " + "from routers") + + +def downgrade(active_plugins=None, options=None): + if not migration.should_run(active_plugins, migration_for_plugins): + return + + op.drop_table('neutron_nsx_router_mappings') diff --git a/neutron/plugins/nicira/NeutronPlugin.py b/neutron/plugins/nicira/NeutronPlugin.py index 2f543c870..c4ad96bea 100644 --- a/neutron/plugins/nicira/NeutronPlugin.py +++ b/neutron/plugins/nicira/NeutronPlugin.py @@ -246,7 +246,7 @@ class NvpPluginV2(addr_pair_db.AllowedAddressPairsMixin, return ip_addresses def _create_and_attach_router_port(self, cluster, context, - router_id, port_data, + nsx_router_id, port_data, attachment_type, attachment, attachment_vlan=None, subnet_ids=None): @@ -258,19 +258,20 @@ class NvpPluginV2(addr_pair_db.AllowedAddressPairsMixin, subnet_ids)) try: lrouter_port = nvplib.create_router_lport( - cluster, router_id, port_data.get('tenant_id', 'fake'), + cluster, nsx_router_id, port_data.get('tenant_id', 'fake'), port_data.get('id', 'fake'), port_data.get('name', 'fake'), port_data.get('admin_state_up', True), ip_addresses, port_data.get('mac_address')) LOG.debug(_("Created NVP router port:%s"), lrouter_port['uuid']) except NvpApiClient.NvpApiException: LOG.exception(_("Unable to create port on NVP logical router %s"), - router_id) + nsx_router_id) raise nvp_exc.NvpPluginException( err_msg=_("Unable to create logical router port for neutron " - "port id %(port_id)s on router %(router_id)s") % - {'port_id': port_data.get('id'), 'router_id': router_id}) - self._update_router_port_attachment(cluster, context, router_id, + "port id %(port_id)s on router %(nsx_router_id)s") % + {'port_id': port_data.get('id'), + 'nsx_router_id': nsx_router_id}) + self._update_router_port_attachment(cluster, context, nsx_router_id, port_data, lrouter_port['uuid'], attachment_type, attachment, attachment_vlan) @@ -307,12 +308,14 @@ class NvpPluginV2(addr_pair_db.AllowedAddressPairsMixin, ctx_elevated = context.elevated() if remove_snat_rules or add_snat_rules: cidrs = self._find_router_subnets_cidrs(ctx_elevated, router_id) + nsx_router_id = nsx_utils.get_nsx_router_id( + context.session, self.cluster, router_id) if remove_snat_rules: # Be safe and concede NAT rules might not exist. # Therefore use min_num_expected=0 for cidr in cidrs: nvplib.delete_nat_rules_by_match( - self.cluster, router_id, "SourceNatRule", + self.cluster, nsx_router_id, "SourceNatRule", max_num_expected=1, min_num_expected=0, source_ip_addresses=cidr) if add_snat_rules: @@ -322,14 +325,14 @@ class NvpPluginV2(addr_pair_db.AllowedAddressPairsMixin, for cidr in cidrs: cidr_prefix = int(cidr.split('/')[1]) nvplib.create_lrouter_snat_rule( - self.cluster, router_id, + self.cluster, nsx_router_id, ip_addresses[0].split('/')[0], ip_addresses[0].split('/')[0], order=NVP_EXTGW_NAT_RULES_ORDER - cidr_prefix, match_criteria={'source_ip_addresses': cidr}) def _update_router_port_attachment(self, cluster, context, - router_id, port_data, + nsx_router_id, port_data, nvp_router_port_id, attachment_type, attachment, @@ -337,7 +340,7 @@ class NvpPluginV2(addr_pair_db.AllowedAddressPairsMixin, if not nvp_router_port_id: nvp_router_port_id = self._find_router_gw_port(context, port_data) try: - nvplib.plug_router_port_attachment(cluster, router_id, + nvplib.plug_router_port_attachment(cluster, nsx_router_id, nvp_router_port_id, attachment, attachment_type, @@ -346,7 +349,7 @@ class NvpPluginV2(addr_pair_db.AllowedAddressPairsMixin, {'att': attachment, 'port': nvp_router_port_id}) except NvpApiClient.NvpApiException: # Must remove NVP logical port - nvplib.delete_router_lport(cluster, router_id, + nvplib.delete_router_lport(cluster, nsx_router_id, nvp_router_port_id) LOG.exception(_("Unable to plug attachment in NVP logical " "router port %(r_port_id)s, associated with " @@ -359,7 +362,7 @@ class NvpPluginV2(addr_pair_db.AllowedAddressPairsMixin, "on router %(router_id)s") % {'r_port_id': nvp_router_port_id, 'q_port_id': port_data.get('id'), - 'router_id': router_id})) + 'router_id': nsx_router_id})) def _get_port_by_device_id(self, context, device_id, device_owner): """Retrieve ports associated with a specific device id. @@ -520,7 +523,8 @@ class NvpPluginV2(addr_pair_db.AllowedAddressPairsMixin, def _nvp_delete_router_port(self, context, port_data): # Delete logical router port - lrouter_id = port_data['device_id'] + nsx_router_id = nsx_utils.get_nsx_router_id( + context.session, self.cluster, port_data['device_id']) nvp_switch_id, nvp_port_id = nsx_utils.get_nsx_switch_and_port_id( context.session, self.cluster, port_data['id']) if not nvp_port_id: @@ -528,11 +532,11 @@ class NvpPluginV2(addr_pair_db.AllowedAddressPairsMixin, "Terminating delete operation. A dangling router port " "might have been left on router %(router_id)s"), {'port_id': port_data['id'], - 'router_id': lrouter_id}) + 'router_id': nsx_router_id}) return try: nvplib.delete_peer_router_lport(self.cluster, - lrouter_id, + nsx_router_id, nvp_switch_id, nvp_port_id) except NvpApiClient.NvpApiException: @@ -566,10 +570,11 @@ class NvpPluginV2(addr_pair_db.AllowedAddressPairsMixin, # Assuming subnet being attached is on first fixed ip # element in port data subnet_id = port_data['fixed_ips'][0]['subnet_id'] - router_id = port_data['device_id'] + nsx_router_id = nsx_utils.get_nsx_router_id( + context.session, self.cluster, port_data['device_id']) # Create peer port on logical router self._create_and_attach_router_port( - self.cluster, context, router_id, port_data, + self.cluster, context, nsx_router_id, port_data, "PatchAttachment", ls_port['uuid'], subnet_ids=[subnet_id]) nicira_db.add_neutron_nsx_port_mapping( @@ -592,13 +597,15 @@ class NvpPluginV2(addr_pair_db.AllowedAddressPairsMixin, "order to create an external gateway " "port for network %s"), port_data['network_id']) - - lr_port = nvplib.find_router_gw_port(context, self.cluster, router_id) + nsx_router_id = nsx_utils.get_nsx_router_id( + context.session, self.cluster, router_id) + lr_port = nvplib.find_router_gw_port(context, self.cluster, + nsx_router_id) if not lr_port: raise nvp_exc.NvpPluginException( - err_msg=(_("The gateway port for the router %s " - "was not found on the NVP backend") - % router_id)) + err_msg=(_("The gateway port for the NSX router %s " + "was not found on the backend") + % nsx_router_id)) return lr_port @lockutils.synchronized('nicira', 'neutron-') @@ -615,9 +622,10 @@ class NvpPluginV2(addr_pair_db.AllowedAddressPairsMixin, # the fabric status of the NVP router will be down. # admin_status should always be up for the gateway port # regardless of what the user specifies in neutron - router_id = port_data['device_id'] + nsx_router_id = nsx_utils.get_nsx_router_id( + context.session, self.cluster, port_data['device_id']) nvplib.update_router_lport(self.cluster, - router_id, + nsx_router_id, lr_port['uuid'], port_data['tenant_id'], port_data['id'], @@ -630,7 +638,7 @@ class NvpPluginV2(addr_pair_db.AllowedAddressPairsMixin, physical_network = (ext_network[pnet.PHYSICAL_NETWORK] or self.cluster.default_l3_gw_service_uuid) self._update_router_port_attachment( - self.cluster, context, router_id, port_data, + self.cluster, context, nsx_router_id, port_data, lr_port['uuid'], "L3GatewayAttachment", physical_network, @@ -640,7 +648,7 @@ class NvpPluginV2(addr_pair_db.AllowedAddressPairsMixin, "%(ext_net_id)s, attached to router:%(router_id)s. " "NVP port id is %(nvp_port_id)s"), {'ext_net_id': port_data['network_id'], - 'router_id': router_id, + 'router_id': nsx_router_id, 'nvp_port_id': lr_port['uuid']}) @lockutils.synchronized('nicira', 'neutron-') @@ -652,8 +660,10 @@ class NvpPluginV2(addr_pair_db.AllowedAddressPairsMixin, # Delete is actually never a real delete, otherwise the NVP # logical router will stop working router_id = port_data['device_id'] + nsx_router_id = nsx_utils.get_nsx_router_id( + context.session, self.cluster, router_id) nvplib.update_router_lport(self.cluster, - router_id, + nsx_router_id, lr_port['uuid'], port_data['tenant_id'], port_data['id'], @@ -662,7 +672,7 @@ class NvpPluginV2(addr_pair_db.AllowedAddressPairsMixin, ['0.0.0.0/31']) # Reset attachment self._update_router_port_attachment( - self.cluster, context, router_id, port_data, + self.cluster, context, nsx_router_id, port_data, lr_port['uuid'], "L3GatewayAttachment", self.cluster.default_l3_gw_service_uuid) @@ -676,9 +686,9 @@ class NvpPluginV2(addr_pair_db.AllowedAddressPairsMixin, err_msg=_("Unable to update logical router" "on NVP Platform")) LOG.debug(_("_nvp_delete_ext_gw_port completed on external network " - "%(ext_net_id)s, attached to router:%(router_id)s"), + "%(ext_net_id)s, attached to NSX router:%(router_id)s"), {'ext_net_id': port_data['network_id'], - 'router_id': router_id}) + 'router_id': nsx_router_id}) def _nvp_create_l2_gw_port(self, context, port_data): """Create a switch port, and attach it to a L2 gateway attachment.""" @@ -1038,8 +1048,10 @@ class NvpPluginV2(addr_pair_db.AllowedAddressPairsMixin, for port in router_iface_ports: try: if nvp_port_id: + nsx_router_id = nsx_utils.get_nsx_router_id( + context.session, self.cluster, port['device_id']) nvplib.delete_peer_router_lport(self.cluster, - port['device_id'], + nsx_router_id, nvp_switch_id, nvp_port_id) else: @@ -1381,11 +1393,11 @@ class NvpPluginV2(addr_pair_db.AllowedAddressPairsMixin, def _create_lrouter(self, context, router, nexthop): tenant_id = self._get_tenant_id_for_create(context, router) - name = router['name'] distributed = router.get('distributed') try: lrouter = nvplib.create_lrouter( - self.cluster, tenant_id, name, nexthop, + self.cluster, router['id'], + tenant_id, router['name'], nexthop, distributed=attr.is_attr_set(distributed) and distributed) except nvp_exc.NvpInvalidVersion: msg = _("Cannot create a distributed router with the NVP " @@ -1415,7 +1427,8 @@ class NvpPluginV2(addr_pair_db.AllowedAddressPairsMixin, nvplib.delete_lrouter(self.cluster, lrouter['uuid']) # Return user a 500 with an apter message raise nvp_exc.NvpPluginException( - err_msg=_("Unable to create router %s") % router['name']) + err_msg=(_("Unable to create router %s on NSX backend") % + router['id'])) lrouter['status'] = plugin_const.ACTIVE return lrouter @@ -1449,9 +1462,12 @@ class NvpPluginV2(addr_pair_db.AllowedAddressPairsMixin, if ext_net.subnets: ext_subnet = ext_net.subnets[0] nexthop = ext_subnet.gateway_ip + # NOTE(salv-orlando): Pre-generating uuid for Neutron + # router. This will be removed once the router create operation + # becomes an asynchronous task + neutron_router_id = str(uuid.uuid4()) + r['id'] = neutron_router_id lrouter = self._create_lrouter(context, r, nexthop) - # Use NVP identfier for Neutron resource - r['id'] = lrouter['uuid'] # Update 'distributed' with value returned from NVP # This will be useful for setting the value if the API request # did not specify any value for the 'distributed' attribute @@ -1463,13 +1479,19 @@ class NvpPluginV2(addr_pair_db.AllowedAddressPairsMixin, # Transaction nesting is needed to avoid foreign key violations # when processing the distributed router binding with context.session.begin(subtransactions=True): - router_db = l3_db.Router(id=lrouter['uuid'], + router_db = l3_db.Router(id=neutron_router_id, tenant_id=tenant_id, name=r['name'], admin_state_up=r['admin_state_up'], status=lrouter['status']) - self._process_nsx_router_create(context, router_db, r) context.session.add(router_db) + self._process_nsx_router_create(context, router_db, r) + # Ensure neutron router is moved into the transaction's buffer + context.session.flush() + # Add mapping between neutron and nsx identifiers + nicira_db.add_neutron_nsx_router_mapping( + context.session, router_db['id'], lrouter['uuid']) + if has_gw_info: # NOTE(salv-orlando): This operation has been moved out of the # database transaction since it performs several NVP queries, @@ -1490,17 +1512,20 @@ class NvpPluginV2(addr_pair_db.AllowedAddressPairsMixin, "gateway. Router:%s has been removed from " "DB and backend"), router_id) - router = self._make_router_dict(router_db) - return router + return self._make_router_dict(router_db) def _update_lrouter(self, context, router_id, name, nexthop, routes=None): + nsx_router_id = nsx_utils.get_nsx_router_id( + context.session, self.cluster, router_id) return nvplib.update_lrouter( - self.cluster, router_id, name, + self.cluster, nsx_router_id, name, nexthop, routes=routes) - def _update_lrouter_routes(self, router_id, routes): + def _update_lrouter_routes(self, context, router_id, routes): + nsx_router_id = nsx_utils.get_nsx_router_id( + context.session, self.cluster, router_id) nvplib.update_explicit_routes_lrouter( - self.cluster, router_id, routes) + self.cluster, nsx_router_id, routes) def update_router(self, context, router_id, router): # Either nexthop is updated or should be kept as it was before @@ -1561,10 +1586,12 @@ class NvpPluginV2(addr_pair_db.AllowedAddressPairsMixin, with excutils.save_and_reraise_exception(): # revert changes made to NVP self._update_lrouter_routes( - router_id, previous_routes) + context, router_id, previous_routes) - def _delete_lrouter(self, context, id): - nvplib.delete_lrouter(self.cluster, id) + def _delete_lrouter(self, context, router_id, nsx_router_id): + # The neutron router id (router_id) is ignored in this routine, + # but used in plugins deriving from this one + nvplib.delete_lrouter(self.cluster, nsx_router_id) def delete_router(self, context, router_id): with context.session.begin(subtransactions=True): @@ -1594,34 +1621,36 @@ class NvpPluginV2(addr_pair_db.AllowedAddressPairsMixin, if ports: raise l3.RouterInUse(router_id=router_id) + nsx_router_id = nsx_utils.get_nsx_router_id( + context.session, self.cluster, router_id) # It is safe to remove the router from the database, so remove it # from the backend try: - self._delete_lrouter(context, router_id) + self._delete_lrouter(context, router_id, nsx_router_id) except q_exc.NotFound: # This is not a fatal error, but needs to be logged LOG.warning(_("Logical router '%s' not found " - "on NVP Platform"), router_id) + "on NVP Platform"), nsx_router_id) except NvpApiClient.NvpApiException: raise nvp_exc.NvpPluginException( err_msg=(_("Unable to delete logical router '%s' " - "on NVP Platform") % router_id)) - - # Perform the actual delete on the Neutron DB + "on NVP Platform") % nsx_router_id)) + # Remove the NSX mapping first in order to ensure a mapping to + # a non-existent NSX router is not left in the DB in case of + # failure while removing the router from the neutron DB try: - super(NvpPluginV2, self).delete_router(context, router_id) - except Exception: - # NOTE(salv-orlando): Broad catching as the following action - # needs to be performed for every exception. - # Put the router in ERROR status - LOG.exception(_("Failure while removing router:%s from database. " - "The router will be put in ERROR status"), - router_id) - with context.session.begin(subtransactions=True): - router_db = self._get_router(context, router_id) - router_db['status'] = constants.NET_STATUS_ERROR + nicira_db.delete_neutron_nsx_router_mapping( + context.session, router_id) + except db_exc.DBError as d_exc: + # Do not make this error fatal + LOG.warn(_("Unable to remove NSX mapping for Neutron router " + "%(router_id)s because of the following exception:" + "%(d_exc)s"), {'router_id': router_id, + 'd_exc': str(d_exc)}) + # Perform the actual delete on the Neutron DB + super(NvpPluginV2, self).delete_router(context, router_id) - def _add_subnet_snat_rule(self, router, subnet): + def _add_subnet_snat_rule(self, context, router, subnet): gw_port = router.gw_port if gw_port and router.enable_snat: # There is a change gw_port might have multiple IPs @@ -1629,16 +1658,20 @@ class NvpPluginV2(addr_pair_db.AllowedAddressPairsMixin, if gw_port.get('fixed_ips'): snat_ip = gw_port['fixed_ips'][0]['ip_address'] cidr_prefix = int(subnet['cidr'].split('/')[1]) + nsx_router_id = nsx_utils.get_nsx_router_id( + context.session, self.cluster, router['id']) nvplib.create_lrouter_snat_rule( - self.cluster, router['id'], snat_ip, snat_ip, + self.cluster, nsx_router_id, snat_ip, snat_ip, order=NVP_EXTGW_NAT_RULES_ORDER - cidr_prefix, match_criteria={'source_ip_addresses': subnet['cidr']}) - def _delete_subnet_snat_rule(self, router, subnet): + def _delete_subnet_snat_rule(self, context, router, subnet): # Remove SNAT rule if external gateway is configured if router.gw_port: + nsx_router_id = nsx_utils.get_nsx_router_id( + context.session, self.cluster, router['id']) nvplib.delete_nat_rules_by_match( - self.cluster, router['id'], "SourceNatRule", + self.cluster, nsx_router_id, "SourceNatRule", max_num_expected=1, min_num_expected=1, source_ip_addresses=subnet['cidr']) @@ -1650,6 +1683,8 @@ class NvpPluginV2(addr_pair_db.AllowedAddressPairsMixin, context, router_id, interface_info) # router_iface_info will always have a subnet_id attribute subnet_id = router_iface_info['subnet_id'] + nsx_router_id = nsx_utils.get_nsx_router_id( + context.session, self.cluster, router_id) if port_id: port_data = self._get_port(context, port_id) nvp_switch_id, nvp_port_id = nsx_utils.get_nsx_switch_and_port_id( @@ -1659,15 +1694,15 @@ class NvpPluginV2(addr_pair_db.AllowedAddressPairsMixin, nvp_port_id, "NoAttachment") # Create logical router port and plug patch attachment self._create_and_attach_router_port( - self.cluster, context, router_id, port_data, + self.cluster, context, nsx_router_id, port_data, "PatchAttachment", nvp_port_id, subnet_ids=[subnet_id]) subnet = self._get_subnet(context, subnet_id) # If there is an external gateway we need to configure the SNAT rule. # Fetch router from DB router = self._get_router(context, router_id) - self._add_subnet_snat_rule(router, subnet) + self._add_subnet_snat_rule(context, router, subnet) nvplib.create_lrouter_nosnat_rule( - self.cluster, router_id, + self.cluster, nsx_router_id, order=NVP_NOSNAT_RULES_ORDER, match_criteria={'destination_ip_addresses': subnet['cidr']}) @@ -1728,11 +1763,13 @@ class NvpPluginV2(addr_pair_db.AllowedAddressPairsMixin, # If router is enabled_snat = False there are no snat rules to # delete. if router.enable_snat: - self._delete_subnet_snat_rule(router, subnet) + self._delete_subnet_snat_rule(context, router, subnet) # Relax the minimum expected number as the nosnat rules # do not exist in 2.x deployments + nsx_router_id = nsx_utils.get_nsx_router_id( + context.session, self.cluster, router_id) nvplib.delete_nat_rules_by_match( - self.cluster, router_id, "NoSourceNatRule", + self.cluster, nsx_router_id, "NoSourceNatRule", max_num_expected=1, min_num_expected=0, destination_ip_addresses=subnet['cidr']) except NvpApiClient.ResourceNotFound: @@ -1746,24 +1783,27 @@ class NvpPluginV2(addr_pair_db.AllowedAddressPairsMixin, return info def _retrieve_and_delete_nat_rules(self, context, floating_ip_address, - internal_ip, router_id, + internal_ip, nsx_router_id, min_num_rules_expected=0): + """Finds and removes NAT rules from a NSX router.""" + # NOTE(salv-orlando): The context parameter is ignored in this method + # but used by derived classes try: # Remove DNAT rule for the floating IP nvplib.delete_nat_rules_by_match( - self.cluster, router_id, "DestinationNatRule", + self.cluster, nsx_router_id, "DestinationNatRule", max_num_expected=1, min_num_expected=min_num_rules_expected, destination_ip_addresses=floating_ip_address) # Remove SNAT rules for the floating IP nvplib.delete_nat_rules_by_match( - self.cluster, router_id, "SourceNatRule", + self.cluster, nsx_router_id, "SourceNatRule", max_num_expected=1, min_num_expected=min_num_rules_expected, source_ip_addresses=internal_ip) nvplib.delete_nat_rules_by_match( - self.cluster, router_id, "SourceNatRule", + self.cluster, nsx_router_id, "SourceNatRule", max_num_expected=1, min_num_expected=min_num_rules_expected, destination_ip_addresses=internal_ip) @@ -1782,14 +1822,16 @@ class NvpPluginV2(addr_pair_db.AllowedAddressPairsMixin, # Remove floating IP address from logical router port # Fetch logical port of router's external gateway router_id = fip_db.router_id + nsx_router_id = nsx_utils.get_nsx_router_id( + context.session, self.cluster, router_id) nvp_gw_port_id = nvplib.find_router_gw_port( - context, self.cluster, router_id)['uuid'] + context, self.cluster, nsx_router_id)['uuid'] ext_neutron_port_db = self._get_port(context.elevated(), fip_db.floating_port_id) nvp_floating_ips = self._build_ip_address_list( context.elevated(), ext_neutron_port_db['fixed_ips']) nvplib.update_lrouter_port_ips(self.cluster, - router_id, + nsx_router_id, nvp_gw_port_id, ips_to_add=[], ips_to_remove=nvp_floating_ips) @@ -1834,10 +1876,10 @@ class NvpPluginV2(addr_pair_db.AllowedAddressPairsMixin, floating_ip = floatingip_db['floating_ip_address'] # If there's no association router_id will be None if router_id: - self._retrieve_and_delete_nat_rules(context, - floating_ip, - internal_ip, - router_id) + nsx_router_id = nsx_utils.get_nsx_router_id( + context.session, self.cluster, router_id) + self._retrieve_and_delete_nat_rules( + context, floating_ip, internal_ip, nsx_router_id) # Fetch logical port of router's external gateway # Fetch logical port of router's external gateway nvp_floating_ips = self._build_ip_address_list( @@ -1845,6 +1887,8 @@ class NvpPluginV2(addr_pair_db.AllowedAddressPairsMixin, floating_ip = floatingip_db['floating_ip_address'] # Retrieve and delete existing NAT rules, if any if old_router_id: + nsx_old_router_id = nsx_utils.get_nsx_router_id( + context.session, self.cluster, old_router_id) # Retrieve the current internal ip _p, _s, old_internal_ip = self._internal_fip_assoc_data( context, {'id': floatingip_db.id, @@ -1852,22 +1896,22 @@ class NvpPluginV2(addr_pair_db.AllowedAddressPairsMixin, 'fixed_ip_address': floatingip_db.fixed_ip_address, 'tenant_id': floatingip_db.tenant_id}) nvp_gw_port_id = nvplib.find_router_gw_port( - context, self.cluster, old_router_id)['uuid'] + context, self.cluster, nsx_old_router_id)['uuid'] self._retrieve_and_delete_nat_rules( - context, floating_ip, old_internal_ip, old_router_id) + context, floating_ip, old_internal_ip, nsx_old_router_id) nvplib.update_lrouter_port_ips( - self.cluster, old_router_id, nvp_gw_port_id, + self.cluster, nsx_old_router_id, nvp_gw_port_id, ips_to_add=[], ips_to_remove=nvp_floating_ips) if router_id: nvp_gw_port_id = nvplib.find_router_gw_port( - context, self.cluster, router_id)['uuid'] + context, self.cluster, nsx_router_id)['uuid'] # Re-create NAT rules only if a port id is specified if fip.get('port_id'): try: # Setup DNAT rules for the floating IP nvplib.create_lrouter_dnat_rule( - self.cluster, router_id, internal_ip, + self.cluster, nsx_router_id, internal_ip, order=NVP_FLOATINGIP_NAT_RULES_ORDER, match_criteria={'destination_ip_addresses': floating_ip}) @@ -1885,7 +1929,7 @@ class NvpPluginV2(addr_pair_db.AllowedAddressPairsMixin, context, internal_port['fixed_ips'], subnet_ids=subnet_ids)[0] nvplib.create_lrouter_snat_rule( - self.cluster, router_id, floating_ip, floating_ip, + self.cluster, nsx_router_id, floating_ip, floating_ip, order=NVP_NOSNAT_RULES_ORDER - 1, match_criteria={'source_ip_addresses': internal_subnet_cidr, @@ -1894,13 +1938,13 @@ class NvpPluginV2(addr_pair_db.AllowedAddressPairsMixin, # setup snat rule such that src ip of a IP packet when # using floating is the floating ip itself. nvplib.create_lrouter_snat_rule( - self.cluster, router_id, floating_ip, floating_ip, + self.cluster, nsx_router_id, floating_ip, floating_ip, order=NVP_FLOATINGIP_NAT_RULES_ORDER, match_criteria={'source_ip_addresses': internal_ip}) # Add Floating IP address to router_port nvplib.update_lrouter_port_ips(self.cluster, - router_id, + nsx_router_id, nvp_gw_port_id, ips_to_add=nvp_floating_ips, ips_to_remove=[]) @@ -1922,10 +1966,12 @@ class NvpPluginV2(addr_pair_db.AllowedAddressPairsMixin, fip_db = self._get_floatingip(context, id) # Check whether the floating ip is associated or not if fip_db.fixed_port_id: + nsx_router_id = nsx_utils.get_nsx_router_id( + context.session, self.cluster, fip_db.router_id) self._retrieve_and_delete_nat_rules(context, fip_db.floating_ip_address, fip_db.fixed_ip_address, - fip_db.router_id, + nsx_router_id, min_num_rules_expected=1) # Remove floating IP address from logical router port self._remove_floatingip_address(context, fip_db) @@ -1935,10 +1981,12 @@ class NvpPluginV2(addr_pair_db.AllowedAddressPairsMixin, try: fip_qry = context.session.query(l3_db.FloatingIP) fip_db = fip_qry.filter_by(fixed_port_id=port_id).one() + nsx_router_id = nsx_utils.get_nsx_router_id( + context.session, self.cluster, fip_db.router_id) self._retrieve_and_delete_nat_rules(context, fip_db.floating_ip_address, fip_db.fixed_ip_address, - fip_db.router_id, + nsx_router_id, min_num_rules_expected=1) self._remove_floatingip_address(context, fip_db) except sa_exc.NoResultFound: diff --git a/neutron/plugins/nicira/NeutronServicePlugin.py b/neutron/plugins/nicira/NeutronServicePlugin.py index cfb87f06f..348a381f0 100644 --- a/neutron/plugins/nicira/NeutronServicePlugin.py +++ b/neutron/plugins/nicira/NeutronServicePlugin.py @@ -1,4 +1,4 @@ - # vim: tabstop=4 shiftwidth=4 softtabstop=4 +# vim: tabstop=4 shiftwidth=4 softtabstop=4 # Copyright 2013 VMware, Inc. # All Rights Reserved @@ -355,17 +355,17 @@ class NvpAdvancedPlugin(sr_db.ServiceRouter_mixin, self._update_nat_rules(context, router) - def _add_subnet_snat_rule(self, router, subnet): + def _add_subnet_snat_rule(self, context, router, subnet): # NOP for service router if not self._is_advanced_service_router(router=router): super(NvpAdvancedPlugin, self)._add_subnet_snat_rule( - router, subnet) + context, router, subnet) - def _delete_subnet_snat_rule(self, router, subnet): + def _delete_subnet_snat_rule(self, context, router, subnet): # NOP for service router if not self._is_advanced_service_router(router=router): super(NvpAdvancedPlugin, self)._delete_subnet_snat_rule( - router, subnet) + context, router, subnet) def _remove_floatingip_address(self, context, fip_db): # NOP for service router @@ -374,15 +374,17 @@ class NvpAdvancedPlugin(sr_db.ServiceRouter_mixin, super(NvpAdvancedPlugin, self)._remove_floatingip_address( context, fip_db) - def _create_advanced_service_router(self, context, name, lrouter, lswitch): + def _create_advanced_service_router(self, context, neutron_router_id, + name, lrouter, lswitch): # store binding binding = vcns_db.add_vcns_router_binding( - context.session, lrouter['uuid'], None, lswitch['uuid'], + context.session, neutron_router_id, None, lswitch['uuid'], service_constants.PENDING_CREATE) # deploy edge jobdata = { + 'neutron_router_id': neutron_router_id, 'lrouter': lrouter, 'lswitch': lswitch, 'context': context @@ -477,7 +479,7 @@ class NvpAdvancedPlugin(sr_db.ServiceRouter_mixin, try: self._create_advanced_service_router( - context, name, lrouter, lswitch) + context, router['id'], name, lrouter, lswitch) except Exception: msg = (_("Unable to create advance service router for %s") % name) LOG.exception(msg) @@ -488,13 +490,15 @@ class NvpAdvancedPlugin(sr_db.ServiceRouter_mixin, lrouter['status'] = service_constants.PENDING_CREATE return lrouter - def _delete_lrouter(self, context, id): - binding = vcns_db.get_vcns_router_binding(context.session, id) + def _delete_lrouter(self, context, router_id, nsx_router_id): + binding = vcns_db.get_vcns_router_binding(context.session, router_id) if not binding: - super(NvpAdvancedPlugin, self)._delete_lrouter(context, id) + super(NvpAdvancedPlugin, self)._delete_lrouter( + context, router_id, nsx_router_id) else: vcns_db.update_vcns_router_binding( - context.session, id, status=service_constants.PENDING_DELETE) + context.session, router_id, + status=service_constants.PENDING_DELETE) lswitch_id = binding['lswitch_id'] edge_id = binding['edge_id'] @@ -509,13 +513,13 @@ class NvpAdvancedPlugin(sr_db.ServiceRouter_mixin, jobdata = { 'context': context } - self.vcns_driver.delete_edge(id, edge_id, jobdata=jobdata) + self.vcns_driver.delete_edge(router_id, edge_id, jobdata=jobdata) - # delete LR - nvplib.delete_lrouter(self.cluster, id) + # delete NSX logical router + nvplib.delete_lrouter(self.cluster, nsx_router_id) if id in self._router_type: - del self._router_type[id] + del self._router_type[router_id] def _update_lrouter(self, context, router_id, name, nexthop, routes=None): if not self._is_advanced_service_router(context, router_id): @@ -572,7 +576,6 @@ class NvpAdvancedPlugin(sr_db.ServiceRouter_mixin, def _get_vse_status(self, context, id): binding = vcns_db.get_vcns_router_binding(context.session, id) - edge_status_level = self.vcns_driver.get_edge_status( binding['edge_id']) edge_db_status_level = ROUTER_STATUS_LEVEL[binding.status] @@ -1519,20 +1522,20 @@ class VcnsCallbacks(object): def edge_deploy_started(self, task): """callback when deployment task started.""" jobdata = task.userdata['jobdata'] - lrouter = jobdata['lrouter'] context = jobdata['context'] edge_id = task.userdata.get('edge_id') + neutron_router_id = jobdata['neutron_router_id'] name = task.userdata['router_name'] if edge_id: LOG.debug(_("Start deploying %(edge_id)s for router %(name)s"), { 'edge_id': edge_id, 'name': name}) vcns_db.update_vcns_router_binding( - context.session, lrouter['uuid'], edge_id=edge_id) + context.session, neutron_router_id, edge_id=edge_id) else: LOG.debug(_("Failed to deploy Edge for router %s"), name) vcns_db.update_vcns_router_binding( - context.session, lrouter['uuid'], + context.session, neutron_router_id, status=service_constants.ERROR) def edge_deploy_result(self, task): @@ -1541,9 +1544,11 @@ class VcnsCallbacks(object): lrouter = jobdata['lrouter'] context = jobdata['context'] name = task.userdata['router_name'] + neutron_router_id = jobdata['neutron_router_id'] router_db = None try: - router_db = self.plugin._get_router(context, lrouter['uuid']) + router_db = self.plugin._get_router( + context, neutron_router_id) except l3.RouterNotFound: # Router might have been deleted before deploy finished LOG.exception(_("Router %s not found"), lrouter['uuid']) @@ -1558,18 +1563,18 @@ class VcnsCallbacks(object): router_db['status'] = service_constants.ACTIVE binding = vcns_db.get_vcns_router_binding( - context.session, lrouter['uuid']) + context.session, neutron_router_id) # only update status to active if its status is pending create if binding['status'] == service_constants.PENDING_CREATE: vcns_db.update_vcns_router_binding( - context.session, lrouter['uuid'], + context.session, neutron_router_id, status=service_constants.ACTIVE) else: LOG.debug(_("Failed to deploy Edge for router %s"), name) if router_db: router_db['status'] = service_constants.ERROR vcns_db.update_vcns_router_binding( - context.session, lrouter['uuid'], + context.session, neutron_router_id, status=service_constants.ERROR) def edge_delete_result(self, task): diff --git a/neutron/plugins/nicira/common/nsx_utils.py b/neutron/plugins/nicira/common/nsx_utils.py index 7965fe6d9..56b149854 100644 --- a/neutron/plugins/nicira/common/nsx_utils.py +++ b/neutron/plugins/nicira/common/nsx_utils.py @@ -111,17 +111,16 @@ def get_nsx_switch_and_port_id(session, cluster, neutron_port_id): nvp_port = nvp_ports[0] nvp_switch_id = (nvp_port['_relations'] ['LogicalSwitchConfig']['uuid']) - with session.begin(subtransactions=True): - if nvp_port_id: - # Mapping already exists. Delete before recreating - nicira_db.delete_neutron_nsx_port_mapping( - session, neutron_port_id) - else: - nvp_port_id = nvp_port['uuid'] - # (re)Create DB mapping - nicira_db.add_neutron_nsx_port_mapping( - session, neutron_port_id, - nvp_switch_id, nvp_port_id) + if nvp_port_id: + # Mapping already exists. Delete before recreating + nicira_db.delete_neutron_nsx_port_mapping( + session, neutron_port_id) + else: + nvp_port_id = nvp_port['uuid'] + # (re)Create DB mapping + nicira_db.add_neutron_nsx_port_mapping( + session, neutron_port_id, + nvp_switch_id, nvp_port_id) return nvp_switch_id, nvp_port_id @@ -142,3 +141,37 @@ def create_nsx_cluster(cluster_opts, concurrent_connections, nsx_gen_timeout): concurrent_connections=concurrent_connections, nvp_gen_timeout=nsx_gen_timeout) return cluster + + +def get_nsx_router_id(session, cluster, neutron_router_id): + """Return the NSX router uuid for a given neutron router. + + First, look up the Neutron database. If not found, execute + a query on NSX platform as the mapping might be missing. + """ + nsx_router_id = nicira_db.get_nsx_router_id( + session, neutron_router_id) + if not nsx_router_id: + # Find logical router from backend. + # This is a rather expensive query, but it won't be executed + # more than once for each router in Neutron's lifetime + nsx_routers = nvplib.query_lrouters( + cluster, '*', + filters={'tag': neutron_router_id, + 'tag_scope': 'q_router_id'}) + # Only one result expected + # NOTE(salv-orlando): Not handling the case where more than one + # port is found with the same neutron port tag + if not nsx_routers: + LOG.warn(_("Unable to find NSX router for Neutron router %s"), + neutron_router_id) + return + nsx_router = nsx_routers[0] + nsx_router_id = nsx_router['uuid'] + with session.begin(subtransactions=True): + # Create DB mapping + nicira_db.add_neutron_nsx_router_mapping( + session, + neutron_router_id, + nsx_router_id) + return nsx_router_id diff --git a/neutron/plugins/nicira/common/sync.py b/neutron/plugins/nicira/common/sync.py index c6fb03880..19543a781 100644 --- a/neutron/plugins/nicira/common/sync.py +++ b/neutron/plugins/nicira/common/sync.py @@ -317,8 +317,10 @@ class NvpSynchronizer(): # Try to get router from nvp try: # This query will return the logical router status too + nsx_router_id = nsx_utils.get_nsx_router_id( + context.session, self._cluster, neutron_router_data['id']) lrouter = nvplib.get_lrouter( - self._cluster, neutron_router_data['id']) + self._cluster, nsx_router_id) except exceptions.NotFound: # NOTE(salv-orlando): We should be catching # NvpApiClient.ResourceNotFound here diff --git a/neutron/plugins/nicira/dbexts/nicira_db.py b/neutron/plugins/nicira/dbexts/nicira_db.py index a682b18f5..4e04c3173 100644 --- a/neutron/plugins/nicira/dbexts/nicira_db.py +++ b/neutron/plugins/nicira/dbexts/nicira_db.py @@ -82,6 +82,14 @@ def add_neutron_nsx_port_mapping(session, neutron_id, return mapping +def add_neutron_nsx_router_mapping(session, neutron_id, nsx_router_id): + with session.begin(subtransactions=True): + mapping = nicira_models.NeutronNsxRouterMapping( + neutron_id=neutron_id, nsx_id=nsx_router_id) + session.add(mapping) + return mapping + + def get_nsx_switch_ids(session, neutron_id): # This function returns a list of NSX switch identifiers because of # the possibility of chained logical switches @@ -102,9 +110,28 @@ def get_nsx_switch_and_port_id(session, neutron_id): return None, None +def get_nsx_router_id(session, neutron_id): + try: + mapping = (session.query(nicira_models.NeutronNsxRouterMapping). + filter_by(neutron_id=neutron_id).one()) + return mapping['nsx_id'] + except exc.NoResultFound: + LOG.debug(_("NSX identifiers for neutron router %s not yet " + "stored in Neutron DB"), neutron_id) + + +def _delete_by_neutron_id(session, model, neutron_id): + return session.query(model).filter_by(neutron_id=neutron_id).delete() + + def delete_neutron_nsx_port_mapping(session, neutron_id): - return (session.query(nicira_models.NeutronNsxPortMapping). - filter_by(neutron_id=neutron_id).delete()) + return _delete_by_neutron_id( + session, nicira_models.NeutronNsxPortMapping, neutron_id) + + +def delete_neutron_nsx_router_mapping(session, neutron_id): + return _delete_by_neutron_id( + session, nicira_models.NeutronNsxRouterMapping, neutron_id) def unset_default_network_gateways(session): diff --git a/neutron/plugins/nicira/dbexts/nicira_models.py b/neutron/plugins/nicira/dbexts/nicira_models.py index 72ae5b02f..9d9a0cb32 100644 --- a/neutron/plugins/nicira/dbexts/nicira_models.py +++ b/neutron/plugins/nicira/dbexts/nicira_models.py @@ -86,6 +86,15 @@ class NeutronNsxPortMapping(model_base.BASEV2): self.nsx_port_id = nsx_port_id +class NeutronNsxRouterMapping(model_base.BASEV2): + """Maps neutron router identifiers to NSX identifiers.""" + __tablename__ = 'neutron_nsx_router_mappings' + neutron_id = Column(String(36), + ForeignKey('routers.id', ondelete='CASCADE'), + primary_key=True) + nsx_id = Column(String(36)) + + class MultiProviderNetworks(model_base.BASEV2): """Networks that were provision through multiprovider extension.""" diff --git a/neutron/plugins/nicira/nvplib.py b/neutron/plugins/nicira/nvplib.py index 6447430ab..d151ebe83 100644 --- a/neutron/plugins/nicira/nvplib.py +++ b/neutron/plugins/nicira/nvplib.py @@ -317,11 +317,12 @@ def create_l2_gw_service(cluster, tenant_id, display_name, devices): json.dumps(gwservice_obj), cluster=cluster) -def _prepare_lrouter_body(name, tenant_id, router_type, +def _prepare_lrouter_body(name, neutron_router_id, tenant_id, router_type, distributed=None, **kwargs): body = { "display_name": utils.check_and_truncate(name), "tags": [{"tag": tenant_id, "scope": "os_tid"}, + {"tag": neutron_router_id, "scope": "q_router_id"}, {"tag": NEUTRON_VERSION, "scope": "quantum"}], "routing_config": { "type": router_type @@ -336,9 +337,8 @@ def _prepare_lrouter_body(name, tenant_id, router_type, return body -def _create_implicit_routing_lrouter(cluster, tenant_id, - display_name, nexthop, - distributed=None): +def _create_implicit_routing_lrouter(cluster, neutron_router_id, tenant_id, + display_name, nexthop, distributed=None): implicit_routing_config = { "default_route_next_hop": { "gateway_ip_address": nexthop, @@ -346,7 +346,7 @@ def _create_implicit_routing_lrouter(cluster, tenant_id, }, } lrouter_obj = _prepare_lrouter_body( - display_name, tenant_id, + display_name, neutron_router_id, tenant_id, "SingleDefaultRouteImplicitRoutingConfig", distributed=distributed, **implicit_routing_config) @@ -354,7 +354,7 @@ def _create_implicit_routing_lrouter(cluster, tenant_id, json.dumps(lrouter_obj), cluster=cluster) -def create_implicit_routing_lrouter(cluster, tenant_id, +def create_implicit_routing_lrouter(cluster, neutron_router_id, tenant_id, display_name, nexthop): """Create a NVP logical router on the specified cluster. @@ -367,11 +367,12 @@ def create_implicit_routing_lrouter(cluster, tenant_id, with the NVP controller """ return _create_implicit_routing_lrouter( - cluster, tenant_id, display_name, nexthop) + cluster, neutron_router_id, tenant_id, display_name, nexthop) def create_implicit_routing_lrouter_with_distribution( - cluster, tenant_id, display_name, nexthop, distributed=None): + cluster, neutron_router_id, tenant_id, + display_name, nexthop, distributed=None): """Create a NVP logical router on the specified cluster. This function also allows for creating distributed lrouters @@ -385,14 +386,16 @@ def create_implicit_routing_lrouter_with_distribution( with the NVP controller """ return _create_implicit_routing_lrouter( - cluster, tenant_id, display_name, nexthop, distributed) + cluster, neutron_router_id, tenant_id, + display_name, nexthop, distributed) -def create_explicit_routing_lrouter(cluster, tenant_id, +def create_explicit_routing_lrouter(cluster, neutron_router_id, tenant_id, display_name, nexthop, distributed=None): lrouter_obj = _prepare_lrouter_body( - display_name, tenant_id, "RoutingTableRoutingConfig", + display_name, neutron_router_id, + tenant_id, "RoutingTableRoutingConfig", distributed=distributed) router = do_request(HTTP_POST, _build_uri_path(LROUTER_RESOURCE), json.dumps(lrouter_obj), cluster=cluster) @@ -437,7 +440,17 @@ def get_l2_gw_service(cluster, gateway_id): cluster=cluster) +def query_lrouters(cluster, fields=None, filters=None): + return get_all_query_pages( + _build_uri_path(LROUTER_RESOURCE, + fields=fields, + relations='LogicalRouterStatus', + filters=filters), + cluster) + + def get_lrouters(cluster, tenant_id, fields=None, filters=None): + # FIXME(salv-orlando): Fields parameter is ignored in this routine actual_filters = {} if filters: actual_filters.update(filters) @@ -445,12 +458,7 @@ def get_lrouters(cluster, tenant_id, fields=None, filters=None): actual_filters['tag'] = tenant_id actual_filters['tag_scope'] = 'os_tid' lrouter_fields = "uuid,display_name,fabric_status,tags" - return get_all_query_pages( - _build_uri_path(LROUTER_RESOURCE, - fields=lrouter_fields, - relations='LogicalRouterStatus', - filters=actual_filters), - cluster) + return query_lrouters(cluster, lrouter_fields, actual_filters) def get_l2_gw_services(cluster, tenant_id=None, diff --git a/neutron/tests/unit/nicira/test_nicira_plugin.py b/neutron/tests/unit/nicira/test_nicira_plugin.py index a640fdf95..bdb848dfa 100644 --- a/neutron/tests/unit/nicira/test_nicira_plugin.py +++ b/neutron/tests/unit/nicira/test_nicira_plugin.py @@ -570,8 +570,11 @@ class TestNiciraL3NatTestCase(NiciraL3NatTest, """Verify data on fake NVP API client in order to validate plugin did set them properly """ + # First find the NSX router ID + ctx = context.get_admin_context() + nsx_router_id = nicira_db.get_nsx_router_id(ctx.session, router_id) ports = [port for port in self.fc._fake_lrouter_lport_dict.values() - if (port['lr_uuid'] == router_id and + if (port['lr_uuid'] == nsx_router_id and port['att_type'] == "L3GatewayAttachment")] self.assertEqual(len(ports), 1) self.assertEqual(ports[0]['attachment_gwsvc_uuid'], l3_gw_uuid) diff --git a/neutron/tests/unit/nicira/test_nsx_utils.py b/neutron/tests/unit/nicira/test_nsx_utils.py index ea085cfad..9aad462da 100644 --- a/neutron/tests/unit/nicira/test_nsx_utils.py +++ b/neutron/tests/unit/nicira/test_nsx_utils.py @@ -26,18 +26,21 @@ from neutron.tests.unit.nicira import nicira_method class NsxUtilsTestCase(base.BaseTestCase): - def _mock_db_calls(self, get_switch_port_id_ret_value): + def setUp(self): + self.addCleanup(mock.patch.stopall) + super(NsxUtilsTestCase, self).setUp() + + def _mock_port_mapping_db_calls(self, ret_value): # Mock relevant db calls # This will allow for avoiding setting up the plugin # for creating db entries mock.patch(nicira_method('get_nsx_switch_and_port_id', module_name='dbexts.nicira_db'), - return_value=get_switch_port_id_ret_value).start() + return_value=ret_value).start() mock.patch(nicira_method('add_neutron_nsx_port_mapping', module_name='dbexts.nicira_db')).start() mock.patch(nicira_method('delete_neutron_nsx_port_mapping', module_name='dbexts.nicira_db')).start() - self.addCleanup(mock.patch.stopall) def _mock_network_mapping_db_calls(self, ret_value): # Mock relevant db calls @@ -48,7 +51,16 @@ class NsxUtilsTestCase(base.BaseTestCase): return_value=ret_value).start() mock.patch(nicira_method('add_neutron_nsx_network_mapping', module_name='dbexts.nicira_db')).start() - self.addCleanup(mock.patch.stopall) + + def _mock_router_mapping_db_calls(self, ret_value): + # Mock relevant db calls + # This will allow for avoiding setting up the plugin + # for creating db entries + mock.patch(nicira_method('get_nsx_router_id', + module_name='dbexts.nicira_db'), + return_value=ret_value).start() + mock.patch(nicira_method('add_neutron_nsx_router_mapping', + module_name='dbexts.nicira_db')).start() def _verify_get_nsx_switch_and_port_id(self, exp_ls_uuid, exp_lp_uuid): # The nvplib and db calls are mocked, therefore the cluster @@ -68,13 +80,19 @@ class NsxUtilsTestCase(base.BaseTestCase): exp_ls_uuids.remove(ls_uuid) self.assertFalse(exp_ls_uuids) + def _verify_get_nsx_router_id(self, exp_lr_uuid): + # The nvplib and db calls are mocked, therefore the cluster + # and the neutron_router_id parameters can be set to None + lr_uuid = nsx_utils.get_nsx_router_id(db_api.get_session(), None, None) + self.assertEqual(exp_lr_uuid, lr_uuid) + def test_get_nsx_switch_and_port_id_from_db_mappings(self): # This test is representative of the 'standard' case in which both the # switch and the port mappings were stored in the neutron db exp_ls_uuid = uuidutils.generate_uuid() exp_lp_uuid = uuidutils.generate_uuid() ret_value = exp_ls_uuid, exp_lp_uuid - self._mock_db_calls(ret_value) + self._mock_port_mapping_db_calls(ret_value) self._verify_get_nsx_switch_and_port_id(exp_ls_uuid, exp_lp_uuid) def test_get_nsx_switch_and_port_id_only_port_db_mapping(self): @@ -83,7 +101,7 @@ class NsxUtilsTestCase(base.BaseTestCase): exp_ls_uuid = uuidutils.generate_uuid() exp_lp_uuid = uuidutils.generate_uuid() ret_value = None, exp_lp_uuid - self._mock_db_calls(ret_value) + self._mock_port_mapping_db_calls(ret_value) with mock.patch(nicira_method('query_lswitch_lports'), return_value=[{'uuid': exp_lp_uuid, '_relations': { @@ -98,7 +116,7 @@ class NsxUtilsTestCase(base.BaseTestCase): exp_ls_uuid = uuidutils.generate_uuid() exp_lp_uuid = uuidutils.generate_uuid() ret_value = None, None - self._mock_db_calls(ret_value) + self._mock_port_mapping_db_calls(ret_value) with mock.patch(nicira_method('query_lswitch_lports'), return_value=[{'uuid': exp_lp_uuid, '_relations': { @@ -111,7 +129,7 @@ class NsxUtilsTestCase(base.BaseTestCase): # This test verifies that the function return (None, None) if the # mappings are not found both in the db and the backend ret_value = None, None - self._mock_db_calls(ret_value) + self._mock_port_mapping_db_calls(ret_value) with mock.patch(nicira_method('query_lswitch_lports'), return_value=[]): self._verify_get_nsx_switch_and_port_id(None, None) @@ -140,3 +158,27 @@ class NsxUtilsTestCase(base.BaseTestCase): with mock.patch(nicira_method('get_lswitches'), return_value=[]): self._verify_get_nsx_switch_ids(None) + + def test_get_nsx_router_id_from_db_mappings(self): + # This test is representative of the 'standard' case in which the + # router mapping was stored in the neutron db + exp_lr_uuid = uuidutils.generate_uuid() + self._mock_router_mapping_db_calls(exp_lr_uuid) + self._verify_get_nsx_router_id(exp_lr_uuid) + + def test_get_nsx_router_id_no_db_mapping(self): + # This test is representative of the case where db mappings where not + # found for a given port identifier + exp_lr_uuid = uuidutils.generate_uuid() + self._mock_router_mapping_db_calls(None) + with mock.patch(nicira_method('query_lrouters'), + return_value=[{'uuid': exp_lr_uuid}]): + self._verify_get_nsx_router_id(exp_lr_uuid) + + def test_get_nsx_router_id_no_mapping_returns_None(self): + # This test verifies that the function returns None if the mapping + # are not found both in the db and in the backend + self._mock_router_mapping_db_calls(None) + with mock.patch(nicira_method('query_lrouters'), + return_value=[]): + self._verify_get_nsx_router_id(None) diff --git a/neutron/tests/unit/nicira/test_nvp_sync.py b/neutron/tests/unit/nicira/test_nvp_sync.py index 65d7228b4..7e4f5a4e6 100644 --- a/neutron/tests/unit/nicira/test_nvp_sync.py +++ b/neutron/tests/unit/nicira/test_nvp_sync.py @@ -385,7 +385,9 @@ class NvpSyncTestCase(base.BaseTestCase): lp_uuid = self.fc._fake_lswitch_lport_dict.keys()[0] neutron_port_id = self._get_tag_dict( self.fc._fake_lswitch_lport_dict[lp_uuid]['tags'])['q_port_id'] - neutron_rtr_id = lr_uuid = self.fc._fake_lrouter_dict.keys()[0] + lr_uuid = self.fc._fake_lrouter_dict.keys()[0] + neutron_rtr_id = self._get_tag_dict( + self.fc._fake_lrouter_dict[lr_uuid]['tags'])['q_router_id'] if action_callback: action_callback(ls_uuid, lp_uuid, lr_uuid) # Make chunk big enough to read everything @@ -605,7 +607,9 @@ class NvpSyncTestCase(base.BaseTestCase): ctx = context.get_admin_context() with self._populate_data(ctx): # Put a router down to verify synchronization - q_rtr_id = lr_uuid = self.fc._fake_lrouter_dict.keys()[0] + lr_uuid = self.fc._fake_lrouter_dict.keys()[0] + q_rtr_id = self._get_tag_dict( + self.fc._fake_lrouter_dict[lr_uuid]['tags'])['q_router_id'] self.fc._fake_lrouter_dict[lr_uuid]['status'] = 'false' q_rtr_data = self._plugin._get_router(ctx, q_rtr_id) self._plugin._synchronizer.synchronize_router(ctx, q_rtr_data) @@ -623,7 +627,9 @@ class NvpSyncTestCase(base.BaseTestCase): ctx = context.get_admin_context() with self._populate_data(ctx): # Put a router down to verify punctual synchronization - q_rtr_id = lr_uuid = self.fc._fake_lrouter_dict.keys()[0] + lr_uuid = self.fc._fake_lrouter_dict.keys()[0] + q_rtr_id = self._get_tag_dict( + self.fc._fake_lrouter_dict[lr_uuid]['tags'])['q_router_id'] self.fc._fake_lrouter_dict[lr_uuid]['status'] = 'false' q_rtr_data = self._plugin.get_router(ctx, q_rtr_id) self.assertEqual(constants.NET_STATUS_DOWN, q_rtr_data['status']) diff --git a/neutron/tests/unit/nicira/test_nvplib.py b/neutron/tests/unit/nicira/test_nvplib.py index 0b005a69d..01e07f71a 100644 --- a/neutron/tests/unit/nicira/test_nvplib.py +++ b/neutron/tests/unit/nicira/test_nvplib.py @@ -22,6 +22,7 @@ import mock from neutron.common import constants from neutron.common import exceptions +from neutron.openstack.common import uuidutils from neutron.plugins.nicira.common import config # noqa from neutron.plugins.nicira.common import exceptions as nvp_exc from neutron.plugins.nicira.common import utils @@ -81,6 +82,7 @@ class TestNvplibNatRules(NvplibTestCase): new=lambda: NvpApiClient.NVPVersion(version)): tenant_id = 'pippo' lrouter = nvplib.create_lrouter(self.fake_cluster, + uuidutils.generate_uuid(), tenant_id, 'fake_router', '192.168.0.1') @@ -166,6 +168,7 @@ class NvplibNegativeTests(base.BaseTestCase): self.assertRaises(nvplib.NvpApiClient.NvpApiException, nvplib.create_lrouter, self.fake_cluster, + uuidutils.generate_uuid(), 'pluto', 'fake_router', 'my_hop') @@ -439,12 +442,14 @@ class TestNvplibExplicitLRouters(NvplibTestCase): def test_prepare_body_with_implicit_routing_config(self): router_name = 'fake_router_name' tenant_id = 'fake_tenant_id' + neutron_router_id = 'pipita_higuain' router_type = 'SingleDefaultRouteImplicitRoutingConfig' route_config = { 'default_route_next_hop': {'gateway_ip_address': 'fake_address', 'type': 'RouterNextHop'}, } - body = nvplib._prepare_lrouter_body(router_name, tenant_id, - router_type, **route_config) + body = nvplib._prepare_lrouter_body(router_name, neutron_router_id, + tenant_id, router_type, + **route_config) expected = {'display_name': 'fake_router_name', 'routing_config': { 'default_route_next_hop': @@ -452,6 +457,7 @@ class TestNvplibExplicitLRouters(NvplibTestCase): 'type': 'RouterNextHop'}, 'type': 'SingleDefaultRouteImplicitRoutingConfig'}, 'tags': [{'scope': 'os_tid', 'tag': 'fake_tenant_id'}, + {'scope': 'q_router_id', 'tag': 'pipita_higuain'}, {'scope': 'quantum', 'tag': nvplib.NEUTRON_VERSION}], 'type': 'LogicalRouterConfig'} @@ -461,11 +467,14 @@ class TestNvplibExplicitLRouters(NvplibTestCase): router_name = 'fake_router_name' tenant_id = 'fake_tenant_id' router_type = 'RoutingTableRoutingConfig' - body = nvplib._prepare_lrouter_body(router_name, tenant_id, - router_type) + neutron_router_id = 'marekiaro_hamsik' + body = nvplib._prepare_lrouter_body(router_name, neutron_router_id, + tenant_id, router_type) expected = {'display_name': 'fake_router_name', 'routing_config': {'type': 'RoutingTableRoutingConfig'}, 'tags': [{'scope': 'os_tid', 'tag': 'fake_tenant_id'}, + {'scope': 'q_router_id', + 'tag': 'marekiaro_hamsik'}, {'scope': 'quantum', 'tag': nvplib.NEUTRON_VERSION}], 'type': 'LogicalRouterConfig'} @@ -503,7 +512,9 @@ class TestNvplibExplicitLRouters(NvplibTestCase): return_value=self._get_lrouter(tenant_id, router_name, router_id)): - lrouter = nvplib.create_lrouter(self.fake_cluster, tenant_id, + lrouter = nvplib.create_lrouter(self.fake_cluster, + uuidutils.generate_uuid(), + tenant_id, router_name, nexthop_ip) self.assertEqual(lrouter['routing_config']['type'], 'RoutingTableRoutingConfig') @@ -591,6 +602,7 @@ class TestNvplibLogicalRouters(NvplibTestCase): expected_display_name, expected_nexthop, expected_tenant_id, + expected_neutron_id=None, expected_distributed=None): self.assertEqual(res_lrouter['uuid'], expected_uuid) nexthop = (res_lrouter['routing_config'] @@ -603,52 +615,68 @@ class TestNvplibLogicalRouters(NvplibTestCase): if expected_distributed is not None: self.assertEqual(expected_distributed, res_lrouter['distributed']) + if expected_neutron_id: + self.assertIn('q_router_id', router_tags) + self.assertEqual(expected_neutron_id, router_tags['q_router_id']) def test_get_lrouters(self): lrouter_uuids = [nvplib.create_lrouter( - self.fake_cluster, 'pippo', 'fake-lrouter-%s' % k, + self.fake_cluster, 'whatever', 'pippo', 'fake-lrouter-%s' % k, '10.0.0.1')['uuid'] for k in range(3)] routers = nvplib.get_lrouters(self.fake_cluster, 'pippo') for router in routers: self.assertIn(router['uuid'], lrouter_uuids) - def _create_lrouter(self, version, distributed=None): + def _create_lrouter(self, version, neutron_id=None, distributed=None): with mock.patch.object( self.fake_cluster.api_client, 'get_nvp_version', return_value=NvpApiClient.NVPVersion(version)): + if not neutron_id: + neutron_id = uuidutils.generate_uuid() lrouter = nvplib.create_lrouter( - self.fake_cluster, 'pippo', 'fake-lrouter', - '10.0.0.1', distributed=distributed) + self.fake_cluster, neutron_id, 'pippo', + 'fake-lrouter', '10.0.0.1', distributed=distributed) return nvplib.get_lrouter(self.fake_cluster, lrouter['uuid']) def test_create_and_get_lrouter_v30(self): - res_lrouter = self._create_lrouter('3.0') + neutron_id = uuidutils.generate_uuid() + res_lrouter = self._create_lrouter('3.0', neutron_id=neutron_id) self._verify_lrouter(res_lrouter, res_lrouter['uuid'], - 'fake-lrouter', '10.0.0.1', 'pippo') + 'fake-lrouter', '10.0.0.1', 'pippo', + neutron_id) def test_create_and_get_lrouter_v31_centralized(self): - res_lrouter = self._create_lrouter('3.1', distributed=False) + neutron_id = uuidutils.generate_uuid() + res_lrouter = self._create_lrouter('3.1', neutron_id=neutron_id, + distributed=False) self._verify_lrouter(res_lrouter, res_lrouter['uuid'], 'fake-lrouter', '10.0.0.1', 'pippo', + expected_neutron_id=neutron_id, expected_distributed=False) def test_create_and_get_lrouter_v31_distributed(self): - res_lrouter = self._create_lrouter('3.1', distributed=True) + neutron_id = uuidutils.generate_uuid() + res_lrouter = self._create_lrouter('3.1', neutron_id=neutron_id, + distributed=True) self._verify_lrouter(res_lrouter, res_lrouter['uuid'], 'fake-lrouter', '10.0.0.1', 'pippo', + expected_neutron_id=neutron_id, expected_distributed=True) def test_create_and_get_lrouter_name_exceeds_40chars(self): + neutron_id = uuidutils.generate_uuid() display_name = '*' * 50 lrouter = nvplib.create_lrouter(self.fake_cluster, + neutron_id, 'pippo', display_name, '10.0.0.1') res_lrouter = nvplib.get_lrouter(self.fake_cluster, lrouter['uuid']) self._verify_lrouter(res_lrouter, lrouter['uuid'], - '*' * 40, '10.0.0.1', 'pippo') + '*' * 40, '10.0.0.1', 'pippo', + expected_neutron_id=neutron_id) def _test_version_dependent_update_lrouter(self, version): def foo(*args, **kwargs): @@ -690,7 +718,9 @@ class TestNvplibLogicalRouters(NvplibTestCase): self._test_version_dependent_update_lrouter("4.1")) def test_update_lrouter_no_nexthop(self): + neutron_id = uuidutils.generate_uuid() lrouter = nvplib.create_lrouter(self.fake_cluster, + neutron_id, 'pippo', 'fake-lrouter', '10.0.0.1') @@ -701,10 +731,13 @@ class TestNvplibLogicalRouters(NvplibTestCase): res_lrouter = nvplib.get_lrouter(self.fake_cluster, lrouter['uuid']) self._verify_lrouter(res_lrouter, lrouter['uuid'], - 'new_name', '10.0.0.1', 'pippo') + 'new_name', '10.0.0.1', 'pippo', + expected_neutron_id=neutron_id) def test_update_lrouter(self): + neutron_id = uuidutils.generate_uuid() lrouter = nvplib.create_lrouter(self.fake_cluster, + neutron_id, 'pippo', 'fake-lrouter', '10.0.0.1') @@ -715,16 +748,19 @@ class TestNvplibLogicalRouters(NvplibTestCase): res_lrouter = nvplib.get_lrouter(self.fake_cluster, lrouter['uuid']) self._verify_lrouter(res_lrouter, lrouter['uuid'], - 'new_name', '192.168.0.1', 'pippo') + 'new_name', '192.168.0.1', 'pippo', + expected_neutron_id=neutron_id) def test_update_nonexistent_lrouter_raises(self): self.assertRaises(exceptions.NotFound, nvplib.update_lrouter, - self.fake_cluster, 'whatever', + self.fake_cluster, + 'whatever', 'foo', '9.9.9.9') def test_delete_lrouter(self): lrouter = nvplib.create_lrouter(self.fake_cluster, + uuidutils.generate_uuid(), 'pippo', 'fake-lrouter', '10.0.0.1') @@ -736,6 +772,7 @@ class TestNvplibLogicalRouters(NvplibTestCase): def test_query_lrouter_ports(self): lrouter = nvplib.create_lrouter(self.fake_cluster, + uuidutils.generate_uuid(), 'pippo', 'fake-lrouter', '10.0.0.1') @@ -757,6 +794,7 @@ class TestNvplibLogicalRouters(NvplibTestCase): def test_create_and_get_lrouter_port(self): lrouter = nvplib.create_lrouter(self.fake_cluster, + uuidutils.generate_uuid(), 'pippo', 'fake-lrouter', '10.0.0.1') @@ -781,6 +819,7 @@ class TestNvplibLogicalRouters(NvplibTestCase): def test_update_lrouter_port(self): lrouter = nvplib.create_lrouter(self.fake_cluster, + uuidutils.generate_uuid(), 'pippo', 'fake-lrouter', '10.0.0.1') @@ -812,6 +851,7 @@ class TestNvplibLogicalRouters(NvplibTestCase): def test_update_lrouter_port_nonexistent_port_raises(self): lrouter = nvplib.create_lrouter(self.fake_cluster, + uuidutils.generate_uuid(), 'pippo', 'fake-lrouter', '10.0.0.1') @@ -822,6 +862,7 @@ class TestNvplibLogicalRouters(NvplibTestCase): def test_delete_lrouter_port(self): lrouter = nvplib.create_lrouter(self.fake_cluster, + uuidutils.generate_uuid(), 'pippo', 'fake-lrouter', '10.0.0.1') @@ -842,6 +883,7 @@ class TestNvplibLogicalRouters(NvplibTestCase): def test_delete_lrouter_port_nonexistent_port_raises(self): lrouter = nvplib.create_lrouter(self.fake_cluster, + uuidutils.generate_uuid(), 'pippo', 'fake-lrouter', '10.0.0.1') @@ -851,6 +893,7 @@ class TestNvplibLogicalRouters(NvplibTestCase): def test_delete_peer_lrouter_port(self): lrouter = nvplib.create_lrouter(self.fake_cluster, + uuidutils.generate_uuid(), 'pippo', 'fake-lrouter', '10.0.0.1') @@ -869,6 +912,7 @@ class TestNvplibLogicalRouters(NvplibTestCase): def test_update_lrouter_port_ips_add_only(self): lrouter = nvplib.create_lrouter(self.fake_cluster, + uuidutils.generate_uuid(), 'pippo', 'fake-lrouter', '10.0.0.1') @@ -886,6 +930,7 @@ class TestNvplibLogicalRouters(NvplibTestCase): def test_update_lrouter_port_ips_remove_only(self): lrouter = nvplib.create_lrouter(self.fake_cluster, + uuidutils.generate_uuid(), 'pippo', 'fake-lrouter', '10.0.0.1') @@ -903,6 +948,7 @@ class TestNvplibLogicalRouters(NvplibTestCase): def test_update_lrouter_port_ips_add_and_remove(self): lrouter = nvplib.create_lrouter(self.fake_cluster, + uuidutils.generate_uuid(), 'pippo', 'fake-lrouter', '10.0.0.1') @@ -924,6 +970,7 @@ class TestNvplibLogicalRouters(NvplibTestCase): def test_update_lrouter_port_ips_nvp_exception_raises(self): lrouter = nvplib.create_lrouter(self.fake_cluster, + uuidutils.generate_uuid(), 'pippo', 'fake-lrouter', '10.0.0.1') @@ -952,6 +999,7 @@ class TestNvplibLogicalRouters(NvplibTestCase): tenant_id, 'xyz', 'name', 'device_id', True) lrouter = nvplib.create_lrouter(self.fake_cluster, + uuidutils.generate_uuid(), tenant_id, 'fake-lrouter', '10.0.0.1') @@ -967,6 +1015,7 @@ class TestNvplibLogicalRouters(NvplibTestCase): def test_plug_lrouter_port_l3_gw_attachment(self): lrouter = nvplib.create_lrouter(self.fake_cluster, + uuidutils.generate_uuid(), 'pippo', 'fake-lrouter', '10.0.0.1') @@ -983,6 +1032,7 @@ class TestNvplibLogicalRouters(NvplibTestCase): def test_plug_lrouter_port_l3_gw_attachment_with_vlan(self): lrouter = nvplib.create_lrouter(self.fake_cluster, + uuidutils.generate_uuid(), 'pippo', 'fake-lrouter', '10.0.0.1') @@ -1002,6 +1052,7 @@ class TestNvplibLogicalRouters(NvplibTestCase): def test_plug_lrouter_port_invalid_attachment_type_raises(self): lrouter = nvplib.create_lrouter(self.fake_cluster, + uuidutils.generate_uuid(), 'pippo', 'fake-lrouter', '10.0.0.1') @@ -1015,6 +1066,7 @@ class TestNvplibLogicalRouters(NvplibTestCase): def _test_create_router_snat_rule(self, version): lrouter = nvplib.create_lrouter(self.fake_cluster, + uuidutils.generate_uuid(), 'pippo', 'fake-lrouter', '10.0.0.1') @@ -1036,6 +1088,7 @@ class TestNvplibLogicalRouters(NvplibTestCase): def _test_create_router_dnat_rule(self, version, dest_port=None): lrouter = nvplib.create_lrouter(self.fake_cluster, + uuidutils.generate_uuid(), 'pippo', 'fake-lrouter', '10.0.0.1') @@ -1064,6 +1117,7 @@ class TestNvplibLogicalRouters(NvplibTestCase): def test_create_router_snat_rule_invalid_match_keys_raises(self): # In this case the version does not make a difference lrouter = nvplib.create_lrouter(self.fake_cluster, + uuidutils.generate_uuid(), 'pippo', 'fake-lrouter', '10.0.0.1') @@ -1079,6 +1133,7 @@ class TestNvplibLogicalRouters(NvplibTestCase): def _test_create_router_nosnat_rule(self, version, expected=1): lrouter = nvplib.create_lrouter(self.fake_cluster, + uuidutils.generate_uuid(), 'pippo', 'fake-lrouter', '10.0.0.1') @@ -1101,6 +1156,7 @@ class TestNvplibLogicalRouters(NvplibTestCase): def _prepare_nat_rules_for_delete_tests(self): lrouter = nvplib.create_lrouter(self.fake_cluster, + uuidutils.generate_uuid(), 'pippo', 'fake-lrouter', '10.0.0.1') -- 2.45.2