Merge "Fix mqtt log url reporting when report-build-page is active"

doc/source/reference/drivers/mqtt.rst

@@ -102,6 +102,12 @@ An MQTT report uses this schema:
 
             The build log url (not present in start report).
 
+         .. attr:: web_url
+
+            The url to the build result page if :attr:`tenant.report-build-page`
+            is enabled. The build log url otherwise (not present in start
+            report).
+
          .. attr:: result
 
             The build results (not present in start report).
@@ -163,6 +169,7 @@ Here is an example of a success message:
         'start_time': 1524801179.8557224,
         'end_time': 1524801208.928095,
         'log_url': 'https://logs.example.com/logs/3/3/1/check/linters/16e3e55/',
+        'web_url': 'https://tenant.example.com/t/tenant-one/build/16e3e55aca984c6c9a50cc3c5b21bb83/',
         'result': 'SUCCESS',
         'dependencies': []
       }],

releasenotes/notes/mqtt-report-build-page-1780225ca98c7456.yaml

@@ -0,0 +1,13 @@
+---
+fixes:
+  - |
+    Although the documentation states that the MQTT reporter reports the build's
+    log_url, this was only true as long as :attr:`tenant.report-build-page` was
+    disabled. As soon as the setting was enabled, the MQTT reporter reported the
+    url to the build's result page in Zuul. As MQTT is meant to be consumed by
+    machines, this broke use cases like log post processing.
+
+    This was fixed so that the :attr:`<mqtt schema>.buildset.builds.log_url` now
+    always contains the log url, while an additional field
+    :attr:`<mqtt schema>.buildset.builds.web_url` contains the url to the
+    build's result page if :attr:`tenant.report-build-page` is enabled.

tests/fixtures/config/mqtt-driver-report-build-page/git/common-config/playbooks/test.yaml

@@ -0,0 +1,2 @@
+- hosts: all
+  tasks: []

tests/fixtures/config/mqtt-driver-report-build-page/git/common-config/zuul.d/config.yaml

@@ -0,0 +1,45 @@
+- pipeline:
+    name: check
+    manager: independent
+    trigger:
+      gerrit:
+        - event: patchset-created
+    start:
+      mqtt:
+        topic: "{tenant}/zuul_start/{pipeline}/{project}/{branch}"
+    success:
+      gerrit:
+        Verified: 1
+      mqtt:
+        topic: "{tenant}/zuul_buildset/{pipeline}/{project}/{branch}"
+    failure:
+      gerrit:
+        Verified: -1
+      mqtt:
+        topic: "{tenant}/zuul_buildset/{pipeline}/{project}/{branch}"
+
+- job:
+    name: base
+    parent: null
+
+- job:
+    name: test
+    run: playbooks/test.yaml
+
+- job:
+    name: dependent-test
+    run: playbooks/test.yaml
+
+- project:
+    name: org/project
+    check:
+      jobs:
+        - test
+        - dependent-test:
+            dependencies:
+              - test
+
+- project:
+    name: common-config
+    check:
+      jobs: []

tests/fixtures/config/mqtt-driver-report-build-page/git/org_project/README

@@ -0,0 +1 @@
+test

tests/fixtures/config/mqtt-driver-report-build-page/main.yaml

@@ -0,0 +1,9 @@
+- tenant:
+    name: tenant-one
+    report-build-page: true
+    source:
+      gerrit:
+        config-projects:
+          - common-config
+        untrusted-projects:
+          - org/project

tests/fixtures/zuul-mqtt-driver.conf

@@ -20,3 +20,6 @@ sshkey=fake_id_rsa1
 
 [connection mqtt]
 driver=mqtt
+
+[web]
+root=https://tenant.example.com/

tests/unit/test_connection.py

@@ -542,6 +542,9 @@ class TestMQTTConnection(ZuulTestCase):
         "Test the MQTT reporter"
         # Add a success result
         A = self.fake_gerrit.addFakeChange('org/project', 'master', 'A')
+        self.executor_server.returnData(
+            "test", A, {"zuul": {"log_url": "some-log-url"}}
+        )
         self.fake_gerrit.addEvent(A.getPatchsetCreatedEvent(1))
         self.waitUntilSettled()
 
@@ -572,6 +575,10 @@ class TestMQTTConnection(ZuulTestCase):
         self.assertEquals(test_job['job_name'], 'test')
         self.assertEquals(test_job['result'], 'SUCCESS')
         self.assertEquals(test_job['dependencies'], [])
