From a8d20e05079903a3fb2e952b7ec2c4cf27b208e9 Mon Sep 17 00:00:00 2001 From: armando-migliaccio Date: Wed, 14 Oct 2015 14:27:09 -0700 Subject: [PATCH] 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 --- .../openvswitch/agent/ovs_neutron_agent.py | 9 ++++++ .../agent/test_ovs_neutron_agent.py | 29 ++++++++++++------- 2 files changed, 27 insertions(+), 11 deletions(-) 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 9f744b684..fa849237b 100644 --- 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 c3539c1c1..bace76eee 100644 --- 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() -- 2.45.2