From 5554d620d14175b9bade595226bdc797ba498cf3 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Maciej=20J=C3=B3zefczyk?= <mjozefcz@redhat.com>
Date: Tue, 14 Jul 2020 11:59:58 +0000
Subject: [PATCH] [OVN] Fix logic issue while deleting port with QoS

While port has QoS policy configured the policy wasn't deleted
because of logic issue.

Change-Id: I3d7e70a4a110c68a89d6c575abf121cd9b97e439
Closes-Bug: #1886962
(cherry picked from commit 0e53673c5d4f49846bb439b2e1ab8e984c20d6f8)
---
 .../drivers/ovn/mech_driver/ovsdb/extensions/qos.py  |  2 +-
 .../ml2/drivers/ovn/mech_driver/ovsdb/ovn_client.py  | 12 ++++--------
 .../ovn/mech_driver/ovsdb/extensions/test_qos.py     |  8 ++++++++
 3 files changed, 13 insertions(+), 9 deletions(-)

diff --git a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/extensions/qos.py b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/extensions/qos.py
index 20de16e55c7..38ee1e9a20a 100644
--- a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/extensions/qos.py
+++ b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/extensions/qos.py
@@ -188,7 +188,7 @@ class OVNClientQosExtension(object):
 
     def update_port(self, txn, port, original_port, reset=False, delete=False,
                     qos_rules=None):
-        if not reset and not original_port:
+        if (not reset and not original_port) and not delete:
             # If there is no information about the previous QoS policy, do not
             # make any change, unless the port is new or the QoS information
             # must be reset (delete any previous configuration and set new
diff --git a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_client.py b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_client.py
index 7db5858f009..4278ecda4bf 100644
--- a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_client.py
+++ b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_client.py
@@ -564,17 +564,13 @@ class OVNClient(object):
         if ovn_port is None:
             return
 
-        network_id = ovn_port.external_ids.get(
+        ovn_network_name = ovn_port.external_ids.get(
             ovn_const.OVN_NETWORK_NAME_EXT_ID_KEY)
-
-        # TODO(lucasagomes): For backward compatibility, if network_id
-        # is not in the OVNDB, look at the port_object
-        if not network_id and port_object:
-            network_id = port_object['network_id']
+        network_id = ovn_network_name.strip('neutron-')
 
         with self._nb_idl.transaction(check_error=True) as txn:
             txn.add(self._nb_idl.delete_lswitch_port(
-                port_id, network_id))
+                port_id, ovn_network_name))
 
             p_object = ({'id': port_id, 'network_id': network_id}
                         if not port_object else port_object)
@@ -586,7 +582,7 @@ class OVNClient(object):
             # Check if the port being deleted is a virtual parent
             if (ovn_port.type != ovn_const.LSP_TYPE_VIRTUAL and
                     self._is_virtual_port_supported()):
-                ls = self._nb_idl.ls_get(network_id).execute(
+                ls = self._nb_idl.ls_get(ovn_network_name).execute(
                     check_error=True)
                 cmd = self._nb_idl.unset_lswitch_port_to_virtual_type
                 for lsp in ls.ports:
diff --git a/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/extensions/test_qos.py b/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/extensions/test_qos.py
index a33932eebac..28886d46e28 100644
--- a/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/extensions/test_qos.py
+++ b/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/extensions/test_qos.py
@@ -253,6 +253,14 @@ class TestOVNClientQosExtension(test_plugin.Ml2PluginV2TestCase):
         self.mock_rules.assert_called_once_with(
             mock.ANY, port.id, port.network_id, self.qos_policies[0].id, None)
 
+    def test_delete_port(self):
+        self.mock_rules.reset_mock()
+        self.qos_driver.delete_port(mock.ANY, self.ports[1])
+
+        # Assert that rules are deleted
+        self.mock_rules.assert_called_once_with(
+            mock.ANY, self.ports[1].id, self.ports[1].network_id, None, None)
+
     def test_update_network(self):
         """Test update network.