From 82a2be53d43b42e00f3a00ac21e3633896885ceb Mon Sep 17 00:00:00 2001 From: Lars Karlitski Date: Fri, 13 Dec 2019 19:31:20 +0100 Subject: [PATCH] pipeline: return logs in --json mode A pipeline run only returned logs in the `StageFailed` and `AssemblerFailed` exceptions. Remove those and always return structured data instead. It only returns data for stages that actually ran (i.e., didn't come from the cache). This is similar to the output in interactive mode. Also change osbuildtest to be able to deal with output that is larger than the pipe buffer by using subprocess.communicate(). --- osbuild/__init__.py | 4 +- osbuild/__main__.py | 23 +++++------ osbuild/pipeline.py | 95 ++++++++++++++++++++++++--------------------- test/osbuildtest.py | 18 ++++----- 4 files changed, 67 insertions(+), 73 deletions(-) diff --git a/osbuild/__init__.py b/osbuild/__init__.py index 859334af..49007abf 100644 --- a/osbuild/__init__.py +++ b/osbuild/__init__.py @@ -1,13 +1,11 @@ -from .pipeline import Assembler, AssemblerFailed, load, load_build, Pipeline, Stage, StageFailed +from .pipeline import Assembler, load, load_build, Pipeline, Stage __all__ = [ "Assembler", - "AssemblerFailed", "load", "load_build", "Pipeline", "Stage", - "StageFailed", ] diff --git a/osbuild/__main__.py b/osbuild/__main__.py index 49582cc3..94c46822 100755 --- a/osbuild/__main__.py +++ b/osbuild/__main__.py @@ -38,29 +38,24 @@ def main(): pipeline.prepend_build_env(build_pipeline, runner) try: - pipeline.run(args.store, interactive=not args.json, libdir=args.libdir) + r = pipeline.run(args.store, interactive=not args.json, libdir=args.libdir) except KeyboardInterrupt: print() print(f"{RESET}{BOLD}{RED}Aborted{RESET}") return 130 - except (osbuild.StageFailed, osbuild.AssemblerFailed) as error: - print() - print(f"{RESET}{BOLD}{RED}{error.name} failed with code {error.returncode}{RESET}") - if args.json: - print(error.output) - return 1 if args.json: - json.dump({ - "tree_id": pipeline.tree_id, - "output_id": pipeline.output_id, - }, sys.stdout) + json.dump(r, sys.stdout) sys.stdout.write("\n") else: - print("tree id:", pipeline.tree_id) - print("output id:", pipeline.output_id) + if r["success"]: + print("tree id:", pipeline.tree_id) + print("output id:", pipeline.output_id) + else: + print() + print(f"{RESET}{BOLD}{RED}Failed{RESET}") - return 0 + return 0 if r["success"] else 1 if __name__ == "__main__": diff --git a/osbuild/pipeline.py b/osbuild/pipeline.py index 6f2aff38..ffb97b1c 100644 --- a/osbuild/pipeline.py +++ b/osbuild/pipeline.py @@ -17,22 +17,6 @@ RESET = "\033[0m" BOLD = "\033[1m" -class StageFailed(Exception): - def __init__(self, name, returncode, output): - super(StageFailed, self).__init__() - self.name = name - self.returncode = returncode - self.output = output - - -class AssemblerFailed(Exception): - def __init__(self, name, returncode, output): - super(AssemblerFailed, self).__init__() - self.name = name - self.returncode = returncode - self.output = output - - def print_header(title, options): print() print(f"{RESET}{BOLD}{title}{RESET} " + json.dumps(options or {}, indent=2)) @@ -62,7 +46,7 @@ class Stage: description["options"] = self.options return description - def run(self, tree, runner, build_tree, interactive=False, check=True, libdir=None): + def run(self, tree, runner, build_tree, interactive=False, libdir=None): with buildroot.BuildRoot(build_tree, runner, libdir=libdir) as build_root: if interactive: print_header(f"{self.name}: {self.id}", self.options) @@ -79,8 +63,13 @@ class Stage: binds=[f"{tree}:/run/osbuild/tree"], stdin=subprocess.DEVNULL, ) - if check and r.returncode != 0: - raise StageFailed(self.name, r.returncode, api.output) + return { + "name": self.name, + "id": self.id, + "options": self.options, + "success": r.returncode == 0, + "output": api.output + } return r.returncode == 0 @@ -108,7 +97,7 @@ class Assembler: description["options"] = self.options return description - def run(self, tree, runner, build_tree, output_dir=None, interactive=False, check=True, libdir=None): + def run(self, tree, runner, build_tree, output_dir=None, interactive=False, libdir=None): with buildroot.BuildRoot(build_tree, runner, libdir=libdir) as build_root: if interactive: print_header(f"Assembler {self.name}: {self.id}", self.options) @@ -141,10 +130,13 @@ class Assembler: readonly_binds=ro_binds, stdin=subprocess.DEVNULL, ) - if check and r.returncode != 0: - raise AssemblerFailed(self.name, r.returncode, api.output) - - return r.returncode == 0 + return { + "name": self.name, + "id": self.id, + "options": self.options, + "success": r.returncode == 0, + "output": api.output + } class Pipeline: @@ -206,13 +198,14 @@ class Pipeline: finally: subprocess.run(["umount", "--lazy", tmp], check=True) - def run(self, store, interactive=False, check=True, libdir=None): + def run(self, store, interactive=False, libdir=None): os.makedirs("/run/osbuild", exist_ok=True) object_store = objectstore.ObjectStore(store) if self.build: - if not self.build.run(store, interactive, check, libdir): + if not self.build.run(store, interactive, libdir): return False + results = {} with self.get_buildtree(object_store) as build_tree: if self.stages: if not object_store.contains(self.tree_id): @@ -225,34 +218,46 @@ class Pipeline: base = self.stages[i].id base_idx = i break + # The tree does not exist. Create it and save it to the object store. If # two run() calls race each-other, two trees may be generated, and it # is nondeterministic which of them will end up referenced by the tree_id # in the content store. However, we guarantee that all tree_id's and all # generated trees remain valid. + results["stages"] = [] with object_store.new(self.tree_id, base_id=base) as tree: for stage in self.stages[base_idx + 1:]: - if not stage.run(tree, - self.runner, - build_tree, - interactive=interactive, - check=check, - libdir=libdir): - return False + r = stage.run(tree, + self.runner, + build_tree, + interactive=interactive, + libdir=libdir) + results["stages"].append(r) + if not r["success"]: + results["success"] = False + return results - if self.assembler and not object_store.contains(self.output_id): - with object_store.get(self.tree_id) as tree, \ - object_store.new(self.output_id) as output_dir: - if not self.assembler.run(tree, - self.runner, - build_tree, - output_dir=output_dir, - interactive=interactive, - check=check, - libdir=libdir): - return False + results["tree_id"] = self.tree_id - return True + if self.assembler: + if not object_store.contains(self.output_id): + with object_store.get(self.tree_id) as tree, \ + object_store.new(self.output_id) as output_dir: + r = self.assembler.run(tree, + self.runner, + build_tree, + output_dir=output_dir, + interactive=interactive, + libdir=libdir) + results["assembler"] = r + if not r["success"]: + results["success"] = False + return results + + results["output_id"] = self.output_id + + results["success"] = True + return results def load_build(description): diff --git a/test/osbuildtest.py b/test/osbuildtest.py index 7a29c55f..e8c5db3d 100644 --- a/test/osbuildtest.py +++ b/test/osbuildtest.py @@ -42,23 +42,19 @@ class TestCase(unittest.TestCase): stdin = subprocess.PIPE if input else None - p = subprocess.Popen(osbuild_cmd, encoding="utf-8", stdin=stdin, stdout=subprocess.PIPE) - if input: - p.stdin.write(input) - p.stdin.close() + p = subprocess.Popen(osbuild_cmd, encoding="utf-8", stdin=stdin, stdout=subprocess.PIPE, stderr=subprocess.STDOUT) try: - r = p.wait() - if r != 0: - print(p.stdout.read()) - self.assertEqual(r, 0) + output, _ = p.communicate(input) + if p.returncode != 0: + print(output) + self.assertEqual(p.returncode, 0) except KeyboardInterrupt: # explicitly wait again to let osbuild clean up p.wait() raise - result = json.load(p.stdout) - p.stdout.close() - return result["tree_id"], result["output_id"] + result = json.loads(output) + return result.get("tree_id"), result.get("output_id") def run_tree_diff(self, tree1, tree2): tree_diff_cmd = ["./tree-diff", tree1, tree2]