From 8bda3c2ed33906ab2fbacc23f5775b4e0f4b76b0 Mon Sep 17 00:00:00 2001
From: Swaminathan Vasudevan <SVasudevan@suse.com>
Date: Sun, 1 Oct 2017 23:27:29 -0700
Subject: [PATCH] DVR: Cleanup ml2 dvr portbindings on migration

When a DVR router is migrated from distributed to
centralized, we are unbinding the router from the
agents, but the ml2 distributed portbindings for
the router port still remains intact.

This patch will fix the issue by deleting the
binding entry for multiple hosts.

Closes-Bug: #1718345
Change-Id: If139790eb336ff13b07b094151946af30322ad3e
(cherry picked from commit 32bfc3edeca935fc9c16b3019aa0af3091cd19e7)
---
 neutron/db/l3_dvr_db.py                 | 13 +++++++++++++
 neutron/plugins/ml2/plugin.py           |  7 +++++++
 neutron/tests/unit/db/test_l3_dvr_db.py | 26 +++++++++++++++++++++++++
 3 files changed, 46 insertions(+)

diff --git a/neutron/db/l3_dvr_db.py b/neutron/db/l3_dvr_db.py
index fef38b64dc4..99533df61b4 100644
--- a/neutron/db/l3_dvr_db.py
+++ b/neutron/db/l3_dvr_db.py
@@ -156,6 +156,19 @@ class DVRResourceOperationHandler(object):
             payload.context, payload.desired_state,
             'distributed', migrating_to_distributed)
 
+    @registry.receives(resources.ROUTER, [events.AFTER_UPDATE],
+                       priority_group.PRIORITY_ROUTER_EXTENDED_ATTRIBUTE)
+    def _delete_distributed_port_bindings_after_change(self, resource, event,
+                                                       trigger, context,
+                                                       router_id, router,
+                                                       request_attrs,
+                                                       router_db, **kwargs):
+        old_router = kwargs['old_router']
+        if (old_router and old_router['distributed'] and not
+                router['distributed']):
+            self._core_plugin.delete_distributed_port_bindings_by_router_id(
+                context.elevated(), router_db['id'])
+
     @registry.receives(resources.ROUTER, [events.AFTER_UPDATE],
                        priority_group.PRIORITY_ROUTER_EXTENDED_ATTRIBUTE)
     def _delete_snat_interfaces_after_change(self, resource, event, trigger,
diff --git a/neutron/plugins/ml2/plugin.py b/neutron/plugins/ml2/plugin.py
index fc4286b4f72..6630eaa5438 100644
--- a/neutron/plugins/ml2/plugin.py
+++ b/neutron/plugins/ml2/plugin.py
@@ -1573,6 +1573,13 @@ class Ml2Plugin(db_base_plugin_v2.NeutronDbPluginV2,
         # merge into session to reflect changes
         binding.persist_state_to_session(plugin_context.session)
 
+    def delete_distributed_port_bindings_by_router_id(self, context,
+                                                      router_id):
+        for binding in (context.session.query(models.DistributedPortBinding).
+                filter_by(router_id=router_id)):
+            db.clear_binding_levels(context, binding.port_id, binding.host)
+            context.session.delete(binding)
+
     @utils.transaction_guard
     @db_api.retry_if_session_inactive()
     def update_distributed_port_binding(self, context, id, port):
diff --git a/neutron/tests/unit/db/test_l3_dvr_db.py b/neutron/tests/unit/db/test_l3_dvr_db.py
index a1f854932d5..0f65f6a4319 100644
--- a/neutron/tests/unit/db/test_l3_dvr_db.py
+++ b/neutron/tests/unit/db/test_l3_dvr_db.py
@@ -130,6 +130,32 @@ class L3DvrTestCase(test_db_base_plugin_v2.NeutronDbPluginV2TestCase):
         self.assertEqual(1,
                          self.mixin._migrate_router_ports.call_count)
 
+    def test_update_router_db_distributed_to_centralized(self):
+        router = {'name': 'foo_router', 'admin_state_up': True,
+                  'distributed': True}
+        agent = {'id': _uuid(), 'host': 'xyz'}
+        router_db = self._create_router(router)
+        router_id = router_db['id']
+        self.assertTrue(router_db.extra_attributes.distributed)
+        self.mixin._get_router = mock.Mock(return_value=router_db)
+        self.mixin._validate_router_migration = mock.Mock()
+        self.mixin._migrate_router_ports = mock.Mock()
+        self.mixin._core_plugin.\
+            delete_distributed_port_bindings_by_router_id = mock.Mock()
+        self.mixin.list_l3_agents_hosting_router = mock.Mock(
+            return_value={'agents': [agent]})
+        self.mixin._unbind_router = mock.Mock()
+        updated_router = self.mixin.update_router(self.ctx, router_id,
+            {'router': {'distributed': False}})
+        # Assert that the DB value has changed
+        self.assertFalse(updated_router['distributed'])
+        self.assertEqual(1,
+                         self.mixin._migrate_router_ports.call_count)
+        self.assertEqual(
+            1,
+            self.mixin._core_plugin.
+            delete_distributed_port_bindings_by_router_id.call_count)
+
     def _test_get_device_owner(self, is_distributed=False,
                                expected=const.DEVICE_OWNER_ROUTER_INTF,
                                pass_router_id=True):