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(
_______________________________________________ Mailing list: https://launchpad.net/~launchpad-reviewers Post to : launchpad-reviewers@lists.launchpad.net Unsubscribe : https://launchpad.net/~launchpad-reviewers More help : https://help.launchpad.net/ListHelp