From 77a430cc382fdb2b05e2a9d9e92efe0b9ec6fec2 Mon Sep 17 00:00:00 2001
From: Lizzie Salmon <lizzie.salmon@stfc.ac.uk>
Date: Fri, 20 Dec 2024 13:09:33 +0000
Subject: [PATCH] Added in functionality to disable the snapshotting.

---
 actions/server.migrate.yaml                   |  4 ++++
 lib/openstack_api/openstack_server.py         |  6 +++--
 .../openstack_api/test_openstack_server.py    | 23 +++++++++++++++++++
 .../lib/workflows/test_search_by_property.py  |  8 +++++--
 4 files changed, 37 insertions(+), 4 deletions(-)

diff --git a/actions/server.migrate.yaml b/actions/server.migrate.yaml
index 015511c3..6b5f8394 100644
--- a/actions/server.migrate.yaml
+++ b/actions/server.migrate.yaml
@@ -34,4 +34,8 @@ parameters:
     type: string
     required: false
     description: "Optional, host to migrate server to"
+  snapshot:
+    type: boolean
+    required: true
+    default: true
 runner_type: python-script
diff --git a/lib/openstack_api/openstack_server.py b/lib/openstack_api/openstack_server.py
index 68a31f9e..9d5b34e6 100644
--- a/lib/openstack_api/openstack_server.py
+++ b/lib/openstack_api/openstack_server.py
@@ -8,16 +8,18 @@ def snapshot_and_migrate_server(
     conn: Connection,
     server_id: str,
     server_status: str,
+    snapshot: bool,
     dest_host: Optional[str] = None,
 ) -> None:
     """
-    Snapshot a server and then migrate it to a new host
+    Optionally snapshot a server and then migrate it to a new host
     :param conn: Openstack Connection
     :param server_id: Server ID to migrate
     :param server_status: Status of machine to migrate - must be ACTIVE or SHUTOFF
     :param dest_host: Optional host to migrate to, otherwise chosen by scheduler
     """
-    snapshot_server(conn=conn, server_id=server_id)
+    if snapshot:
+        snapshot_server(conn=conn, server_id=server_id)
     if server_status == "ACTIVE":
         live = True
     elif server_status == "SHUTOFF":
diff --git a/tests/lib/openstack_api/test_openstack_server.py b/tests/lib/openstack_api/test_openstack_server.py
index db3d9007..5f5565b7 100644
--- a/tests/lib/openstack_api/test_openstack_server.py
+++ b/tests/lib/openstack_api/test_openstack_server.py
@@ -18,6 +18,7 @@ def test_active_migration(mock_snapshot_server, dest_host):
         server_id=mock_server_id,
         server_status=mock_server_status,
         dest_host=dest_host,
+        snapshot=True,
     )
     mock_snapshot_server.assert_called_once_with(
         conn=mock_connection, server_id=mock_server_id
@@ -42,6 +43,7 @@ def test_shutoff_migration(mock_snapshot_server, dest_host):
         server_id=mock_server_id,
         server_status=mock_server_status,
         dest_host=dest_host,
+        snapshot=True,
     )
     mock_snapshot_server.assert_called_once_with(
         conn=mock_connection, server_id=mock_server_id
@@ -52,8 +54,28 @@ def test_shutoff_migration(mock_snapshot_server, dest_host):
     )
 
 
+@patch("openstack_api.openstack_server.snapshot_server")
+def test_no_snapshot_migration(mock_snapshot_server):
+    """
+    Test migration with no snapshot
+    """
+    mock_connection = MagicMock()
+    mock_server_id = "server1"
+    mock_server_status = "ACTIVE"
+    snapshot_and_migrate_server(
+        conn=mock_connection,
+        server_id=mock_server_id,
+        server_status=mock_server_status,
+        snapshot=False,
+    )
+    mock_snapshot_server.assert_not_called()
+
+
 @patch("openstack_api.openstack_server.snapshot_server")
 def test_migration_fail(mock_snapshot_server):
+    """
+    Test failure of migration when the status is not ACTIVE or SHUTOFF
+    """
     mock_connection = MagicMock()
     mock_server_id = "server1"
     mock_server_status = "TEST"
@@ -65,6 +87,7 @@ def test_migration_fail(mock_snapshot_server):
             conn=mock_connection,
             server_id=mock_server_id,
             server_status=mock_server_status,
+            snapshot=True,
         )
     mock_snapshot_server.assert_called_once_with(
         conn=mock_connection, server_id=mock_server_id
diff --git a/tests/lib/workflows/test_search_by_property.py b/tests/lib/workflows/test_search_by_property.py
index e90cd7c0..691ff536 100644
--- a/tests/lib/workflows/test_search_by_property.py
+++ b/tests/lib/workflows/test_search_by_property.py
@@ -120,9 +120,10 @@ def test_search_by_property_all(mock_openstackquery, mock_to_webhook, output_typ
 
 
 @patch("workflows.search_by_property.to_webhook")
-def test_search_by_property_migrate_webhook(mock_openstackquery):
+@patch("workflows.search_by_property.openstackquery")
+def test_search_by_property_migrate_webhook(mock_openstackquery, mock_to_webhook):
     """
-    Runs search_by_property with webhook as migrate-server
+    Tests search_by_property with webhook as migrate-server adds params to the query needed for the migrate action
     """
 
     mock_query = MagicMock()
@@ -148,3 +149,6 @@ def test_search_by_property_migrate_webhook(mock_openstackquery):
     mock_query.select.assert_called_once_with(
         *params["properties_to_select"], "server_id", "server_status"
     )
+    mock_to_webhook.assert_called_once_with(
+        webhook="migrate-server", payload=mock_query.to_props.return_value
+    )