diff --git a/doc/source/reference/jobs.rst b/doc/source/reference/jobs.rst index 28d2bd2e7d..e9c8a9e5c1 100644 --- a/doc/source/reference/jobs.rst +++ b/doc/source/reference/jobs.rst @@ -884,6 +884,25 @@ zuul.child_jobs is empty, all jobs will be marked as SKIPPED. Invalid dependent 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 45faa35770..5f0275da3a 100644 --- a/tests/unit/test_v3.py +++ b/tests/unit/test_v3.py @@ -6818,3 +6818,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 9b81b51f38..f2d98aa699 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 766a6e04f0..93757789a7 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 @@ -1178,6 +1179,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 759e92a148..a6864d0321 100644 --- a/zuul/model.py +++ b/zuul/model.py @@ -33,7 +33,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