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