Handle portinfo msg after port deletion in NEC plugin

Fixes bug 1187701

Change-Id: I9c5ef3320ad6e9438dca9ccccf27f4cedfe4ceec

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,12 +654,15 @@ 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)
+            portinfo = ndb.get_portinfo(session, id)
-            if port and 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', ''))
+            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)
@@ -675,7 +679,13 @@ class NECPluginV2RPCCallbacks(object):
                           {'registered': portinfo.datapath_id,
                            'received': datapath_id})
                 continue
-            port = self.plugin.get_port(rpc_context, id)
-            if port:
             ndb.del_portinfo(session, id)
+            port = self._get_port(rpc_context, id)
+            if port:
                 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

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,16 +136,38 @@ 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])
+
+        # Ensure port deletion is called once.
+        self.assertEqual(self.ofc.delete_ofc_port.call_count, 1)
         self.assertIsNone(self._get_portinfo(port_id))
 
         expected = [
@@ -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