From 0d17f2cededd98d80d5d8f1486e62899ed953f43 Mon Sep 17 00:00:00 2001 From: Max K Date: Thu, 13 Jun 2024 16:14:57 +0200 Subject: [PATCH 1/5] CI: FinishCheck to set failure if workflow failed --- .github/workflows/backport_branches.yml | 2 +- .github/workflows/master.yml | 2 +- .github/workflows/merge_queue.yml | 2 +- .github/workflows/pull_request.yml | 2 +- .github/workflows/release_branches.yml | 2 +- tests/ci/finish_check.py | 42 ++++++++++++++++++------- 6 files changed, 36 insertions(+), 16 deletions(-) diff --git a/.github/workflows/backport_branches.yml b/.github/workflows/backport_branches.yml index b0380b939bb..c8c6ba30b0b 100644 --- a/.github/workflows/backport_branches.yml +++ b/.github/workflows/backport_branches.yml @@ -273,5 +273,5 @@ jobs: - name: Finish label run: | cd "$GITHUB_WORKSPACE/tests/ci" - python3 finish_check.py + python3 finish_check.py --wf-status ${{ contains(needs.*.result, 'failure') && 'failure' || 'success' }} python3 merge_pr.py diff --git a/.github/workflows/master.yml b/.github/workflows/master.yml index 91dcb6a4968..f5c78a6b6a1 100644 --- a/.github/workflows/master.yml +++ b/.github/workflows/master.yml @@ -173,4 +173,4 @@ jobs: - name: Finish label run: | cd "$GITHUB_WORKSPACE/tests/ci" - python3 finish_check.py + python3 finish_check.py --wf-status ${{ contains(needs.*.result, 'failure') && 'failure' || 'success' }} diff --git a/.github/workflows/merge_queue.yml b/.github/workflows/merge_queue.yml index c8b2452829b..3f45daf0fb4 100644 --- a/.github/workflows/merge_queue.yml +++ b/.github/workflows/merge_queue.yml @@ -112,4 +112,4 @@ jobs: - name: Finish label run: | cd "$GITHUB_WORKSPACE/tests/ci" - python3 finish_check.py ${{ (contains(needs.*.result, 'failure') && github.event_name == 'merge_group') && '--pipeline-failure' || '' }} + python3 finish_check.py --wf-status ${{ contains(needs.*.result, 'failure') && 'failure' || 'success' }} diff --git a/.github/workflows/pull_request.yml b/.github/workflows/pull_request.yml index e4deaf9f35e..079208eb65a 100644 --- a/.github/workflows/pull_request.yml +++ b/.github/workflows/pull_request.yml @@ -191,7 +191,7 @@ jobs: - name: Finish label run: | cd "$GITHUB_WORKSPACE/tests/ci" - python3 finish_check.py + python3 finish_check.py --wf-status ${{ contains(needs.*.result, 'failure') && 'failure' || 'success' }} ############################################################################################# ###################################### JEPSEN TESTS ######################################### diff --git a/.github/workflows/release_branches.yml b/.github/workflows/release_branches.yml index 4d45c8d8d4b..f9b8a4fa764 100644 --- a/.github/workflows/release_branches.yml +++ b/.github/workflows/release_branches.yml @@ -496,4 +496,4 @@ jobs: - name: Finish label run: | cd "$GITHUB_WORKSPACE/tests/ci" - python3 finish_check.py + python3 finish_check.py --wf-status ${{ contains(needs.*.result, 'failure') && 'failure' || 'success' }} diff --git a/tests/ci/finish_check.py b/tests/ci/finish_check.py index 12756599865..6a32ba71bed 100644 --- a/tests/ci/finish_check.py +++ b/tests/ci/finish_check.py @@ -1,4 +1,5 @@ #!/usr/bin/env python3 +import argparse import logging import sys @@ -20,27 +21,38 @@ from report import FAILURE, PENDING, SUCCESS, StatusType from synchronizer_utils import SYNC_BRANCH_PREFIX +def parse_args() -> argparse.Namespace: + parser = argparse.ArgumentParser( + formatter_class=argparse.ArgumentDefaultsHelpFormatter, + description="Script to merge the given PR. Additional checks for approved " + "status and green commit statuses could be done", + ) + parser.add_argument( + "--wf-status", + type=str, + default="", + help="overall workflow status [success|failure]", + ) + return parser.parse_args() + + def main(): logging.basicConfig(level=logging.INFO) + args = parse_args() - has_failure = False - - # FIXME: temporary hack to fail Mergeable Check in MQ if pipeline has any failed jobs - if len(sys.argv) > 1 and sys.argv[1] == "--pipeline-failure": - has_failure = True + has_workflow_failures = args.wf_status == FAILURE pr_info = PRInfo(need_orgs=True) gh = Github(get_best_robot_token(), per_page=100) commit = get_commit(gh, pr_info.sha) - statuses = None if pr_info.is_merge_queue: - # in MQ Mergeable check status must never be green if any failures in workflow - if has_failure: - set_mergeable_check(commit, "workflow failed", "failure") + # in MQ Mergeable check status must never be green if any failures in the workflow + if has_workflow_failures: + set_mergeable_check(commit, "workflow failed", FAILURE) else: # This must be the only place where green MCheck is set in the MQ (in the end of CI) to avoid early merge - set_mergeable_check(commit, "workflow passed", "success") + set_mergeable_check(commit, "workflow passed", SUCCESS) else: statuses = get_commit_filtered_statuses(commit) state = trigger_mergeable_check(commit, statuses, set_if_green=True) @@ -67,6 +79,7 @@ def main(): has_failure = False has_pending = False + error_cnt = 0 for status in statuses: if status.context in (StatusNames.MERGEABLE, StatusNames.CI): # do not account these statuses @@ -80,12 +93,19 @@ def main(): continue else: has_failure = True + error_cnt += 1 ci_state = SUCCESS # type: StatusType + description = "All checks finished" if has_failure: ci_state = FAILURE + description = f"All checks finished. {error_cnt} jobs failed" + elif has_workflow_failures: + ci_state = FAILURE + description = "All checks finished. Workflow has failures." elif has_pending: print("ERROR: CI must not have pending jobs by the time of finish check") + description = "ERROR: workflow has pending jobs" ci_state = FAILURE if ci_status.state == PENDING: @@ -93,7 +113,7 @@ def main(): commit, ci_state, ci_status.target_url, - "All checks finished", + description, StatusNames.CI, pr_info, dump_to_file=True, From a77eda6388bbb3c887d1b621e6a191269c6ba5c5 Mon Sep 17 00:00:00 2001 From: Max K Date: Thu, 13 Jun 2024 16:27:41 +0200 Subject: [PATCH 2/5] fix pylint --- tests/ci/finish_check.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/ci/finish_check.py b/tests/ci/finish_check.py index 6a32ba71bed..2f624fd91f8 100644 --- a/tests/ci/finish_check.py +++ b/tests/ci/finish_check.py @@ -1,7 +1,6 @@ #!/usr/bin/env python3 import argparse import logging -import sys from github import Github From fc255456cee541f70f54c4365c410d053d073c22 Mon Sep 17 00:00:00 2001 From: Max K Date: Thu, 13 Jun 2024 16:36:07 +0200 Subject: [PATCH 3/5] comment --- tests/ci/finish_check.py | 119 ++++++++++++++++++++------------------- 1 file changed, 60 insertions(+), 59 deletions(-) diff --git a/tests/ci/finish_check.py b/tests/ci/finish_check.py index 2f624fd91f8..0d59c3b43a4 100644 --- a/tests/ci/finish_check.py +++ b/tests/ci/finish_check.py @@ -52,71 +52,72 @@ def main(): else: # This must be the only place where green MCheck is set in the MQ (in the end of CI) to avoid early merge set_mergeable_check(commit, "workflow passed", SUCCESS) - else: - statuses = get_commit_filtered_statuses(commit) - state = trigger_mergeable_check(commit, statuses, set_if_green=True) + return - # Process upstream StatusNames.SYNC - if ( - pr_info.head_ref.startswith(f"{SYNC_BRANCH_PREFIX}/pr/") - and GITHUB_REPOSITORY != GITHUB_UPSTREAM_REPOSITORY - ): - upstream_pr_number = int(pr_info.head_ref.split("/pr/", maxsplit=1)[1]) - update_upstream_sync_status( - upstream_pr_number, - pr_info.number, - gh, - state, - can_set_green_mergeable_status=True, - ) + statuses = get_commit_filtered_statuses(commit) + state = trigger_mergeable_check(commit, statuses, set_if_green=True) - ci_running_statuses = [s for s in statuses if s.context == StatusNames.CI] - if not ci_running_statuses: - return - # Take the latest status - ci_status = ci_running_statuses[-1] + # Process upstream StatusNames.SYNC + if ( + pr_info.head_ref.startswith(f"{SYNC_BRANCH_PREFIX}/pr/") + and GITHUB_REPOSITORY != GITHUB_UPSTREAM_REPOSITORY + ): + upstream_pr_number = int(pr_info.head_ref.split("/pr/", maxsplit=1)[1]) + update_upstream_sync_status( + upstream_pr_number, + pr_info.number, + gh, + state, + can_set_green_mergeable_status=True, + ) - has_failure = False - has_pending = False - error_cnt = 0 - for status in statuses: - if status.context in (StatusNames.MERGEABLE, StatusNames.CI): - # do not account these statuses + ci_running_statuses = [s for s in statuses if s.context == StatusNames.CI] + if not ci_running_statuses: + return + # Take the latest status + ci_status = ci_running_statuses[-1] + + has_failure = False + has_pending = False + error_cnt = 0 + for status in statuses: + if status.context in (StatusNames.MERGEABLE, StatusNames.CI): + # do not account these statuses + continue + if status.state == PENDING: + if status.context == StatusNames.SYNC: + # do not account sync status if pending - it's a different WF continue - if status.state == PENDING: - if status.context == StatusNames.SYNC: - # do not account sync status if pending - it's a different WF - continue - has_pending = True - elif status.state == SUCCESS: - continue - else: - has_failure = True - error_cnt += 1 + has_pending = True + elif status.state == SUCCESS: + continue + else: + has_failure = True + error_cnt += 1 - ci_state = SUCCESS # type: StatusType - description = "All checks finished" - if has_failure: - ci_state = FAILURE - description = f"All checks finished. {error_cnt} jobs failed" - elif has_workflow_failures: - ci_state = FAILURE - description = "All checks finished. Workflow has failures." - elif has_pending: - print("ERROR: CI must not have pending jobs by the time of finish check") - description = "ERROR: workflow has pending jobs" - ci_state = FAILURE + ci_state = SUCCESS # type: StatusType + description = "All checks finished" + if has_failure: + ci_state = FAILURE + description = f"All checks finished. {error_cnt} jobs failed" + elif has_workflow_failures: + ci_state = FAILURE + description = "All checks finished. Workflow has failures." + elif has_pending: + print("ERROR: CI must not have pending jobs by the time of finish check") + description = "ERROR: workflow has pending jobs" + ci_state = FAILURE - if ci_status.state == PENDING: - post_commit_status( - commit, - ci_state, - ci_status.target_url, - description, - StatusNames.CI, - pr_info, - dump_to_file=True, - ) + if ci_status.state == PENDING: + post_commit_status( + commit, + ci_state, + ci_status.target_url, + description, + StatusNames.CI, + pr_info, + dump_to_file=True, + ) if __name__ == "__main__": From 5bc879c07c77f4ddaf4498ba6db52ecffc49fb3e Mon Sep 17 00:00:00 2001 From: Max K Date: Thu, 13 Jun 2024 16:40:01 +0200 Subject: [PATCH 4/5] comment --- tests/ci/finish_check.py | 28 +++++++++++----------------- 1 file changed, 11 insertions(+), 17 deletions(-) diff --git a/tests/ci/finish_check.py b/tests/ci/finish_check.py index 0d59c3b43a4..904b565ad86 100644 --- a/tests/ci/finish_check.py +++ b/tests/ci/finish_check.py @@ -81,17 +81,12 @@ def main(): has_pending = False error_cnt = 0 for status in statuses: - if status.context in (StatusNames.MERGEABLE, StatusNames.CI): + if status.context in (StatusNames.MERGEABLE, StatusNames.CI, StatusNames.SYNC): # do not account these statuses continue if status.state == PENDING: - if status.context == StatusNames.SYNC: - # do not account sync status if pending - it's a different WF - continue has_pending = True - elif status.state == SUCCESS: - continue - else: + elif status.state != SUCCESS: has_failure = True error_cnt += 1 @@ -108,16 +103,15 @@ def main(): description = "ERROR: workflow has pending jobs" ci_state = FAILURE - if ci_status.state == PENDING: - post_commit_status( - commit, - ci_state, - ci_status.target_url, - description, - StatusNames.CI, - pr_info, - dump_to_file=True, - ) + post_commit_status( + commit, + ci_state, + ci_status.target_url, + description, + StatusNames.CI, + pr_info, + dump_to_file=True, + ) if __name__ == "__main__": From 99ce17fb2b0a1b5c7787470690f8e14c11005101 Mon Sep 17 00:00:00 2001 From: Max K Date: Thu, 13 Jun 2024 22:15:39 +0200 Subject: [PATCH 5/5] style fix --- .github/workflows/pull_request.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/pull_request.yml b/.github/workflows/pull_request.yml index 079208eb65a..66ca3381a40 100644 --- a/.github/workflows/pull_request.yml +++ b/.github/workflows/pull_request.yml @@ -191,7 +191,7 @@ jobs: - name: Finish label run: | cd "$GITHUB_WORKSPACE/tests/ci" - python3 finish_check.py --wf-status ${{ contains(needs.*.result, 'failure') && 'failure' || 'success' }} + python3 finish_check.py --wf-status ${{ contains(needs.*.result, 'failure') && 'failure' || 'success' }} ############################################################################################# ###################################### JEPSEN TESTS #########################################