fix(reconciliation): implement UUID-based domain reconciliation and connection safety

computes/test_reconciliation.py

@@ -0,0 +1,135 @@
+from unittest.mock import MagicMock, patch
+from django.contrib.auth import get_user_model
+from django.test import TestCase
+
+from accounts.models import UserInstance
+from computes.models import Compute
+from computes.utils import refresh_instance_database
+from instances.models import Instance
+from instances.utils import refr
+
+
+class MockDomain:
+    def __init__(self, name, uuid_str):
+        self._name = name
+        self._uuid_str = uuid_str
+
+    def name(self):
+        return self._name
+
+    def UUIDString(self):
+        return self._uuid_str
+
+
+class InstanceReconciliationTestCase(TestCase):
+    def setUp(self):
+        User = get_user_model()
+        self.user = User.objects.create_user(username="test_reconcile_user", password="password")
+        self.compute = Compute.objects.create(
+            name="reconcile-compute",
+            hostname="127.0.0.1",
+            login="root",
+            password="",
+            type=1,
+        )
+
+    def test_new_domain_created_in_libvirt_is_added_to_db(self):
+        dom1 = MockDomain(name="vm-alpha", uuid_str="11111111-1111-1111-1111-111111111111")
+        dom2 = MockDomain(name="vm-beta", uuid_str="22222222-2222-2222-2222-222222222222")
+
+        mock_proxy = MagicMock()
+        mock_proxy.wvm.listAllDomains.return_value = [dom1, dom2]
+        self.compute.__dict__["proxy"] = mock_proxy
+
+        refresh_instance_database(self.compute)
+
+        instances = Instance.objects.filter(compute=self.compute).order_by("name")
+        self.assertEqual(instances.count(), 2)
+        self.assertEqual(instances[0].name, "vm-alpha")
+        self.assertEqual(instances[0].uuid, "11111111-1111-1111-1111-111111111111")
+        self.assertEqual(instances[1].name, "vm-beta")
+        self.assertEqual(instances[1].uuid, "22222222-2222-2222-2222-222222222222")
+
+    def test_domain_deleted_in_libvirt_is_deleted_from_db(self):
+        inst = Instance.objects.create(
+            compute=self.compute,
+            name="vm-to-delete",
+            uuid="33333333-3333-3333-3333-333333333333",
+        )
+        UserInstance.objects.create(instance=inst, user=self.user)
+
+        mock_proxy = MagicMock()
+        mock_proxy.wvm.listAllDomains.return_value = []
+        self.compute.__dict__["proxy"] = mock_proxy
+
+        refresh_instance_database(self.compute)
+
+        self.assertFalse(Instance.objects.filter(compute=self.compute, uuid="33333333-3333-3333-3333-333333333333").exists())
+        self.assertFalse(UserInstance.objects.filter(user=self.user).exists())
+
+    def test_domain_renamed_in_libvirt_preserves_userinstance_and_id(self):
+        uuid = "44444444-4444-4444-4444-444444444444"
+        inst = Instance.objects.create(
+            compute=self.compute,
+            name="vm-original-name",
+            uuid=uuid,
+        )
+        original_inst_id = inst.id
+        user_inst = UserInstance.objects.create(instance=inst, user=self.user, is_change=True)
+
+        dom_renamed = MockDomain(name="vm-new-name", uuid_str=uuid)
+
+        mock_proxy = MagicMock()
+        mock_proxy.wvm.listAllDomains.return_value = [dom_renamed]
+        self.compute.__dict__["proxy"] = mock_proxy
+
+        refresh_instance_database(self.compute)
+
+        inst.refresh_from_db()
+        self.assertEqual(inst.id, original_inst_id)
+        self.assertEqual(inst.name, "vm-new-name")
+        self.assertEqual(inst.uuid, uuid)
+
+        # Verify UserInstance still exists and still points to the same instance!
+        user_inst.refresh_from_db()
+        self.assertEqual(user_inst.instance_id, original_inst_id)
+        self.assertTrue(user_inst.is_change)
+
+    def test_libvirt_connection_error_leaves_db_untouched(self):
+        inst = Instance.objects.create(
+            compute=self.compute,
+            name="vm-persistent",
+            uuid="55555555-5555-5555-5555-555555555555",
+        )
+
+        mock_proxy = MagicMock()
+        mock_proxy.wvm.listAllDomains.side_effect = Exception("Libvirt socket connection refused")
+        self.compute.__dict__["proxy"] = mock_proxy
+
+        refresh_instance_database(self.compute)
+
+        # Instance should NOT be deleted!
+        self.assertTrue(Instance.objects.filter(id=inst.id).exists())
+
+    def test_broken_domain_metadata_aborts_sync_safely(self):
+        inst = Instance.objects.create(
+            compute=self.compute,
+            name="vm-persistent-2",
+            uuid="66666666-6666-6666-6666-666666666666",
+        )
+
+        broken_dom = MagicMock()
+        broken_dom.UUIDString.side_effect = Exception("Corrupted domain metadata")
+
+        mock_proxy = MagicMock()
+        mock_proxy.wvm.listAllDomains.return_value = [broken_dom]
+        self.compute.__dict__["proxy"] = mock_proxy
+
+        refresh_instance_database(self.compute)
+
+        self.assertTrue(Instance.objects.filter(id=inst.id).exists())
+
+    def test_refr_invokes_refresh_instance_database(self):
+        with patch("computes.utils.refresh_instance_database") as mock_refresh:
+            refr(self.compute)
+            mock_refresh.assert_called_once_with(self.compute)

