Deleting HA router with attached port causes DB inconsistencies

When a HA router is being deleted with 'python-neutronclient'
while it has an attached interface the deletion will fail since
the router is in use. The order in which the deletion
is done is - first remove the HA interfaces from DB and
then delete the router. In this case the HA interfaces were
indeed deleted but the router itself was not (router is in use).
This causes the DB to be inconsistent where an HA router
exists in the DB while it's ports were removed from the DB.

This patch simply deletes the router first, and then
we know it's safe to remove it's HA interfaces as
well. If the router is in use and deletion fails
the HA interfaces remain intact.

Closes-Bug: #1402698

Change-Id: I956d0094ae6e2412e859d79feeb4003941d2bb4b

diff --git a/neutron/db/l3_hamode_db.py b/neutron/db/l3_hamode_db.py
index d3215bb47c02773882633663127d8e6d65239baf..6742abf30114596d9f6d755992b696f8c2c1cf95 100644 (file)
--- a/neutron/db/l3_hamode_db.py
+++ b/neutron/db/l3_hamode_db.py
@@ -417,6 +417,8 @@ class L3_HA_NAT_db_mixin(l3_dvr_db.L3_NAT_with_dvr_db_mixin):
 
     def delete_router(self, context, id):
         router_db = self._get_router(context, id)
+        super(L3_HA_NAT_db_mixin, self).delete_router(context, id)
+
         if router_db.extra_attributes.ha:
             ha_network = self.get_ha_network(context,
                                              router_db.tenant_id)
@@ -425,8 +427,6 @@ class L3_HA_NAT_db_mixin(l3_dvr_db.L3_NAT_with_dvr_db_mixin):
                     context, ha_network, router_db.extra_attributes.ha_vr_id)
                 self._delete_ha_interfaces(context, router_db.id)
 
-        return super(L3_HA_NAT_db_mixin, self).delete_router(context, id)
-
     def get_ha_router_port_bindings(self, context, router_ids, host=None):
         query = context.session.query(L3HARouterAgentPortBinding)
 

diff --git a/neutron/tests/unit/db/test_l3_ha_db.py b/neutron/tests/unit/db/test_l3_ha_db.py
index c8625c06232db2ef64699168d9b1e8ba74518dfc..604f37f0017b140f0bfb7ed8e99d5ea162343790 100644 (file)
--- a/neutron/tests/unit/db/test_l3_ha_db.py
+++ b/neutron/tests/unit/db/test_l3_ha_db.py
@@ -15,12 +15,14 @@
 import mock
 from oslo.config import cfg
 
+from neutron.api.v2 import attributes
 from neutron.common import constants
 from neutron import context
 from neutron.db import agents_db
 from neutron.db import common_db_mixin
 from neutron.db import l3_agentschedulers_db
 from neutron.db import l3_hamode_db
+from neutron.extensions import l3
 from neutron.extensions import l3_ext_ha_mode
 from neutron import manager
 from neutron.openstack.common import uuidutils
@@ -419,6 +421,48 @@ class L3HATestCase(L3HATestFramework):
         self.assertEqual(2, num_ha_candidates)
 
 
+class L3HAModeDbTestCase(L3HATestFramework):
+
+    def _create_network(self, plugin, ctx, name='net',
+                        tenant_id='tenant1'):
+        network = {'network': {'name': name,
+                               'shared': False,
+                               'admin_state_up': True,
+                               'tenant_id': tenant_id}}
+        return plugin.create_network(ctx, network)['id']
+
+    def _create_subnet(self, plugin, ctx, network_id, cidr='10.0.0.0/8',
+                       name='subnet', tenant_id='tenant1'):
+        subnet = {'subnet': {'name': name,
+                  'ip_version': 4,
+                  'network_id': network_id,
+                  'cidr': cidr,
+                  'gateway_ip': attributes.ATTR_NOT_SPECIFIED,
+                  'allocation_pools': attributes.ATTR_NOT_SPECIFIED,
+                  'dns_nameservers': attributes.ATTR_NOT_SPECIFIED,
+                  'host_routes': attributes.ATTR_NOT_SPECIFIED,
+                  'tenant_id': tenant_id,
+                  'enable_dhcp': True,
+                  'ipv6_ra_mode': attributes.ATTR_NOT_SPECIFIED}}
+        created_subnet = plugin.create_subnet(ctx, subnet)
+        return created_subnet
+
+    def test_remove_ha_in_use(self):
+        router = self._create_router(ctx=self.admin_ctx)
+        network_id = self._create_network(self.core_plugin, self.admin_ctx)
+        subnet = self._create_subnet(self.core_plugin, self.admin_ctx,
+                                     network_id)
+        interface_info = {'subnet_id': subnet['id']}
+        self.plugin.add_router_interface(self.admin_ctx,
+                                         router['id'],
+                                         interface_info)
+        self.assertRaises(l3.RouterInUse, self.plugin.delete_router,
+                          self.admin_ctx, router['id'])
+        bindings = self.plugin.get_ha_router_port_bindings(
+            self.admin_ctx, [router['id']])
+        self.assertEqual(2, len(bindings))
+
+
 class L3HAUserTestCase(L3HATestFramework):
 
     def setUp(self):