From d2a637cf33cc9fd3b5d28854146caeb56f2b95b3 Mon Sep 17 00:00:00 2001 From: catborise Date: Sat, 7 Nov 2026 22:42:15 +0300 Subject: [PATCH] fix(migrate): recovery ends a cancelled migration by where the VM is A libvirt error after a cancel ended the job cancelled at once; a migration that finished but lost its last reply left the VM on the destination and its record on the source. The error goes to recovery, which ends the job cancelled when the VM stayed and succeeded when it moved. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01Kk4h2TcUiaWmU5MUszYSea --- instances/jobs.py | 6 ++---- instances/tests/test_migrate_job.py | 16 ++++++++++++---- 2 files changed, 14 insertions(+), 8 deletions(-) diff --git a/instances/jobs.py b/instances/jobs.py index 5d5639ed..db738bc7 100644 --- a/instances/jobs.py +++ b/instances/jobs.py @@ -1078,13 +1078,11 @@ class MigrateInstance(Kind): on_moved=lambda: ctx.record_intent(outcome_logged=True), **{k: p[k] for k in MIGRATE_OPTIONS}) except Exception as err: - # migrate_instance wrote the error's Logs row; recovery may still find the VM moved + # migrate_instance wrote the error's Logs row; recovery reads where the VM is, + # also after a cancel: a migration may end with an error and 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 # a superuser's job: libvirt's words, which a host the source cannot # reach for the migration would hide behind "cannot be reached"; # ours keep their text and values diff --git a/instances/tests/test_migrate_job.py b/instances/tests/test_migrate_job.py index f528fcd2..9e5d304f 100644 --- a/instances/tests/test_migrate_job.py +++ b/instances/tests/test_migrate_job.py @@ -13,9 +13,10 @@ 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.jobs import Context, get_kind from operations.models import Claim, Operation, acquire from operations.utils import public_error_text +from vrtManager.util import OperationError UUID = "31313131-4242-5353-6464-757575757575" @@ -197,15 +198,22 @@ class MigrateJobTestCase(TransactionTestCase): {"compute_id": alias.pk, "submission_key": "key-mig00002"}) self.assertFalse(Operation.objects.exists()) - def test_a_cancelled_migration_ends_cancelled_and_the_vm_stays(self): + def test_a_cancelled_migration_is_ended_by_where_recovery_finds_the_vm(self): + """libvirt may fail a migration it finished: a cancel alone does not tell.""" op = self.queue(live_migrate="true") ctx = self.start(op) self.dest_conn.moveto.side_effect = libvirtError("operation aborted: migration out: canceled by client") Operation.objects.filter(pk=op.pk).update(cancel_requested=True) - with self.assertRaises(Cancelled): + with self.assertRaises(OperationError): get_kind(op.kind).run(op, ctx) + op.refresh_from_db() + dom = MagicMock(**{"isActive.return_value": True, "jobInfo.return_value": [0] * 12}) + with patch("instances.jobs._domain", side_effect=[dom, None]): # source, destination + self.assertEqual(MigrateInstance().reconcile(op).state, Operation.CANCELLED) + with patch("instances.jobs._domain", side_effect=[None, dom]): + self.assertEqual(MigrateInstance().reconcile(op).state, Operation.SUCCEEDED) self.vm.refresh_from_db() - self.assertEqual(self.vm.compute, self.source) + self.assertEqual(self.vm.compute, self.dest) def test_a_failed_migration_says_what_libvirt_said(self): # libvirt reports the source host's failure to reach the destination's migration