From f13cc924df19d12b3ca76dd51dce82e256d52d9a Mon Sep 17 00:00:00 2001 From: "James E. Blair" Date: Thu, 8 Aug 2019 09:24:13 -0700 Subject: [PATCH] Add option to report build page This adds a tenant option to use the Zuul web build page as the URL reported to the code review system when a build completes. The setting is per-tenant (because it requires that the tenant have a working SQL reporter configured in all pipelines) and defaults to false, since we can't guarantee that. In the future, we expect to make SQL reporting implicit, then this can default to true and eventually be deprecated. A new zuul.conf option is added and marked required to supply the root web URL. As we perform further integration with the web app, we may be able to deprecate other similar settings, such as "status_url". Change-Id: Iaa3be10525994722d020d2aa5a7dcf141f2404d9 --- doc/source/admin/components.rst | 9 ++++ doc/source/admin/drivers/sql.rst | 2 + doc/source/admin/tenants.rst | 23 ++++++++++ .../report-build-page-49088c2b0a36e1b5.yaml | 17 ++++++++ .../git/common-config/playbooks/python27.yaml | 2 + .../build-page/git/common-config/zuul.yaml | 24 +++++++++++ .../config/build-page/git/org_project1/README | 1 + .../build-page/git/org_project1/zuul.yaml | 4 ++ .../config/build-page/git/org_project2/README | 1 + .../build-page/git/org_project2/zuul.yaml | 4 ++ .../config/build-page/git/org_project3/README | 1 + .../build-page/git/org_project3/zuul.yaml | 4 ++ tests/fixtures/config/build-page/main.yaml | 29 +++++++++++++ tests/fixtures/zuul.conf | 1 + tests/unit/test_scheduler.py | 43 +++++++++++++++++++ zuul/configloader.py | 5 +++ zuul/model.py | 14 +++++- zuul/scheduler.py | 5 +++ 18 files changed, 188 insertions(+), 1 deletion(-) create mode 100644 releasenotes/notes/report-build-page-49088c2b0a36e1b5.yaml create mode 100644 tests/fixtures/config/build-page/git/common-config/playbooks/python27.yaml create mode 100644 tests/fixtures/config/build-page/git/common-config/zuul.yaml create mode 100644 tests/fixtures/config/build-page/git/org_project1/README create mode 100644 tests/fixtures/config/build-page/git/org_project1/zuul.yaml create mode 100644 tests/fixtures/config/build-page/git/org_project2/README create mode 100644 tests/fixtures/config/build-page/git/org_project2/zuul.yaml create mode 100644 tests/fixtures/config/build-page/git/org_project3/README create mode 100644 tests/fixtures/config/build-page/git/org_project3/zuul.yaml create mode 100644 tests/fixtures/config/build-page/main.yaml diff --git a/doc/source/admin/components.rst b/doc/source/admin/components.rst index 74ca471a12..61d63c783a 100644 --- a/doc/source/admin/components.rst +++ b/doc/source/admin/components.rst @@ -250,6 +250,15 @@ The following sections of ``zuul.conf`` are used by the scheduler: .. attr:: web + .. attr:: root + :required: + + The root URL of the web service (e.g., + ``https://zuul.example.com/``). + + See :attr:`tenant.web-root` for additional options for + whitelabeled tenant configuration. + .. attr:: status_url URL that will be posted in Zuul comments made to changes when diff --git a/doc/source/admin/drivers/sql.rst b/doc/source/admin/drivers/sql.rst index e8912cf4bc..85ee05c720 100644 --- a/doc/source/admin/drivers/sql.rst +++ b/doc/source/admin/drivers/sql.rst @@ -51,6 +51,8 @@ The connection options for the SQL driver are: if you rely on external databases which you don't have under control. The default is to have no prefix. +.. _sql_reporter: + Reporter Configuration ---------------------- diff --git a/doc/source/admin/tenants.rst b/doc/source/admin/tenants.rst index 0e619d1d37..7c52478330 100644 --- a/doc/source/admin/tenants.rst +++ b/doc/source/admin/tenants.rst @@ -42,6 +42,7 @@ configuration. Some examples of tenant definitions are: name: my-tenant max-nodes-per-job: 5 exclude-unprotected-branches: false + report-build-page: true source: gerrit: config-projects: @@ -307,6 +308,28 @@ configuration. Some examples of tenant definitions are: this setting can be used to restrict what labels a tenant can use. Without this setting, the tenant can use any labels. + .. attr:: report-build-page + :default: false + + If this is set to ``true``, then Zuul will use the URL of the + build page in Zuul's web interface when reporting to the code + review system. In this case, :attr:`job.success-url` and + :attr:`job.failure-url` are ignored for the report (though they + are still used on the status page before the buildset is + complete and reported). + + This requires that all the pipelines in the tenant have a + :ref:`SQL reporter` configured, and at least one of + :attr:`tenant.web-root` or :attr:`web.root` must be defined. + + .. attr:: web-root + + If this tenant has a whitelabeled installation of zuul-web, set + its externally visible URL here (e.g., + ``https://tenant.example.com/``). This will override the + :attr:`web.root` setting when constructing URLs for this tenant. + + .. _admin_rule_definition: Access Rule diff --git a/releasenotes/notes/report-build-page-49088c2b0a36e1b5.yaml b/releasenotes/notes/report-build-page-49088c2b0a36e1b5.yaml new file mode 100644 index 0000000000..ffb9e8931f --- /dev/null +++ b/releasenotes/notes/report-build-page-49088c2b0a36e1b5.yaml @@ -0,0 +1,17 @@ +--- +features: + - | + An option to use the URL of the Zuul build page when reporting has + been added. This fetaure requires that all the pipelines in the + tenant have a :ref:`SQL reporter` configured, and at + least one of :attr:`tenant.web-root` or :attr:`web.root` must be + defined. + + See :attr:`tenant.report-build-page`. +upgrade: + - | + As further integration with the web interface is planned, the + :attr:`web.root` setting in ``zuul.conf`` is marked required and + future releases may error if it is missing. Please add it to your + configuration now. See :attr:`tenant.web-root` for additional + information about whitelabel tenants. diff --git a/tests/fixtures/config/build-page/git/common-config/playbooks/python27.yaml b/tests/fixtures/config/build-page/git/common-config/playbooks/python27.yaml new file mode 100644 index 0000000000..f679dceaef --- /dev/null +++ b/tests/fixtures/config/build-page/git/common-config/playbooks/python27.yaml @@ -0,0 +1,2 @@ +- hosts: all + tasks: [] diff --git a/tests/fixtures/config/build-page/git/common-config/zuul.yaml b/tests/fixtures/config/build-page/git/common-config/zuul.yaml new file mode 100644 index 0000000000..31f1e27d9a --- /dev/null +++ b/tests/fixtures/config/build-page/git/common-config/zuul.yaml @@ -0,0 +1,24 @@ +- pipeline: + name: check + manager: independent + trigger: + gerrit: + - event: patchset-created + success: + gerrit: + Verified: 1 + failure: + gerrit: + Verified: -1 + +- job: + name: base + parent: null + +- job: + name: python27 + nodeset: + nodes: + - name: controller + label: ubuntu-trusty + run: playbooks/python27.yaml diff --git a/tests/fixtures/config/build-page/git/org_project1/README b/tests/fixtures/config/build-page/git/org_project1/README new file mode 100644 index 0000000000..9daeafb986 --- /dev/null +++ b/tests/fixtures/config/build-page/git/org_project1/README @@ -0,0 +1 @@ +test diff --git a/tests/fixtures/config/build-page/git/org_project1/zuul.yaml b/tests/fixtures/config/build-page/git/org_project1/zuul.yaml new file mode 100644 index 0000000000..9e56410d09 --- /dev/null +++ b/tests/fixtures/config/build-page/git/org_project1/zuul.yaml @@ -0,0 +1,4 @@ +- project: + check: + jobs: + - python27 diff --git a/tests/fixtures/config/build-page/git/org_project2/README b/tests/fixtures/config/build-page/git/org_project2/README new file mode 100644 index 0000000000..9daeafb986 --- /dev/null +++ b/tests/fixtures/config/build-page/git/org_project2/README @@ -0,0 +1 @@ +test diff --git a/tests/fixtures/config/build-page/git/org_project2/zuul.yaml b/tests/fixtures/config/build-page/git/org_project2/zuul.yaml new file mode 100644 index 0000000000..9e56410d09 --- /dev/null +++ b/tests/fixtures/config/build-page/git/org_project2/zuul.yaml @@ -0,0 +1,4 @@ +- project: + check: + jobs: + - python27 diff --git a/tests/fixtures/config/build-page/git/org_project3/README b/tests/fixtures/config/build-page/git/org_project3/README new file mode 100644 index 0000000000..9daeafb986 --- /dev/null +++ b/tests/fixtures/config/build-page/git/org_project3/README @@ -0,0 +1 @@ +test diff --git a/tests/fixtures/config/build-page/git/org_project3/zuul.yaml b/tests/fixtures/config/build-page/git/org_project3/zuul.yaml new file mode 100644 index 0000000000..9e56410d09 --- /dev/null +++ b/tests/fixtures/config/build-page/git/org_project3/zuul.yaml @@ -0,0 +1,4 @@ +- project: + check: + jobs: + - python27 diff --git a/tests/fixtures/config/build-page/main.yaml b/tests/fixtures/config/build-page/main.yaml new file mode 100644 index 0000000000..49513f481c --- /dev/null +++ b/tests/fixtures/config/build-page/main.yaml @@ -0,0 +1,29 @@ +- tenant: + name: tenant-one + web-root: https://one.example.com/ + report-build-page: true + source: + gerrit: + config-projects: + - common-config + untrusted-projects: + - org/project1 + +- tenant: + name: tenant-two + report-build-page: true + source: + gerrit: + config-projects: + - common-config + untrusted-projects: + - org/project2 + +- tenant: + name: tenant-three + source: + gerrit: + config-projects: + - common-config + untrusted-projects: + - org/project3 diff --git a/tests/fixtures/zuul.conf b/tests/fixtures/zuul.conf index d915e38d90..38036c2333 100644 --- a/tests/fixtures/zuul.conf +++ b/tests/fixtures/zuul.conf @@ -33,3 +33,4 @@ default_to=you@example.com [web] static_cache_expiry=1200 +root=https://zuul.example.com/ diff --git a/tests/unit/test_scheduler.py b/tests/unit/test_scheduler.py index e8e71ea925..ca644eb2ef 100644 --- a/tests/unit/test_scheduler.py +++ b/tests/unit/test_scheduler.py @@ -7597,3 +7597,46 @@ class TestPipelineSupersedes(ZuulTestCase): dict(name='test-job', result='ABORTED', changes='1,1'), dict(name='test-job', result='SUCCESS', changes='1,1'), ], ordered=False) + + +class TestReportBuildPage(ZuulTestCase): + tenant_config_file = 'config/build-page/main.yaml' + + def test_tenant_url(self): + """ + Test that the tenant url is used in reporting the build page. + """ + A = self.fake_gerrit.addFakeChange('org/project1', 'master', 'A') + self.fake_gerrit.addEvent(A.getPatchsetCreatedEvent(1)) + self.waitUntilSettled() + self.assertHistory([ + dict(name='python27', result='SUCCESS', changes='1,1'), + ]) + self.assertIn('python27 https://one.example.com/build/', + A.messages[0]) + + def test_base_url(self): + """ + Test that the web base url is used in reporting the build page. + """ + A = self.fake_gerrit.addFakeChange('org/project2', 'master', 'A') + self.fake_gerrit.addEvent(A.getPatchsetCreatedEvent(1)) + self.waitUntilSettled() + self.assertHistory([ + dict(name='python27', result='SUCCESS', changes='1,1'), + ]) + self.assertIn('python27 https://zuul.example.com/t/tenant-two/build/', + A.messages[0]) + + def test_no_build_page(self): + """ + Test that we fall back to the old behavior if the tenant is + not configured to report the build page + """ + A = self.fake_gerrit.addFakeChange('org/project3', 'master', 'A') + self.fake_gerrit.addEvent(A.getPatchsetCreatedEvent(1)) + self.waitUntilSettled() + self.assertHistory([ + dict(name='python27', result='SUCCESS', changes='1,1'), + ]) + self.assertIn('python27 finger://', A.messages[0]) diff --git a/zuul/configloader.py b/zuul/configloader.py index cd795de93b..bb7cb34816 100644 --- a/zuul/configloader.py +++ b/zuul/configloader.py @@ -1454,6 +1454,8 @@ class TenantParser(object): 'default-parent': str, 'default-ansible-version': vs.Any(str, float), 'admin-rules': to_list(str), + 'report-build-page': bool, + 'web-root': str, } return vs.Schema(tenant) @@ -1469,6 +1471,9 @@ class TenantParser(object): conf['exclude-unprotected-branches'] if conf.get('admin-rules') is not None: tenant.authorization_rules = conf['admin-rules'] + if conf.get('report-build-page') is not None: + tenant.report_build_page = conf['report-build-page'] + tenant.web_root = conf.get('web-root', self.scheduler.web_root) tenant.allowed_triggers = conf.get('allowed-triggers') tenant.allowed_reporters = conf.get('allowed-reporters') tenant.allowed_labels = conf.get('allowed-labels') diff --git a/zuul/model.py b/zuul/model.py index de2d61ff53..30739a1a35 100644 --- a/zuul/model.py +++ b/zuul/model.py @@ -2670,6 +2670,17 @@ class QueueItem(object): return url def formatJobResult(self, job): + if (self.pipeline.tenant.report_build_page and + self.pipeline.tenant.web_root): + build = self.current_build_set.getBuild(job.name) + pattern = urllib.parse.urljoin(self.pipeline.tenant.web_root, + 'build/{build.uuid}') + url = self.formatUrlPattern(pattern, job, build) + return (build.result, url) + else: + return self.formatProvisionalJobResult(job) + + def formatProvisionalJobResult(self, job): build = self.current_build_set.getBuild(job.name) result = build.result pattern = None @@ -2764,7 +2775,7 @@ class QueueItem(object): urlformat += '&websocket_url={websocket_url}' build_url = urlformat.format( build=build, websocket_url=websocket_url) - (unused, report_url) = self.formatJobResult(job) + (unused, report_url) = self.formatProvisionalJobResult(job) if build.start_time: if build.end_time: elapsed = int((build.end_time - @@ -4217,6 +4228,7 @@ class Tenant(object): self.max_job_timeout = 10800 self.exclude_unprotected_branches = False self.default_base_job = None + self.report_build_page = False self.layout = None # The unparsed configuration from the main zuul config for # this tenant. diff --git a/zuul/scheduler.py b/zuul/scheduler.py index 48b0357457..e5d4da5da9 100644 --- a/zuul/scheduler.py +++ b/zuul/scheduler.py @@ -25,6 +25,7 @@ import socket import sys import threading import time +import urllib from zuul import configloader from zuul import model @@ -330,6 +331,10 @@ class Scheduler(threading.Thread): if self.config.has_option('scheduler', 'relative_priority'): if self.config.getboolean('scheduler', 'relative_priority'): self.use_relative_priority = True + web_root = get_default(self.config, 'web', 'root', None) + if web_root: + web_root = urllib.parse.urljoin(web_root, 't/{tenant.name}/') + self.web_root = web_root default_ansible_version = get_default( self.config, 'scheduler', 'default_ansible_version', None)