From 7632f5dba619bbd9286708f59c31fa392b9c419c Mon Sep 17 00:00:00 2001 From: Yoni Shafrir Date: Mon, 22 Dec 2014 11:25:39 +0200 Subject: [PATCH] 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 --- neutron/db/l3_hamode_db.py | 4 +-- neutron/tests/unit/db/test_l3_ha_db.py | 44 ++++++++++++++++++++++++++ 2 files changed, 46 insertions(+), 2 deletions(-) diff --git a/neutron/db/l3_hamode_db.py b/neutron/db/l3_hamode_db.py index d3215bb47c0..6742abf3011 100644 --- 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 c8625c06232..604f37f0017 100644 --- 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):