mirror of
https://github.com/retspen/webvirtcloud
synced 2026-11-09 03:36:48 +00:00
fix(computes): VM ownership survives a move or a short absence
The sync deleted the Instance of every VM missing from its compute, and its owners and rights went with it. A VM moved with virsh, or undefined and defined again, came back with no owner. Now, when a VM disappears from a compute: - if the same UUID is recorded on another compute, it moved: its owners and template flag go there; - otherwise they are kept in an InstanceTombstone and restored when the UUID appears again within INSTANCE_OWNERSHIP_RETENTION_DAYS (30). A UUID recorded on two computes at the same time is a copy and gets no owners. VMs deleted through WebVirtCloud leave no tombstone.
This commit is contained in:
catborise
parent
abdd229a1f
commit
d054b13a87
5 changed files with 237 additions and 6 deletions
112
computes/test_ownership.py
Normal file
112
computes/test_ownership.py
Normal file
|
|
@ -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, []))
|
||||
|
|
@ -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
|
||||
|
|
|
|||
32
instances/migrations/0013_instancetombstone.py
Normal file
32
instances/migrations/0013_instancetombstone.py
Normal file
|
|
@ -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)),
|
||||
],
|
||||
),
|
||||
]
|
||||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue