From 27ff0d4fa6599a8ecd70ea8e387867ddc664c630 Mon Sep 17 00:00:00 2001 From: Robert Kukura Date: Thu, 21 Feb 2019 11:03:03 -0500 Subject: [PATCH] [AIM] Move code from policy driver to mechanism driver Move the HAIP model class, its UTs, and the ip_address_owner_update RPC implementation, as well as the nova_client module, from the aim_mapping policy driver to the apic_aim mechanism driver. Although we don't test or support such configurations, this change should allow Neutron workflows, including HAIP/AAP, to function without having the GBP service plugin or its aim_mapping policy driver configured. The aim_mapping PD code still needs to be present due to some config items needed by the MD being defined in the PD's config module. Change-Id: Ie1798dbe07d9da8206a202ab15c4982342855823 --- gbpservice/neutron/db/all_models.py | 3 - .../plugins/ml2plus/drivers/apic_aim/db.py | 154 +++++++++++++++ .../drivers/apic_aim/mechanism_driver.py | 14 +- .../ml2plus/drivers/apic_aim}/nova_client.py | 0 .../plugins/ml2plus/drivers/apic_aim/rpc.py | 27 +-- .../drivers/cisco/apic/aim_mapping.py | 5 +- .../drivers/cisco/apic/aim_mapping_rpc.py | 9 +- .../cisco/apic/port_ha_ipaddress_binding.py | 180 ------------------ .../unit/plugins/ml2plus/test_apic_aim.py | 45 ++++- .../ml2plus/test_db_apic_aim.py} | 30 ++- .../grouppolicy/test_aim_mapping_driver.py | 28 +-- 11 files changed, 244 insertions(+), 251 deletions(-) rename gbpservice/neutron/{services/grouppolicy/drivers/cisco/apic => plugins/ml2plus/drivers/apic_aim}/nova_client.py (100%) delete mode 100644 gbpservice/neutron/services/grouppolicy/drivers/cisco/apic/port_ha_ipaddress_binding.py rename gbpservice/neutron/tests/unit/{db/test_port_ha_ipaddress_binding.py => plugins/ml2plus/test_db_apic_aim.py} (91%) diff --git a/gbpservice/neutron/db/all_models.py b/gbpservice/neutron/db/all_models.py index 59f20c5ab..3ffb1af70 100644 --- a/gbpservice/neutron/db/all_models.py +++ b/gbpservice/neutron/db/all_models.py @@ -45,9 +45,6 @@ from gbpservice.neutron.services.grouppolicy.drivers import ( # noqa nsp_manager, resource_mapping ) -from gbpservice.neutron.services.grouppolicy.drivers.cisco.apic import ( # noqa - port_ha_ipaddress_binding -) from gbpservice.neutron.services.servicechain.plugins.ncp import ( # noqa model ) diff --git a/gbpservice/neutron/plugins/ml2plus/drivers/apic_aim/db.py b/gbpservice/neutron/plugins/ml2plus/drivers/apic_aim/db.py index bf2873a2d..3b48c43b1 100644 --- a/gbpservice/neutron/plugins/ml2plus/drivers/apic_aim/db.py +++ b/gbpservice/neutron/plugins/ml2plus/drivers/apic_aim/db.py @@ -14,9 +14,12 @@ # under the License. from aim.api import resource as aim_resource +from neutron.db import api as db_api from neutron.db.models import address_scope as as_db from neutron.db import models_v2 +from neutron_lib import context as n_context from neutron_lib.db import model_base +from oslo_db import exception as db_exc from oslo_log import log import sqlalchemy as sa from sqlalchemy.ext import baked @@ -80,6 +83,24 @@ class NetworkMapping(model_base.BASEV2): vrf_tenant_name = sa.Column(sa.String(64)) +class HAIPAddressToPortAssociation(model_base.BASEV2): + + """Port Owner for HA IP Address. + + This table is used to store the mapping between the HA IP Address + and the Port ID of the Neutron Port which currently owns this + IP Address. + """ + + __tablename__ = 'apic_ml2_ha_ipaddress_to_port_owner' + + ha_ip_address = sa.Column(sa.String(64), nullable=False, + primary_key=True) + port_id = sa.Column(sa.String(64), sa.ForeignKey('ports.id', + ondelete='CASCADE'), + nullable=False, primary_key=True) + + class VMName(model_base.BASEV2): __tablename__ = 'apic_aim_vm_names' @@ -99,6 +120,9 @@ class VMNameUpdate(model_base.BASEV2): class DbMixin(object): + + # AddressScopeMapping functions. + def _add_address_scope_mapping(self, session, scope_id, vrf, vrf_owned=True, update_scope=True): mapping = AddressScopeMapping( @@ -163,6 +187,8 @@ class DbMixin(object): tenant_name=mapping.vrf_tenant_name, name=mapping.vrf_name) + # NetworkMapping functions. + def _add_network_mapping(self, session, network_id, bd, epg, vrf, ext_net=None, update_network=True): if not ext_net: @@ -318,6 +344,132 @@ class DbMixin(object): mapping.vrf_tenant_name = vrf.tenant_name mapping.vrf_name = vrf.name + # HAIPAddressToPortAssociation functions. + + def _get_ha_ipaddress(self, port_id, ipaddress, session=None): + session = session or db_api.get_reader_session() + + query = BAKERY(lambda s: s.query( + HAIPAddressToPortAssociation)) + query += lambda q: q.filter_by( + port_id=sa.bindparam('port_id'), + ha_ip_address=sa.bindparam('ipaddress')) + return query(session).params( + port_id=port_id, ipaddress=ipaddress).first() + + def get_port_for_ha_ipaddress(self, ipaddress, network_id, + session=None): + """Returns the Neutron Port ID for the HA IP Addresss.""" + session = session or db_api.get_reader_session() + query = BAKERY(lambda s: s.query( + HAIPAddressToPortAssociation)) + query += lambda q: q.join( + models_v2.Port, + models_v2.Port.id == HAIPAddressToPortAssociation.port_id) + query += lambda q: q.filter( + HAIPAddressToPortAssociation.ha_ip_address == + sa.bindparam('ipaddress')) + query += lambda q: q.filter( + models_v2.Port.network_id == sa.bindparam('network_id')) + port_ha_ip = query(session).params( + ipaddress=ipaddress, network_id=network_id).first() + return port_ha_ip + + def get_ha_ipaddresses_for_port(self, port_id, session=None): + """Returns the HA IP Addressses associated with a Port.""" + session = session or db_api.get_reader_session() + + query = BAKERY(lambda s: s.query( + HAIPAddressToPortAssociation)) + query += lambda q: q.filter_by( + port_id=sa.bindparam('port_id')) + objs = query(session).params( + port_id=port_id).all() + + # REVISIT: Do the sorting in the UT? + return sorted([x['ha_ip_address'] for x in objs]) + + def set_port_id_for_ha_ipaddress(self, port_id, ipaddress, session=None): + """Stores a Neutron Port Id as owner of HA IP Addr (idempotent API).""" + session = session or db_api.get_writer_session() + try: + with session.begin(subtransactions=True): + obj = self._get_ha_ipaddress(port_id, ipaddress, session) + if obj: + return obj + else: + obj = HAIPAddressToPortAssociation( + port_id=port_id, ha_ip_address=ipaddress) + session.add(obj) + return obj + except db_exc.DBDuplicateEntry: + LOG.debug('Duplicate IP ownership entry for tuple %s', + (port_id, ipaddress)) + + def delete_port_id_for_ha_ipaddress(self, port_id, ipaddress, + session=None): + session = session or db_api.get_writer_session() + with session.begin(subtransactions=True): + try: + # REVISIT: Can this query be baked? The + # sqlalchemy.ext.baked.Result class does not have a + # delete() method, and adding delete() to the baked + # query before executing it seems to result in the + # params() not being evaluated. + return session.query( + HAIPAddressToPortAssociation).filter_by( + port_id=port_id, + ha_ip_address=ipaddress).delete() + except orm.exc.NoResultFound: + return + + def get_ha_port_associations(self): + session = db_api.get_reader_session() + + query = BAKERY(lambda s: s.query( + HAIPAddressToPortAssociation)) + return query(session).all() + + # REVISIT: Move this method to the mechanism_driver or rpc module, + # as it is above the DB level. This will also require some rework + # of its unit tests. + def update_ip_owner(self, ip_owner_info): + ports_to_update = set() + port_id = ip_owner_info.get('port') + ipv4 = ip_owner_info.get('ip_address_v4') + ipv6 = ip_owner_info.get('ip_address_v6') + network_id = ip_owner_info.get('network_id') + if not port_id or (not ipv4 and not ipv6): + return ports_to_update + LOG.debug("Got IP owner update: %s", ip_owner_info) + # REVISIT: Just use SQLAlchemy session and models_v2.Port? + port = self.plugin.get_port(n_context.get_admin_context(), port_id) + if not port: + LOG.debug("Ignoring update for non-existent port: %s", port_id) + return ports_to_update + ports_to_update.add(port_id) + for ipa in [ipv4, ipv6]: + if not ipa: + continue + try: + # REVISIT: Why isn't this a single transaction at the + # top-level, so that the port itself is guaranteed to + # still exist. + session = db_api.get_writer_session() + with session.begin(subtransactions=True): + old_owner = self.get_port_for_ha_ipaddress( + ipa, network_id or port['network_id'], session=session) + self.set_port_id_for_ha_ipaddress(port_id, ipa, session) + if old_owner and old_owner['port_id'] != port_id: + self.delete_port_id_for_ha_ipaddress( + old_owner['port_id'], ipa, session=session) + ports_to_update.add(old_owner['port_id']) + except db_exc.DBReferenceError as dbe: + LOG.debug("Ignoring FK error for port %s: %s", port_id, dbe) + return ports_to_update + + # VMName functions. + def _get_vm_name(self, session, device_id, is_detailed=False): if is_detailed: query = BAKERY(lambda s: s.query(VMName)) @@ -350,6 +502,8 @@ class DbMixin(object): if db_obj: session.delete(db_obj) + # VMNameUpdate functions. + def _get_vm_name_update(self, session): query = BAKERY(lambda s: s.query(VMNameUpdate)) return query(session).one_or_none() 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 5368addf9..06d3f905e 100644 --- a/gbpservice/neutron/plugins/ml2plus/drivers/apic_aim/mechanism_driver.py +++ b/gbpservice/neutron/plugins/ml2plus/drivers/apic_aim/mechanism_driver.py @@ -83,10 +83,9 @@ from gbpservice.neutron.plugins.ml2plus.drivers.apic_aim import config # noqa from gbpservice.neutron.plugins.ml2plus.drivers.apic_aim import db from gbpservice.neutron.plugins.ml2plus.drivers.apic_aim import exceptions from gbpservice.neutron.plugins.ml2plus.drivers.apic_aim import extension_db +from gbpservice.neutron.plugins.ml2plus.drivers.apic_aim import nova_client from gbpservice.neutron.plugins.ml2plus.drivers.apic_aim import rpc from gbpservice.neutron.plugins.ml2plus.drivers.apic_aim import trunk_driver -from gbpservice.neutron.services.grouppolicy.drivers.cisco.apic import ( - nova_client as nclient) # REVISIT: We need the aim_mapping policy driver's config until # advertise_mtu and nested_host_vlan are moved to the mechanism @@ -289,7 +288,7 @@ class ApicMechanismDriver(api_plus.MechanismDriver, self.apic_nova_vm_name_cache_update_interval * 10): is_full_update = False - nova_vms = nclient.NovaClient().get_servers( + nova_vms = nova_client.NovaClient().get_servers( is_full_update, self.apic_nova_vm_name_cache_update_interval * 10) # This means Nova API has thrown an exception if nova_vms is None: @@ -2091,8 +2090,6 @@ class ApicMechanismDriver(api_plus.MechanismDriver, provisioning_blocks.L2_AGENT_ENTITY) def _check_allowed_address_pairs(self, context, port): - if not self.gbp_driver: - return aap_current = context.current.get('allowed_address_pairs', []) aap_original = context.original.get('allowed_address_pairs', []) # If there was a change in configured AAPs, then we may need @@ -2108,12 +2105,11 @@ class ApicMechanismDriver(api_plus.MechanismDriver, # Get all the owned IP addresses for the port, and if # they match a removed AAP entry, delete that entry # from the DB - ha_handler = self.gbp_driver.ha_ip_handler - ha_ips = ha_handler.get_ha_ipaddresses_for_port(port['id'], - session=session) + ha_ips = self.get_ha_ipaddresses_for_port( + port['id'], session=session) for ip in ha_ips: if ip in cidr: - ha_handler.delete_port_id_for_ha_ipaddress( + self.delete_port_id_for_ha_ipaddress( port['id'], ip, session=session) def update_port_precommit(self, context): diff --git a/gbpservice/neutron/services/grouppolicy/drivers/cisco/apic/nova_client.py b/gbpservice/neutron/plugins/ml2plus/drivers/apic_aim/nova_client.py similarity index 100% rename from gbpservice/neutron/services/grouppolicy/drivers/cisco/apic/nova_client.py rename to gbpservice/neutron/plugins/ml2plus/drivers/apic_aim/nova_client.py diff --git a/gbpservice/neutron/plugins/ml2plus/drivers/apic_aim/rpc.py b/gbpservice/neutron/plugins/ml2plus/drivers/apic_aim/rpc.py index ef48accbb..305df8144 100644 --- a/gbpservice/neutron/plugins/ml2plus/drivers/apic_aim/rpc.py +++ b/gbpservice/neutron/plugins/ml2plus/drivers/apic_aim/rpc.py @@ -44,10 +44,6 @@ from gbpservice.neutron.plugins.ml2plus.drivers.apic_aim import constants from gbpservice.neutron.plugins.ml2plus.drivers.apic_aim import db from gbpservice.neutron.plugins.ml2plus.drivers.apic_aim import extension_db -# REVISIT: This should be moved to the mechanism driver. -from gbpservice.neutron.services.grouppolicy.drivers.cisco.apic import ( - port_ha_ipaddress_binding as ha_ip_db) - LOG = log.getLogger(__name__) BAKERY = baked.bakery(_size_alert=lambda c: LOG.warning( @@ -199,14 +195,6 @@ class ApicRpcHandlerMixin(object): return conn.consume_in_threads() # The following five methods handle RPCs from the Opflex agent. - # - # REVISIT: These handler methods are currently called by - # corresponding handler methods in the aim_mapping_rpc - # module. Once these RPC handlers are all fully implemented and - # tested, move the instantiation of the - # opflexagent.rpc.GBPServerRpcCallback class from aim_mapping_rpc - # to this module and eliminate the other RPC handler - # implementations. def get_gbp_details(self, context, **kwargs): LOG.debug("APIC AIM MD handling get_gbp_details for: %s", kwargs) @@ -280,9 +268,12 @@ class ApicRpcHandlerMixin(object): def ip_address_owner_update(self, context, **kwargs): LOG.debug("APIC AIM MD handling ip_address_owner_update for: %s", kwargs) - # REVISIT: Move actual handler implementation to this class. - if self.gbp_driver: - self.gbp_driver.ip_address_owner_update(context, **kwargs) + if not kwargs.get('ip_owner_info'): + return + ports_to_update = self.update_ip_owner(kwargs['ip_owner_info']) + for p in ports_to_update: + LOG.debug("APIC ownership update for port %s", p) + self._notify_port_update(context, p) @db_api.retry_if_session_inactive() def _get_vrf_details(self, context, vrf_id): @@ -637,17 +628,17 @@ class ApicRpcHandlerMixin(object): def _query_endpoint_haip_owned_ip_info(self, session, port_id, network_id): query = BAKERY(lambda s: s.query( - ha_ip_db.HAIPAddressToPortAssocation.ha_ip_address, + db.HAIPAddressToPortAssociation.ha_ip_address, models_v2.IPAllocation.port_id, )) query += lambda q: q.outerjoin( models_v2.IPAllocation, models_v2.IPAllocation.ip_address == - ha_ip_db.HAIPAddressToPortAssocation.ha_ip_address and + db.HAIPAddressToPortAssociation.ha_ip_address and models_v2.IPAllocation.network_id == sa.bindparam('network_id')) query += lambda q: q.filter( - ha_ip_db.HAIPAddressToPortAssocation.port_id == + db.HAIPAddressToPortAssociation.port_id == sa.bindparam('port_id')) return [EndpointOwnedIpInfo._make(row) for row in query(session).params( 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 1a017474f..b80dc1757 100644 --- a/gbpservice/neutron/services/grouppolicy/drivers/cisco/apic/aim_mapping.py +++ b/gbpservice/neutron/services/grouppolicy/drivers/cisco/apic/aim_mapping.py @@ -46,6 +46,7 @@ from gbpservice.neutron.plugins.ml2plus.drivers.apic_aim import ( from gbpservice.neutron.plugins.ml2plus.drivers.apic_aim import ( mechanism_driver as md) from gbpservice.neutron.plugins.ml2plus.drivers.apic_aim import apic_mapper +from gbpservice.neutron.plugins.ml2plus.drivers.apic_aim import nova_client from gbpservice.neutron.services.grouppolicy.common import ( constants as gp_const) from gbpservice.neutron.services.grouppolicy.common import constants as g_const @@ -58,8 +59,6 @@ from gbpservice.neutron.services.grouppolicy.drivers.cisco.apic import ( aim_validation) from gbpservice.neutron.services.grouppolicy.drivers.cisco.apic import ( apic_mapping_lib as alib) -from gbpservice.neutron.services.grouppolicy.drivers.cisco.apic import ( - nova_client as nclient) from gbpservice.neutron.services.grouppolicy.drivers.cisco.apic import config # noqa from gbpservice.neutron.services.grouppolicy import plugin as gbp_plugin @@ -1051,7 +1050,7 @@ class AIMMappingDriver(nrd.CommonNeutronBase, aim_rpc.AIMMappingRPCMixin): context._plugin_context, l2p['l3_policy_id']) if l3p.get('allowed_vm_names'): ok_to_bind = False - vm = nclient.NovaClient().get_server(port['device_id']) + vm = nova_client.NovaClient().get_server(port['device_id']) for allowed_vm_name in l3p['allowed_vm_names']: match = re.search(allowed_vm_name, vm.name) if match: 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 1c2c2300e..f26ff8ef8 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 @@ -35,8 +35,6 @@ from gbpservice.neutron.plugins.ml2plus.drivers.apic_aim import ( constants as md_const) from gbpservice.neutron.plugins.ml2plus.drivers.apic_aim import ( mechanism_driver as md) -from gbpservice.neutron.services.grouppolicy.drivers.cisco.apic import ( - port_ha_ipaddress_binding as ha_ip_db) LOG = log.getLogger(__name__) @@ -54,7 +52,7 @@ EndpointPtInfo = namedtuple( 'is_auto_ptg']) -class AIMMappingRPCMixin(ha_ip_db.HAIPOwnerDbMixin): +class AIMMappingRPCMixin(object): """RPC mixin for AIM mapping. Collection of all the RPC methods consumed by the AIM mapping. @@ -168,7 +166,8 @@ class AIMMappingRPCMixin(ha_ip_db.HAIPOwnerDbMixin): def ip_address_owner_update(self, context, **kwargs): if not kwargs.get('ip_owner_info'): return - ports_to_update = self.update_ip_owner(kwargs['ip_owner_info']) + ports_to_update = self.aim_mech_driver.update_ip_owner( + kwargs['ip_owner_info']) for p in ports_to_update: LOG.debug("APIC ownership update for port %s", p) self._send_port_update_notification(context, p) @@ -769,7 +768,7 @@ class AIMMappingRPCMixin(ha_ip_db.HAIPOwnerDbMixin): details['vm-name'] = vm_name def _get_owned_addresses(self, plugin_context, port_id): - return set(self.ha_ip_handler.get_ha_ipaddresses_for_port(port_id)) + return set(self.aim_mech_driver.get_ha_ipaddresses_for_port(port_id)) def _add_security_group_details(self, context, port, details): vif_details = port.get('binding:vif_details') diff --git a/gbpservice/neutron/services/grouppolicy/drivers/cisco/apic/port_ha_ipaddress_binding.py b/gbpservice/neutron/services/grouppolicy/drivers/cisco/apic/port_ha_ipaddress_binding.py deleted file mode 100644 index e92b48ca4..000000000 --- a/gbpservice/neutron/services/grouppolicy/drivers/cisco/apic/port_ha_ipaddress_binding.py +++ /dev/null @@ -1,180 +0,0 @@ -# All Rights Reserved. -# -# Licensed under the Apache License, Version 2.0 (the "License"); you may -# not use this file except in compliance with the License. You may obtain -# a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT -# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the -# License for the specific language governing permissions and limitations -# under the License. - -import sqlalchemy as sa -from sqlalchemy.ext import baked -from sqlalchemy import orm - -from neutron.db import api as db_api -from neutron.db import models_v2 -from neutron_lib import context as nctx -from oslo_db import exception as db_exc -from oslo_log import log as logging - -from neutron_lib.db import model_base -from neutron_lib.plugins import directory - -LOG = logging.getLogger(__name__) - -BAKERY = baked.bakery(_size_alert=lambda c: LOG.warning( - "sqlalchemy baked query cache size exceeded in %s" % __name__)) - - -# REVISIT: Fix the misspelling of 'association'. -class HAIPAddressToPortAssocation(model_base.BASEV2): - - """Port Owner for HA IP Address. - - This table is used to store the mapping between the HA IP Address - and the Port ID of the Neutron Port which currently owns this - IP Address. - """ - - __tablename__ = 'apic_ml2_ha_ipaddress_to_port_owner' - - ha_ip_address = sa.Column(sa.String(64), nullable=False, - primary_key=True) - port_id = sa.Column(sa.String(64), sa.ForeignKey('ports.id', - ondelete='CASCADE'), - nullable=False, primary_key=True) - - -class PortForHAIPAddress(object): - - def _get_ha_ipaddress(self, port_id, ipaddress, session=None): - session = session or db_api.get_reader_session() - - query = BAKERY(lambda s: s.query( - HAIPAddressToPortAssocation)) - query += lambda q: q.filter_by( - port_id=sa.bindparam('port_id'), - ha_ip_address=sa.bindparam('ipaddress')) - return query(session).params( - port_id=port_id, ipaddress=ipaddress).first() - - def get_port_for_ha_ipaddress(self, ipaddress, network_id, - session=None): - """Returns the Neutron Port ID for the HA IP Addresss.""" - session = session or db_api.get_reader_session() - query = BAKERY(lambda s: s.query( - HAIPAddressToPortAssocation)) - query += lambda q: q.join( - models_v2.Port, - models_v2.Port.id == HAIPAddressToPortAssocation.port_id) - query += lambda q: q.filter( - HAIPAddressToPortAssocation.ha_ip_address == - sa.bindparam('ipaddress')) - query += lambda q: q.filter( - models_v2.Port.network_id == sa.bindparam('network_id')) - port_ha_ip = query(session).params( - ipaddress=ipaddress, network_id=network_id).first() - return port_ha_ip - - def get_ha_ipaddresses_for_port(self, port_id, session=None): - """Returns the HA IP Addressses associated with a Port.""" - session = session or db_api.get_reader_session() - - query = BAKERY(lambda s: s.query( - HAIPAddressToPortAssocation)) - query += lambda q: q.filter_by( - port_id=sa.bindparam('port_id')) - objs = query(session).params( - port_id=port_id).all() - - return sorted([x['ha_ip_address'] for x in objs]) - - def set_port_id_for_ha_ipaddress(self, port_id, ipaddress, session=None): - """Stores a Neutron Port Id as owner of HA IP Addr (idempotent API).""" - session = session or db_api.get_writer_session() - try: - with session.begin(subtransactions=True): - obj = self._get_ha_ipaddress(port_id, ipaddress, session) - if obj: - return obj - else: - obj = HAIPAddressToPortAssocation(port_id=port_id, - ha_ip_address=ipaddress) - session.add(obj) - return obj - except db_exc.DBDuplicateEntry: - LOG.debug('Duplicate IP ownership entry for tuple %s', - (port_id, ipaddress)) - - def delete_port_id_for_ha_ipaddress(self, port_id, ipaddress, - session=None): - session = session or db_api.get_writer_session() - with session.begin(subtransactions=True): - try: - # REVISIT: Can this query be baked? The - # sqlalchemy.ext.baked.Result class does not have a - # delete() method, and adding delete() to the baked - # query before executing it seems to result in the - # params() not being evaluated. - return session.query( - HAIPAddressToPortAssocation).filter_by( - port_id=port_id, - ha_ip_address=ipaddress).delete() - except orm.exc.NoResultFound: - return - - def get_ha_port_associations(self): - session = db_api.get_reader_session() - - query = BAKERY(lambda s: s.query( - HAIPAddressToPortAssocation)) - return query(session).all() - - -class HAIPOwnerDbMixin(object): - - def __init__(self): - self.ha_ip_handler = PortForHAIPAddress() - - def _get_plugin(self): - return directory.get_plugin() - - def update_ip_owner(self, ip_owner_info): - ports_to_update = set() - port_id = ip_owner_info.get('port') - ipv4 = ip_owner_info.get('ip_address_v4') - ipv6 = ip_owner_info.get('ip_address_v6') - network_id = ip_owner_info.get('network_id') - if not port_id or (not ipv4 and not ipv6): - return ports_to_update - LOG.debug("Got IP owner update: %s", ip_owner_info) - core_plugin = self._get_plugin() - # REVISIT: just use SQLAlchemy session and models_v2.Port? - port = core_plugin.get_port(nctx.get_admin_context(), port_id) - if not port: - LOG.debug("Ignoring update for non-existent port: %s", port_id) - return ports_to_update - ports_to_update.add(port_id) - for ipa in [ipv4, ipv6]: - if not ipa: - continue - try: - session = db_api.get_writer_session() - with session.begin(subtransactions=True): - old_owner = self.ha_ip_handler.get_port_for_ha_ipaddress( - ipa, network_id or port['network_id'], session=session) - self.ha_ip_handler.set_port_id_for_ha_ipaddress(port_id, - ipa, - session) - if old_owner and old_owner['port_id'] != port_id: - self.ha_ip_handler.delete_port_id_for_ha_ipaddress( - old_owner['port_id'], ipa, session=session) - ports_to_update.add(old_owner['port_id']) - except db_exc.DBReferenceError as dbe: - LOG.debug("Ignoring FK error for port %s: %s", port_id, dbe) - return ports_to_update 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 fbff6b960..cf5715d7c 100644 --- a/gbpservice/neutron/tests/unit/plugins/ml2plus/test_apic_aim.py +++ b/gbpservice/neutron/tests/unit/plugins/ml2plus/test_apic_aim.py @@ -245,8 +245,8 @@ class ApicAimTestCase(test_address_scope.AddressScopeTestCase, def setUp(self, mechanism_drivers=None, tenant_network_types=None, plugin=None, ext_mgr=None): self.nova_client = mock.patch( - 'gbpservice.neutron.services.grouppolicy.drivers.cisco.' - 'apic.nova_client.NovaClient.get_servers').start() + 'gbpservice.neutron.plugins.ml2plus.drivers.apic_aim.' + 'nova_client.NovaClient.get_servers').start() self.nova_client.return_value = [] # Enable the test mechanism driver to ensure that # we can successfully call through to all mechanism @@ -8594,3 +8594,44 @@ class TestOpflexRpc(ApicAimTestCase): # REVISIT: Test with missing request, missing device, invalid # device prefix, unbindable port, port bound to wrong host. + + def test_ip_address_owner_update(self): + self._register_agent('h1', AGENT_CONF_OPFLEX) + self._register_agent('h2', AGENT_CONF_OPFLEX) + + net = self._make_network(self.fmt, 'net1', True) + net_id = net['network']['id'] + + self._make_subnet(self.fmt, net, '10.0.1.1', '10.0.1.0/24') + + port1_id = self._make_port(self.fmt, net_id)['port']['id'] + port2_id = self._make_port(self.fmt, net_id)['port']['id'] + + self._bind_port_to_host(port1_id, 'h1') + self._bind_port_to_host(port2_id, 'h2') + + ip_owner_info = {'port': port1_id, 'ip_address_v4': '1.2.3.4'} + self.driver._notify_port_update = mock.Mock() + + # Set new owner and check. + self.driver.ip_address_owner_update( + n_context.get_admin_context(), ip_owner_info=ip_owner_info, + host='h1') + obj = self.driver.get_port_for_ha_ipaddress('1.2.3.4', net_id) + self.assertEqual(port1_id, obj['port_id']) + self.driver._notify_port_update.assert_called_with( + mock.ANY, port1_id) + + # Update existing owner and check. + self.driver._notify_port_update.reset_mock() + ip_owner_info['port'] = port2_id + self.driver.ip_address_owner_update( + n_context.get_admin_context(), ip_owner_info=ip_owner_info, + host='h2') + obj = self.driver.get_port_for_ha_ipaddress('1.2.3.4', net_id) + self.assertEqual(port2_id, obj['port_id']) + exp_calls = [ + mock.call(mock.ANY, port1_id), + mock.call(mock.ANY, port2_id)] + self._check_call_list(exp_calls, + self.driver._notify_port_update.call_args_list) diff --git a/gbpservice/neutron/tests/unit/db/test_port_ha_ipaddress_binding.py b/gbpservice/neutron/tests/unit/plugins/ml2plus/test_db_apic_aim.py similarity index 91% rename from gbpservice/neutron/tests/unit/db/test_port_ha_ipaddress_binding.py rename to gbpservice/neutron/tests/unit/plugins/ml2plus/test_db_apic_aim.py index 846f14a37..6738ce395 100644 --- a/gbpservice/neutron/tests/unit/db/test_port_ha_ipaddress_binding.py +++ b/gbpservice/neutron/tests/unit/plugins/ml2plus/test_db_apic_aim.py @@ -17,8 +17,7 @@ from oslo_db import exception as exc from oslo_utils import importutils from gbpservice.neutron.db import all_models # noqa -from gbpservice.neutron.services.grouppolicy.drivers.cisco.apic import ( - port_ha_ipaddress_binding as ha) +from gbpservice.neutron.plugins.ml2plus.drivers.apic_aim import db DB_PLUGIN_KLASS = 'neutron.db.db_base_plugin_v2.NeutronDbPluginV2' @@ -74,7 +73,8 @@ class PortToHAIPAddressBindingTestCase(testlib_api.SqlTestCase): self.port1 = self.plugin.create_port(self.context, self.port1_data) self.port1_2 = self.plugin.create_port(self.context, self.port1_2_data) self.port2 = self.plugin.create_port(self.context, self.port2_data) - self.port_haip = ha.PortForHAIPAddress() + self.port_haip = db.DbMixin() + self.port_haip.plugin = self.plugin def test_set_and_get_port_to_ha_ip_binding(self): # Test new HA IP address to port binding can be created @@ -150,14 +150,12 @@ class PortToHAIPAddressBindingTestCase(testlib_api.SqlTestCase): self.assertEqual(0, result) def test_ip_owner_update(self): - mixin = ha.HAIPOwnerDbMixin() - mixin._get_plugin = mock.Mock(return_value=self.plugin) ip_owner_info = {'port': self.port1['id'], 'ip_address_v4': self.ha_ip1} # set new owner - ports = mixin.update_ip_owner(ip_owner_info) - obj = mixin.ha_ip_handler.get_port_for_ha_ipaddress( + ports = self.port_haip.update_ip_owner(ip_owner_info) + obj = self.port_haip.get_port_for_ha_ipaddress( self.ha_ip1, self.port1['network_id']) self.assertEqual(self.port1['id'], obj['port_id']) self.assertTrue(self.port1['id'] in ports) @@ -169,28 +167,26 @@ class PortToHAIPAddressBindingTestCase(testlib_api.SqlTestCase): port3 = self.plugin.create_port(self.context, self.port2_data) ip_owner_info['port'] = port3['id'] - ports = mixin.update_ip_owner(ip_owner_info) - obj = mixin.ha_ip_handler.get_port_for_ha_ipaddress( + ports = self.port_haip.update_ip_owner(ip_owner_info) + obj = self.port_haip.get_port_for_ha_ipaddress( self.ha_ip1, port3['network_id']) self.assertEqual(port3['id'], obj['port_id']) def test_ip_replaced(self): - mixin = ha.HAIPOwnerDbMixin() - mixin._get_plugin = mock.Mock(return_value=self.plugin) ip_owner_info = {'port': self.port1['id'], 'ip_address_v4': self.ha_ip1} - mixin.update_ip_owner(ip_owner_info) + self.port_haip.update_ip_owner(ip_owner_info) # Verify only one entry is there - dump = mixin.ha_ip_handler.get_ha_port_associations() + dump = self.port_haip.get_ha_port_associations() self.assertEqual(1, len(dump)) self.assertEqual(self.port1['id'], dump[0].port_id) self.assertEqual(self.ha_ip1, dump[0].ha_ip_address) # Now override with port1_2 ip_owner_info['port'] = self.port1_2['id'] - mixin.update_ip_owner(ip_owner_info) + self.port_haip.update_ip_owner(ip_owner_info) # Verify still one entry exists - dump = mixin.ha_ip_handler.get_ha_port_associations() + dump = self.port_haip.get_ha_port_associations() self.assertEqual(1, len(dump)) self.assertEqual(self.port1_2['id'], dump[0].port_id) self.assertEqual(self.ha_ip1, dump[0].ha_ip_address) @@ -198,9 +194,9 @@ class PortToHAIPAddressBindingTestCase(testlib_api.SqlTestCase): # Override again, but with a different net_id to keep both records ip_owner_info['port'] = self.port1['id'] ip_owner_info['network_id'] = 'new_net_id' - mixin.update_ip_owner(ip_owner_info) + self.port_haip.update_ip_owner(ip_owner_info) # Verify still one entry exists - dump = mixin.ha_ip_handler.get_ha_port_associations() + dump = self.port_haip.get_ha_port_associations() self.assertEqual(2, len(dump)) def test_duplicate_entry_handled_gracefully(self): 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 f5b49705c..4615f2d9c 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 @@ -121,8 +121,8 @@ class AIMBaseTestCase(test_nr_base.CommonNeutronBaseTestCase, l3_plugin=None, sc_plugin=None, trunk_plugin=None, qos_plugin=None, **kwargs): self.nova_client1 = mock.patch( - 'gbpservice.neutron.services.grouppolicy.drivers.cisco.' - 'apic.nova_client.NovaClient.get_servers').start() + 'gbpservice.neutron.plugins.ml2plus.drivers.apic_aim.' + 'nova_client.NovaClient.get_servers').start() self.nova_client1.return_value = [] core_plugin = core_plugin or ML2PLUS_PLUGIN if not l3_plugin: @@ -182,8 +182,8 @@ class AIMBaseTestCase(test_nr_base.CommonNeutronBaseTestCase, self._name_mapper = None self._driver = None nova_client = mock.patch( - 'gbpservice.neutron.services.grouppolicy.drivers.cisco.' - 'apic.nova_client.NovaClient.get_server').start() + 'gbpservice.neutron.plugins.ml2plus.drivers.apic_aim.' + 'nova_client.NovaClient.get_server').start() vm = mock.Mock() vm.name = 'someid' nova_client.return_value = vm @@ -3823,7 +3823,7 @@ class TestPolicyTarget(AIMBaseTestCase, # set new owner self.driver.ip_address_owner_update(self._context, ip_owner_info=ip_owner_info, host='h1') - obj = self.driver.ha_ip_handler.get_port_for_ha_ipaddress( + obj = self.driver.aim_mech_driver.get_port_for_ha_ipaddress( '1.2.3.4', net_id) self.assertEqual(pt1['port_id'], obj['port_id']) @@ -3835,7 +3835,7 @@ class TestPolicyTarget(AIMBaseTestCase, ip_owner_info['port'] = pt2['port_id'] self.driver.ip_address_owner_update(self._context, ip_owner_info=ip_owner_info, host='h2') - obj = self.driver.ha_ip_handler.get_port_for_ha_ipaddress( + obj = self.driver.aim_mech_driver.get_port_for_ha_ipaddress( '1.2.3.4', net_id) self.assertEqual(pt2['port_id'], obj['port_id']) exp_calls = [ @@ -3856,8 +3856,8 @@ class TestPolicyTarget(AIMBaseTestCase, policy_target_group_id=ptg['id'])['policy_target'] nova_client = mock.patch( - 'gbpservice.neutron.services.grouppolicy.drivers.cisco.' - 'apic.nova_client.NovaClient.get_server').start() + 'gbpservice.neutron.plugins.ml2plus.drivers.apic_aim.' + 'nova_client.NovaClient.get_server').start() vm = mock.Mock() vm.name = 'secure_vm1' nova_client.return_value = vm @@ -5895,7 +5895,7 @@ class TestNeutronPortOperation(AIMBaseTestCase): ip_owner_info = {'port': p1['id'], 'ip_address_v4': owned_addr[0], 'network_id': p1['network_id']} - self.driver.update_ip_owner(ip_owner_info) + self.driver.aim_mech_driver.update_ip_owner(ip_owner_info) # Call RPC sent by the agent to get the details for p1 details = self.driver.get_gbp_details( self._neutron_admin_context, device='tap%s' % p1['id'], @@ -5928,7 +5928,7 @@ class TestNeutronPortOperation(AIMBaseTestCase): ip_owner_info = {'port': p2['id'], 'ip_address_v4': owned_addr[1], 'network_id': p2['network_id']} - self.driver.update_ip_owner(ip_owner_info) + self.driver.aim_mech_driver.update_ip_owner(ip_owner_info) # Call RPC sent by the agent to get the details for p2 details = self.driver.get_gbp_details( self._neutron_admin_context, device='tap%s' % p2['id'], @@ -5999,13 +5999,13 @@ class TestNeutronPortOperation(AIMBaseTestCase): p1 = self._update('ports', p1['id'], {'port': {'allowed_address_pairs': update_addr}}, neutron_context=self._neutron_admin_context)['port'] - ips = self.driver.ha_ip_handler.get_ha_ipaddresses_for_port(p1['id']) + ips = self.driver.aim_mech_driver.get_ha_ipaddresses_for_port(p1['id']) self.assertEqual(ips, []) # Request ownership of the new AAP ip_owner_info = {'port': p1['id'], 'ip_address_v4': update_owned_addr[0], 'network_id': p1['network_id']} - self.driver.update_ip_owner(ip_owner_info) + self.driver.aim_mech_driver.update_ip_owner(ip_owner_info) details = self.driver.get_gbp_details( self._neutron_admin_context, device='tap%s' % p1['id'], host='h1') @@ -6016,13 +6016,13 @@ class TestNeutronPortOperation(AIMBaseTestCase): p2 = self._update('ports', p2['id'], {'port': {'allowed_address_pairs': update_addr}}, neutron_context=self._neutron_admin_context)['port'] - ips = self.driver.ha_ip_handler.get_ha_ipaddresses_for_port(p2['id']) + ips = self.driver.aim_mech_driver.get_ha_ipaddresses_for_port(p2['id']) self.assertEqual(ips, []) # Request ownership of the new AAP ip_owner_info = {'port': p2['id'], 'ip_address_v4': update_owned_addr[1], 'network_id': p2['network_id']} - self.driver.update_ip_owner(ip_owner_info) + self.driver.aim_mech_driver.update_ip_owner(ip_owner_info) details = self.driver.get_gbp_details( self._neutron_admin_context, device='tap%s' % p2['id'], host='h2')