Merge "Check router routes connectivity when GW port is updated"
This commit is contained in:
commit
c1d33a9948
|
@ -85,8 +85,18 @@ class ExtraRoute_dbonly_mixin(l3_db.L3_NAT_dbonly_mixin):
|
|||
routes=routes,
|
||||
reason=_('the nexthop is used by router'))
|
||||
|
||||
def _validate_routes(self, context,
|
||||
router_id, routes):
|
||||
def _validate_routes(self, context, router_id, routes, cidrs=None,
|
||||
ip_addresses=None):
|
||||
"""Validate a router routes with its interface subnets CIDRs and IPs
|
||||
|
||||
If any route cannot reach any subnet CIDR from any interface or the
|
||||
route nethop match any interface IP address, this route is invalid.
|
||||
:param context: Neutron request context
|
||||
:param router_id: router ID
|
||||
:param routes: router routes (list of dictionaries)
|
||||
:param cidrs: (optional) list of CIDRs (strings)
|
||||
:param ip_addresses: (optional) list of IP addresses (strings)
|
||||
"""
|
||||
if len(routes) > cfg.CONF.max_routes:
|
||||
raise xroute_exc.RoutesExhausted(
|
||||
router_id=router_id,
|
||||
|
@ -94,17 +104,20 @@ class ExtraRoute_dbonly_mixin(l3_db.L3_NAT_dbonly_mixin):
|
|||
|
||||
context = context.elevated()
|
||||
filters = {'device_id': [router_id]}
|
||||
ports = self._core_plugin.get_ports(context, filters)
|
||||
cidrs = []
|
||||
ips = []
|
||||
for port in ports:
|
||||
for ip in port['fixed_ips']:
|
||||
cidrs.append(self._core_plugin.get_subnet(
|
||||
context, ip['subnet_id'])['cidr'])
|
||||
ips.append(ip['ip_address'])
|
||||
|
||||
cidrs = cidrs or []
|
||||
ip_addresses = ip_addresses or []
|
||||
if not (cidrs or ip_addresses):
|
||||
ports = self._core_plugin.get_ports(context, filters)
|
||||
for port in ports:
|
||||
for ip in port['fixed_ips']:
|
||||
cidrs.append(self._core_plugin.get_subnet(
|
||||
context, ip['subnet_id'])['cidr'])
|
||||
ip_addresses.append(ip['ip_address'])
|
||||
|
||||
for route in routes:
|
||||
self._validate_routes_nexthop(
|
||||
cidrs, ips, routes, route['nexthop'])
|
||||
cidrs, ip_addresses, routes, route['nexthop'])
|
||||
|
||||
def _update_extra_routes(self, context, router, routes):
|
||||
self._validate_routes(context, router['id'], routes)
|
||||
|
|
|
@ -341,24 +341,48 @@ class L3_NAT_dbonly_mixin(l3.RouterPluginBase,
|
|||
)
|
||||
router_port.create()
|
||||
|
||||
def _validate_gw_info(self, context, gw_port, info, ext_ips):
|
||||
def _validate_gw_info(self, context, info, ext_ips, router):
|
||||
network_id = info['network_id'] if info else None
|
||||
gw_subnets = []
|
||||
if network_id:
|
||||
network_db = self._core_plugin._get_network(context, network_id)
|
||||
if not network_db.external:
|
||||
msg = _("Network %s is not an external network") % network_id
|
||||
raise n_exc.BadRequest(resource='router', msg=msg)
|
||||
if ext_ips:
|
||||
subnets = self._core_plugin.get_subnets_by_network(context,
|
||||
network_id)
|
||||
for s in subnets:
|
||||
if not s['gateway_ip']:
|
||||
|
||||
gw_subnets = network_db.subnets
|
||||
for ext_ip in ext_ips:
|
||||
for subnet in network_db.subnets:
|
||||
if not subnet.gateway_ip:
|
||||
continue
|
||||
for ext_ip in ext_ips:
|
||||
if ext_ip.get('ip_address') == s['gateway_ip']:
|
||||
msg = _("External IP %s is the same as the "
|
||||
"gateway IP") % ext_ip.get('ip_address')
|
||||
raise n_exc.BadRequest(resource='router', msg=msg)
|
||||
if ext_ip.get('ip_address') == subnet.gateway_ip:
|
||||
msg = _("External IP %s is the same as the "
|
||||
"gateway IP") % ext_ip.get('ip_address')
|
||||
raise n_exc.BadRequest(resource='router', msg=msg)
|
||||
|
||||
method_validate_routes = getattr(self, '_validate_routes', None)
|
||||
if not method_validate_routes or not router.route_list:
|
||||
return network_id
|
||||
|
||||
cidrs = []
|
||||
ip_addresses = []
|
||||
# All attached ports but the GW port, if exists, that will change
|
||||
# because the GW information is being updated.
|
||||
attached_ports = [rp.port for rp in router.attached_ports if
|
||||
rp.port.id != router.gw_port_id]
|
||||
for port in attached_ports:
|
||||
for ip in port.fixed_ips:
|
||||
cidrs.append(self._core_plugin.get_subnet(
|
||||
context.elevated(), ip.subnet_id)['cidr'])
|
||||
ip_addresses.append(ip.ip_address)
|
||||
|
||||
# NOTE(ralonsoh): the GW port is not updated yet and the its IP address
|
||||
# won't be added to "ip_addresses", only the subnet CIDRs.
|
||||
for gw_subnet in gw_subnets:
|
||||
cidrs.append(gw_subnet.cidr)
|
||||
|
||||
method_validate_routes(context, router.id, router.route_list,
|
||||
cidrs=cidrs, ip_addresses=ip_addresses)
|
||||
return network_id
|
||||
|
||||
# NOTE(yamamoto): This method is an override point for plugins
|
||||
|
@ -470,10 +494,10 @@ class L3_NAT_dbonly_mixin(l3.RouterPluginBase,
|
|||
def _update_router_gw_info(self, context, router_id, info, router=None):
|
||||
router = router or self._get_router(context, router_id)
|
||||
gw_port = router.gw_port
|
||||
ext_ips = info.get('external_fixed_ips') if info else []
|
||||
ext_ips = info.get('external_fixed_ips', []) if info else []
|
||||
ext_ip_change = self._check_for_external_ip_change(
|
||||
context, gw_port, ext_ips)
|
||||
network_id = self._validate_gw_info(context, gw_port, info, ext_ips)
|
||||
network_id = self._validate_gw_info(context, info, ext_ips, router)
|
||||
if gw_port and ext_ip_change and gw_port['network_id'] == network_id:
|
||||
self._update_current_gw_port(context, router_id, router,
|
||||
ext_ips)
|
||||
|
|
|
@ -24,14 +24,17 @@ from neutron_lib import constants as n_const
|
|||
from neutron_lib import context
|
||||
from neutron_lib.db import api as db_api
|
||||
from neutron_lib import exceptions as n_exc
|
||||
from neutron_lib.exceptions import extraroute as xroute_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
|
||||
import testtools
|
||||
|
||||
from neutron.db import extraroute_db
|
||||
from neutron.db import l3_db
|
||||
from neutron.db.models import l3 as l3_models
|
||||
from neutron.db import models_v2
|
||||
from neutron.extensions import segment as segment_ext
|
||||
from neutron.objects import base as base_obj
|
||||
from neutron.objects import network as network_obj
|
||||
|
@ -48,7 +51,12 @@ class TestL3_NAT_dbonly_mixin(
|
|||
|
||||
def setUp(self, *args, **kwargs):
|
||||
super(TestL3_NAT_dbonly_mixin, self).setUp(*args, **kwargs)
|
||||
self.db = l3_db.L3_NAT_dbonly_mixin()
|
||||
# "extraroute_db.ExtraRoute_dbonly_mixin" inherits from
|
||||
# "l3_db.L3_NAT_dbonly_mixin()", the class under test. This is used
|
||||
# instead to test the validation of router routes and GW change because
|
||||
# implements "_validate_routes".
|
||||
self.db = extraroute_db.ExtraRoute_dbonly_mixin()
|
||||
self.ctx = mock.Mock()
|
||||
|
||||
def test__each_port_having_fixed_ips_none(self):
|
||||
"""Be sure the method returns an empty list when None is passed"""
|
||||
|
@ -416,6 +424,93 @@ class TestL3_NAT_dbonly_mixin(
|
|||
cb_payload.metadata.get('network_id'))
|
||||
self.assertEqual(router_id, cb_payload.resource_id)
|
||||
|
||||
def _create_router(self, gw_port=True, num_ports=2, create_routes=True):
|
||||
# GW CIDR: 10.0.0.0/24
|
||||
# Interface CIDRS: 10.0.1.0/24, 10.0.2.0/24, etc.
|
||||
router_id = uuidutils.generate_uuid()
|
||||
port_gw_cidr = netaddr.IPNetwork('10.0.0.0/24')
|
||||
rports = []
|
||||
if gw_port:
|
||||
port_gw = models_v2.Port(
|
||||
id=uuidutils.generate_uuid(),
|
||||
fixed_ips=[models_v2.IPAllocation(
|
||||
ip_address=str(port_gw_cidr.ip + 1))])
|
||||
rports.append(l3_models.RouterPort(router_id=router_id,
|
||||
port=port_gw))
|
||||
else:
|
||||
port_gw = None
|
||||
|
||||
port_cidrs = []
|
||||
port_subnets = []
|
||||
for idx in range(num_ports):
|
||||
cidr = port_gw_cidr.cidr.next(idx + 1)
|
||||
port = models_v2.Port(
|
||||
id=uuidutils.generate_uuid(),
|
||||
fixed_ips=[models_v2.IPAllocation(
|
||||
ip_address=str(cidr.ip + 1))])
|
||||
port_cidrs.append(cidr)
|
||||
rports.append(l3_models.RouterPort(router_id=router_id, port=port))
|
||||
port_subnets.append({'cidr': str(cidr)})
|
||||
|
||||
routes = []
|
||||
if create_routes:
|
||||
for cidr in [*port_cidrs, port_gw_cidr]:
|
||||
routes.append(l3_models.RouterRoute(
|
||||
destination=str(cidr.next(100)),
|
||||
nexthop=str(cidr.ip + 10)))
|
||||
return (l3_models.Router(
|
||||
id=router_id, attached_ports=rports, route_list=routes,
|
||||
gw_port_id=port_gw.id if port_gw else None), port_subnets)
|
||||
|
||||
def test__validate_gw_info(self):
|
||||
gw_network = mock.Mock(subnets=[mock.Mock(cidr='10.0.0.0/24')],
|
||||
external=True)
|
||||
router, port_subnets = self._create_router(gw_port=False)
|
||||
info = {'network_id': 'net_id'}
|
||||
with mock.patch.object(self.db._core_plugin, '_get_network',
|
||||
return_value=gw_network), \
|
||||
mock.patch.object(self.db._core_plugin, 'get_subnet',
|
||||
side_effect=port_subnets):
|
||||
self.assertEqual(
|
||||
'net_id',
|
||||
self.db._validate_gw_info(self.ctx, info, [], router))
|
||||
|
||||
def test__validate_gw_info_no_route_connectivity(self):
|
||||
gw_network = mock.Mock(subnets=[mock.Mock(cidr='10.50.0.0/24')],
|
||||
external=True)
|
||||
router, port_subnets = self._create_router(gw_port=False)
|
||||
info = {'network_id': 'net_id'}
|
||||
with mock.patch.object(self.db._core_plugin, '_get_network',
|
||||
return_value=gw_network), \
|
||||
mock.patch.object(self.db._core_plugin, 'get_subnet',
|
||||
side_effect=port_subnets):
|
||||
self.assertRaises(
|
||||
xroute_exc.InvalidRoutes, self.db._validate_gw_info, self.ctx,
|
||||
info, [], router)
|
||||
|
||||
def test__validate_gw_info_delete_gateway(self):
|
||||
router, port_subnets = self._create_router()
|
||||
info = {'network_id': None}
|
||||
with mock.patch.object(self.db._core_plugin, '_get_network',
|
||||
return_value=None), \
|
||||
mock.patch.object(self.db._core_plugin, 'get_subnet',
|
||||
side_effect=port_subnets):
|
||||
self.assertRaises(
|
||||
xroute_exc.InvalidRoutes, self.db._validate_gw_info, self.ctx,
|
||||
info, [], router)
|
||||
|
||||
def test__validate_gw_info_delete_gateway_no_route(self):
|
||||
gw_network = mock.Mock(subnets=[mock.Mock(cidr='10.50.0.0/24')],
|
||||
external=True)
|
||||
router, port_subnets = self._create_router(create_routes=False)
|
||||
info = {'network_id': None}
|
||||
with mock.patch.object(self.db._core_plugin, '_get_network',
|
||||
return_value=gw_network), \
|
||||
mock.patch.object(self.db._core_plugin, 'get_subnet',
|
||||
side_effect=port_subnets):
|
||||
self.assertIsNone(
|
||||
self.db._validate_gw_info(mock.ANY, info, [], router))
|
||||
|
||||
|
||||
class L3_NAT_db_mixin(base.BaseTestCase):
|
||||
def setUp(self):
|
||||
|
|
|
@ -0,0 +1,6 @@
|
|||
---
|
||||
features:
|
||||
- |
|
||||
Reject any router route or gateway update if not all route nexthops have
|
||||
connectivity with any gateway subnets CIDRs; in other words, all route
|
||||
nexthops IP addresses should belong to one gateway subnet CIDR.
|
Loading…
Reference in New Issue