From 3f16de51e7f47bb2e29a85f40cd8dbafc632ae7a Mon Sep 17 00:00:00 2001 From: Jamie Lennox Date: Tue, 9 May 2017 14:24:11 +1000 Subject: [PATCH] Expose buildset to the executor and url formatter For github (and probably other providers) we really only have the option of returning 1 url for the entire buildset, as opposed to 1 per build. To make log uploading within that easier we really need a way to globally identify all the different builds that belong to 1 change. The zuul ref is already available however this is a concept that is planned to be deprecated, so instead add a UUID parameter to the buildset that we can pass through. This UUID is used to build the ref to make migration easier. Change-Id: I1cab8af5c9d7f6875591fbe4ac4e184b90f6ca12 Signed-off-by: Jamie Lennox --- tests/fixtures/layouts/reporting-github.yaml | 2 +- tests/unit/test_github_driver.py | 19 +++++++++++++++---- zuul/executor/client.py | 1 + zuul/model.py | 18 +++++++++++++++--- 4 files changed, 32 insertions(+), 8 deletions(-) diff --git a/tests/fixtures/layouts/reporting-github.yaml b/tests/fixtures/layouts/reporting-github.yaml index 43e71c7a72..d054df7a26 100644 --- a/tests/fixtures/layouts/reporting-github.yaml +++ b/tests/fixtures/layouts/reporting-github.yaml @@ -29,7 +29,7 @@ github: comment: false status: 'success' - status-url: http://logs.example.com/{tenant.name}/{pipeline.name}/{change.project}/{change.number}/{change.patchset}/ + status-url: http://logs.example.com/{tenant.name}/{pipeline.name}/{change.project}/{change.number}/{buildset.uuid}/ failure: github: comment: false diff --git a/tests/unit/test_github_driver.py b/tests/unit/test_github_driver.py index ab304b2a67..636abb48ba 100644 --- a/tests/unit/test_github_driver.py +++ b/tests/unit/test_github_driver.py @@ -13,7 +13,7 @@ # under the License. import re -from testtools.matchers import MatchesRegex +from testtools.matchers import MatchesRegex, StartsWith import time from tests.base import ZuulTestCase, simple_layout, random_sha1 @@ -300,9 +300,20 @@ class TestGithubDriver(ZuulTestCase): self.assertEqual('tenant-one/reporting', report_status['context']) self.assertEqual('success', report_status['state']) self.assertEqual(2, len(A.comments)) - report_url = ('http://logs.example.com/tenant-one/reporting/' - '%s/%s/%s/' % (A.project, A.number, A.head_sha)) - self.assertEqual(report_url, report_status['url']) + + base = 'http://logs.example.com/tenant-one/reporting/%s/%s/' % ( + A.project, A.number) + + # Deconstructing the URL because we don't save the BuildSet UUID + # anywhere to do a direct comparison and doing regexp matches on a full + # URL is painful. + + # The first part of the URL matches the easy base string + self.assertThat(report_status['url'], StartsWith(base)) + + # The rest of the URL is a UUID and a trailing slash. + self.assertThat(report_status['url'][len(base):], + MatchesRegex('^[a-fA-F0-9]{32}\/$')) @simple_layout('layouts/merging-github.yaml', driver='github') def test_report_pull_merge(self): diff --git a/zuul/executor/client.py b/zuul/executor/client.py index b7e5a4594d..52074a1baa 100644 --- a/zuul/executor/client.py +++ b/zuul/executor/client.py @@ -193,6 +193,7 @@ class ExecutorClient(object): canonical_name=item.change.project.canonical_name) zuul_params = dict(uuid=uuid, + ref=item.current_build_set.ref, pipeline=pipeline.name, job=job.name, project=project, diff --git a/zuul/model.py b/zuul/model.py index 59f5531272..612aa6f605 100644 --- a/zuul/model.py +++ b/zuul/model.py @@ -1194,7 +1194,7 @@ class BuildSet(object): self.result = None self.next_build_set = None self.previous_build_set = None - self.ref = None + self.uuid = None self.commit = None self.zuul_url = None self.dependent_items = None @@ -1210,6 +1210,13 @@ class BuildSet(object): self.layout = None self.tries = {} + @property + def ref(self): + # NOTE(jamielennox): The concept of buildset ref is to be removed and a + # buildset UUID identifier available instead. Currently the ref is + # checked to see if the BuildSet has been configured. + return 'Z' + self.uuid if self.uuid else None + def __repr__(self): return '' % ( self.item, @@ -1227,8 +1234,8 @@ class BuildSet(object): items.append(next_item) next_item = next_item.item_ahead self.dependent_items = items - if not self.ref: - self.ref = 'Z' + uuid4().hex + if not self.uuid: + self.uuid = uuid4().hex if self.merger_items is None: items = [self.item] + self.dependent_items items.reverse() @@ -1309,6 +1316,9 @@ class BuildSet(object): return project_config.merge_mode return MERGER_MERGE_RESOLVE + def getSafeAttributes(self): + return Attributes(uuid=self.uuid) + class QueueItem(object): """Represents the position of a Change in a ChangeQueue. @@ -1589,12 +1599,14 @@ class QueueItem(object): safe_change = self.change.getSafeAttributes() safe_pipeline = self.pipeline.getSafeAttributes() safe_tenant = self.pipeline.layout.tenant.getSafeAttributes() + safe_buildset = self.current_build_set.getSafeAttributes() safe_job = job.getSafeAttributes() if job else {} safe_build = build.getSafeAttributes() if build else {} try: url = url_pattern.format(change=safe_change, pipeline=safe_pipeline, tenant=safe_tenant, + buildset=safe_buildset, job=safe_job, build=safe_build) except KeyError as e: