From 416c76bc6e01ef433506e4aa4ebd7c76b57acc51 Mon Sep 17 00:00:00 2001 From: John Schwarz Date: Mon, 12 Oct 2015 16:54:17 +0300 Subject: [PATCH] Support migrating of legacy routers to HA and back This patch adds support for migration of legacy routers to HA and vice-versa. This patch also: 1. Reverts I4171ab481e3943e0110bd9a300d965bbebe44871, which was used to disable such migrations until support was inserted to the codebase. 2. Adds an exception to indicate that such migrations are only available on routers that have their admin_state_up set to False. Closes-Bug: #1365426 DocImpact (Handled in patch 233695) Change-Id: Ie92f8033f47e1bf9ba6310373b3bfc9833317580 --- neutron/db/l3_hamode_db.py | 36 ++++++++++-- neutron/extensions/l3_ext_ha_mode.py | 2 +- neutron/tests/unit/db/test_l3_hamode_db.py | 68 +++++++++++++++++++--- 3 files changed, 92 insertions(+), 14 deletions(-) diff --git a/neutron/db/l3_hamode_db.py b/neutron/db/l3_hamode_db.py index 75eec2375..56fa3d0a7 100644 --- a/neutron/db/l3_hamode_db.py +++ b/neutron/db/l3_hamode_db.py @@ -23,6 +23,7 @@ from sqlalchemy import orm from neutron.api.v2 import attributes from neutron.common import constants +from neutron.common import exceptions as n_exc from neutron.common import utils as n_utils from neutron.db import agents_db from neutron.db import l3_dvr_db @@ -405,28 +406,47 @@ class L3_HA_NAT_db_mixin(l3_dvr_db.L3_NAT_with_dvr_db_mixin): def _update_router_db(self, context, router_id, data, gw_info): router_db = self._get_router(context, router_id) + + original_distributed_state = router_db.extra_attributes.distributed original_ha_state = router_db.extra_attributes.ha - if original_ha_state and data.get('distributed'): + + requested_ha_state = data.pop('ha', None) + requested_distributed_state = data.get('distributed', None) + + if ((original_ha_state and requested_distributed_state) or + (requested_ha_state and original_distributed_state) or + (requested_ha_state and requested_distributed_state)): raise l3_ha.DistributedHARouterNotSupported() with context.session.begin(subtransactions=True): router_db = super(L3_HA_NAT_db_mixin, self)._update_router_db( context, router_id, data, gw_info) - ha = data.pop('ha', None) - ha_not_changed = ha is None or ha == original_ha_state + ha_not_changed = (requested_ha_state is None or + requested_ha_state == original_ha_state) if ha_not_changed: return router_db + if router_db.admin_state_up: + msg = _('Cannot change HA attribute of active routers. Please ' + 'set router admin_state_up to False prior to upgrade.') + raise n_exc.BadRequest(resource='router', msg=msg) + ha_network = self.get_ha_network(context, router_db.tenant_id) - router_db.extra_attributes.ha = ha - if not ha: + router_db.extra_attributes.ha = requested_ha_state + if not requested_ha_state: self._delete_vr_id_allocation( context, ha_network, router_db.extra_attributes.ha_vr_id) router_db.extra_attributes.ha_vr_id = None - if ha: + # The HA attribute has changed. First unbind the router from agents + # to force a proper re-scheduling to agents. + # TODO(jschwarz): This will have to be more selective to get HA + DVR + # working (Only unbind from dvr_snat nodes). + self._unbind_ha_router(context, router_id) + + if requested_ha_state: if not ha_network: ha_network = self._create_ha_network(context, router_db.tenant_id) @@ -452,6 +472,10 @@ 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) + def _unbind_ha_router(self, context, router_id): + for agent in self.get_l3_agents_hosting_routers(context, [router_id]): + self.remove_router_from_l3_agent(context, agent['id'], router_id) + def get_ha_router_port_bindings(self, context, router_ids, host=None): if not router_ids: return [] diff --git a/neutron/extensions/l3_ext_ha_mode.py b/neutron/extensions/l3_ext_ha_mode.py index a2a3b859d..edb1e323f 100644 --- a/neutron/extensions/l3_ext_ha_mode.py +++ b/neutron/extensions/l3_ext_ha_mode.py @@ -21,7 +21,7 @@ from neutron.common import exceptions HA_INFO = 'ha' EXTENDED_ATTRIBUTES_2_0 = { 'routers': { - HA_INFO: {'allow_post': True, 'allow_put': False, + HA_INFO: {'allow_post': True, 'allow_put': True, 'default': attributes.ATTR_NOT_SPECIFIED, 'is_visible': True, 'enforce_policy': True, 'convert_to': attributes.convert_to_boolean_if_not_none} diff --git a/neutron/tests/unit/db/test_l3_hamode_db.py b/neutron/tests/unit/db/test_l3_hamode_db.py index f7b52fd5d..10b0bf4e5 100644 --- a/neutron/tests/unit/db/test_l3_hamode_db.py +++ b/neutron/tests/unit/db/test_l3_hamode_db.py @@ -19,6 +19,7 @@ from oslo_utils import uuidutils from neutron.api.rpc.handlers import l3_rpc from neutron.api.v2 import attributes from neutron.common import constants +from neutron.common import exceptions as n_exc from neutron import context from neutron.db import agents_db from neutron.db import common_db_mixin @@ -72,12 +73,20 @@ class L3HATestFramework(testlib_api.SqlTestCase): router['distributed'] = distributed return self.plugin.create_router(ctx, {'router': router}) - def _update_router(self, router_id, ha=None, distributed=None, ctx=None): + def _migrate_router(self, router_id, ha): + self._update_router(router_id, admin_state=False) + self._update_router(router_id, ha=ha) + return self._update_router(router_id, admin_state=True) + + def _update_router(self, router_id, ha=None, distributed=None, ctx=None, + admin_state=None): if ctx is None: ctx = self.admin_ctx data = {'ha': ha} if ha is not None else {} if distributed is not None: data['distributed'] = distributed + if admin_state is not None: + data['admin_state_up'] = admin_state return self.plugin._update_router_db(ctx, router_id, data, None) @@ -190,7 +199,7 @@ class L3HATestCase(L3HATestFramework): router = self._create_router() self.assertTrue(router['ha']) - router = self._update_router(router['id'], ha=False) + router = self._migrate_router(router['id'], False) self.assertFalse(router.extra_attributes['ha']) self.assertIsNone(router.extra_attributes['ha_vr_id']) @@ -198,10 +207,17 @@ class L3HATestCase(L3HATestFramework): router = self._create_router(ha=False) self.assertFalse(router['ha']) - router = self._update_router(router['id'], ha=True) + router = self._migrate_router(router['id'], True) self.assertTrue(router.extra_attributes['ha']) self.assertIsNotNone(router.extra_attributes['ha_vr_id']) + def test_migration_requires_admin_state_down(self): + router = self._create_router(ha=False) + self.assertRaises(n_exc.BadRequest, + self._update_router, + router['id'], + ha=True) + def test_migrate_ha_router_to_distributed(self): router = self._create_router() self.assertTrue(router['ha']) @@ -211,6 +227,44 @@ class L3HATestCase(L3HATestFramework): router['id'], distributed=True) + def test_migrate_distributed_router_to_ha(self): + router = self._create_router(ha=False, distributed=True) + self.assertFalse(router['ha']) + self.assertTrue(router['distributed']) + + self.assertRaises(l3_ext_ha_mode.DistributedHARouterNotSupported, + self._update_router, + router['id'], + ha=True) + + def test_migrate_legacy_router_to_distributed_and_ha(self): + router = self._create_router(ha=False, distributed=False) + self.assertFalse(router['ha']) + self.assertFalse(router['distributed']) + + self.assertRaises(l3_ext_ha_mode.DistributedHARouterNotSupported, + self._update_router, + router['id'], + ha=True, + distributed=True) + + def test_unbind_ha_router(self): + router = self._create_router() + self._bind_router(router['id']) + + bound_agents = self.plugin.get_l3_agents_hosting_routers( + self.admin_ctx, [router['id']]) + self.assertEqual(2, len(bound_agents)) + + with mock.patch.object(manager.NeutronManager, + 'get_service_plugins') as mock_manager: + self.plugin._unbind_ha_router(self.admin_ctx, router['id']) + + bound_agents = self.plugin.get_l3_agents_hosting_routers( + self.admin_ctx, [router['id']]) + self.assertEqual(0, len(bound_agents)) + self.assertEqual(2, mock_manager.call_count) + def test_l3_agent_routers_query_interface(self): router = self._create_router() self._bind_router(router['id']) @@ -251,7 +305,7 @@ class L3HATestCase(L3HATestFramework): else: self.assertIsNotNone(interface) - self._update_router(router['id'], to_ha) + self._migrate_router(router['id'], to_ha) routers = self.plugin.get_ha_sync_data_for_host(self.admin_ctx) router = routers[0] interface = router.get(constants.HA_INTERFACE_KEY) @@ -273,7 +327,7 @@ class L3HATestCase(L3HATestFramework): def test_update_router_to_ha_notifies_agent(self): router = self._create_router(ha=False) self.notif_m.reset_mock() - self._update_router(router['id'], ha=True) + self._migrate_router(router['id'], True) self.assertTrue(self.notif_m.called) def test_unique_vr_id_between_routers(self): @@ -333,7 +387,7 @@ class L3HATestCase(L3HATestFramework): allocs_before = self.plugin._get_allocated_vr_id(self.admin_ctx, network.network_id) router = self._create_router() - self._update_router(router['id'], ha=False) + self._migrate_router(router['id'], False) allocs_after = self.plugin._get_allocated_vr_id(self.admin_ctx, network.network_id) self.assertEqual(allocs_before, allocs_after) @@ -617,7 +671,7 @@ class L3HAUserTestCase(L3HATestFramework): def test_update_router(self): router = self._create_router(ctx=self.user_ctx) - self._update_router(router['id'], ha=False, ctx=self.user_ctx) + self._update_router(router['id'], ctx=self.user_ctx) def test_delete_router(self): router = self._create_router(ctx=self.user_ctx) -- 2.45.2