ENH: Add days_threshold for errored_vm schema and correct the unittests

actions/email.users.with.errored.vms.yaml

@@ -33,11 +33,11 @@ parameters:
     description: "Comma-spaced project to limit action to - incompatible with all_projects"
     required: false
     default: null
-  age_filter_value:
+  days_threshold:
     type: integer
     description: "An integer which specifies the minimum age (in days) of the servers to be found"
     required: false
-    default: -1
+    default: 60
   all_projects:
     type: boolean
     description: "Tick to search in all projects - default True"

actions/email.users.with.shutoff.vms.yaml

@@ -33,6 +33,11 @@ parameters:
     description: "Comma-spaced project to limit action to - incompatible with all_projects"
     required: false
     default: null
+  days_threshold:
+    type: integer
+    description: "An integer which specifies the minimum age (in days) of the servers to be found"
+    required: false
+    default: 60
   all_projects:
     type: boolean
     description: "Tick to search in all projects - default True"

lib/apis/email_api/email_template_schemas.yaml

@@ -43,6 +43,7 @@ shutoff_vm:
   schema:
     username: null
     shutoff_table: "No Shutoff VMs"
+    days_threshold: 60
   html_filepath: "html/shutoff_vm.html.j2"
   plaintext_filepath: "plaintext/shutoff_vm.txt.j2"
 
@@ -80,6 +81,7 @@ errored_vm:
   schema:
     username: null
     error_table: "No Errored VMs"
+    days_threshold: 60
   html_filepath: "html/errored_vm.html.j2"
   plaintext_filepath: "plaintext/errored_vm.txt.j2"
 
@@ -100,5 +102,3 @@ decom_capi_image:
     affected_images_table: "No Images"
   html_filepath: "html/decom_capi_image.html.j2"
   plaintext_filepath: "plaintext/decom_capi_image.txt.j2"
-
-

lib/apis/openstack_query_api/server_queries.py

@@ -13,6 +13,7 @@ def find_servers_on_hv(
     webhook: Optional[str] = None,
 ):
     """
+    Use QueryAPI to find VMs on a hypervisor.
     :param cloud_account: string represents cloud account to use
     :param from_projects: A list of project identifiers to limit search in
     """
@@ -47,7 +48,7 @@ def find_servers_with_flavors(
     from_projects: Optional[List[str]] = None,
 ):
     """
-    Use QueryAPI to run the query to find decom flavors
+    Use QueryAPI to run the query to find decom flavors.
     :param cloud_account: string represents cloud account to use
     :param flavor_name_list: A list of flavor names to be decommissioned
     :param from_projects: A list of project identifiers to limit search in
@@ -88,21 +89,21 @@ def find_servers_with_flavors(
 
 def find_servers_with_errored_vms(
     cloud_account: str,
-    age_filter_value: int,
+    days_threshold: int = 0,
     from_projects: Optional[List[str]] = None,
 ) -> ServerQuery:
     """
     Search for machines that are in error state and returns the user id, name and email address.
     :param cloud_account: String representing cloud account to use
-    :param age_filter_value: An integer which specifies the minimum age (in days) of the servers to be found
+    :param days_threshold: An integer which specifies the minimum age (in days) of the servers to be found
     :param from_projects: A list of project identifiers to limit search to
     """
     server_query = ServerQuery()
-    if age_filter_value > 0:
+    if days_threshold > 0:
         server_query.where(
             "older_than",
             "server_last_updated_date",
-            days=age_filter_value,
+            days=days_threshold,
         )
     server_query.where("any_in", "server_status", values=["ERROR"])
     server_query.run(
@@ -119,14 +120,26 @@ def find_servers_with_errored_vms(
     return server_query
 
 
-def find_shutoff_servers(cloud_account: str, from_projects: Optional[List[str]] = None):
+def find_shutoff_servers(
+    cloud_account: str,
+    days_threshold: int = 0,
+    from_projects: Optional[List[str]] = None,
+):
     """
