launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #28948
[Merge] ~cjwatson/lpcraft:input-properties into lpcraft:main
Colin Watson has proposed merging ~cjwatson/lpcraft:input-properties into lpcraft:main.
Commit message:
Add input properties
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~cjwatson/lpcraft/+git/lpcraft/+merge/427820
This allows jobs to use artifacts built by previous pipeline stages.
For now, this is restricted to only allowing input from jobs with a single instance (i.e. non-matrix jobs), since it isn't obvious how to specify input from matrix jobs in a comprehensible way.
--
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/lpcraft:input-properties into lpcraft:main.
diff --git a/docs/configuration.rst b/docs/configuration.rst
index a020b07..f73c7a4 100644
--- a/docs/configuration.rst
+++ b/docs/configuration.rst
@@ -80,6 +80,9 @@ Job definitions
``output`` (optional)
See the :ref:`output-properties` section below.
+``input`` (optional)
+ See the :ref:`input-properties` section below.
+
``matrix`` (optional)
A list of mappings, each of which is a partial job definition. The
final list of concrete jobs to run for this job name is constructed by
@@ -138,6 +141,29 @@ Output properties
<https://pydantic-docs.helpmanual.io/usage/types/#datetime-types>`_,
restricted to non-negative timedeltas.
+.. _input-properties:
+
+Input properties
+----------------
+
+Input makes artifacts from previous pipeline stages available. This only
+works if those artifacts were saved using the ``--output-directory`` option
+to ``lpcraft run``.
+
+``lpcraft`` copies artifact data to the ``files`` subdirectory of the
+designated target directory, and writes a ``properties`` file in the
+designated target directory with JSON-encoded properties of the copied
+artifacts. (This mirrors the output file structure created by ``lpcraft run
+--output-directory``.)
+
+``job-name``
+ The name of a previously-executed job whose artifacts should be made
+ available.
+
+``target-directory``
+ A path, relative to the build tree of a project, identifying a directory
+ to which the artifacts of the chosen job will be copied; the directory
+ will be created if necessary. Paths may not escape the build tree.
.. _package-repositories:
diff --git a/lpcraft/commands/run.py b/lpcraft/commands/run.py
index 131811f..231178a 100644
--- a/lpcraft/commands/run.py
+++ b/lpcraft/commands/run.py
@@ -21,7 +21,7 @@ from jinja2 import BaseLoader, Environment
from pluggy import PluginManager
from lpcraft import env
-from lpcraft.config import Config, Job, Output, PackageRepository
+from lpcraft.config import Config, Input, Job, Output, PackageRepository
from lpcraft.errors import CommandError
from lpcraft.plugin.manager import get_plugin_manager
from lpcraft.plugins import PLUGINS
@@ -116,6 +116,55 @@ def _resolve_symlinks(
return [PurePath(os.fsdecode(p)) for p in paths]
+def _copy_input_paths(
+ input: Input, remote_cwd: Path, instance: Executor, output_path: Path
+) -> None:
+ """Copy designated input artifacts into a job."""
+ source_parent_path = output_path / input.job_name
+ source_jobs = (
+ list(source_parent_path.iterdir())
+ if source_parent_path.exists()
+ else []
+ )
+ if not source_jobs:
+ raise CommandError(
+ f"Requested input from {input.job_name!r}, but that job was not "
+ f"previously executed or did not produce any output artifacts."
+ )
+ elif len(source_jobs) > 1:
+ raise CommandError(
+ f"Requested input from {input.job_name!r}, but more than one job "
+ f"with that name was previously executed and produced output "
+ f"artifacts."
+ )
+ source_path = source_jobs[0]
+
+ [target_path] = _resolve_symlinks(
+ instance, [remote_cwd / input.target_directory]
+ )
+ _check_relative_path(target_path, remote_cwd)
+ instance.execute_run(
+ ["mkdir", "-p", str(target_path / "files")], check=True
+ )
+
+ try:
+ for path in sorted((source_path / "files").iterdir()):
+ instance.push_file(
+ source=path,
+ # Path() here works around
+ # https://github.com/canonical/craft-providers/pull/135.
+ destination=Path(target_path / "files" / path.name),
+ )
+ instance.push_file(
+ source=source_path / "properties",
+ # Path() here works around
+ # https://github.com/canonical/craft-providers/pull/135.
+ destination=Path(target_path / "properties"),
+ )
+ except Exception as e:
+ raise CommandError(str(e), retcode=1)
+
+
def _copy_output_paths(
output: Output, remote_cwd: Path, instance: Executor, target_path: Path
) -> None:
@@ -442,6 +491,10 @@ def _run_job(
environment=environment,
secrets=secrets,
)
+
+ if job.input is not None and output is not None:
+ _copy_input_paths(job.input, remote_cwd, instance, output)
+
for cmd in (pre_run_command, run_command, post_run_command):
if cmd:
_run_instance_command(
diff --git a/lpcraft/commands/tests/test_run.py b/lpcraft/commands/tests/test_run.py
index 3dc6bbc..8bf3030 100644
--- a/lpcraft/commands/tests/test_run.py
+++ b/lpcraft/commands/tests/test_run.py
@@ -5,6 +5,7 @@ import io
import json
import os
import re
+import shutil
import subprocess
from pathlib import Path, PosixPath
from textwrap import dedent
@@ -1408,6 +1409,374 @@ class TestRun(RunBaseTestCase):
[error] = result.errors
self.assertIn("/target", str(error))
+ @patch("lpcraft.env.get_managed_environment_project_path")
+ @patch("lpcraft.commands.run.get_provider")
+ @patch("lpcraft.commands.run.get_host_architecture", return_value="amd64")
+ def test_copies_input_paths(
+ self,
+ mock_get_host_architecture,
+ mock_get_provider,
+ mock_get_project_path,
+ ):
+ def fake_pull_file(source: Path, destination: Path) -> None:
+ shutil.copy2(source, destination)
+
+ def fake_push_file(source: Path, destination: Path) -> None:
+ shutil.copy2(source, destination)
+
+ target_path = Path(self.useFixture(TempDir()).path)
+ launcher = Mock(spec=launch)
+ provider = makeLXDProvider(lxd_launcher=launcher)
+ mock_get_provider.return_value = provider
+ execute_run = LocalExecuteRun(self.tmp_project_path)
+ launcher.return_value.execute_run = execute_run
+ mock_get_project_path.return_value = self.tmp_project_path
+ launcher.return_value.pull_file.side_effect = fake_pull_file
+ launcher.return_value.push_file.side_effect = fake_push_file
+ config = dedent(
+ """
+ pipeline:
+ - build
+ - test
+
+ jobs:
+ build:
+ series: focal
+ architectures: [amd64]
+ run: "true"
+ output:
+ paths: [binary]
+
+ test:
+ series: focal
+ architectures: [amd64]
+ run: "true"
+ input:
+ job-name: build
+ target-directory: artifacts
+ """
+ )
+ Path(".launchpad.yaml").write_text(config)
+ Path("binary").write_bytes(b"binary")
+ result = self.run_command(
+ "run", "--output-directory", str(target_path)
+ )
+
+ self.assertEqual(0, result.exit_code)
+ build_job_output = target_path / "build" / "0"
+ artifacts_path = self.tmp_project_path / "artifacts"
+ self.assertEqual(
+ [
+ call(
+ source=self.tmp_project_path / "binary",
+ destination=build_job_output / "files" / "binary",
+ )
+ ],
+ launcher.return_value.pull_file.call_args_list,
+ )
+ self.assertEqual(
+ [
+ call(
+ source=build_job_output / "files" / "binary",
+ destination=artifacts_path / "files" / "binary",
+ ),
+ call(
+ source=build_job_output / "properties",
+ destination=artifacts_path / "properties",
+ ),
+ ],
+ launcher.return_value.push_file.call_args_list,
+ )
+ self.assertEqual(
+ ["files", "properties"],
+ sorted(path.name for path in artifacts_path.iterdir()),
+ )
+ self.assertEqual(
+ ["binary"],
+ sorted(path.name for path in (artifacts_path / "files").iterdir()),
+ )
+ self.assertEqual(
+ b"binary", (artifacts_path / "files" / "binary").read_bytes()
+ )
+
+ @patch("lpcraft.env.get_managed_environment_project_path")
+ @patch("lpcraft.commands.run.get_provider")
+ @patch("lpcraft.commands.run.get_host_architecture", return_value="amd64")
+ def test_input_target_directory_not_previously_executed(
+ self,
+ mock_get_host_architecture,
+ mock_get_provider,
+ mock_get_project_path,
+ ):
+ target_path = Path(self.useFixture(TempDir()).path)
+ launcher = Mock(spec=launch)
+ provider = makeLXDProvider(lxd_launcher=launcher)
+ mock_get_provider.return_value = provider
+ execute_run = LocalExecuteRun(self.tmp_project_path)
+ launcher.return_value.execute_run = execute_run
+ mock_get_project_path.return_value = self.tmp_project_path
+ config = dedent(
+ """
+ pipeline:
+ - test
+
+ jobs:
+ test:
+ series: focal
+ architectures: amd64
+ run: "true"
+ input:
+ job-name: build
+ target-directory: artifacts
+ """
+ )
+ Path(".launchpad.yaml").write_text(config)
+
+ result = self.run_command(
+ "run", "--output-directory", str(target_path)
+ )
+
+ self.assertThat(
+ result,
+ MatchesStructure.byEquality(
+ exit_code=1,
+ errors=[
+ CommandError(
+ "Requested input from 'build', but that job was not "
+ "previously executed or did not produce any output "
+ "artifacts.",
+ retcode=1,
+ )
+ ],
+ ),
+ )
+
+ @patch("lpcraft.env.get_managed_environment_project_path")
+ @patch("lpcraft.commands.run.get_provider")
+ @patch("lpcraft.commands.run.get_host_architecture", return_value="amd64")
+ def test_input_target_directory_multiple_jobs(
+ self,
+ mock_get_host_architecture,
+ mock_get_provider,
+ mock_get_project_path,
+ ):
+ target_path = Path(self.useFixture(TempDir()).path)
+ launcher = Mock(spec=launch)
+ provider = makeLXDProvider(lxd_launcher=launcher)
+ mock_get_provider.return_value = provider
+ execute_run = LocalExecuteRun(self.tmp_project_path)
+ launcher.return_value.execute_run = execute_run
+ mock_get_project_path.return_value = self.tmp_project_path
+ config = dedent(
+ """
+ pipeline:
+ - build
+ - test
+
+ jobs:
+ build:
+ matrix:
+ - series: bionic
+ - series: focal
+ architectures: [amd64]
+ run: "true"
+ output:
+ paths: [binary]
+
+ test:
+ series: focal
+ architectures: amd64
+ run: "true"
+ input:
+ job-name: build
+ target-directory: artifacts
+ """
+ )
+ Path(".launchpad.yaml").write_text(config)
+ Path("binary").touch()
+
+ result = self.run_command(
+ "run", "--output-directory", str(target_path)
+ )
+
+ self.assertThat(
+ result,
+ MatchesStructure.byEquality(
+ exit_code=1,
+ errors=[
+ CommandError(
+ "Requested input from 'build', but more than one job "
+ "with that name was previously executed and produced "
+ "output artifacts.",
+ retcode=1,
+ )
+ ],
+ ),
+ )
+
+ @patch("lpcraft.env.get_managed_environment_project_path")
+ @patch("lpcraft.commands.run.get_provider")
+ @patch("lpcraft.commands.run.get_host_architecture", return_value="amd64")
+ def test_input_target_directory_escapes_directly(
+ self,
+ mock_get_host_architecture,
+ mock_get_provider,
+ mock_get_project_path,
+ ):
+ target_path = Path(self.useFixture(TempDir()).path)
+ launcher = Mock(spec=launch)
+ provider = makeLXDProvider(lxd_launcher=launcher)
+ mock_get_provider.return_value = provider
+ execute_run = LocalExecuteRun(self.tmp_project_path)
+ launcher.return_value.execute_run = execute_run
+ mock_get_project_path.return_value = self.tmp_project_path
+ config = dedent(
+ """
+ pipeline:
+ - build
+ - test
+
+ jobs:
+ build:
+ series: focal
+ architectures: [amd64]
+ run: "true"
+ output:
+ paths: [binary]
+
+ test:
+ series: focal
+ architectures: amd64
+ run: "true"
+ input:
+ job-name: build
+ target-directory: "../etc/secrets"
+ """
+ )
+ Path(".launchpad.yaml").write_text(config)
+ Path("binary").touch()
+
+ result = self.run_command(
+ "run", "--output-directory", str(target_path)
+ )
+
+ # The exact error message differs between Python 3.8 and 3.9, so
+ # don't test it in detail, but make sure it includes the offending
+ # path.
+ self.assertEqual(1, result.exit_code)
+ [error] = result.errors
+ self.assertIn("/etc/secrets", str(error))
+
+ @patch("lpcraft.env.get_managed_environment_project_path")
+ @patch("lpcraft.commands.run.get_provider")
+ @patch("lpcraft.commands.run.get_host_architecture", return_value="amd64")
+ def test_input_target_directory_escapes_symlink(
+ self,
+ mock_get_host_architecture,
+ mock_get_provider,
+ mock_get_project_path,
+ ):
+ target_path = Path(self.useFixture(TempDir()).path)
+ launcher = Mock(spec=launch)
+ provider = makeLXDProvider(lxd_launcher=launcher)
+ mock_get_provider.return_value = provider
+ execute_run = LocalExecuteRun(self.tmp_project_path)
+ launcher.return_value.execute_run = execute_run
+ mock_get_project_path.return_value = self.tmp_project_path
+ config = dedent(
+ """
+ pipeline:
+ - build
+ - test
+
+ jobs:
+ build:
+ series: focal
+ architectures: [amd64]
+ run: "true"
+ output:
+ paths: [binary]
+
+ test:
+ series: focal
+ architectures: amd64
+ run: "true"
+ input:
+ job-name: build
+ target-directory: artifacts
+ """
+ )
+ Path(".launchpad.yaml").write_text(config)
+ Path("binary").touch()
+ Path("artifacts").symlink_to("../secrets")
+
+ result = self.run_command(
+ "run", "--output-directory", str(target_path)
+ )
+
+ # The exact error message differs between Python 3.8 and 3.9, so
+ # don't test it in detail, but make sure it includes the offending
+ # path.
+ self.assertEqual(1, result.exit_code)
+ [error] = result.errors
+ self.assertIn("/secrets", str(error))
+
+ @patch("lpcraft.env.get_managed_environment_project_path")
+ @patch("lpcraft.commands.run.get_provider")
+ @patch("lpcraft.commands.run.get_host_architecture", return_value="amd64")
+ def test_input_push_file_fails(
+ self,
+ mock_get_host_architecture,
+ mock_get_provider,
+ mock_get_project_path,
+ ):
+ target_path = Path(self.useFixture(TempDir()).path)
+ launcher = Mock(spec=launch)
+ provider = makeLXDProvider(lxd_launcher=launcher)
+ mock_get_provider.return_value = provider
+ execute_run = LocalExecuteRun(self.tmp_project_path)
+ launcher.return_value.execute_run = execute_run
+ launcher.return_value.push_file.side_effect = FileNotFoundError(
+ "File not found"
+ )
+ mock_get_project_path.return_value = self.tmp_project_path
+ config = dedent(
+ """
+ pipeline:
+ - build
+ - test
+
+ jobs:
+ build:
+ series: focal
+ architectures: [amd64]
+ run: "true"
+ output:
+ paths: [binary]
+
+ test:
+ series: focal
+ architectures: amd64
+ run: "true"
+ input:
+ job-name: build
+ target-directory: artifacts
+ """
+ )
+ Path(".launchpad.yaml").write_text(config)
+ Path("binary").touch()
+
+ result = self.run_command(
+ "run", "--output-directory", str(target_path)
+ )
+
+ self.assertThat(
+ result,
+ MatchesStructure.byEquality(
+ exit_code=1, errors=[CommandError("File not found", retcode=1)]
+ ),
+ )
+
@responses.activate
@patch("lpcraft.commands.run.get_provider")
@patch("lpcraft.commands.run.get_host_architecture", return_value="amd64")
diff --git a/lpcraft/config.py b/lpcraft/config.py
index 328cd56..0a3b3dd 100644
--- a/lpcraft/config.py
+++ b/lpcraft/config.py
@@ -60,6 +60,13 @@ class Output(ModelConfigDefaults):
return v
+class Input(ModelConfigDefaults):
+ """Job input properties."""
+
+ job_name: StrictStr
+ target_directory: StrictStr
+
+
def _validate_plugin_config(
plugin: Type[BasePlugin],
values: Dict[StrictStr, Any],
@@ -157,6 +164,7 @@ class Job(ModelConfigDefaults):
run_after: Optional[StrictStr]
environment: Optional[Dict[str, Optional[str]]]
output: Optional[Output]
+ input: Optional[Input]
snaps: Optional[List[StrictStr]]
packages: Optional[List[StrictStr]]
package_repositories: Optional[List[PackageRepository]]
diff --git a/lpcraft/tests/test_config.py b/lpcraft/tests/test_config.py
index 575f284..64f5fa8 100644
--- a/lpcraft/tests/test_config.py
+++ b/lpcraft/tests/test_config.py
@@ -313,6 +313,41 @@ class TestConfig(TestCase):
path,
)
+ def test_input(self):
+ path = self.create_config(
+ dedent(
+ """
+ pipeline:
+ - build
+ - test
+
+ jobs:
+ build:
+ series: focal
+ architectures: [amd64]
+ packages: [make]
+ run: make
+ output:
+ paths: [binary]
+
+ test:
+ series: focal
+ architectures: [amd64]
+ run: artifacts/binary
+ input:
+ job-name: build
+ target-directory: artifacts
+ """
+ )
+ )
+ config = Config.load(path)
+ self.assertThat(
+ config.jobs["test"][0].input,
+ MatchesStructure.byEquality(
+ job_name="build", target_directory="artifacts"
+ ),
+ )
+
def test_load_snaps(self):
path = self.create_config(
dedent(
