From: Assaf Muller <amuller@redhat.com>
Date: Mon, 12 Oct 2015 20:17:13 +0000 (-0400)
Subject: Fix error returned when an HA router is updated to DVR
X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=c1f2df2f6b87b435039d4cb52ccb14c0d9c53adc;p=openstack-build%2Fneutron-build.git

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 7b286869e..75eec2375 100644
--- 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 69a6826df..b1fb45628 100644
--- 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 {}