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

Proposed by Igor Brovtsin
Status: Merged
Approved by: Igor Brovtsin
Approved revision: 19593037aa1ad8ce52acaccf92b4ba1efa6d3837
Merge reported by: MAAS Lander
Merged at revision: not available
Proposed branch: ~igor-brovtsin/maas:lp-1999064
Merge into: maas:master
Diff against target: 117 lines (+42/-42)
1 file modified
src/metadataserver/user_data/templates/snippets/maas_run_scripts.py (+42/-42)
Reviewer Review Type Date Requested Status
Adam Collard (community) Approve
MAAS Lander Approve
Review via email: mp+434256@code.launchpad.net

Commit message

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

To post a comment you must log in.
Revision history for this message
MAAS Lander (maas-lander) wrote :

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

STATUS: SUCCESS
COMMIT: 1dbaa02b76b61b54e5c455ec0aa950ca26d8ec76

review: Approve
Revision history for this message
Adam Collard (adam-collard) :
review: Needs Information
Revision history for this message
Adam Collard (adam-collard) wrote :

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.

~igor-brovtsin/maas:lp-1999064 updated
96028f5... by Igor Brovtsin

Made `base_path` a required argument for `ScriptsPaths`

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

Revision history for this message
MAAS Lander (maas-lander) wrote :

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
~igor-brovtsin/maas:lp-1999064 updated
ed4fefd... by Igor Brovtsin

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

Revision history for this message
MAAS Lander (maas-lander) wrote :

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

STATUS: SUCCESS
COMMIT: ed4fefdabab16d1bffc7cb282a298cb7b0adf12b

review: Approve
~igor-brovtsin/maas:lp-1999064 updated
1959303... by Igor Brovtsin

Use `TemporaryDirectory` context manager

Revision history for this message
MAAS Lander (maas-lander) wrote :

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

STATUS: SUCCESS
COMMIT: 19593037aa1ad8ce52acaccf92b4ba1efa6d3837

review: Approve
Revision history for this message
Adam Collard (adam-collard) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
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
18import subprocess18import subprocess
19import sys19import sys
20import tarfile20import tarfile
21from tempfile import mkdtemp21from tempfile import TemporaryDirectory
22import time22import time
23import urllib.error23import urllib.error
2424
@@ -46,9 +46,7 @@ class ExitError(Exception):
4646
4747
48class ScriptsPaths:48class ScriptsPaths:
49 def __init__(self, base_path=None):49 def __init__(self, base_path):
50 if base_path is None:
51 base_path = Path(mkdtemp())
52 self.scripts = base_path / "scripts"50 self.scripts = base_path / "scripts"
53 self.out = base_path / "out"51 self.out = base_path / "out"
54 self.downloads = base_path / "downloads"52 self.downloads = base_path / "downloads"
@@ -375,52 +373,54 @@ def action_report_results(ns):
375 if not config.metadata_url:373 if not config.metadata_url:
376 raise ExitError("No MAAS URL set")374 raise ExitError("No MAAS URL set")
377375
378 paths = ScriptsPaths()376 with TemporaryDirectory() as base_path:
379 paths.ensure()377 paths = ScriptsPaths(Path(base_path))
378 paths.ensure()
380379
381 maas_url = get_base_url(config.metadata_url)380 maas_url = get_base_url(config.metadata_url)
382 metadata_url = maas_url + "/MAAS/metadata/" + MD_VERSION + "/"381 metadata_url = maas_url + "/MAAS/metadata/" + MD_VERSION + "/"
383382
384 print(
385 "* Fetching scripts from {url} to {dir}".format(
386 url=metadata_url, dir=paths.scripts
387 )
388 )
389 for script in fetch_scripts(
390 maas_url, metadata_url, paths, config.credentials
391 ):
392 if not script.should_run():
393 continue
394 print(383 print(
395 f"* Running '{script.name}'...",384 "* Fetching scripts from {url} to {dir}".format(
396 end="\n" if ns.debug else " ",385 url=metadata_url, dir=paths.scripts
386 )
397 )387 )
398 result = script.run(console_output=ns.debug)388
399 if ns.debug:389 for script in fetch_scripts(
390 maas_url, metadata_url, paths, config.credentials
391 ):
392 if not script.should_run():
393 continue
400 print(394 print(
401 f"* Finished running '{script.name}': ",395 f"* Running '{script.name}'...",
402 end=" ",396 end="\n" if ns.debug else " ",
403 )397 )
404 if result.exit_status == 0:398 result = script.run(console_output=ns.debug)
405 print("success")399 if ns.debug:
406 else:400 print(
407 print(401 f"* Finished running '{script.name}': ",
408 "FAILED (status {result.exit_status}): {result.error}".format(402 end=" ",
409 result=result
410 )403 )
404 if result.exit_status == 0:
405 print("success")
406 else:
407 print(
408 "FAILED (status {result.exit_status}): {result.error}".format(
409 result=result
410 )
411 )
412 signal(
413 metadata_url,
414 config.credentials,
415 result.status,
416 error=result.error,
417 script_name=script.name,
418 script_result_id=script.info.get("script_result_id"),
419 files=result.result_files,
420 runtime=result.runtime,
421 exit_status=result.exit_status,
422 script_version_id=script.info.get("script_version_id"),
411 )423 )
412 signal(
413 metadata_url,
414 config.credentials,
415 result.status,
416 error=result.error,
417 script_name=script.name,
418 script_result_id=script.info.get("script_result_id"),
419 files=result.result_files,
420 runtime=result.runtime,
421 exit_status=result.exit_status,
422 script_version_id=script.info.get("script_version_id"),
423 )
424424
425425
426def action_register_machine(ns):426def action_register_machine(ns):

Subscribers

People subscribed via source and target branches