computes/utils.py

@@ -1,17 +1,64 @@
+import logging
+from django.db import transaction
 from instances.models import Instance
 
+logger = logging.getLogger(__name__)
+
 
 def refresh_instance_database(compute):
-    domains = compute.proxy.wvm.listAllDomains()
-    domain_names = [d.name() for d in domains]
-    domain_uuids = [d.UUIDString() for d in domains]
-    # Delete instances that're not on host from DB
-    Instance.objects.filter(compute=compute).exclude(name__in=domain_names).delete()
-    Instance.objects.filter(compute=compute).exclude(uuid__in=domain_uuids).delete()
-    # Create instances that're on host but not in DB
-    names = Instance.objects.filter(compute=compute).values_list("name", flat=True)
-    for domain in domains:
-        if domain.name() not in names:
-            Instance(
-                compute=compute, name=domain.name(), uuid=domain.UUIDString()
-            ).save()
+    """
+    Synchronizes the WebVirtCloud database with libvirt domain state.
+    Libvirt is the single source of truth:
+      - VMs removed in virt-manager/virsh are deleted from the database.
+      - VMs created in virt-manager/virsh are added to the database.
+      - VMs renamed in virt-manager/virsh have their name updated without losing
+        UserInstance permission relationships (matched by UUID).
+      - If libvirt is unreachable (connection/network error), the database is left
+        untouched to avoid false deletions.
+    """
+    try:
+        if hasattr(compute, "status") and compute.status is False:
+            return
+        domains = compute.proxy.wvm.listAllDomains()
+    except Exception as e:
+        logger.warning(
+            "Failed to retrieve domains from compute %s (%s): %s",
+            getattr(compute, "name", compute),
+            getattr(compute, "hostname", ""),
+            e,
+        )
+        return
+
+    # Map libvirt UUIDs to current domain names
+    host_domains = {}
+    for d in domains:
+        try:
+            host_domains[d.UUIDString()] = d.name()
+        except Exception as e:
+            logger.warning(
+                "Error reading domain metadata from compute %s: %s",
+                getattr(compute, "name", compute),
+                e,
+            )
+            # If we fail to read a domain's metadata, abort sync to prevent accidental deletion
+            return
+
+    db_instances = Instance.objects.filter(compute=compute)
+    db_uuids = set(db_instances.values_list("uuid", flat=True))
+
+    with transaction.atomic():
+        # 1. Update name for any VM whose name changed out-of-band in virsh/virt-manager
+        for inst in db_instances:
+            if inst.uuid in host_domains:
+                new_name = host_domains[inst.uuid]
+                if inst.name != new_name:
+                    inst.name = new_name
+                    inst.save(update_fields=["name"])
+
+        # 2. Delete instances that were genuinely undefined/deleted in virsh/virt-manager
+        db_instances.exclude(uuid__in=host_domains.keys()).delete()
+
+        # 3. Create new instances that were created out-of-band in virsh/virt-manager
+        for uuid, name in host_domains.items():
+            if uuid not in db_uuids:
+                Instance.objects.create(compute=compute, name=name, uuid=uuid)

instances/utils.py

@@ -166,18 +166,9 @@ def migrate_instance(
 
 
 def refr(compute):
-    if compute.status is True:
-        domains = compute.proxy.wvm.listAllDomains()
-        domain_names = [d.name() for d in domains]
-        domain_uuids = [d.UUIDString() for d in domains]
-        # Delete instances that're not on host
-        Instance.objects.filter(compute=compute).exclude(name__in=domain_names).delete()
-        Instance.objects.filter(compute=compute).exclude(uuid__in=domain_uuids).delete()
-        # Create instances that're not in DB
-        names = Instance.objects.filter(compute=compute).values_list("name", flat=True)
-        for domain in domains:
-            if domain.name() not in names:
-                Instance(compute=compute, name=domain.name(), uuid=domain.UUIDString()).save()
+    from computes.utils import refresh_instance_database
+
+    refresh_instance_database(compute)
 
 
 def get_dhcp_mac_address(vname):