[OVN] `DelLSwitchPortCommand` also deletes external ports HCG

The ``DelLSwitchPortCommand`` now removes the external ports
``HA_Chassis_Group`` associated registers. It is not needed to handle
it outside the deletion command.

Related-Bug: #2125553
Signed-off-by: Rodolfo Alonso Hernandez <ralonsoh@redhat.com>
Change-Id: I1e7b8a1df23c6dab2a8de604f18063fbcf6aeef7

neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/commands.py

@@ -386,6 +386,14 @@ class DelLSwitchPortCommand(command.BaseCommand):
         for uuid_ in cur_port_dhcp_opts:
             self.api._tables['DHCP_Options'].rows[uuid_].delete()
 
+        # Delete the HA_Chassis_Group associated to an external port.
+        if (lport.type == ovn_const.LSP_TYPE_EXTERNAL and
+                lport.ha_chassis_group):
+            hcg = lport.ha_chassis_group[0]
+            lport.delvalue('ha_chassis_group', hcg)
+            if hcg.name == utils.ovn_extport_chassis_group_name(lport.name):
+                hcg.delete()
+
         _delvalue_from_list(lswitch, 'ports', lport)
         self.api._tables['Logical_Switch_Port'].rows[lport.uuid].delete()
 

neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_client.py

@@ -848,15 +848,6 @@ class OVNClient:
                             ovn_const.LSP_OPTIONS_VIRTUAL_PARENTS_KEY, ''):
                         txn.add(cmd(lsp.name, port_id, if_exists=True))
 
-        # NOTE(lucasagomes): We need to delete the LSP before we attempt
-        # to remove the HA Chassis Group or it will fail with a violation
-        # error due to the LSP reference in the group
-        with self._nb_idl.transaction(check_error=True) as txn:
-            if ovn_port.type == ovn_const.LSP_TYPE_EXTERNAL:
-                ha_ch_grp_name = utils.ovn_extport_chassis_group_name(port_id)
-                txn.add(self._nb_idl.ha_chassis_group_del(
-                    ha_ch_grp_name, if_exists=True))
-
     # TODO(lucasagomes): The ``port_object`` parameter was added to
     # keep things backward compatible. Remove it in the Rocky release.
     def delete_port(self, context, port_id, port_object=None):

neutron/tests/functional/common/ovn/test_utils.py

@@ -156,9 +156,13 @@ class TestSyncHaChassisGroup(base.TestOVNFunctionalBase):
 
         # Invoke the sync method
         with self.nb_api.transaction(check_error=True) as txn:
