From 9879cab0d112ff19be3146f5aba1538eb276555d Mon Sep 17 00:00:00 2001 From: Tobias Henkel Date: Wed, 10 Apr 2019 14:49:38 +0200 Subject: [PATCH] Support emitting warnings via zuul_return We already have the infrastructure in place for adding warnings to the reporting. Plumb that through to zuul_return so jobs can do that on purpose as well. An example could be a post playbook that analyzes performance statistics and emits a warning about inefficient usage of the build node resources. Change-Id: I4c3b85dc8f4c69c55cbc6168b8a66afce8b50a97 --- doc/source/reference/jobs.rst | 19 ++++++++++++ .../notes/job-warnings-b72a9e80574b0969.yaml | 5 +++ .../git/common-config/zuul.yaml | 17 ++++++++++ .../git/org_project/playbooks/warnings.yaml | 14 +++++++++ .../return-warnings/git/org_project/zuul.yaml | 8 +++++ .../fixtures/config/return-warnings/main.yaml | 8 +++++ tests/unit/test_v3.py | 21 +++++++++++++ .../ansible/base/actiongeneral/zuul_return.py | 31 +++++++++---------- zuul/driver/sql/sqlreporter.py | 2 +- zuul/executor/server.py | 2 ++ zuul/lib/{artifacts.py => result_data.py} | 29 ++++++++++++++--- zuul/model.py | 2 +- 12 files changed, 134 insertions(+), 24 deletions(-) create mode 100644 releasenotes/notes/job-warnings-b72a9e80574b0969.yaml create mode 100644 tests/fixtures/config/return-warnings/git/common-config/zuul.yaml create mode 100644 tests/fixtures/config/return-warnings/git/org_project/playbooks/warnings.yaml create mode 100644 tests/fixtures/config/return-warnings/git/org_project/zuul.yaml create mode 100644 tests/fixtures/config/return-warnings/main.yaml rename zuul/lib/{artifacts.py => result_data.py} (74%) diff --git a/doc/source/reference/jobs.rst b/doc/source/reference/jobs.rst index 8c58a63c71..ad0c98d43d 100644 --- a/doc/source/reference/jobs.rst +++ b/doc/source/reference/jobs.rst @@ -865,6 +865,25 @@ zuul.child_jobs is empty, all jobs will be marked as SKIPPED. Invalid child jobs are stripped and ignored, if only invalid jobs are listed it is the same as providing an empty list to zuul.child_jobs. +Leaving warnings +~~~~~~~~~~~~~~~~ + +A job can leave warnings that will be appended to the comment zuul leaves on +the change. Use *zuul_return* to add a list of warnings. For example: + +.. code-block:: yaml + + tasks: + - zuul_return: + data: + zuul: + warnings: + - This warning will be posted on the change. + +If *zuul_return* is invoked multiple times (e.g., via multiple +playbooks), then the elements of **zuul.warnings** from each +invocation will be appended. + Leaving file comments ~~~~~~~~~~~~~~~~~~~~~ diff --git a/releasenotes/notes/job-warnings-b72a9e80574b0969.yaml b/releasenotes/notes/job-warnings-b72a9e80574b0969.yaml new file mode 100644 index 0000000000..df92815bc0 --- /dev/null +++ b/releasenotes/notes/job-warnings-b72a9e80574b0969.yaml @@ -0,0 +1,5 @@ +--- +features: + - | + A job now can leave warning messages on the change using zuul_return. + See :ref:`return_values` for examples. diff --git a/tests/fixtures/config/return-warnings/git/common-config/zuul.yaml b/tests/fixtures/config/return-warnings/git/common-config/zuul.yaml new file mode 100644 index 0000000000..a07342e2ec --- /dev/null +++ b/tests/fixtures/config/return-warnings/git/common-config/zuul.yaml @@ -0,0 +1,17 @@ +- pipeline: + name: check + manager: independent + post-review: true + trigger: + gerrit: + - event: patchset-created + success: + gerrit: + Verified: 1 + failure: + gerrit: + Verified: -1 + +- job: + name: base + parent: null diff --git a/tests/fixtures/config/return-warnings/git/org_project/playbooks/warnings.yaml b/tests/fixtures/config/return-warnings/git/org_project/playbooks/warnings.yaml new file mode 100644 index 0000000000..80b769c77c --- /dev/null +++ b/tests/fixtures/config/return-warnings/git/org_project/playbooks/warnings.yaml @@ -0,0 +1,14 @@ +- hosts: localhost + tasks: + - name: Emit first warning + zuul_return: + data: + zuul: + warnings: + - This is the first warning + - name: Emit second warning + zuul_return: + data: + zuul: + warnings: + - This is the second warning diff --git a/tests/fixtures/config/return-warnings/git/org_project/zuul.yaml b/tests/fixtures/config/return-warnings/git/org_project/zuul.yaml new file mode 100644 index 0000000000..7ebee823ba --- /dev/null +++ b/tests/fixtures/config/return-warnings/git/org_project/zuul.yaml @@ -0,0 +1,8 @@ +- job: + name: emit-warnings + run: playbooks/warnings.yaml + +- project: + check: + jobs: + - emit-warnings diff --git a/tests/fixtures/config/return-warnings/main.yaml b/tests/fixtures/config/return-warnings/main.yaml new file mode 100644 index 0000000000..208e274b13 --- /dev/null +++ b/tests/fixtures/config/return-warnings/main.yaml @@ -0,0 +1,8 @@ +- tenant: + name: tenant-one + source: + gerrit: + config-projects: + - common-config + untrusted-projects: + - org/project diff --git a/tests/unit/test_v3.py b/tests/unit/test_v3.py index dc8e8f6a14..8742ceecf4 100644 --- a/tests/unit/test_v3.py +++ b/tests/unit/test_v3.py @@ -6622,3 +6622,24 @@ class TestDefaultAnsibleVersion(AnsibleZuulTestCase): dict(name='ansible-28', result='SUCCESS', changes='1,1'), dict(name='ansible-29', result='SUCCESS', changes='1,1'), ], ordered=False) + + +class TestReturnWarnings(AnsibleZuulTestCase): + tenant_config_file = 'config/return-warnings/main.yaml' + + def test_return_warnings(self): + """ + Tests that jobs can emit custom warnings that get reported. + """ + + A = self.fake_gerrit.addFakeChange('org/project', 'master', 'A') + self.fake_gerrit.addEvent(A.getPatchsetCreatedEvent(1)) + self.waitUntilSettled() + + self.assertHistory([ + dict(name='emit-warnings', result='SUCCESS', changes='1,1'), + ]) + + self.assertTrue(A.reported) + self.assertIn('This is the first warning', A.messages[0]) + self.assertIn('This is the second warning', A.messages[0]) diff --git a/zuul/ansible/base/actiongeneral/zuul_return.py b/zuul/ansible/base/actiongeneral/zuul_return.py index 540b9843bf..91a8e8c0d1 100644 --- a/zuul/ansible/base/actiongeneral/zuul_return.py +++ b/zuul/ansible/base/actiongeneral/zuul_return.py @@ -39,36 +39,33 @@ def merge_dict(dict_a, dict_b): return dict_b +def merge_zuul_list(dict_a, dict_b, key): + value_a = dict_a.get('zuul', {}).get(key, []) + value_b = dict_b.get('zuul', {}).get(key, []) + if not isinstance(value_a, list): + value_a = [] + if not isinstance(value_b, list): + value_b = [] + return value_a + value_b + + def merge_data(dict_a, dict_b): """ Merge dict_a into dict_b, handling any special cases for zuul variables """ - artifacts = merge_artifacts(dict_a, dict_b) + artifacts = merge_zuul_list(dict_a, dict_b, 'artifacts') file_comments = merge_file_comments(dict_a, dict_b) + warnings = merge_zuul_list(dict_a, dict_b, 'warnings') merge_dict(dict_a, dict_b) if artifacts: dict_b.setdefault('zuul', {})['artifacts'] = artifacts if file_comments: dict_b.setdefault("zuul", {})["file_comments"] = file_comments + if warnings: + dict_b.setdefault('zuul', {})['warnings'] = warnings return dict_b -def merge_artifacts(dict_a, dict_b): - """Merge artifacts from both dictionary - - The artifacts from both dictionaries will be merged (additive) into a new - list. - """ - artifacts_a = dict_a.get('zuul', {}).get("artifacts", []) - if not isinstance(artifacts_a, list): - artifacts_a = [] - artifacts_b = dict_b.get('zuul', {}).get("artifacts", []) - if not isinstance(artifacts_b, list): - artifacts_b = [] - artifacts = artifacts_a + artifacts_b - return artifacts - - def merge_file_comments(dict_a, dict_b): """Merge file_comments from both dictionary. diff --git a/zuul/driver/sql/sqlreporter.py b/zuul/driver/sql/sqlreporter.py index b77f9e6cf2..68e94ce553 100644 --- a/zuul/driver/sql/sqlreporter.py +++ b/zuul/driver/sql/sqlreporter.py @@ -19,8 +19,8 @@ import time import voluptuous as v from zuul.lib.logutil import get_annotated_logger +from zuul.lib.result_data import get_artifacts_from_result_data from zuul.reporter import BaseReporter -from zuul.lib.artifacts import get_artifacts_from_result_data class SQLReporter(BaseReporter): diff --git a/zuul/executor/server.py b/zuul/executor/server.py index 337660a724..aade3329f9 100644 --- a/zuul/executor/server.py +++ b/zuul/executor/server.py @@ -37,6 +37,7 @@ from urllib.parse import urlsplit from zuul.lib.ansible import AnsibleManager from zuul.lib.gearworker import ZuulGearWorker +from zuul.lib.result_data import get_warnings_from_result_data from zuul.lib.yamlutil import yaml from zuul.lib.config import get_default from zuul.lib.logutil import get_annotated_logger @@ -1175,6 +1176,7 @@ class AnsibleJob(object): data = self.getResultData() warnings = [] self.mapLines(merger, args, data, item_commit, warnings) + warnings.extend(get_warnings_from_result_data(data, logger=self.log)) result_data = json.dumps(dict(result=result, warnings=warnings, data=data)) diff --git a/zuul/lib/artifacts.py b/zuul/lib/result_data.py similarity index 74% rename from zuul/lib/artifacts.py rename to zuul/lib/result_data.py index b6da6e28d6..f8be8acc30 100644 --- a/zuul/lib/artifacts.py +++ b/zuul/lib/result_data.py @@ -21,7 +21,7 @@ artifact = { 'metadata': dict, } -zuul_data = { +artifact_data = { 'zuul': { 'log_url': str, 'artifacts': [artifact], @@ -30,12 +30,22 @@ zuul_data = { v.Extra: object, } -artifact_schema = v.Schema(zuul_data) +warning_data = { + 'zuul': { + 'log_url': str, + 'warnings': [str], + v.Extra: object, + }, + v.Extra: object, +} + +artifact_schema = v.Schema(artifact_data) +warning_schema = v.Schema(warning_data) -def validate_artifact_schema(data): +def validate_schema(data, schema): try: - artifact_schema(data) + schema(data) except Exception: return False return True @@ -43,7 +53,7 @@ def validate_artifact_schema(data): def get_artifacts_from_result_data(result_data, logger=None): ret = [] - if validate_artifact_schema(result_data): + if validate_schema(result_data, artifact_schema): artifacts = result_data.get('zuul', {}).get( 'artifacts', []) default_url = result_data.get('zuul', {}).get( @@ -71,3 +81,12 @@ def get_artifacts_from_result_data(result_data, logger=None): logger.debug("Result data did not pass artifact schema " "validation: %s", result_data) return ret + + +def get_warnings_from_result_data(result_data, logger=None): + if validate_schema(result_data, warning_schema): + return result_data.get('zuul', {}).get('warnings', []) + else: + if logger: + logger.debug("Result data did not pass warnings schema " + "validation: %s", result_data) diff --git a/zuul/model.py b/zuul/model.py index cf0191730c..14b72aebaa 100644 --- a/zuul/model.py +++ b/zuul/model.py @@ -31,7 +31,7 @@ import jsonpath_rw from zuul import change_matcher from zuul.lib.config import get_default -from zuul.lib.artifacts import get_artifacts_from_result_data +from zuul.lib.result_data import get_artifacts_from_result_data from zuul.lib.logutil import get_annotated_logger from zuul.lib.capabilities import capabilities_registry