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 2edbcab03..6b4363546 100644 --- a/gbpservice/neutron/plugins/ml2plus/drivers/apic_aim/mechanism_driver.py +++ b/gbpservice/neutron/plugins/ml2plus/drivers/apic_aim/mechanism_driver.py @@ -2694,6 +2694,36 @@ class ApicMechanismDriver(api_plus.MechanismDriver, self.delete_port_id_for_ha_ipaddress( port['id'], ip, session=session) + def _update_svi_ports_for_added_subnet(self, ctx, new_subnets, network_id): + """ Subnet added to bound port on SVI net, add to SVI ports if needed. + + We have to ensure that the SVI ports are updated with the added + subnet _before_ we get into updating the static paths to generate + the Interface Profile for the AF of tne new subnet. Doing this + in the context of the BEFORE_UPDATE event guarantees that there + is no race condition. + + """ + # Get the SVI ports + filters = {'network_id': [network_id], + 'device_owner': [aim_cst.DEVICE_OWNER_SVI_PORT]} + svi_ports = self.plugin.get_ports(ctx, filters) + for svi_port in svi_ports: + subnet_added = False + svi_port_subnets = [s['subnet_id'] + for s in svi_port['fixed_ips']] + # update the SVI port if the added subnet is not present. + for s in new_subnets: + if s not in svi_port_subnets: + svi_port['fixed_ips'].append( + {'subnet_id': s}) + subnet_added = True + if subnet_added: + port_info = { + 'port': {'fixed_ips': svi_port['fixed_ips']} + } + self.plugin.update_port(ctx, svi_port['id'], port_info) + # NOTE: This event is generated by the ML2 plugin in three places, # all outside of transactions. It is generated from update_port # and from _update_individual_port_db_status without the @@ -2703,6 +2733,28 @@ class ApicMechanismDriver(api_plus.MechanismDriver, def _before_update_port(self, resource, event, trigger, context, port, original_port, orig_binding=None, new_binding=None): + if self._is_port_bound(original_port) and 'fixed_ips' in port: + # When a bound port is updated with a subnet, if the port + # is on a SVI network, we need to ensure that the SVI ports + # which are used for creating the Interface Profile for each + # AF also gets updated with the new subnet. + # + # Since we are not in the port binding workflow, can't get the + # bind_context + + curr = [s['subnet_id'] + for s in port['fixed_ips']] + orig = [s['subnet_id'] + for s in original_port['fixed_ips']] + new_subnets = list(set(curr) - set(orig)) + if not new_subnets: + return + ctx = nctx.get_admin_context() + net_db = self.plugin._get_network(ctx, original_port['network_id']) + if self._is_svi_db(net_db): + self._update_svi_ports_for_added_subnet(ctx, new_subnets, + original_port['network_id']) + # We are only interested in port update callbacks from # _commit_port_binding, in which the port is about to become # bound. Any other callbacks using this event are ignored. @@ -4791,7 +4843,8 @@ class ApicMechanismDriver(api_plus.MechanismDriver, {'port': attrs, 'ex': e}) def _update_static_path_for_svi(self, session, plugin_context, network, - l3out, static_port, remove=False): + l3out, static_port, remove=False, + del_subnet=None): """Configure static path for an SVI on a L3 Out Add, update, or delete a single static path on an SVI. Adds @@ -4809,7 +4862,7 @@ class ApicMechanismDriver(api_plus.MechanismDriver, # so if we're creating new nodes, then we need to allocate # new IDs for each one. aim_ctx = aim_context.AimContext(db_session=session) - if not remove and path: + if not remove and not del_subnet and path: for node_path in node_paths: # REVISIT: We should check to see if this node is # already present under the node profile. @@ -4841,7 +4894,6 @@ class ApicMechanismDriver(api_plus.MechanismDriver, 'mask': str(netaddr.IPNetwork( subnet['cidr']).prefixlen), 'primary_ips': []} - for node in nodes: # Get the IP of the SVI port for this node on this network. # @@ -4855,6 +4907,8 @@ class ApicMechanismDriver(api_plus.MechanismDriver, for fixed_ip in svi_ports[0]['fixed_ips']: ip_vers = netaddr.IPAddress( fixed_ip['ip_address']).version + if ip_vers not in subnets_dict: + continue subnets_dict[ip_vers]['primary_ips'].append( fixed_ip['ip_address'] + '/' + ( @@ -4915,10 +4969,26 @@ class ApicMechanismDriver(api_plus.MechanismDriver, self.aim.create(aim_ctx, aim_bgp_peer_prefix, overwrite=True) - if remove: + if remove or del_subnet: + if del_subnet: + # This subnet is not present on this host, Find the AF, + # to map the IProfile to remove. + query = BAKERY(lambda s: s.query( + models_v2.Subnet)) + query += lambda q: q.filter( + models_v2.Subnet.id == sa.bindparam('subnet_id')) + subnet = query(session).params( + subnet_id=del_subnet[0]).one_or_none() + if subnet.ip_version == 6: + if_profiles = [L3OUT_IF_PROFILE_NAME6] + else: + if_profiles = [L3OUT_IF_PROFILE_NAME] + else: + # Last port being removed so remove all IProfiles. + if_profiles = [L3OUT_IF_PROFILE_NAME, L3OUT_IF_PROFILE_NAME6] # REVISIT: Should we also delete the node profiles if there aren't # any more instances on this host? - for if_profile in [L3OUT_IF_PROFILE_NAME, L3OUT_IF_PROFILE_NAME6]: + for if_profile in if_profiles: aim_l3out_if = aim_resource.L3OutInterface( tenant_name=l3out.tenant_name, l3out_name=l3out.name, @@ -5292,7 +5362,20 @@ class ApicMechanismDriver(api_plus.MechanismDriver, LOG.debug('Port %s is not bound to any segment', port_context.current['id']) return - if remove: + + del_subnet = [] + if port_context.current and port_context.original: + # We are in a port update. + # If a subnet is deleted on this bound port, we need + # evaluate if we need to remove the corresponding + # IProfile. + curr = [s['subnet_id'] + for s in port_context.current['fixed_ips']] + orig = [s['subnet_id'] + for s in port_context.original['fixed_ips']] + del_subnet = list(set(orig) - set(curr)) + + if remove or del_subnet: # check if there are any other ports from this network on the host query = BAKERY(lambda s: s.query( models.PortBindingLevel)) @@ -5304,11 +5387,26 @@ class ApicMechanismDriver(api_plus.MechanismDriver, exist = query(session).params( host=host, segment_id=segment['id'], - port_id=port_context.current['id']).first() + port_id=port_context.current['id']).all() - if exist: + if remove and exist: + # We are removing a bound port, but there is at least + # one other port on this host so don't remove the + # IProfile. return + if del_subnet and exist: + # Subnet is being removed from the port, if there is + # at least one other port with the subnet on the host - + # Leave the IProfile for this AF intact. + host_ports = [p['port_id'] for p in exist] + if session.query(models_v2.Port).filter( + models_v2.Port.id.in_(host_ports)).filter( + models_v2.Port.fixed_ips.any( + models_v2.IPAllocation.subnet_id.in_( + del_subnet))).all(): + return + static_ports = self._get_static_ports(port_context._plugin_context, host, segment, port_context=port_context) @@ -5319,7 +5417,7 @@ class ApicMechanismDriver(api_plus.MechanismDriver, self._update_static_path_for_svi( session, port_context._plugin_context, port_context.network.current, l3out, - static_port, remove=remove) + static_port, remove=remove, del_subnet=del_subnet) else: self._update_static_path_for_network( session, port_context.network.current, 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 9e2a8f9c2..e086d9e38 100644 --- a/gbpservice/neutron/tests/unit/plugins/ml2plus/test_apic_aim.py +++ b/gbpservice/neutron/tests/unit/plugins/ml2plus/test_apic_aim.py @@ -5109,6 +5109,163 @@ class TestPortBinding(ApicAimTestCase): bgp_peer = self.aim_mgr.get(aim_ctx, bgp_peer) self.assertIsNone(bgp_peer) + def test_dualstack_svi_opflex_agent_add_subnet(self): + aim_ctx = aim_context.AimContext(self.db_session) + + hlink1 = aim_infra.HostLink( + host_name='h1', + interface_name='eth1', + path='topology/pod-1/paths-102/pathep-[eth1/8]') + self.aim_mgr.create(aim_ctx, hlink1) + + self._register_agent('h1', AGENT_CONF_OPFLEX) + + net1 = self._make_network(self.fmt, 'net1', True, + arg_list=self.extension_attributes, + **{'apic:svi': 'True', + 'provider:network_type': u'vlan', + 'apic:bgp_enable': 'True', + 'apic:bgp_asn': '2'})['network'] + + with self.subnet(network={'network': net1}) as sub1: + with self.port(subnet=sub1) as p1: + # bind p1 to host h1 + p1 = self._bind_port_to_host(p1['port']['id'], 'h1') + vlan_p1 = self._check_binding(p1['port']['id'], + expected_binding_info=[('apic_aim', 'vlan')]) + ext_net = aim_resource.ExternalNetwork.from_dn( + net1[DN]['ExternalNetwork']) + + l3out_np = aim_resource.L3OutNodeProfile( + tenant_name=ext_net.tenant_name, + l3out_name=ext_net.l3out_name, + name=md.L3OUT_NODE_PROFILE_NAME) + l3out_np = self.aim_mgr.get(aim_ctx, l3out_np) + self.assertEqual(l3out_np.name, md.L3OUT_NODE_PROFILE_NAME) + + l3out_node = aim_resource.L3OutNode( + tenant_name=ext_net.tenant_name, + l3out_name=ext_net.l3out_name, + node_profile_name=md.L3OUT_NODE_PROFILE_NAME, + node_path='topology/pod-1/node-102') + l3out_node = self.aim_mgr.get(aim_ctx, l3out_node) + self.assertIsNotNone(l3out_node) + + l3out_ifs = [] + bgp_peers = [] + for if_profile in [md.L3OUT_IF_PROFILE_NAME, + md.L3OUT_IF_PROFILE_NAME6]: + l3out_ip = aim_resource.L3OutInterfaceProfile( + tenant_name=ext_net.tenant_name, + l3out_name=ext_net.l3out_name, + node_profile_name=md.L3OUT_NODE_PROFILE_NAME, + name=if_profile) + l3out_ip = self.aim_mgr.get(aim_ctx, l3out_ip) + + l3out_if = aim_resource.L3OutInterface( + tenant_name=ext_net.tenant_name, + l3out_name=ext_net.l3out_name, + node_profile_name=md.L3OUT_NODE_PROFILE_NAME, + interface_profile_name=if_profile, + interface_path=hlink1.path) + l3out_if = self.aim_mgr.get(aim_ctx, l3out_if) + if if_profile == md.L3OUT_IF_PROFILE_NAME: + self.assertEqual(l3out_ip.name, if_profile) + self.assertEqual(l3out_if.encap, 'vlan-%s' % vlan_p1) + self.assertEqual(l3out_if.secondary_addr_a_list, + [{'addr': '10.0.0.1/24'}]) + + primary = netaddr.IPNetwork(l3out_if.primary_addr_a) + subnet = str(primary.cidr) + bgp_peer = aim_resource.L3OutInterfaceBgpPeerP( + tenant_name=ext_net.tenant_name, + l3out_name=ext_net.l3out_name, + node_profile_name=md.L3OUT_NODE_PROFILE_NAME, + interface_profile_name=if_profile, + interface_path=hlink1.path, + addr=subnet) + bgp_peer = self.aim_mgr.get(aim_ctx, bgp_peer) + self.assertEqual(bgp_peer.asn, '2') + else: + self.assertIsNone(l3out_ip) + self.assertIsNone(l3out_if) + + sub2 = self._make_subnet(self.fmt, + {'network': net1}, '2001:db8::1', + '2001:db8::0/64', ip_version=6)['subnet'] + + p1['port']['fixed_ips'].append({'subnet_id': sub2['id']}) + port_info = {'port': {'fixed_ips': p1['port']['fixed_ips']}} + p1 = self._update('ports', p1['port']['id'], port_info) + + # The bound port now has the newly created v6 subnet, + # now we should have the v6 Interface Profile + for if_profile in [md.L3OUT_IF_PROFILE_NAME, + md.L3OUT_IF_PROFILE_NAME6]: + l3out_ip = aim_resource.L3OutInterfaceProfile( + tenant_name=ext_net.tenant_name, + l3out_name=ext_net.l3out_name, + node_profile_name=md.L3OUT_NODE_PROFILE_NAME, + name=if_profile) + l3out_ip = self.aim_mgr.get(aim_ctx, l3out_ip) + self.assertEqual(l3out_ip.name, if_profile) + + l3out_if = aim_resource.L3OutInterface( + tenant_name=ext_net.tenant_name, + l3out_name=ext_net.l3out_name, + node_profile_name=md.L3OUT_NODE_PROFILE_NAME, + interface_profile_name=if_profile, + interface_path=hlink1.path) + l3out_if = self.aim_mgr.get(aim_ctx, l3out_if) + self.assertEqual(l3out_if.encap, 'vlan-%s' % vlan_p1) + if if_profile == md.L3OUT_IF_PROFILE_NAME: + self.assertEqual(l3out_if.secondary_addr_a_list, + [{'addr': '10.0.0.1/24'}]) + else: + self.assertEqual(l3out_if.secondary_addr_a_list, + [{'addr': '2001:db8::1/64'}]) + l3out_ifs.append(l3out_if) + + primary = netaddr.IPNetwork(l3out_if.primary_addr_a) + subnet = str(primary.cidr) + bgp_peer = aim_resource.L3OutInterfaceBgpPeerP( + tenant_name=ext_net.tenant_name, + l3out_name=ext_net.l3out_name, + node_profile_name=md.L3OUT_NODE_PROFILE_NAME, + interface_profile_name=if_profile, + interface_path=hlink1.path, + addr=subnet) + bgp_peer = self.aim_mgr.get(aim_ctx, bgp_peer) + self.assertEqual(bgp_peer.asn, '2') + bgp_peers.append(bgp_peer) + + # Remove the v6 subnet from the port and verify that + # the v6 Profile is deleted. + v4_only = [d for d in p1['port']['fixed_ips'] + if not (d['subnet_id'] == sub2['id'])] + port_info = {'port': {'fixed_ips': v4_only}} + p1 = self._update('ports', p1['port']['id'], port_info) + + for l3out_if in l3out_ifs: + l3out_if = self.aim_mgr.get(aim_ctx, l3out_if) + if l3out_if and l3out_if.interface_profile_name == ( + md.L3OUT_IF_PROFILE_NAME): + self.assertEqual(l3out_if.secondary_addr_a_list, + [{'addr': '10.0.0.1/24'}]) + else: + self.assertIsNone(l3out_if) + + self._delete('ports', p1['port']['id']) + self._check_no_dynamic_segment(net1['id']) + + for l3out_if in l3out_ifs: + l3out_if = self.aim_mgr.get(aim_ctx, l3out_if) + self.assertIsNone(l3out_if) + + for bgp_peer in bgp_peers: + bgp_peer = self.aim_mgr.get(aim_ctx, bgp_peer) + self.assertIsNone(bgp_peer) + def test_bind_opflex_agent_svi(self): self._register_agent('host1', AGENT_CONF_OPFLEX) aim_ctx = aim_context.AimContext(self.db_session)