Reinstate server_query check, create group_servers_by_user_id function, update unit tests

Author: jcornall

lib/apis/openstack_query_api/server_queries.py

@@ -2,6 +2,7 @@
 
 from typing import List, Optional
 from openstackquery import ImageQuery
+from openstackquery.api.query_api import QueryAPI
 
 
 def find_servers_with_image(
@@ -49,18 +50,21 @@ def find_servers_with_image(
         "addresses",
     )
 
-    if not server_query.to_props():
-        raise RuntimeError(
-            f"No servers found with images {', '.join(image_name_list)} on projects "
-            f"{','.join(from_projects) if from_projects else '[all projects]'}"
-        )
-
     server_query.append_from("PROJECT_QUERY", cloud_account, ["name"])
-    server_query.group_by("user_id")
 
     return server_query
 
 
+def group_servers_by_user_id(server_query: QueryAPI):
+    """
+    Group the servers in a server query by "user_id" and return the results
+    :param server_query: QueryAPI object containing the results of a server query
+    """
+    grouped_query = server_query.group_by("user_id")
+
+    return grouped_query
+
+
 def list_to_regex_pattern(string_list):
     """
     converts a list of strings into a regex pattern that matches any occurrence of any string in the input list.

lib/workflows/send_decom_image_email.py

@@ -6,7 +6,10 @@
 from apis.email_api.structs.email_template_details import EmailTemplateDetails
 from apis.email_api.structs.smtp_account import SMTPAccount
 from apis.openstack_api.enums.cloud_domains import CloudDomains
-from apis.openstack_query_api.server_queries import find_servers_with_image
+from apis.openstack_query_api.server_queries import (
+    find_servers_with_image,
+    group_servers_by_user_id,
+)
 from apis.openstack_query_api.user_queries import find_user_info
 from tabulate import tabulate
 
@@ -175,7 +178,15 @@ def send_decom_image_email(
         cloud_account, image_name_list, limit_by_projects
     )
 
-    for user_id in server_query.to_props().keys():
+    if not server_query.to_props():
+        raise RuntimeError(
+            f"No servers found with images {', '.join(image_name_list)} on projects "
+            f"{','.join(limit_by_projects) if limit_by_projects else '[all projects]'}"
+        )
+
+    grouped_query = group_servers_by_user_id(server_query)
+
+    for user_id in grouped_query.to_props().keys():
         # if email_address not found - send to override_email_address
         # also send to override_email_address if override_email set
         user_name, email_addr = find_user_info(
@@ -191,18 +202,18 @@ def send_decom_image_email(
                 user_name,
                 as_html,
                 get_affected_images_plaintext(decom_image_info),
-                server_query.to_string(groups=[user_id], include_group_titles=False),
+                grouped_query.to_string(groups=[user_id], include_group_titles=False),
             )
 
         else:
             if as_html:
                 image_list = get_affected_images_html(decom_image_info)
-                server_list = server_query.to_html(
+                server_list = grouped_query.to_html(
                     groups=[user_id], include_group_titles=False
                 )
             else:
                 image_list = get_affected_images_plaintext(decom_image_info)
-                server_list = server_query.to_string(
+                server_list = grouped_query.to_string(
                     groups=[user_id], include_group_titles=False
                 )
 

tests/lib/apis/openstack_query_api/test_server_queries.py

@@ -3,6 +3,7 @@
 import pytest
 from apis.openstack_query_api.server_queries import (
     find_servers_with_image,
+    group_servers_by_user_id,
     list_to_regex_pattern,
 )
 
@@ -17,6 +18,18 @@ def test_list_to_regex_pattern():
     assert res == "(.*img1|.*img2|.*img3)"
 
 
+@patch("apis.openstack_query_api.server_queries.QueryAPI")
+def test_group_by_user_id(mock_server_query):
+    """
+    Tests group_servers_by_user_id() function
+    """
+    mock_server_query_obj = mock_server_query.return_value
+    mock_grouped_query_obj = mock_server_query_obj.group_by.return_value
+    res = group_servers_by_user_id(mock_server_query_obj)
+    mock_server_query_obj.group_by.assert_called_once_with("user_id")
+    assert res == mock_grouped_query_obj
+
+
 @patch("apis.openstack_query_api.server_queries.ImageQuery")
 @patch("apis.openstack_query_api.server_queries.list_to_regex_pattern")
 def test_find_servers_with_image_valid(mock_list_to_regex, mock_image_query):
@@ -59,12 +72,9 @@ def test_find_servers_with_image_valid(mock_list_to_regex, mock_image_query):
         all_projects=False,
     )
     mock_server_query_obj.select.assert_called_once_with("id", "name", "addresses")
-    mock_server_query_obj.to_props.assert_called_once()
-
     mock_server_query_obj.append_from.assert_called_once_with(
         "PROJECT_QUERY", "test-cloud-account", ["name"]
     )
-    mock_server_query_obj.group_by.assert_called_once_with("user_id")
     assert res == mock_server_query_obj
 
 
@@ -98,48 +108,3 @@ def test_find_servers_with_image_invalid_images(mock_list_to_regex, mock_image_q
         ("id", "ascending"), ("name", "ascending")
     )
     mock_image_query_obj.to_props.assert_called_once()
-
-
-@patch("apis.openstack_query_api.server_queries.ImageQuery")
-@patch("apis.openstack_query_api.server_queries.list_to_regex_pattern")
-def test_find_servers_with_image_no_servers_found(mock_list_to_regex, mock_image_query):
-    """
-    Tests that find_servers_with_images() raises an error when no servers are found
-    """
-
-    mock_image_query_obj = mock_image_query.return_value
-    mock_server_query_obj = mock_image_query_obj.then.return_value
-    mock_server_query_obj.to_props.return_value = None
-
-    with pytest.raises(RuntimeError):
-        find_servers_with_image(
-            "test-cloud-account", ["img1", "img2"], ["project1", "project2"]
-        )
-
-    mock_image_query.assert_called_once()
-    mock_list_to_regex.assert_called_once_with(["img1", "img2"])
-    mock_image_query_obj.where.assert_called_once_with(
-        "matches_regex",
-        "name",
-        value=mock_list_to_regex.return_value,
-    )
-
-    mock_image_query_obj.run.assert_called_once_with(
-        "test-cloud-account",
-        as_admin=True,
-        from_projects=["project1", "project2"],
-        all_projects=False,
-    )
-    mock_image_query_obj.sort_by.assert_called_once_with(
-        ("id", "ascending"), ("name", "ascending")
-    )
-    mock_image_query_obj.to_props.assert_called_once()
-
-    mock_server_query_obj.run.assert_called_once_with(
-        "test-cloud-account",
-        as_admin=True,
-        from_projects=["project1", "project2"],
-        all_projects=False,
-    )
-    mock_server_query_obj.select.assert_called_once_with("id", "name", "addresses")
-    mock_server_query_obj.to_props.assert_called_once()