+        # Both log- and web-url should point to the same URL which is specified
+        # in the build result data under zuul.log_url.
+        self.assertEquals(test_job['log_url'], 'some-log-url/')
+        self.assertEquals(test_job['web_url'], 'some-log-url')
         self.assertIn('execute_time', test_job)
         self.assertIn('timestamp', mqtt_payload)
         self.assertIn('enqueue_time', mqtt_payload)
@@ -600,3 +607,48 @@ class TestMQTTConnection(ZuulTestCase):
 
         self.assertIn("topic component 'bad' is invalid", A.messages[0],
                       "A should report a syntax error")
+
+
+class TestMQTTConnectionBuildPage(ZuulTestCase):
+    config_file = "zuul-mqtt-driver.conf"
+    tenant_config_file = "config/mqtt-driver-report-build-page/main.yaml"
+
+    def test_mqtt_reporter(self):
+        "Test the MQTT reporter with 'report-build-page' enabled"
+
+        # Add a sucess result
+        A = self.fake_gerrit.addFakeChange("org/project", "master", "A")
+        self.executor_server.returnData(
+            "test", A, {"zuul": {"log_url": "some-log-url"}}
+        )
+        self.fake_gerrit.addEvent(A.getPatchsetCreatedEvent(1))
+        self.waitUntilSettled()
+
+        success_event = self.mqtt_messages.pop()
+
+        self.assertEquals(
+            success_event.get("topic"),
+            "tenant-one/zuul_buildset/check/org/project/master",
+        )
+
+        mqtt_payload = success_event["msg"]
+        self.assertEquals(mqtt_payload["project"], "org/project")
+        self.assertEquals(mqtt_payload["branch"], "master")
+        self.assertEquals(mqtt_payload["buildset"]["result"], "SUCCESS")
+
+        builds = mqtt_payload["buildset"]["builds"]
+        test_job = [b for b in builds if b["job_name"] == "test"][0]
+        self.assertEquals(test_job["job_name"], "test")
+        self.assertEquals(test_job["result"], "SUCCESS")
+
+        build_id = test_job["uuid"]
+        # When report-build-page is enabled, the log_url should still point to
+        # the URL that is specified in the build result data under
+        # zuul.log_url. The web_url will instead point to the builds page.
+        self.assertEquals(test_job["log_url"], "some-log-url/")
+        self.assertEquals(
+            test_job["web_url"],
+            "https://tenant.example.com/t/tenant-one/build/{}".format(
+                build_id
+            ),
+        )

zuul/driver/mqtt/mqttreporter.py

@@ -61,13 +61,14 @@ class MQTTReporter(BaseReporter):
             build = item.current_build_set.getBuild(job.name)
             if build:
                 # Report build data if available
-                (result, url) = item.formatJobResult(job)
+                (result, web_url) = item.formatJobResult(job)
                 job_informations.update({
                     'uuid': build.uuid,
                     'start_time': build.start_time,
                     'end_time': build.end_time,
                     'execute_time': build.execute_time,
-                    'log_url': url,
+                    'log_url': build.log_url,
+                    'web_url': web_url,
                     'result': result,
                     'dependencies': [j.name for j in job.dependencies],
                 })

zuul/driver/sql/sqlreporter.py

@@ -30,10 +30,7 @@ class SQLReporter(BaseReporter):
     log = logging.getLogger("zuul.SQLReporter")
 
     def _getBuildData(self, item, job, build):
-        (result, url) = item.formatJobResult(job, build)
-        log_url = build.result_data.get('zuul', {}).get('log_url')
-        if log_url and log_url[-1] != '/':
-            log_url = log_url + '/'
+        (result, _) = item.formatJobResult(job, build)
         start = end = None
         if build.start_time:
             start = datetime.datetime.fromtimestamp(
@@ -43,7 +40,7 @@ class SQLReporter(BaseReporter):
             end = datetime.datetime.fromtimestamp(
                 build.end_time,
                 tz=datetime.timezone.utc)
-        return result, log_url, start, end
+        return result, build.log_url, start, end
 
     def createBuildEntry(self, item, job, db_buildset, build, final=True):
         # Ensure end_time is defined

zuul/model.py

@@ -1906,6 +1906,13 @@ class Build(object):
     def pipeline(self):
         return self.build_set.item.pipeline
 
+    @property
+    def log_url(self):
+        log_url = self.result_data.get('zuul', {}).get('log_url')
+        if log_url and log_url[-1] != '/':
+            log_url = log_url + '/'
+        return log_url
+
     def getSafeAttributes(self):
         return Attributes(uuid=self.uuid,
                           result=self.result,