1
0
Fork 0
mirror of https://github.com/retspen/webvirtcloud synced 2026-11-08 11:16:47 +00:00

feat(jobs): deleting a VM is a job that recovery finishes

The delete ran in the request: undefine, then the VM's record, then its
disk files. A request that died in between (a gunicorn worker past its
timeout, OOM, a restart), or a database write that failed after the
undefine, left a record that later showed as removed outside the panel,
and the disk files the user asked to delete were never deleted.

Deleting a VM is now an instance.delete job. It holds the compute
exclusively, as the request did, so no other VM attaches a file it is
about to delete. The job chooses the files before it stops the VM (a host
that cannot be checked still stops nothing) and records them first. It
then stops and undefines the VM, removes its record, and deletes the
files, each with its own Logs row. Recovery decides from libvirt: a VM
still on its host means the delete failed; a VM gone means libvirt did
it, so recovery removes the record and deletes the recorded files that
are still there.

The page queues the job and returns to the VM page, where the job panel
follows it and opens the VM list when it ends; disks that were kept are
the result's warning. The API's delete answers 202 with the job and
needs a submission_key. backing_path moves to wvmConnect, as recovery
has no domain to ask.
This commit is contained in:
catborise 2026-11-06 09:49:31 +03:00
parent ce066052d9
commit 02412b7ea5
19 changed files with 923 additions and 764 deletions

View file

