Merge "OFAgent: Avoid processing ports which are not yet ready"

neutron/plugins/ofagent/agent/ofa_neutron_agent.py

@@ -648,7 +648,7 @@ class OFANeutronAgent(sg_rpc.SecurityGroupAgentRpcCallbackMixin):
         if cur_tag != str(lvm.vlan):
             self.int_br.set_db_attribute("Port", port.port_name, "tag",
                                          str(lvm.vlan))
-            if int(port.ofport) != -1:
+            if port.ofport != -1:
                 match = self.int_br.ofparser.OFPMatch(in_port=port.ofport)
                 msg = self.int_br.ofparser.OFPFlowMod(
                     self.int_br.datapath,
@@ -990,6 +990,13 @@ class OFANeutronAgent(sg_rpc.SecurityGroupAgentRpcCallbackMixin):
     def treat_vif_port(self, vif_port, port_id, network_id, network_type,
                        physical_network, segmentation_id, admin_state_up):
         if vif_port:
+            # When this function is called for a port, the port should have
+            # an OVS ofport configured, as only these ports were considered
+            # for being treated. If that does not happen, it is a potential
+            # error condition of which operators should be aware
+            if not vif_port.ofport:
+                LOG.warn(_("VIF port: %s has no ofport configured, and might "
+                           "not be able to transmit"), vif_port.vif_id)
             if admin_state_up:
                 self.port_bound(vif_port, network_id, network_type,
                                 physical_network, segmentation_id)
@@ -1060,6 +1067,14 @@ class OFANeutronAgent(sg_rpc.SecurityGroupAgentRpcCallbackMixin):
         resync = False
         for device in devices:
             LOG.debug(_("Processing port %s"), device)
+            port = self.int_br.get_vif_port_by_id(device)
+            if not port:
+                # The port has disappeared and should not be processed
+                # There is no need to put the port DOWN in the plugin as
+                # it never went up in the first place
+                LOG.info(_("Port %s was not found on the integration bridge "
+                           "and will therefore not be processed"), device)
+                continue
             try:
                 details = self.plugin_rpc.get_device_details(self.context,
                                                              device,
@@ -1070,7 +1085,6 @@ class OFANeutronAgent(sg_rpc.SecurityGroupAgentRpcCallbackMixin):
                           {'device': device, 'e': e})
                 resync = True
                 continue
-            port = self.int_br.get_vif_port_by_id(details['device'])
             if 'port_id' in details:
                 LOG.info(_("Port %(device)s updated. Details: %(details)s"),
                          {'device': device, 'details': details})
@@ -1093,7 +1107,7 @@ class OFANeutronAgent(sg_rpc.SecurityGroupAgentRpcCallbackMixin):
                 LOG.info(_("Configuration for device %s completed."), device)
             else:
                 LOG.warn(_("Device %s not defined on plugin"), device)
-                if (port and int(port.ofport) != -1):
+                if (port and port.ofport != -1):
                     self.port_dead(port)
         return resync
 

neutron/tests/unit/ofagent/test_ofa_neutron_agent.py

@@ -366,8 +366,11 @@ class TestOFANeutronAgent(OFAAgentTestCase):
         self.assertEqual(expected, actual)
 
     def test_treat_devices_added_returns_true_for_missing_device(self):
-        with mock.patch.object(self.agent.plugin_rpc, 'get_device_details',
+        with contextlib.nested(
-                               side_effect=Exception()):
+            mock.patch.object(self.agent.plugin_rpc, 'get_device_details',
+                              side_effect=Exception()),
+            mock.patch.object(self.agent.int_br, 'get_vif_port_by_id',
+                              return_value=mock.Mock())):
             self.assertTrue(self.agent.treat_devices_added_or_updated([{}]))
 
     def _mock_treat_devices_added_updated(self, details, port, func_name):
@@ -402,6 +405,14 @@ class TestOFANeutronAgent(OFAAgentTestCase):
         self.assertTrue(self._mock_treat_devices_added_updated(
             mock.MagicMock(), port, 'port_dead'))
 
+    def test_treat_devices_added_does_not_process_missing_port(self):
+        with contextlib.nested(
+            mock.patch.object(self.agent.plugin_rpc, 'get_device_details'),
+            mock.patch.object(self.agent.int_br, 'get_vif_port_by_id',
+                              return_value=None)
+        ) as (get_dev_fn, get_vif_func):
+            self.assertFalse(get_dev_fn.called)
+
     def test_treat_devices_added_updated_updates_known_port(self):
         details = mock.MagicMock()
         details.__contains__.side_effect = lambda x: True