-            utils.sync_ha_chassis_group_network(
+            hcg, _ = utils.sync_ha_chassis_group_network(
                 self.context, self.nb_api, self.sb_api, port['id'],
                 net['id'], txn)
+            # It is needed to assign the HCG to the LSP. When the port is
+            # deleted, the external port HCG associated will be deleted too.
+            txn.add(
+                self.nb_api.set_lswitch_port(port['id'], ha_chassis_group=hcg))
 
         # Assert only the eligible chassis are present in HA Chassis
         ha_chassis = self.nb_api.db_find('HA_Chassis').execute(

neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/test_mech_driver.py

@@ -20,6 +20,7 @@ import time
 from unittest import mock
 import uuid
 
+import ddt
 import netaddr
 from neutron_lib.api.definitions import external_net
 from neutron_lib.api.definitions import portbindings
@@ -704,6 +705,7 @@ class TestVirtualPorts(base.TestOVNFunctionalBase):
                          ovn_vport.options)
 
 
+@ddt.ddt
 class TestExternalPorts(base.TestOVNFunctionalBase):
 
     def setUp(self):
@@ -720,7 +722,12 @@ class TestExternalPorts(base.TestOVNFunctionalBase):
         rows = cmd.execute(check_error=True)
         return rows[0] if rows else None
 
-    def _test_external_port_create(self, vnic_type):
+    def _test_external_port_create_and_delete(
+            self, vnic_type, enable_as_gw):
+        for host in ('host1', 'host2', 'host3'):
+            self.add_fake_chassis(
+                host, enable_chassis_as_gw=enable_as_gw,
+                enable_chassis_as_extport=not enable_as_gw)
         net_id = self.n1['network']['id']
         port_data = {
             'port': {'network_id': net_id,
@@ -732,10 +739,25 @@ class TestExternalPorts(base.TestOVNFunctionalBase):
         port = self.deserialize(self.fmt, port_res)['port']
 
         ovn_port = self._find_port_row_by_name(port['id'])
+        hcg_name = str(ovn_port.ha_chassis_group[0].name)
         self.assertEqual(ovn_const.LSP_TYPE_EXTERNAL, ovn_port.type)
         self.assertEqual(1, len(ovn_port.ha_chassis_group))
-        self.assertEqual(utils.ovn_name(net_id),
-                         str(ovn_port.ha_chassis_group[0].name))
+        group_name = (utils.ovn_name(net_id) if enable_as_gw else
+                      utils.ovn_extport_chassis_group_name(port['id']))
+        self.assertEqual(group_name, hcg_name)
+        hcg = self.nb_api.lookup('HA_Chassis_Group', hcg_name)
+        self.assertEqual(hcg_name, hcg.name)
+
+        port_req = self.new_delete_request('ports', port['id'])
+        port_req.get_response(self.api)
+        hcg = self.nb_api.lookup('HA_Chassis_Group', hcg_name, None)
+        if enable_as_gw:
+            self.assertEqual(hcg_name, hcg.name)
+        else:
+            # If the HCG has been created only for this port (that happens
+            # when there are chassis for external ports), it should be deleted
+            # along with the port.
+            self.assertIsNone(hcg)
 
     def _create_router_port(self, vnic_type):
         net_id = self.n1['network']['id']
@@ -808,14 +830,21 @@ class TestExternalPorts(base.TestOVNFunctionalBase):
         n_utils.wait_until_true(lambda: test_up_event.get_count() == 1,
                                 timeout=10)
 
-    def test_external_port_create_vnic_direct(self):
-        self._test_external_port_create(portbindings.VNIC_DIRECT)
+    @ddt.data(True, False)
+    def test_external_port_create_and_delete_vnic_direct(self, enable_as_gw):
+        self._test_external_port_create_and_delete(
+            portbindings.VNIC_DIRECT, enable_as_gw)
 
-    def test_external_port_create_vnic_direct_physical(self):
-        self._test_external_port_create(portbindings.VNIC_DIRECT_PHYSICAL)
+    @ddt.data(True, False)
+    def test_external_port_create_and_delete_direct_physical(
+            self, enable_as_gw):
+        self._test_external_port_create_and_delete(
+            portbindings.VNIC_DIRECT_PHYSICAL, enable_as_gw)
 
-    def test_external_port_create_vnic_macvtap(self):
-        self._test_external_port_create(portbindings.VNIC_MACVTAP)
+    @ddt.data(True, False)
+    def test_external_port_create_and_delete_vnic_macvtap(self, enable_as_gw):
+        self._test_external_port_create_and_delete(
+            portbindings.VNIC_MACVTAP, enable_as_gw)
 
     def _test_external_port_update(self, vnic_type):
         net_id = self.n1['network']['id']

neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_commands.py

@@ -402,7 +402,10 @@ class TestDelLSwitchPortCommand(TestBaseCommand):
             attrs={'name': 'lsp',
                    'external_ids': ext_ids,
                    'dhcpv4_options': [fake_dhcpv4_options],
-                   'dhcpv6_options': [fake_dhcpv6_options]})
+                   'dhcpv6_options': [fake_dhcpv6_options],
+                   'type': '',
+                   },
+        )
         self.ovn_api._tables['Logical_Switch_Port'].rows[fake_lsp.uuid] = \
             fake_lsp
         fake_lswitch = fakes.FakeOvsdbRow.create_one_ovsdb_row(