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

diff --git a/neutron/db/l3_hamode_db.py b/neutron/db/l3_hamode_db.py
index 75eec2375bfefe2e755aac6358665c172fb00f97..56fa3d0a756c915929031382f84140625984f89d 100644 (file)
--- 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 a2a3b859dbcd998efd7d78c051b04f1ae4a30d88..edb1e323f3da004bc7033e9ed89e7b751c2ca15e 100644 (file)
--- 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 f7b52fd5d69aeb57f58220f7549e119078bc21e3..10b0bf4e5bd369fcfb1b29013c6f85347680f88f 100644 (file)
--- 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)