fix(computes): a deleted compute's VMs keep their owners

Deleting a compute removed its VMs from the panel together with their
owners, while the VMs kept running on the host: added again, the host's
VMs came back with no owners. A compute's delete now treats each VM as
one that left its compute: its owners follow the same UUID on another
compute or wait in a tombstone for the retention period. One function,
forget(), does this for the refresh and for the delete; the API's
compute delete goes through the same model delete.

The delete holds the compute exclusive and locks its row before its
VMs', as the reconcile does: a reconcile, VM operation or job on the
compute makes it busy instead of losing owners to a concurrent restore
or deadlocking with it.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

README.md

@@ -578,7 +578,7 @@ A version from before v3.0.0 has no worker: `systemctl disable --now webvirt-wor
 - The admin's first-login password change survives losing `data/admin_password` (migration `accounts.0007`); run `migrate` as a user who can read that file.
 - "VM Clone Auto Migrate" is removed; a clone stays on its source host.
 - A template VM is changed or deleted only by superusers and staff owners with the matching permission; other owners can still view, open the console and clone it. Power cycle no longer starts a template.
-- A VM that vanishes from its compute keeps its owners for `INSTANCE_OWNERSHIP_RETENTION_DAYS` (30) in case the same UUID comes back; superusers list and remove these records on the "Removed VMs" admin page.
+- A VM that vanishes from its compute, or whose compute is deleted, keeps its owners for `INSTANCE_OWNERSHIP_RETENTION_DAYS` (30) in case the same UUID comes back; superusers list and remove these records on the "Removed VMs" admin page.
 - The log rotates (10 MB × 5) in `data/webvirtcloud.log`.
 - A disk can be grown while its VM runs or is paused, not only when it is shut off. A disk on an LVM pool cannot be grown (libvirt cannot resize its volumes): the resize says so instead of failing, and the VM's other disks still grow.
 - Enabling vCPU hot plug keeps the VM's current vCPU count (it enabled all of them up to the maximum).

computes/models.py

@@ -1,5 +1,6 @@
 from ipaddress import ip_address
 
+from django.db import transaction
 from django.db.models import CharField, IntegerField, Model
 from django.utils.functional import cached_property
 from django.utils.translation import gettext_lazy as _
@@ -88,3 +89,17 @@ class Compute(Model):
 
     def __str__(self):
         return self.name
+
+    def delete(self, *args, **kwargs):
+        """Its VMs keep running on the host: their owners wait in tombstones,
+        or follow a VM already listed on another compute. The compute is held
+        exclusive, so no reconcile, VM operation or job on it runs meanwhile
+        (Busy if one does); its row is locked before its VMs', as the
+        reconcile does."""
+        from computes.utils import forget, libvirt_compute_lock
+
+        with libvirt_compute_lock(self), transaction.atomic():
+            Compute.objects.select_for_update().filter(pk=self.pk).first()
+            for inst in self.instance_set.select_for_update():
+                forget(inst)
+            return super().delete(*args, **kwargs)

computes/tests/test_ownership.py

@@ -8,8 +8,12 @@ from computes.models import Compute
 from computes.utils import refresh_instance_database
 from django.contrib.auth import get_user_model
 from django.test import TestCase, override_settings
+from django.urls import reverse
 from django.utils import timezone
 from instances.models import Instance, InstanceTombstone
+from operations import claims
+from operations.models import Claim
+from rest_framework.test import APIClient
 
 UUID = "44444444-4444-4444-4444-444444444444"
 
@@ -111,3 +115,52 @@ class OwnershipTestCase(TestCase):
         refresh_instance_database(self.host_a)
 
         self.assertEqual(self.ownership(self.host_a), (True, []))
+
+    # a deleted compute's VMs keep running on the host: their owners wait in
+    # tombstones as for a VM that disappeared, or follow a VM already on another compute
+    def delete(self, compute):
+        admin = get_user_model().objects.create_superuser("cd-admin", "cd@example.com", "pw")
+        self.client.force_login(admin)
+        self.client.post(reverse("compute_delete", args=[compute.id]))
+        self.assertFalse(Compute.objects.filter(pk=compute.pk).exists())
+
+    def test_a_host_added_again_gets_its_vms_back_with_their_owners(self):
+        self.owned_vm(self.host_a)
+        self.delete(self.host_a)
+        self.assertEqual(InstanceTombstone.objects.get().owners[0]["user"], self.owner.id)
+
+        again = self.compute("host-a-again")
+        self.on(again, [("vm", UUID)])
+        refresh_instance_database(again)
+        self.assertEqual(self.ownership(again), (True, [(self.owner, True, False, True)]))
+
+    def test_a_vm_already_on_another_compute_keeps_its_owners_there(self):
+        self.owned_vm(self.host_a)
+        self.on(self.host_b, [("vm", UUID)])
+        refresh_instance_database(self.host_b)  # listed on B too: a copy for now
+        self.delete(self.host_a)
+        self.assertEqual(self.ownership(self.host_b), (True, [(self.owner, True, False, True)]))
+        self.assertFalse(InstanceTombstone.objects.exists())
+
+    def test_the_api_delete_keeps_the_owners_too(self):
+        self.owned_vm(self.host_a)
+        api = APIClient()
+        api.force_login(get_user_model().objects.create_superuser("cd-api", "cda@example.com", "pw"))
+        self.assertEqual(api.delete(reverse("compute-detail", args=[self.host_a.id])).status_code, 204)
+        self.assertEqual(InstanceTombstone.objects.get().owners[0]["user"], self.owner.id)
+
+    def test_a_compute_in_use_is_not_deleted(self):
+        # a reconcile, VM operation or job holds it: the delete waits for none of them
+        vm = self.owned_vm(self.host_a)
+        claims.admit({claims.compute_key(self.host_a.id): Claim.SHARED}, holder="192.0.2.30:1:1")
+        admin = get_user_model().objects.create_superuser("cd-busy", "cdb@example.com", "pw")
+        self.client.force_login(admin)
+        api = APIClient()
+        api.force_login(admin)
+        url = reverse("compute_delete", args=[self.host_a.id])
+        self.assertRedirects(self.client.post(url, HTTP_REFERER=f"http://testserver{url}"), url,
+                             fetch_redirect_response=False)
+        self.assertEqual(api.delete(reverse("compute-detail", args=[self.host_a.id])).status_code, 409)
+        self.assertTrue(Compute.objects.filter(pk=self.host_a.pk).exists())
+        self.assertEqual(list(vm.userinstance_set.values_list("user", flat=True)), [self.owner.id])
+        self.assertFalse(InstanceTombstone.objects.exists())

computes/utils.py

@@ -62,6 +62,19 @@ def _move_ownership(source, target):
         target.save(update_fields=["is_template"])
 
 
+def forget(inst):
+    """Remove an instance whose VM its compute no longer lists, or whose
+    compute is deleted. Its ownership goes to the same UUID on another
+    compute (an out-of-band move) or is kept in a tombstone in case the VM
+    comes back."""
+    elsewhere = Instance.objects.filter(uuid=inst.uuid).exclude(compute_id=inst.compute_id).first()
+    if elsewhere:
+        _move_ownership(inst, elsewhere)
+    else:
+        _bury(inst)
+    inst.delete()
+
+
 def _bury(inst):
     owners = [
         {"user": ui.user_id, "is_change": ui.is_change, "is_delete": ui.is_delete, "is_vnc": ui.is_vnc}
@@ -165,17 +178,10 @@ def refresh_instance_database(compute):
                             inst.name = new_name
                             inst.save(update_fields=["name"])
 
-                # 2. Instances gone from this compute. Their ownership goes to the
+                # 2. Instances gone from this compute
-                #    same UUID on another compute (an out-of-band move) or is kept
-                #    in a tombstone in case the VM comes back.
                 for inst in db_instances:
                     if inst.uuid not in host_domains:
-                        elsewhere = Instance.objects.filter(uuid=inst.uuid).exclude(compute=compute).first()
+                        forget(inst)
-                        if elsewhere:
-                            _move_ownership(inst, elsewhere)
-                        else:
-                            _bury(inst)
-                        inst.delete()
 
                 # 3. Instances created out-of-band. A UUID that also exists on another
                 #    compute is a copy and gets no owners; otherwise a tombstone's

computes/views.py

@@ -1,6 +1,7 @@
 import json
 
 from admin.decorators import superuser_only
+from django.conf import settings
 from django.core import signing
 from django.http import HttpResponse
 from django.shortcuts import get_object_or_404, redirect, render
@@ -129,8 +130,9 @@ def compute_delete(request, compute_id):
         request,
         "common/confirm_delete.html",
         {"object": compute,
-         "consequence": _("Its VMs keep running on the host, but the panel forgets them and who owns them; "
+         "consequence": _("Its VMs keep running on the host and leave the panel; their owners are kept for "
-                          "added again, the host's VMs come back without owners.")},
+                          "%(days)s days on the Removed VMs page and come back if the host is added again.")
+                        % {"days": getattr(settings, "INSTANCE_OWNERSHIP_RETENTION_DAYS", 30)}},
     )
 
 

storages/tests/test_upload_job.py

@@ -303,7 +303,9 @@ class LocalUploadTests(LocalPool, UploadTestCase):
 
     def test_ending_without_the_compute(self):
         op = self.send(0, b"aaa")
-        self.compute.delete()
+        with self.assertRaises(claims.Busy):  # the upload holds the compute
+            self.compute.delete()
+        Compute.objects.filter(pk=self.compute.pk).delete()  # the row gone without Compute.delete()
         op = jobs.cancel(op.pk, self.admin)
         self.assertEqual((op.state, op.error), (Operation.FAILED, "The compute was deleted"))
 

webvirtcloud/tests/test_delete_pages.py

@@ -19,7 +19,7 @@ class DeletePageTests(TestCase):
         compute = Compute.objects.create(name="del-host", hostname="192.0.2.9", login="u", password="p", type=1)
         vm = Instance.objects.create(compute=compute, name="del-vm", uuid="d0000000-0000-4000-8000-000000000001")
         pages = {
-            reverse("compute_delete", args=[compute.id]): "the panel forgets them and who owns them",
+            reverse("compute_delete", args=[compute.id]): "their owners are kept for 30 days on the Removed VMs page",
             reverse("admin:user_delete", args=[user.id]): "the VMs themselves stay",
             reverse("admin:group_delete", args=[Group.objects.create(name="del-group").id]): "lose the permissions",
             reverse("admin:tombstone_delete", args=[InstanceTombstone.objects.create(uuid=vm.uuid, name="gone").id]):