diff --git a/lib/apis/openstack_api/openstack_server.py b/lib/apis/openstack_api/openstack_server.py index c91cc53ca..e2f997beb 100644 --- a/lib/apis/openstack_api/openstack_server.py +++ b/lib/apis/openstack_api/openstack_server.py @@ -10,7 +10,7 @@ logger = logging.getLogger(__name__) -def check_can_be_live_migrated(server: Server): +def can_be_migrated(server: Server): if server.flavor.name.startswith("g-") or server.flavor.name.startswith("f-"): raise ValueError( f"Attempted to move GPU or FPGA flavor, {server.flavor.name}, which is not allowed!" @@ -74,30 +74,26 @@ def snapshot_and_migrate_server( :param dest_host: Optional host to migrate to, otherwise chosen by scheduler """ server = conn.compute.get_server(server_id) + 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) + server_status = server.status.casefold() match server_status: case "shutoff": - # Can't live migrate something that isn't 'alive' - logger.info("Server %s is shutoff, forcing cold migration", server.id) - live_migration = False + _cold_migration(conn, server, dest_host) case "active": if live_migration: - check_can_be_live_migrated(server) + 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" ) - - 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 live_migration: - _live_migration(conn, server, dest_host) - else: - _cold_migration(conn, server, dest_host) logger.info("Migration completed of server: %s", server.id) diff --git a/tests/lib/apis/openstack_api/test_openstack_server.py b/tests/lib/apis/openstack_api/test_openstack_server.py index 0d0236e3b..27a9a6c22 100644 --- a/tests/lib/apis/openstack_api/test_openstack_server.py +++ b/tests/lib/apis/openstack_api/test_openstack_server.py @@ -1,7 +1,7 @@ import itertools from datetime import datetime from typing import Tuple, Any -from unittest.mock import MagicMock, patch, PropertyMock, NonCallableMock +from unittest.mock import MagicMock, patch, PropertyMock import pytest from apis.openstack_api.openstack_server import ( @@ -465,47 +465,6 @@ def test_wait_for_migration_status_error(bad_state): ) -@patch("apis.openstack_api.openstack_server.snapshot_server") -@patch("apis.openstack_api.openstack_server._live_migration") -@patch("apis.openstack_api.openstack_server._cold_migration") -def test_we_skip_all_steps_for_error(cold_migration, live_migration, snapshot_method): - """ - Ensures that we don't attempt to perform any work at all if a server is in an invalid state - as the snapshot step especially can take multiple hours when we already know up-front we'll fail - """ - mock_server = NonCallableMock(return_value="error") - mock_connection = MagicMock() - mock_connection.compute.get_server.return_value = mock_server - - def _assert_no_migration_steps(): - snapshot_method.assert_not_called() - cold_migration.assert_not_called() - live_migration.assert_not_called() - - snapshot_method.reset_mock() - cold_migration.reset_mock() - live_migration.reset_mock() - - # Ensure no combination of flags bypasses these checks - with pytest.raises(ValueError): - snapshot_and_migrate_server( - conn=mock_connection, server_id="", snapshot=True, live_migration=False - ) - _assert_no_migration_steps() - - with pytest.raises(ValueError): - snapshot_and_migrate_server( - conn=mock_connection, server_id="", snapshot=False, live_migration=False - ) - _assert_no_migration_steps() - - with pytest.raises(ValueError): - snapshot_and_migrate_server( - conn=mock_connection, server_id="", snapshot=False, live_migration=True - ) - _assert_no_migration_steps() - - def test_build_server(): """ Test build server on a given hypervisor