Colin Watson has proposed merging ~cjwatson/launchpad-buildd:lpcraft-common-output-directory into launchpad-buildd:master.
Commit message: Use a common output directory for all lpcraft jobs Requested reviews: Launchpad code reviewers (launchpad-reviewers) For more details, see: https://code.launchpad.net/~cjwatson/launchpad-buildd/+git/launchpad-buildd/+merge/427734 In order to implement the `input` keyword in lpcraft, launchpad-buildd needs to use the same top-level output directory for each job so that lpcraft can find artifacts from previously-executed jobs. This is problematic with the current arrangements, because launchpad-buildd has to pass different `--output-directory` options for each individual job so that it can accurately determine which output artifacts belong to which job. However, with https://code.launchpad.net/~cjwatson/lpcraft/+git/lpcraft/+merge/427724, it becomes possible for launchpad-buildd to identify artifacts by job name and index within a single common output directory, so we can take advantage of that by looking for output files more precisely. This also gathers the `properties` file (currently unused) in a slightly different way, preparing for Launchpad to extract those output properties and store them in the database in a way that can conveniently be used downstream. This mustn't be merged until https://code.launchpad.net/~cjwatson/lpcraft/+git/lpcraft/+merge/427724 has been released to the `stable` channel. -- Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad-buildd:lpcraft-common-output-directory into launchpad-buildd:master.
diff --git a/debian/changelog b/debian/changelog index 810b64d..679201c 100644 --- a/debian/changelog +++ b/debian/changelog @@ -1,3 +1,9 @@ +launchpad-buildd (218) UNRELEASED; urgency=medium + + * Use a common output directory for all lpcraft jobs. + + -- Colin Watson <[email protected]> Tue, 02 Aug 2022 15:55:19 +0100 + launchpad-buildd (217) focal; urgency=medium [ Colin Watson ] diff --git a/lpbuildd/ci.py b/lpbuildd/ci.py index ac6c585..fcfd8d4 100644 --- a/lpbuildd/ci.py +++ b/lpbuildd/ci.py @@ -241,16 +241,20 @@ class CIBuildManager(BuildManagerProxyMixin, DebianBuildManager): This is called once for each CI job in the pipeline. """ job_status = {} - output_path = os.path.join("/build", "output", self.current_job_id) - log_path = "%s.log" % output_path - if self.backend.path_exists(log_path): - log_name = "%s.log" % self.current_job_id - self.addWaitingFileFromBackend(log_path, log_name) - job_status["log"] = self._builder.waitingfiles[log_name] - if self.backend.path_exists(output_path): + job_name, job_index = self.current_job + job_output_path = os.path.join( + "/build", "output", job_name, str(job_index)) + for item_name in ("log", "properties"): + item_path = os.path.join(job_output_path, item_name) + if self.backend.path_exists(item_path): + item_id = f"{self.current_job_id}.{item_name}" + self.addWaitingFileFromBackend(item_path, name=item_id) + job_status[item_name] = self._builder.waitingfiles[item_id] + files_path = os.path.join(job_output_path, "files") + if self.backend.path_exists(files_path): for entry in sorted(self.backend.find( - output_path, include_directories=False)): - path = os.path.join(output_path, entry) + files_path, include_directories=False)): + path = os.path.join(files_path, entry) if self.backend.islink(path): continue entry_base = os.path.basename(entry) diff --git a/lpbuildd/target/run_ci.py b/lpbuildd/target/run_ci.py index 75deaf9..7a8fa3b 100644 --- a/lpbuildd/target/run_ci.py +++ b/lpbuildd/target/run_ci.py @@ -127,8 +127,11 @@ class RunCI(BuilderProxyOperationMixin, Operation): env = self.build_proxy_environment(proxy_url=self.args.proxy_url) job_id = f"{self.args.job_name}:{self.args.job_index}" logger.info("Running %s" % job_id) - output_path = os.path.join("/build", "output", job_id) - self.backend.run(["mkdir", "-p", output_path]) + output_path = os.path.join("/build", "output") + # This matches the per-job output path used by lpcraft. + job_output_path = os.path.join( + output_path, self.args.job_name, str(self.args.job_index)) + self.backend.run(["mkdir", "-p", job_output_path]) lpcraft_args = [ "lpcraft", "-v", @@ -161,7 +164,7 @@ class RunCI(BuilderProxyOperationMixin, Operation): escaped_lpcraft_args = ( " ".join(shell_escape(arg) for arg in lpcraft_args)) - tee_args = ["tee", "%s.log" % output_path] + tee_args = ["tee", os.path.join(job_output_path, "log")] escaped_tee_args = " ".join(shell_escape(arg) for arg in tee_args) args = [ "/bin/bash", "-o", "pipefail", "-c", diff --git a/lpbuildd/target/tests/test_run_ci.py b/lpbuildd/target/tests/test_run_ci.py index 2f18bd5..ed6e0a2 100644 --- a/lpbuildd/target/tests/test_run_ci.py +++ b/lpbuildd/target/tests/test_run_ci.py @@ -316,11 +316,11 @@ class TestRunCI(TestCase): run_ci = parse_args(args=args).operation run_ci.run_job() self.assertThat(run_ci.backend.run.calls, MatchesListwise([ - RanCommand(["mkdir", "-p", "/build/output/test:0"]), + RanCommand(["mkdir", "-p", "/build/output/test/0"]), RanBuildCommand([ "/bin/bash", "-o", "pipefail", "-c", - "lpcraft -v run-one --output-directory /build/output/test:0 test 0 2>&1 " # noqa: E501 - "| tee /build/output/test:0.log", + "lpcraft -v run-one --output-directory /build/output test 0 2>&1 " # noqa: E501 + "| tee /build/output/test/0/log", ], cwd="/build/tree"), ])) @@ -340,11 +340,11 @@ class TestRunCI(TestCase): "SNAPPY_STORE_NO_CDN": "1", } self.assertThat(run_ci.backend.run.calls, MatchesListwise([ - RanCommand(["mkdir", "-p", "/build/output/test:0"]), + RanCommand(["mkdir", "-p", "/build/output/test/0"]), RanBuildCommand([ "/bin/bash", "-o", "pipefail", "-c", - "lpcraft -v run-one --output-directory /build/output/test:0 test 0 2>&1 " # noqa: E501 - "| tee /build/output/test:0.log", + "lpcraft -v run-one --output-directory /build/output test 0 2>&1 " # noqa: E501 + "| tee /build/output/test/0/log", ], cwd="/build/tree", **env), ])) @@ -359,15 +359,15 @@ class TestRunCI(TestCase): run_ci = parse_args(args=args).operation run_ci.run_job() self.assertThat(run_ci.backend.run.calls, MatchesListwise([ - RanCommand(["mkdir", "-p", "/build/output/test:0"]), + RanCommand(["mkdir", "-p", "/build/output/test/0"]), RanBuildCommand([ "/bin/bash", "-o", "pipefail", "-c", - "lpcraft -v run-one --output-directory /build/output/test:0 " + "lpcraft -v run-one --output-directory /build/output " "test 0 " "--set-env PIP_INDEX_URL=http://example " "--set-env SOME_PATH=/etc/some_path " "2>&1 " - "| tee /build/output/test:0.log", + "| tee /build/output/test/0/log", ], cwd="/build/tree"), ])) @@ -384,15 +384,15 @@ class TestRunCI(TestCase): run_ci = parse_args(args=args).operation run_ci.run_job() self.assertThat(run_ci.backend.run.calls, MatchesListwise([ - RanCommand(["mkdir", "-p", "/build/output/test:0"]), + RanCommand(["mkdir", "-p", "/build/output/test/0"]), RanBuildCommand([ "/bin/bash", "-o", "pipefail", "-c", - "lpcraft -v run-one --output-directory /build/output/test:0 " + "lpcraft -v run-one --output-directory /build/output " "test 0 " "--apt-replace-repositories 'deb http://archive.ubuntu.com/ubuntu/ focal main restricted' " # noqa: E501 "--apt-replace-repositories 'deb http://archive.ubuntu.com/ubuntu/ focal universe' " # noqa: E501 "2>&1 " - "| tee /build/output/test:0.log", + "| tee /build/output/test/0/log", ], cwd="/build/tree"), ])) @@ -407,15 +407,15 @@ class TestRunCI(TestCase): run_ci = parse_args(args=args).operation run_ci.run_job() self.assertThat(run_ci.backend.run.calls, MatchesListwise([ - RanCommand(["mkdir", "-p", "/build/output/test:0"]), + RanCommand(["mkdir", "-p", "/build/output/test/0"]), RanBuildCommand([ "/bin/bash", "-o", "pipefail", "-c", - "lpcraft -v run-one --output-directory /build/output/test:0 " + "lpcraft -v run-one --output-directory /build/output " "test 0 " "--plugin-setting " "miniconda_conda_channel=https://user:[email protected]/artifactory/soss-conda-stable-local/ " # noqa: E501 "2>&1 " - "| tee /build/output/test:0.log", + "| tee /build/output/test/0/log", ], cwd="/build/tree"), ])) @@ -429,14 +429,14 @@ class TestRunCI(TestCase): run_ci = parse_args(args=args).operation run_ci.run_job() self.assertThat(run_ci.backend.run.calls, MatchesListwise([ - RanCommand(["mkdir", "-p", "/build/output/test:0"]), + RanCommand(["mkdir", "-p", "/build/output/test/0"]), RanBuildCommand([ "/bin/bash", "-o", "pipefail", "-c", - "lpcraft -v run-one --output-directory /build/output/test:0 " + "lpcraft -v run-one --output-directory /build/output " "test 0 " "--secrets /build/.launchpad-secrets.yaml " "2>&1 " - "| tee /build/output/test:0.log", + "| tee /build/output/test/0/log", ], cwd="/build/tree"), ])) @@ -451,7 +451,7 @@ class TestRunCI(TestCase): # Just check that it did something in each step, not every detail. self.assertThat( run_ci.backend.run.calls, - AnyMatch(RanCommand(["mkdir", "-p", "/build/output/test:0"]))) + AnyMatch(RanCommand(["mkdir", "-p", "/build/output/test/0"]))) def test_run_install_fails(self): class FailInstall(FakeMethod): diff --git a/lpbuildd/tests/test_ci.py b/lpbuildd/tests/test_ci.py index f67300d..8fa6ee7 100644 --- a/lpbuildd/tests/test_ci.py +++ b/lpbuildd/tests/test_ci.py @@ -148,17 +148,19 @@ class TestCIBuildManagerIteration(TestCase): ] yield self.expectRunJob("build", "0", options=expected_job_options) self.buildmanager.backend.add_file( - "/build/output/build:0.log", b"I am a CI build job log.") + "/build/output/build/0/log", b"I am a CI build job log.") self.buildmanager.backend.add_file( - "/build/output/build:0/ci.whl", + "/build/output/build/0/files/ci.whl", b"I am output from a CI build job.") + self.buildmanager.backend.add_file( + "/build/output/build/0/properties", b'{"key": "value"}') # Collect the output of the first job and start running the second. yield self.expectRunJob("test", "0", options=expected_job_options) self.buildmanager.backend.add_file( - "/build/output/test:0.log", b"I am a CI test job log.") + "/build/output/test/0/log", b"I am a CI test job log.") self.buildmanager.backend.add_file( - "/build/output/test:0/ci.tar.gz", + "/build/output/test/0/files/ci.tar.gz", b"I am output from a CI test job.") # Output from the first job is visible in the status response. @@ -167,6 +169,8 @@ class TestCIBuildManagerIteration(TestCase): { "build:0": { "log": self.builder.waitingfiles["build:0.log"], + "properties": ( + self.builder.waitingfiles["build:0.properties"]), "output": { "ci.whl": self.builder.waitingfiles["build:0/ci.whl"], }, @@ -188,6 +192,7 @@ class TestCIBuildManagerIteration(TestCase): self.assertFalse(self.builder.wasCalled("buildFail")) self.assertThat(self.builder, HasWaitingFiles.byEquality({ "build:0.log": b"I am a CI build job log.", + "build:0.properties": b'{"key": "value"}', "build:0/ci.whl": b"I am output from a CI build job.", "test:0.log": b"I am a CI test job log.", "test:0/ci.tar.gz": b"I am output from a CI test job.", @@ -199,6 +204,8 @@ class TestCIBuildManagerIteration(TestCase): { "build:0": { "log": self.builder.waitingfiles["build:0.log"], + "properties": ( + self.builder.waitingfiles["build:0.properties"]), "output": { "ci.whl": self.builder.waitingfiles["build:0/ci.whl"], }, @@ -283,7 +290,7 @@ class TestCIBuildManagerIteration(TestCase): ] yield self.expectRunJob("lint", "0", options=expected_job_options) self.buildmanager.backend.add_file( - "/build/output/lint:0.log", b"I am a failing CI lint job log.") + "/build/output/lint/0/log", b"I am a failing CI lint job log.") # Collect the output of the first job and start running the second. # (Note that `retcode` is the return code of the *first* job, not the @@ -292,7 +299,7 @@ class TestCIBuildManagerIteration(TestCase): "build", "0", options=expected_job_options, retcode=RETCODE_FAILURE_BUILD) self.buildmanager.backend.add_file( - "/build/output/build:0.log", b"I am a CI build job log.") + "/build/output/build/0/log", b"I am a CI build job log.") # Output from the first job is visible in the status response. extra_status = self.buildmanager.status()
_______________________________________________ Mailing list: https://launchpad.net/~launchpad-reviewers Post to : [email protected] Unsubscribe : https://launchpad.net/~launchpad-reviewers More help : https://help.launchpad.net/ListHelp
