From 481213a8dd8a69a94cd72dae6d18c782becf8a1c Mon Sep 17 00:00:00 2001 From: Tom Gundersen Date: Mon, 10 Feb 2020 08:29:21 +0100 Subject: [PATCH] pipeline: pin the sources options in the pipeline object Make the sources options a static property of the pipeline, in particular of each stage, rather than being passed in on `run()`. This more closely matches the intended semantics of sources and pipeline having similar lifetimes and being fairly coupled together. The difference between the pipeline and the sources is that the sources do not contribute to identifying the pipeline (they are not part of the hash for the pipeline id), and they could be swapped out without changing the output image (as long as they are valid). However, a pipeline without A sources object would not be useful, and typically the pipeline and the sources are generated, passed around and used together. This is different from the build environment and the secrets object, which both are specific to either the host or the caller, unlike the pipeline which should be universal. This changes the `load()` function to take a `manifest`, which is a map containing both the pipeline and the sources. Note that the semantics of the build-env parameter remains unchanged: It shares the sources with the rest of the pipeline. We may want to reconsider this in future commits, as the build-env is specific to the host, whereas the regular pipeline is not. Signed-off-by: Tom Gundersen --- osbuild/__main__.py | 17 +++++++++-------- osbuild/pipeline.py | 25 ++++++++++++------------- test/test_osbuild.py | 12 ++++++------ 3 files changed, 27 insertions(+), 27 deletions(-) diff --git a/osbuild/__main__.py b/osbuild/__main__.py index 9f087e9a..06ef1dc3 100755 --- a/osbuild/__main__.py +++ b/osbuild/__main__.py @@ -54,19 +54,21 @@ def main(): f = sys.stdin else: f = open(args.pipeline_path) - pipeline = osbuild.load(json.load(f)) + pipeline = json.load(f) f.close() + sources_options = {} + if args.sources: + with open(args.sources) as f: + sources_options = json.load(f) + + pipeline = osbuild.load(pipeline, sources_options) + if args.build_env: with open(args.build_env) as f: - build_pipeline, runner = osbuild.load_build(json.load(f)) + build_pipeline, runner = osbuild.load_build(json.load(f), sources_options) pipeline.prepend_build_env(build_pipeline, runner) - source_options = {} - if args.sources: - with open(args.sources) as f: - source_options = json.load(f) - secrets = {} if args.secrets: with open(args.secrets) as f: @@ -85,7 +87,6 @@ def main(): args.store, interactive=not args.json, libdir=args.libdir, - source_options=source_options, secrets=secrets ) except KeyboardInterrupt: diff --git a/osbuild/pipeline.py b/osbuild/pipeline.py index 5d3ccb6d..a435141c 100644 --- a/osbuild/pipeline.py +++ b/osbuild/pipeline.py @@ -46,8 +46,9 @@ def print_header(title, options): class Stage: - def __init__(self, name, build, base, options): + def __init__(self, name, source_options, build, base, options): self.name = name + self.sources = source_options self.build = build self.base = base self.options = options @@ -77,7 +78,6 @@ class Stage: interactive=False, libdir=None, var="/var/tmp", - source_options=None, secrets=None): with buildroot.BuildRoot(build_tree, runner, libdir=libdir, var=var) as build_root, \ tempfile.TemporaryDirectory(prefix="osbuild-sources-output-", dir=var) as sources_output: @@ -103,7 +103,7 @@ class Stage: with API(f"{build_root.api}/osbuild", args, interactive) as api, \ sources.SourcesServer(f"{build_root.api}/sources", sources_dir, - source_options, + self.sources, f"{cache}/sources", sources_output, secrets): @@ -194,9 +194,9 @@ class Pipeline: def output_id(self): return self.assembler.id if self.assembler else None - def add_stage(self, name, options=None): + def add_stage(self, name, sources_options=None, options=None): build = self.build.tree_id if self.build else None - stage = Stage(name, build, self.tree_id, options or {}) + stage = Stage(name, sources_options, build, self.tree_id, options or {}) self.stages.append(stage) if self.assembler: self.assembler.base = stage.id @@ -238,13 +238,13 @@ class Pipeline: finally: subprocess.run(["umount", "--lazy", tmp], check=True) - def run(self, store, interactive=False, libdir=None, source_options=None, secrets=None): + def run(self, store, interactive=False, libdir=None, secrets=None): os.makedirs("/run/osbuild", exist_ok=True) object_store = objectstore.ObjectStore(store) results = {} if self.build: - r = self.build.run(store, interactive, libdir, source_options, secrets) + r = self.build.run(store, interactive, libdir, secrets) results["build"] = r if not r["success"]: results["success"] = False @@ -279,7 +279,6 @@ class Pipeline: interactive=interactive, libdir=libdir, var=store, - source_options=source_options, secrets=secrets) if stage.checkpoint: object_store.snapshot(tree, stage.id) @@ -315,27 +314,27 @@ class Pipeline: return results -def load_build(description): +def load_build(description, sources_options): pipeline = description.get("pipeline") if pipeline: - build_pipeline = load(pipeline) + build_pipeline = load(pipeline, sources_options) else: build_pipeline = None return build_pipeline, description["runner"] -def load(description): +def load(description, sources_options): build = description.get("build") if build: - build_pipeline, runner = load_build(build) + build_pipeline, runner = load_build(build, sources_options) else: build_pipeline, runner = None, "org.osbuild.host" pipeline = Pipeline(runner, build_pipeline) for s in description.get("stages", []): - pipeline.add_stage(s["name"], s.get("options", {})) + pipeline.add_stage(s["name"], sources_options, s.get("options", {})) a = description.get("assembler") if a: diff --git a/test/test_osbuild.py b/test/test_osbuild.py index f26a6488..32ae76b1 100644 --- a/test/test_osbuild.py +++ b/test/test_osbuild.py @@ -19,15 +19,15 @@ class TestDescriptions(unittest.TestCase): ] for pipeline in cases: with self.subTest(pipeline): - self.assertEqual(osbuild.load(pipeline).description(), {}) + self.assertEqual(osbuild.load(pipeline, {}).description(), {}) def test_stage(self): name = "org.osbuild.test" options = { "one": 1 } cases = [ - (osbuild.Stage(name, None, None, {}), {"name": name}), - (osbuild.Stage(name, None, None, None), {"name": name}), - (osbuild.Stage(name, None, None, options), {"name": name, "options": options}), + (osbuild.Stage(name, {}, None, None, {}), {"name": name}), + (osbuild.Stage(name, {}, None, None, None), {"name": name}), + (osbuild.Stage(name, {}, None, None, options), {"name": name, "options": options}), ] for stage, description in cases: with self.subTest(description): @@ -47,10 +47,10 @@ class TestDescriptions(unittest.TestCase): def test_pipeline(self): build = osbuild.Pipeline("org.osbuild.test") - build.add_stage("org.osbuild.test", { "one": 1 }) + build.add_stage("org.osbuild.test", {}, { "one": 1 }) pipeline = osbuild.Pipeline("org.osbuild.test", build) - pipeline.add_stage("org.osbuild.test", { "one": 2 }) + pipeline.add_stage("org.osbuild.test", {}, { "one": 2 }) pipeline.set_assembler("org.osbuild.test") self.assertEqual(pipeline.description(), {