Fix error returned when an HA router is updated to DVR

Before this patch, the code compares the 'ha' flag that
comes in from the user, and the current state of the 'distributed'
flag in the DB. This is wrong because if a router is currently
HA in the DB, and the update request contains only
{'distributed': True}, then the 'ha' flag from the request
is None and the error condition is never raised!

The reason the unit tests
(Specifically test_migrate_ha_router_to_distributed)
did not catch this issue is because
of another bug: The _update_router helper method in the L3 HA
unit tests had an 'ha' default value of True, when it should
have had a default value of None. Setting it to None fails
the unit test (Because it raises the wrong exception),
and the contents of the patch makes the unit test pass.

Change-Id: Ie979b6a8400490b578ded17dc6529529e9637b34
Closes-Bug: #1505375

diff --git a/neutron/db/l3_hamode_db.py b/neutron/db/l3_hamode_db.py
index 7b286869e1a9c3080e2caefef734e6a867e1c7c0..75eec2375bfefe2e755aac6358665c172fb00f97 100644 (file)
--- a/neutron/db/l3_hamode_db.py
+++ b/neutron/db/l3_hamode_db.py
@@ -404,16 +404,17 @@ class L3_HA_NAT_db_mixin(l3_dvr_db.L3_NAT_with_dvr_db_mixin):
         return router_dict
 
     def _update_router_db(self, context, router_id, data, gw_info):
-        ha = data.pop('ha', None)
-
-        if ha and data.get('distributed'):
+        router_db = self._get_router(context, router_id)
+        original_ha_state = router_db.extra_attributes.ha
+        if original_ha_state and data.get('distributed'):
             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_not_changed = ha is None or ha == router_db.extra_attributes.ha
+            ha = data.pop('ha', None)
+            ha_not_changed = ha is None or ha == original_ha_state
             if ha_not_changed:
                 return router_db
 

diff --git a/neutron/tests/unit/db/test_l3_hamode_db.py b/neutron/tests/unit/db/test_l3_hamode_db.py
index 69a6826dfe86a2298be89256a843abb7887fd362..b1fb45628c5320066946a6cd843cc550da4cf7cd 100644 (file)
--- a/neutron/tests/unit/db/test_l3_hamode_db.py
+++ b/neutron/tests/unit/db/test_l3_hamode_db.py
@@ -81,7 +81,7 @@ class L3HATestFramework(testlib_api.SqlTestCase):
             router['distributed'] = distributed
         return self.plugin.create_router(ctx, {'router': router})
 
-    def _update_router(self, router_id, ha=True, distributed=None, ctx=None):
+    def _update_router(self, router_id, ha=None, distributed=None, ctx=None):
         if ctx is None:
             ctx = self.admin_ctx
         data = {'ha': ha} if ha is not None else {}