Fix OVSBridge.delete_flows when called with no args

With this change calling delete_flows with no kwargs will (instead
of resulting in calling "ovs-ofctl <action> <bridge> -", which does
nothing with no flow spec given on stdin) result in calling
"ovs-ofctl <action> <bridge>", which will delete all flows.

This aligns the behavior of delete_flows with the behavior currently seen by
all callers for the same method shadowed by the implementations in
OpenFlowSwitchMixin classes.

Change-Id: Ic0449acb3a0d4915ce025300d6f3c507a3cd8e48
Closes-Bug: 1658019

neutron/agent/common/ovs_lib.py

@@ -311,12 +311,18 @@ class OVSBridge(BaseOVS):
                                self.br_name, 'datapath_id')
 
     def do_action_flows(self, action, kwargs_list):
-        if action != 'del':
-            for kw in kwargs_list:
-                if 'cookie' not in kw:
-                    kw['cookie'] = self._default_cookie
-        flow_strs = [_build_flow_expr_str(kw, action) for kw in kwargs_list]
-        self.run_ofctl('%s-flows' % action, ['-'], '\n'.join(flow_strs))
+        if action == 'del' and {} in kwargs_list:
+            # the 'del' case simplifies itself if kwargs_list has at least
+            # one item that matches everything
+            self.run_ofctl('%s-flows' % action, [])
+        else:
+            if action != 'del':
+                for kw in kwargs_list:
+                    if 'cookie' not in kw:
+                        kw['cookie'] = self._default_cookie
+            flow_strs = [_build_flow_expr_str(kw, action)
+                         for kw in kwargs_list]
+            self.run_ofctl('%s-flows' % action, ['-'], '\n'.join(flow_strs))
 
     def add_flow(self, **kwargs):
         self.do_action_flows('add', [kwargs])

neutron/plugins/ml2/drivers/openvswitch/agent/openflow/ovs_ofctl/ofswitch.py

@@ -78,11 +78,8 @@ class OpenFlowSwitchMixin(object):
     def delete_flows(self, **kwargs):
         # NOTE(yamamoto): super() points to ovs_lib.OVSBridge.
         # See ovs_bridge.py how this class is actually used.
-        if kwargs:
-            super(OpenFlowSwitchMixin, self).delete_flows(
-                **self._conv_args(kwargs))
-        else:
-            super(OpenFlowSwitchMixin, self).remove_all_flows()
+        super(OpenFlowSwitchMixin, self).delete_flows(
+              **self._conv_args(kwargs))
 
     def _filter_flows(self, flows):
         cookie_list = self.reserved_cookies

neutron/tests/functional/agent/test_ovs_lib.py

@@ -469,6 +469,11 @@ class OVSBridgeTestCase(OVSBridgeTestBase):
         self.assertRaises((RuntimeError, idlutils.RowNotFound),
                           del_port_mod_iface)
 
+    def test_delete_flows_no_args(self):
+        self.br.add_flow(in_port=1, actions="output:2")
+        self.br.delete_flows()
+        self.assertEqual([], self.br.dump_all_flows())
+
 
 class OVSLibTestCase(base.BaseOVSLinuxTestCase):
 

neutron/tests/unit/agent/common/test_ovs_lib.py

@@ -296,6 +296,7 @@ class OVS_Lib_Test(base.BaseTestCase):
         self.br.delete_flows(in_port=ofport)
         self.br.delete_flows(tun_id=lsw_id)
         self.br.delete_flows(dl_vlan=vid)
+        self.br.delete_flows()
         expected_calls = [
             self._ofctl_mock("del-flows", self.BR_NAME, '-',
                              process_input="in_port=" + ofport),
@@ -303,6 +304,26 @@ class OVS_Lib_Test(base.BaseTestCase):
                              process_input="tun_id=%s" % lsw_id),
             self._ofctl_mock("del-flows", self.BR_NAME, '-',
                              process_input="dl_vlan=%s" % vid),
+            self._ofctl_mock("del-flows", self.BR_NAME,
+                             process_input=None),
+        ]
+        self.execute.assert_has_calls(expected_calls)
+
+    def test_do_action_flows_delete_flows(self):
+        # test what the deffered bridge implementation calls in the case of a
+        # delete_flows() among calls to delete_flows(foo=bar)
+        self.br.do_action_flows('del', [{'in_port': 5}, {}])
+        expected_calls = [
+            self._ofctl_mock("del-flows", self.BR_NAME,
+                             process_input=None),
+        ]
+        self.execute.assert_has_calls(expected_calls)
+
+    def test_do_action_flows_delete_flows_empty(self):
+        self.br.delete_flows()
+        expected_calls = [
+            self._ofctl_mock("del-flows", self.BR_NAME,
+                             process_input=None),
         ]
         self.execute.assert_has_calls(expected_calls)