@ -772,7 +772,7 @@ What an update from an older version changes for operators and users:
- The panel backs itself up every day ([Backups](#backups)); in Docker into a new volume, `webvirtcloud-backups`.
- In Docker, the TLS client certificates of TLS computes are kept in a new volume, `webvirtcloud-pki` (`/var/www/.pki`), and backed up; put them there once ([Adding a Compute Node](#adding-a-compute-node), step 2).
- The SQLite database must be on a local file system (WAL). Outside compose, stop the Docker container with `docker stop -t 95` so the worker can finish running jobs.
- Long operations run as jobs in a worker process (`manage.py run_worker`): creating, cloning and migrating VMs, cloning volumes, taking and reverting snapshots and deleting external snapshots. The page returns at once and the job panel and the Jobs page follow them; a migration can be cancelled. The API's VM create and migrate answer 202 with the job and need a new `submission_key` per request. The create form no longer takes a console password; set it on the VM page.
- Long operations run as jobs in a worker process (`manage.py run_worker`): creating, cloning, migrating and deleting VMs, cloning volumes, taking and reverting snapshots and deleting external snapshots. The page returns at once and the job panel and the Jobs page follow them; a migration can be cancelled. The API's VM create, migrate and delete answer 202 with the job and need a new `submission_key` per request. The create form no longer takes a console password; set it on the VM page.
- An ISO upload is tracked as a job that can be cancelled. The browser sends the file, so stay on the page until it ends; an upload left idle for an hour is ended and its partial file deleted.
- A clone of a running VM (libvirt 11.10 or later, local file disks, no TPM) locks the VM until the copy ends and can be cancelled; cancelling a clone of a shut-off VM has no effect. A job that ends uncertain keeps its VM locked until a superuser resolves it on its job page.
- The job worker runs at most half of its concurrency (Settings, at least one) of one host's jobs at a time, so a host that does not answer cannot take every slot.

View file

@ -70,6 +70,12 @@ class DestroyOptionsSerializer(serializers.Serializer):
delete_nvram = serializers.BooleanField(default=False)
class DestroySerializer(DestroyOptionsSerializer):
"""The API's delete: a job, which a new submission key per request names."""
submission_key = serializers.CharField(write_only=True)
class CreateInstanceSerializer(serializers.ModelSerializer):
# BIOS, UEFI or UEFI Secure Boot, as the host offers; left out: the machine's default
firmware = serializers.CharField(required=False)

View file

@ -7,7 +7,6 @@ from django.shortcuts import get_object_or_404
from django.urls import reverse
from instances.jobs import MIGRATE_OPTIONS
from instances.models import Flavor, Instance
from instances.views import destroy as instance_destroy
from instances.views import (
force_off,
get_instance,
@ -28,7 +27,7 @@ from webvirtcloud.serializers import ActionStatusSerializer
from .serializers import (
CreateInstanceSerializer,
DestroyOptionsSerializer,
DestroySerializer,
FlavorSerializer,
InstanceDetailsSerializer,
InstanceSerializer,
@ -40,8 +39,8 @@ def instance_action_response(action, request, pk, message):
response = action(request, pk)
if response.status_code >= 400:
return response
# the page's view reports a partial result (a disk that was kept) as a
# message for the page it redirects to: the API answers with them
# the page's view reports a partial result as a message for the page it
# redirects to: the API answers with them
return Response({"status": message, "messages": [str(m) for m in get_messages(request)]})
@ -124,12 +123,19 @@ class InstanceViewSet(viewsets.ViewSet):
return Response(serializer.data)
@extend_schema(request=DestroyOptionsSerializer, responses=ActionStatusSerializer)
@extend_schema(request=DestroySerializer, responses={202: OperationSerializer, 400: dict})
def destroy(self, request, pk=None, compute_pk=None):
queryset = get_instance(request.user, pk, perm_type="delete")
if compute_pk is not None and str(queryset.compute_id) != str(compute_pk):
raise Http404()
return instance_action_response(instance_destroy, request, pk, "Instance is destroyed")
serializer = DestroySerializer(data=request.data)
if not serializer.is_valid():
return Response(serializer.errors, status=status.HTTP_400_BAD_REQUEST)
data = dict(serializer.validated_data)
key = data.pop("submission_key")
op = jobs.submit("instance.delete", request.user, {"instance": queryset.pk, **data}, key)
return Response(OperationSerializer(op, context={"request": request}).data, status=status.HTTP_202_ACCEPTED,
headers={"Location": reverse("operation-detail", args=[op.pk])})
@extend_schema(request=None, responses=ActionStatusSerializer)
@action(detail=True, methods=["post"])

View file

@ -14,7 +14,9 @@ from django.template.loader import render_to_string
from django.utils.translation import gettext_noop
from django.utils.translation import gettext_noop as _
from libvirt import (VIR_CONNECT_LIST_STORAGE_POOLS_ACTIVE, VIR_DOMAIN_JOB_NONE, VIR_DOMAIN_SNAPSHOT_LIST_EXTERNAL,
VIR_DOMAIN_SHUTOFF, VIR_ERR_NO_DOMAIN, VIR_ERR_NO_STORAGE_VOL, libvirtError)
VIR_DOMAIN_SHUTOFF, VIR_DOMAIN_UNDEFINE_KEEP_NVRAM, VIR_DOMAIN_UNDEFINE_MANAGED_SAVE,
VIR_DOMAIN_UNDEFINE_NVRAM, VIR_DOMAIN_UNDEFINE_SNAPSHOTS_METADATA, VIR_ERR_NO_DOMAIN,
VIR_ERR_NO_STORAGE_VOL, libvirtError)
from lxml import etree
from vrtManager import util
from vrtManager.connection import wvmConnect
@ -27,6 +29,7 @@ from appsettings.settings import app_settings
from computes.models import Compute
from instances import utils
from instances.models import Instance
from logs.audit import write
from operations import claims
from operations.utils import safe_text, say
from operations.jobs import Cancelled, Kind, Outcome, Plan, register
@ -1215,3 +1218,108 @@ class MoveDisk(Kind):
def log_entry(self, op, state):
return _log(op, state, f"move disk {op.params['dev']} to pool {op.params['pool']}")
DISK_KEPT_BELOW = _("Disk %(path)s was kept: a disk file that stays needs it")
DISK_NOT_DELETED = _("The VM is removed, but disk %(path)s was not deleted: %(err)s")
DISK_SHARED = _("Disk %(path)s is used by another VM and was kept")
def _deletable(user, pk):
"""The delete view's rule: the VM's delete right."""
from instances.views import get_instance
if user is None or not user.is_active:
raise PermissionDenied
return get_instance(user, pk, perm_type="delete")
def _removed(op, recovering=False):
"""What follows the undefine: the VM's record, then the disk files the job
chose before it stopped the VM; the job's result. Each disk has its own
Logs row; a file that stays keeps the files below it, which it needs. In
recovery a file already gone was deleted before the worker stopped."""
Instance.objects.filter(compute_id=op.intent["compute"], uuid=op.intent["uuid"]).delete()
compute = Compute.objects.get(pk=op.intent["compute"])
conn = wvmConnect(compute.hostname, compute.login, compute.password, compute.type)
notes, kept = [], set()
for path in op.intent["to_delete"]:
if path in kept:
notes += say(DISK_KEPT_BELOW, path=path)
continue
try:
conn.get_volume_by_path(path).delete(0)
except util.ConnectionFailed:
raise # unreachable: recovery waits and retries
except libvirtError as err:
if recovering and err.get_error_code() == VIR_ERR_NO_STORAGE_VOL:
continue
notes += say(DISK_NOT_DELETED, path=path, err=err)
write(op.username, compute.name, op.intent["name"], f"VM {op.intent['name']}: delete volume {path}: "
f"{safe_text(err)}", False, uuid=op.intent["uuid"])
try:
while (path := conn.backing_path(path)) and path not in kept:
kept.add(path)
except libvirtError:
kept.update(op.intent["to_delete"]) # the files it needs are unknown: none of the rest goes
else:
write(op.username, compute.name, op.intent["name"], f"VM {op.intent['name']}: delete volume {path}",
True, uuid=op.intent["uuid"])
for path in op.intent["shared"]:
notes += say(DISK_SHARED, path=path)
return {"open": reverse("instances:index"), **({"warning": notes} if notes else {})}
@register
class DeleteInstance(Kind):
"""Undefine the VM, then remove its record and the disk files chosen
before it was stopped. Recovery finishes a delete libvirt has done."""
name = "instance.delete"
label = gettext_noop("Delete VM")
def normalize(self, raw):
return {"instance": int(raw["instance"]), "delete_disk": bool(raw.get("delete_disk")),
"delete_nvram": bool(raw.get("delete_nvram"))}
def validate(self, user, params):
_deletable(user, params["instance"])
def plan(self, user, params):
instance = _instance(params["instance"])
# the whole compute: a file to delete must not be attached by another VM meanwhile
return Plan(claims={claims.compute_key(instance.compute_id): Claim.EXCLUSIVE,
claims.instance_key(instance.uuid): Claim.EXCLUSIVE},
resource=claims.instance_key(instance.uuid), label=f"{instance.compute.name}/{instance.name}",
intent={"compute": instance.compute_id, "uuid": instance.uuid, "name": instance.name})
def precheck(self, op, ctx):
if _deletable(op.user, op.params["instance"]).compute_id != op.intent["compute"]:
raise util.OperationError(MOVED)
def run(self, op, ctx):
instance = _instance(op.params["instance"])
proxy = _proxy(instance)
to_delete, shared = [], []
if op.params["delete_disk"]:
# known before the VM is stopped: a host that cannot be checked stops nothing
to_delete, shared = proxy.split_disk_paths_by_use(
lambda own: utils.paths_used_on_other_computes(instance.compute, own))
ctx.record_intent(to_delete=to_delete, shared=shared)
ctx.set_phase(gettext_noop("Deleting the VM"))
# paused and other active states stop too, or the VM runs on as a transient domain
if proxy.instance.isActive():
proxy.force_shutdown()
# snapshot metadata goes with the domain; internal snapshot data stays in kept disk images
flags = VIR_DOMAIN_UNDEFINE_MANAGED_SAVE | VIR_DOMAIN_UNDEFINE_SNAPSHOTS_METADATA
flags |= VIR_DOMAIN_UNDEFINE_NVRAM if op.params["delete_nvram"] else VIR_DOMAIN_UNDEFINE_KEEP_NVRAM
proxy.delete(flags)
return _removed(op)
def reconcile(self, op):
if _domain(op.intent["compute"], op.intent["uuid"]) is not None:
return Outcome(Operation.FAILED, error=say(_("The VM was not deleted; it may have been shut off")))
return Outcome(Operation.SUCCEEDED, result=_removed(op, recovering=True))
def log_entry(self, op, state):
return _log(op, state, "delete")

View file

@ -1,5 +1,6 @@
{% extends 'base.html' %}
{% load i18n %}
{% load operations %}
{% block title %}{% blocktrans with name=instance.name %}Destroy {{ name }}{% endblocktrans %}{% endblock title %}
{% block page_heading %}
@ -8,7 +9,7 @@
{% block content %}
<p class="small">{% trans "The VM's definition, saved state and snapshot records are removed. A running VM is forced off first; what the guest has not written to disk is lost." %}</p>
<form action="{% url 'instances:destroy' instance.id %}" method="post" id="delete_form">{% csrf_token %}
<form action="{% url 'instances:destroy' instance.id %}" method="post" id="delete_form">{% csrf_token %}{% submission_key %}
<fieldset class="mb-3">
<legend class="h6">{% trans "What should happen to the files?" %}</legend>
<div class="row g-2">

View file

@ -1,36 +1,67 @@
from contextlib import contextmanager
from types import SimpleNamespace
from unittest.mock import MagicMock, patch
from computes.models import Compute
from django.contrib.auth import get_user_model
from django.contrib.messages import get_messages
from django.test import TestCase
from django.test import TestCase, TransactionTestCase
from django.urls import reverse
from instances.jobs import DeleteInstance
from instances.models import Instance
from instances.utils import paths_used_on_other_computes
from libvirt import VIR_DOMAIN_UNDEFINE_KEEP_NVRAM, VIR_DOMAIN_UNDEFINE_NVRAM, libvirtError
from operations.utils import public_error_text
from libvirt import VIR_DOMAIN_UNDEFINE_KEEP_NVRAM, VIR_DOMAIN_UNDEFINE_NVRAM, VIR_ERR_NO_STORAGE_VOL, libvirtError
from logs.models import Logs
from operations.jobs import Context
from operations.models import Claim, Operation, acquire
from operations.utils import public_error_text, words
from vrtManager.util import ConnectionFailed, OperationError
UUID = "11111111-2222-3333-4444-555555555555"
class DestroyOrderTestCase(TestCase):
class DestroyOrderTestCase(TransactionTestCase):
def setUp(self):
admin = get_user_model().objects.create_superuser("destroy_admin", "d@example.com", "pw")
self.client.force_login(admin)
self.admin = get_user_model().objects.create_superuser("destroy_admin", "d@example.com", "pw")
self.client.force_login(self.admin)
self.client.raise_request_exception = False
compute = Compute.objects.create(name="destroy-compute", hostname="127.0.0.1", login="", password="", type=4)
self.instance = Instance.objects.create(
compute=compute, name="destroy-vm", uuid="11111111-2222-3333-4444-555555555555"
)
self.instance = Instance.objects.create(compute=compute, name="destroy-vm", uuid=UUID)
self.proxy = MagicMock()
self.proxy.instance.isActive.return_value = False
self.proxy.backing_path.return_value = None
self.proxy.split_disk_paths_by_use.return_value = (["/pool/own.qcow2"], ["/pool/shared.qcow2"])
# the job's VM and its compute's connection are the same host here
for target in ("instances.jobs.wvmInstance", "instances.jobs.wvmConnect", "instances.models.wvmInstance"):
patch(target, return_value=self.proxy).start()
self.addCleanup(patch.stopall)
def destroy(self, undefine_error=None, data=None):
with patch("instances.models.wvmInstance") as wvm:
proxy = wvm.return_value
proxy.instance.isActive.return_value = False
proxy.split_disk_paths_by_use.return_value = (["/pool/own.qcow2"], ["/pool/shared.qcow2"])
proxy.delete.side_effect = undefine_error
self.client.post(reverse("instances:destroy", args=[self.instance.id]), data or {"delete_disk": "1"})
return proxy
def queue(self, **data):
self.client.post(reverse("instances:destroy", args=[self.instance.id]),
{"submission_key": "key-destroy01", **data})
return Operation.objects.get()
def run_job(self, op):
"""The job as the worker runs it; its result."""
acquire("wvc-test:1:a", "wvc-test")
Operation.objects.filter(pk=op.pk).update(state=Operation.RUNNING, worker="wvc-test:1:a")
op.refresh_from_db()
ctx = Context(op, SimpleNamespace(identity="wvc-test:1:a"))
DeleteInstance().precheck(op, ctx)
return DeleteInstance().run(op, ctx)
def test_the_page_queues_the_delete_and_changes_nothing(self):
response = self.client.post(reverse("instances:destroy", args=[self.instance.id]),
{"submission_key": "key-destroy01", "delete_disk": "true"})
self.assertRedirects(response, reverse("instances:instance", args=[self.instance.id]),
fetch_redirect_response=False)
op = Operation.objects.get()
self.assertEqual((op.kind, op.params), ("instance.delete", {
"instance": self.instance.pk, "delete_disk": True, "delete_nvram": False}))
# the whole compute: a file to delete must not be attached meanwhile
self.assertEqual(set(Claim.objects.values_list("resource", "mode")), {
(f"compute:{self.instance.compute_id}", "exclusive"), (f"instance:{UUID}", "exclusive")})
self.assertEqual(self.proxy.mock_calls, [])
def test_the_nvram_goes_only_when_asked(self):
for data, wanted, unwanted in (
@ -38,47 +69,35 @@ class DestroyOrderTestCase(TestCase):
({}, VIR_DOMAIN_UNDEFINE_KEEP_NVRAM, VIR_DOMAIN_UNDEFINE_NVRAM),
):
with self.subTest(data=data):
Instance.objects.get_or_create(
compute=self.instance.compute, name="destroy-vm", uuid="11111111-2222-3333-4444-555555555555"
)
self.instance = Instance.objects.get(uuid="11111111-2222-3333-4444-555555555555")
flags = self.destroy(data=data).delete.call_args[0][0]
self.instance = Instance.objects.get_or_create(compute=self.instance.compute, name="destroy-vm",
uuid=UUID)[0]
Operation.objects.all().delete()
self.run_job(self.queue(**data))
flags = self.proxy.delete.call_args[0][0]
self.assertTrue(flags & wanted)
self.assertFalse(flags & unwanted)
def test_the_api_takes_its_options_as_booleans(self):
url = f"/api/v1/computes/{self.instance.compute_id}/instances/{self.instance.id}/"
for body, content_type, deleted in (
({"delete_disk": True}, "application/json", True),
("delete_disk=false", "application/x-www-form-urlencoded", False),
):
with self.subTest(body=body):
Instance.objects.get_or_create(
compute=self.instance.compute, name="destroy-vm", uuid="11111111-2222-3333-4444-555555555555"
)
self.instance = Instance.objects.get(uuid="11111111-2222-3333-4444-555555555555")
with patch("instances.models.wvmInstance") as wvm:
proxy = wvm.return_value
proxy.instance.isActive.return_value = False
proxy.split_disk_paths_by_use.return_value = (["/pool/own.qcow2"], [])
url = f"/api/v1/computes/{self.instance.compute_id}/instances/{self.instance.id}/"
response = self.client.delete(url, body, content_type=content_type)
self.assertEqual(response.status_code, 200)
self.assertEqual(proxy.get_volume_by_path.called, deleted)
def test_the_files_are_checked_before_the_vm_is_stopped_and_deleted_after_the_undefine(self):
self.proxy.instance.isActive.return_value = True
op = self.queue(delete_disk="true")
result = self.run_job(op)
calls = [name for name, *_ in self.proxy.mock_calls
if name in ("split_disk_paths_by_use", "force_shutdown", "delete", "get_volume_by_path")]
self.assertEqual(calls, ["split_disk_paths_by_use", "force_shutdown", "delete", "get_volume_by_path"])
self.proxy.get_volume_by_path.assert_called_once_with("/pool/own.qcow2")
self.assertFalse(Instance.objects.filter(pk=self.instance.pk).exists())
self.assertEqual(Operation.objects.get().intent["to_delete"], ["/pool/own.qcow2"]) # recorded first
self.assertEqual(result["open"], reverse("instances:index"))
self.assertEqual(words(result["warning"]), "Disk /pool/shared.qcow2 is used by another VM and was kept")
self.assertEqual(list(Logs.objects.exclude(message="Login").values_list("message", "success")),
[("VM destroy-vm: delete volume /pool/own.qcow2", True)])
def test_the_api_tells_which_disks_were_not_deleted(self):
with patch("instances.models.wvmInstance") as wvm:
proxy = wvm.return_value
proxy.instance.isActive.return_value = False
proxy.split_disk_paths_by_use.return_value = (["/pool/own.qcow2"], ["/pool/shared.qcow2"])
proxy.backing_path.return_value = None
proxy.get_volume_by_path.return_value.delete.side_effect = libvirtError("Permission denied")
url = f"/api/v1/computes/{self.instance.compute_id}/instances/{self.instance.id}/"
response = self.client.delete(url, {"delete_disk": True}, content_type="application/json")
self.assertEqual(response.status_code, 200)
self.assertEqual(response.json(), {"status": "Instance is destroyed", "messages": [
"The VM is removed, but disk /pool/own.qcow2 was not deleted: Permission denied",
"Disk /pool/shared.qcow2 is used by another VM and was kept"]})
def test_a_failed_undefine_deletes_no_disk(self):
self.proxy.delete.side_effect = libvirtError("refused")
with self.assertRaises(libvirtError):
self.run_job(self.queue(delete_disk="true"))
self.proxy.get_volume_by_path.assert_not_called()
self.assertTrue(Instance.objects.filter(pk=self.instance.pk).exists())
def test_a_disk_file_that_stays_keeps_the_files_it_needs(self):
# the overlay is not deleted: the file below it would leave it broken
@ -90,64 +109,85 @@ class DestroyOrderTestCase(TestCase):
vol.delete.side_effect = libvirtError("Permission denied")
return vol
with patch("instances.models.wvmInstance") as wvm:
proxy = wvm.return_value
proxy.instance.isActive.return_value = False
proxy.split_disk_paths_by_use.return_value = (["/pool/vm.snap1", "/pool/vm.qcow2", "/pool/vm-data.qcow2"], [])
proxy.get_volume_by_path.side_effect = volume
proxy.backing_path.side_effect = {"/pool/vm.snap1": "/pool/vm.qcow2"}.get
response = self.client.post(reverse("instances:destroy", args=[self.instance.id]), {"delete_disk": "true"})
self.proxy.split_disk_paths_by_use.return_value = (
["/pool/vm.snap1", "/pool/vm.qcow2", "/pool/vm-data.qcow2"], [])
self.proxy.get_volume_by_path.side_effect = volume
self.proxy.backing_path.side_effect = {"/pool/vm.snap1": "/pool/vm.qcow2"}.get
result = self.run_job(self.queue(delete_disk="true"))
self.assertNotIn("/pool/vm.qcow2", volumes)
volumes["/pool/vm-data.qcow2"].delete.assert_called_once_with(0)
self.assertEqual([str(m) for m in get_messages(response.wsgi_request)][-1],
# each disk has its own row, and the job's end the VM's (log_entry)
self.assertEqual(list(Logs.objects.exclude(message="Login").values_list("message", "success", "instance_uuid")), [
("VM destroy-vm: delete volume /pool/vm.snap1: Permission denied", False, UUID),
("VM destroy-vm: delete volume /pool/vm-data.qcow2", True, UUID)])
self.assertEqual(DeleteInstance().log_entry(Operation.objects.get(), Operation.SUCCEEDED),
("destroy-compute", "destroy-vm", UUID, "VM destroy-vm: delete"))
self.assertEqual(words(result["warning"]),
"The VM is removed, but disk /pool/vm.snap1 was not deleted: Permission denied. "
"Disk /pool/vm.qcow2 was kept: a disk file that stays needs it")
def test_when_the_files_a_kept_one_needs_are_unknown_none_of_the_rest_goes(self):
with patch("instances.models.wvmInstance") as wvm:
proxy = wvm.return_value
proxy.instance.isActive.return_value = False
proxy.split_disk_paths_by_use.return_value = (["/pool/a.qcow2", "/pool/b.qcow2"], [])
proxy.get_volume_by_path.return_value.delete.side_effect = libvirtError("Permission denied")
proxy.backing_path.side_effect = libvirtError("pool unreachable")
response = self.client.post(reverse("instances:destroy", args=[self.instance.id]), {"delete_disk": "true"})
self.assertEqual(response.status_code, 302)
proxy.get_volume_by_path.assert_called_once_with("/pool/a.qcow2")
def test_the_files_are_checked_before_the_vm_is_stopped(self):
with patch("instances.models.wvmInstance") as wvm:
proxy = wvm.return_value
proxy.instance.isActive.return_value = True
proxy.split_disk_paths_by_use.return_value = (["/pool/own.qcow2"], [])
self.client.post(reverse("instances:destroy", args=[self.instance.id]), {"delete_disk": "true"})
calls = [name for name, *_ in proxy.mock_calls if name in ("split_disk_paths_by_use", "force_shutdown", "delete")]
self.assertEqual(calls, ["split_disk_paths_by_use", "force_shutdown", "delete"])
self.proxy.split_disk_paths_by_use.return_value = (["/pool/a.qcow2", "/pool/b.qcow2"], [])
self.proxy.get_volume_by_path.return_value.delete.side_effect = libvirtError("Permission denied")
self.proxy.backing_path.side_effect = libvirtError("pool unreachable")
self.run_job(self.queue(delete_disk="true"))
self.proxy.get_volume_by_path.assert_called_once_with("/pool/a.qcow2")
def test_a_host_that_cannot_be_checked_stops_nothing(self):
Compute.objects.create(name="other-host", hostname="192.0.2.50", login="u", password="p", type=1)
def connect(host, *args):
if host == "192.0.2.50":
raise libvirtError("unreachable")
return MagicMock()
with patch("instances.models.wvmInstance") as wvm, patch("instances.utils.wvmConnect", side_effect=connect):
proxy = wvm.return_value
proxy.instance.isActive.return_value = True
proxy.split_disk_paths_by_use.side_effect = lambda elsewhere: elsewhere(["/pool/own.qcow2"])
response = self.client.post(reverse("instances:destroy", args=[self.instance.id]), {"delete_disk": "true"})
proxy.force_shutdown.assert_not_called()
proxy.delete.assert_not_called()
self.proxy.instance.isActive.return_value = True
self.proxy.split_disk_paths_by_use.side_effect = lambda elsewhere: elsewhere(["/pool/own.qcow2"])
op = self.queue(delete_disk="true")
with patch("instances.utils.wvmConnect", side_effect=connect), \
self.assertRaisesMessage(ConnectionFailed, "The disk files cannot be checked on other-host"):
self.run_job(op)
self.proxy.force_shutdown.assert_not_called()
self.proxy.delete.assert_not_called()
self.assertFalse(Operation.objects.get().effects_started) # the job fails without recovery
self.assertTrue(Instance.objects.filter(pk=self.instance.pk).exists())
self.assertIn("The disk files cannot be checked on other-host", str(list(get_messages(response.wsgi_request))[-1]))
def test_disks_are_deleted_after_the_undefine(self):
proxy = self.destroy()
proxy.get_volume_by_path.assert_called_once_with("/pool/own.qcow2")
def test_recovery_finishes_a_delete_libvirt_has_done(self):
op = self.queue(delete_disk="true")
Operation.objects.filter(pk=op.pk).update(intent={**op.intent, "to_delete": ["/pool/gone.qcow2", "/pool/own.qcow2"],
"shared": []})
op.refresh_from_db()
gone = libvirtError("no volume")
gone.err = (VIR_ERR_NO_STORAGE_VOL, 0, "no volume")
self.proxy.get_volume_by_path.side_effect = lambda path: (_ for _ in ()).throw(gone) \
if path == "/pool/gone.qcow2" else MagicMock()
with patch("instances.jobs._domain", return_value=None):
outcome = DeleteInstance().reconcile(op)
self.assertEqual((outcome.state, outcome.result), (Operation.SUCCEEDED, {"open": reverse("instances:index")}))
self.assertFalse(Instance.objects.filter(pk=self.instance.pk).exists())
self.assertEqual(list(Logs.objects.exclude(message="Login").values_list("message", flat=True)),
["VM destroy-vm: delete volume /pool/own.qcow2"])
def test_failed_undefine_deletes_no_disk(self):
proxy = self.destroy(undefine_error=libvirtError("refused"))
proxy.get_volume_by_path.assert_not_called()
def test_recovery_of_a_vm_still_on_its_host_fails(self):
op = self.queue(delete_disk="true")
with patch("instances.jobs._domain", return_value=MagicMock()):
self.assertEqual(DeleteInstance().reconcile(op).state, Operation.FAILED)
self.assertTrue(Instance.objects.filter(pk=self.instance.pk).exists())
self.proxy.get_volume_by_path.assert_not_called()
def test_the_api_queues_the_delete_with_its_options_as_booleans(self):
url = f"/api/v1/computes/{self.instance.compute_id}/instances/{self.instance.id}/"
for n, (body, content_type, deleted) in enumerate((
({"delete_disk": True, "submission_key": "key-api00001"}, "application/json", True),
("delete_disk=false&submission_key=key-api00002", "application/x-www-form-urlencoded", False),
)):
with self.subTest(body=body):
Operation.objects.all().delete()
response = self.client.delete(url, body, content_type=content_type)
self.assertEqual(response.status_code, 202)
self.assertEqual(Operation.objects.get().params["delete_disk"], deleted)
self.assertEqual(self.client.delete(url, {"delete_disk": True}, content_type="application/json").status_code,
400) # no submission key
def test_destroy_and_delete_vol_hold_the_compute_exclusively(self):
calls = []
@ -158,10 +198,8 @@ class DestroyOrderTestCase(TestCase):
yield
with patch("instances.views.libvirt_compute_lock", record):
with patch("instances.models.wvmInstance"):
self.client.post(reverse("instances:delete_vol", args=[self.instance.id]), {"dev": "vda"})
self.destroy()
self.assertEqual(calls, [False, False])
self.client.post(reverse("instances:delete_vol", args=[self.instance.id]), {"dev": "vda"})
self.assertEqual(calls, [False])
def test_a_volume_the_vms_snapshots_need_is_kept_after_the_detach(self):
disk = {"dev": "vdb", "path": "/pool/data.qcow2", "storage": "default", "image": "data.qcow2"}

View file

@ -8,6 +8,8 @@ TEST_LIBVIRT_HOST set (see instances/tests/livetest.py).
import os
import unittest
from types import SimpleNamespace
from uuid import uuid4
import libvirt
from computes.models import Compute
@ -18,7 +20,10 @@ from django.urls import reverse
from vrtManager.connection import connection_manager
from . import livetest
from instances.jobs import DeleteInstance
from instances.models import Instance
from operations.jobs import Context
from operations.models import Operation, acquire
from instances.utils import refr
P = livetest.PREFIX
@ -75,6 +80,19 @@ class LiveDataLossTestCase(TestCase):
HTTP_REFERER=reverse("instances:instance", args=[instance.id]),
)
def destroy(self, instance, data=None):
"""The delete page's request, then its job as the worker runs it."""
response = self.post("destroy", instance, {"submission_key": f"key-{uuid4().hex[:12]}", **(data or {})})
op = Operation.objects.get(kind="instance.delete", state=Operation.QUEUED)
acquire("live-test:1:a", "live-test")
Operation.objects.filter(pk=op.pk).update(state=Operation.RUNNING, worker="live-test:1:a")
op.refresh_from_db()
ctx = Context(op, SimpleNamespace(identity="live-test:1:a"))
DeleteInstance().precheck(op, ctx)
DeleteInstance().run(op, ctx)
Operation.objects.filter(pk=op.pk).update(state=Operation.SUCCEEDED)
return response
def disk_sources(self, dom, flags=0):
from lxml import etree
@ -156,7 +174,7 @@ class LiveDataLossTestCase(TestCase):
_, inst = self.vm("r14-a", [shared])
self.vm("r14-b", [shared])
response = self.post("destroy", inst, {"delete_disk": "1"})
response = self.destroy(inst, {"delete_disk": "1"})
self.assertEqual(response.status_code, 302)
self.assertTrue(livetest.volume_exists(self.conn, shared), "destroy deleted another VM's disk")
@ -175,7 +193,7 @@ class LiveDataLossTestCase(TestCase):
if running:
dom_b.create()
response = self.post("destroy", inst, {"delete_disk": "1"})
response = self.destroy(inst, {"delete_disk": "1"})
self.assertEqual(response.status_code, 302)
self.assertTrue(livetest.volume_exists(self.conn, base), "deleted the backing file of another VM's disk")
@ -192,7 +210,7 @@ class LiveDataLossTestCase(TestCase):
dom.create()
dom.managedSave(0)
self.post("destroy", inst, {"delete_disk": "1"})
self.destroy(inst, {"delete_disk": "1"})
uuid = dom.UUIDString()
self.assertFalse(self.domain_exists(uuid), "VM is still defined")
@ -204,7 +222,7 @@ class LiveDataLossTestCase(TestCase):
dom.create()
dom.suspend()
self.post("destroy", inst)
self.destroy(inst)
self.assertFalse(self.domain_exists(dom.UUIDString()), "paused VM is still running")
@ -231,7 +249,7 @@ class LiveDataLossTestCase(TestCase):
dom.destroy()
self.assertIn(f"{P}r14-nvram_VARS.fd", self.nvram_files())
self.post("destroy", inst, data)
self.destroy(inst, data)
self.assertFalse(self.domain_exists(dom.UUIDString()), "VM is still defined")
return f"{P}r14-nvram_VARS.fd" in self.nvram_files()
@ -267,7 +285,7 @@ class LiveDataLossTestCase(TestCase):
dom, inst = self.vm("r14-chain", [overlay])
dom.snapshotCreateXML("<domainsnapshot><name>snap</name></domainsnapshot>", 0)
response = self.post("destroy", inst, {"delete_disk": "1"})
response = self.destroy(inst, {"delete_disk": "1"})
self.assertEqual(response.status_code, 302)
self.assertFalse(self.domain_exists(dom.UUIDString()), "VM is still defined")
@ -295,7 +313,7 @@ class LiveDataLossTestCase(TestCase):
made = {name for name in pool.listVolumes() if name.startswith(P + "r14-ext.")} - {P + "r14-ext.qcow2"}
self.assertEqual(len(made), 2, made) # one overlay per snapshot
response = self.post("destroy", inst, {"delete_disk": "1"})
response = self.destroy(inst, {"delete_disk": "1"})
self.assertEqual(response.status_code, 302)
self.assertFalse(self.domain_exists(dom.UUIDString()), "VM is still defined")
@ -313,7 +331,7 @@ class LiveDataLossTestCase(TestCase):
inst.proxy.create_external_snapshot("b")
inst.proxy.revert_external_snapshot("a") # b's overlay is off the disk's chain now
response = self.post("destroy", inst, {"delete_disk": "1"})
response = self.destroy(inst, {"delete_disk": "1"})
self.assertEqual(response.status_code, 302)
pool = self.conn.storagePoolLookupByName(livetest.POOL)

View file

@ -20,7 +20,7 @@ from vrtManager.instance import wvmInstance
from appsettings.settings import get_settings
from computes.models import Compute
from instances.jobs import DeleteExternalSnapshot
from instances.jobs import DeleteExternalSnapshot, DeleteInstance
from instances.models import Instance
from operations import jobs
from operations.models import Operation
@ -111,6 +111,23 @@ class LiveJobsTestCase(TransactionTestCase):
self.assertEqual(op.state, Operation.SUCCEEDED, op.error)
return op
def test_the_worker_deletes_a_running_vm_with_its_disk_and_recovery_agrees(self):
disk = livetest.create_volume(self.conn, P + "job-delete")
dom = livetest.define_vm(self.conn, P + "job-delete", [disk])
dom.create()
instance = Instance.objects.create(compute=self.compute, name=dom.name(), uuid=dom.UUIDString())
op = self.run_job(jobs.submit("instance.delete", self.admin, {"instance": instance.pk, "delete_disk": True},
"key-delete01"))
self.assertEqual(op.result, {"open": reverse("instances:index")})
self.assertFalse(Instance.objects.filter(pk=instance.pk).exists())
self.assertFalse(livetest.volume_exists(self.conn, disk))
with self.assertRaises(libvirt.libvirtError):
self.conn.lookupByUUIDString(op.intent["uuid"])
# a worker that stopped after the undefine: recovery finds the work done
Operation.objects.filter(pk=op.pk).update(state=Operation.RUNNING)
op.refresh_from_db()
self.assertEqual(DeleteInstance().reconcile(op).state, Operation.SUCCEEDED)
def test_snapshot_create_and_revert_run_as_jobs(self):
running, _vm = self.vm("smoke-ext", "running")
stopped, _vm = self.vm("smoke-int")

View file

@ -12,7 +12,10 @@ from django.test import TestCase
from django.urls import reverse
from computes.models import Compute
from instances.jobs import DeleteInstance
from instances.models import Instance
from operations.models import Operation
from vrtManager.util import OperationError
class MovedWhileWaitingTestCase(TestCase):
@ -54,18 +57,11 @@ class MovedWhileWaitingTestCase(TestCase):
self.assertTrue(touched)
def test_destroy_of_a_vm_moved_while_it_waited_is_not_run(self):
# destroy holds the whole compute; the migration moved the VM before
# releasing it
@contextmanager
def compute_lock_after_a_migration(compute, *, shared=False):
Instance.objects.filter(pk=self.vm.pk).update(compute=self.target)
yield
with patch("instances.views.libvirt_compute_lock", compute_lock_after_a_migration), patch(
"instances.models.wvmInstance"
) as wvm, patch.object(Compute, "status", new_callable=PropertyMock, return_value=True):
response = self.client.post(reverse("instances:destroy", args=[self.vm.id]), {"delete_disk": "on"})
self.assertFalse(wvm.return_value.delete.called)
self.assertTrue(Instance.objects.filter(pk=self.vm.pk).exists())
messages = [str(m) for m in response.wsgi_request._messages]
self.assertIn("The VM was moved meanwhile; try again.", messages)
# the delete job holds the whole compute; a migration moved the VM before it ran
with patch("instances.models.wvmInstance"):
self.client.post(reverse("instances:destroy", args=[self.vm.id]),
{"delete_disk": "on", "submission_key": "key-moved001"})
op = Operation.objects.get(kind="instance.delete")
Instance.objects.filter(pk=self.vm.pk).update(compute=self.target)
with self.assertRaisesMessage(OperationError, "The VM moved to another compute"):
DeleteInstance().precheck(op, None)

View file

@ -9,6 +9,7 @@ from django.urls import reverse
from accounts.models import UserInstance
from computes.models import Compute
from instances.models import Instance
from operations.models import Operation
from operations import claims
from instances.views import get_instance
@ -257,11 +258,11 @@ class InstanceSecurityMutationsTestCase(TestCase):
# 3. Superuser can destroy VM
self.client.force_login(self.superuser)
with patch("instances.models.wvmInstance") as mock_wvm_cls:
mock_wvm_cls.return_value.get_status.return_value = 5 # shutoff
res = self.client.post(reverse("instances:destroy", args=[self.instance.id]), {})
with patch("instances.models.wvmInstance"):
res = self.client.post(reverse("instances:destroy", args=[self.instance.id]),
{"submission_key": "key-destroy01"})
self.assertEqual(res.status_code, 302)
self.assertFalse(Instance.objects.filter(id=self.instance.id).exists())
self.assertTrue(Operation.objects.filter(kind="instance.delete").exists()) # queued
def test_migration_deduplicate_instances_logic(self):
import importlib

View file

@ -12,6 +12,7 @@ from rest_framework.test import APIClient
from accounts.models import UserInstance
from computes.models import Compute
from instances.models import Instance
from operations.models import Operation
class TemplateRulesTestCase(TestCase):
@ -46,10 +47,9 @@ class TemplateRulesTestCase(TestCase):
self.assertFalse(wvm.return_value.delete.called)
self.client.force_login(self.staff)
with patch("instances.models.wvmInstance") as wvm:
wvm.return_value.instance.isActive.return_value = False
self.client.post(reverse("instances:destroy", args=[self.template.id]))
self.assertTrue(wvm.return_value.delete.called)
with patch("instances.models.wvmInstance"):
self.client.post(reverse("instances:destroy", args=[self.template.id]), {"submission_key": "key-tpldel01"})
self.assertTrue(Operation.objects.filter(kind="instance.delete").exists()) # queued
def test_the_destroy_confirmation_page_offers_the_form_to_who_may_delete(self):
url = reverse("instances:destroy", args=[self.template.id])

View file

@ -143,17 +143,6 @@ class VMRowTests(TestCase):
self.assertEqual(self.rows(), [("VM vm: set console password", True)])
self.proxy.set_console_listener_addr.assert_not_called()
def test_a_deleted_vm_and_each_of_its_disks_have_their_own_row(self):
self.proxy.split_disk_paths_by_use.return_value = (["/pool/a.qcow2", "/pool/b.qcow2"], [])
volume = self.proxy.get_volume_by_path.return_value
volume.delete.side_effect = [libvirtError("Permission denied"), None]
self.client.post(reverse("instances:destroy", args=[self.vm.pk]), {"delete_disk": "1"})
self.assertFalse(Instance.objects.filter(pk=self.vm.pk).exists())
self.assertEqual(self.rows(), [("VM vm: delete", True),
("VM vm: delete volume /pool/a.qcow2: Permission denied", False),
("VM vm: delete volume /pool/b.qcow2", True)])
self.assertEqual(set(Logs.objects.values_list("instance_uuid", flat=True)), {UUID})
def test_a_disk_delete_logs_the_detach_and_the_volume(self):
disk = {"dev": "vdb", "path": "/pool/data.qcow2", "storage": "default", "image": "data.qcow2"}
self.proxy.get_disk_devices.return_value = [disk]

View file

@ -42,10 +42,6 @@ from django.views.decorators.debug import sensitive_post_parameters
from libvirt import (VIR_DOMAIN_PAUSED,
VIR_DOMAIN_RUNNING,
VIR_DOMAIN_SHUTOFF,
VIR_DOMAIN_UNDEFINE_KEEP_NVRAM,
VIR_DOMAIN_UNDEFINE_MANAGED_SAVE,
VIR_DOMAIN_UNDEFINE_SNAPSHOTS_METADATA,
VIR_DOMAIN_UNDEFINE_NVRAM,
VIR_DOMAIN_XML_INACTIVE,
VIR_ERR_NO_DOMAIN,
VIR_ERR_NO_SUPPORT,
@ -861,62 +857,21 @@ def force_off(request, pk):
)
@serialize_compute_mutation
def destroy(request, pk):
if request.method in ["POST", "DELETE"]:
instance = get_instance(request.user, pk, perm_type="delete")
# an API request is a DRF request whose body is in .data; refused
# options stop nothing
options = DestroyOptionsSerializer(data=getattr(request, "data", request.POST))
get_instance(request.user, pk, perm_type="delete")
options = DestroyOptionsSerializer(data=request.POST)
options.is_valid(raise_exception=True)
with libvirt_instance_lock(instance):
with audit_vm(request, instance, "delete"):
proxy = instance.proxy
# the files to delete are known before the VM is stopped: a
# host that cannot be checked refuses the destruction
to_delete, shared = [], []
if options.validated_data["delete_disk"]:
to_delete, shared = proxy.split_disk_paths_by_use(
lambda own: utils.paths_used_on_other_computes(instance.compute, own))
# Paused and other active states must stop too, or the VM keeps
# running as a transient domain after the undefine.
if proxy.instance.isActive():
proxy.force_shutdown()
# Undefine before deleting any disk: if it fails, nothing is lost.
# Snapshot metadata goes with the domain; internal snapshot data
# stays in the disk images that are kept.
flags = VIR_DOMAIN_UNDEFINE_MANAGED_SAVE | VIR_DOMAIN_UNDEFINE_SNAPSHOTS_METADATA
if options.validated_data["delete_nvram"]:
flags |= VIR_DOMAIN_UNDEFINE_NVRAM
else:
flags |= VIR_DOMAIN_UNDEFINE_KEEP_NVRAM
proxy.delete(flags)
instance.delete()
# each disk has its own row: the VM is gone even if one is kept; a
# file that stays keeps the files below it, which it needs
kept = set()
for path in to_delete:
if path in kept:
messages.warning(request, _("Disk %(path)s was kept: a disk file that stays needs it") % {"path": path})
continue
try:
with audit_vm(request, instance, f"delete volume {path}"):
proxy.get_volume_by_path(path).delete(0)
except libvirtError as err:
messages.error(request, _("The VM is removed, but disk %(path)s was not deleted: %(err)s") % {
"path": path, "err": error_text(request, err)})
try:
while (path := proxy.backing_path(path)) and path not in kept:
kept.add(path)
except libvirtError:
# the files it needs are unknown: none of the rest goes
kept.update(to_delete)
for path in shared:
messages.warning(request, _("Disk %(path)s is used by another VM and was kept") % {"path": path})
return redirect(reverse("instances:index"))
try:
op = jobs.submit("instance.delete", request.user, {"instance": pk, **options.validated_data},
request.POST.get("submission_key", ""))
except claims.Busy as busy:
return _busy(request, pk, busy)
except (jobs.SubmissionConflict, libvirtError) as err:
messages.error(request, error_text(request, err))
return redirect(reverse("instances:destroy", args=[pk]))
jobs.remember_job(request, op)
return redirect(reverse("instances:instance", args=[pk]))
instance = get_instance(request.user, pk, perm_type="delete")
try:

Binary file not shown.

File diff suppressed because it is too large Load diff

Binary file not shown.

File diff suppressed because it is too large Load diff

View file

@ -976,6 +976,18 @@ class wvmConnect(object):
def get_volume_by_path(self, path):
return self.wvm.storageVolLookupByPath(path)
def backing_path(self, path):
"""The backing file of a volume; None if it has none or is in no pool.
Any other error is raised: a chain cut short would let a file a kept
one needs count as deletable."""
try:
vol = self.get_volume_by_path(path)
except libvirtError as err:
if err.get_error_code() != libvirt.VIR_ERR_NO_STORAGE_VOL:
raise
return None
return util.get_xml_path(vol.XMLDesc(0), "/volume/backingStore/path")
def remove_nvram(self, path, destination):
"""Deletes the NVRAM file of a VM that migrated from this host to
destination (a wvmConnect). Only a file in libvirt's per-host NVRAM

View file

@ -1833,18 +1833,6 @@ class wvmInstance(wvmConnect):
grew = True
return files[len(tops):] + memory
def backing_path(self, path):
"""The backing file of a volume; None if it has none or is in no pool.
Any other error is raised: a chain cut short would let a file a kept
one needs count as deletable."""
try:
vol = self.get_volume_by_path(path)
except libvirtError as err:
if err.get_error_code() != VIR_ERR_NO_STORAGE_VOL:
raise
return None
return util.get_xml_path(vol.XMLDesc(0), "/volume/backingStore/path")
def _snapshotCreateXML(self, xml, flag):
self.instance.snapshotCreateXML(xml, flag)