From c49c9c528680fa63faf0cbb382b11f42a790358f Mon Sep 17 00:00:00 2001 From: Kevin Benton Date: Tue, 2 Jun 2015 16:52:14 -0700 Subject: [PATCH] Check for 'removed' in port_info before reference scan_ports can return early on no changes, in which case 'removed' won't be present in the dict. The deleted_ports logic wasn't setup to handle that. This patch checks for the key before trying to reference it. Change-Id: I0e2c6d76515ad8e2a2addc8d40451ac003a150f7 Closes-Bug: #1461325 (cherry picked from commit 75f3aaa4cc42c2c1280f6c578e27e64cff8f860c) --- .../plugins/openvswitch/agent/ovs_neutron_agent.py | 11 ++++++----- .../openvswitch/agent/test_ovs_neutron_agent.py | 11 ++++++++++- 2 files changed, 16 insertions(+), 6 deletions(-) diff --git a/neutron/plugins/openvswitch/agent/ovs_neutron_agent.py b/neutron/plugins/openvswitch/agent/ovs_neutron_agent.py index bdd44df98..13903ec02 100644 --- a/neutron/plugins/openvswitch/agent/ovs_neutron_agent.py +++ b/neutron/plugins/openvswitch/agent/ovs_neutron_agent.py @@ -343,7 +343,11 @@ class OVSNeutronAgent(sg_rpc.SecurityGroupAgentRpcCallbackMixin, self.deleted_ports.add(port_id) LOG.debug("port_delete message processed for port %s", port_id) - def process_deleted_ports(self): + def process_deleted_ports(self, port_info): + # don't try to process removed ports as deleted ports since + # they are already gone + if 'removed' in port_info: + self.deleted_ports -= port_info['removed'] while self.deleted_ports: port_id = self.deleted_ports.pop() # Flush firewall rules and move to dead VLAN so deleted ports no @@ -1582,10 +1586,7 @@ class OVSNeutronAgent(sg_rpc.SecurityGroupAgentRpcCallbackMixin, self.updated_ports = set() reg_ports = (set() if ovs_restarted else ports) port_info = self.scan_ports(reg_ports, updated_ports_copy) - # don't try to process removed ports as deleted ports since - # they are already gone - self.deleted_ports -= port_info['removed'] - self.process_deleted_ports() + self.process_deleted_ports(port_info) self.update_stale_ofport_rules() LOG.debug("Agent rpc_loop - iteration:%(iter_num)d - " "port information retrieved. " diff --git a/neutron/tests/unit/plugins/openvswitch/agent/test_ovs_neutron_agent.py b/neutron/tests/unit/plugins/openvswitch/agent/test_ovs_neutron_agent.py index 1b8ee6646..e1ff55f98 100644 --- a/neutron/tests/unit/plugins/openvswitch/agent/test_ovs_neutron_agent.py +++ b/neutron/tests/unit/plugins/openvswitch/agent/test_ovs_neutron_agent.py @@ -531,7 +531,7 @@ class TestOvsNeutronAgent(base.BaseTestCase): int_br.get_vif_port_by_id.return_value = vif self.agent.port_delete("unused_context", port_id='id') - self.agent.process_deleted_ports() + self.agent.process_deleted_ports(port_info={}) # the main things we care about are that it gets put in the # dead vlan and gets blocked int_br.set_db_attribute.assert_any_call( @@ -541,6 +541,15 @@ class TestOvsNeutronAgent(base.BaseTestCase): in_port=vif.ofport, actions='drop') + def test_port_delete_removed_port(self): + with mock.patch.object(self.agent, 'int_br') as int_br: + self.agent.port_delete("unused_context", + port_id='id') + # if it was removed from the bridge, we shouldn't be processing it + self.agent.process_deleted_ports(port_info={'removed': {'id', }}) + self.assertFalse(int_br.set_db_attribute.called) + self.assertFalse(int_br.drop_port.called) + def test_setup_physical_bridges(self): with contextlib.nested( mock.patch.object(ip_lib, "device_exists"), -- 2.45.2