From 016601cc21469a272315d694a9add19e0b68328c Mon Sep 17 00:00:00 2001 From: erikchen Date: Fri, 30 Nov 2018 15:03:19 +0000 Subject: [PATCH] Add mechanism for telling CQ to avoid retrying builds. The function can be called by chromium_test to set a property 'do_not_retry' which will be propagated into buildbucket output. Change-Id: I32d8ea925b7cb98d9b25d24226686e116c17801c Bug: 910193 Reviewed-on: https://chromium-review.googlesource.com/c/1351542 Commit-Queue: Erik Chen Reviewed-by: Andrii Shyshkalov Reviewed-by: Ben Pastene --- recipes/README.recipes.md | 25 ++++++++++++------- recipes/recipe_modules/tryserver/api.py | 9 +++++++ .../full.expected/with_gerrit_patch.json | 1 + .../with_gerrit_patch_and_target_ref.json | 1 + .../full.expected/with_git_patch.json | 1 + .../full.expected/with_git_patch_luci.json | 1 + .../full.expected/with_wrong_patch.json | 3 ++- .../full.expected/with_wrong_patch_new.json | 3 ++- .../recipe_modules/tryserver/examples/full.py | 1 + 9 files changed, 34 insertions(+), 11 deletions(-) diff --git a/recipes/README.recipes.md b/recipes/README.recipes.md index 42e0e9c0f..965ca44d4 100644 --- a/recipes/README.recipes.md +++ b/recipes/README.recipes.md @@ -751,7 +751,7 @@ Return a presubmit step. #### **class [TryserverApi](/recipes/recipe_modules/tryserver/api.py#11)([RecipeApi][recipe_engine/wkt/RecipeApi]):** -— **def [add\_failure\_reason](/recipes/recipe_modules/tryserver/api.py#206)(self, reason):** +— **def [add\_failure\_reason](/recipes/recipe_modules/tryserver/api.py#215)(self, reason):** Records a more detailed reason why build is failing. @@ -791,11 +791,11 @@ Argument: Returned paths will be relative to to patch_root. -— **def [get\_footer](/recipes/recipe_modules/tryserver/api.py#262)(self, tag, patch_text=None):** +— **def [get\_footer](/recipes/recipe_modules/tryserver/api.py#271)(self, tag, patch_text=None):** Gets a specific tag from a CL description -— **def [get\_footers](/recipes/recipe_modules/tryserver/api.py#242)(self, patch_text=None):** +— **def [get\_footers](/recipes/recipe_modules/tryserver/api.py#251)(self, patch_text=None):** Retrieves footers from the patch description. @@ -814,13 +814,20 @@ Returns true iff the properties exist to match a Gerrit issue. Returns true iff we have a change to check out. -— **def [normalize\_footer\_name](/recipes/recipe_modules/tryserver/api.py#266)(self, footer):** +— **def [normalize\_footer\_name](/recipes/recipe_modules/tryserver/api.py#275)(self, footer):** -— **def [set\_compile\_failure\_tryjob\_result](/recipes/recipe_modules/tryserver/api.py#185)(self):** +— **def [set\_compile\_failure\_tryjob\_result](/recipes/recipe_modules/tryserver/api.py#194)(self):** Mark the tryjob result as a compile failure. -  **@contextlib.contextmanager**
— **def [set\_failure\_hash](/recipes/recipe_modules/tryserver/api.py#215)(self):** +— **def [set\_do\_not\_retry\_build](/recipes/recipe_modules/tryserver/api.py#181)(self):** + +A flag to indicate the build should not be retried by the CQ. + +This mechanism is used to reduce CQ duration when retrying will likely +return an identical result. + +  **@contextlib.contextmanager**
— **def [set\_failure\_hash](/recipes/recipe_modules/tryserver/api.py#224)(self):** Context manager that sets a failure_hash build property on StepFailure. @@ -829,7 +836,7 @@ for the same reason. For example, if a patch is bad (breaks something), we'd expect it to always break in the same way. Different failures for the same patch are usually a sign of flakiness. -— **def [set\_invalid\_test\_results\_tryjob\_result](/recipes/recipe_modules/tryserver/api.py#197)(self):** +— **def [set\_invalid\_test\_results\_tryjob\_result](/recipes/recipe_modules/tryserver/api.py#206)(self):** Mark the tryjob result as having invalid test results. @@ -837,7 +844,7 @@ This means we run some tests, but the results were not valid (e.g. no list of specific test cases that failed, or too many tests failing, etc). -— **def [set\_patch\_failure\_tryjob\_result](/recipes/recipe_modules/tryserver/api.py#181)(self):** +— **def [set\_patch\_failure\_tryjob\_result](/recipes/recipe_modules/tryserver/api.py#190)(self):** Mark the tryjob result as failure to apply the patch. @@ -848,7 +855,7 @@ Adds a subproject tag to the build. This can be used to distinguish between builds that execute different steps depending on what was patched, e.g. blink vs. pure chromium patches. -— **def [set\_test\_failure\_tryjob\_result](/recipes/recipe_modules/tryserver/api.py#189)(self):** +— **def [set\_test\_failure\_tryjob\_result](/recipes/recipe_modules/tryserver/api.py#198)(self):** Mark the tryjob result as a test failure. diff --git a/recipes/recipe_modules/tryserver/api.py b/recipes/recipe_modules/tryserver/api.py index c95ca5561..6b039eb16 100644 --- a/recipes/recipe_modules/tryserver/api.py +++ b/recipes/recipe_modules/tryserver/api.py @@ -178,6 +178,15 @@ class TryserverApi(recipe_api.RecipeApi): step_result = self.m.step.active_result step_result.presentation.properties['failure_type'] = failure_type + def set_do_not_retry_build(self): + """A flag to indicate the build should not be retried by the CQ. + + This mechanism is used to reduce CQ duration when retrying will likely + return an identical result. + """ + step_result = self.m.step.active_result + step_result.presentation.properties['do_not_retry'] = True + def set_patch_failure_tryjob_result(self): """Mark the tryjob result as failure to apply the patch.""" self._set_failure_type('PATCH_FAILURE') diff --git a/recipes/recipe_modules/tryserver/examples/full.expected/with_gerrit_patch.json b/recipes/recipe_modules/tryserver/examples/full.expected/with_gerrit_patch.json index b0e66ad20..b17395b3f 100644 --- a/recipes/recipe_modules/tryserver/examples/full.expected/with_gerrit_patch.json +++ b/recipes/recipe_modules/tryserver/examples/full.expected/with_gerrit_patch.json @@ -117,6 +117,7 @@ "~followup_annotations": [ "@@@STEP_LOG_LINE@files@None/foo.cc@@@", "@@@STEP_LOG_END@files@@@", + "@@@SET_BUILD_PROPERTY@do_not_retry@true@@@", "@@@SET_BUILD_PROPERTY@failure_type@\"INVALID_TEST_RESULTS\"@@@", "@@@SET_BUILD_PROPERTY@subproject_tag@\"v8\"@@@" ] diff --git a/recipes/recipe_modules/tryserver/examples/full.expected/with_gerrit_patch_and_target_ref.json b/recipes/recipe_modules/tryserver/examples/full.expected/with_gerrit_patch_and_target_ref.json index 0c24003a4..e880ab211 100644 --- a/recipes/recipe_modules/tryserver/examples/full.expected/with_gerrit_patch_and_target_ref.json +++ b/recipes/recipe_modules/tryserver/examples/full.expected/with_gerrit_patch_and_target_ref.json @@ -117,6 +117,7 @@ "~followup_annotations": [ "@@@STEP_LOG_LINE@files@None/foo.cc@@@", "@@@STEP_LOG_END@files@@@", + "@@@SET_BUILD_PROPERTY@do_not_retry@true@@@", "@@@SET_BUILD_PROPERTY@failure_type@\"INVALID_TEST_RESULTS\"@@@", "@@@SET_BUILD_PROPERTY@subproject_tag@\"v8\"@@@" ] diff --git a/recipes/recipe_modules/tryserver/examples/full.expected/with_git_patch.json b/recipes/recipe_modules/tryserver/examples/full.expected/with_git_patch.json index aed5a39d7..005a6db78 100644 --- a/recipes/recipe_modules/tryserver/examples/full.expected/with_git_patch.json +++ b/recipes/recipe_modules/tryserver/examples/full.expected/with_git_patch.json @@ -15,6 +15,7 @@ "~followup_annotations": [ "@@@STEP_LOG_LINE@files@v8/foo.cc@@@", "@@@STEP_LOG_END@files@@@", + "@@@SET_BUILD_PROPERTY@do_not_retry@true@@@", "@@@SET_BUILD_PROPERTY@failure_type@\"INVALID_TEST_RESULTS\"@@@", "@@@SET_BUILD_PROPERTY@subproject_tag@\"v8\"@@@" ] diff --git a/recipes/recipe_modules/tryserver/examples/full.expected/with_git_patch_luci.json b/recipes/recipe_modules/tryserver/examples/full.expected/with_git_patch_luci.json index aed5a39d7..005a6db78 100644 --- a/recipes/recipe_modules/tryserver/examples/full.expected/with_git_patch_luci.json +++ b/recipes/recipe_modules/tryserver/examples/full.expected/with_git_patch_luci.json @@ -15,6 +15,7 @@ "~followup_annotations": [ "@@@STEP_LOG_LINE@files@v8/foo.cc@@@", "@@@STEP_LOG_END@files@@@", + "@@@SET_BUILD_PROPERTY@do_not_retry@true@@@", "@@@SET_BUILD_PROPERTY@failure_type@\"INVALID_TEST_RESULTS\"@@@", "@@@SET_BUILD_PROPERTY@subproject_tag@\"v8\"@@@" ] diff --git a/recipes/recipe_modules/tryserver/examples/full.expected/with_wrong_patch.json b/recipes/recipe_modules/tryserver/examples/full.expected/with_wrong_patch.json index d9b6590da..763fd3652 100644 --- a/recipes/recipe_modules/tryserver/examples/full.expected/with_wrong_patch.json +++ b/recipes/recipe_modules/tryserver/examples/full.expected/with_wrong_patch.json @@ -13,7 +13,8 @@ "stdout": "/path/to/tmp/", "~followup_annotations": [ "@@@STEP_LOG_LINE@files@foo.cc@@@", - "@@@STEP_LOG_END@files@@@" + "@@@STEP_LOG_END@files@@@", + "@@@SET_BUILD_PROPERTY@do_not_retry@true@@@" ] }, { diff --git a/recipes/recipe_modules/tryserver/examples/full.expected/with_wrong_patch_new.json b/recipes/recipe_modules/tryserver/examples/full.expected/with_wrong_patch_new.json index 790f0b78e..c29131661 100644 --- a/recipes/recipe_modules/tryserver/examples/full.expected/with_wrong_patch_new.json +++ b/recipes/recipe_modules/tryserver/examples/full.expected/with_wrong_patch_new.json @@ -14,7 +14,8 @@ "stdout": "/path/to/tmp/", "~followup_annotations": [ "@@@STEP_LOG_LINE@files@sub/project/foo.cc@@@", - "@@@STEP_LOG_END@files@@@" + "@@@STEP_LOG_END@files@@@", + "@@@SET_BUILD_PROPERTY@do_not_retry@true@@@" ] }, { diff --git a/recipes/recipe_modules/tryserver/examples/full.py b/recipes/recipe_modules/tryserver/examples/full.py index 313320eee..4d4c342bc 100644 --- a/recipes/recipe_modules/tryserver/examples/full.py +++ b/recipes/recipe_modules/tryserver/examples/full.py @@ -46,6 +46,7 @@ def RunSteps(api): api.tryserver.set_subproject_tag('v8') api.tryserver.set_patch_failure_tryjob_result() + api.tryserver.set_do_not_retry_build() api.tryserver.set_compile_failure_tryjob_result() api.tryserver.set_test_failure_tryjob_result() api.tryserver.set_invalid_test_results_tryjob_result()