mirror of
https://github.com/retspen/webvirtcloud
synced 2026-11-11 04:36:48 +00:00
fix(logs): a row says what happened: a recovered migration, a failed clone step, a device not there
- A migration whose libvirt call failed but which recovery finds moved (or cannot tell) gets a row of that end; only a failure already logged by migrate_instance is not logged twice. - A clone made with its resize or start failed, and a recovered migration whose autostart or NVRAM step failed, write a failed row of that step, as migrate_instance does for its later steps. - Setting the link state of an interface or the video model of a VM that has none is refused: it changed nothing and was logged as done. - The rows of a removed VM record (deleted or expired) name its host, VM and UUID, so the VM's history has them.
This commit is contained in:
catborise
parent
470cc9860e
commit
528b4492f6
14 changed files with 390 additions and 304 deletions
|
|
@ -225,6 +225,8 @@ def _finish_clone(op, clone, warning):
|
|||
proxy.start()
|
||||
except libvirtError as err:
|
||||
notes += say(_("The clone is made, but not as asked: %(error)s"), error=err)
|
||||
write(op.username, c.name, op.intent["name"], f"VM {op.intent['name']}: resize or start the clone: "
|
||||
f"{safe_text(err)}", False, uuid=op.intent["uuid"])
|
||||
return {"warning": notes} if notes else {}
|
||||
|
||||
|
||||
|
|
@ -966,17 +968,25 @@ def _moved(op, dest):
|
|||
stopped before it: autostart is per host, and the source keeps the
|
||||
NVRAM file. The job's result, with a warning for a step that failed."""
|
||||
notes = []
|
||||
source, _sep, name = op.resource_label.rpartition("/")
|
||||
target = Compute.objects.get(pk=op.params["target"]).name
|
||||
|
||||
def failed(action, err): # as migrate_instance logs its later steps
|
||||
write(op.username, source, name, f"VM {name}: {action}: {safe_text(err)}", False, uuid=op.intent["uuid"])
|
||||
|
||||
if op.intent.get("autostart"):
|
||||
try:
|
||||
dest.setAutostart(1)
|
||||
except libvirtError as err:
|
||||
notes += say(_("The VM migrated, but autostart could not be set on the destination: %(error)s"),
|
||||
error=err)
|
||||
failed(f"restore autostart on {target}", err)
|
||||
if nvram := op.intent.get("nvram"):
|
||||
try:
|
||||
_connect(op.intent["compute"]).remove_nvram(nvram, _connect(op.params["target"]))
|
||||
except libvirtError as err:
|
||||
notes += say(_("The VM migrated, but its NVRAM file stays on the source: %(error)s"), error=err)
|
||||
failed(f"remove NVRAM file {nvram} on the source", err)
|
||||
return {"warning": notes} if notes else {}
|
||||
|
||||
|
||||
|
|
@ -1034,8 +1044,13 @@ class MigrateInstance(Kind):
|
|||
try:
|
||||
utils.migrate_instance(Compute.objects.get(pk=p["target"]), instance, op.user,
|
||||
disk_pools=p["disk_pools"],
|
||||
on_moved=lambda: ctx.record_intent(outcome_logged=True),
|
||||
**{k: p[k] for k in MIGRATE_OPTIONS})
|
||||
except libvirtError as err:
|
||||
except Exception as err:
|
||||
# migrate_instance wrote the error's Logs row; recovery may still find the VM moved
|
||||
ctx.record_intent(failure_logged=True)
|
||||
if not isinstance(err, libvirtError):
|
||||
raise
|
||||
op.refresh_from_db(fields=["cancel_requested"])
|
||||
if op.cancel_requested:
|
||||
raise Cancelled from None
|
||||
|
|
@ -1045,15 +1060,13 @@ class MigrateInstance(Kind):
|
|||
if isinstance(err, util.OperationError):
|
||||
raise
|
||||
raise util.OperationError(str(err)) from err
|
||||
finally:
|
||||
# migrate_instance wrote the outcome's Logs row (log_entry)
|
||||
ctx.record_intent(outcome_logged=True)
|
||||
return {}
|
||||
|
||||
def log_entry(self, op, state):
|
||||
"""A row for an end migrate_instance did not log: refused before it
|
||||
ran, cancelled while queued, or ended by recovery."""
|
||||
if op.intent.get("outcome_logged"):
|
||||
ran, cancelled while queued, or ended by recovery other than the
|
||||
failure migrate_instance logged."""
|
||||
if op.intent.get("outcome_logged") or (op.intent.get("failure_logged") and state == Operation.FAILED):
|
||||
return None
|
||||
target = Compute.objects.filter(pk=op.params["target"]).values_list("name", flat=True).first()
|
||||
return _log(op, state, f"migrate to {target}")
|
||||
|
|
|
|||
|
|
@ -17,6 +17,7 @@ from appsettings.settings import get_settings
|
|||
from computes.models import Compute
|
||||
from instances.jobs import CloneInstance
|
||||
from instances.models import Instance
|
||||
from logs.models import Logs
|
||||
from operations.utils import words
|
||||
from operations.jobs import Cancelled, Context
|
||||
from vrtManager.util import ConnectionFailed, OperationError
|
||||
|
|
@ -148,6 +149,9 @@ class CloneJobTestCase(TransactionTestCase):
|
|||
op, result = self.clone_with(**{"clone-start": "true"})
|
||||
self.assertTrue(Instance.objects.filter(uuid=op.intent["uuid"]).exists())
|
||||
self.assertIn("The clone is made, but not as asked", words(result["warning"]))
|
||||
# the job succeeded: Logs has the step that failed too
|
||||
self.assertTrue(Logs.objects.filter(instance_uuid=op.intent["uuid"], success=False,
|
||||
message="VM copy: resize or start the clone: no memory").exists())
|
||||
|
||||
def queued(self):
|
||||
self.client.post(reverse("instances:clone", args=[self.source.pk]),
|
||||
|
|
|
|||
|
|
@ -11,6 +11,7 @@ from appsettings.settings import get_settings
|
|||
from computes.models import Compute
|
||||
from instances.jobs import MigrateInstance
|
||||
from instances.models import Instance
|
||||
from logs.models import Logs
|
||||
from libvirt import VIR_DOMAIN_XML_INACTIVE, VIR_ERR_SYSTEM_ERROR, VIR_FROM_RPC, libvirtError
|
||||
from operations.jobs import Cancelled, Context, get_kind
|
||||
from operations.models import Claim, Operation, acquire
|
||||
|
|
@ -124,6 +125,8 @@ class MigrateJobTestCase(TransactionTestCase):
|
|||
outcome = MigrateInstance().reconcile(op)
|
||||
self.assertEqual(outcome.state, Operation.SUCCEEDED)
|
||||
self.assertIn("autostart could not be set", outcome.result["warning"][0]["text"])
|
||||
self.assertTrue(Logs.objects.filter(instance_uuid=UUID, success=False,
|
||||
message="VM mig-vm: restore autostart on mig-dst: refused").exists())
|
||||
|
||||
def test_a_copy_claims_its_volume_on_both_computes(self):
|
||||
Instance.proxy.get_disk_devices.return_value = [
|
||||
|
|
@ -261,8 +264,6 @@ class MigrateJobTestCase(TransactionTestCase):
|
|||
proxy.return_value.instance.abortJob.assert_not_called()
|
||||
|
||||
def test_each_end_of_a_migration_leaves_one_logs_row(self):
|
||||
from logs.models import Logs
|
||||
|
||||
op = self.queue(live_migrate="true")
|
||||
ctx = self.start(op)
|
||||
get_kind(op.kind).run(op, ctx) # migrate_instance writes its row
|
||||
|
|
@ -275,6 +276,29 @@ class MigrateJobTestCase(TransactionTestCase):
|
|||
self.assertEqual(MigrateInstance().log_entry(queued, Operation.CANCELLED),
|
||||
("mig-src", "mig-vm", UUID, "VM mig-vm: migrate to mig-dst (cancelled)"))
|
||||
|
||||
def test_a_failed_migration_recovery_finds_moved_has_a_row_of_that_end(self):
|
||||
op = self.queue(live_migrate="true")
|
||||
ctx = self.start(op)
|
||||
self.dest_conn.moveto.side_effect = libvirtError("connection reset while migrating")
|
||||
with self.assertRaises(Exception):
|
||||
get_kind(op.kind).run(op, ctx) # migrate_instance writes the failed row
|
||||
op.refresh_from_db()
|
||||
self.assertIsNone(MigrateInstance().log_entry(op, Operation.FAILED)) # that row is the end
|
||||
for state, message in ((Operation.SUCCEEDED, "VM mig-vm: migrate to mig-dst"),
|
||||
(Operation.UNCERTAIN, "VM mig-vm: migrate to mig-dst (uncertain)")):
|
||||
with self.subTest(state):
|
||||
self.assertEqual(MigrateInstance().log_entry(op, state)[3], message)
|
||||
|
||||
def test_a_migration_moved_then_failed_to_record_has_its_row_once(self):
|
||||
op = self.queue(live_migrate="true")
|
||||
ctx = self.start(op)
|
||||
with patch("instances.utils.record_migration", side_effect=RuntimeError("database locked")), \
|
||||
self.assertRaises(RuntimeError):
|
||||
get_kind(op.kind).run(op, ctx) # the migration's row, then the failed record's
|
||||
op.refresh_from_db()
|
||||
self.assertIsNone(MigrateInstance().log_entry(op, Operation.SUCCEEDED)) # recovery finds it moved
|
||||
self.assertEqual(Logs.objects.filter(instance_uuid=UUID, message__startswith="VM mig-vm: migrate (").count(), 1)
|
||||
|
||||
def test_recording_a_migration_holds_the_ownership_lock(self):
|
||||
# a sync of a third compute must not move the owners at the same time
|
||||
from instances import utils
|
||||
|
|
|
|||
|
|
@ -214,13 +214,14 @@ def migrate_instance(
|
|||
compress=False,
|
||||
delete_source=False,
|
||||
disk_pools=None,
|
||||
on_moved=None,
|
||||
):
|
||||
"""Migrates instance to new_compute and logs the outcome (the mode
|
||||
used, or the error), then each later step that failed in a row of its
|
||||
own. disk_pools: {target: pool} of the disks copied into pools of
|
||||
new_compute (moveto's pools); their source files go with delete_source
|
||||
unless another VM or volume uses them, and are named in the log
|
||||
otherwise. Returns the mode."""
|
||||
own; on_moved is called once the outcome's row is written. disk_pools:
|
||||
{target: pool} of the disks copied into pools of new_compute (moveto's
|
||||
pools); their source files go with delete_source unless another VM or
|
||||
volume uses them, and are named in the log otherwise. Returns the mode."""
|
||||
source, target = instance.compute, migration_target(new_compute)
|
||||
|
||||
def log(action, error=None):
|
||||
|
|
@ -259,6 +260,8 @@ def migrate_instance(
|
|||
# The VM runs on new_compute now: what follows must not stop it
|
||||
# from being recorded there.
|
||||
log(f"migrate ({mode}) to {target}")
|
||||
if on_moved is not None: # its row is written: a later step that raises does not undo it
|
||||
on_moved()
|
||||
if autostart:
|
||||
conn_new = None
|
||||
try:
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue