diff --git a/computes/test_ownership.py b/computes/test_ownership.py new file mode 100644 index 00000000..e7d58f3b --- /dev/null +++ b/computes/test_ownership.py @@ -0,0 +1,112 @@ +"""R-05: ownership survives a VM's temporary disappearance or an out-of-band move.""" + +from datetime import timedelta +from unittest.mock import MagicMock + +from accounts.models import UserInstance +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.utils import timezone +from instances.models import Instance, InstanceTombstone + +UUID = "44444444-4444-4444-4444-444444444444" + + +class Domain: + def __init__(self, name, uuid): + self._name, self._uuid = name, uuid + + def name(self): + return self._name + + def UUIDString(self): + return self._uuid + + +class OwnershipTestCase(TestCase): + def setUp(self): + self.owner = get_user_model().objects.create_user("owner", password="pw") + self.host_a = self.compute("host-a") + self.host_b = self.compute("host-b") + + def compute(self, name): + compute = Compute.objects.create(name=name, hostname=f"{name}.example", login="", password="", type=1) + compute.__dict__["proxy"] = MagicMock() + self.on(compute, []) + return compute + + def on(self, compute, domains): + compute.proxy.wvm.listAllDomains.return_value = [Domain(n, u) for n, u in domains] + + def owned_vm(self, compute): + vm = Instance.objects.create(compute=compute, name="vm", uuid=UUID, is_template=True) + UserInstance.objects.create(instance=vm, user=self.owner, is_change=True, is_vnc=True) + return vm + + def ownership(self, compute): + vm = Instance.objects.get(compute=compute, uuid=UUID) + return vm.is_template, [(ui.user, ui.is_change, ui.is_delete, ui.is_vnc) for ui in vm.userinstance_set.all()] + + def test_vm_that_disappears_and_returns_keeps_its_owners(self): + self.owned_vm(self.host_a) + refresh_instance_database(self.host_a) # undefined for a while + self.assertFalse(Instance.objects.filter(uuid=UUID).exists()) + + self.on(self.host_a, [("vm", UUID)]) # defined again + refresh_instance_database(self.host_a) + + self.assertEqual(self.ownership(self.host_a), (True, [(self.owner, True, False, True)])) + self.assertFalse(InstanceTombstone.objects.exists()) + + def test_move_seen_on_the_new_host_first(self): + self.owned_vm(self.host_a) + self.on(self.host_a, [("vm", UUID)]) + self.on(self.host_b, [("vm", UUID)]) + refresh_instance_database(self.host_b) # still listed on A: a copy for now + self.on(self.host_a, []) + refresh_instance_database(self.host_a) # gone from A: it moved to B + + self.assertEqual(self.ownership(self.host_b), (True, [(self.owner, True, False, True)])) + self.assertFalse(Instance.objects.filter(compute=self.host_a).exists()) + + def test_move_seen_on_the_old_host_first(self): + self.owned_vm(self.host_a) + refresh_instance_database(self.host_a) + self.on(self.host_b, [("vm", UUID)]) + refresh_instance_database(self.host_b) + + self.assertEqual(self.ownership(self.host_b), (True, [(self.owner, True, False, True)])) + + def test_copied_uuid_grants_nothing(self): + self.owned_vm(self.host_a) + self.on(self.host_a, [("vm", UUID)]) + self.on(self.host_b, [("vm-copy", UUID)]) + refresh_instance_database(self.host_a) + refresh_instance_database(self.host_b) + + self.assertEqual(self.ownership(self.host_a), (True, [(self.owner, True, False, True)])) + self.assertEqual(self.ownership(self.host_b), (False, [])) + + @override_settings(INSTANCE_OWNERSHIP_RETENTION_DAYS=30) + def test_ownership_is_kept_for_a_bounded_time(self): + self.owned_vm(self.host_a) + refresh_instance_database(self.host_a) + InstanceTombstone.objects.update(removed=timezone.now() - timedelta(days=31)) + + self.on(self.host_a, [("vm", UUID)]) + refresh_instance_database(self.host_a) + + self.assertEqual(self.ownership(self.host_a), (False, [])) + self.assertFalse(InstanceTombstone.objects.exists()) + + def test_deleted_users_are_skipped(self): + self.owned_vm(self.host_a) + refresh_instance_database(self.host_a) + self.owner.delete() + + self.on(self.host_a, [("vm", UUID)]) + refresh_instance_database(self.host_a) + + self.assertEqual(self.ownership(self.host_a), (True, [])) diff --git a/computes/utils.py b/computes/utils.py index 97bd13a0..96f3f28b 100644 --- a/computes/utils.py +++ b/computes/utils.py @@ -6,9 +6,15 @@ import stat import tempfile import threading import time +from datetime import timedelta + +from accounts.models import UserInstance +from django.conf import settings +from django.contrib.auth import get_user_model from django.db import transaction +from django.utils import timezone from computes.models import Compute -from instances.models import Instance +from instances.models import Instance, InstanceTombstone logger = logging.getLogger(__name__) @@ -147,11 +153,61 @@ def libvirt_instance_lock(instance, timeout=15.0): yield +def _move_ownership(source, target): + """Give target the owners and template flag of source; a user's rights are merged.""" + for ui in UserInstance.objects.filter(instance=source): + existing = UserInstance.objects.filter(instance=target, user=ui.user).first() + if existing is None: + ui.instance = target + ui.save(update_fields=["instance"]) + continue + for flag in ("is_change", "is_delete", "is_vnc"): + if getattr(ui, flag): + setattr(existing, flag, True) + existing.save() + if source.is_template and not target.is_template: + target.is_template = True + target.save(update_fields=["is_template"]) + + +def _bury(inst): + owners = [ + {"user": ui.user_id, "is_change": ui.is_change, "is_delete": ui.is_delete, "is_vnc": ui.is_vnc} + for ui in UserInstance.objects.filter(instance=inst) + ] + if owners or inst.is_template: + InstanceTombstone.objects.create(uuid=inst.uuid, name=inst.name, is_template=inst.is_template, owners=owners) + logger.info("Instance %s (%s) disappeared; ownership kept", inst.name, inst.uuid) + + +def _restore(inst): + tombstone = InstanceTombstone.objects.filter(uuid=inst.uuid).order_by("-removed").first() + if tombstone is None: + return + users = set(get_user_model().objects.filter(id__in=[o["user"] for o in tombstone.owners]).values_list("id", flat=True)) + for owner in tombstone.owners: + if owner["user"] in users: + UserInstance.objects.create( + instance=inst, + user_id=owner["user"], + is_change=owner["is_change"], + is_delete=owner["is_delete"], + is_vnc=owner["is_vnc"], + ) + if tombstone.is_template: + inst.is_template = True + inst.save(update_fields=["is_template"]) + InstanceTombstone.objects.filter(uuid=inst.uuid).delete() + logger.info("Instance %s (%s) is back; ownership restored", inst.name, inst.uuid) + + def refresh_instance_database(compute): """ 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 removed in virt-manager/virsh are deleted from the database; their + ownership is moved to the same UUID on another compute or kept in a + tombstone for INSTANCE_OWNERSHIP_RETENTION_DAYS (R-05). - 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). @@ -207,17 +263,32 @@ def refresh_instance_database(compute): inst.name = new_name inst.save(update_fields=["name"]) - # 2. Delete instances that were genuinely undefined/deleted in virsh/virt-manager - Instance.objects.filter(compute=compute).exclude(uuid__in=host_domains.keys()).delete() + # 2. Instances gone from this compute. Their 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 (R-05). + for inst in db_instances: + if inst.uuid not in host_domains: + elsewhere = Instance.objects.filter(uuid=inst.uuid).exclude(compute=compute).first() + if elsewhere: + _move_ownership(inst, elsewhere) + else: + _bury(inst) + inst.delete() - # 3. Create or update new instances that were created out-of-band in virsh/virt-manager + # 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 + # ownership is restored. + retention = getattr(settings, "INSTANCE_OWNERSHIP_RETENTION_DAYS", 30) + InstanceTombstone.objects.filter(removed__lt=timezone.now() - timedelta(days=retention)).delete() for uuid, name in host_domains.items(): if uuid not in db_uuids: - Instance.objects.get_or_create( + inst, created = Instance.objects.get_or_create( compute=compute, uuid=uuid, defaults={"name": name}, ) + if created and not Instance.objects.filter(uuid=uuid).exclude(compute=compute).exists(): + _restore(inst) except (OSError, PermissionError, TimeoutError) as e: logger.error("Could not synchronize instances for compute %s: %s; failing closed", compute_pk, e) return diff --git a/instances/migrations/0013_instancetombstone.py b/instances/migrations/0013_instancetombstone.py new file mode 100644 index 00000000..ef55a77c --- /dev/null +++ b/instances/migrations/0013_instancetombstone.py @@ -0,0 +1,32 @@ +# Generated by Django 5.2.17 on 2026-10-03 19:44 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ("instances", "0012_instance_unique_compute_uuid"), + ] + + operations = [ + migrations.CreateModel( + name="InstanceTombstone", + fields=[ + ( + "id", + models.BigAutoField( + auto_created=True, + primary_key=True, + serialize=False, + verbose_name="ID", + ), + ), + ("uuid", models.CharField(db_index=True, max_length=36)), + ("name", models.CharField(max_length=120)), + ("is_template", models.BooleanField(default=False)), + ("owners", models.JSONField(default=list)), + ("removed", models.DateTimeField(auto_now_add=True)), + ], + ), + ] diff --git a/instances/models.py b/instances/models.py index c356f035..d85e625e 100644 --- a/instances/models.py +++ b/instances/models.py @@ -299,6 +299,20 @@ class CreateInstance(models.Model): managed = False +class InstanceTombstone(models.Model): + """Ownership of a VM that disappeared from its compute (R-05). + + Kept for INSTANCE_OWNERSHIP_RETENTION_DAYS so that the owners come back + when the same UUID shows up again, on this or another compute. + """ + + uuid = models.CharField(max_length=36, db_index=True) + name = models.CharField(max_length=120) + is_template = models.BooleanField(default=False) + owners = models.JSONField(default=list) # [{"user": id, "is_change": .., "is_delete": .., "is_vnc": ..}] + removed = models.DateTimeField(auto_now_add=True) + + class PermissionSet(models.Model): """ Dummy model for holding set of permissions we need to be automatically added by Django diff --git a/webvirtcloud/settings.py.template b/webvirtcloud/settings.py.template index 75941bd4..da3edc17 100644 --- a/webvirtcloud/settings.py.template +++ b/webvirtcloud/settings.py.template @@ -287,6 +287,8 @@ LIBVIRT_KEEPALIVE_COUNT = 5 ALLOW_EMPTY_PASSWORD = False NEW_USER_DEFAULT_INSTANCES = [] +# Days the owners of a VM that vanished from its compute are kept, in case it comes back +INSTANCE_OWNERSHIP_RETENTION_DAYS = 30 SHOW_PROFILE_EDIT_PASSWORD = True