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

diff --git a/neutron/db/l3_db.py b/neutron/db/l3_db.py
index 6464bc6e9711e1c545fb4e6c0df2cf39bd18f6be..d92fa6c7212e865ef661c2d5b2b6c6dcdfd3f40c 100644 (file)
--- 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 99b51919f0e936ff9ec11ca0fdb2dc816fa10fe4..b38326ff385550b068f782a1312f467ea16c4ffa 100644 (file)
--- 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 de7488e21080a3c5c2227942f5eb5d786dab8928..313ff7e3b472e314abe8bf4b6e3e4842bafbed43 100644 (file)
--- 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 c91944932663e4c2d2dd8996d04620b7e229bf45..0690e7a812aff1f3f0a38ae57f990a0626df0824 100644 (file)
--- 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',