diff --git a/instances/test_migration_log.py b/instances/test_migration_log.py
new file mode 100644
index 00000000..c14e62e9
--- /dev/null
+++ b/instances/test_migration_log.py
@@ -0,0 +1,73 @@
+"""A migration is logged where it runs (migrate_instance), for the VM page
+and the API alike: the mode it used on success, the error on failure."""
+
+from unittest.mock import MagicMock, patch
+
+from computes.models import Compute
+from django.contrib.auth import get_user_model
+from django.test import TestCase
+from django.urls import reverse
+from logs.models import Logs
+from rest_framework.test import APIClient
+
+from instances.models import Instance
+from instances.utils import migrate_instance
+from vrtManager import util
+
+
+class MigrationLogTests(TestCase):
+ def setUp(self):
+ self.admin = get_user_model().objects.create_superuser("ml-admin", "ml@example.com", "pw")
+ self.source = Compute.objects.create(name="ml-src", hostname="198.51.100.8", login="root", password="", type=2)
+ self.dest = Compute.objects.create(name="ml-dst", hostname="198.51.100.9", login="root", password="", type=2)
+ self.vm = Instance.objects.create(compute=self.source, name="vm", uuid="u-ml")
+
+ def migrate(self, outcome, **request):
+ with patch("instances.utils.connection_manager.host_is_up", return_value=True), \
+ patch("instances.utils.wvmInstances") as instances, \
+ patch("instances.utils.wvmInstance"), \
+ patch.object(Instance, "proxy", MagicMock()), \
+ patch.object(Instance, "autostart", False):
+ if isinstance(outcome, Exception):
+ instances.return_value.moveto.side_effect = outcome
+ else:
+ instances.return_value.moveto.return_value = outcome
+ return migrate_instance(self.dest, self.vm, self.admin, **request)
+
+ def test_success_names_the_mode_used(self):
+ self.migrate("non-live, compressed", live=False, compress=True)
+ log = Logs.objects.get()
+ self.assertEqual(log.message, "Instance is migrated(non-live, compressed) to 198.51.100.9")
+ self.assertEqual((log.user, log.host, log.instance), ("ml-admin", "198.51.100.8", "vm"))
+
+ def test_success_names_the_migration_address(self):
+ self.dest.migration_address = "203.0.113.9"
+ self.migrate("live", live=True)
+ self.assertEqual(Logs.objects.get().message, "Instance is migrated(live) to 198.51.100.9 via 203.0.113.9")
+ self.vm.compute = self.source
+ self.dest.migration_address = "198.51.100.9" # the host name itself
+ self.migrate("live", live=True)
+ self.assertEqual(Logs.objects.latest("id").message, "Instance is migrated(live) to 198.51.100.9")
+
+ def test_failure_is_logged_and_raised(self):
+ with self.assertRaisesRegex(util.OperationError, "A shut-off VM cannot be migrated live"):
+ self.migrate(util.OperationError("A shut-off VM cannot be migrated live"), live=True)
+ self.assertEqual(
+ Logs.objects.get().message,
+ "Instance migration to 198.51.100.9 failed: A shut-off VM cannot be migrated live",
+ )
+
+ def test_the_page_and_the_api_log_once_through_migrate_instance(self):
+ self.client.force_login(self.admin)
+ api = APIClient()
+ api.force_authenticate(self.admin)
+ with patch("instances.views.utils.migrate_instance", return_value="live") as page_migrate:
+ self.client.post(reverse("instances:migrate", args=[self.vm.id]),
+ {"compute_id": self.dest.id, "live_migrate": "true"})
+ with patch("instances.api.viewsets.migrate_instance", return_value="live") as api_migrate:
+ response = api.post("/api/v1/migrate/", {"instance": self.vm.id, "target_compute": self.dest.id,
+ "live": True, "offline": False}, format="json")
+ self.assertEqual(page_migrate.call_count, 1)
+ self.assertEqual(api_migrate.call_count, 1)
+ self.assertFalse(Logs.objects.exists()) # migrate_instance logs; it is patched here
+ self.assertEqual(response.json(), {"status": "instance is migrated (live)"})
diff --git a/instances/test_security_mutations.py b/instances/test_security_mutations.py
index 8420e288..f5dc53cb 100644
--- a/instances/test_security_mutations.py
+++ b/instances/test_security_mutations.py
@@ -273,6 +273,7 @@ class InstanceSecurityMutationsTestCase(TestCase):
patch("instances.utils.wvmInstances") as mock_instances_cls, \
patch("instances.utils.wvmInstance"), \
patch("instances.utils.transaction.atomic"), \
+ patch("instances.utils.addlogmsg"), \
patch("instances.utils.Instance") as mock_instance_model:
mock_instances_cls.return_value.moveto.side_effect = slow_moveto
diff --git a/instances/utils.py b/instances/utils.py
index c1b34507..aeb07070 100644
--- a/instances/utils.py
+++ b/instances/utils.py
@@ -10,6 +10,7 @@ from django.conf import settings
from django.db import transaction
from django.utils.translation import gettext_lazy as _
from libvirt import libvirtError
+from logs.views import addlogmsg
from vrtManager import util
from vrtManager.connection import connection_manager
from vrtManager.instance import wvmInstance, wvmInstances
@@ -159,12 +160,14 @@ def migrate_instance(
offline=False,
autoconverge=False,
compress=False,
- postcopy=False,
):
+ """Migrates instance to new_compute and logs the outcome (the mode
+ used, or the error). Returns the mode."""
if new_compute == instance.compute:
raise util.OperationError(_("The instance is already on %(compute)s") % {"compute": new_compute.name})
if not connection_manager.host_is_up(new_compute.type, new_compute.hostname):
raise util.OperationError(_("%(compute)s is not reachable") % {"compute": new_compute.name})
+ source_host = instance.compute.hostname
c1, c2 = (
(instance.compute, new_compute)
if instance.compute.id < new_compute.id
@@ -182,7 +185,7 @@ def migrate_instance(
)
autostart = instance.autostart
- conn_migrate.moveto(
+ mode = conn_migrate.moveto(
instance.proxy,
instance.name,
live,
@@ -190,9 +193,15 @@ def migrate_instance(
offline,
autoconverge,
compress,
- postcopy,
uri=new_compute.migration_uri,
)
+ except Exception as error:
+ msg = _("Instance migration to %(target)s failed: %(error)s") % {
+ "target": migration_target(new_compute),
+ "error": error,
+ }
+ addlogmsg(user.username, source_host, instance.name, msg)
+ raise
finally:
if conn_migrate is not None:
conn_migrate.close()
@@ -247,6 +256,18 @@ def migrate_instance(
instance.compute = new_compute
instance.save()
+ msg = _("Instance is migrated(%(mode)s) to %(target)s") % {"mode": mode, "target": migration_target(new_compute)}
+ addlogmsg(user.username, source_host, instance.name, msg)
+ return mode
+
+
+def migration_target(compute):
+ """The destination for the log: its host name, and the migration address
+ when one is set and differs."""
+ if compute.migration_address not in ("", compute.hostname):
+ return _("%(hostname)s via %(address)s") % {"hostname": compute.hostname, "address": compute.migration_address}
+ return compute.hostname
+
def refr(compute):
from computes.utils import refresh_instance_database
diff --git a/instances/views.py b/instances/views.py
index 5807bb1f..8debc2eb 100755
--- a/instances/views.py
+++ b/instances/views.py
@@ -660,36 +660,16 @@ def migrate(request, pk):
offline = request.POST.get("offline_migrate", False)
autoconverge = request.POST.get("autoconverge", False)
compress = request.POST.get("compress", False)
- postcopy = request.POST.get("postcopy", False)
- current_host = instance.compute.hostname
if not compute_id.isdigit():
raise Http404
target_host = get_object_or_404(Compute, pk=compute_id)
try:
- utils.migrate_instance(
- target_host,
- instance,
- request.user,
- live,
- unsafe,
- offline,
- autoconverge,
- compress,
- postcopy,
- )
+ # migrate_instance logs the outcome
+ utils.migrate_instance(target_host, instance, request.user, live, unsafe, offline, autoconverge, compress)
except (libvirtError, OSError) as err: # OSError includes a lock timeout
messages.error(request, err)
- else:
- migration_method = "live" if live else "offline"
- msg = _("Instance is migrated(%(method)s) to %(hostname)s") % {
- "hostname": target_host.hostname,
- "method": migration_method,
- }
- if target_host.migration_address:
- msg = _("%(msg)s via %(address)s") % {"msg": msg, "address": target_host.migration_address}
- addlogmsg(request.user.username, current_host, instance.name, msg)
return get_safe_redirect(
request, default=reverse("instances:instance", args=[instance.id])
diff --git a/vrtManager/instance.py b/vrtManager/instance.py
index 862ff88e..94232683 100644
--- a/vrtManager/instance.py
+++ b/vrtManager/instance.py
@@ -27,7 +27,6 @@ try:
VIR_MIGRATE_LIVE,
VIR_MIGRATE_OFFLINE,
VIR_MIGRATE_PERSIST_DEST,
- VIR_MIGRATE_POSTCOPY,
VIR_MIGRATE_UNDEFINE_SOURCE,
VIR_MIGRATE_UNSAFE,
VIR_DOMAIN_SNAPSHOT_CREATE_DISK_ONLY,
@@ -111,35 +110,45 @@ class wvmInstances(wvmConnect):
dom = self.get_instance(name)
dom.resume()
- def moveto(
- self,
- conn,
- name,
- live,
- unsafe,
- offline,
- autoconverge=False,
- compress=False,
- postcopy=False,
- uri=None,
- ):
- """uri: the native migration URI (tcp://address) on the destination;
- None lets libvirt use the destination's own hostname."""
+ def moveto(self, conn, name, live, unsafe, offline, autoconverge=False, compress=False, uri=None):
+ """Migrates the VM of conn (a wvmInstance on the source) here.
+
+ The mode follows the VM's state, read once: a running VM migrates
+ live or, without live, paused for the transfer (non-live); a paused
+ VM stays paused; a shut-off VM moves offline. A request that does not
+ fit the state is refused. unsafe, autoconverge (live only) and
+ compress apply to a running VM. uri: the native migration URI
+ (tcp://address) on the destination; None lets libvirt use the
+ destination's own hostname. Returns the mode and options used, for
+ the log."""
+ state = conn.get_status()
+ if state == 5:
+ if live:
+ raise util.OperationError("A shut-off VM cannot be migrated live")
+ mode = ["offline"]
+ elif state in (1, 3):
+ if offline:
+ raise util.OperationError("A running or paused VM cannot be migrated offline")
+ mode = ["live" if live else "non-live"]
+ else:
+ raise util.OperationError("The VM can be migrated when it is running, paused or shut off")
+
# The source definition always goes: one left behind points at the
# same (shared) disks as the migrated VM and could be started again.
flags = VIR_MIGRATE_PERSIST_DEST | VIR_MIGRATE_UNDEFINE_SOURCE
- if live and conn.get_status() != 5:
- flags |= VIR_MIGRATE_LIVE
- if unsafe and conn.get_status() == 1:
- flags |= VIR_MIGRATE_UNSAFE
- if offline and conn.get_status() == 5:
+ if state == 5:
flags |= VIR_MIGRATE_OFFLINE
- if not offline and autoconverge:
- flags |= VIR_MIGRATE_AUTO_CONVERGE
- if not offline and compress and conn.get_status() == 1:
- flags |= VIR_MIGRATE_COMPRESSED
- if not offline and postcopy and conn.get_status() == 1:
- flags |= VIR_MIGRATE_POSTCOPY
+ elif live:
+ flags |= VIR_MIGRATE_LIVE
+ if state == 1:
+ for wanted, flag, label in (
+ (unsafe, VIR_MIGRATE_UNSAFE, "unsafe"),
+ (autoconverge and live, VIR_MIGRATE_AUTO_CONVERGE, "auto converge"),
+ (compress, VIR_MIGRATE_COMPRESSED, "compressed"),
+ ):
+ if wanted:
+ flags |= flag
+ mode.append(label)
dom = conn.instance
@@ -151,13 +160,15 @@ class wvmInstances(wvmConnect):
"Destination host emulator is different. Cannot be migrated"
)
- if conn.get_status() == 5:
+ if state == 5:
# Workaround for a libvirt bug (RHEL-156800), see move_definition.
# Once supported hosts all run a fixed libvirt, this branch and
- # move_definition can go: VIR_MIGRATE_OFFLINE is set above.
+ # move_definition can go: the flags above are those for
+ # dom.migrate.
self.move_definition(dom)
- return
- dom.migrate(self.wvm, flags, None, uri, 0)
+ else:
+ dom.migrate(self.wvm, flags, None, uri, 0)
+ return ", ".join(mode)
def move_definition(self, dom):
"""Migrates a shut-off VM: that is only its definition, as the disks
diff --git a/vrtManager/test_migration.py b/vrtManager/test_migration.py
index fd916369..676e02ae 100644
--- a/vrtManager/test_migration.py
+++ b/vrtManager/test_migration.py
@@ -106,7 +106,10 @@ class FakeSource:
self.state = state
self.instance = FakeDomain(**domain)
+ reads = 0
+
def get_status(self):
+ self.reads += 1
return self.state
def get_arch(self):
@@ -206,3 +209,42 @@ class ShutOffMigrationTestCase(unittest.TestCase):
self.assertEqual(source.instance.redefined, ["vm"])
self.assertEqual(source.instance.restored_autostart, 1)
self.assertIsNotNone(host.new[0].undefined)
+
+
+class MigrationModeTestCase(unittest.TestCase):
+ """The state is read once; a request that does not fit it is refused
+ before anything changes, and moveto names the mode it used."""
+
+ def migrate(self, state, **request):
+ source = FakeSource(state)
+ options = {"live": False, "unsafe": False, "offline": False, **request}
+ mode = destination().moveto(source, "vm", **options)
+ return mode, source.instance.calls[0][0] if source.instance.calls else None
+
+ def test_refused_requests(self):
+ for state, request in ((5, {"live": True}), (1, {"offline": True}), (3, {"offline": True}),
+ (1, {"live": True, "offline": True}), (6, {}), (4, {}), (7, {}), (0, {})):
+ with self.subTest(state=state, request=request):
+ source = FakeSource(state)
+ with self.assertRaises(util.OperationError):
+ destination().moveto(source, "vm", unsafe=False, **{"live": False, "offline": False, **request})
+ self.assertEqual(source.instance.calls, [])
+
+ def test_modes(self):
+ live, unsafe = libvirt.VIR_MIGRATE_LIVE, libvirt.VIR_MIGRATE_UNSAFE
+ converge, compressed = libvirt.VIR_MIGRATE_AUTO_CONVERGE, libvirt.VIR_MIGRATE_COMPRESSED
+ mode, flags = self.migrate(1, live=True, autoconverge=True, compress=True, unsafe=True)
+ self.assertEqual(mode, "live, unsafe, auto converge, compressed")
+ self.assertEqual(flags & (live | unsafe | converge | compressed), live | unsafe | converge | compressed)
+ source = FakeSource(1)
+ destination().moveto(source, "vm", live=True, unsafe=False, offline=False)
+ self.assertEqual(source.reads, 1)
+ mode, flags = self.migrate(1, compress=True, autoconverge=True)
+ self.assertEqual(mode, "non-live, compressed")
+ self.assertEqual(flags & (live | converge | compressed), compressed)
+ # a paused VM stays paused; the tuning options are for running VMs
+ mode, flags = self.migrate(3, live=True, autoconverge=True, compress=True, unsafe=True)
+ self.assertEqual(mode, "live")
+ self.assertEqual(flags & (live | unsafe | converge | compressed), live)
+ self.assertEqual(self.migrate(5), ("offline", None))
+ self.assertEqual(self.migrate(5, offline=True), ("offline", None))