Handle portinfo msg after port deletion in NEC plugin

Fixes bug 1187701

Change-Id: I9c5ef3320ad6e9438dca9ccccf27f4cedfe4ceec

diff --git a/quantum/plugins/nec/nec_plugin.py b/quantum/plugins/nec/nec_plugin.py
index 0d62f0b29b9ebdde840aa0a5ba0354644e5e6732..4120bc42e40d04ee38fc6cabbb3cc46cdf1674b5 100644 (file)
--- a/quantum/plugins/nec/nec_plugin.py
+++ b/quantum/plugins/nec/nec_plugin.py
@@ -19,6 +19,7 @@
 from quantum.agent import securitygroups_rpc as sg_rpc
 from quantum.api.rpc.agentnotifiers import dhcp_rpc_agent_api
 from quantum.api.rpc.agentnotifiers import l3_rpc_agent_api
+from quantum.common import exceptions as q_exc
 from quantum.common import rpc as q_rpc
 from quantum.common import topics
 from quantum.db import agents_db
@@ -653,13 +654,16 @@ class NECPluginV2RPCCallbacks(object):
         session = rpc_context.session
         for p in kwargs.get('port_added', []):
             id = p['id']
-            port = self.plugin.get_port(rpc_context, id)
-            if port and ndb.get_portinfo(session, id):
+            portinfo = ndb.get_portinfo(session, id)
+            if portinfo:
                 ndb.del_portinfo(session, id)
-                self.plugin.deactivate_port(rpc_context, port)
             ndb.add_portinfo(session, id, datapath_id, p['port_no'],
                              mac=p.get('mac', ''))
-            self.plugin.activate_port_if_ready(rpc_context, port)
+            port = self._get_port(rpc_context, id)
+            if port:
+                if portinfo:
+                    self.plugin.deactivate_port(rpc_context, port)
+                self.plugin.activate_port_if_ready(rpc_context, port)
         for id in kwargs.get('port_removed', []):
             portinfo = ndb.get_portinfo(session, id)
             if not portinfo:
@@ -675,7 +679,13 @@ class NECPluginV2RPCCallbacks(object):
                           {'registered': portinfo.datapath_id,
                            'received': datapath_id})
                 continue
-            port = self.plugin.get_port(rpc_context, id)
+            ndb.del_portinfo(session, id)
+            port = self._get_port(rpc_context, id)
             if port:
-                ndb.del_portinfo(session, id)
                 self.plugin.deactivate_port(rpc_context, port)
+
+    def _get_port(self, context, port_id):
+        try:
+            return self.plugin.get_port(context, port_id)
+        except q_exc.PortNotFound:
+            return None

diff --git a/quantum/tests/unit/nec/test_nec_plugin.py b/quantum/tests/unit/nec/test_nec_plugin.py
index 9638b6cbd684d18d977decfa0b23d16a236a135a..94b11c6b7edc4c03156c6dd43179c4457df2153f 100644 (file)
--- a/quantum/tests/unit/nec/test_nec_plugin.py
+++ b/quantum/tests/unit/nec/test_nec_plugin.py
@@ -114,11 +114,12 @@ class TestNecPortsV2Callback(NecPluginV2TestCase):
     def _get_portinfo(self, port_id):
         return ndb.get_portinfo(self.context.session, port_id)
 
-    def test_port_create(self):
+    def test_portinfo_create(self):
         with self.port() as port:
             port_id = port['port']['id']
             sport = self.plugin.get_port(self.context, port_id)
             self.assertEqual(sport['status'], 'DOWN')
+            self.assertEqual(self.ofc.create_ofc_port.call_count, 0)
             self.assertIsNone(self._get_portinfo(port_id))
 
             portinfo = {'id': port_id, 'port_no': 123}
@@ -126,6 +127,7 @@ class TestNecPortsV2Callback(NecPluginV2TestCase):
 
             sport = self.plugin.get_port(self.context, port_id)
             self.assertEqual(sport['status'], 'ACTIVE')
+            self.assertEqual(self.ofc.create_ofc_port.call_count, 1)
             self.assertIsNotNone(self._get_portinfo(port_id))
 
             expected = [
@@ -134,17 +136,39 @@ class TestNecPortsV2Callback(NecPluginV2TestCase):
             ]
             self.ofc.assert_has_calls(expected)
 
