diff --git a/instances/jobs.py b/instances/jobs.py index e01e97c6..c7da6194 100644 --- a/instances/jobs.py +++ b/instances/jobs.py @@ -941,6 +941,12 @@ def _domain(compute_pk, uuid): return None +def _shut_off_again(op, dom): + """A shut-off VM the migration started paused, left running by a job that stopped.""" + if op.intent.get("shut_off") and dom.isActive(): + dom.destroy() + + @register class MigrateInstance(Kind): name = "instance.migrate" @@ -987,7 +993,8 @@ class MigrateInstance(Kind): def run(self, op, ctx): p = op.params instance = _instance(p["instance"]) - ctx.record_intent() + # a shut-off VM may migrate started paused (migrate_running): recovery shuts it off again + ctx.record_intent(shut_off=instance.proxy.get_status() == 5) ctx.set_phase(gettext_noop("Migrating the VM")) try: utils.migrate_instance(Compute.objects.get(pk=p["target"]), instance, op.user, @@ -1040,8 +1047,10 @@ class MigrateInstance(Kind): instance = Instance.objects.filter(pk=op.params["instance"], compute_id=op.intent["compute"]).first() if instance is not None: # the job stopped before it moved the record utils.record_migration(instance, Compute.objects.get(pk=op.params["target"])) + _shut_off_again(op, dest) return Outcome(Operation.SUCCEEDED) if source is not None and dest is None: + _shut_off_again(op, source) return Outcome(Operation.FAILED, error=say(_("The VM stayed on the source host"))) return Outcome(Operation.UNCERTAIN, error=say(_( "The VM is on both computes or on neither; check them, then resolve this job."))) diff --git a/instances/tests/test_migrate_job.py b/instances/tests/test_migrate_job.py index 275e3e23..0c98f81c 100644 --- a/instances/tests/test_migrate_job.py +++ b/instances/tests/test_migrate_job.py @@ -79,6 +79,27 @@ class MigrateJobTestCase(TransactionTestCase): self.vm.refresh_from_db() self.assertEqual(self.vm.compute, self.dest) + def test_recovery_shuts_off_a_shut_off_vm_the_migration_left_started(self): + self.vm.proxy.get_status.return_value = 5 + op = self.queue() + ctx = self.start(op) + self.dest_conn.moveto.side_effect = OSError("the worker stops here") + with self.assertRaises(OSError): + get_kind(op.kind).run(op, ctx) + self.assertTrue(op.intent["shut_off"]) + dom = MagicMock() + dom.isActive.return_value = True + dom.jobInfo.return_value = [0] * 12 # VIR_DOMAIN_JOB_NONE + for found in ([dom, None], [None, dom]): # stayed on the source, or moved + with self.subTest(found=found), patch("instances.jobs._domain", side_effect=found): + MigrateInstance().reconcile(op) + dom.destroy.assert_called_once_with() + dom.destroy.reset_mock() + op.intent["shut_off"] = False # a running VM keeps running + with patch("instances.jobs._domain", side_effect=[dom, None]): + MigrateInstance().reconcile(op) + dom.destroy.assert_not_called() + def test_a_copy_claims_its_volume_on_both_computes(self): Instance.proxy.get_disk_devices.return_value = [ {"dev": "vda", "storage": "default", "image": "mig-vm.qcow2", "path": "/i/mig-vm.qcow2"},