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

Currently the mqtt reporter uses the report url as log_url. This is
fine as long as report-build-page is disabled. As soon as
report-build-page is enabled on a tenant it reports the url to the
result page of the build. As mqtt is meant to be consumed by machines
this breaks e.g. log post processing.

Fix this by reporting the real log url as log_url and add the field
web_url for use cases where really the human url is required.

This fixes also a wrong indentation in the mqtt driver documentation,
resulting in all buildset.builds.* attributes being listed as buildset.*
attributes.

Change-Id: I91ce93a7000ddd0d70ce504b70742262d8239a8f

doc/source/reference/drivers/mqtt.rst

@@ -72,7 +72,7 @@ An MQTT report uses this schema:
 
       .. attr:: builds
 
-      The list of builds.
+         The list of builds.
 
          .. attr:: job_name
 
@@ -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,