From 9ca4ae390b49f4b042b352dcb7dc40037a2d365f Mon Sep 17 00:00:00 2001 From: Kent Wu Date: Mon, 4 Feb 2019 17:24:33 -0800 Subject: [PATCH] Support GBP workflow for get_gbp_details() raw sql approach Change-Id: Id5328e91fa3aa0fe261dea310436c73f4ac6c7f7 --- .../drivers/cisco/apic/aim_mapping.py | 37 ++++++---- .../drivers/cisco/apic/aim_mapping_rpc.py | 71 +++++++++++++------ .../grouppolicy/drivers/neutron_resources.py | 11 +-- .../grouppolicy/test_aim_mapping_driver.py | 59 +++++++++++++-- 4 files changed, 136 insertions(+), 42 deletions(-) diff --git a/gbpservice/neutron/services/grouppolicy/drivers/cisco/apic/aim_mapping.py b/gbpservice/neutron/services/grouppolicy/drivers/cisco/apic/aim_mapping.py index 2c5f63b83..043ad458f 100644 --- a/gbpservice/neutron/services/grouppolicy/drivers/cisco/apic/aim_mapping.py +++ b/gbpservice/neutron/services/grouppolicy/drivers/cisco/apic/aim_mapping.py @@ -1806,11 +1806,11 @@ class AIMMappingDriver(nrd.CommonNeutronBase, aim_rpc.AIMMappingRPCMixin): session = context._plugin_context.session return aim_context.AimContext(session) - def _is_port_promiscuous(self, plugin_context, port, is_gbp=True): - if is_gbp: - pt = self._port_id_to_pt(plugin_context, port['id']) + def _is_port_promiscuous(self, plugin_context, port, details=None): + if details and 'pt' in details['_cache']: + pt = details['_cache']['pt'] else: - pt = None + pt = self._port_id_to_pt(plugin_context, port['id']) if (pt and pt.get('cluster_id') and pt.get('cluster_id') != pt['id']): master = self._get_policy_target(plugin_context, pt['cluster_id']) @@ -1835,9 +1835,15 @@ class AIMMappingDriver(nrd.CommonNeutronBase, aim_rpc.AIMMappingRPCMixin): details['dhcp_lease_time'] = ( self.aim_mech_driver.apic_optimized_dhcp_lease_time) - def _get_port_epg(self, plugin_context, port): - ptg, pt = self._port_id_to_ptg(plugin_context, port['id']) + def _get_port_epg(self, plugin_context, port, details=None): + if details and 'pt' in details['_cache']: + pt = details['_cache']['pt'] + ptg = self._pt_to_ptg(plugin_context, pt) + else: + ptg, pt = self._port_id_to_ptg(plugin_context, port['id']) + if ptg: + # TODO(Kent): optimize this also for GBP workflow? return self._get_aim_endpoint_group(plugin_context.session, ptg) else: # Return default EPG based on network @@ -1851,12 +1857,12 @@ class AIMMappingDriver(nrd.CommonNeutronBase, aim_rpc.AIMMappingRPCMixin): "port %s", port['id']) return epg - def _get_subnet_details(self, plugin_context, port, details, is_gbp=True): + def _get_subnet_details(self, plugin_context, port, details): # L2P might not exist for a pure Neutron port - if is_gbp: - l2p = self._network_id_to_l2p(plugin_context, port['network_id']) + if 'l2p' in details['_cache']: + l2p = details['_cache']['l2p'] else: - l2p = None + l2p = self._network_id_to_l2p(plugin_context, port['network_id']) # TODO(ivar): support shadow network # if not l2p and self._ptg_needs_shadow_network(context, ptg): # l2p = self._get_l2_policy(context._plugin_context, @@ -2027,10 +2033,13 @@ class AIMMappingDriver(nrd.CommonNeutronBase, aim_rpc.AIMMappingRPCMixin): return net_ids def _get_segmentation_labels(self, plugin_context, port, details): - pt = self._port_id_to_pt(plugin_context, port['id']) - if self.apic_segmentation_label_driver and pt and ( - 'segmentation_labels' in pt): - return pt['segmentation_labels'] + if self.apic_segmentation_label_driver: + if 'pt' in details['_cache']: + pt = details['_cache']['pt'] + else: + pt = self._port_id_to_pt(plugin_context, port['id']) + if pt and 'segmentation_labels' in pt: + return pt['segmentation_labels'] def _get_nat_details(self, plugin_context, port, host, details): """ Add information about IP mapping for DNAT/SNAT """ diff --git a/gbpservice/neutron/services/grouppolicy/drivers/cisco/apic/aim_mapping_rpc.py b/gbpservice/neutron/services/grouppolicy/drivers/cisco/apic/aim_mapping_rpc.py index 20fb2b0e7..bc13eed24 100644 --- a/gbpservice/neutron/services/grouppolicy/drivers/cisco/apic/aim_mapping_rpc.py +++ b/gbpservice/neutron/services/grouppolicy/drivers/cisco/apic/aim_mapping_rpc.py @@ -285,8 +285,8 @@ class AIMMappingRPCMixin(ha_ip_db.HAIPOwnerDbMixin): in_str = self._compose_in_filter_str(sg_list) sg_query = ("SELECT id, project_id FROM securitygroups WHERE " "id in " + in_str) - sg_result = session.execute(sg_query) - details['_cache']['security_groups'] = sg_result + details['_cache']['security_groups'] = list( + session.execute(sg_query)) # Get the subnet info subnets = [] @@ -382,7 +382,8 @@ class AIMMappingRPCMixin(ha_ip_db.HAIPOwnerDbMixin): all_vrfs_bds_query = ( "SELECT name, tenant_name FROM aim_bridge_domains " "WHERE vrf_name = '" + network['vrf_name'] + "'") - all_vrfs_bds_result = session.execute(all_vrfs_bds_query) + all_vrfs_bds_result = list( + session.execute(all_vrfs_bds_query)) all_vrfs_query = ( "SELECT tenant_name FROM aim_vrfs WHERE " "name = '" + network['vrf_name'] + "'") @@ -411,7 +412,7 @@ class AIMMappingRPCMixin(ha_ip_db.HAIPOwnerDbMixin): "network_id in " + in_str) vrf_subnet_result = session.execute(vrf_subnet_query) vrf_subnets = [x['cidr'] for x in vrf_subnet_result] - details['_cache']['address_scope'] = as_result + details['_cache']['address_scope'] = list(as_result) details['_cache']['subnetpools'] = subnetpools details['_cache']['vrf_subnets'] = vrf_subnets @@ -467,6 +468,39 @@ class AIMMappingRPCMixin(ha_ip_db.HAIPOwnerDbMixin): allowed_vlan.vlan) details['_cache']['network'] = network + # For PT + pt_query = ( + "SELECT id, policy_target_group_id, cluster_id FROM " + "gp_policy_targets WHERE " + "port_id = '" + port['id'] + "'") + pt = session.execute(pt_query).first() + if pt: + pt = dict(pt) + segmentation_label_query = ( + "SELECT segmentation_label FROM " + "gp_apic_mapping_segmentation_labels WHERE " + "policy_target_id = '" + pt['id'] + "'") + st_label_result = session.execute(segmentation_label_query) + pt['segmentation_labels'] = [x['segmentation_label'] + for x in st_label_result] + group_default_gw_query = ( + "SELECT group_default_gateway FROM " + "gp_proxy_gateway_mappings WHERE " + "policy_target_id = '" + pt['id'] + "'") + group_default_gw = session.execute(group_default_gw_query).first() + if group_default_gw: + pt['group_default_gateway'] = group_default_gw[ + 'group_default_gateway'] + details['_cache']['pt'] = pt + + # For L2P + l2p_query = ( + "SELECT inject_default_route FROM " + "gp_l2_policies WHERE " + "network_id = '" + network['id'] + "'") + l2p = session.execute(l2p_query).first() + details['_cache']['l2p'] = l2p + def _get_gbp_details_new(self, context, request, host): with context.session.begin(subtransactions=True): device = request.get('device') @@ -597,9 +631,6 @@ class AIMMappingRPCMixin(ha_ip_db.HAIPOwnerDbMixin): # name mapper 'ptg_tenant': network['epg_tenant_name'], 'endpoint_group_name': network['epg_name'], - # TODO(kentwu): make it to support GBP workflow also - 'promiscuous_mode': self._is_port_promiscuous( - context, port, is_gbp=False), 'extra_ips': [], 'floating_ip': [], 'ip_mapping': [], @@ -611,14 +642,19 @@ class AIMMappingRPCMixin(ha_ip_db.HAIPOwnerDbMixin): details['_cache'] = {} self._build_up_details_cache( context.session, details, port, network) + # EPG name is different for GBP workflow + if details['_cache']['pt']: + epg = self._get_port_epg(context, port, details) + details['endpoint_group_name'] = epg.name + details['promiscuous_mode'] = self._is_port_promiscuous( + context, port, details) mtu = self._get_port_mtu(context, port, details) if mtu: details['interface_mtu'] = mtu details['dns_domain'] = network['dns_domain'] if port.get('security_groups'): self._add_security_group_details(context, port, details) - # TODO(kentwu): make it to support GBP workflow if needed - self._add_subnet_details(context, port, details, is_gbp=False) + self._add_subnet_details(context, port, details) self._add_allowed_address_pairs_details(context, port, details) details['l3_policy_id'] = '%s %s' % ( network['vrf_tenant_name'], network['vrf_name']) @@ -654,9 +690,7 @@ class AIMMappingRPCMixin(ha_ip_db.HAIPOwnerDbMixin): details['_cache']['fips'] = fips self._add_nat_details(context, port, host, details) self._add_extra_details(context, port, details) - # TODO(kentwu): make it to support GBP workflow also - self._add_segmentation_label_details(context, port, details, - is_gbp=False) + self._add_segmentation_label_details(context, port, details) self._set_dhcp_lease_time(details) self._add_nested_domain_details(context, port, details) details.pop('_cache', None) @@ -745,12 +779,11 @@ class AIMMappingRPCMixin(ha_ip_db.HAIPOwnerDbMixin): # Child class needs to support: # - self._get_subnet_details(context, port, details) - def _add_subnet_details(self, context, port, details, is_gbp=True): + def _add_subnet_details(self, context, port, details): # This method needs to define requirements for this Mixin's child # classes in order to fill the following result parameters: # - subnets; - details['subnets'] = self._get_subnet_details(context, port, details, - is_gbp) + details['subnets'] = self._get_subnet_details(context, port, details) def _add_nat_details(self, context, port, host, details): # This method needs to define requirements for this Mixin's child @@ -810,16 +843,14 @@ class AIMMappingRPCMixin(ha_ip_db.HAIPOwnerDbMixin): # Child class needs to support: # - self._get_segmentation_labels(context, port, details) - def _add_segmentation_label_details(self, context, port, details, - is_gbp=True): + def _add_segmentation_label_details(self, context, port, details): # This method needs to define requirements for this Mixin's child # classes in order to fill the following result parameters: # - segmentation_labels # apic_segmentation_label is a GBP driver extension configured # for the aim_mapping driver - if is_gbp: - details['segmentation_labels'] = self._get_segmentation_labels( - context, port, details) + details['segmentation_labels'] = self._get_segmentation_labels( + context, port, details) def _add_extra_details(self, context, port, details): # TODO(ivar): Extra details depend on HA and SC implementation diff --git a/gbpservice/neutron/services/grouppolicy/drivers/neutron_resources.py b/gbpservice/neutron/services/grouppolicy/drivers/neutron_resources.py index 9f9cf4f5c..f410cc918 100644 --- a/gbpservice/neutron/services/grouppolicy/drivers/neutron_resources.py +++ b/gbpservice/neutron/services/grouppolicy/drivers/neutron_resources.py @@ -86,12 +86,15 @@ class CommonNeutronBase(ipd.ImplicitPolicyBase, rmd.OwnedResourcesOperations, if pts: return pts[0] - def _port_id_to_ptg(self, plugin_context, port_id): - pt = self._port_id_to_pt(plugin_context, port_id) + def _pt_to_ptg(self, plugin_context, pt): if pt: return self.gbp_plugin.get_policy_target_group( - plugin_context, pt['policy_target_group_id']), pt - return None, None + plugin_context, pt['policy_target_group_id']) + return None + + def _port_id_to_ptg(self, plugin_context, port_id): + pt = self._port_id_to_pt(plugin_context, port_id) + return self._pt_to_ptg(plugin_context, pt), pt def _network_id_to_l2p(self, context, network_id): l2ps = self.gbp_plugin.get_l2_policies( diff --git a/gbpservice/neutron/tests/unit/services/grouppolicy/test_aim_mapping_driver.py b/gbpservice/neutron/tests/unit/services/grouppolicy/test_aim_mapping_driver.py index 2ac5e3767..9fa2910f6 100644 --- a/gbpservice/neutron/tests/unit/services/grouppolicy/test_aim_mapping_driver.py +++ b/gbpservice/neutron/tests/unit/services/grouppolicy/test_aim_mapping_driver.py @@ -3302,7 +3302,17 @@ class TestPolicyTarget(AIMBaseTestCase, fip['nat_epg_name'] = ext_epg_name fip['nat_epg_tenant'] = ext_epg_tenant fip['nat_epg_app_profile'] = ext_epg_app_profile - self.assertEqual(fip, mapping['floating_ip'][0]) + + fip_mapping = mapping['floating_ip'][0] + self.assertEqual(fip['id'], fip_mapping['id']) + self.assertEqual(fip['port_id'], fip_mapping['port_id']) + self.assertEqual(fip['project_id'], fip_mapping['project_id']) + self.assertEqual(fip['fixed_ip_address'], + fip_mapping['fixed_ip_address']) + self.assertEqual(fip['floating_ip_address'], + fip_mapping['floating_ip_address']) + self.assertEqual(fip['floating_network_id'], + fip_mapping['floating_network_id']) def _verify_ip_mapping_details(self, mapping, ext_segment_name, ext_epg_tenant, ext_epg_name, @@ -3323,7 +3333,9 @@ class TestPolicyTarget(AIMBaseTestCase, 'prefixlen': int(prefix)}, mapping['host_snat_ips'][0]) - def _do_test_get_gbp_details(self, pre_vrf=None): + def _do_test_get_gbp_details(self, pre_vrf=None, enable_raw_sql=False): + self.driver.aim_mech_driver.enable_raw_sql_for_device_rpc = ( + enable_raw_sql) self.driver.aim_mech_driver.apic_optimized_dhcp_lease_time = 100 es1, es1_sub = self._setup_external_segment( 'es1', dn='uni/tn-t1/out-l1/instP-n1') @@ -3452,7 +3464,9 @@ class TestPolicyTarget(AIMBaseTestCase, self.assertEqual(2000, mapping['interface_mtu']) def _do_test_gbp_details_no_pt(self, use_as=True, routed=True, - pre_vrf=None): + pre_vrf=None, enable_raw_sql=False): + self.driver.aim_mech_driver.enable_raw_sql_for_device_rpc = ( + enable_raw_sql) # Create port and bind it address_scope = self._make_address_scope_for_vrf( pre_vrf.dn if pre_vrf else None, @@ -3570,6 +3584,9 @@ class TestPolicyTarget(AIMBaseTestCase, def test_get_gbp_details(self): self._do_test_get_gbp_details() + def test_get_gbp_details_with_raw_sql(self): + self._do_test_get_gbp_details(enable_raw_sql=True) + def test_get_gbp_details_pre_existing_vrf(self): aim_ctx = aim_context.AimContext(self.db_session) vrf = self.aim_mgr.create( @@ -3577,11 +3594,23 @@ class TestPolicyTarget(AIMBaseTestCase, monitored=True)) self._do_test_get_gbp_details(pre_vrf=vrf) + def test_get_gbp_details_pre_existing_vrf_with_raw_sql(self): + aim_ctx = aim_context.AimContext(self.db_session) + vrf = self.aim_mgr.create( + aim_ctx, aim_resource.VRF(tenant_name='common', name='ctx1', + monitored=True)) + self._do_test_get_gbp_details(pre_vrf=vrf, enable_raw_sql=True) + def test_get_gbp_details_no_pt(self): # Test that traditional Neutron ports behave correctly from the # RPC perspective self._do_test_gbp_details_no_pt() + def test_get_gbp_details_no_pt_with_raw_sql(self): + # Test that traditional Neutron ports behave correctly from the + # RPC perspective + self._do_test_gbp_details_no_pt(enable_raw_sql=True) + def test_get_gbp_details_no_pt_pre_existing_vrf(self): aim_ctx = aim_context.AimContext(self.db_session) vrf = self.aim_mgr.create( @@ -3589,13 +3618,29 @@ class TestPolicyTarget(AIMBaseTestCase, monitored=True)) self._do_test_gbp_details_no_pt(pre_vrf=vrf) + def test_get_gbp_details_no_pt_pre_existing_vrf_with_raw_sql(self): + aim_ctx = aim_context.AimContext(self.db_session) + vrf = self.aim_mgr.create( + aim_ctx, aim_resource.VRF(tenant_name='common', name='ctx1', + monitored=True)) + self._do_test_gbp_details_no_pt(pre_vrf=vrf, enable_raw_sql=True) + def test_get_gbp_details_no_pt_no_as(self): self._do_test_gbp_details_no_pt(use_as=False) + def test_get_gbp_details_no_pt_no_as_with_raw_sql(self): + self._do_test_gbp_details_no_pt(use_as=False, enable_raw_sql=True) + def test_get_gbp_details_no_pt_no_as_unrouted(self): self._do_test_gbp_details_no_pt(use_as=False, routed=False) - def test_gbp_details_ext_net_no_pt(self): + def test_get_gbp_details_no_pt_no_as_unrouted_with_raw_sql(self): + self._do_test_gbp_details_no_pt(use_as=False, routed=False, + enable_raw_sql=True) + + def _test_gbp_details_ext_net_no_pt(self, enable_raw_sql=False): + self.driver.aim_mech_driver.enable_raw_sql_for_device_rpc = ( + enable_raw_sql) # Test ports created on Neutron external networks ext_net1, _, sn1 = self._setup_external_network( 'l1', dn='uni/tn-common/out-l1/instP-n1') @@ -3674,6 +3719,12 @@ class TestPolicyTarget(AIMBaseTestCase, self._verify_vrf_details_assertions( vrf_mapping, "EXT-l2", vrf_id, supernet, "t1") + def test_gbp_details_ext_net_no_pt(self): + self._test_gbp_details_ext_net_no_pt() + + def test_gbp_details_ext_net_no_pt_with_raw_sql(self): + self._test_gbp_details_ext_net_no_pt(enable_raw_sql=True) + def test_ip_address_owner_update(self): l3p = self.create_l3_policy(name='myl3')['l3_policy'] l2p = self.create_l2_policy(name='myl2',