Merge ~igor-brovtsin/maas:lp-1999064 into maas:master

[Igor Brovtsin]

fix(LP#1999064): add cleanup to `maas_run_scripts.py` when no scripts failed

[MAAS Lander (maas-lander)]

UNIT TESTS
-b lp-1999064 lp:~igor-brovtsin/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: SUCCESS
COMMIT: 1dbaa02b76b61b54e5c455ec0aa950ca26d8ec76

review: Approve

[Adam Collard (adam-collard)]

review: Needs Information

[Adam Collard (adam-collard)]

Let's refactor that script so that ScriptsPaths is always given a directory that exists, the caller is responsible for making a directory if needed, and cleanup afterwards.

Always clean up the directory, even if scripts fail - MAAS gets told what the errors are, and this is just a scratch directory that we use to download and execute.

96028f5... by Igor Brovtsin on 2022-12-08

Made `base_path` a required argument for `ScriptsPaths`

So that directory creation and deletion is a responsibility of the
called

[MAAS Lander (maas-lander)]

UNIT TESTS
-b lp-1999064 lp:~igor-brovtsin/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: FAILED
LOG: http://maas-ci.internal:8080/job/maas-tester/1575/consoleText
COMMIT: 96028f5acecab2186dfdeb6e3f3c39f033164e28

review: Needs Fixing

ed4fefd... by Igor Brovtsin on 2022-12-09

Fixed missing `Path()` wrapper around temp dir path

[MAAS Lander (maas-lander)]

UNIT TESTS
-b lp-1999064 lp:~igor-brovtsin/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: SUCCESS
COMMIT: ed4fefdabab16d1bffc7cb282a298cb7b0adf12b

review: Approve

1959303... by Igor Brovtsin on 2022-12-09

Use `TemporaryDirectory` context manager

[MAAS Lander (maas-lander)]

UNIT TESTS
-b lp-1999064 lp:~igor-brovtsin/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: SUCCESS
COMMIT: 19593037aa1ad8ce52acaccf92b4ba1efa6d3837

review: Approve

[Adam Collard (adam-collard)]

review: Approve

diff --git a/src/metadataserver/user_data/templates/snippets/maas_run_scripts.py b/src/metadataserver/user_data/templates/snippets/maas_run_scripts.py
index 09e8855..b073771 100644
--- a/src/metadataserver/user_data/templates/snippets/maas_run_scripts.py
+++ b/src/metadataserver/user_data/templates/snippets/maas_run_scripts.py
@@ -18,7 +18,7 @@ import shutil
 import subprocess
 import sys
 import tarfile
-from tempfile import mkdtemp
+from tempfile import TemporaryDirectory
 import time
 import urllib.error
 
@@ -46,9 +46,7 @@ class ExitError(Exception):
 
 
 class ScriptsPaths:
-    def __init__(self, base_path=None):
-        if base_path is None:
-            base_path = Path(mkdtemp())
+    def __init__(self, base_path):
         self.scripts = base_path / "scripts"
         self.out = base_path / "out"
         self.downloads = base_path / "downloads"
@@ -375,52 +373,54 @@ def action_report_results(ns):
     if not config.metadata_url:
         raise ExitError("No MAAS URL set")
 
-    paths = ScriptsPaths()
-    paths.ensure()
+    with TemporaryDirectory() as base_path:
+        paths = ScriptsPaths(Path(base_path))
+        paths.ensure()
 
-    maas_url = get_base_url(config.metadata_url)
-    metadata_url = maas_url + "/MAAS/metadata/" + MD_VERSION + "/"
+        maas_url = get_base_url(config.metadata_url)
+        metadata_url = maas_url + "/MAAS/metadata/" + MD_VERSION + "/"
 
-    print(
-        "* Fetching scripts from {url} to {dir}".format(
-            url=metadata_url, dir=paths.scripts
-        )
-    )
-    for script in fetch_scripts(
-        maas_url, metadata_url, paths, config.credentials
-    ):
-        if not script.should_run():
-            continue
         print(
-            f"* Running '{script.name}'...",
-            end="\n" if ns.debug else " ",
+            "* Fetching scripts from {url} to {dir}".format(
+                url=metadata_url, dir=paths.scripts
+            )
         )
-        result = script.run(console_output=ns.debug)
-        if ns.debug:
+
+        for script in fetch_scripts(
+            maas_url, metadata_url, paths, config.credentials
+        ):
+            if not script.should_run():
+                continue
             print(
-                f"* Finished running '{script.name}': ",
-                end=" ",
+                f"* Running '{script.name}'...",
+                end="\n" if ns.debug else " ",
             )
-        if result.exit_status == 0:
-            print("success")
-        else:
-            print(
-                "FAILED (status {result.exit_status}): {result.error}".format(
-                    result=result
+            result = script.run(console_output=ns.debug)
+            if ns.debug:
+                print(
+                    f"* Finished running '{script.name}': ",
+                    end=" ",
                 )
+            if result.exit_status == 0:
+                print("success")
+            else:
+                print(
+                    "FAILED (status {result.exit_status}): {result.error}".format(
+                        result=result
+                    )
+                )
+            signal(
+                metadata_url,
+                config.credentials,
+                result.status,
+                error=result.error,
+                script_name=script.name,
+                script_result_id=script.info.get("script_result_id"),
+                files=result.result_files,
+                runtime=result.runtime,
+                exit_status=result.exit_status,
+                script_version_id=script.info.get("script_version_id"),
             )
-        signal(
-            metadata_url,
-            config.credentials,
-            result.status,
-            error=result.error,
-            script_name=script.name,
-            script_result_id=script.info.get("script_result_id"),
-            files=result.result_files,
-            runtime=result.runtime,
-            exit_status=result.exit_status,
-            script_version_id=script.info.get("script_version_id"),
-        )
 
 
 def action_register_machine(ns):