launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #27736
[Merge] ~cjwatson/lpcraft:matrix into lpcraft:main
Colin Watson has proposed merging ~cjwatson/lpcraft:matrix into lpcraft:main.
Commit message:
Add job matrix support
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~cjwatson/lpcraft/+git/lpcraft/+merge/412024
I'm not completely happy with the way this is done at the `pydantic` level, but it works well enough for the moment.
I realized that my handling of the case where a job isn't defined for the host architecture as an error doesn't make a lot of sense, especially in the context of larger job matrices, so I revised that to skip such jobs instead.
--
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/lpcraft:matrix into lpcraft:main.
diff --git a/lpcraft/commands/run.py b/lpcraft/commands/run.py
index b3917cf..6730e3d 100644
--- a/lpcraft/commands/run.py
+++ b/lpcraft/commands/run.py
@@ -4,6 +4,7 @@
import subprocess
from argparse import Namespace
from pathlib import Path
+from typing import List, Optional
from craft_cli import EmitterMode, emit
@@ -14,11 +15,19 @@ from lpcraft.providers import get_provider, replay_logs
from lpcraft.utils import get_host_architecture
-def _get_job(config: Config, job_name: str) -> Job:
- try:
- return config.jobs[job_name]
- except KeyError:
+def _get_jobs(
+ config: Config, job_name: str, series: Optional[str] = None
+) -> List[Job]:
+ jobs = config.jobs.get(job_name, [])
+ if not jobs:
raise CommandError(f"No job definition for {job_name!r}")
+ if series is not None:
+ jobs = [job for job in jobs if job.series == series]
+ if not jobs:
+ raise CommandError(
+ f"No job definition for {job_name!r} for {series}"
+ )
+ return jobs
def _run_job(args: Namespace) -> None:
@@ -29,7 +38,13 @@ def _run_job(args: Namespace) -> None:
raise CommandError("Job name is required in managed mode")
config = load(".launchpad.yaml")
- job = _get_job(config, args.job_name)
+ jobs = _get_jobs(config, args.job_name, series=args.series)
+ if len(jobs) > 1:
+ raise CommandError(
+ f"Ambiguous job definitions for {args.job_name!r} for "
+ f"{args.series}"
+ )
+ [job] = jobs
if job.run is None:
raise CommandError(f"'run' not set for job {args.job_name!r}")
proc = subprocess.run(["bash", "--noprofile", "--norc", "-ec", job.run])
@@ -51,45 +66,45 @@ def _run_pipeline(args: Namespace) -> None:
provider.ensure_provider_is_available()
for job_name in config.pipeline:
- job = _get_job(config, job_name)
- if host_architecture not in job.architectures:
- raise CommandError(
- f"Job {job_name!r} not defined for {host_architecture}"
+ jobs = _get_jobs(config, job_name)
+ for job in jobs:
+ if host_architecture not in job.architectures:
+ continue
+
+ cmd = ["lpcraft"]
+ if emit.get_mode() == EmitterMode.QUIET:
+ cmd.append("--quiet")
+ elif emit.get_mode() == EmitterMode.VERBOSE:
+ cmd.append("--verbose")
+ elif emit.get_mode() == EmitterMode.TRACE:
+ cmd.append("--trace")
+ cmd.extend(["run", "--series", job.series, job_name])
+
+ emit.progress(
+ f"Launching environment for {job.series}/{host_architecture}"
)
-
- cmd = ["lpcraft"]
- if emit.get_mode() == EmitterMode.QUIET:
- cmd.append("--quiet")
- elif emit.get_mode() == EmitterMode.VERBOSE:
- cmd.append("--verbose")
- elif emit.get_mode() == EmitterMode.TRACE:
- cmd.append("--trace")
- cmd.extend(["run", "--series", job.series, job_name])
-
- emit.progress(
- f"Launching environment for {job.series}/{host_architecture}"
- )
- with provider.launched_environment(
- project_name=cwd.name,
- project_path=cwd,
- series=job.series,
- architecture=host_architecture,
- ) as instance:
- emit.progress("Running the job")
- with emit.open_stream(f"Running {cmd}") as stream:
- proc = instance.execute_run(
- cmd,
- cwd=env.get_managed_environment_project_path(),
- stdout=stream,
- stderr=stream,
- )
- if proc.returncode != 0:
- replay_logs(instance)
- raise CommandError(
- f"Job {job_name!r} for {job.series}/{host_architecture} "
- f"failed with exit status {proc.returncode}.",
- retcode=proc.returncode,
- )
+ with provider.launched_environment(
+ project_name=cwd.name,
+ project_path=cwd,
+ series=job.series,
+ architecture=host_architecture,
+ ) as instance:
+ emit.progress("Running the job")
+ with emit.open_stream(f"Running {cmd}") as stream:
+ proc = instance.execute_run(
+ cmd,
+ cwd=env.get_managed_environment_project_path(),
+ stdout=stream,
+ stderr=stream,
+ )
+ if proc.returncode != 0:
+ replay_logs(instance)
+ raise CommandError(
+ f"Job {job_name!r} for "
+ f"{job.series}/{host_architecture} failed with "
+ f"exit status {proc.returncode}.",
+ retcode=proc.returncode,
+ )
def run(args: Namespace) -> int:
diff --git a/lpcraft/commands/tests/test_run.py b/lpcraft/commands/tests/test_run.py
index ae74b83..669e26d 100644
--- a/lpcraft/commands/tests/test_run.py
+++ b/lpcraft/commands/tests/test_run.py
@@ -87,6 +87,64 @@ class RunJobTestCase(CommandBaseTestCase):
),
)
+ def test_no_matching_job_for_series(self):
+ config = dedent(
+ """
+ pipeline:
+ - test
+
+ jobs:
+ test:
+ series: focal
+ architectures: amd64
+ run: tox
+ """
+ )
+ Path(".launchpad.yaml").write_text(config)
+
+ result = self.run_command("run", "--series", "bionic", "test")
+
+ self.assertThat(
+ result,
+ MatchesStructure.byEquality(
+ exit_code=1,
+ errors=[
+ CommandError("No job definition for 'test' for bionic")
+ ],
+ ),
+ )
+
+ def test_ambiguous_job_definitions(self):
+ config = dedent(
+ """
+ pipeline:
+ - test
+
+ jobs:
+ test:
+ matrix:
+ - run: make
+ - run: tox
+ series: focal
+ architectures: amd64
+ """
+ )
+ Path(".launchpad.yaml").write_text(config)
+
+ result = self.run_command("run", "--series", "focal", "test")
+
+ self.assertThat(
+ result,
+ MatchesStructure.byEquality(
+ exit_code=1,
+ errors=[
+ CommandError(
+ "Ambiguous job definitions for 'test' for focal"
+ )
+ ],
+ ),
+ )
+
def test_no_run_definition(self):
config = dedent(
"""
@@ -223,7 +281,7 @@ class RunPipelineTestCase(CommandBaseTestCase):
config = dedent(
"""
pipeline: []
- jobs: []
+ jobs: {}
"""
)
Path(".launchpad.yaml").write_text(config)
@@ -249,7 +307,7 @@ class RunPipelineTestCase(CommandBaseTestCase):
pipeline:
- test
- jobs: []
+ jobs: {}
"""
)
Path(".launchpad.yaml").write_text(config)
@@ -265,11 +323,18 @@ class RunPipelineTestCase(CommandBaseTestCase):
)
@patch("lpcraft.commands.run.get_provider")
- @patch("lpcraft.commands.run.get_host_architecture", return_value="i386")
+ @patch("lpcraft.commands.run.get_host_architecture", return_value="arm64")
def test_job_not_defined_for_host_architecture(
self, mock_get_host_architecture, mock_get_provider
):
- mock_get_provider.return_value = self.makeLXDProvider()
+ # Jobs not defined for the host architecture are skipped. (It is
+ # assumed that the dispatcher won't dispatch anything for an
+ # architecture if it has no jobs at all.)
+ launcher = Mock(spec=launch)
+ provider = self.makeLXDProvider(lxd_launcher=launcher)
+ mock_get_provider.return_value = provider
+ execute_run = launcher.return_value.execute_run
+ execute_run.return_value = subprocess.CompletedProcess([], 0)
config = dedent(
"""
pipeline:
@@ -279,18 +344,28 @@ class RunPipelineTestCase(CommandBaseTestCase):
test:
series: focal
architectures: [amd64, arm64]
+ run: tox
+ build-wheel:
+ series: focal
+ architectures: amd64
+ run: pyproject-build
"""
)
Path(".launchpad.yaml").write_text(config)
result = self.run_command("run")
- self.assertThat(
- result,
- MatchesStructure.byEquality(
- exit_code=1,
- errors=[CommandError("Job 'test' not defined for i386")],
- ),
+ self.assertEqual(0, result.exit_code)
+ self.assertEqual(
+ [
+ call(
+ ["lpcraft", "run", "--series", "focal", "test"],
+ cwd=Path("/root/project"),
+ stdout=ANY,
+ stderr=ANY,
+ )
+ ],
+ execute_run.call_args_list,
)
@patch("lpcraft.commands.run.get_provider")
@@ -393,3 +468,62 @@ class RunPipelineTestCase(CommandBaseTestCase):
],
execute_run.call_args_list,
)
+
+ @patch("lpcraft.commands.run.get_provider")
+ @patch("lpcraft.commands.run.get_host_architecture", return_value="amd64")
+ def test_expands_matrix(
+ self, mock_get_host_architecture, mock_get_provider
+ ):
+ launcher = Mock(spec=launch)
+ provider = self.makeLXDProvider(lxd_launcher=launcher)
+ mock_get_provider.return_value = provider
+ execute_run = launcher.return_value.execute_run
+ execute_run.return_value = subprocess.CompletedProcess([], 0)
+ config = dedent(
+ """
+ pipeline:
+ - test
+ - build-wheel
+
+ jobs:
+ test:
+ matrix:
+ - series: bionic
+ architectures: amd64
+ - series: focal
+ architectures: [amd64, s390x]
+ run: tox
+ build-wheel:
+ series: bionic
+ architectures: amd64
+ run: pyproject-build
+ """
+ )
+ Path(".launchpad.yaml").write_text(config)
+
+ result = self.run_command("run")
+
+ self.assertEqual(0, result.exit_code)
+ self.assertEqual(
+ [
+ call(
+ ["lpcraft", "run", "--series", "bionic", "test"],
+ cwd=Path("/root/project"),
+ stdout=ANY,
+ stderr=ANY,
+ ),
+ call(
+ ["lpcraft", "run", "--series", "focal", "test"],
+ cwd=Path("/root/project"),
+ stdout=ANY,
+ stderr=ANY,
+ ),
+ call(
+ ["lpcraft", "run", "--series", "bionic", "build-wheel"],
+ cwd=Path("/root/project"),
+ stdout=ANY,
+ stderr=ANY,
+ ),
+ ],
+ execute_run.call_args_list,
+ )
diff --git a/lpcraft/config.py b/lpcraft/config.py
index cd6b128..e3b4f97 100644
--- a/lpcraft/config.py
+++ b/lpcraft/config.py
@@ -2,7 +2,7 @@
# GNU General Public License version 3 (see the file LICENSE).
from pathlib import Path
-from typing import Dict, List, Optional, Union
+from typing import Any, Dict, List, Optional, Union
import pydantic
from pydantic import StrictStr
@@ -15,6 +15,7 @@ class ModelConfigDefaults(
extra=pydantic.Extra.forbid,
frozen=True,
alias_generator=lambda s: s.replace("_", "-"),
+ underscore_attrs_are_private=True,
):
"""Define lpcraft's model defaults."""
@@ -35,11 +36,41 @@ class Job(ModelConfigDefaults):
return v
+def _expand_job_values(
+ values: Dict[StrictStr, Any]
+) -> List[Dict[StrictStr, Any]]:
+ expanded_values = []
+ if "matrix" in values:
+ base_values = values.copy()
+ del base_values["matrix"]
+ for variant in values["matrix"]:
+ variant_values = base_values.copy()
+ variant_values.update(variant)
+ expanded_values.append(variant_values)
+ else:
+ expanded_values.append(values)
+ return expanded_values
+
+
class Config(ModelConfigDefaults):
"""A .launchpad.yaml configuration file."""
pipeline: List[StrictStr]
- jobs: Dict[StrictStr, Job]
+ jobs: Dict[StrictStr, List[Job]]
+
+ # XXX cjwatson 2021-11-17: This expansion strategy works, but it may
+ # produce suboptimal error messages, and doesn't have a good way to do
+ # things like limiting the keys that can be set in a matrix.
+ @pydantic.root_validator(pre=True)
+ def expand_matrix(
+ cls, values: Dict[StrictStr, Any]
+ ) -> Dict[StrictStr, Any]:
+ expanded_values = values.copy()
+ expanded_values["jobs"] = {
+ job_name: _expand_job_values(job_values)
+ for job_name, job_values in values["jobs"].items()
+ }
+ return expanded_values
def load(filename: str) -> Config:
diff --git a/lpcraft/tests/test_config.py b/lpcraft/tests/test_config.py
index 9065665..6f69136 100644
--- a/lpcraft/tests/test_config.py
+++ b/lpcraft/tests/test_config.py
@@ -6,7 +6,12 @@ from textwrap import dedent
from fixtures import TempDir
from testtools import TestCase
-from testtools.matchers import Equals, MatchesDict, MatchesStructure
+from testtools.matchers import (
+ Equals,
+ MatchesDict,
+ MatchesListwise,
+ MatchesStructure,
+)
from lpcraft.config import load
@@ -44,10 +49,14 @@ class TestConfig(TestCase):
pipeline=Equals(["test"]),
jobs=MatchesDict(
{
- "test": MatchesStructure.byEquality(
- series="focal",
- architectures=["amd64", "arm64"],
- run="tox\n",
+ "test": MatchesListwise(
+ [
+ MatchesStructure.byEquality(
+ series="focal",
+ architectures=["amd64", "arm64"],
+ run="tox\n",
+ )
+ ]
)
}
),
@@ -71,4 +80,50 @@ class TestConfig(TestCase):
)
)
config = load(str(path))
- self.assertEqual(["amd64"], config.jobs["test"].architectures)
+ self.assertEqual(["amd64"], config.jobs["test"][0].architectures)
+
+ def test_expands_matrix(self):
+ path = self.create_config(
+ dedent(
+ """
+ pipeline:
+ - test
+
+ jobs:
+ test:
+ matrix:
+ - series: bionic
+ architectures: [amd64, arm64]
+
+ - series: focal
+ architectures: amd64
+ run: tox -e py38
+ run: tox
+ """
+ )
+ )
+ config = load(str(path))
+ self.assertThat(
+ config,
+ MatchesStructure(
+ pipeline=Equals(["test"]),
+ jobs=MatchesDict(
+ {
+ "test": MatchesListwise(
+ [
+ MatchesStructure.byEquality(
+ series="bionic",
+ architectures=["amd64", "arm64"],
+ run="tox",
+ ),
+ MatchesStructure.byEquality(
+ series="focal",
+ architectures=["amd64"],
+ run="tox -e py38",
+ ),
+ ]
+ )
+ }
+ ),
+ ),
+ )
