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)

diff --git a/neutron/plugins/openvswitch/agent/ovs_neutron_agent.py b/neutron/plugins/openvswitch/agent/ovs_neutron_agent.py
index bdd44df98d39e526f5c690a361c8b3afaa9c8765..13903ec02a68e9047c2e01114fe749d81f725d2b 100644 (file)
--- 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 1b8ee66465e2c9b3683e003d7b800c4d31f79e2e..e1ff55f9848be6fabf6904b9d34d6577c042e909 100644 (file)
--- 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"),