From 46a2cde8d7dbb0512ec2cfd1151cf7428f1c91a2 Mon Sep 17 00:00:00 2001 From: Amit Bose Date: Wed, 18 Jan 2017 17:26:35 -0800 Subject: [PATCH] [AIM] External-connecitvity for unscoped shared networks Networks that don't use an address-scope can move between VRFs as they are connected/disconnected via routers to other networks. Accordingly, the external-connectivity of the VRFs needs to be updated. This is required only for the case where the "router topology" changes VRF - the old VRF loses external connectivity available through the router whereas the new VRF gains the same. Change-Id: I7b6d22405a8c7b0a93a88e7bc186ab7c158757da Signed-off-by: Amit Bose (cherry picked from commit d1044e846d4249ce33ad44ee7dc6676f265f7383) --- .../drivers/apic_aim/mechanism_driver.py | 42 +++++++-- .../unit/plugins/ml2plus/test_apic_aim.py | 92 ++++++++++++++++++- 2 files changed, 126 insertions(+), 8 deletions(-) diff --git a/gbpservice/neutron/plugins/ml2plus/drivers/apic_aim/mechanism_driver.py b/gbpservice/neutron/plugins/ml2plus/drivers/apic_aim/mechanism_driver.py index 6c4bc57ec..3c86407bf 100644 --- a/gbpservice/neutron/plugins/ml2plus/drivers/apic_aim/mechanism_driver.py +++ b/gbpservice/neutron/plugins/ml2plus/drivers/apic_aim/mechanism_driver.py @@ -871,6 +871,8 @@ class ApicMechanismDriver(api_plus.MechanismDriver): if different_router and different_subnet: raise UnsupportedRoutingTopology() + router_topo_moved = False + # Ensure that all the BDs and EPGs in the resulting topology # are mapped under the same Tenant so that the BDs can all # reference the topology's VRF and the EPGs can all provide @@ -920,6 +922,7 @@ class ApicMechanismDriver(api_plus.MechanismDriver): vrf = self._ensure_default_vrf(aim_ctx, intf_vrf) self._move_topology( aim_ctx, router_topology, router_vrf, vrf) + router_topo_moved = True # REVISIT: Delete router_vrf if no longer used? elif router_shared_net: # Router topology has shared network, so move @@ -980,15 +983,25 @@ class ApicMechanismDriver(api_plus.MechanismDriver): epg = self.aim.update(aim_ctx, epg, provided_contract_names=contracts) + # If external-gateway is set, handle external-connectivity changes. if router.gw_port_id: + net = self.plugin.get_network(context, + router.gw_port.network_id) # If this is first interface-port, then that will determine - # the VRF for this router. Setup exteral-connectivity for VRF - # if external-gateway is set. + # the VRF for this router. Setup external-connectivity for VRF. if not router_intf_count: - net = self.plugin.get_network(context, - router.gw_port.network_id) self._manage_external_connectivity(context, router, None, net, vrf) + elif router_topo_moved: + # Router moved from router_vrf to vrf, so + # 1. Update router_vrf's external connectivity to exclude + # router + # 2. Update vrf's external connectivity to include router + self._manage_external_connectivity(context, router, net, None, + router_vrf) + self._manage_external_connectivity(context, router, None, net, + vrf) + # SNAT information of ports on the subnet will change because # of router interface addition. Send a port update so that it may # be recalculated. @@ -1052,6 +1065,7 @@ class ApicMechanismDriver(api_plus.MechanismDriver): epg = self.aim.update(aim_ctx, epg, provided_contract_names=contracts) + router_topo_moved = False # If unscoped topologies have split, move VRFs as needed. if scope_id == NO_ADDR_SCOPE: # If the interface's network has not become unrouted, see @@ -1075,24 +1089,36 @@ class ApicMechanismDriver(api_plus.MechanismDriver): if old_vrf.identity != router_vrf.identity: self._move_topology( aim_ctx, router_topology, old_vrf, router_vrf) + router_topo_moved = True # If network is no longer connected to any router, make the # network's BD unrouted. if not router_ids: self._dissassociate_network_from_vrf(aim_ctx, network_db, old_vrf) + # If external-gateway is set, handle external-connectivity changes. if router_db.gw_port_id: + net = self.plugin.get_network(context, + router_db.gw_port.network_id) # If this was the last interface-port, then we no longer know # the VRF for this router. So update external-conectivity to # exclude this router. if not self._get_router_intf_count(session, router_db): - net = self.plugin.get_network(context, - router_db.gw_port.network_id) self._manage_external_connectivity( context, router_db, net, None, old_vrf) self._delete_snat_ip_ports_if_reqd(context, net['id'], router_id) + elif router_topo_moved: + # Router moved from old_vrf to router_vrf, so + # 1. Update old_vrf's external connectivity to exclude router + # 2. Update router_vrf's external connectivity to include + # router + self._manage_external_connectivity(context, router_db, net, + None, old_vrf) + self._manage_external_connectivity(context, router_db, None, + net, router_vrf) + # SNAT information of ports on the subnet will change because # of router interface removal. Send a port update so that it may # be recalculated. @@ -1925,6 +1951,10 @@ class ApicMechanismDriver(api_plus.MechanismDriver): aim_ctx = aim_context.AimContext(db_session=session) vrf = vrf or self._get_vrf_for_router(session, router) + # Keep only the identity attributes of the VRF so that calls to + # nat-library have consistent resource values. This + # is mainly required to ease unit-test verification. + vrf = aim_resource.VRF(tenant_name=vrf.tenant_name, name=vrf.name) rtr_dbs = self._get_routers_for_vrf(session, vrf) ext_db = extension_db.ExtensionDbMixin() diff --git a/gbpservice/neutron/tests/unit/plugins/ml2plus/test_apic_aim.py b/gbpservice/neutron/tests/unit/plugins/ml2plus/test_apic_aim.py index bd27f3bd9..d987af66a 100644 --- a/gbpservice/neutron/tests/unit/plugins/ml2plus/test_apic_aim.py +++ b/gbpservice/neutron/tests/unit/plugins/ml2plus/test_apic_aim.py @@ -2828,8 +2828,6 @@ class TestExternalConnectivityBase(object): for router, subnets, addr_scope in router_list: if addr_scope: a_vrf.name = addr_scope['id'] - else: - a_vrf.display_name = 'DefaultRoutedVRF' contract = router['id'] a_ext_net.provided_contract_names.append(contract) a_ext_net.provided_contract_names.extend( @@ -3240,6 +3238,92 @@ class TestExternalConnectivityBase(object): {'router': {'external_gateway_info': {}}}) mock_notif.assert_has_calls(port_calls, any_order=True) + def test_shared_unscoped_network(self): + # 0. Initial state: Router r1 is connected to external-network + # 1. Create unshared network net1 in tenant tenant_1, then connect + # it to router r1 + # 2. Create shared network net2 in tenant tenant_2, then connect + # it to router r1 + # 3. Create unshared network net3 in tenant test-tenant, then connect + # it to router r1 + # 4. Disconnect net3 from r1 + # 5. Disconnect net2 from r1 + # 6. Disconnect net1 from r1 + + cv = self.mock_ns.connect_vrf + dv = self.mock_ns.disconnect_vrf + + ext_net1 = self._make_ext_network('ext-net1', + dn='uni/tn-t1/out-l1/instP-n1') + self._make_subnet( + self.fmt, {'network': ext_net1}, '100.100.100.1', + '100.100.100.0/24') + router = self._make_router( + self.fmt, ext_net1['tenant_id'], 'router1', + external_gateway_info={'network_id': ext_net1['id']})['router'] + cv.assert_not_called() + dv.assert_not_called() + + contract = router['id'] + a_ext_net1 = aim_resource.ExternalNetwork( + tenant_name='t1', l3out_name='l1', name='n1', + provided_contract_names=[contract], + consumed_contract_names=[contract]) + a_ext_net1_no_contracts = aim_resource.ExternalNetwork( + tenant_name='t1', l3out_name='l1', name='n1') + + # 1. Create unshared network net1 in tenant tenant_1, then connect + # it to router r1 + net1 = self._make_network(self.fmt, 'net1', True, + tenant_id='tenant_1')['network'] + sub1 = self._make_subnet(self.fmt, {'network': net1}, + '10.10.10.1', '10.10.10.0/24')['subnet'] + self._router_interface_action('add', router['id'], sub1['id'], None) + a_vrf1 = aim_resource.VRF(tenant_name='tenant_1', name='DefaultVRF') + cv.assert_called_once_with(mock.ANY, a_ext_net1, a_vrf1) + dv.assert_not_called() + + # 2. Create shared network net2 in tenant tenant_2, then connect + # it to router r1 + self.mock_ns.reset_mock() + net2 = self._make_network(self.fmt, 'net2', True, + tenant_id='tenant_2', shared=True)['network'] + sub2 = self._make_subnet(self.fmt, {'network': net2}, + '20.20.20.1', '20.20.20.0/24')['subnet'] + self._router_interface_action('add', router['id'], sub2['id'], None) + a_vrf2 = aim_resource.VRF(tenant_name='tenant_2', name='DefaultVRF') + cv.assert_called_once_with(mock.ANY, a_ext_net1, a_vrf2) + dv.assert_called_once_with(mock.ANY, a_ext_net1_no_contracts, a_vrf1) + + # 3. Create unshared network net3 in tenant test-tenant, then connect + # it to router r1 + self.mock_ns.reset_mock() + net3 = self._make_network(self.fmt, 'net3', True, + tenant_id='test-tenant')['network'] + sub3 = self._make_subnet(self.fmt, {'network': net3}, + '30.30.30.1', '30.30.30.0/24')['subnet'] + self._router_interface_action('add', router['id'], sub3['id'], None) + cv.assert_not_called() + dv.assert_not_called() + + # 4. Disconnect net3 from r1 + self.mock_ns.reset_mock() + self._router_interface_action('remove', router['id'], sub3['id'], None) + cv.assert_not_called() + dv.assert_not_called() + + # 5. Disconnect net2 from r1 + self.mock_ns.reset_mock() + self._router_interface_action('remove', router['id'], sub2['id'], None) + cv.assert_called_once_with(mock.ANY, a_ext_net1, a_vrf1) + dv.assert_called_once_with(mock.ANY, a_ext_net1_no_contracts, a_vrf2) + + # 6. Disconnect net1 from r1 + self.mock_ns.reset_mock() + self._router_interface_action('remove', router['id'], sub1['id'], None) + cv.assert_not_called() + dv.assert_called_once_with(mock.ANY, a_ext_net1_no_contracts, a_vrf1) + class TestExternalDistributedNat(TestExternalConnectivityBase, ApicAimTestCase): @@ -3257,6 +3341,10 @@ class TestExternalNoNat(TestExternalConnectivityBase, tenant_1 = 't1' tenant_2 = 'common' + def test_shared_unscoped_network(self): + # Skip test since the topology tested is not valid with no-NAT + pass + class TestSnatIpAllocation(ApicAimTestCase):