diff --git a/doc/source/images/check1-1002.png b/doc/source/images/check1-1002.png index 7230b55d69..710a00a168 100644 Binary files a/doc/source/images/check1-1002.png and b/doc/source/images/check1-1002.png differ diff --git a/doc/source/images/check2-1002.png b/doc/source/images/check2-1002.png index 9cb3a6d699..69b393838f 100644 Binary files a/doc/source/images/check2-1002.png and b/doc/source/images/check2-1002.png differ diff --git a/doc/source/reference/drivers/mqtt.rst b/doc/source/reference/drivers/mqtt.rst index be8fe67738..0826cf28c1 100644 --- a/doc/source/reference/drivers/mqtt.rst +++ b/doc/source/reference/drivers/mqtt.rst @@ -120,9 +120,8 @@ An MQTT report uses this schema: .. 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). + The url to the build result page. Not present in start + report. .. attr:: result diff --git a/doc/source/reference/job_def.rst b/doc/source/reference/job_def.rst index 3438e67c6e..f4e7e731ba 100644 --- a/doc/source/reference/job_def.rst +++ b/doc/source/reference/job_def.rst @@ -183,25 +183,6 @@ Here is an example of two job definitions: the result for the job. If set, this option may be used to supply a different string. - .. attr:: success-url - - When a job succeeds, this URL is reported along with the result. - If this value is not supplied, Zuul uses the content of the job - :ref:`return value ` **zuul.log_url**. This is - recommended as it allows the code which stores the URL to the - job artifacts to report exactly where they were stored. To - override this value, or if it is not set, supply an absolute URL - in this field. If a relative URL is supplied in this field, and - **zuul.log_url** is set, then the two will be combined to - produce the URL used for the report. This can be used to - specify that certain jobs should "deep link" into the stored job - artifacts. - - .. attr:: failure-url - - When a job fails, this URL is reported along with the result. - Otherwise behaves the same as **success-url**. - .. attr:: hold-following-changes :default: false diff --git a/doc/source/reference/tenants.rst b/doc/source/reference/tenants.rst index aad51b7d53..d05154dd02 100644 --- a/doc/source/reference/tenants.rst +++ b/doc/source/reference/tenants.rst @@ -42,7 +42,6 @@ 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: @@ -322,20 +321,6 @@ configuration. Some examples of tenant definitions are: check for `allowed-labels` and may therefore be used to further restrict the set of permitted 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 diff --git a/doc/source/tutorials/quick-start.rst b/doc/source/tutorials/quick-start.rst index 561f067cfe..b5c3b26af9 100644 --- a/doc/source/tutorials/quick-start.rst +++ b/doc/source/tutorials/quick-start.rst @@ -385,10 +385,10 @@ uploaded, Zuul will run the new job and report back on the change. Visit http://localhost:8080/dashboard/self and open the change you just uploaded. If the build is complete, Zuul should have left a Verified: +1 vote on the change, along with a comment at the bottom. -Expand the comments and you should see that the job succeeded, but -there are no logs and no way to see the output, only a `finger` URL -(which is what Zuul reports when it doesn't know where build logs are -stored). +Expand the comments and you should see that the job succeeded, and a +link to the build result in Zuul is provided. You can follow that +link to see some information about the build, but you won't find any +logs since Zuul hasn't been told where to save them yet. .. image:: /images/check1-1002.png :align: center @@ -523,9 +523,14 @@ the change: .. image:: /images/check2-1002.png :align: center -Follow the link and you will be able to browse the console log for the -job. In the middle of the log, you should see the "Hello, world!" -output from the job's playbook. +Follow the link and you will be directed to the build result page. If +you click on the `Logs` tab, you'll be able to browse the console log +for the job. In the middle of the log, you should see the "Hello, +world!" output from the job's playbook. + +Also try the `Console` tab for a more structured view of the log. +Click on the `OK` button in the middle of the page to see the output +of just the task we're interested in. Further Steps ------------- diff --git a/playbooks/tutorial/quick-start.yaml b/playbooks/tutorial/quick-start.yaml index f2f4d95c8a..8f114f16f7 100644 --- a/playbooks/tutorial/quick-start.yaml +++ b/playbooks/tutorial/quick-start.yaml @@ -29,12 +29,12 @@ - name: Save test1 change info set_fact: changetest1: "{{ changeinfo }}" - json_query_finger: "messages[?contains(@.message, 'testjob finger://')].message | [0]" + json_query_finger: "messages[?contains(@.message, 'testjob http://')].message | [0]" -- name: "Fetch url finger://" +- name: "Check build URL" assert: that: - - (result_json | to_json | from_json | json_query(json_query_finger) | regex_search('(finger://[^ ]*)') | length > 0) + - (result_json | to_json | from_json | json_query(json_query_finger) | regex_search('(http://[^ ]*)') | length > 0) - name: Configure a Base Job zuul-config copy: @@ -82,15 +82,26 @@ projectname: test1 check_number: 2 -- name: Find the log URL with regex +- name: Find the build URL with regex set_fact: - log_url: "{{ result_json | to_json | from_json | json_query(json_query_log_url) | regex_search('(http://[^ ]*)') }}" + build_url: "{{ result_json | to_json | from_json | json_query(json_query_log_url) | regex_search('(http://[^ ]*)') }}" vars: - json_query_log_url: "messages[?contains(@.message, 'http://')].message | [0]" + json_query_log_url: "messages[?contains(@.message, 'http://')].message | [1]" + +- name: Extract build id from build url + set_fact: + build_uuid: "{{ build_url.split('/')[-1] }}" + +- name: Find log URL via Zuul API + uri: + url: "http://localhost:9000/api/tenant/example-tenant/build/{{ build_uuid }}" + method: GET + return_content: true + register: result - name: Fetch log URL get_url: - url: "{{ log_url }}job-output.txt" + url: "{{ result.json.log_url }}job-output.txt" dest: "{{ workspace }}/job-output.txt" - name: Verify log contents diff --git a/releasenotes/notes/mqtt-report-build-page-1780225ca98c7457.yaml b/releasenotes/notes/mqtt-report-build-page-1780225ca98c7457.yaml index 34b1a7348b..0d1b223679 100644 --- a/releasenotes/notes/mqtt-report-build-page-1780225ca98c7457.yaml +++ b/releasenotes/notes/mqtt-report-build-page-1780225ca98c7457.yaml @@ -2,7 +2,7 @@ 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 + log_url, this was only true as long as ``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. @@ -10,4 +10,4 @@ fixes: This was fixed so that the :attr:`.buildset.builds.log_url` now always contains the log url, while an additional field :attr:`.buildset.builds.web_url` contains the url to the - build's result page if :attr:`tenant.report-build-page` is enabled. + build's result page if ``tenant.report-build-page`` is enabled. diff --git a/releasenotes/notes/remove-build-page-8d6acda308d513f1.yaml b/releasenotes/notes/remove-build-page-8d6acda308d513f1.yaml new file mode 100644 index 0000000000..18df9c5944 --- /dev/null +++ b/releasenotes/notes/remove-build-page-8d6acda308d513f1.yaml @@ -0,0 +1,24 @@ +--- +deprecations: + - | + The following attributes are now ignored: + + * The ``report-build-page`` tenant configuration setting. + * The ``success-url`` job attribute. + * The ``failure-url`` job attribute. + + The URL of the build page is now always reported for every build. + This is now also true for in-progress builds, which provides for a + more consistent user experience. + + Since the build page is always reported as the URL, the success + and failure URL job attributes are no longer useful, so this + functionality has also been removed. + + Zuul's configuration syntax checker will continue to allow these + settings for now (they are simply ignored) but this will be + removed in version 5.0 and using them will be considered an error. + + To achieve a similar result, consider returning the URL as an + artifact from the job via `zuul_return`. This will cause a link + to appear in the "Artifacts" section of the build page. diff --git a/releasenotes/notes/report-build-page-2-49088c2b0a36e1b7.yaml b/releasenotes/notes/report-build-page-2-49088c2b0a36e1b7.yaml index 3d78d12beb..8f662a5345 100644 --- a/releasenotes/notes/report-build-page-2-49088c2b0a36e1b7.yaml +++ b/releasenotes/notes/report-build-page-2-49088c2b0a36e1b7.yaml @@ -7,7 +7,7 @@ features: least one of :attr:`tenant.web-root` or :attr:`web.root` must be defined. - See :attr:`tenant.report-build-page`. + See ``tenant.report-build-page``. upgrade: - | As further integration with the web interface is planned, the diff --git a/tests/fixtures/config/ansible/git/common-config/zuul.yaml b/tests/fixtures/config/ansible/git/common-config/zuul.yaml index 2555d2f375..78f5f30fd9 100644 --- a/tests/fixtures/config/ansible/git/common-config/zuul.yaml +++ b/tests/fixtures/config/ansible/git/common-config/zuul.yaml @@ -79,14 +79,8 @@ - name: test_node label: test_label -- job: - name: base-urls - success-url: https://success.example.com/zuul-logs/{build.uuid}/ - failure-url: https://failure.example.com/zuul-logs/{build.uuid}/ - - job: name: python27 - parent: base-urls run: playbooks/python27.yaml pre-run: playbooks/pre.yaml post-run: playbooks/post.yaml @@ -236,7 +230,6 @@ - ubuntu-xenial - job: - parent: base-urls name: hello run: playbooks/hello-post.yaml post-run: playbooks/hello-post.yaml diff --git a/tests/fixtures/config/build-page/main.yaml b/tests/fixtures/config/build-page/main.yaml index 49513f481c..836fb668a2 100644 --- a/tests/fixtures/config/build-page/main.yaml +++ b/tests/fixtures/config/build-page/main.yaml @@ -1,7 +1,6 @@ - tenant: name: tenant-one web-root: https://one.example.com/ - report-build-page: true source: gerrit: config-projects: @@ -11,7 +10,6 @@ - tenant: name: tenant-two - report-build-page: true source: gerrit: config-projects: diff --git a/tests/fixtures/config/data-return/git/common-config/zuul.yaml b/tests/fixtures/config/data-return/git/common-config/zuul.yaml index 1c1f369bda..05a53c558d 100644 --- a/tests/fixtures/config/data-return/git/common-config/zuul.yaml +++ b/tests/fixtures/config/data-return/git/common-config/zuul.yaml @@ -34,7 +34,6 @@ - job: name: data-return-relative - success-url: docs/index.html run: playbooks/data-return-relative.yaml - job: diff --git a/tests/fixtures/config/mqtt-driver-report-build-page/git/common-config/playbooks/test.yaml b/tests/fixtures/config/mqtt-driver-report-build-page/git/common-config/playbooks/test.yaml deleted file mode 100644 index f679dceaef..0000000000 --- a/tests/fixtures/config/mqtt-driver-report-build-page/git/common-config/playbooks/test.yaml +++ /dev/null @@ -1,2 +0,0 @@ -- hosts: all - tasks: [] diff --git a/tests/fixtures/config/mqtt-driver-report-build-page/git/common-config/zuul.d/config.yaml b/tests/fixtures/config/mqtt-driver-report-build-page/git/common-config/zuul.d/config.yaml deleted file mode 100644 index c842e94246..0000000000 --- a/tests/fixtures/config/mqtt-driver-report-build-page/git/common-config/zuul.d/config.yaml +++ /dev/null @@ -1,45 +0,0 @@ -- 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: [] diff --git a/tests/fixtures/config/mqtt-driver-report-build-page/git/org_project/README b/tests/fixtures/config/mqtt-driver-report-build-page/git/org_project/README deleted file mode 100644 index 9daeafb986..0000000000 --- a/tests/fixtures/config/mqtt-driver-report-build-page/git/org_project/README +++ /dev/null @@ -1 +0,0 @@ -test diff --git a/tests/fixtures/config/mqtt-driver-report-build-page/main.yaml b/tests/fixtures/config/mqtt-driver-report-build-page/main.yaml deleted file mode 100644 index 2a5a4f4cc3..0000000000 --- a/tests/fixtures/config/mqtt-driver-report-build-page/main.yaml +++ /dev/null @@ -1,9 +0,0 @@ -- tenant: - name: tenant-one - report-build-page: true - source: - gerrit: - config-projects: - - common-config - untrusted-projects: - - org/project diff --git a/tests/fixtures/config/success-url/git/common-config/playbooks/docs-draft-test.yaml b/tests/fixtures/config/success-url/git/common-config/playbooks/docs-draft-test.yaml deleted file mode 100644 index f679dceaef..0000000000 --- a/tests/fixtures/config/success-url/git/common-config/playbooks/docs-draft-test.yaml +++ /dev/null @@ -1,2 +0,0 @@ -- hosts: all - tasks: [] diff --git a/tests/fixtures/config/success-url/git/common-config/playbooks/docs-draft-test2.yaml b/tests/fixtures/config/success-url/git/common-config/playbooks/docs-draft-test2.yaml deleted file mode 100644 index f679dceaef..0000000000 --- a/tests/fixtures/config/success-url/git/common-config/playbooks/docs-draft-test2.yaml +++ /dev/null @@ -1,2 +0,0 @@ -- hosts: all - tasks: [] diff --git a/tests/fixtures/config/success-url/git/common-config/zuul.yaml b/tests/fixtures/config/success-url/git/common-config/zuul.yaml deleted file mode 100644 index b9f4bff99c..0000000000 --- a/tests/fixtures/config/success-url/git/common-config/zuul.yaml +++ /dev/null @@ -1,38 +0,0 @@ -- pipeline: - name: check - manager: independent - trigger: - gerrit: - - event: patchset-created - start: - smtp: - to: alternative_me@example.com - success: - gerrit: - Verified: 1 - smtp: - to: alternative_me@example.com - failure: - gerrit: - Verified: -1 - -- job: - name: base - parent: null - -- job: - name: docs-draft-test - success-url: http://docs-draft.example.org/{change.number:.2}/{change.number}/{change.patchset}/{pipeline.name}/{job.name}/{build.uuid:.7}/publish-docs/ - run: playbooks/docs-draft-test.yaml - -- job: - name: docs-draft-test2 - success-url: http://docs-draft.example.org/{NOPE}/{build.parameters[BAD]}/publish-docs/ - run: playbooks/docs-draft-test2.yaml - -- project: - name: org/docs - check: - jobs: - - docs-draft-test - - docs-draft-test2 diff --git a/tests/fixtures/config/success-url/git/org_docs/README b/tests/fixtures/config/success-url/git/org_docs/README deleted file mode 100644 index 9daeafb986..0000000000 --- a/tests/fixtures/config/success-url/git/org_docs/README +++ /dev/null @@ -1 +0,0 @@ -test diff --git a/tests/fixtures/config/success-url/main.yaml b/tests/fixtures/config/success-url/main.yaml deleted file mode 100644 index 0027ae1484..0000000000 --- a/tests/fixtures/config/success-url/main.yaml +++ /dev/null @@ -1,8 +0,0 @@ -- tenant: - name: tenant-one - source: - gerrit: - config-projects: - - common-config - untrusted-projects: - - org/docs diff --git a/tests/fixtures/layouts/timer-smtp.yaml b/tests/fixtures/layouts/timer-smtp.yaml index d9e4282816..ff69e133ad 100644 --- a/tests/fixtures/layouts/timer-smtp.yaml +++ b/tests/fixtures/layouts/timer-smtp.yaml @@ -17,12 +17,10 @@ - job: name: project-bitrot-stable-old - success-url: http://logs.example.com/{job.name}/{build.number} run: playbooks/project-bitrot-stable-old.yaml - job: name: project-bitrot-stable-older - success-url: http://logs.example.com/{job.name}/{build.number} run: playbooks/project-bitrot-stable-older.yaml - project: diff --git a/tests/unit/test_connection.py b/tests/unit/test_connection.py index cb1f7b6bae..c3a8bbf049 100644 --- a/tests/unit/test_connection.py +++ b/tests/unit/test_connection.py @@ -633,10 +633,14 @@ class TestMQTTConnection(ZuulTestCase): self.assertEquals(test_job['result'], 'SUCCESS') self.assertEquals(test_job['dependencies'], []) self.assertEquals(test_job['artifacts'], [artifact]) - # 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') + build_id = test_job["uuid"] + self.assertEquals( + test_job["web_url"], + "https://tenant.example.com/t/tenant-one/build/{}".format( + build_id + ), + ) self.assertIn('execute_time', test_job) self.assertIn('timestamp', mqtt_payload) self.assertIn('enqueue_time', mqtt_payload) @@ -667,51 +671,6 @@ class TestMQTTConnection(ZuulTestCase): "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 - ), - ) - - class TestElasticsearchConnection(AnsibleZuulTestCase): config_file = 'zuul-elastic-driver.conf' tenant_config_file = 'config/elasticsearch-driver/main.yaml' diff --git a/tests/unit/test_scheduler.py b/tests/unit/test_scheduler.py index ef2ab0f2d9..cfb1629aba 100644 --- a/tests/unit/test_scheduler.py +++ b/tests/unit/test_scheduler.py @@ -5805,15 +5805,15 @@ For CI problems and help debugging, contact ci@example.org""" self.assertEqual(0, len(G.messages)) self.assertIn('Build failed.', self.smtp_messages[0]['body']) self.assertIn( - 'project-test1 finger://', self.smtp_messages[0]['body']) + 'project-test1 https://', self.smtp_messages[0]['body']) self.assertEqual(0, len(H.messages)) self.assertIn('Build failed.', self.smtp_messages[1]['body']) self.assertIn( - 'project-test1 finger://', self.smtp_messages[1]['body']) + 'project-test1 https://', self.smtp_messages[1]['body']) self.assertEqual(0, len(I.messages)) self.assertIn('Build succeeded.', self.smtp_messages[2]['body']) self.assertIn( - 'project-test1 finger://', self.smtp_messages[2]['body']) + 'project-test1 https://', self.smtp_messages[2]['body']) # Now reload the configuration (simulate a HUP) to check the pipeline # comes out of disabled @@ -6098,9 +6098,10 @@ For CI problems and help debugging, contact ci@example.org""" self.assertEqual(A.data['status'], 'NEW') self.assertEqual(A.reported, 2) - self.assertIn('project-merge : NODE_FAILURE', A.messages[1]) - self.assertIn('project-test1 : SKIPPED', A.messages[1]) - self.assertIn('project-test2 : SKIPPED', A.messages[1]) + self.assertTrue(re.search('project-merge .* NODE_FAILURE', + A.messages[1])) + self.assertTrue(re.search('project-test1 .* SKIPPED', A.messages[1])) + self.assertTrue(re.search('project-test2 .* SKIPPED', A.messages[1])) def test_nodepool_resources(self): "Test that resources are reported" @@ -7317,46 +7318,6 @@ class TestSchedulerTemplatedProject(ZuulTestCase): self.assertEqual(len(stable_items), 0) -class TestSchedulerSuccessURL(ZuulTestCase): - tenant_config_file = 'config/success-url/main.yaml' - - def test_success_url(self): - "Ensure bad build params are ignored" - self.scheds.execute(lambda app: app.sched.reconfigure(app.config)) - self.init_repo('org/docs') - - A = self.fake_gerrit.addFakeChange('org/docs', 'master', 'A') - self.fake_gerrit.addEvent(A.getPatchsetCreatedEvent(1)) - self.waitUntilSettled() - - # Both builds ran: docs-draft-test + docs-draft-test2 - self.assertEqual(len(self.history), 2) - - # Grab build id - for build in self.history: - if build.name == 'docs-draft-test': - uuid = build.uuid[:7] - elif build.name == 'docs-draft-test2': - uuid_test2 = build.uuid - - # Two msgs: 'Starting...' + results - self.assertEqual(len(self.smtp_messages), 2) - body = self.smtp_messages[1]['body'].splitlines() - self.assertEqual('Build succeeded.', body[0]) - - self.assertIn( - '- docs-draft-test http://docs-draft.example.org/1/1/1/check/' - 'docs-draft-test/%s/publish-docs/' % uuid, - body[2]) - - # NOTE: This default URL is currently hard-coded in executor/server.py - self.assertIn( - '- docs-draft-test2 finger://{hostname}/{uuid}'.format( - hostname=self.executor_server.hostname, - uuid=uuid_test2), - body[3]) - - class TestSchedulerMerges(ZuulTestCase): tenant_config_file = 'config/merges/main.yaml' @@ -7763,8 +7724,8 @@ class TestSemaphore(ZuulTestCase): 0) self.assertEquals(1, A.reported) - self.assertIn('semaphore-one-test3 semaphore-one-test3 : NODE_FAILURE', - A.messages[0]) + self.assertTrue(re.search('semaphore-one-test3 .* NODE_FAILURE', + A.messages[0])) def test_semaphore_resources_first(self): "Test semaphores with max=1 (mutex) and get resources first" @@ -7845,8 +7806,9 @@ class TestSemaphore(ZuulTestCase): self.assertEqual(len(tenant.semaphore_handler.semaphoreHolders( "test-semaphore")), 0) self.assertEquals(1, A.reported) - self.assertIn('semaphore-one-test1-resources-first : NODE_FAILURE', - A.messages[0]) + self.assertTrue( + re.search('semaphore-one-test1-resources-first .* NODE_FAILURE', + A.messages[0])) def test_semaphore_zk_error(self): "Test semaphore release with zk error" @@ -8768,7 +8730,7 @@ class TestReportBuildPage(ZuulTestCase): self.assertHistory([ dict(name='python27', result='SUCCESS', changes='1,1'), ]) - self.assertIn('python27 finger://', A.messages[0]) + self.assertIn('python27 https://', A.messages[0]) class TestSchedulerSmartReconfiguration(ZuulTestCase): diff --git a/tests/unit/test_v3.py b/tests/unit/test_v3.py index 652a793d9c..7de60dae75 100644 --- a/tests/unit/test_v3.py +++ b/tests/unit/test_v3.py @@ -20,6 +20,7 @@ import os import sys import textwrap import gc +import re from time import sleep from unittest import skip, skipIf from zuul.lib import yamlutil @@ -3231,17 +3232,6 @@ class FunctionalAnsibleMixIn(object): with open(secrets_path) as f: self.assertEqual(f.read(), "test-username test-password") - msg = A.messages[0] - success = "{} https://success.example.com/zuul-logs/{}" - fail = "{} https://failure.example.com/zuul-logs/{}" - self.assertIn(success.format("python27", build_python27.uuid), msg) - self.assertIn(fail.format("faillocal", build_faillocal.uuid), msg) - self.assertIn(success.format("check-vars", - build_check_vars.uuid), msg) - self.assertIn(success.format("hello-world", build_hello.uuid), msg) - self.assertIn(fail.format("timeout", build_timeout.uuid), msg) - self.assertIn(fail.format("failpost", build_failpost.uuid), msg) - def test_repo_ansible(self): A = self.fake_gerrit.addFakeChange('org/ansible', 'master', 'A') self.fake_gerrit.addEvent(A.getPatchsetCreatedEvent(1)) @@ -4238,9 +4228,9 @@ class TestRoles(RoleTestCase): files=file_dict) self.fake_gerrit.addEvent(A.getPatchsetCreatedEvent(1)) self.waitUntilSettled() - self.assertIn( - '- project-test project-test : ERROR Unable to find role', - A.messages[-1]) + self.assertTrue(re.search( + '- project-test .* ERROR Unable to find role', + A.messages[-1])) class TestImplicitRoles(RoleTestCase): @@ -4320,10 +4310,9 @@ class TestDataReturn(AnsibleZuulTestCase): dict(name='data-return-relative', result='SUCCESS', changes='1,1'), dict(name='child', result='SUCCESS', changes='1,1'), ], ordered=False) - self.assertIn('- data-return http://example.com/test/log/url/', + self.assertIn('- data-return https://zuul.example.com/', A.messages[-1]) - self.assertIn('- data-return-relative ' - 'http://example.com/test/log/url/docs/index.html', + self.assertIn('- data-return-relative https://zuul.example.com', A.messages[-1]) def test_data_return_child_jobs(self): @@ -4356,12 +4345,12 @@ class TestDataReturn(AnsibleZuulTestCase): dict(name='data-return', result='SUCCESS', changes='1,1'), ]) self.assertIn( - '- data-return-child-jobs http://example.com/test/log/url/', + '- data-return-child-jobs https://zuul.example.com/', A.messages[-1]) self.assertIn( - '- data-return http://example.com/test/log/url/', + '- data-return https://zuul.example.com/', A.messages[-1]) - self.assertIn('child : SKIPPED', A.messages[-1]) + self.assertTrue(re.search('child .* SKIPPED', A.messages[-1])) self.assertIn('Build succeeded', A.messages[-1]) def test_data_return_invalid_child_job(self): @@ -4372,9 +4361,9 @@ class TestDataReturn(AnsibleZuulTestCase): dict(name='data-return-invalid-child-job', result='SUCCESS', changes='1,1')]) self.assertIn( - '- data-return-invalid-child-job http://example.com/test/log/url/', + '- data-return-invalid-child-job https://zuul.example.com', A.messages[-1]) - self.assertIn('data-return : SKIPPED', A.messages[-1]) + self.assertTrue(re.search('data-return .* SKIPPED', A.messages[-1])) self.assertIn('Build succeeded', A.messages[-1]) def test_data_return_skip_all_child_jobs(self): @@ -4386,10 +4375,10 @@ class TestDataReturn(AnsibleZuulTestCase): changes='1,1'), ]) self.assertIn( - '- data-return-skip-all http://example.com/test/log/url/', + '- data-return-skip-all https://zuul.example.com/', A.messages[-1]) - self.assertIn('child : SKIPPED', A.messages[-1]) - self.assertIn('data-return : SKIPPED', A.messages[-1]) + self.assertTrue(re.search('child .* SKIPPED', A.messages[-1])) + self.assertTrue(re.search('data-return .* SKIPPED', A.messages[-1])) self.assertIn('Build succeeded', A.messages[-1]) def test_data_return_skip_all_child_jobs_with_soft_dependencies(self): @@ -4401,10 +4390,10 @@ class TestDataReturn(AnsibleZuulTestCase): dict(name='data-return-c', result='SUCCESS', changes='1,1'), dict(name='data-return-d', result='SUCCESS', changes='1,1'), ]) - self.assertIn('- data-return-cd http://example.com/test/log/url/', + self.assertIn('- data-return-cd https://zuul.example.com/', A.messages[-1]) - self.assertIn('data-return-a : SKIPPED', A.messages[-1]) - self.assertIn('data-return-b : SKIPPED', A.messages[-1]) + self.assertTrue(re.search('data-return-a .* SKIPPED', A.messages[-1])) + self.assertTrue(re.search('data-return-b .* SKIPPED', A.messages[-1])) self.assertIn('Build succeeded', A.messages[-1]) def test_several_zuul_return(self): @@ -4416,9 +4405,9 @@ class TestDataReturn(AnsibleZuulTestCase): changes='1,1'), ]) self.assertIn( - '- several-zuul-return-child http://example.com/test/log/url/', + '- several-zuul-return-child https://zuul.example.com/', A.messages[-1]) - self.assertIn('data-return : SKIPPED', A.messages[-1]) + self.assertTrue(re.search('data-return .* SKIPPED', A.messages[-1])) self.assertIn('Build succeeded', A.messages[-1]) def test_data_return_child_jobs_failure(self): @@ -6286,7 +6275,7 @@ class TestJobPause(AnsibleZuulTestCase): dict(name='compile', result='SUCCESS', changes='1,1'), ]) - self.assertIn('test : SKIPPED', A.messages[0]) + self.assertTrue(re.search('test .* SKIPPED', A.messages[0])) def test_job_pause_pre_skipped_child(self): """ @@ -6334,7 +6323,7 @@ class TestJobPause(AnsibleZuulTestCase): dict(name='compile', result='SUCCESS', changes='1,1'), ]) - self.assertIn('test : SKIPPED', A.messages[0]) + self.assertTrue(re.search('test .* SKIPPED', A.messages[0])) def test_job_pause_skipped_child_retry(self): """ @@ -7108,7 +7097,7 @@ class TestProvidesRequiresMysql(ZuulTestCase): dict(name='hold', result='SUCCESS', changes='1,1'), dict(name='hold', result='SUCCESS', changes='1,1 2,1'), ], ordered=False) - self.assertIn('image-user : FAILURE', B.messages[0]) + self.assertTrue(re.search('image-user .* FAILURE', B.messages[0])) self.assertEqual( B.messages[0].count( 'Job image-user requires artifact(s) images'), @@ -7134,7 +7123,7 @@ class TestProvidesRequiresMysql(ZuulTestCase): dict(name='image-builder', result='FAILURE', changes='1,1'), dict(name='hold', result='SUCCESS', changes='1,1'), ], ordered=False) - self.assertIn('image-user : SKIPPED', A.messages[0]) + self.assertTrue(re.search('image-user .* SKIPPED', A.messages[0])) B = self.fake_gerrit.addFakeChange('org/project1', 'master', 'B') B.data['commitMessage'] = '%s\n\nDepends-On: %s\n' % ( @@ -7148,7 +7137,7 @@ class TestProvidesRequiresMysql(ZuulTestCase): dict(name='image-builder', result='FAILURE', changes='1,1 2,1'), dict(name='hold', result='SUCCESS', changes='1,1 2,1'), ], ordered=False) - self.assertIn('image-user : FAILURE', B.messages[0]) + self.assertTrue(re.search('image-user .* FAILURE', B.messages[0])) self.assertEqual( B.messages[0].count( 'Job image-user requires artifact(s) images'), diff --git a/tests/unit/test_web.py b/tests/unit/test_web.py index 3c7cf9527a..2cc2547a59 100644 --- a/tests/unit/test_web.py +++ b/tests/unit/test_web.py @@ -180,10 +180,8 @@ class TestWeb(BaseTestWeb): hostname=self.executor_server.hostname, uuid=status_jobs[0]['uuid']), status_jobs[0]['finger_url']) - # TOOD(mordred) configure a success-url on the base job self.assertEqual( - 'finger://{hostname}/{uuid}'.format( - hostname=self.executor_server.hostname, + 'https://zuul.example.com/t/tenant-one/build/{uuid}'.format( uuid=status_jobs[0]['uuid']), status_jobs[0]['report_url']) self.assertEqual('project-test1', status_jobs[1]['name']) @@ -197,8 +195,7 @@ class TestWeb(BaseTestWeb): uuid=status_jobs[1]['uuid']), status_jobs[1]['finger_url']) self.assertEqual( - 'finger://{hostname}/{uuid}'.format( - hostname=self.executor_server.hostname, + 'https://zuul.example.com/t/tenant-one/build/{uuid}'.format( uuid=status_jobs[1]['uuid']), status_jobs[1]['report_url']) @@ -213,8 +210,7 @@ class TestWeb(BaseTestWeb): uuid=status_jobs[2]['uuid']), status_jobs[2]['finger_url']) self.assertEqual( - 'finger://{hostname}/{uuid}'.format( - hostname=self.executor_server.hostname, + 'https://zuul.example.com/t/tenant-one/build/{uuid}'.format( uuid=status_jobs[2]['uuid']), status_jobs[2]['report_url']) diff --git a/zuul/configloader.py b/zuul/configloader.py index 79677d8045..b56d3ca701 100644 --- a/zuul/configloader.py +++ b/zuul/configloader.py @@ -583,7 +583,9 @@ class JobParser(object): 'provides': to_list(str), 'failure-message': str, 'success-message': str, + # TODO: ignored, remove for v5 'failure-url': str, + # TODO: ignored, remove for v5 'success-url': str, 'hold-following-changes': bool, 'voting': bool, @@ -642,8 +644,6 @@ class JobParser(object): 'attempts', 'failure-message', 'success-message', - 'failure-url', - 'success-url', 'override-branch', 'override-checkout', 'match-on-config-updates', @@ -1533,6 +1533,7 @@ class TenantParser(object): 'default-parent': str, 'default-ansible-version': vs.Any(str, float), 'admin-rules': to_list(str), + # TODO: Ignored, allowed for backwards compat, remove for v5. 'report-build-page': bool, 'web-root': str, } @@ -1552,8 +1553,6 @@ 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) if tenant.web_root and not tenant.web_root.endswith('/'): tenant.web_root += '/' diff --git a/zuul/model.py b/zuul/model.py index 1eef6f6967..5d071cde2a 100644 --- a/zuul/model.py +++ b/zuul/model.py @@ -1279,8 +1279,6 @@ class Job(ConfigObject): hold_following_changes=False, failure_message=None, success_message=None, - failure_url=None, - success_url=None, branch_matcher=None, _branches=(), file_matcher=None, @@ -3150,16 +3148,12 @@ class QueueItem(object): return url def formatJobResult(self, job, build=None): - if (self.pipeline.tenant.report_build_page and - self.pipeline.tenant.web_root): - if build is None: - 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, build) + if build is None: + 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) def formatStatusUrl(self): # If we don't have a web root set, we can't format any url @@ -3185,46 +3179,6 @@ class QueueItem(object): ) return self.formatUrlPattern(pattern) - def formatProvisionalJobResult(self, job, build=None): - if build is None: - build = self.current_build_set.getBuild(job.name) - result = build.result - pattern = None - if result == 'SUCCESS': - if job.success_message: - result = job.success_message - if job.success_url: - pattern = job.success_url - else: - if job.failure_message: - result = job.failure_message - if job.failure_url: - pattern = job.failure_url - url = None # The final URL - default_url = build.result_data.get('zuul', {}).get('log_url') - if pattern: - job_url = self.formatUrlPattern(pattern, job, build) - else: - job_url = None - try: - if job_url: - u = urllib.parse.urlparse(job_url) - if u.scheme: - # The job success or failure url is absolute, so it's - # our final url. - url = job_url - else: - # We have a relative job url. Combine it with our - # default url. - if default_url: - url = urllib.parse.urljoin(default_url, job_url) - except Exception: - self.log.exception("Error while parsing url for job %s:" - % (job,)) - if not url: - url = default_url or build.url or None - return (result, url) - def formatJSON(self, websocket_url=None): ret = {} ret['active'] = self.active @@ -5247,7 +5201,6 @@ 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.