-    def test_port_delete(self):
+    def test_portinfo_delete_before_port_deletion(self):
+        self._test_portinfo_delete()
+
+    def test_portinfo_delete_after_port_deletion(self):
+        self._test_portinfo_delete(portinfo_delete_first=False)
+
+    def _test_portinfo_delete(self, portinfo_delete_first=True):
         with self.port() as port:
             port_id = port['port']['id']
             portinfo = {'id': port_id, 'port_no': 456}
+            self.assertEqual(self.ofc.create_ofc_port.call_count, 0)
             self.assertIsNone(self._get_portinfo(port_id))
 
             self._rpcapi_update_ports(added=[portinfo])
+            self.assertEqual(self.ofc.create_ofc_port.call_count, 1)
+            self.assertEqual(self.ofc.delete_ofc_port.call_count, 0)
             self.assertIsNotNone(self._get_portinfo(port_id))
 
+            # Before port-deletion, switch port removed message is sent.
+            if portinfo_delete_first:
+                self._rpcapi_update_ports(removed=[port_id])
+                self.assertEqual(self.ofc.delete_ofc_port.call_count, 1)
+                self.assertIsNone(self._get_portinfo(port_id))
+
+        # The port is expected to delete when exiting with-clause.
+        if not portinfo_delete_first:
+            self.assertEqual(self.ofc.delete_ofc_port.call_count, 1)
+            self.assertIsNotNone(self._get_portinfo(port_id))
             self._rpcapi_update_ports(removed=[port_id])
-            self.assertIsNone(self._get_portinfo(port_id))
+
+        # Ensure port deletion is called once.
+        self.assertEqual(self.ofc.delete_ofc_port.call_count, 1)
+        self.assertIsNone(self._get_portinfo(port_id))
 
         expected = [
             mock.call.exists_ofc_port(mock.ANY, port_id),
@@ -154,6 +178,54 @@ class TestNecPortsV2Callback(NecPluginV2TestCase):
         ]
         self.ofc.assert_has_calls(expected)
 
+    def test_portinfo_added_unknown_port(self):
+        portinfo = {'id': 'dummy-p1', 'port_no': 123}
+        self._rpcapi_update_ports(added=[portinfo])
+        self.assertIsNotNone(ndb.get_portinfo(self.context.session,
+                                              'dummy-p1'))
+        self.assertEqual(self.ofc.exists_ofc_port.call_count, 0)
+        self.assertEqual(self.ofc.create_ofc_port.call_count, 0)
+
+    def _test_portinfo_change(self, portinfo_change_first=True):
+        with self.port() as port:
+            port_id = port['port']['id']
+            self.assertEqual(self.ofc.create_ofc_port.call_count, 0)
+
+            portinfo = {'id': port_id, 'port_no': 123}
+            self._rpcapi_update_ports(added=[portinfo])
+            self.assertEqual(self.ofc.create_ofc_port.call_count, 1)
+            self.assertEqual(self.ofc.delete_ofc_port.call_count, 0)
+            self.assertEqual(ndb.get_portinfo(self.context.session,
+                                              port_id).port_no, 123)
+
+            if portinfo_change_first:
+                portinfo = {'id': port_id, 'port_no': 456}
+                self._rpcapi_update_ports(added=[portinfo])
+                # OFC port is recreated.
+                self.assertEqual(self.ofc.create_ofc_port.call_count, 2)
+                self.assertEqual(self.ofc.delete_ofc_port.call_count, 1)
+                self.assertEqual(ndb.get_portinfo(self.context.session,
+                                                  port_id).port_no, 456)
+
+        if not portinfo_change_first:
+            # The port is expected to delete when exiting with-clause.
+            self.assertEqual(self.ofc.create_ofc_port.call_count, 1)
+            self.assertEqual(self.ofc.delete_ofc_port.call_count, 1)
+
+            portinfo = {'id': port_id, 'port_no': 456}
+            self._rpcapi_update_ports(added=[portinfo])
+            # No OFC operations are expected.
+            self.assertEqual(self.ofc.create_ofc_port.call_count, 1)
+            self.assertEqual(self.ofc.delete_ofc_port.call_count, 1)
+            self.assertEqual(ndb.get_portinfo(self.context.session,
+                                              port_id).port_no, 456)
+
+    def test_portinfo_change(self):
+        self._test_portinfo_change()
+
+    def test_portinfo_change_for_nonexisting_port(self):
+        self._test_portinfo_change(portinfo_change_first=False)
+
     def test_port_migration(self):
         agent_id_a, datapath_id_a, port_no_a = 'nec-q-agent.aa', '0xaaa', 10
         agent_id_b, datapath_id_b, port_no_b = 'nec-q-agent.bb', '0xbbb', 11