Merge "Handle ports assigned to routers without routerports" into stable/train
This commit is contained in:
Zuul
Gerrit Code Review
commit
0d462fdc7a
@ -925,21 +925,13 @@ class L3_NAT_dbonly_mixin(l3.RouterPluginBase,
|
||||
|
||||
def _remove_interface_by_port(self, context, router_id,
|
||||
port_id, subnet_id, owner):
|
||||
obj = l3_obj.RouterPort.get_object(
|
||||
context,
|
||||
port_id=port_id,
|
||||
router_id=router_id,
|
||||
port_type=owner
|
||||
)
|
||||
if obj:
|
||||
try:
|
||||
port = self._core_plugin.get_port(context, obj.port_id)
|
||||
except n_exc.PortNotFound:
|
||||
raise l3_exc.RouterInterfaceNotFound(
|
||||
router_id=router_id, port_id=port_id)
|
||||
else:
|
||||
ports = port_obj.Port.get_ports_by_router_and_port(
|
||||
context, router_id, owner, port_id)
|
||||
if len(ports) < 1:
|
||||
raise l3_exc.RouterInterfaceNotFound(
|
||||
router_id=router_id, port_id=port_id)
|
||||
|
||||
port = ports[0]
|
||||
port_subnet_ids = [fixed_ip['subnet_id']
|
||||
for fixed_ip in port['fixed_ips']]
|
||||
if subnet_id and subnet_id not in port_subnet_ids:
|
||||
@ -952,46 +944,41 @@ class L3_NAT_dbonly_mixin(l3.RouterPluginBase,
|
||||
context, router_id, port_subnet_id)
|
||||
self._core_plugin.delete_port(context, port['id'],
|
||||
l3_port_check=False)
|
||||
return (port, subnets)
|
||||
return port, subnets
|
||||
|
||||
def _remove_interface_by_subnet(self, context,
|
||||
router_id, subnet_id, owner):
|
||||
self._confirm_router_interface_not_in_use(
|
||||
context, router_id, subnet_id)
|
||||
subnet = self._core_plugin.get_subnet(context, subnet_id)
|
||||
ports = port_obj.Port.get_ports_by_router_and_network(
|
||||
context, router_id, owner, subnet['network_id'])
|
||||
|
||||
try:
|
||||
ports = port_obj.Port.get_ports_by_router(
|
||||
context, router_id, owner, subnet)
|
||||
|
||||
for p in ports:
|
||||
try:
|
||||
p = self._core_plugin.get_port(context, p.id)
|
||||
except n_exc.PortNotFound:
|
||||
continue
|
||||
port_subnets = [fip['subnet_id'] for fip in p['fixed_ips']]
|
||||
if subnet_id in port_subnets and len(port_subnets) > 1:
|
||||
# multiple prefix port - delete prefix from port
|
||||
fixed_ips = [dict(fip) for fip in p['fixed_ips']
|
||||
if fip['subnet_id'] != subnet_id]
|
||||
self._core_plugin.update_port(
|
||||
context, p['id'], {'port': {'fixed_ips': fixed_ips}})
|
||||
return (p, [subnet])
|
||||
elif subnet_id in port_subnets:
|
||||
# only one subnet on port - delete the port
|
||||
self._core_plugin.delete_port(context, p['id'],
|
||||
l3_port_check=False)
|
||||
return (p, [subnet])
|
||||
except exc.NoResultFound:
|
||||
pass
|
||||
for p in ports:
|
||||
try:
|
||||
p = self._core_plugin.get_port(context, p.id)
|
||||
except n_exc.PortNotFound:
|
||||
continue
|
||||
port_subnets = [fip['subnet_id'] for fip in p['fixed_ips']]
|
||||
if subnet_id in port_subnets and len(port_subnets) > 1:
|
||||
# multiple prefix port - delete prefix from port
|
||||
fixed_ips = [dict(fip) for fip in p['fixed_ips']
|
||||
if fip['subnet_id'] != subnet_id]
|
||||
self._core_plugin.update_port(
|
||||
context, p['id'], {'port': {'fixed_ips': fixed_ips}})
|
||||
return (p, [subnet])
|
||||
elif subnet_id in port_subnets:
|
||||
# only one subnet on port - delete the port
|
||||
self._core_plugin.delete_port(context, p['id'],
|
||||
l3_port_check=False)
|
||||
return (p, [subnet])
|
||||
raise l3_exc.RouterInterfaceNotFoundForSubnet(
|
||||
router_id=router_id, subnet_id=subnet_id)
|
||||
|
||||
@db_api.retry_if_session_inactive()
|
||||
def remove_router_interface(self, context, router_id, interface_info):
|
||||
remove_by_port, remove_by_subnet = (
|
||||
self._validate_interface_info(interface_info, for_removal=True)
|
||||
)
|
||||
remove_by_port, _ = self._validate_interface_info(interface_info,
|
||||
for_removal=True)
|
||||
port_id = interface_info.get('port_id')
|
||||
subnet_id = interface_info.get('subnet_id')
|
||||
device_owner = self._get_device_owner(context, router_id)
|
||||
@ -999,9 +986,6 @@ class L3_NAT_dbonly_mixin(l3.RouterPluginBase,
|
||||
port, subnets = self._remove_interface_by_port(context, router_id,
|
||||
port_id, subnet_id,
|
||||
device_owner)
|
||||
# remove_by_subnet is not used here, because the validation logic of
|
||||
# _validate_interface_info ensures that at least one of remote_by_*
|
||||
# is True.
|
||||
else:
|
||||
port, subnets = self._remove_interface_by_subnet(
|
||||
context, router_id, subnet_id, device_owner)
|
||||
|
||||
@ -15,6 +15,7 @@
|
||||
import netaddr
|
||||
from neutron_lib import constants
|
||||
from neutron_lib.utils import net as net_utils
|
||||
from oslo_log import log as logging
|
||||
from oslo_utils import versionutils
|
||||
from oslo_versionedobjects import fields as obj_fields
|
||||
|
||||
@ -28,6 +29,8 @@ from neutron.objects.db import api as obj_db_api
|
||||
from neutron.objects.qos import binding
|
||||
from neutron.plugins.ml2 import models as ml2_models
|
||||
|
||||
LOG = logging.getLogger(__name__)
|
||||
|
||||
|
||||
class PortBindingBase(base.NeutronDbObject):
|
||||
|
||||
@ -508,15 +511,65 @@ class Port(base.NeutronDbObject):
|
||||
primitive.pop('qos_network_policy_id', None)
|
||||
|
||||
@classmethod
|
||||
def get_ports_by_router(cls, context, router_id, owner, subnet):
|
||||
rport_qry = context.session.query(models_v2.Port).join(
|
||||
l3.RouterPort)
|
||||
ports = rport_qry.filter(
|
||||
l3.RouterPort.router_id == router_id,
|
||||
l3.RouterPort.port_type == owner,
|
||||
models_v2.Port.network_id == subnet['network_id']
|
||||
)
|
||||
return [cls._load_object(context, db_obj) for db_obj in ports.all()]
|
||||
def get_ports_by_router_and_network(cls, context, router_id, owner,
|
||||
network_id):
|
||||
"""Returns port objects filtering by router ID, owner and network ID"""
|
||||
rports_filter = (models_v2.Port.network_id == network_id, )
|
||||
router_filter = (models_v2.Port.network_id == network_id, )
|
||||
return cls._get_ports_by_router(context, router_id, owner,
|
||||
rports_filter, router_filter)
|
||||
|
||||
@classmethod
|
||||
def get_ports_by_router_and_port(cls, context, router_id, owner, port_id):
|
||||
"""Returns port objects filtering by router ID, owner and port ID"""
|
||||
rports_filter = (l3.RouterPort.port_id == port_id, )
|
||||
router_filter = (models_v2.Port.id == port_id, )
|
||||
return cls._get_ports_by_router(context, router_id, owner,
|
||||
rports_filter, router_filter)
|
||||
|
||||
@classmethod
|
||||
def _get_ports_by_router(cls, context, router_id, owner, rports_filter,
|
||||
router_filter):
|
||||
"""Returns port objects filtering by router id and owner
|
||||
|
||||
The method will receive extra filters depending of the caller (filter
|
||||
by network or filter by port).
|
||||
|
||||
The ports are retrieved using:
|
||||
- The RouterPort registers. Each time a port is assigned to a router,
|
||||
a new RouterPort register is added to the DB.
|
||||
- The port owner and device_id information.
|
||||
|
||||
Both searches should return the same result. If not, a warning message
|
||||
is logged and the port list to be returned is completed with the
|
||||
missing ones.
|
||||
"""
|
||||
rports_filter += (l3.RouterPort.router_id == router_id,
|
||||
l3.RouterPort.port_type == owner)
|
||||
router_filter += (models_v2.Port.device_id == router_id,
|
||||
models_v2.Port.device_owner == owner)
|
||||
|
||||
ports = context.session.query(models_v2.Port).join(
|
||||
l3.RouterPort).filter(*rports_filter)
|
||||
ports_rports = [cls._load_object(context, db_obj)
|
||||
for db_obj in ports.all()]
|
||||
|
||||
ports = context.session.query(models_v2.Port).filter(*router_filter)
|
||||
ports_router = [cls._load_object(context, db_obj)
|
||||
for db_obj in ports.all()]
|
||||
|
||||
ports_rports_ids = {p.id for p in ports_rports}
|
||||
ports_router_ids = {p.id for p in ports_router}
|
||||
missing_port_ids = ports_router_ids - ports_rports_ids
|
||||
if missing_port_ids:
|
||||
LOG.warning('The following ports, assigned to router '
|
||||
'%(router_id)s, do not have a "routerport" register: '
|
||||
'%(port_ids)s', {'router_id': router_id,
|
||||
'port_ids': missing_port_ids})
|
||||
port_objs = [p for p in ports_router if p.id in missing_port_ids]
|
||||
ports_rports += port_objs
|
||||
|
||||
return ports_rports
|
||||
|
||||
@classmethod
|
||||
def get_ports_ids_by_security_groups(cls, context, security_group_ids,
|
||||
|
||||
@ -22,6 +22,7 @@ from neutron_lib import constants as n_const
|
||||
from neutron_lib import context
|
||||
from neutron_lib import exceptions as n_exc
|
||||
from neutron_lib.exceptions import l3 as l3_exc
|
||||
from neutron_lib.plugins import constants as plugin_constants
|
||||
from neutron_lib.plugins import directory
|
||||
from neutron_lib.plugins import utils as plugin_utils
|
||||
from oslo_utils import uuidutils
|
||||
@ -30,8 +31,12 @@ import testtools
|
||||
from neutron.db import l3_db
|
||||
from neutron.db.models import l3 as l3_models
|
||||
from neutron.objects import base as base_obj
|
||||
from neutron.objects import network as network_obj
|
||||
from neutron.objects import ports as port_obj
|
||||
from neutron.objects import router as l3_obj
|
||||
from neutron.objects import subnet as subnet_obj
|
||||
from neutron.tests import base
|
||||
from neutron.tests.unit.db import test_db_base_plugin_v2
|
||||
|
||||
|
||||
class TestL3_NAT_dbonly_mixin(base.BaseTestCase):
|
||||
@ -377,3 +382,148 @@ class L3_NAT_db_mixin(base.BaseTestCase):
|
||||
self.assertRaises(
|
||||
n_exc.BadRequest,
|
||||
self.db.add_router_interface, mock.Mock(), router_db.id)
|
||||
|
||||
|
||||
class FakeL3Plugin(l3_db.L3_NAT_dbonly_mixin):
|
||||
pass
|
||||
|
||||
|
||||
class L3TestCase(test_db_base_plugin_v2.NeutronDbPluginV2TestCase):
|
||||
|
||||
GET_PORTS_BY_ROUTER_MSG = (
|
||||
'The following ports, assigned to router %(router_id)s, do not have a '
|
||||
'"routerport" register: %(port_ids)s')
|
||||
|
||||
def setUp(self, *args, **kwargs):
|
||||
super(L3TestCase, self).setUp(plugin='ml2')
|
||||
self.core_plugin = directory.get_plugin()
|
||||
self.ctx = context.get_admin_context()
|
||||
self.mixin = FakeL3Plugin()
|
||||
directory.add_plugin(plugin_constants.L3, self.mixin)
|
||||
self.network = self.create_network()
|
||||
self.subnets = []
|
||||
self.subnets.append(self.create_subnet(self.network, '1.1.1.1',
|
||||
'1.1.1.0/24'))
|
||||
self.subnets.append(self.create_subnet(self.network, '1.1.2.1',
|
||||
'1.1.2.0/24'))
|
||||
router = {'router': {'name': 'foo_router', 'admin_state_up': True,
|
||||
'tenant_id': 'foo_tenant'}}
|
||||
self.router = self.create_router(router)
|
||||
self.ports = []
|
||||
for subnet in self.subnets:
|
||||
ipa = str(netaddr.IPNetwork(subnet['subnet']['cidr']).ip + 10)
|
||||
fixed_ips = [{'subnet_id': subnet['subnet']['id'],
|
||||
'ip_address': ipa}]
|
||||
self.ports.append(self.create_port(
|
||||
self.network['network']['id'], {'fixed_ips': fixed_ips}))
|
||||
self.addCleanup(self._clean_objs)
|
||||
|
||||
def _clean_objs(self):
|
||||
port_obj.Port.delete_objects(
|
||||
self.ctx, network_id=self.network['network']['id'])
|
||||
subnet_obj.Subnet.delete_objects(
|
||||
self.ctx, network_id=self.network['network']['id'])
|
||||
network_obj.Network.get_object(
|
||||
self.ctx, id=self.network['network']['id']).delete()
|
||||
l3_obj.Router.get_object(self.ctx, id=self.router['id']).delete()
|
||||
|
||||
def create_router(self, router):
|
||||
with self.ctx.session.begin(subtransactions=True):
|
||||
return self.mixin.create_router(self.ctx, router)
|
||||
|
||||
def create_port(self, net_id, port_info):
|
||||
with self.ctx.session.begin(subtransactions=True):
|
||||
return self._make_port(self.fmt, net_id, **port_info)
|
||||
|
||||
def create_network(self, name=None, **kwargs):
|
||||
name = name or 'network1'
|
||||
with self.ctx.session.begin(subtransactions=True):
|
||||
return self._make_network(self.fmt, name, True, **kwargs)
|
||||
|
||||
def create_subnet(self, network, gateway, cidr, **kwargs):
|
||||
with self.ctx.session.begin(subtransactions=True):
|
||||
return self._make_subnet(self.fmt, network, gateway, cidr,
|
||||
**kwargs)
|
||||
|
||||
def _add_router_interfaces(self):
|
||||
return [self.mixin.add_router_interface(
|
||||
self.ctx, self.router['id'],
|
||||
interface_info={'port_id': port['port']['id']})
|
||||
for port in self.ports]
|
||||
|
||||
def _check_routerports(self, ri_statuses):
|
||||
port_ids = []
|
||||
for idx, ri_status in enumerate(ri_statuses):
|
||||
rp_obj = l3_obj.RouterPort.get_object(
|
||||
self.ctx, port_id=self.ports[idx]['port']['id'],
|
||||
router_id=self.router['id'])
|
||||
if ri_status:
|
||||
self.assertEqual(self.ports[idx]['port']['id'], rp_obj.port_id)
|
||||
port_ids.append(rp_obj.port_id)
|
||||
else:
|
||||
self.assertIsNone(rp_obj)
|
||||
|
||||
_router_obj = l3_obj.Router.get_object(self.ctx, id=self.router['id'])
|
||||
router_port_ids = [rp.port_id for rp in
|
||||
_router_obj.db_obj.attached_ports]
|
||||
self.assertEqual(sorted(port_ids), sorted(router_port_ids))
|
||||
|
||||
@mock.patch.object(port_obj, 'LOG')
|
||||
def test_remove_router_interface_by_port(self, mock_log):
|
||||
self._add_router_interfaces()
|
||||
self._check_routerports((True, True))
|
||||
|
||||
interface_info = {'port_id': self.ports[0]['port']['id']}
|
||||
self.mixin.remove_router_interface(self.ctx, self.router['id'],
|
||||
interface_info)
|
||||
mock_log.warning.assert_not_called()
|
||||
self._check_routerports((False, True))
|
||||
|
||||
@mock.patch.object(port_obj, 'LOG')
|
||||
def test_remove_router_interface_by_port_removed_rport(self, mock_log):
|
||||
self._add_router_interfaces()
|
||||
self._check_routerports((True, True))
|
||||
|
||||
rp_obj = l3_obj.RouterPort.get_object(
|
||||
self.ctx, router_id=self.router['id'],
|
||||
port_id=self.ports[0]['port']['id'])
|
||||
rp_obj.delete()
|
||||
|
||||
interface_info = {'port_id': self.ports[0]['port']['id']}
|
||||
self.mixin.remove_router_interface(self.ctx, self.router['id'],
|
||||
interface_info)
|
||||
msg_vars = {'router_id': self.router['id'],
|
||||
'port_ids': {self.ports[0]['port']['id']}}
|
||||
mock_log.warning.assert_called_once_with(self.GET_PORTS_BY_ROUTER_MSG,
|
||||
msg_vars)
|
||||
self._check_routerports((False, True))
|
||||
|
||||
@mock.patch.object(port_obj, 'LOG')
|
||||
def test_remove_router_interface_by_subnet(self, mock_log):
|
||||
self._add_router_interfaces()
|
||||
self._check_routerports((True, True))
|
||||
|
||||
interface_info = {'subnet_id': self.subnets[1]['subnet']['id']}
|
||||
self.mixin.remove_router_interface(self.ctx, self.router['id'],
|
||||
interface_info)
|
||||
mock_log.warning.not_called_once()
|
||||
self._check_routerports((True, False))
|
||||
|
||||
@mock.patch.object(port_obj, 'LOG')
|
||||
def test_remove_router_interface_by_subnet_removed_rport(self, mock_log):
|
||||
self._add_router_interfaces()
|
||||
self._check_routerports((True, True))
|
||||
|
||||
rp_obj = l3_obj.RouterPort.get_object(
|
||||
self.ctx, router_id=self.router['id'],
|
||||
port_id=self.ports[0]['port']['id'])
|
||||
rp_obj.delete()
|
||||
|
||||
interface_info = {'subnet_id': self.subnets[0]['subnet']['id']}
|
||||
self.mixin.remove_router_interface(self.ctx, self.router['id'],
|
||||
interface_info)
|
||||
msg_vars = {'router_id': self.router['id'],
|
||||
'port_ids': {self.ports[0]['port']['id']}}
|
||||
mock_log.warning.assert_called_once_with(self.GET_PORTS_BY_ROUTER_MSG,
|
||||
msg_vars)
|
||||
self._check_routerports((False, True))
|
||||
|
||||
Loading…
Reference in New Issue
Block a user