+    Use QueryAPI to find machines that are in shutoff state.
     :param cloud_account: string represents cloud account to use
+    :param days_threshold: An integer which specifies the minimum age (in days) of the servers to be found
     :param from_projects: A list of project identifiers to limit search in
     """
 
     # Find VMs that have been in shutoff state for more than 60 days
     server_query = ServerQuery()
+    if days_threshold > 0:
+        server_query.where(
+            "older_than",
+            "server_last_updated_date",
+            days=days_threshold,
+        )
     server_query.where("any_in", "server_status", values=["SHUTOFF"])
     server_query.run(
         cloud_account,

lib/workflows/send_errored_vm_email.py

@@ -18,6 +18,7 @@ def print_email_params(
     user_name: str,
     as_html: bool,
     error_table: str,
+    days_threshold: int,
 ):
     """
     Prints email params instead of sending the email.
@@ -31,12 +32,14 @@ def print_email_params(
         f"email_templates errored-email: username {user_name}\n"
         f"send as html: {as_html}\n"
         f"error table: {error_table}\n"
+        f"days_threshold: {days_threshold}\n"
     )
 
 
 def build_email_params(
     user_name: str,
     error_table: str,
+    days_threshold: int,
     **email_kwargs,
 ) -> EmailParams:
     """
@@ -50,6 +53,7 @@ def build_email_params(
         template_params={
             "username": user_name,
             "error_table": error_table,
+            "days_threshold": days_threshold,
         },
     )
 
@@ -64,7 +68,7 @@ def send_errored_vm_email(
     smtp_account: SMTPAccount,
     cloud_account: Union[CloudDomains, str],
     limit_by_projects: Optional[List[str]] = None,
-    age_filter_value: int = -1,
+    days_threshold: int = 60,
     all_projects: bool = False,
     as_html: bool = False,
     send_email: bool = False,
@@ -79,7 +83,7 @@ def send_errored_vm_email(
     :param smtp_account: (SMTPAccount): SMTP config
     :param cloud_account: String representing the cloud account to use
     :param limit_by_projects: A list of project names or ids to limit search in
-    :param age_filter_value: An integer which specifies the minimum age (in days) of the servers to be found
+    :param days_threshold: An integer which specifies the minimum age (in days) of the servers to be found
     :param all_projects: A boolean which, if True, will search in all projects
     :param as_html: A boolean which, if True, will send the email as html
     :param send_email: A boolean which, if True, will send the email instead of printing what will be sent
@@ -98,7 +102,7 @@ def send_errored_vm_email(
         )
 
     server_query = find_servers_with_errored_vms(
-        cloud_account, age_filter_value, limit_by_projects
+        cloud_account, days_threshold, limit_by_projects
     )
 
     if not server_query.to_props():
@@ -127,14 +131,17 @@ def send_errored_vm_email(
             )
 
         if not send_email:
-            print_email_params(send_to[0], user_name, as_html, server_list)
+            print_email_params(
+                send_to[0], user_name, as_html, server_list, days_threshold
+            )
             continue
 
         email_params = build_email_params(
             user_name,
             server_list,
             email_to=send_to,
             as_html=as_html,
+            days_threshold=days_threshold,
             email_cc=("cloud-support@stfc.ac.uk",) if cc_cloud_support else None,
             **email_params_kwargs,
         )

lib/workflows/send_shutoff_vm_email.py

@@ -14,24 +14,32 @@
 
 
 def print_email_params(
-    email_addr: str, user_name: str, as_html: bool, shutoff_table: str
+    email_addr: str,
+    user_name: str,
+    as_html: bool,
+    shutoff_table: str,
+    days_threshold: int,
 ):
     """
     Prints email params instead of sending the email.
     :param email_addr: Email address to send to
     :param user_name: Name of user in OpenStack
     :param as_html: A boolean which if selected will send an email, otherwise prints email details only
     :param shutoff_table: A table representing info found in OpenStack about VMs in shutoff state
