Add flag live migration

actions/hypervisor.drain.yaml

@@ -21,3 +21,8 @@ parameters:
     description: Reason for draining the hypervisor
     required: true
     type: string
+  live_migration:
+    description: Decides if an ACTIVE Server should go under Live Migration (default) or Cold Migration. A Cold Migration will shutdown ACTIVE Servers, and therefore ths flag does not affect SHUTOFF Servers. 
+    required: true
+    type: boolean
+    default: true

actions/server.migrate.yaml

@@ -34,4 +34,9 @@ parameters:
     type: boolean
     required: true
     default: true
+  live_migration:
+    description: Decides if an ACTIVE Server should go under Live Migration (default) or Cold Migration. A Cold Migration will shutdown ACTIVE Servers, and therefore this flag does not affect SHUTOFF Servers.
+    required: true
+    type: boolean
+    default: true
 runner_type: python-script

actions/workflows/hypervisor.drain.workflow.yaml

@@ -6,6 +6,7 @@ input:
   - cloud_account
   - hypervisor_name
   - disabled_reason
+  - live_migration: true
 
 vars:
   - servers: null
@@ -48,3 +49,4 @@ tasks:
       cloud_account: <% ctx().cloud_account %>
       server_id: <% item(server_id) %>
       snapshot: <% not item(server_name).startsWith("amphora-") %>
+      live_migration: <% ctx().live_migration %>

lib/apis/openstack_api/openstack_server.py

@@ -15,20 +15,53 @@ def can_be_migrated(server: Server):
         raise ValueError(
             f"Attempted to move GPU or FPGA flavor, {server.flavor.name}, which is not allowed!"
         )
-    if server.status not in ["ACTIVE", "SHUTOFF"]:
-        raise ValueError(
-            f"Server status: {server.status}. The server must be ACTIVE or SHUTOFF to be migrated"
-        )
     if server.flavor.vcpus > 60:
         raise ValueError(
             f"Attempted to move flavor with greater than 60 cores, {server.flavor.name}, which is not allowed!"
         )
 
 
+def _cold_migration(
+    conn: Connection,
+    server: Server,
+    dest_host: Optional[str] = None,
+) -> None:
+    conn.compute.migrate_server(server=server.id, host=dest_host)
+
+    # Using conn.compute.wait_for_server will wait even if our migration transitions to error
+    # causing a hang until timeout so we must wait for the migration then wait for OpenStack
+    # to update the status after....
+    wait_for_migration_status(conn, server.id, "finished")
+
+    # We're just waiting for Nova to update as the migration has completed here
+    conn.compute.wait_for_server(server, status="VERIFY_RESIZE", wait=10)
+
+    if server.status.casefold() != "VERIFY_RESIZE".casefold():
+        raise RuntimeError(
+            f"Migration caused VM to enter unexpected state {server.status}"
+            " instead of 'VERIFY_RESIZE'."
+        )
+
+    logger.info("Confirming resize for %s", server.id)
+    conn.compute.confirm_server_resize(server.id)
+
+
+def _live_migration(
+    conn: Connection,
+    server: Server,
+    dest_host: Optional[str] = None,
+) -> None:
+    conn.compute.live_migrate_server(
+        server=server.id, host=dest_host, block_migration=True
+    )
+    wait_for_migration_status(conn, server.id, "completed")
+
+
 def snapshot_and_migrate_server(
     conn: Connection,
     server_id: str,
     snapshot: bool,
+    live_migration: bool,
     dest_host: Optional[str] = None,
 ) -> None:
     """
@@ -37,24 +70,30 @@ def snapshot_and_migrate_server(
     :param server_id: Server ID to migrate
     :param server_status: Status of machine to migrate - must be ACTIVE or SHUTOFF
     :param flavor_name: Server flavor name
+    :param live_migration: decides if an ACTIVE Server should go under Cold Migration or Live Migration
     :param dest_host: Optional host to migrate to, otherwise chosen by scheduler
     """
     server = conn.compute.get_server(server_id)
-    can_be_migrated(server)
     if snapshot:
         snapshot_server(conn=conn, server_id=server_id)
         time.sleep(10)  # Ensure server task status has updated after snapshot
     logger.info("Migrating server: %s", server.id)
-    if server.status == "SHUTOFF":
-        conn.compute.migrate_server(server=server_id, host=dest_host)
-        conn.compute.wait_for_server(server, status="VERIFY_RESIZE", wait=3600)
-        conn.compute.confirm_server_resize(server_id)
-        wait_for_migration_status(conn, server_id, "confirmed")
-    if server.status == "ACTIVE":
-        conn.compute.live_migrate_server(
-            server=server_id, host=dest_host, block_migration=True
-        )
-        wait_for_migration_status(conn, server_id, "completed")
+
+    server_status = server.status.casefold()
+
+    match server_status:
+        case "shutoff":
+            _cold_migration(conn, server, dest_host)
+        case "active":
+            if live_migration:
+                can_be_migrated(server)
+                _live_migration(conn, server, dest_host)
+            else:
+                _cold_migration(conn, server, dest_host)
+        case _:
+            raise ValueError(
+                f"Server status: {server.status}. The server must be ACTIVE or SHUTOFF to be migrated"
+            )
     logger.info("Migration completed of server: %s", server.id)
 
 

