+
{% bs_icon 'download' %}
{% else %}
@@ -120,7 +121,7 @@
@@ -157,17 +158,17 @@
+ onclick="return confirm(this.dataset.confirm)" {% if busy or held %}disabled{% endif %}>
{% bs_icon 'download' %}
-
diff --git a/instances/tests/test_live_jobs.py b/instances/tests/test_live_jobs.py
new file mode 100644
index 00000000..1a16f3dc
--- /dev/null
+++ b/instances/tests/test_live_jobs.py
@@ -0,0 +1,92 @@
+"""Live checks for the instance jobs: the worker runs them against libvirt
+and recovery reads the same outcome back. Runs only with TEST_LIBVIRT_HOST
+set (see instances/tests/livetest.py)."""
+
+import os
+import time
+import unittest
+
+from django.contrib.auth import get_user_model
+from django.test import TransactionTestCase
+from vrtManager.connection import connection_manager
+from vrtManager.instance import wvmInstance
+
+from computes.models import Compute
+from instances.jobs import DeleteExternalSnapshot
+from instances.models import Instance
+from operations import jobs
+from operations.models import Operation
+from operations.tests import fakes
+from operations.worker import Worker
+
+from . import livetest
+
+P = livetest.PREFIX
+
+
+def wait_for(predicate, seconds):
+ end = time.monotonic() + seconds
+ while time.monotonic() < end:
+ if predicate():
+ return True
+ time.sleep(0.5)
+ return False
+
+
+class LiveJobsTestCase(TransactionTestCase):
+ @classmethod
+ def setUpClass(cls):
+ if not livetest.enabled():
+ raise unittest.SkipTest("Set TEST_LIBVIRT_HOST to run the live libvirt tests")
+ cls.args = (
+ os.environ["TEST_LIBVIRT_HOST"],
+ os.environ.get("TEST_LIBVIRT_LOGIN", ""),
+ os.environ.get("TEST_LIBVIRT_PASSWORD", ""),
+ int(os.environ.get("TEST_LIBVIRT_TYPE", 4)),
+ )
+ cls.conn = connection_manager.get_connection(*cls.args)
+ livetest.cleanup(cls.conn)
+ livetest.ensure_pool(cls.conn)
+ cls.inventory = livetest.inventory(cls.conn)
+ super().setUpClass()
+
+ @classmethod
+ def tearDownClass(cls):
+ try:
+ livetest.remove_pool(cls.conn)
+ finally:
+ super().tearDownClass()
+ if livetest.inventory(cls.conn) != cls.inventory:
+ raise AssertionError("Live tests changed objects outside the test namespace")
+
+ def setUp(self):
+ host, login, password, conn_type = self.args
+ self.compute = Compute.objects.create(name=P + "compute", hostname=host, login=login, password=password,
+ type=conn_type)
+ self.admin = get_user_model().objects.create_superuser("job_live_admin", "a@example.com", "pw")
+
+ def tearDown(self):
+ livetest.cleanup(self.conn)
+
+ def vm(self, name, state="stopped"):
+ base = livetest.create_volume(self.conn, P + name)
+ dom = livetest.define_vm(self.conn, P + name, [base])
+ if state == "running":
+ dom.create()
+ return dom, wvmInstance(*self.args, None, uuid=dom.UUIDString())
+
+ def test_the_worker_deletes_a_snapshot_and_recovery_agrees(self):
+ dom, vm = self.vm("job-del", "running")
+ vm.create_external_snapshot("one")
+ instance = Instance.objects.create(compute=self.compute, name=dom.name(), uuid=dom.UUIDString())
+ op = jobs.submit("snapshot.delete", self.admin, {"instance": instance.pk, "name": "one"}, "key-live0001")
+ worker = Worker(concurrency=1, tick=0.1, heartbeat_every=1, progress_every=1, exit_on_lost_lease=False)
+ thread = fakes.start_worker(worker)
+ try:
+ self.assertTrue(wait_for(lambda: Operation.objects.get(pk=op.pk).state == Operation.SUCCEEDED, 180))
+ finally:
+ fakes.stop_worker(worker, thread)
+ self.assertEqual(dom.snapshotNum(0), 0)
+ Operation.objects.filter(pk=op.pk).update(state=Operation.RUNNING)
+ op.refresh_from_db()
+ self.assertEqual(DeleteExternalSnapshot().reconcile(op).state, Operation.SUCCEEDED)
diff --git a/instances/tests/test_live_snapshots.py b/instances/tests/test_live_snapshots.py
index b78c9ef3..d3795872 100644
--- a/instances/tests/test_live_snapshots.py
+++ b/instances/tests/test_live_snapshots.py
@@ -101,6 +101,11 @@ class LiveExternalSnapshotTestCase(SimpleTestCase):
self.assertEqual(etree.tostring(etree.fromstring(dom.XMLDesc(libvirt.VIR_DOMAIN_XML_INACTIVE)).find("os")), os_before)
self.assertEqual(dom.snapshotNum(0), 0)
+ def test_a_shut_off_vm_without_a_merge_has_no_job(self):
+ # libvirt reports a shut-off VM's merge as an offline block job; none runs here
+ _, vm = self.vm("snap-nojob")
+ self.assertEqual(vm.get_snapshot_jobs(), [])
+
def test_back_to_back_reverts_keep_a_running_vm_running(self):
# Without the retry, the second revert within one second stopped the VM.
dom, vm = self.vm("snap-collide", "running")
diff --git a/instances/tests/test_snapshot_delete_job.py b/instances/tests/test_snapshot_delete_job.py
new file mode 100644
index 00000000..90a92d9c
--- /dev/null
+++ b/instances/tests/test_snapshot_delete_job.py
@@ -0,0 +1,212 @@
+"""External snapshot delete is a job: the request queues it; the worker
+deletes through libvirt; recovery follows the reconcile table."""
+
+from datetime import timedelta
+from types import SimpleNamespace
+from unittest.mock import MagicMock, patch
+
+from django.contrib.auth import get_user_model
+from django.test import TransactionTestCase
+from django.urls import reverse
+from django.utils import timezone
+from libvirt import libvirtError
+
+from computes.models import Compute
+from instances.jobs import DeleteExternalSnapshot
+from instances.models import Instance
+from logs.models import Logs
+from operations import jobs
+from operations.jobs import Context, Outcome
+from operations.models import Claim, Operation, acquire
+from operations.worker import housekeeping
+from vrtManager.instance import wvmInstance
+
+
+def proxy(listed=("one",), jobs=(), before=None, left=()):
+ p = MagicMock()
+ p.instance.snapshotListNames.return_value = list(listed)
+ p.get_snapshot_jobs.return_value = list(jobs)
+ p.snapshot_files_state.return_value = before or {"disks": {}, "own": [], "points": []}
+ p.delete_external_snapshot.return_value = list(left)
+ p.left_behind_since.return_value = list(left)
+ return p
+
+
+class SnapshotDeleteJobTestCase(TransactionTestCase):
+ def setUp(self):
+ compute = Compute.objects.create(name="job-compute", hostname="192.0.2.10", login="root", password="", type=1)
+ self.instance = Instance.objects.create(compute=compute, name="job-vm",
+ uuid="11111111-2222-3333-4444-555555555555")
+ self.admin = get_user_model().objects.create_superuser("job_admin", "a@example.com", "pw")
+ self.client.force_login(self.admin)
+ patcher = patch("instances.jobs.wvmInstance", return_value=proxy())
+ self.wvm = patcher.start()
+ self.addCleanup(patcher.stop)
+
+ def post(self, key="key-00000001", name="one"):
+ return self.client.post(
+ reverse("instances:delete_external_snapshot", args=[self.instance.pk]),
+ {"name": name, "submission_key": key},
+ HTTP_REFERER=reverse("instances:instance", args=[self.instance.pk]),
+ )
+
+ def test_the_request_queues_one_job_holding_the_vm(self):
+ self.post()
+ self.post()
+ op = Operation.objects.get()
+ self.assertEqual((op.kind, op.state, op.params),
+ ("snapshot.delete", Operation.QUEUED, {"instance": self.instance.pk, "name": "one"}))
+ self.assertEqual(set(Claim.objects.values_list("resource", "mode")),
+ {(f"compute:{self.instance.compute_id}", "shared"),
+ (f"instance:{self.instance.uuid}", "exclusive")})
+
+ def test_a_retry_after_the_job_ran_returns_it_without_libvirt(self):
+ self.post()
+ self.wvm.return_value = proxy(listed=()) # the snapshot is gone now
+ self.post()
+ self.assertEqual(Operation.objects.count(), 1)
+
+ def test_a_second_delete_is_refused_with_its_reason(self):
+ self.post(key="key-00000001")
+ response = self.post(key="key-00000002")
+ self.assertEqual(Operation.objects.count(), 1)
+ self.assertTrue(any("Delete snapshot is queued" in str(m) for m in response.wsgi_request._messages))
+
+ def test_a_long_name_is_kept_whole(self):
+ # its truncated prefix could name another snapshot
+ name = "s" * 100
+ self.assertEqual(DeleteExternalSnapshot().normalize({"instance": "1", "name": name})["name"], name)
+
+ def submitted(self):
+ return jobs.submit("snapshot.delete", self.admin, {"instance": self.instance.pk, "name": "one"},
+ "key-00000009")
+
+ def test_run_records_the_files_before_the_delete(self):
+ op = self.submitted()
+ 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()
+ self.wvm.return_value = proxy(before={"disks": {"vda": ["/p/a"]}, "own": ["/p/a.one"], "points": []},
+ left=["/p/a.one"])
+ ctx = Context(op, SimpleNamespace(identity="wvc-test:1:a"))
+ kind = DeleteExternalSnapshot()
+ kind.precheck(op, ctx)
+ result = kind.run(op, ctx)
+ op.refresh_from_db()
+ self.wvm.return_value.delete_external_snapshot.assert_called_once_with("one")
+ self.assertEqual(op.intent["before"]["own"], ["/p/a.one"])
+ self.assertEqual(result, {"left_behind": ["/p/a.one"]})
+ jobs.finish(op, Outcome(Operation.SUCCEEDED, result=result))
+ self.assertTrue(Logs.objects.filter(message="Delete external snapshot: one").exists())
+
+ def logged(self):
+ return list(Logs.objects.values_list("user", "host", "instance", "message"))
+
+ def test_every_end_is_logged_with_its_state(self):
+ for state in (Operation.FAILED, Operation.UNCERTAIN):
+ Logs.objects.all().delete()
+ Operation.objects.all().delete()
+ op = self.submitted()
+ jobs.finish(op, Outcome(state, error="x"))
+ self.assertEqual(self.logged(), [("job_admin", "job-compute", "job-vm",
+ f"Delete external snapshot: one ({state})")])
+ Logs.objects.all().delete()
+ Operation.objects.all().delete()
+ jobs.cancel(self.submitted().pk, self.admin)
+ self.assertEqual(self.logged(), [("job_admin", "job-compute", "job-vm", "Delete external snapshot: one (cancelled)")])
+
+ def test_a_compute_name_with_a_slash_is_logged_whole(self):
+ Compute.objects.filter(pk=self.instance.compute_id).update(name="region/job-compute")
+ op = self.submitted()
+ jobs.finish(op, Outcome(Operation.SUCCEEDED))
+ self.assertEqual(self.logged(), [("job_admin", "region/job-compute", "job-vm", "Delete external snapshot: one")])
+
+ def test_a_resolved_job_is_logged_on_its_vm(self):
+ op = self.submitted()
+ jobs.finish(op, Outcome(Operation.UNCERTAIN, error="x"))
+ Logs.objects.all().delete()
+ jobs.resolve(op.pk, self.admin, Operation.FAILED, "disks checked")
+ self.assertEqual(self.logged(), [("job_admin", "job-compute", "job-vm",
+ f"Job {op.pk} resolved as failed: disks checked")])
+
+ def test_retention_keeps_the_log(self):
+ op = self.submitted()
+ jobs.finish(op, Outcome(Operation.FAILED, error="x"))
+ Operation.objects.filter(pk=op.pk).update(finished_at=timezone.now() - timedelta(days=400))
+ housekeeping()
+ self.assertFalse(Operation.objects.filter(pk=op.pk).exists())
+ self.assertEqual([row[3] for row in self.logged()], ["Delete external snapshot: one (failed)"])
+
+ def test_precheck_refuses_a_moved_vm_and_an_inactive_user(self):
+ op = self.submitted()
+ other = Compute.objects.create(name="other", hostname="192.0.2.11", login="root", password="", type=1)
+ Instance.objects.filter(pk=self.instance.pk).update(compute=other)
+ with self.assertRaisesRegex(Exception, "moved"):
+ DeleteExternalSnapshot().precheck(op, Context(op, worker=None))
+ Instance.objects.filter(pk=self.instance.pk).update(compute=self.instance.compute)
+ get_user_model().objects.filter(pk=self.admin.pk).update(is_active=False)
+ op = Operation.objects.select_related("user").get(pk=op.pk)
+ with self.assertRaises(Exception):
+ DeleteExternalSnapshot().precheck(op, Context(op, worker=None))
+
+ def reconcile(self, **kw):
+ Operation.objects.all().delete()
+ self.wvm.return_value = proxy() # the snapshot exists when the job is submitted
+ op = self.submitted()
+ Operation.objects.filter(pk=op.pk).update(state=Operation.RUNNING, effects_started=True,
+ intent={"compute": self.instance.compute_id,
+ "before": {"disks": {}, "own": [], "points": []}})
+ op.refresh_from_db()
+ self.wvm.return_value = proxy(**kw)
+ return DeleteExternalSnapshot().reconcile(op)
+
+ def test_reconcile_table(self):
+ merging = [{"dev": "vda", "cur": 1, "end": 2}]
+ self.assertEqual(self.reconcile(listed=()).state, Operation.SUCCEEDED)
+ self.assertEqual(self.reconcile(jobs=merging).state, Operation.RUNNING)
+ self.assertEqual(self.reconcile(listed=(), jobs=merging).state, Operation.RUNNING)
+ interrupted = self.reconcile()
+ self.assertEqual(interrupted.state, Operation.UNCERTAIN)
+ self.assertIn("It cannot be told whether the disks changed; check the VM before deleting the snapshot again",
+ interrupted.error)
+
+ def test_reconcile_reports_a_merge_that_stays_at_its_end_as_stalled(self):
+ at_end = self.reconcile(jobs=[{"dev": "vda", "cur": 2, "end": 2}, {"dev": "vdb", "cur": 5, "end": 5}])
+ self.assertEqual((at_end.state, at_end.stalled), (Operation.RUNNING, True))
+ copying = self.reconcile(jobs=[{"dev": "vda", "cur": 2, "end": 2}, {"dev": "vdb", "cur": 1, "end": 5}])
+ self.assertEqual((copying.state, copying.stalled), (Operation.RUNNING, False))
+
+ def test_reconcile_reports_the_merge_progress(self):
+ merging = self.reconcile(jobs=[{"dev": "vda", "cur": 2, "end": 2}, {"dev": "vdb", "cur": 1, "end": 5}])
+ self.assertEqual(merging.progress, (3, 7))
+
+ def test_reconcile_watches_an_offline_merge_of_a_shut_off_vm(self):
+ # libvirt reports the merge of a shut-off VM's snapshot as an offline block job
+ p = proxy()
+ p._XMLDesc.return_value = (""
+ " ")
+ p._disk_sources = wvmInstance._disk_sources
+ p.instance.isActive.return_value = False
+ p.instance.blockJobInfo.side_effect = lambda dev, flags: {"cur": 1, "end": 2, "type": 4}
+ p.get_snapshot_jobs = lambda: wvmInstance.get_snapshot_jobs(p)
+ Operation.objects.all().delete()
+ self.wvm.return_value = proxy()
+ op = self.submitted()
+ Operation.objects.filter(pk=op.pk).update(state=Operation.RUNNING, effects_started=True,
+ intent={"compute": self.instance.compute_id,
+ "before": {"disks": {}, "own": [], "points": []}})
+ op.refresh_from_db()
+ self.wvm.return_value = p
+ self.assertEqual(DeleteExternalSnapshot().reconcile(op).state, Operation.RUNNING)
+
+ def test_reconcile_of_a_vm_gone_from_its_compute(self):
+ err = libvirtError("Domain not found")
+ err.err = (42,) # VIR_ERR_NO_DOMAIN
+ Operation.objects.all().delete()
+ op = self.submitted()
+ Operation.objects.filter(pk=op.pk).update(state=Operation.RUNNING, intent={"compute": self.instance.compute_id})
+ op.refresh_from_db()
+ p = proxy()
+ p.instance.snapshotListNames.side_effect = err
+ self.wvm.return_value = p
+ self.assertEqual(DeleteExternalSnapshot().reconcile(op).state, Operation.UNCERTAIN)
diff --git a/instances/tests/test_snapshot_views.py b/instances/tests/test_snapshot_views.py
index e6d1a21b..0001e6b9 100644
--- a/instances/tests/test_snapshot_views.py
+++ b/instances/tests/test_snapshot_views.py
@@ -1,5 +1,6 @@
-"""The snapshot views call the backend once and show its refusals as
-messages; they no longer start, pause or stop the VM themselves."""
+"""The snapshot views call the backend once (an external snapshot delete
+queues a job instead) and show its refusals as messages; they no longer
+start, pause or stop the VM themselves."""
from datetime import datetime
from unittest.mock import PropertyMock, patch
@@ -15,6 +16,7 @@ from vrtManager import util
from appsettings.models import AppSettings
from computes.models import Compute
from instances.models import Instance
+from operations.models import Claim, Operation
class SnapshotViewsTestCase(TestCase):
@@ -32,7 +34,6 @@ class SnapshotViewsTestCase(TestCase):
method = {
"create_external_snapshot": proxy.create_external_snapshot,
"revert_external_snapshot": proxy.revert_external_snapshot,
- "delete_external_snapshot": proxy.delete_external_snapshot,
"revert_snapshot": proxy.snapshot_revert,
"delete_snapshot": proxy.snapshot_delete,
"snapshot": proxy.create_snapshot,
@@ -66,26 +67,48 @@ class SnapshotViewsTestCase(TestCase):
_, method, _ = self.post("create_external_snapshot", {"name": "a"})
method.assert_called_once_with("a", "", False, False, **expected)
- def test_revert_and_delete_call_the_backend_once_and_leave_power_alone(self):
- for view in ("revert_external_snapshot", "delete_external_snapshot"):
- with self.subTest(view):
- proxy, method, _ = self.post(view, {"name": "a"})
- method.assert_called_once_with("a")
- for power in ("start", "force_shutdown", "shutdown"):
- getattr(proxy, power).assert_not_called()
+ def test_revert_calls_the_backend_once_and_leaves_power_alone(self):
+ proxy, method, _ = self.post("revert_external_snapshot", {"name": "a"})
+ method.assert_called_once_with("a")
+ for power in ("start", "force_shutdown", "shutdown"):
+ getattr(proxy, power).assert_not_called()
def test_files_left_behind_are_named_in_a_warning(self):
- for view in ("revert_external_snapshot", "delete_external_snapshot"):
- with self.subTest(view):
- _, _, messages = self.post(view, {"name": "a"}, left=["/pool/a.one", "/pool/vm.one.mem"])
- warning = [m for m in messages if "/pool/a.one, /pool/vm.one.mem" in m]
- self.assertEqual(len(warning), 1, messages)
- self.assertIn("qemu user", warning[0])
- _, _, messages = self.post(view, {"name": "a"})
- self.assertFalse([m for m in messages if "not removed" in m], messages)
+ _, _, messages = self.post("revert_external_snapshot", {"name": "a"}, left=["/pool/a.one", "/pool/vm.one.mem"])
+ warning = [m for m in messages if "/pool/a.one, /pool/vm.one.mem" in m]
+ self.assertEqual(len(warning), 1, messages)
+ self.assertIn("qemu user", warning[0])
+ _, _, messages = self.post("revert_external_snapshot", {"name": "a"})
+ self.assertFalse([m for m in messages if "not removed" in m], messages)
+
+ def delete(self, listed):
+ with patch("instances.jobs.wvmInstance") as mock_wvm:
+ proxy = mock_wvm.return_value
+ proxy.instance.snapshotListNames.return_value = list(listed)
+ self.client.cookies.pop("messages", None)
+ response = self.client.post(
+ reverse("instances:delete_external_snapshot", args=[self.instance.id]),
+ {"name": "a", "submission_key": "key-00000001"},
+ HTTP_REFERER=reverse("instances:instance", args=[self.instance.id]),
+ )
+ self.assertEqual(response.status_code, 302)
+ return proxy, [str(m) for m in get_messages(response.wsgi_request)]
+
+ def test_delete_queues_a_job_and_leaves_libvirt_to_the_worker(self):
+ proxy, messages = self.delete(listed=["a"])
+ op = Operation.objects.get()
+ self.assertEqual((op.kind, op.state, op.params),
+ ("snapshot.delete", Operation.QUEUED, {"instance": self.instance.id, "name": "a"}))
+ self.assertTrue(any("being deleted" in m for m in messages), messages)
+ proxy.delete_external_snapshot.assert_not_called()
+
+ def test_a_refused_delete_becomes_a_message_and_queues_nothing(self):
+ _, messages = self.delete(listed=[])
+ self.assertTrue(any("has no external snapshot named a" in m for m in messages), messages)
+ self.assertFalse(Operation.objects.exists())
def test_refusals_become_messages(self):
- for view in ("create_external_snapshot", "revert_external_snapshot", "delete_external_snapshot",
+ for view in ("create_external_snapshot", "revert_external_snapshot",
"revert_snapshot", "delete_snapshot", "snapshot"):
with self.subTest(view):
_, _, messages = self.post(view, {"name": "a"}, error="refused for a reason")
@@ -174,6 +197,40 @@ class SnapshotTabTestCase(TestCase):
self.assertTrue(all(b.get("disabled") is not None
for b in doc.xpath("//div[@id='snapshots']//button[@type='submit']")))
+ def hold(self, state):
+ key = f"instance:{self.instance.uuid}"
+ op = Operation.objects.create(kind="snapshot.delete", username="snap_admin", submission_key="k" * 12,
+ resource=key, resource_label="snap-vm", state=state)
+ return Claim.objects.create(operation=op, resource=key, mode=Claim.EXCLUSIVE)
+
+ def test_an_active_job_disables_actions_before_libvirt_has_a_block_job(self):
+ # the button each case leaves enabled, so only the job can disable it
+ cases = (("none", {}, ("takesnapshot", "takeextsnapshot")),
+ ("internal only", {"internal": [self.snap("i")]}, ("takesnapshot", "managesnapshot")),
+ ("external only", {"external": [self.snap("e")]}, ("takeextsnapshot", "managesnapshot")))
+ buttons = "//div[@id='snapshots']//button[@type='submit']"
+
+ def enabled(doc, panes):
+ return [bool(doc.xpath(f"//div[@id='{pane}']//button[@type='submit' and not(@disabled)]"))
+ for pane in panes]
+
+ for name, lists, panes in cases:
+ with self.subTest(name):
+ doc = self.page(**lists, status=5)
+ self.assertTrue(all(enabled(doc, panes)), "enabled without a job")
+ claim = self.hold(Operation.QUEUED)
+ doc = self.page(**lists, status=5)
+ self.assertFalse(doc.xpath(buttons + "[not(@disabled)]"), "disabled while the job holds the VM")
+ self.assertNotIn("A disk job is running", doc.text_content())
+ claim.operation.delete()
+ self.assertTrue(all(enabled(self.page(**lists, status=5), panes)), "enabled after the job ends")
+
+ def test_an_uncertain_job_keeps_the_buttons_disabled(self):
+ self.hold(Operation.UNCERTAIN)
+ doc = self.page(internal=[self.snap("i")], status=5)
+ self.assertTrue(doc.xpath("//button[@name='revert_snapshot']"))
+ self.assertFalse(doc.xpath("//div[@id='snapshots']//button[@type='submit'][not(@disabled)]"))
+
def test_lists_are_separate(self):
doc = self.page(internal=[self.snap("i")], external=[self.snap("e")])
self.assertEqual(doc.xpath("//table[@id='internal-snapshots']//strong/text()"), ["i"])
diff --git a/instances/views.py b/instances/views.py
index 97c8762c..b71a133a 100755
--- a/instances/views.py
+++ b/instances/views.py
@@ -38,7 +38,7 @@ from libvirt import (VIR_DOMAIN_UNDEFINE_KEEP_NVRAM,
VIR_DOMAIN_UNDEFINE_NVRAM,
libvirtError)
from logs.views import addlogmsg
-from operations import claims
+from operations import claims, jobs
from operations.utils import busy_message, summary
from webvirtcloud.middleware import error_text
from vrtManager import util
@@ -1459,20 +1459,23 @@ def revert_external_snapshot(request, pk):
@require_POST
-@serialize_instance_mutation
def delete_external_snapshot(request, pk):
- instance = get_instance(request.user, pk, perm_type="change")
+ get_instance(request.user, pk, perm_type="change")
if not request.user.has_perm("instances.snapshot_instances"):
raise PermissionDenied
- name = request.POST.get("name", "")
try:
- left = instance.proxy.delete_external_snapshot(name)
+ op = jobs.submit("snapshot.delete", request.user,
+ {"instance": pk, "name": request.POST.get("name", "")},
+ request.POST.get("submission_key", ""))
+ except claims.Busy as busy:
+ messages.error(request, busy_message(busy, request.user))
+ except jobs.SubmissionConflict as err:
+ messages.error(request, str(err))
except libvirtError as err:
messages.error(request, _("Snapshot was not deleted: %(err)s") % {"err": error_text(request, err)})
else:
- _warn_left_behind(request, left)
- msg = _("Delete external snapshot: %(snap)s") % {"snap": name}
- addlogmsg(request.user.username, instance.compute.name, instance.name, msg)
+ messages.info(request, _("The snapshot is being deleted; follow it in the job panel."))
+ jobs.remember_job(request, op)
return _back(request, pk, "managesnapshot")
diff --git a/operations/jobs.py b/operations/jobs.py
index 52ce5c35..4b529f70 100644
--- a/operations/jobs.py
+++ b/operations/jobs.py
@@ -248,3 +248,13 @@ def resolve(op_id, user, outcome_state, note):
_log(user.username, host, instance, f"Job {op.pk} resolved as {outcome_state}: {note}")
op.refresh_from_db()
return op
+
+
+def remember_job(request, op):
+ """The resource page's panel shows this job even if it ends before the first poll."""
+ ids = request.session.get("operations_shown", [])
+ request.session["operations_shown"] = [*ids, str(op.pk)][-20:]
+
+
+def take_remembered(request):
+ return request.session.pop("operations_shown", [])
diff --git a/operations/tests/test_service.py b/operations/tests/test_service.py
index 6d6d9539..cacad79b 100644
--- a/operations/tests/test_service.py
+++ b/operations/tests/test_service.py
@@ -4,7 +4,7 @@ from unittest.mock import patch
from django.contrib.auth import get_user_model
from django.core.exceptions import PermissionDenied
-from django.test import TransactionTestCase
+from django.test import SimpleTestCase, TransactionTestCase
from django.utils import timezone
from vrtManager.util import OperationError
@@ -192,3 +192,13 @@ class CancelFinishResolveTestCase(TransactionTestCase):
self.assertFalse(Claim.objects.exists())
entry = Logs.objects.get(message__icontains="resolved")
self.assertEqual((entry.user, entry.host, entry.instance), (self.admin.username, "c", "vm"))
+
+
+class RememberJobTestCase(SimpleTestCase):
+ def test_the_last_twenty_jobs_are_remembered_until_taken(self):
+ request = SimpleNamespace(session={})
+ for n in range(25):
+ jobs.remember_job(request, SimpleNamespace(pk=n))
+ self.assertEqual(request.session["operations_shown"], [str(n) for n in range(5, 25)])
+ self.assertEqual(jobs.take_remembered(request), [str(n) for n in range(5, 25)])
+ self.assertEqual(jobs.take_remembered(request), [])
diff --git a/vrtManager/instance.py b/vrtManager/instance.py
index 5473bec8..c7350bcf 100644
--- a/vrtManager/instance.py
+++ b/vrtManager/instance.py
@@ -38,6 +38,7 @@ try:
VIR_DOMAIN_SNAPSHOT_REVERT_PAUSED,
VIR_DOMAIN_SNAPSHOT_REVERT_RUNNING,
VIR_ERR_CONFIG_UNSUPPORTED,
+ VIR_ERR_OPERATION_INVALID,
VIR_DOMAIN_SNAPSHOT_LIST_INTERNAL,
VIR_DOMAIN_SNAPSHOT_LIST_EXTERNAL,
libvirtError,
@@ -1751,6 +1752,17 @@ class wvmInstance(wvmConnect):
points.update(self._disk_sources(saved).values())
return disks, own, points | own
+ def snapshot_files_state(self):
+ """_snapshot_files as plain lists, recorded before a snapshot change."""
+ disks, own, points = self._snapshot_files()
+ return {"disks": {dev: sorted(files) for dev, files in disks.items()},
+ "own": sorted(own), "points": sorted(points)}
+
+ def left_behind_since(self, state):
+ """_left_behind from a state recorded by snapshot_files_state."""
+ before = ({dev: set(files) for dev, files in state["disks"].items()}, set(state["own"]), set(state["points"]))
+ return self._left_behind(before)
+
def _refresh_pools_after_snapshot(self):
"""Refresh the pools once libvirt has changed the snapshot files. The
snapshot operation is done by then, so an error is only logged;
@@ -1844,12 +1856,19 @@ class wvmInstance(wvmConnect):
return snapshots
def get_snapshot_jobs(self):
- """Block jobs (snapshot merges) running on the VM's disks."""
- if not self.instance.isActive():
- return []
+ """Block jobs (snapshot merges) running on the VM's disks. libvirt
+ merges a shut-off VM's snapshot as an offline block job; a libvirt
+ without offline jobs refuses the query for an inactive VM, which
+ means no job."""
+ active = self.instance.isActive()
jobs = []
- for dev in self._disk_sources(self._XMLDesc(0)):
- info = self.instance.blockJobInfo(dev, 0)
+ for dev in self._disk_sources(self._XMLDesc(0 if active else PERSISTENT_XML)):
+ try:
+ info = self.instance.blockJobInfo(dev, 0)
+ except libvirtError as err:
+ if active or err.get_error_code() != VIR_ERR_OPERATION_INVALID:
+ raise
+ info = None
if info:
jobs.append({"dev": dev, "cur": info["cur"], "end": info["end"]})
return jobs
diff --git a/vrtManager/tests/test_external_snapshots.py b/vrtManager/tests/test_external_snapshots.py
index 227ec280..eebde260 100644
--- a/vrtManager/tests/test_external_snapshots.py
+++ b/vrtManager/tests/test_external_snapshots.py
@@ -2,6 +2,7 @@
checks preconditions and picks the flags. libvirt is mocked here; the live
cases are in instances/tests/test_live_snapshots.py."""
+import json
import unittest
from datetime import datetime
from unittest.mock import MagicMock, patch
@@ -167,10 +168,42 @@ class SnapshotJobsTestCase(unittest.TestCase):
p = proxy(jobs={"vda": {"cur": 5, "end": 10, "type": 3, "bandwidth": 0}})
self.assertEqual(p.get_snapshot_jobs(), [{"dev": "vda", "cur": 5, "end": 10}])
- def test_a_stopped_vm_has_no_jobs(self):
+ def test_a_stopped_vm_without_a_job_has_none(self):
+ self.assertEqual(proxy(status=5).get_snapshot_jobs(), [])
+
+ def test_an_offline_merge_of_a_stopped_vm_is_reported(self):
+ # libvirt deletes an external snapshot of a shut-off VM as an offline block job
+ p = proxy(status=5, jobs={"vdb": {"cur": 3, "end": 8, "type": 4, "bandwidth": 0}})
+ p._XMLDesc = lambda flags: DOMAIN if flags == PERSISTENT_XML else " "
+ self.assertEqual(p.get_snapshot_jobs(), [{"dev": "vdb", "cur": 3, "end": 8}])
+
+ def test_a_libvirt_without_offline_jobs_reports_none(self):
+ err = libvirtError("domain is not running")
+ err.err = (libvirt.VIR_ERR_OPERATION_INVALID,)
p = proxy(status=5)
+ p.instance.blockJobInfo.side_effect = err
self.assertEqual(p.get_snapshot_jobs(), [])
- p.instance.blockJobInfo.assert_not_called()
+
+ def test_another_error_of_a_stopped_vm_propagates(self):
+ err = libvirtError("cannot reach the host")
+ err.err = (libvirt.VIR_ERR_SYSTEM_ERROR,)
+ p = proxy(status=5)
+ p.instance.blockJobInfo.side_effect = err
+ with self.assertRaises(libvirtError):
+ p.get_snapshot_jobs()
+
+ def test_a_running_vm_does_not_swallow_operation_invalid(self):
+ err = libvirtError("operation invalid")
+ err.err = (libvirt.VIR_ERR_OPERATION_INVALID,)
+ p = proxy()
+ p.instance.blockJobInfo.side_effect = err
+ with self.assertRaises(libvirtError):
+ p.get_snapshot_jobs()
+
+ def test_a_running_vm_reads_the_live_disks(self):
+ p = proxy(jobs={"vdb": {"cur": 1, "end": 2}})
+ p._XMLDesc = lambda flags: DOMAIN if flags == 0 else " "
+ self.assertEqual(p.get_snapshot_jobs(), [{"dev": "vdb", "cur": 1, "end": 2}])
class SnapshotLookupTestCase(unittest.TestCase):
@@ -192,6 +225,10 @@ class SnapshotStateTestCase(unittest.TestCase):
with self.assertRaisesRegex(util.OperationError, "still running"):
proxy(jobs={"vda": {"cur": 1, "end": 2}})._check_snapshot_state()
+ def test_an_offline_merge_of_a_stopped_vm_is_refused(self):
+ with self.assertRaisesRegex(util.OperationError, "still running"):
+ proxy(status=5, jobs={"vda": {"cur": 1, "end": 2}})._check_snapshot_state()
+
def test_a_mixed_vm_passes_so_it_can_be_cleaned_up(self):
proxy(internal=[fake_snap("i")], external=[fake_snap("e")])._check_snapshot_state()
@@ -499,7 +536,7 @@ class LeftBehindTestCase(unittest.TestCase):
discarded file (on NFS it removes them as the qemu user), so the delete
and revert report the files nothing uses any more that still exist."""
- def run_delete(self, still_there):
+ def delete_fixture(self, still_there):
one = chain_snap("one", "/pool/a.one", memory="/pool/vm.one.mem")
two = chain_snap("two", "/pool/a.two")
p = proxy(external=[one, two])
@@ -515,7 +552,20 @@ class LeftBehindTestCase(unittest.TestCase):
return MagicMock()
p.get_volume_by_path = volume
- return p.delete_external_snapshot("one")
+ return p
+
+ def run_delete(self, still_there):
+ return self.delete_fixture(still_there).delete_external_snapshot("one")
+
+ def test_a_recorded_state_gives_the_same_files(self):
+ # the delete job records the state before the merge and compares later
+ still_there = {"/pool/a.one", "/pool/vm.one.mem", "/pool/a.two", "/pool/a.qcow2"}
+ p = self.delete_fixture(still_there)
+ direct = p._left_behind(p._snapshot_files())
+ p = self.delete_fixture(still_there)
+ state = json.loads(json.dumps(p.snapshot_files_state())) # as stored in the job
+ self.assertEqual(p.left_behind_since(state), direct)
+ self.assertEqual(direct, ["/pool/a.one", "/pool/vm.one.mem"])
def test_files_libvirt_could_not_remove_are_reported(self):
self.assertEqual(