From b7a753ce80d34f7d98c4b401e364b5f6a740c3f4 Mon Sep 17 00:00:00 2001 From: fumihiko kakuma Date: Mon, 14 Apr 2014 12:54:39 +0900 Subject: [PATCH] OFAgent: Avoid processing ports which are not yet ready Port the following patch to OFAgent. commit: 2702baed390d094b0eac07d0ae167ed236868d00 https://review.openstack.org/#/c/65838/ Closes-Bug: 1307745 Change-Id: Ie1fe0944fb541ba45ffb5b03747d5da289cede51 --- .../ofagent/agent/ofa_neutron_agent.py | 20 ++++++++++++++++--- .../unit/ofagent/test_ofa_neutron_agent.py | 15 ++++++++++++-- 2 files changed, 30 insertions(+), 5 deletions(-) diff --git a/neutron/plugins/ofagent/agent/ofa_neutron_agent.py b/neutron/plugins/ofagent/agent/ofa_neutron_agent.py index 8eeb7577b..58e3df3c0 100644 --- a/neutron/plugins/ofagent/agent/ofa_neutron_agent.py +++ b/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 diff --git a/neutron/tests/unit/ofagent/test_ofa_neutron_agent.py b/neutron/tests/unit/ofagent/test_ofa_neutron_agent.py index 4b2f5556a..2fb4ea031 100644 --- a/neutron/tests/unit/ofagent/test_ofa_neutron_agent.py +++ b/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', - side_effect=Exception()): + with contextlib.nested( + 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 -- 2.45.2