Fix AttributeError on port_bound for missing ports

If the port is concurrently deleted, db_get_val returns None, and
that causes the Exception to be raised. However, the exception is just
log noise if the port has been deleted concurrently and it does not
lead to failures.

This can happen if port_update and port_delete operations occur
in short sequence and interleave. To prevent this trace from
occurring, this patch checks that the port is being eliminated and
emits an error trace only if the port is indeed to be expected
amongst the list of ports to be updated.

We do not raise an exception to avoid disrupting the agent sync
process, and leave to the admin the investigation of the issue
(should that be cronic rather than transient).

Under normal circumstances, if the port is expected it should
be there, and if it isn't this should be treated as a bug to be
investigated further.

Closes-bug: #1455320

Change-Id: Ic609af0ef6a09b536c882c58c23167c0a469b155

diff --git a/neutron/plugins/ml2/drivers/openvswitch/agent/ovs_neutron_agent.py b/neutron/plugins/ml2/drivers/openvswitch/agent/ovs_neutron_agent.py
index 9f744b68400b67147f1fd5eb5caf356e9ba64354..fa849237b5cdb1ef18c55e38e4672a702aaf7de4 100644 (file)
--- a/neutron/plugins/ml2/drivers/openvswitch/agent/ovs_neutron_agent.py
+++ b/neutron/plugins/ml2/drivers/openvswitch/agent/ovs_neutron_agent.py
@@ -791,6 +791,15 @@ class OVSNeutronAgent(sg_rpc.SecurityGroupAgentRpcCallbackMixin,
                                         device_owner)
         port_other_config = self.int_br.db_get_val("Port", port.port_name,
                                                    "other_config")
+        if port_other_config is None:
+            if port.vif_id in self.deleted_ports:
+                LOG.debug("Port %s deleted concurrently", port.vif_id)
+            elif port.vif_id in self.updated_ports:
+                LOG.error(_LE("Expected port %s not found"), port.vif_id)
+            else:
+                LOG.debug("Unable to get config for port %s", port.vif_id)
+            return
+
         vlan_mapping = {'net_uuid': net_uuid,
                         'network_type': network_type,
                         'physical_network': physical_network,

diff --git a/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/test_ovs_neutron_agent.py b/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/test_ovs_neutron_agent.py
index c3539c1c1c8318fba1c0e7419842510b72c160f9..bace76eeee4621257b61e1903e96beb1cafe67c7 100644 (file)
--- a/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/test_ovs_neutron_agent.py
+++ b/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/test_ovs_neutron_agent.py
@@ -152,7 +152,7 @@ class TestOvsNeutronAgent(object):
         self.agent.sg_agent = mock.Mock()
 
     def _mock_port_bound(self, ofport=None, new_local_vlan=None,
-                         old_local_vlan=None):
+                         old_local_vlan=None, db_get_val=None):
         port = mock.Mock()
         port.ofport = ofport
         net_uuid = 'my-net-uuid'
@@ -163,16 +163,19 @@ class TestOvsNeutronAgent(object):
                 self.mod_agent.LocalVLANMapping(
                     old_local_vlan, None, None, None))
         with mock.patch.object(self.agent, 'int_br', autospec=True) as int_br:
-            int_br.db_get_val.return_value = {}
+            int_br.db_get_val.return_value = db_get_val
             int_br.set_db_attribute.return_value = True
             self.agent.port_bound(port, net_uuid, 'local', None, None,
                                   fixed_ips, "compute:None", False)
-        vlan_mapping = {'net_uuid': net_uuid,
-                        'network_type': 'local',
-                        'physical_network': None,
-                        'segmentation_id': None}
-        int_br.set_db_attribute.assert_called_once_with(
-            "Port", mock.ANY, "other_config", vlan_mapping)
+        if db_get_val is None:
+            self.assertEqual(0, int_br.set_db_attribute.call_count)
+        else:
+            vlan_mapping = {'net_uuid': net_uuid,
+                            'network_type': 'local',
+                            'physical_network': None,
+                            'segmentation_id': None}
+            int_br.set_db_attribute.assert_called_once_with(
+                "Port", mock.ANY, "other_config", vlan_mapping)
 
     def _test_restore_local_vlan_maps(self, tag):
         port = mock.Mock()
@@ -284,13 +287,17 @@ class TestOvsNeutronAgent(object):
         self.assertIsNone(self.agent._check_agent_configurations())
 
     def test_port_bound_deletes_flows_for_valid_ofport(self):
-        self._mock_port_bound(ofport=1, new_local_vlan=1)
+        self._mock_port_bound(ofport=1, new_local_vlan=1, db_get_val={})
 
     def test_port_bound_ignores_flows_for_invalid_ofport(self):
-        self._mock_port_bound(ofport=-1, new_local_vlan=1)
+        self._mock_port_bound(ofport=-1, new_local_vlan=1, db_get_val={})
 
     def test_port_bound_does_not_rewire_if_already_bound(self):
-        self._mock_port_bound(ofport=-1, new_local_vlan=1, old_local_vlan=1)
+        self._mock_port_bound(
+            ofport=-1, new_local_vlan=1, old_local_vlan=1, db_get_val={})
+
+    def test_port_bound_not_found(self):
+        self._mock_port_bound(ofport=1, new_local_vlan=1, db_get_val=None)
 
     def _test_port_dead(self, cur_tag=None):
         port = mock.Mock()