From b1349ad063b38699c8db87a00350ed11923e793f Mon Sep 17 00:00:00 2001
From: Tzu-Mainn Chen <tzumainn@redhat.com>
Date: Tue, 7 Jan 2025 13:16:41 -0500
Subject: [PATCH] Update lease removal function to be more comprehensive

This change updates the lease removal function to undeploy the
node if it is in any deployed state, and to remove any vif attachments
on the node otherwise.
---
 esi_leap/resource_objects/ironic_node.py      | 15 ++++++-
 .../resource_objects/test_ironic_node.py      | 43 +++++++++++++++++++
 2 files changed, 56 insertions(+), 2 deletions(-)

diff --git a/esi_leap/resource_objects/ironic_node.py b/esi_leap/resource_objects/ironic_node.py
index 0f9b110..a5b995b 100644
--- a/esi_leap/resource_objects/ironic_node.py
+++ b/esi_leap/resource_objects/ironic_node.py
@@ -170,10 +170,21 @@ def remove_lease(self, lease):
         get_ironic_client().node.set_console_mode(self._uuid, False)
         console_auth_token.ConsoleAuthToken.clean_console_tokens_for_node(self._uuid)
 
-        # unprovision the node if needed
+        # unprovision the node if needed; otherwise remove any vif attachments
         state = self._get_node().provision_state
-        if state == "active":
+        if state in [
+            "active",
+            "wait call-back",
+            "deploying",
+            "deploy failed",
+            "adopting",
+            "adopt failed",
+        ]:
             get_ironic_client().node.set_provision_state(self._uuid, "deleted")
+        else:
+            vifs = get_ironic_client().node.vif_list(self._uuid)
+            for vif in vifs:
+                get_ironic_client().node.vif_detach(self._uuid, vif.id)
 
     def _get_node(self, resource_list=None):
         try:
diff --git a/esi_leap/tests/resource_objects/test_ironic_node.py b/esi_leap/tests/resource_objects/test_ironic_node.py
index 9bf2b68..9e04051 100644
--- a/esi_leap/tests/resource_objects/test_ironic_node.py
+++ b/esi_leap/tests/resource_objects/test_ironic_node.py
@@ -208,6 +208,49 @@ def test_remove_lease(self, mock_client, mock_gn, mock_glu, mock_glpi, mock_cctf
         mock_client.return_value.node.set_provision_state.assert_called_once_with(
             fake_uuid, "deleted"
         )
+        mock_client.return_value.node.vif_list.assert_not_called()
+
+    @mock.patch(
+        "esi_leap.objects.console_auth_token.ConsoleAuthToken.clean_console_tokens_for_node"
+    )
+    @mock.patch(
+        "esi_leap.resource_objects.ironic_node.IronicNode." "get_lessee_project_id"
+    )
+    @mock.patch("esi_leap.resource_objects.ironic_node.IronicNode." "get_lease_uuid")
+    @mock.patch("esi_leap.resource_objects.ironic_node.IronicNode._get_node")
+    @mock.patch.object(ironic_node, "get_ironic_client", autospec=True)
+    def test_remove_lease_node_available(
+        self, mock_client, mock_gn, mock_glu, mock_glpi, mock_cctfn
+    ):
+        fake_get_node = FakeIronicNode()
+        fake_get_node.provision_state = "available"
+        fake_lease = FakeLease()
+
+        mock_gn.return_value = fake_get_node
+        mock_glu.return_value = fake_lease.uuid
+        mock_glpi.return_value = fake_lease.project_id
+
+        test_ironic_node = ironic_node.IronicNode(fake_uuid)
+        test_ironic_node.remove_lease(fake_lease)
+
+        mock_glpi.assert_called_once()
+        mock_glu.assert_called_once()
+        self.assertEqual(mock_gn.call_count, 2)
+        self.assertEqual(mock_client.call_count, 3)
+        mock_client.return_value.node.update.assert_called_once_with(
+            fake_uuid,
+            [
+                {"op": "remove", "path": "/properties/lease_uuid"},
+                {"op": "remove", "path": "/lessee"},
+                {"op": "remove", "path": "/instance_info"},
+            ],
+        )
+        mock_client.return_value.node.set_console_mode.assert_called_once_with(
+            fake_uuid, False
+        )
+        mock_cctfn.assert_called_once_with(fake_uuid)
+        mock_client.return_value.node.set_provision_state.assert_not_called()
+        mock_client.return_value.node.vif_list.assert_called_once_with(fake_uuid)
 
     @mock.patch("esi_leap.resource_objects.ironic_node.IronicNode." "get_lease_uuid")
     def test_expire_lease_no_match(self, mock_glu):