From e2666293c449ca98c52fc7f661be43323ee36828 Mon Sep 17 00:00:00 2001 From: Andrew Boik Date: Fri, 27 Feb 2015 18:48:29 -0500 Subject: [PATCH] Allow update of ext gateway IP's w/out port delete (Patch set #5 for the multiple-ipv6-prefixes blueprint) Updating an external gateway port currently triggers a port-delete followed by a port-create. In the multi-prefix case, if a second subnet is added to an external gateway port, the port will be deleted, freeing the original IP allocation, and then the port will be recreated with new IP allocations from the two subnets. This is undesirable as the port can't keep the same IP address from the original subnet. This patch modifies the behavior so that a fixed-ip change on an external gateway port will cause a port-update instead of a delete/create. If the gateway port network id has changed, however, the port will be deleted and recreated as before. Change-Id: I5b19d3b167668ce5c04e7ce8adc63249a4501d0e Partially-implements: blueprint multiple-ipv6-prefixes --- neutron/db/l3_db.py | 35 ++++++++++++++----------- neutron/db/l3_dvr_db.py | 11 ++++---- neutron/tests/unit/db/test_l3_dvr_db.py | 2 +- neutron/tests/unit/test_l3_plugin.py | 29 ++++++++++++++++++++ 4 files changed, 54 insertions(+), 23 deletions(-) diff --git a/neutron/db/l3_db.py b/neutron/db/l3_db.py index 6464bc6e9..d92fa6c72 100644 --- a/neutron/db/l3_db.py +++ b/neutron/db/l3_db.py @@ -273,9 +273,6 @@ class L3_NAT_dbonly_mixin(l3.RouterPluginBase): return candidates def _create_router_gw_port(self, context, router, network_id, ext_ips): - if ext_ips and len(ext_ips) > 1: - msg = _("Routers support only 1 external IP") - raise n_exc.BadRequest(resource='router', msg=msg) # Port has no 'tenant-id', as it is hidden from user gw_port = self._core_plugin.create_port(context.elevated(), { 'port': {'tenant_id': '', # intentionally not set @@ -325,13 +322,10 @@ class L3_NAT_dbonly_mixin(l3.RouterPluginBase): raise n_exc.BadRequest(resource='router', msg=msg) return network_id - def _delete_current_gw_port(self, context, router_id, router, new_network, - ext_ip_change): - """Delete gw port if attached to an old network or IPs changed.""" + def _delete_current_gw_port(self, context, router_id, router, new_network): + """Delete gw port if attached to an old network.""" port_requires_deletion = ( - router.gw_port and - (router.gw_port['network_id'] != new_network or ext_ip_change) - ) + router.gw_port and router.gw_port['network_id'] != new_network) if not port_requires_deletion: return admin_ctx = context.elevated() @@ -362,9 +356,9 @@ class L3_NAT_dbonly_mixin(l3.RouterPluginBase): raise l3.RouterInUse(router_id=router_id, reason=e) def _create_gw_port(self, context, router_id, router, new_network, - ext_ips, ext_ip_change): + ext_ips): new_valid_gw_port_attachment = ( - new_network and (not router.gw_port or ext_ip_change or + new_network and (not router.gw_port or router.gw_port['network_id'] != new_network)) if new_valid_gw_port_attachment: subnets = self._core_plugin._get_subnets_by_network(context, @@ -375,6 +369,11 @@ class L3_NAT_dbonly_mixin(l3.RouterPluginBase): subnet['cidr']) self._create_router_gw_port(context, router, new_network, ext_ips) + def _update_current_gw_port(self, context, router_id, router, ext_ips): + self._core_plugin.update_port(context, router.gw_port['id'], {'port': + {'fixed_ips': ext_ips}}) + context.session.expire(router.gw_port) + def _update_router_gw_info(self, context, router_id, info, router=None): # TODO(salvatore-orlando): guarantee atomic behavior also across # operations that span beyond the model classes handled by this @@ -385,10 +384,14 @@ class L3_NAT_dbonly_mixin(l3.RouterPluginBase): 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) - self._delete_current_gw_port(context, router_id, router, network_id, - ext_ip_change) - self._create_gw_port(context, router_id, router, network_id, ext_ips, - ext_ip_change) + 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) + else: + self._delete_current_gw_port(context, router_id, router, + network_id) + self._create_gw_port(context, router_id, router, network_id, + ext_ips) def _check_for_external_ip_change(self, context, gw_port, ext_ips): # determine if new external IPs differ from the existing fixed_ips @@ -425,7 +428,7 @@ class L3_NAT_dbonly_mixin(l3.RouterPluginBase): #TODO(nati) Refactor here when we have router insertion model router = self._ensure_router_not_in_use(context, id) - self._delete_current_gw_port(context, id, router, None, False) + self._delete_current_gw_port(context, id, router, None) router_ports = router.attached_ports.all() for rp in router_ports: diff --git a/neutron/db/l3_dvr_db.py b/neutron/db/l3_dvr_db.py index 99b51919f..b38326ff3 100644 --- a/neutron/db/l3_dvr_db.py +++ b/neutron/db/l3_dvr_db.py @@ -151,20 +151,19 @@ class L3_NAT_with_dvr_db_mixin(l3_db.L3_NAT_db_mixin, agent['id']) return router_db - def _delete_current_gw_port(self, context, router_id, router, new_network, - ext_ip_change): + def _delete_current_gw_port(self, context, router_id, router, new_network): super(L3_NAT_with_dvr_db_mixin, self)._delete_current_gw_port(context, router_id, - router, new_network, ext_ip_change) + router, new_network) if router.extra_attributes.distributed: self.delete_csnat_router_interface_ports( context.elevated(), router) - def _create_gw_port(self, context, router_id, router, new_network, ext_ips, - ext_ip_change): + def _create_gw_port(self, context, router_id, router, new_network, + ext_ips): super(L3_NAT_with_dvr_db_mixin, self)._create_gw_port(context, router_id, router, new_network, - ext_ips, ext_ip_change) + ext_ips) # Make sure that the gateway port exists before creating the # snat interface ports for distributed router. if router.extra_attributes.distributed and router.gw_port: diff --git a/neutron/tests/unit/db/test_l3_dvr_db.py b/neutron/tests/unit/db/test_l3_dvr_db.py index de7488e21..313ff7e3b 100644 --- a/neutron/tests/unit/db/test_l3_dvr_db.py +++ b/neutron/tests/unit/db/test_l3_dvr_db.py @@ -200,7 +200,7 @@ class L3DvrTestCase(testlib_api.SqlTestCase): ) as (cw, cs): self.mixin._create_gw_port( self.ctx, router_id, router_db, mock.ANY, - mock.ANY, mock.ANY) + mock.ANY) self.assertFalse(cs.call_count) def test_build_routers_list_with_gw_port_mismatch(self): diff --git a/neutron/tests/unit/test_l3_plugin.py b/neutron/tests/unit/test_l3_plugin.py index c91944932..0690e7a81 100644 --- a/neutron/tests/unit/test_l3_plugin.py +++ b/neutron/tests/unit/test_l3_plugin.py @@ -848,6 +848,35 @@ class L3NatTestCaseBase(L3NatTestCaseMixin): fip['floatingip']['router_id'], None, expected_code=exc.HTTPConflict.code) + def test_router_update_gateway_add_multiple_prefixes_ipv6(self): + with self.network() as n: + with self.subnet(network=n) as s1, \ + self.subnet(network=n, ip_version=6, cidr='2001:db8::/32') \ + as s2, (self.router()) as r: + self._set_net_external(n['network']['id']) + res1 = self._add_external_gateway_to_router( + r['router']['id'], + n['network']['id'], + ext_ips=[{'subnet_id': s1['subnet']['id']}]) + fip1 = (res1['router']['external_gateway_info'] + ['external_fixed_ips'][0]) + self.assertEqual(s1['subnet']['id'], fip1['subnet_id']) + res2 = self._add_external_gateway_to_router( + r['router']['id'], + n['network']['id'], + ext_ips=[{'ip_address': fip1['ip_address'], + 'subnet_id': s1['subnet']['id']}, + {'subnet_id': s2['subnet']['id']}]) + self.assertEqual(fip1, res2['router']['external_gateway_info'] + ['external_fixed_ips'][0]) + fip2 = (res2['router']['external_gateway_info'] + ['external_fixed_ips'][1]) + self.assertEqual(s2['subnet']['id'], fip2['subnet_id']) + self.assertNotEqual(fip1['subnet_id'], + fip2['subnet_id']) + self.assertNotEqual(fip1['ip_address'], + fip2['ip_address']) + def _test_router_add_interface_subnet(self, router, subnet, msg=None): exp_notifications = ['router.create.start', 'router.create.end', -- 2.45.2