tests/lib/apis/openstack_api/test_openstack_server.py

@@ -1,6 +1,7 @@
 import itertools
 from datetime import datetime
-from unittest.mock import MagicMock, patch
+from typing import Tuple, Any
+from unittest.mock import MagicMock, patch, PropertyMock
 
 import pytest
 from apis.openstack_api.openstack_server import (
@@ -34,6 +35,7 @@ def test_active_migration(
     mock_connection = MagicMock()
     mock_server_id = "server1"
     mock_server = MagicMock()
+    mock_server.id = mock_server_id
     mock_server.flavor.name = "l3.nano"
     mock_server.flavor.vcpus = 2
     mock_server.status = "ACTIVE"
@@ -43,6 +45,7 @@ def test_active_migration(
         server_id=mock_server_id,
         dest_host=dest_host,
         snapshot=True,
+        live_migration=True,
     )
     mock_snapshot_server.assert_called_once_with(
         conn=mock_connection, server_id=mock_server_id
@@ -57,6 +60,55 @@ def test_active_migration(
     mock_connection.compute.migrate_server.assert_not_called()
 
 
+def _migration_side_effect_generator(
+    mock_server, mock_connection, return_values: Tuple[Any, Any]
+) -> None:
+    """
+    Instruments a fake server to always return the first value of a tuple until migrate is called
+    after it will always return the second value of the tuple
+    """
+
+    class MigrateAPIFake:
+        def __init__(self):
+            self.called = False
+
+        def callable_migration(self, *_, **__):
+            self.called = True
+
+        def return_val(self, *_, **__):
+            return return_values[0] if not self.called else return_values[1]
+
+    api_fake = MigrateAPIFake()
+
+    # Patch in a side effect of calling migrate_server will cause mock_server.status to update to VERIFY_RESIZE
+    mock_connection.compute.migrate_server.side_effect = api_fake.callable_migration
+    type(mock_server).status = PropertyMock(side_effect=api_fake.return_val)
+    mock_connection.compute.get_server.return_value = mock_server
+
+
+@patch("apis.openstack_api.openstack_server.snapshot_server")
+@patch("apis.openstack_api.openstack_server.wait_for_migration_status")
+def test_active_cold_migration(_, __):
+    """
+    Tests that active VMs are also be migratable
+    """
+    mock_connection = MagicMock()
+    mock_server = MagicMock()
+
+    _migration_side_effect_generator(
+        mock_server, mock_connection, return_values=("ACTIVE", "VERIFY_RESIZE")
+    )
+    snapshot_and_migrate_server(
+        conn=mock_connection,
+        server_id="",
+        snapshot=False,
+        live_migration=False,
+    )
+
+    # If we see migrate server called we can assume it's done the same calls as shutoff any rely on that unit test
+    mock_connection.compute.migrate_server.assert_called_once()
+
+
 @pytest.mark.parametrize("dest_host", [None, "hv01"])
 @patch("apis.openstack_api.openstack_server.snapshot_server")
 @patch("apis.openstack_api.openstack_server.wait_for_migration_status")
@@ -69,17 +121,25 @@ def test_shutoff_migration(
     mock_connection = MagicMock()
     mock_server_id = "server1"
     mock_server = MagicMock()
-    mock_server.status = "SHUTOFF"
+    mock_server.id = mock_server_id
     mock_server.flavor.name = "l3.nano"
     mock_server.flavor.vcpus = 2
 
-    mock_connection.compute.get_server.return_value = mock_server
+    _migration_side_effect_generator(
+        mock_server,
+        mock_connection,
+        # Mixed case intentional
+        return_values=("SHUToff", "VERIFY_RESIZE"),
+    )
+
     snapshot_and_migrate_server(
         conn=mock_connection,
         server_id=mock_server_id,
         dest_host=dest_host,
         snapshot=True,
+        live_migration=False,
     )
+
     mock_snapshot_server.assert_called_once_with(
         conn=mock_connection, server_id=mock_server_id
     )
@@ -88,25 +148,64 @@ def test_shutoff_migration(
         server=mock_server_id, host=dest_host
     )
     mock_connection.compute.wait_for_server.assert_called_once_with(
-        mock_server, status="VERIFY_RESIZE", wait=3600
+        mock_server, status="VERIFY_RESIZE", wait=10
+    )
+    mock_connection.compute.confirm_server_resize.assert_called_once_with(
+        mock_server_id
     )
-    mock_connection.compute.confirm_server_resize(mock_server_id)
     mock_wait_for_migration_status.assert_called_once_with(
-        mock_connection, mock_server_id, "confirmed"
+        mock_connection, mock_server_id, "finished"
     )
 
 
+@patch("apis.openstack_api.openstack_server.snapshot_server")
+@patch("apis.openstack_api.openstack_server.wait_for_migration_status")
+def test_invalid_status_after_migration(_, __):
+    """
+    Tests if the migration is marked finished but the VM moves into an error or unknown state instead of VERIFY_RESIZE
+    """
+    mock_connection = MagicMock()
+    mock_server = MagicMock()
+
+    _migration_side_effect_generator(
+        mock_server, mock_connection, return_values=("ACTIVE", "ERROR")
+    )
+    with pytest.raises(RuntimeError):
+        snapshot_and_migrate_server(
+            conn=mock_connection,
+            server_id="",
+            snapshot=False,
+            live_migration=False,
+        )
+
+    mock_connection.compute.migrate_server.assert_called_once()
+    mock_connection.compute.confirm_resize.assert_not_called()
+
+    # Also confirm we error on a weird state like ACTIVE which migrate should not go to
+    _migration_side_effect_generator(
+        mock_server, mock_connection, return_values=("ACTIVE", "ACTIVE")
+    )
+    with pytest.raises(RuntimeError):
+        snapshot_and_migrate_server(
+            conn=mock_connection,
+            server_id="",
+            snapshot=False,
+            live_migration=False,
+        )
+
+
 @patch("apis.openstack_api.openstack_server.snapshot_server")
 @patch("apis.openstack_api.openstack_server.wait_for_migration_status")
 def test_active_migration_failed_wait_for_status(
     mock_wait_for_migration_status, mock_snapshot_server
 ):
     """
-    Test migration when the the status of migration raises an error
+    Test migration when the status of migration raises an error
     """
     mock_connection = MagicMock()
     mock_server_id = "server1"
     mock_server = MagicMock()
+    mock_server.id = mock_server_id
     mock_server.flavor.name = "l3.nano"
     mock_server.flavor.vcpus = 2
     mock_server.status = "ACTIVE"
@@ -119,6 +218,7 @@ def test_active_migration_failed_wait_for_status(
             server_id=mock_server_id,
             dest_host=None,
             snapshot=True,
+            live_migration=True,
         )
     mock_snapshot_server.assert_called_once_with(
         conn=mock_connection, server_id=mock_server_id
@@ -141,6 +241,7 @@ def test_no_snapshot_migration(mock_wait_for_migration_status, mock_snapshot_ser
     mock_connection = MagicMock()
     mock_server_id = "server1"
     mock_server = MagicMock()
+    mock_server.id = mock_server_id
     mock_server.flavor.name = "l3.nano"
     mock_server.flavor.vcpus = 2
     mock_server.status = "ACTIVE"
@@ -150,6 +251,7 @@ def test_no_snapshot_migration(mock_wait_for_migration_status, mock_snapshot_ser
         conn=mock_connection,
         server_id=mock_server_id,
         snapshot=False,
+        live_migration=True,
     )
     mock_snapshot_server.assert_not_called()
     mock_connection.compute.live_migrate_server.assert_called_once_with(
@@ -168,6 +270,7 @@ def test_migration_fail(mock_snapshot_server):
     mock_connection = MagicMock()
     mock_server_id = "server1"
     mock_server = MagicMock()
+    mock_server.id = mock_server_id
     mock_server.flavor.name = "l3.nano"
     mock_server.flavor.vcpus = 2
     mock_server.status = "ERROR"
@@ -180,7 +283,8 @@ def test_migration_fail(mock_snapshot_server):
         snapshot_and_migrate_server(
             conn=mock_connection,
             server_id=mock_server_id,
-            snapshot=True,
+            snapshot=False,
+            live_migration=True,
         )
     mock_snapshot_server.assert_not_called()
     mock_connection.compute.live_migrate_server.assert_not_called()
@@ -231,6 +335,7 @@ def test_raise_invalid_migration(flavor_name, flavor_vcpu):
     mock_connection = MagicMock()
     mock_server_id = "server1"
     mock_server = MagicMock()
+    mock_server.id = mock_server_id
     mock_server.flavor.name = flavor_name
     mock_server.flavor.vcpus = flavor_vcpu
     mock_server.status = "ACTIVE"
@@ -241,7 +346,8 @@ def test_raise_invalid_migration(flavor_name, flavor_vcpu):
             conn=mock_connection,
             server_id=mock_server_id,
             dest_host=None,
-            snapshot=True,
+            snapshot=False,
+            live_migration=True,
         )