fix: unmap Nautobot devices from tenant during keystone project deletion

Author: syedhaseebahmed

python/understack-workflows/tests/test_sync_keystone.py

@@ -1,12 +1,37 @@
 import uuid
 from contextlib import nullcontext
+from unittest.mock import MagicMock
 
 import pytest
+from openstack.connection import Connection
 from pytest_lazy_fixtures import lf
 
 from understack_workflows.main.sync_keystone import Event
 from understack_workflows.main.sync_keystone import argument_parser
 from understack_workflows.main.sync_keystone import do_action
+from understack_workflows.main.sync_keystone import handle_project_delete
+
+
+@pytest.fixture
+def mock_pynautobot_api(mocker):
+    mock_client = MagicMock(name="MockPynautobotApi")
+
+    mock_devices = MagicMock()
+    mock_devices.filter.return_value = []
+    mock_devices.update.return_value = True
+    mock_client.dcim.devices = mock_devices
+
+    mock_tenants = MagicMock()
+    mock_tenants.get.return_value = None
+    mock_tenants.delete.return_value = True
+    mock_client.tenancy.tenants = mock_tenants
+
+    mocker.patch(
+        "understack_workflows.main.sync_keystone.pynautobot.api",
+        return_value=mock_client,
+    )
+
+    return mock_client
 
 
 @pytest.mark.parametrize(
@@ -44,32 +69,73 @@ def test_parse_object_id(arg_list, context, expected_id):
 
 def test_create_project(
     os_conn,
-    nautobot,
+    mock_pynautobot_api,
     project_id: uuid.UUID,
     domain_id: uuid.UUID,
 ):
-    ret = do_action(os_conn, nautobot, Event.ProjectCreate, project_id)
+    ret = do_action(os_conn, mock_pynautobot_api, Event.ProjectCreate, project_id)
     os_conn.identity.get_project.assert_any_call(domain_id.hex)
     os_conn.identity.get_project.assert_any_call(project_id.hex)
     assert ret == 0
 
 
 def test_update_project(
     os_conn,
-    nautobot,
+    mock_pynautobot_api,
     project_id: uuid.UUID,
     domain_id: uuid.UUID,
 ):
-    ret = do_action(os_conn, nautobot, Event.ProjectUpdate, project_id)
+    ret = do_action(os_conn, mock_pynautobot_api, Event.ProjectUpdate, project_id)
     os_conn.identity.get_project.assert_any_call(domain_id.hex)
     os_conn.identity.get_project.assert_any_call(project_id.hex)
     assert ret == 0
 
 
 def test_delete_project(
     os_conn,
-    nautobot,
+    mock_pynautobot_api,
     project_id: uuid.UUID,
 ):
-    ret = do_action(os_conn, nautobot, Event.ProjectDelete, project_id)
+    ret = do_action(os_conn, mock_pynautobot_api, Event.ProjectDelete, project_id)
+    assert ret == 0
+
+
+@pytest.mark.parametrize(
+    "tenant_exists, expect_delete_call, expect_unmap_call",
+    [
+        (False, False, False),  # Tenant does NOT exist
+        (True, True, True),  # Tenant exists
+    ],
+)
+def test_handle_project_delete(
+    mocker, mock_pynautobot_api, tenant_exists, expect_delete_call, expect_unmap_call
+):
+    project_id = uuid.uuid4()
+
+    tenant_obj = MagicMock()
+    mock_pynautobot_api.tenancy.tenants.get.return_value = (
+        tenant_obj if tenant_exists else None
+    )
+
+    mock_delete_network = mocker.patch(
+        "understack_workflows.main.sync_keystone._delete_outside_network"
+    )
+    mock_unmap_devices = mocker.patch(
+        "understack_workflows.main.sync_keystone._unmap_tenant_from_devices"
+    )
+    conn_mock: Connection = MagicMock(spec=Connection)
+    ret = handle_project_delete(conn_mock, mock_pynautobot_api, project_id)
+
     assert ret == 0
+    mock_pynautobot_api.tenancy.tenants.get.assert_called_once_with(id=project_id)
+
+    if tenant_exists:
+        mock_delete_network.assert_called_once_with(conn_mock, project_id)
+        mock_unmap_devices.assert_called_once_with(
+            tenant_id=project_id, nautobot=mock_pynautobot_api
+        )
+        tenant_obj.delete.assert_called_once()
+    else:
+        mock_delete_network.assert_not_called()
+        mock_unmap_devices.assert_not_called()
+        tenant_obj.delete.assert_not_called()

python/understack-workflows/understack_workflows/main/sync_keystone.py

@@ -1,12 +1,16 @@
 import argparse
 import logging
 import uuid
+from collections.abc import Sequence
 from enum import StrEnum
+from typing import cast
+
+import pynautobot
+from pynautobot.core.response import Record
 
 from understack_workflows.helpers import credential
 from understack_workflows.helpers import parser_nautobot_args
 from understack_workflows.helpers import setup_logger
-from understack_workflows.nautobot import Nautobot
 from understack_workflows.openstack.client import Connection
 from understack_workflows.openstack.client import get_openstack_client
 
@@ -109,15 +113,24 @@ def _tenant_attrs(conn: Connection, project_id: uuid.UUID) -> tuple[str, str, bo
     return tenant_name, str(project.description), is_default_domain
 
 
+def _unmap_tenant_from_devices(
+    tenant_id: uuid.UUID,
+    nautobot: pynautobot.api,
+):
+    devices: Sequence[Record] = list(nautobot.dcim.devices.filter(tenant=tenant_id))
+    for d in devices:
+        d.tenant = None  # type: ignore[attr-defined]
+        nautobot.dcim.devices.update(devices)
+
+
 def handle_project_create(
-    conn: Connection, nautobot: Nautobot, project_id: uuid.UUID
+    conn: Connection, nautobot: pynautobot.api, project_id: uuid.UUID
 ) -> int:
     logger.info("got request to create tenant %s", project_id.hex)
     tenant_name, tenant_description, is_default_domain = _tenant_attrs(conn, project_id)
 
-    nautobot_tenant_api = nautobot.session.tenancy.tenants
     try:
-        tenant = nautobot_tenant_api.create(
+        tenant = nautobot.tenancy.tenants.create(
             id=str(project_id), name=tenant_name, description=tenant_description
         )
         if is_default_domain:
@@ -133,24 +146,24 @@ def handle_project_create(
 
 
 def handle_project_update(
-    conn: Connection, nautobot: Nautobot, project_id: uuid.UUID
+    conn: Connection, nautobot: pynautobot.api, project_id: uuid.UUID
 ) -> int:
     logger.info("got request to update tenant %s", project_id.hex)
     tenant_name, tenant_description, is_default_domain = _tenant_attrs(conn, project_id)
 
-    tenant_api = nautobot.session.tenancy.tenants
-    existing_tenant = tenant_api.get(project_id)
+    existing_tenant = nautobot.tenancy.tenants.get(id=project_id)
     logger.info("existing_tenant: %s", existing_tenant)
     try:
         if existing_tenant is None:
-            new_tenant = tenant_api.create(
+            new_tenant = nautobot.tenancy.tenants.create(
                 id=str(project_id), name=tenant_name, description=tenant_description
             )
             logger.info("tenant %s created %s", project_id, new_tenant.created)  # type: ignore
         else:
-            existing_tenant.name = tenant_name  # type: ignore
-            existing_tenant.description = tenant_description  # type: ignore
-            existing_tenant.save()  # type: ignore
+            existing_tenant = cast(Record, existing_tenant)
+            existing_tenant.update(
+                {"name": tenant_name, "description": tenant_description}
+            )  # type: ignore
             logger.info(
                 "tenant %s last updated %s",
                 project_id,
@@ -168,23 +181,26 @@ def handle_project_update(
 
 
 def handle_project_delete(
-    conn: Connection, nautobot: Nautobot, project_id: uuid.UUID
+    conn: Connection, nautobot: pynautobot.api, project_id: uuid.UUID
 ) -> int:
     logger.info("got request to delete tenant %s", project_id)
-    ten = nautobot.session.tenancy.tenants.get(project_id)
-    if not ten:
+    tenant = nautobot.tenancy.tenants.get(id=project_id)
+    if not tenant:
         logger.warning("tenant %s does not exist, nothing to delete", project_id)
         return _EXIT_SUCCESS
 
     _delete_outside_network(conn, project_id)
-    ten.delete()  # type: ignore
+    _unmap_tenant_from_devices(tenant_id=project_id, nautobot=nautobot)
+
+    tenant = cast(Record, tenant)
+    tenant.delete()
     logger.info("deleted tenant %s", project_id)
     return _EXIT_SUCCESS
 
 
 def do_action(
     conn: Connection,
-    nautobot: Nautobot,
+    nautobot: pynautobot.api,
     event: Event,
     project_id: uuid.UUID,
 ) -> int:
@@ -206,6 +222,5 @@ def main() -> int:
 
     conn = get_openstack_client(cloud=args.os_cloud)
     nb_token = args.nautobot_token or credential("nb-token", "token")
-    nautobot = Nautobot(args.nautobot_url, nb_token, logger=logger)
-
+    nautobot = pynautobot.api(args.nautobot_url, token=nb_token)
     return do_action(conn, nautobot, args.event, args.object)