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 50e79f69d..1a6fe7857 100644 --- a/gbpservice/neutron/plugins/ml2plus/drivers/apic_aim/mechanism_driver.py +++ b/gbpservice/neutron/plugins/ml2plus/drivers/apic_aim/mechanism_driver.py @@ -112,6 +112,11 @@ NO_ADDR_SCOPE = object() DVS_AGENT_KLASS = 'networking_vsphere.common.dvs_agent_rpc_api.DVSClientAPI' DEFAULT_HOST_DOMAIN = '*' +LEGACY_SNAT_NET_NAME_PREFIX = 'host-snat-network-for-internal-use-' +LEGACY_SNAT_SUBNET_NAME = 'host-snat-pool-for-internal-use' +LEGACY_SNAT_PORT_NAME = 'host-snat-pool-for-internal-use' +LEGACY_SNAT_PORT_DEVICE_OWNER = 'host-snat-pool-port-device-owner-internal-use' + # TODO(kentwu): Move this to AIM utils maybe to avoid adding too much # APIC logic to the mechanism driver ACI_CHASSIS_DESCR_STRING = 'topology/pod-%s/node-%s' @@ -972,10 +977,12 @@ class ApicMechanismDriver(api_plus.MechanismDriver, network_db) if not ext_net: return # Unmanaged external network - ns.delete_subnet(aim_ctx, l3out, - self._subnet_to_gw_ip_mask(original)) - ns.create_subnet(aim_ctx, l3out, - self._subnet_to_gw_ip_mask(current)) + if original['gateway_ip']: + ns.delete_subnet(aim_ctx, l3out, + self._subnet_to_gw_ip_mask(original)) + if current['gateway_ip']: + ns.create_subnet(aim_ctx, l3out, + self._subnet_to_gw_ip_mask(current)) def delete_subnet_precommit(self, context): current = context.current @@ -3066,8 +3073,9 @@ class ApicMechanismDriver(api_plus.MechanismDriver, return None, None, None def _subnet_to_gw_ip_mask(self, subnet): + cidr = subnet['cidr'].split('/') return aim_resource.Subnet.to_gw_ip_mask( - subnet['gateway_ip'], int(subnet['cidr'].split('/')[1])) + subnet['gateway_ip'] or cidr[0], int(cidr[1])) def _get_router_intf_count(self, session, router, scope_id=None): if not scope_id: @@ -4051,6 +4059,10 @@ class ApicMechanismDriver(api_plus.MechanismDriver, network_id=mapping.network_id) def validate_aim_mapping(self, mgr): + # First do any cleanup and/or migration of Neutron resources + # used internally by the legacy plugins. + self._validate_legacy_resources(mgr) + # Register all AIM resource types used by mapping. mgr.register_aim_resource_class(aim_infra.HostDomainMappingV2) mgr.register_aim_resource_class(aim_resource.ApplicationProfile) @@ -4119,6 +4131,90 @@ class ApicMechanismDriver(api_plus.MechanismDriver, self._validate_floatingips(mgr) self._validate_port_bindings(mgr) + def _validate_legacy_resources(self, mgr): + # Delete legacy SNAT ports. + for port_id, in ( + mgr.actual_session.query(models_v2.Port.id). + filter_by( + name=LEGACY_SNAT_PORT_NAME, + device_owner=LEGACY_SNAT_PORT_DEVICE_OWNER)): + if mgr.should_repair( + "legacy APIC driver SNAT port %s" % port_id, "Deleting"): + try: + self.plugin.delete_port(mgr.actual_context, port_id) + except n_exceptions.NeutronException as exc: + mgr.validation_failed( + "deleting legacy APIC driver SNAT port %s failed " + "with %s" % (port_id, exc)) + + # Delete legacy SNAT subnets. + for subnet_id, in ( + mgr.actual_session.query(models_v2.Subnet.id). + filter_by(name=LEGACY_SNAT_SUBNET_NAME)): + subnet = self.plugin.get_subnet(mgr.actual_context, subnet_id) + net = self.plugin.get_network( + mgr.actual_context, subnet['network_id']) + net_name = net['name'] + if net_name and net_name.startswith(LEGACY_SNAT_NET_NAME_PREFIX): + ext_net_id = net_name[len(LEGACY_SNAT_NET_NAME_PREFIX):] + ext_net = (mgr.actual_session.query(models_v2.Network). + filter_by(id=ext_net_id). + one_or_none()) + if ext_net and ext_net.external: + if mgr.should_repair( + "legacy APIC driver SNAT subnet %s" % + subnet['cidr'], + "Migrating"): + try: + del subnet['id'] + del subnet['project_id'] + subnet['tenant_id'] = ext_net.project_id + subnet['network_id'] = ext_net.id + subnet['name'] = 'SNAT host pool' + subnet[cisco_apic.SNAT_HOST_POOL] = True + subnet = self.plugin.create_subnet( + mgr.actual_context, {'subnet': subnet}) + except n_exceptions.NeutronException as exc: + mgr.validation_failed( + "Migrating legacy APIC driver SNAT subnet %s " + "failed with %s" % (subnet['cidr'], exc)) + if mgr.should_repair( + "legacy APIC driver SNAT subnet %s" % subnet_id, + "Deleting"): + try: + self.plugin.delete_subnet(mgr.actual_context, subnet_id) + except n_exceptions.NeutronException as exc: + mgr.validation_failed( + "deleting legacy APIC driver SNAT subnet %s failed " + "with %s" % (subnet_id, exc)) + + # Delete legacy SNAT networks. + for net_id, in ( + mgr.actual_session.query(models_v2.Network.id). + filter(models_v2.Network.name.startswith( + LEGACY_SNAT_NET_NAME_PREFIX))): + if mgr.should_repair( + "legacy APIC driver SNAT network %s" % net_id, + "Deleting"): + try: + self.plugin.delete_network(mgr.actual_context, net_id) + except n_exceptions.NeutronException as exc: + mgr.validation_failed( + "deleting legacy APIC driver SNAT network %s failed " + "with %s" % (net_id, exc)) + + # REVISIT: Without this expunge_all call, the + # test_legacy_cleanup UT intermittently fails with the + # subsequent validation steps attempting to repair missing + # subnet extension data, changing the apic:snat_host_pool + # value of the migrated SNAT subnet from True to False. The + # way the extension_db module creates the SubnetExtensionDb + # instance during create_subnet is apparently not updating the + # relationship from a cached Subnet instance. Until this issue + # is understood and resolved, we expunge all instances from + # the session before proceeding. + mgr.actual_session.expunge_all() + def _validate_static_resources(self, mgr): self._ensure_common_tenant(mgr.expected_aim_ctx) self._ensure_unrouted_vrf(mgr.expected_aim_ctx) @@ -4594,9 +4690,10 @@ class ApicMechanismDriver(api_plus.MechanismDriver, vrf = resource for subnet_db in net_db.subnets: - ns.create_subnet( - mgr.expected_aim_ctx, l3out, - self._subnet_to_gw_ip_mask(subnet_db)) + if subnet_db.gateway_ip: + ns.create_subnet( + mgr.expected_aim_ctx, l3out, + self._subnet_to_gw_ip_mask(subnet_db)) # REVISIT: Process each AIM ExternalNetwork rather than each # external Neutron network? @@ -4777,14 +4874,14 @@ class ApicMechanismDriver(api_plus.MechanismDriver, # REVISIT: Deal with distributed port bindings? Also, consider # moving this to the ML2Plus plugin or to a base validation # manager, as it is not specific to this mechanism driver. - plugin_context = nctx.get_admin_context() for port_id, in (mgr.actual_session.query(models.PortBinding.port_id). filter(models.PortBinding.host != '', models.PortBinding.vif_type == portbindings.VIF_TYPE_UNBOUND)): # REVISIT: Use the more efficient get_bound_port_contexts, # which is not available in stable/newton? - pc = self.plugin.get_bound_port_context(plugin_context, port_id) + pc = self.plugin.get_bound_port_context( + mgr.actual_context, port_id) if (pc.vif_type == portbindings.VIF_TYPE_BINDING_FAILED or pc.vif_type == portbindings.VIF_TYPE_UNBOUND): mgr.validation_failed( 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 35b4329eb..8ff49f250 100644 --- a/gbpservice/neutron/services/grouppolicy/drivers/cisco/apic/aim_validation.py +++ b/gbpservice/neutron/services/grouppolicy/drivers/cisco/apic/aim_validation.py @@ -20,6 +20,7 @@ from aim import aim_store from aim.api import resource as aim_resource from aim import context as aim_context from neutron.db import api as db_api +from neutron_lib import context from neutron_lib.plugins import directory from oslo_log import log @@ -33,6 +34,10 @@ class InternalValidationError(Exception): pass +class RollbackTransaction(Exception): + pass + + class ValidationManager(object): def __init__(self): @@ -63,56 +68,60 @@ class ValidationManager(object): # Start transaction. # # REVISIT: Set session's isolation level to serializable? - self.actual_session = (db_api.get_writer_session() if repair - else db_api.get_reader_session()) - self.actual_session.begin() - self.aim_mgr = self.md.aim - self.actual_aim_ctx = aim_context.AimContext(self.actual_session) - self.expected_session = ValidationSession(self) - self.expected_aim_ctx = aim_context.AimContext( - None, ValidationAimStore(self)) + self.actual_context = context.get_admin_context() + try: + with db_api.context_manager.writer.using( + self.actual_context) as session: + self.actual_session = session + self.aim_mgr = self.md.aim + self.actual_aim_ctx = aim_context.AimContext(session) + self.expected_session = ValidationSession(self) + self.expected_aim_ctx = aim_context.AimContext( + None, ValidationAimStore(self)) - # Validate & repair GBP->Neutron mappings. - if self.pd: - self.pd.validate_neutron_mapping(self) + # Validate & repair GBP->Neutron mappings. + if self.pd: + self.pd.validate_neutron_mapping(self) - # Start with no expected or actual AIM resources or DB records. - self._expected_aim_resources = {} - self._actual_aim_resources = {} - self._expected_db_instances = {} - self._db_instance_primary_keys = {} + # Start with no expected or actual AIM resources or DB + # records. + self._expected_aim_resources = {} + self._actual_aim_resources = {} + self._expected_db_instances = {} + self._db_instance_primary_keys = {} - # Validate Neutron->AIM mapping, getting expected AIM - # resources and DB records. - self.md.validate_aim_mapping(self) + # Validate Neutron->AIM mapping, getting expected AIM + # resources and DB records. + self.md.validate_aim_mapping(self) - # Validate GBP->AIM mapping, getting expected AIM resources - # and DB records. - if self.pd: - self.pd.validate_aim_mapping(self) + # Validate GBP->AIM mapping, getting expected AIM + # resources and DB records. + if self.pd: + self.pd.validate_aim_mapping(self) - # Validate SFC->AIM mapping, getting expected AIM resources - # and DB records. - if self.sfcd: - self.sfcd.validate_aim_mapping(self) + # Validate SFC->AIM mapping, getting expected AIM + # resources and DB records. + if self.sfcd: + self.sfcd.validate_aim_mapping(self) - # Validate that actual AIM resources match expected AIM - # resources. - self._validate_aim_resources() + # Validate that actual AIM resources match expected + # AIM resources. + self._validate_aim_resources() - # Validate that actual DB instances match expected DB - # instances. - self._validate_db_instances() + # Validate that actual DB instances match expected DB + # instances. + self._validate_db_instances() - # Commit or rollback transaction. - if self.result is api.VALIDATION_REPAIRED: - self.output("Committing repairs") - self.actual_session.commit() - else: - if (self.repair and - self.result is api.VALIDATION_FAILED_UNREPAIRABLE): - self.output("Rolling back attempted repairs") - self.actual_session.rollback() + # Commit or rollback transaction. + if self.result is api.VALIDATION_REPAIRED: + self.output("Committing repairs") + else: + if (self.repair and + self.result is api.VALIDATION_FAILED_UNREPAIRABLE): + self.output("Rolling back attempted repairs") + raise RollbackTransaction() + except RollbackTransaction: + pass # Bind unbound ports outside transaction. if (self.repair and 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 71fae29b9..10c08fda0 100644 --- a/gbpservice/neutron/tests/unit/plugins/ml2plus/test_apic_aim.py +++ b/gbpservice/neutron/tests/unit/plugins/ml2plus/test_apic_aim.py @@ -5128,6 +5128,29 @@ class TestExternalConnectivityBase(object): self._check_dn(subnet, ext_sub, 'Subnet') self._validate() + # Update gateway to None + self.mock_ns.reset_mock() + self._update('subnets', subnet['id'], + {'subnet': {'gateway_ip': None}}) + subnet = self._show('subnets', subnet['id'])['subnet'] + self.mock_ns.delete_subnet.assert_called_once_with( + mock.ANY, l3out, '10.0.0.251/24') + self.mock_ns.create_subnet.assert_not_called() + self._check_no_dn(subnet, 'Subnet') + self._validate() + + # Update gateway from None + self.mock_ns.reset_mock() + ext_sub.gw_ip_mask = '10.0.0.251/24' + self._update('subnets', subnet['id'], + {'subnet': {'gateway_ip': '10.0.0.251'}}) + subnet = self._show('subnets', subnet['id'])['subnet'] + self.mock_ns.delete_subnet.assert_not_called() + self.mock_ns.create_subnet.assert_called_once_with( + mock.ANY, l3out, '10.0.0.251/24') + self._check_dn(subnet, ext_sub, 'Subnet') + self._validate() + # delete subnet self.mock_ns.reset_mock() self._delete('subnets', subnet['id']) 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 db906e255..32d03f92b 100644 --- a/gbpservice/neutron/tests/unit/services/grouppolicy/test_aim_validation.py +++ b/gbpservice/neutron/tests/unit/services/grouppolicy/test_aim_validation.py @@ -25,6 +25,7 @@ from neutron.tests.unit.extensions import test_securitygroup from neutron_lib import constants as n_constants from neutron_lib import context as n_context from oslo_config import cfg +import webob.exc from gbpservice.neutron.db.grouppolicy import group_policy_db as gpdb from gbpservice.neutron.plugins.ml2plus.drivers.apic_aim import ( @@ -863,6 +864,61 @@ class TestNeutronMapping(AimValidationTestCase): port_id=port['id']).update({'host': 'yyy'}) self._validate_unrepairable() + def test_legacy_cleanup(self): + # Create external network. + kwargs = {'router:external': True, + 'apic:distinguished_names': + {'ExternalNetwork': 'uni/tn-common/out-l1/instP-n1'}} + ext_net = self._make_network( + self.fmt, 'ext_net', True, tenant_id='tenant_1', + arg_list=self.extension_attributes, **kwargs)['network'] + ext_net_id = ext_net['id'] + + # Create legacy plugin's SNAT-related Neutron network. + net_resp = self._make_network( + self.fmt, + 'host-snat-network-for-internal-use-' + ext_net_id, False) + net = net_resp['network'] + net_id = net['id'] + + # Create legacy plugin's SNAT-related Neutron subnet. + subnet = self._make_subnet( + self.fmt, net_resp, '66.66.66.1', '66.66.66.0/24', + enable_dhcp=False)['subnet'] + subnet_id = subnet['id'] + data = {'subnet': {'name': 'host-snat-pool-for-internal-use'}} + subnet = self._update('subnets', subnet_id, data)['subnet'] + + # Create legacy plugin's SNAT-related Neutron port. + fixed_ips = [{'subnet_id': subnet_id, 'ip_address': '66.66.66.5'}] + port = self._make_port( + self.fmt, net_id, fixed_ips=fixed_ips, + name='host-snat-pool-for-internal-use', + device_owner='host-snat-pool-port-device-owner-internal-use' + )['port'] + port_id = port['id'] + + # Test cleanup of these resources. + self._validate_repair_validate() + self._show( + 'ports', port_id, expected_code=webob.exc.HTTPNotFound.code) + self._show( + 'subnets', subnet_id, expected_code=webob.exc.HTTPNotFound.code) + self._show( + 'networks', net_id, expected_code=webob.exc.HTTPNotFound.code) + + # Ensure new SNAT subnet was properly created on actual + # external network. + ext_subnets = self._show('networks', ext_net_id)['network']['subnets'] + self.assertEqual(1, len(ext_subnets)) + ext_subnet = self._show('subnets', ext_subnets[0])['subnet'] + self.assertEqual(subnet['cidr'], ext_subnet['cidr']) + self.assertEqual(subnet['gateway_ip'], ext_subnet['gateway_ip']) + self.assertEqual(subnet['enable_dhcp'], ext_subnet['enable_dhcp']) + self.assertEqual('SNAT host pool', ext_subnet['name']) + self.assertTrue(ext_subnet['apic:snat_host_pool']) + self.assertEqual(ext_net['project_id'], ext_subnet['project_id']) + class TestGbpMapping(AimValidationTestCase):