+    :param days_threshold: An integer which specifies the minimum age (in days) of the servers to be found
     """
     print(
         f"Send Email To: {email_addr}\n"
         f"email_templates shutoff-email: username {user_name}\n"
         f"send as html: {as_html}\n"
         f"shutoff table: {shutoff_table}\n"
+        f"days threshold: {days_threshold}\n"
     )
 
 
-def build_email_params(user_name: str, shutoff_table: str, **email_kwargs):
+def build_email_params(
+    user_name: str, shutoff_table: str, days_threshold: int, **email_kwargs
+):
     """
     Builds email params dataclass which will be used to configure how to send the email.
     :param user_name: Name of user in OpenStack
@@ -43,6 +51,7 @@ def build_email_params(user_name: str, shutoff_table: str, **email_kwargs):
         template_params={
             "username": user_name,
             "shutoff_table": shutoff_table,
+            "days_threshold": days_threshold,
         },
     )
 
@@ -57,6 +66,7 @@ def send_shutoff_vm_email(
     smtp_account: SMTPAccount,
     cloud_account: Union[CloudDomains, str],
     limit_by_projects: Optional[List[str]] = None,
+    days_threshold: int = 60,
     all_projects: bool = False,
     as_html: bool = False,
     send_email: bool = False,
@@ -71,6 +81,7 @@ def send_shutoff_vm_email(
     :param smtp_account: (SMTPAccount): SMTP config
     :param cloud_account: String representing the cloud account to use
     :param limit_by_projects: A list of project names or ids to limit search in
+    :param days_threshold: An integer which specifies the minimum age (in days) of the servers to be found
     :param all_projects: A boolean which, if True, will search in all projects
     :param send_email: A boolean which, if True, will send the email instead of printing what will be sent
     :param as_html: A boolean which, if True, will send the email as html
@@ -88,7 +99,9 @@ def send_shutoff_vm_email(
             "please provide either a list of project identifiers or with flag 'all_projects' to run globally"
         )
 
-    server_query = find_shutoff_servers(cloud_account, limit_by_projects)
+    server_query = find_shutoff_servers(
+        cloud_account, days_threshold, limit_by_projects
+    )
 
     if not server_query.to_props():
         raise RuntimeError(
@@ -116,14 +129,17 @@ def send_shutoff_vm_email(
             )
 
         if not send_email:
-            print_email_params(send_to[0], user_name, as_html, server_list)
+            print_email_params(
+                send_to[0], user_name, as_html, server_list, days_threshold
+            )
             continue
 
         email_params = build_email_params(
             user_name,
             server_list,
             email_to=send_to,
             as_html=as_html,
+            days_threshold=days_threshold,
             email_cc=("cloud-support@stfc.ac.uk",) if cc_cloud_support else None,
             **email_params_kwargs,
         )

tests/lib/apis/openstack_query_api/test_server_queries.py

@@ -201,7 +201,30 @@ def test_find_servers_with_shutoff_vms_valid(mock_server_query):
     """
     mock_server_query_obj = mock_server_query.return_value
 
-    res = find_shutoff_servers("test-cloud-account", ["project1", "project2"])
+    res = find_shutoff_servers("test-cloud-account", -1, ["project1", "project2"])
+
+    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.append_from.assert_called_once_with(
+        "PROJECT_QUERY", "test-cloud-account", ["name"]
+    )
+    assert res == mock_server_query_obj
+
+
+@patch("apis.openstack_query_api.server_queries.ServerQuery")
+def test_find_servers_with_shutoff_vms_valid_age(mock_server_query):
+    """
+    Tests find_servers_with_shutoff_vms() function when filtering by minimum server age
+    """
+    mock_server_query_obj = mock_server_query.return_value
+
+    res = find_shutoff_servers("test-cloud-account", 10, ["project1", "project2"])
 
     mock_server_query_obj.run.assert_called_once_with(
         "test-cloud-account",