launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #30383
[Merge] ~cjwatson/launchpad:ci-build-track-report-das into launchpad:master
Colin Watson has proposed merging ~cjwatson/launchpad:ci-build-track-report-das into launchpad:master.
Commit message:
Track the series/architecture for each revision status report
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/+git/launchpad/+merge/448904
An awkward legacy of the order in which revision statuses and CI builds were designed is that revision status reports linked to a CI build don't record the series and architecture used for each individual CI build job. This is a problem when we need to be able to do things like filter the artifacts produced by a CI build to select only those built on a particular series, and in general it seems like information that would be worth tracking in the database.
Model the new `RevisionStatusReport.distro_arch_series` column for this, allow passing it when creating a new report, and set it when requesting CI builds if we can. That last step has to hardcode Ubuntu for the same reasons that `lp.code.model.cibuild.determine_DASes_to_build has to do so; and as usual we have to keep a clear head about the distinction between the series/architecture that runs the build farm job (i.e. the container where `lpci` itself is run) and the series/architecture of each individual CI build job (i.e. the container started by `lpci` that runs the workload).
Database MP: https://code.launchpad.net/~cjwatson/launchpad/+git/launchpad/+merge/448900
--
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:ci-build-track-report-das into launchpad:master.
diff --git a/lib/lp/code/interfaces/cibuild.py b/lib/lp/code/interfaces/cibuild.py
index 472b67e..b8c0920 100644
--- a/lib/lp/code/interfaces/cibuild.py
+++ b/lib/lp/code/interfaces/cibuild.py
@@ -191,12 +191,14 @@ class ICIBuildView(IPackageBuildView, IPrivacy):
cannot be parsed.
"""
- def getOrCreateRevisionStatusReport(job_id):
+ def getOrCreateRevisionStatusReport(job_id, distro_arch_series=None):
"""Get the `IRevisionStatusReport` for a given job in this build.
Create the report if necessary.
:param job_id: A job ID, in the form "JOB_NAME:JOB_INDEX".
+ :param distro_arch_series: The series and architecture used for this
+ job, if known.
:return: An `IRevisionStatusReport`.
"""
diff --git a/lib/lp/code/interfaces/revisionstatus.py b/lib/lp/code/interfaces/revisionstatus.py
index c0303bb..f104dcb 100644
--- a/lib/lp/code/interfaces/revisionstatus.py
+++ b/lib/lp/code/interfaces/revisionstatus.py
@@ -36,6 +36,7 @@ from lp.app.validators.attachment import attachment_size_constraint
from lp.code.enums import RevisionStatusArtifactType, RevisionStatusResult
from lp.services.auth.enums import AccessTokenScope
from lp.services.fields import PublicPersonChoice, URIField
+from lp.soyuz.interfaces.distroarchseries import IDistroArchSeries
@error_status(http.client.UNAUTHORIZED)
@@ -160,6 +161,18 @@ class IRevisionStatusReportEditableAttributes(Interface):
)
)
+ distro_arch_series = exported(
+ Reference(
+ title=_(
+ "The series and architecture for the CI build job that "
+ "produced this report."
+ ),
+ schema=IDistroArchSeries,
+ required=False,
+ readonly=True,
+ )
+ )
+
properties = exported(
Dict(
title=_("Metadata for artifacts attached to this report"),
@@ -277,6 +290,8 @@ class IRevisionStatusReportSet(Interface):
date_finished=None,
log=None,
ci_build=None,
+ distro_arch_series=None,
+ properties=None,
):
"""Return a new revision status report.
@@ -292,6 +307,9 @@ class IRevisionStatusReportSet(Interface):
:param date_finished: DateTime that report was completed.
:param log: Stores the content of the artifact for this report.
:param ci_build: The `ICIBuild` that produced this report, if any.
+ :param distro_arch_series: The series and architecture for the build
+ that produced this report, if any.
+ :param properties: Metadata for artifacts attached to this report.
"""
def getByID(id):
diff --git a/lib/lp/code/model/cibuild.py b/lib/lp/code/model/cibuild.py
index edddef8..12cd4e5 100644
--- a/lib/lp/code/model/cibuild.py
+++ b/lib/lp/code/model/cibuild.py
@@ -106,8 +106,8 @@ def get_stages(configuration):
jobs = defaultdict(list)
if job_name not in configuration.jobs:
raise CannotBuild("No job definition for %r" % job_name)
- for i in range(len(configuration.jobs[job_name])):
- for arch in configuration.jobs[job_name][i]["architectures"]:
+ for i, job_config in enumerate(configuration.jobs[job_name]):
+ for arch in job_config["architectures"]:
# Making sure that the previous job is present
# in the pipeline for a given arch.
if previous_job != "":
@@ -121,7 +121,9 @@ def get_stages(configuration):
"in the same pipeline would not"
% (job_name, arch, previous_job),
)
- jobs[arch].append((job_name, i))
+ jobs[arch].append(
+ (job_name, (i, job_config["series"], arch))
+ )
for arch, value in jobs.items():
stages[arch].append(value)
@@ -489,7 +491,7 @@ class CIBuild(PackageBuildMixin, StormBase):
self._jobs = {}
self._jobs["results"] = results
- def getOrCreateRevisionStatusReport(self, job_id):
+ def getOrCreateRevisionStatusReport(self, job_id, distro_arch_series=None):
"""See `ICIBuild`."""
report = getUtility(IRevisionStatusReportSet).getByCIBuildAndTitle(
self, job_id
@@ -505,6 +507,7 @@ class CIBuild(PackageBuildMixin, StormBase):
git_repository=self.git_repository,
commit_sha1=self.commit_sha1,
ci_build=self,
+ distro_arch_series=distro_arch_series,
)
return report
@@ -783,8 +786,20 @@ class CIBuildSet(SpecificBuildFarmJobSourceMixin):
das.architecturetag,
)
build = self.requestBuild(
- git_repository, commit_sha1, das, stages, git_refs
+ git_repository,
+ commit_sha1,
+ das,
+ [
+ [(job_name, i) for job_name, (i, series, arch) in stage]
+ for stage in stages
+ ],
+ git_refs,
)
+ # XXX cjwatson 2023-08-09: We have to hardcode Ubuntu for now,
+ # since the .launchpad.yaml format doesn't currently support
+ # other distributions (although nor does the Launchpad build
+ # farm).
+ distribution = getUtility(ILaunchpadCelebrities).ubuntu
# Create reports for each individual job in this build so that
# they show up as pending in the web UI. The job names
# generated here should match those generated by
@@ -792,11 +807,19 @@ class CIBuildSet(SpecificBuildFarmJobSourceMixin):
# lp.archiveuploader.ciupload looks for this report and attaches
# artifacts to it.
for stage in stages:
- for job_name, i in stage:
+ for job_name, (i, series, arch) in stage:
+ try:
+ job_das = distribution[series][arch]
+ except KeyError:
+ # determine_DASes_to_build logs errors about this
+ # when working out the series/arch combination for
+ # groups of jobs, so we don't need to do it again
+ # here.
+ job_das = None
# XXX cjwatson 2022-03-17: It would be better if we
# could set some kind of meaningful description as well.
build.getOrCreateRevisionStatusReport(
- "%s:%s" % (job_name, i)
+ "%s:%s" % (job_name, i), distro_arch_series=job_das
)
except CIBuildAlreadyRequested:
pass
diff --git a/lib/lp/code/model/revisionstatus.py b/lib/lp/code/model/revisionstatus.py
index c079a87..8428780 100644
--- a/lib/lp/code/model/revisionstatus.py
+++ b/lib/lp/code/model/revisionstatus.py
@@ -60,6 +60,14 @@ class RevisionStatusReport(StormBase):
ci_build_id = Int(name="ci_build", allow_none=True)
ci_build = Reference(ci_build_id, "CIBuild.id")
+ # We don't always know the DAS for this report (for example, external
+ # builds, or historical builds from before we introduced this column),
+ # but when we do it's useful for filtering.
+ distro_arch_series_id = Int(name="distro_arch_series", allow_none=True)
+ distro_arch_series = Reference(
+ distro_arch_series_id, "DistroArchSeries.id"
+ )
+
date_created = DateTime(
name="date_created", tzinfo=timezone.utc, allow_none=False
)
@@ -83,6 +91,7 @@ class RevisionStatusReport(StormBase):
result_summary,
result,
ci_build=None,
+ distro_arch_series=None,
properties=None,
):
super().__init__()
@@ -93,6 +102,7 @@ class RevisionStatusReport(StormBase):
self.url = url
self.result_summary = result_summary
self.ci_build = ci_build
+ self.distro_arch_series = distro_arch_series
self.date_created = UTC_NOW
self.transitionToNewResult(result)
self.properties = properties
@@ -215,6 +225,7 @@ class RevisionStatusReportSet:
date_finished=None,
log=None,
ci_build=None,
+ distro_arch_series=None,
properties=None,
):
"""See `IRevisionStatusReportSet`."""
@@ -228,6 +239,7 @@ class RevisionStatusReportSet:
result_summary,
result,
ci_build=ci_build,
+ distro_arch_series=distro_arch_series,
properties=properties,
)
store.add(report)
diff --git a/lib/lp/code/model/tests/test_cibuild.py b/lib/lp/code/model/tests/test_cibuild.py
index 0847d23..ed55b49 100644
--- a/lib/lp/code/model/tests/test_cibuild.py
+++ b/lib/lp/code/model/tests/test_cibuild.py
@@ -85,6 +85,19 @@ from lp.testing.pages import webservice_for_person
from lp.xmlrpc.interfaces import IPrivateApplication
+class MatchesDistroArchSeries(MatchesStructure):
+ def __init__(self, distribution_name, distroseries_name, architecturetag):
+ super().__init__(
+ distroseries=MatchesStructure(
+ distribution=MatchesStructure.byEquality(
+ name=distribution_name
+ ),
+ name=Equals(distroseries_name),
+ ),
+ architecturetag=Equals(architecturetag),
+ )
+
+
class TestGetAllCommitsForPaths(TestCaseWithFactory):
layer = LaunchpadZopelessLayer
@@ -537,12 +550,34 @@ class TestCIBuild(TestCaseWithFactory):
build = self.factory.makeCIBuild()
self.assertThat(
build.getOrCreateRevisionStatusReport("build:0"),
+ MatchesStructure(
+ creator=Equals(build.git_repository.owner),
+ title=Equals("build:0"),
+ git_repository=Equals(build.git_repository),
+ commit_sha1=Equals(build.commit_sha1),
+ ci_build=Equals(build),
+ distro_arch_series=Is(None),
+ ),
+ )
+
+ def test_getOrCreateRevisionStatusReport_absent_with_das(self):
+ build = self.factory.makeCIBuild()
+ job_das = self.factory.makeDistroArchSeries(
+ distroseries=self.factory.makeDistroSeries(
+ distribution=build.distro_arch_series.distroseries.distribution
+ )
+ )
+ self.assertThat(
+ build.getOrCreateRevisionStatusReport(
+ "build:0", distro_arch_series=job_das
+ ),
MatchesStructure.byEquality(
creator=build.git_repository.owner,
title="build:0",
git_repository=build.git_repository,
commit_sha1=build.commit_sha1,
ci_build=build,
+ distro_arch_series=job_das,
),
)
@@ -994,13 +1029,13 @@ class TestCIBuildSet(TestCaseWithFactory):
)
def test_requestBuildsForRefs_triggers_builds(self):
- ubuntu = getUtility(ILaunchpadCelebrities).ubuntu
- series = self.factory.makeDistroSeries(
- distribution=ubuntu,
- name="focal",
+ self.factory.makeBuildableDistroArchSeries(
+ distroseries=self.factory.makeUbuntuDistroSeries(name="bionic"),
+ architecturetag="amd64",
)
self.factory.makeBuildableDistroArchSeries(
- distroseries=series, architecturetag="amd64"
+ distroseries=self.factory.makeUbuntuDistroSeries(name="focal"),
+ architecturetag="amd64",
)
configuration = dedent(
"""\
@@ -1052,8 +1087,10 @@ class TestCIBuildSet(TestCaseWithFactory):
# check that a build and some reports were created
self.assertEqual(ref.commit_sha1, build.commit_sha1)
- self.assertEqual("focal", build.distro_arch_series.distroseries.name)
- self.assertEqual("amd64", build.distro_arch_series.architecturetag)
+ self.assertThat(
+ build.distro_arch_series,
+ MatchesDistroArchSeries("ubuntu", "focal", "amd64"),
+ )
self.assertEqual(
[[("build", 0), ("build", 1)], [("test", 0)]], build.stages
)
@@ -1061,14 +1098,21 @@ class TestCIBuildSet(TestCaseWithFactory):
reports,
MatchesSetwise(
*(
- MatchesStructure.byEquality(
- creator=repository.owner,
- title=title,
- git_repository=repository,
- commit_sha1=ref.commit_sha1,
- ci_build=build,
+ MatchesStructure(
+ creator=Equals(repository.owner),
+ title=Equals(title),
+ git_repository=Equals(repository),
+ commit_sha1=Equals(ref.commit_sha1),
+ ci_build=Equals(build),
+ distro_arch_series=MatchesDistroArchSeries(
+ "ubuntu", distroseries_name, "amd64"
+ ),
+ )
+ for title, distroseries_name in (
+ ("build:0", "bionic"),
+ ("build:1", "focal"),
+ ("test:0", "focal"),
)
- for title in ("build:0", "build:1", "test:0")
)
),
)
@@ -1162,23 +1206,26 @@ class TestCIBuildSet(TestCaseWithFactory):
reports,
MatchesSetwise(
*(
- MatchesStructure.byEquality(
- creator=repository.owner,
- title=title,
- git_repository=repository,
- commit_sha1=ref.commit_sha1,
- ci_build=build,
+ MatchesStructure(
+ creator=Equals(repository.owner),
+ title=Equals(title),
+ git_repository=Equals(repository),
+ commit_sha1=Equals(ref.commit_sha1),
+ ci_build=Equals(build),
+ distro_arch_series=MatchesDistroArchSeries(
+ "ubuntu", distroseries_name, architecturetag
+ ),
)
- for title, build in [
- # amd
- ("build:0", jammy_test_amd64),
- ("test:0", jammy_test_amd64),
- ("test:1", jammy_test_amd64),
- ("test:2", jammy_test_amd64),
- # arm
- ("build:0", jammy_test_arm64),
- ("test:1", jammy_test_arm64),
- ("test:2", jammy_test_arm64),
+ for title, build, distroseries_name, architecturetag in [
+ # amd64
+ ("build:0", jammy_test_amd64, "focal", "amd64"),
+ ("test:0", jammy_test_amd64, "bionic", "amd64"),
+ ("test:1", jammy_test_amd64, "focal", "amd64"),
+ ("test:2", jammy_test_amd64, "jammy", "amd64"),
+ # arm64
+ ("build:0", jammy_test_arm64, "focal", "arm64"),
+ ("test:1", jammy_test_arm64, "focal", "arm64"),
+ ("test:2", jammy_test_arm64, "jammy", "arm64"),
]
)
),
@@ -1253,16 +1300,19 @@ class TestCIBuildSet(TestCaseWithFactory):
reports,
MatchesSetwise(
*(
- MatchesStructure.byEquality(
- creator=repository.owner,
- title=title,
- git_repository=repository,
- commit_sha1=ref.commit_sha1,
- ci_build=build,
+ MatchesStructure(
+ creator=Equals(repository.owner),
+ title=Equals(title),
+ git_repository=Equals(repository),
+ commit_sha1=Equals(ref.commit_sha1),
+ ci_build=Equals(build),
+ distro_arch_series=MatchesDistroArchSeries(
+ "ubuntu", distroseries_name, "amd64"
+ ),
)
- for title, build in [
- ("build:0", jammy_build),
- ("test:0", jammy_build),
+ for title, build, distroseries_name in [
+ ("build:0", jammy_build, "jammy"),
+ ("test:0", jammy_build, "focal"),
]
)
),
diff --git a/lib/lp/testing/factory.py b/lib/lp/testing/factory.py
index 4479b9d..60cb62e 100644
--- a/lib/lp/testing/factory.py
+++ b/lib/lp/testing/factory.py
@@ -2203,6 +2203,7 @@ class LaunchpadObjectFactory(ObjectFactory):
result=None,
ci_build=None,
properties=None,
+ distro_arch_series=None,
):
"""Create a new RevisionStatusReport."""
if title is None:
@@ -2231,6 +2232,7 @@ class LaunchpadObjectFactory(ObjectFactory):
result,
ci_build=ci_build,
properties=properties,
+ distro_arch_series=distro_arch_series,
)
def makeRevisionStatusArtifact(
