From 51796eab0cd07be7514fed66506a8e388bdc4204 Mon Sep 17 00:00:00 2001 From: Robert Kukura Date: Sun, 19 Aug 2018 09:50:54 -0400 Subject: [PATCH] [AIM] Repair missing extension data when validating If extension data records for networks or subnets are missing, the gbp-validate tool will now fail, or attempt to repair them if repair is enabled. For external Neutron networks, the DN of the ACI ExternalNetwork must be supplied via the migrate_ext_net_dns configuration option in the ml2_apic_aim group, which is a dictionary keyed by the network_id. Change-Id: I427596b9ea01140af44e9eaf415c82935c668ebe --- .../ml2plus/drivers/apic_aim/config.py | 5 + .../ml2plus/drivers/apic_aim/extension_db.py | 4 + .../drivers/apic_aim/mechanism_driver.py | 137 ++++++++++++++---- .../drivers/cisco/apic/aim_validation.py | 9 +- .../grouppolicy/test_aim_validation.py | 51 ++++++- 5 files changed, 170 insertions(+), 36 deletions(-) diff --git a/gbpservice/neutron/plugins/ml2plus/drivers/apic_aim/config.py b/gbpservice/neutron/plugins/ml2plus/drivers/apic_aim/config.py index f07558bcf..1a560fe5f 100644 --- a/gbpservice/neutron/plugins/ml2plus/drivers/apic_aim/config.py +++ b/gbpservice/neutron/plugins/ml2plus/drivers/apic_aim/config.py @@ -48,6 +48,11 @@ apic_opts = [ cfg.StrOpt('apic_router_id_pool', default='199.199.199.1/24', help=("The pool of IPs where we allocate the APIC " "router ID from while creating the SVI interface.")), + cfg.DictOpt('migrate_ext_net_dns', default={}, + help="DNs for external networks being migrated from legacy " + "plugin, formatted as a dictionary mapping Neutron external " + "network IDs (UUIDs) to ACI external network distinguished " + "names."), ] diff --git a/gbpservice/neutron/plugins/ml2plus/drivers/apic_aim/extension_db.py b/gbpservice/neutron/plugins/ml2plus/drivers/apic_aim/extension_db.py index e8d0b2039..d3f2de37c 100644 --- a/gbpservice/neutron/plugins/ml2plus/drivers/apic_aim/extension_db.py +++ b/gbpservice/neutron/plugins/ml2plus/drivers/apic_aim/extension_db.py @@ -87,6 +87,10 @@ class SubnetExtensionDb(model_base.BASEV2): sa.String(36), sa.ForeignKey('subnets.id', ondelete="CASCADE"), primary_key=True) snat_host_pool = sa.Column(sa.Boolean) + subnet = orm.relationship(models_v2.Subnet, + backref=orm.backref( + 'aim_extension_mapping', lazy='joined', + uselist=False, cascade='delete')) class RouterExtensionContractDb(model_base.BASEV2): 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 808d0ffa5..4627dab03 100644 --- a/gbpservice/neutron/plugins/ml2plus/drivers/apic_aim/mechanism_driver.py +++ b/gbpservice/neutron/plugins/ml2plus/drivers/apic_aim/mechanism_driver.py @@ -29,6 +29,7 @@ from aim.api import infra as aim_infra from aim.api import resource as aim_resource from aim.common import utils from aim import context as aim_context +from aim import exceptions as aim_exceptions from aim import utils as aim_utils from neutron.agent import securitygroups_rpc from neutron.callbacks import events @@ -183,7 +184,8 @@ class KeystoneNotificationEndpoint(object): class ApicMechanismDriver(api_plus.MechanismDriver, - db.DbMixin): + db.DbMixin, + extension_db.ExtensionDbMixin): NIC_NAME_LEN = 14 class TopologyRpcEndpoint(object): @@ -496,10 +498,9 @@ class ApicMechanismDriver(api_plus.MechanismDriver, ns.create_external_network(aim_ctx, ext_net) # Get external CIDRs for all external networks that share # this APIC external network. - ext_db = extension_db.ExtensionDbMixin() cidrs = sorted( - ext_db.get_external_cidrs_by_ext_net_dn(session, ext_net.dn, - lock_update=True)) + self.get_external_cidrs_by_ext_net_dn( + session, ext_net.dn, lock_update=True)) ns.update_external_cidrs(aim_ctx, ext_net, cidrs) for resource in ns.get_l3outside_resources(aim_ctx, l3out): @@ -512,10 +513,9 @@ class ApicMechanismDriver(api_plus.MechanismDriver, elif self._is_svi(current): l3out, ext_net, _ = self._get_aim_external_objects(current) if ext_net: - extn_db = extension_db.ExtensionDbMixin() other_nets = set( - extn_db.get_svi_network_ids_by_l3out_dn(session, l3out.dn, - lock_update=True)) + self.get_svi_network_ids_by_l3out_dn( + session, l3out.dn, lock_update=True)) other_nets.discard(current['id']) if other_nets: raise exceptions.PreExistingSVICannotUseSameL3out() @@ -643,9 +643,8 @@ class ApicMechanismDriver(api_plus.MechanismDriver, if old != new: # Get external CIDRs for all external networks that share # this APIC external network. - ext_db = extension_db.ExtensionDbMixin() cidrs = sorted( - ext_db.get_external_cidrs_by_ext_net_dn( + self.get_external_cidrs_by_ext_net_dn( session, ext_net.dn, lock_update=True)) ns.update_external_cidrs(aim_ctx, ext_net, cidrs) # TODO(amitbose) Propagate name updates to AIM @@ -768,19 +767,18 @@ class ApicMechanismDriver(api_plus.MechanismDriver, l3out, ext_net, ns = self._get_aim_nat_strategy(current) if not ext_net: return # Unmanaged external network - extn_db = extension_db.ExtensionDbMixin() # REVISIT: lock_update=True is needed to handle races. Find # alternative solutions since Neutron discourages using such # queries. other_nets = set( - extn_db.get_network_ids_by_ext_net_dn(session, ext_net.dn, - lock_update=True)) + self.get_network_ids_by_ext_net_dn( + session, ext_net.dn, lock_update=True)) other_nets.discard(current['id']) if not other_nets: ns.delete_external_network(aim_ctx, ext_net) other_nets = set( - extn_db.get_network_ids_by_l3out_dn(session, l3out.dn, - lock_update=True)) + self.get_network_ids_by_l3out_dn( + session, l3out.dn, lock_update=True)) other_nets.discard(current['id']) if not other_nets: ns.delete_l3outside(aim_ctx, l3out) @@ -815,7 +813,6 @@ class ApicMechanismDriver(api_plus.MechanismDriver, def extend_network_dict_bulk(self, session, results, single=False): # Gather db objects aim_ctx = aim_context.AimContext(session) - extn_db = extension_db.ExtensionDbMixin() aim_resources = [] res_dict_by_aim_res_dn = {} @@ -856,9 +853,9 @@ class ApicMechanismDriver(api_plus.MechanismDriver, # put the extension call in Pike+ *before* the precommit # calls happen in network creation. I believe this is a but # and should be discussed with the Neutron team. - ext_dict = extn_db.get_network_extn_db(session, net_db.id) + ext_dict = self.get_network_extn_db(session, net_db.id) else: - ext_dict = extn_db.make_network_extn_db_conf_dict( + ext_dict = self.make_network_extn_db_conf_dict( net_db.aim_extension_mapping, net_db.aim_extension_cidr_mapping, net_db.aim_extension_domain_mapping) @@ -905,10 +902,9 @@ class ApicMechanismDriver(api_plus.MechanismDriver, return # Unmanaged external network # Check subnet overlap with subnets from other Neutron # external networks that map to the same APIC L3Out - ext_db = extension_db.ExtensionDbMixin() other_nets = set( - ext_db.get_network_ids_by_l3out_dn(session, l3out.dn, - lock_update=True)) + self.get_network_ids_by_l3out_dn( + session, l3out.dn, lock_update=True)) other_nets.discard(network_id) cidrs = (session.query(models_v2.Subnet.cidr).filter( models_v2.Subnet.network_id.in_(other_nets)).all()) @@ -3048,8 +3044,7 @@ class ApicMechanismDriver(api_plus.MechanismDriver, return self._get_aim_external_objects(network) def _get_aim_external_objects_db(self, session, network_db): - extn_db = extension_db.ExtensionDbMixin() - extn_info = extn_db.get_network_extn_db(session, network_db.id) + extn_info = self.get_network_extn_db(session, network_db.id) if extn_info and cisco_apic.EXTERNAL_NETWORK in extn_info: dn = extn_info[cisco_apic.EXTERNAL_NETWORK] a_ext_net = aim_resource.ExternalNetwork.from_dn(dn) @@ -3144,7 +3139,6 @@ class ApicMechanismDriver(api_plus.MechanismDriver, # 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() prov = set() cons = set() @@ -3154,7 +3148,7 @@ class ApicMechanismDriver(api_plus.MechanismDriver, prov.add(contract.name) cons.add(contract.name) - r_info = ext_db.get_router_extn_db(session, router['id']) + r_info = self.get_router_extn_db(session, router['id']) prov.update(r_info[a_l3.EXTERNAL_PROVIDED_CONTRACTS]) cons.update(r_info[a_l3.EXTERNAL_CONSUMED_CONTRACTS]) @@ -3162,7 +3156,7 @@ class ApicMechanismDriver(api_plus.MechanismDriver, _, ext_net, ns = self._get_aim_nat_strategy(old_network) if ext_net: # Find Neutron networks that share the APIC external network. - eqv_nets = ext_db.get_network_ids_by_ext_net_dn( + eqv_nets = self.get_network_ids_by_ext_net_dn( session, ext_net.dn, lock_update=True) rtr_old = [r for r in rtr_dbs if (r.gw_port_id and @@ -3182,7 +3176,7 @@ class ApicMechanismDriver(api_plus.MechanismDriver, _, ext_net, ns = self._get_aim_nat_strategy(new_network) if ext_net: # Find Neutron networks that share the APIC external network. - eqv_nets = ext_db.get_network_ids_by_ext_net_dn( + eqv_nets = self.get_network_ids_by_ext_net_dn( session, ext_net.dn, lock_update=True) rtr_new = [r for r in rtr_dbs if (r.gw_port_id and @@ -3339,9 +3333,7 @@ class ApicMechanismDriver(api_plus.MechanismDriver, def check_floatingip_external_address(self, context, floatingip): session = context.session if floatingip.get('subnet_id'): - sn_ext = (extension_db.ExtensionDbMixin() - .get_subnet_extn_db(session, - floatingip['subnet_id'])) + sn_ext = self.get_subnet_extn_db(session, floatingip['subnet_id']) if sn_ext.get(cisco_apic.SNAT_HOST_POOL, False): raise exceptions.SnatPoolCannotBeUsedForFloatingIp() elif floatingip.get('floating_ip_address'): @@ -4084,6 +4076,16 @@ class ApicMechanismDriver(api_plus.MechanismDriver, for resource in mgr.actual_aim_resources(resource_class): mgr.aim_mgr.create(mgr.expected_aim_ctx, resource) + # Copy pre-existing AIM resources for external networking from + # actual to expected AIM stores. + for resource_class in [aim_resource.ExternalNetwork, + aim_resource.ExternalSubnet, + aim_resource.L3Outside, + aim_resource.VRF]: + for resource in mgr.actual_aim_resources(resource_class): + if resource.monitored: + mgr.aim_mgr.create(mgr.expected_aim_ctx, resource) + # Register DB tables to be validated. mgr.register_db_instance_class( aim_lib_model.CloneL3Out, ['tenant_name', 'name']) @@ -4177,8 +4179,13 @@ class ApicMechanismDriver(api_plus.MechanismDriver, mgr, net_dbs, routed_nets) for net_db in net_dbs.values(): + if not net_db.aim_extension_mapping: + self._missing_network_extension_mapping(mgr, net_db) self._expect_project(mgr, net_db.project_id) + for subnet_db in net_db.subnets: + if not subnet_db.aim_extension_mapping: + self._missing_subnet_extension_mapping(mgr, subnet_db) self._expect_project(mgr, subnet_db.project_id) for segment_db in net_db.segments: @@ -4377,6 +4384,73 @@ class ApicMechanismDriver(api_plus.MechanismDriver, return network_vrfs, router_vrfs + def _missing_network_extension_mapping(self, mgr, net_db): + # Note that this is intended primarily to handle migration to + # apic_aim, where the previous plugin and/or drivers did not + # populate apic_aim's extension data. Migration of external + # networks is supported through configuration of ACI + # ExternalNetwork DNs, but other apic_aim-specific features + # such as SVI do not apply to these migration use cases. After + # migration, other attributes can be changed via the REST API + # if needed. + + if not mgr.should_repair( + "network %s missing extension data" % net_db.id): + return + + ext_net_dn = None + if net_db.external: + ext_net_dn = cfg.CONF.ml2_apic_aim.migrate_ext_net_dns.get( + net_db.id) + if not ext_net_dn: + mgr.validation_failed( + "missing extension data for external network %s and no " + "external network DN configured" % net_db.id) + try: + ext_net = aim_resource.ExternalNetwork.from_dn(ext_net_dn) + ext_net = mgr.aim_mgr.get(mgr.expected_aim_ctx, ext_net) + if not ext_net: + mgr.validation_failed( + "missing extension data for external network %(net)s " + "and configured external network DN '%(dn)s' does not " + "exist" % {'net': net_db.id, 'dn': ext_net_dn}) + ext_net_dn = None + except aim_exceptions.InvalidDNForAciResource: + mgr.validation_failed( + "missing extension data for external network %(net)s and " + "configured external network DN '%(dn)s' is invalid" % + {'net': net_db.id, 'dn': ext_net_dn}) + ext_net_dn = None + res_dict = { + cisco_apic.EXTERNAL_NETWORK: ext_net_dn, + cisco_apic.SVI: False, + cisco_apic.BGP: False, + cisco_apic.BGP_TYPE: 'default_export', + cisco_apic.BGP_ASN: 0, + cisco_apic.NESTED_DOMAIN_NAME: '', + cisco_apic.NESTED_DOMAIN_TYPE: '', + cisco_apic.NESTED_DOMAIN_INFRA_VLAN: None, + cisco_apic.NESTED_DOMAIN_SERVICE_VLAN: None, + cisco_apic.NESTED_DOMAIN_NODE_NETWORK_VLAN: None, + } + self.set_network_extn_db(mgr.actual_session, net_db.id, res_dict) + + def _missing_subnet_extension_mapping(self, mgr, subnet_db): + # Note that this is intended primarily to handle migration to + # apic_aim, where the previous plugin and/or drivers did not + # populate apic_aim's extension data. After migration, the + # SNAT_HOST_POOL attribute can be changed via the REST API if + # needed. + + if not mgr.should_repair( + "subnet %s missing extension data" % subnet_db.id): + return + + res_dict = { + cisco_apic.SNAT_HOST_POOL: False + } + self.set_subnet_extn_db(mgr.actual_session, subnet_db.id, res_dict) + def _validate_normal_network(self, mgr, net_db, network_vrfs, router_dbs, routed_nets): routed_vrf = network_vrfs.get(net_db.id) @@ -4481,8 +4555,7 @@ class ApicMechanismDriver(api_plus.MechanismDriver, # Get external CIDRs for all external networks that share this # APIC external network. - ext_db = extension_db.ExtensionDbMixin() - cidrs = sorted(ext_db.get_external_cidrs_by_ext_net_dn( + cidrs = sorted(self.get_external_cidrs_by_ext_net_dn( mgr.actual_session, ext_net.dn, lock_update=False)) ns.update_external_cidrs(mgr.expected_aim_ctx, ext_net, cidrs) for resource in ns.get_l3outside_resources( @@ -4501,7 +4574,7 @@ class ApicMechanismDriver(api_plus.MechanismDriver, # REVISIT: Process each AIM ExternalNetwork rather than each # external Neutron network? - eqv_net_ids = ext_db.get_network_ids_by_ext_net_dn( + eqv_net_ids = self.get_network_ids_by_ext_net_dn( mgr.actual_session, ext_net.dn, lock_update=False) router_ids = set() for eqv_net_id in eqv_net_ids: diff --git a/gbpservice/neutron/services/grouppolicy/drivers/cisco/apic/aim_validation.py b/gbpservice/neutron/services/grouppolicy/drivers/cisco/apic/aim_validation.py index 8136cd74a..f4f02b662 100644 --- a/gbpservice/neutron/services/grouppolicy/drivers/cisco/apic/aim_validation.py +++ b/gbpservice/neutron/services/grouppolicy/drivers/cisco/apic/aim_validation.py @@ -132,10 +132,13 @@ class ValidationManager(object): for resource in self.aim_mgr.find( self.actual_aim_ctx, resource_class)} - def expect_aim_resource(self, resource, replace=False): + def expect_aim_resource(self, resource, replace=False, remove=False): expected_resources = self._expected_aim_resources[resource.__class__] key = tuple(resource.identity) - if not replace and key in expected_resources: + if remove: + del expected_resources[key] + return + elif not replace and key in expected_resources: print("resource %s already expected" % resource) raise InternalValidationError() for attr_name, attr_type in resource.other_attributes.items(): @@ -339,7 +342,7 @@ class ValidationAimStore(aim_store.AimStore): self._mgr.expect_aim_resource(db_obj, True) def delete(self, db_obj): - assert(False) + self._mgr.expect_aim_resource(db_obj, remove=True) def query(self, db_obj_type, resource_class, in_=None, notin_=None, order_by=None, lock_update=False, **filters): diff --git a/gbpservice/neutron/tests/unit/services/grouppolicy/test_aim_validation.py b/gbpservice/neutron/tests/unit/services/grouppolicy/test_aim_validation.py index 3da82f48f..c756de715 100644 --- a/gbpservice/neutron/tests/unit/services/grouppolicy/test_aim_validation.py +++ b/gbpservice/neutron/tests/unit/services/grouppolicy/test_aim_validation.py @@ -23,8 +23,11 @@ from neutron.db.models import segment from neutron.plugins.ml2 import models as ml2_models from neutron.tests.unit.extensions import test_securitygroup from neutron_lib import context as n_context +from oslo_config import cfg from gbpservice.neutron.db.grouppolicy import group_policy_db as gpdb +from gbpservice.neutron.plugins.ml2plus.drivers.apic_aim import ( + extension_db as ext_db) from gbpservice.neutron.plugins.ml2plus.drivers.apic_aim import db from gbpservice.neutron.services.grouppolicy import ( group_policy_driver_api as api) @@ -298,6 +301,7 @@ class TestNeutronMapping(AimValidationTestCase): # Create unrouted subnet. subnet = self._make_subnet( self.fmt, net_resp, '10.0.2.1', '10.0.2.0/24')['subnet'] + subnet_id = subnet['id'] self._validate() # Delete the network's mapping record and test. @@ -362,14 +366,29 @@ class TestNeutronMapping(AimValidationTestCase): gw_ip_mask='10.0.2.1/24') self._test_aim_resource(sn, 'gw_ip_mask', '10.0.3.1/24') + # Delete subnet extension data and test migration use case. + (self.db_session.query(ext_db.SubnetExtensionDb). + filter_by(subnet_id=subnet_id). + delete()) + self._validate_repair_validate() + def test_unrouted_network(self): # Create network. net_resp = self._make_network(self.fmt, 'net1', True) + net = net_resp['network'] + net_id = net['id'] self._validate() # Test AIM resources. self._test_network_resources(net_resp) + # Delete network extension data and test migration use case. + (self.db_session.query(ext_db.NetworkExtensionDb). + filter_by(network_id=net_id). + delete()) + self._validate_repair_validate() + self._test_network_dns(net) + def _test_external_network(self, vrf_name='openstack_EXT-l1'): # Create AIM HostDomainMappingV2. hd_mapping = aim_infra.HostDomainMappingV2( @@ -440,6 +459,8 @@ class TestNeutronMapping(AimValidationTestCase): tenant_name='common', filter_name='openstack_EXT-l1', name='Any') self._test_aim_resource(entry) + return net + def test_external_network(self): self._test_external_network() @@ -464,7 +485,35 @@ class TestNeutronMapping(AimValidationTestCase): cidr='0.0.0.0/0', monitored=True) self.aim_mgr.create(self.aim_ctx, ext_sn) - self._test_external_network(vrf_name='v1') + # Run tests. + net = self._test_external_network(vrf_name='v1') + net_id = net['id'] + + # Delete network extension data to test migration use case. + (self.db_session.query(ext_db.NetworkExtensionDb). + filter_by(network_id=net_id). + delete()) + + # Test without DN for migration. + self._validate_unrepairable() + + # Configure invalid DN for migration and test. + cfg.CONF.set_override( + 'migrate_ext_net_dns', {net_id: 'abcdef'}, group='ml2_apic_aim') + self._validate_unrepairable() + + # Configure non-existent DN for migration and test. + cfg.CONF.set_override( + 'migrate_ext_net_dns', {net_id: 'uni/tn-common/out-l9/instP-n1'}, + group='ml2_apic_aim') + self._validate_unrepairable() + + # Configure correct DN for migration and test. + cfg.CONF.set_override( + 'migrate_ext_net_dns', {net_id: 'uni/tn-common/out-l1/instP-n1'}, + group='ml2_apic_aim') + self._validate_repair_validate() + self._test_network_dns(net) def test_svi_network(self): # REVISIT: Test validation of actual mapping once implemented.