Merge pull request #64093 from ClickHouse/ci_mergeable_check_redesign

CI: mergeable check redesign

.github/workflows/pull_request.yml

@@ -130,15 +130,21 @@ jobs:
     with:
       stage: Tests_2
       data: ${{ needs.RunConfig.outputs.data }}
+  # stage for jobs that do not prohibit merge
+  Tests_3:
+    needs: [RunConfig, Tests_1, Tests_2]
+    if: ${{ !failure() && !cancelled() && contains(fromJson(needs.RunConfig.outputs.data).stages_data.stages_to_do, 'Tests_3') }}
+    uses: ./.github/workflows/reusable_test_stage.yml
+    with:
+      stage: Tests_3
+      data: ${{ needs.RunConfig.outputs.data }}
 
   ################################# Reports #################################
-  # Reports should by run even if Builds_1/2 fail, so put them separatly in wf (not in Tests_1/2)
+  # Reports should by run even if Builds_1/2 fail, so put them separately in wf (not in Tests_1/2)
   Builds_1_Report:
     # run report check for failed builds to indicate the CI error
-    if: ${{ !cancelled() && contains(fromJson(needs.RunConfig.outputs.data).jobs_data.jobs_to_do, 'ClickHouse build check') }}
-    needs:
-      - RunConfig
-      - Builds_1
+    if: ${{ !cancelled() && needs.StyleCheck.result == 'success' && contains(fromJson(needs.RunConfig.outputs.data).jobs_data.jobs_to_do, 'ClickHouse build check') }}
+    needs: [RunConfig, StyleCheck, Builds_1]
     uses: ./.github/workflows/reusable_test.yml
     with:
       test_name: ClickHouse build check
@@ -146,25 +152,39 @@ jobs:
       data: ${{ needs.RunConfig.outputs.data }}
   Builds_2_Report:
     # run report check for failed builds to indicate the CI error
-    if: ${{ !cancelled() && contains(fromJson(needs.RunConfig.outputs.data).jobs_data.jobs_to_do, 'ClickHouse special build check') }}
-    needs:
-      - RunConfig
-      - Builds_2
+    if: ${{ !cancelled() && needs.StyleCheck.result == 'success' && contains(fromJson(needs.RunConfig.outputs.data).jobs_data.jobs_to_do, 'ClickHouse special build check') }}
+    needs: [RunConfig, StyleCheck, Builds_2]
     uses: ./.github/workflows/reusable_test.yml
     with:
       test_name: ClickHouse special build check
       runner_type: style-checker-aarch64
       data: ${{ needs.RunConfig.outputs.data }}
 
+  CheckReadyForMerge:
+    if: ${{ !cancelled() && needs.StyleCheck.result == 'success' }}
+    needs: [RunConfig, BuildDockers, StyleCheck, FastTest, Builds_1, Builds_2, Builds_1_Report, Builds_2_Report, Tests_1, Tests_2]
+    runs-on: [self-hosted, style-checker-aarch64]
+    steps:
+      - name: Check out repository code
+        uses: ClickHouse/checkout@v1
+        with:
+          filter: tree:0
+      - name: Check and set merge status
+        run: |
+          cd "$GITHUB_WORKSPACE/tests/ci"
+          python3 merge_pr.py --set-ci-status --wf-status ${{ contains(needs.*.result, 'failure') && 'failure' || 'success' }}
+
   ################################# Stage Final #################################
   #
   FinishCheck:
     if: ${{ !failure() && !cancelled() }}
-    needs: [RunConfig, BuildDockers, StyleCheck, FastTest, Builds_1, Builds_2, Builds_1_Report, Builds_2_Report, Tests_1, Tests_2]
+    needs: [RunConfig, BuildDockers, StyleCheck, FastTest, Builds_1, Builds_2, Builds_1_Report, Builds_2_Report, Tests_1, Tests_2, Tests_3]
     runs-on: [self-hosted, style-checker]
     steps:
       - name: Check out repository code
         uses: ClickHouse/checkout@v1
+        with:
+          filter: tree:0
       - name: Finish label
         run: |
           cd "$GITHUB_WORKSPACE/tests/ci"

tests/ci/ci.py

@@ -17,7 +17,7 @@ from typing import Any, Dict, List, Optional, Sequence, Set, Tuple, Union
 import docker_images_helper
 import upload_result_helper
 from build_check import get_release_or_pr
-from ci_config import CI_CONFIG, Build, CILabels, CIStages, JobNames, StatusNames
+from ci_config import CI_CONFIG, Build, CILabels, CIStages, JobNames
 from ci_utils import GHActions, is_hex, normalize_string
 from clickhouse_helper import (
     CiLogsCredentials,
@@ -34,16 +34,12 @@ from commit_status_helper import (
     get_commit,
     post_commit_status,
     set_status_comment,
-    update_mergeable_check,
-    update_upstream_sync_status,
 )
 from digest_helper import DockerDigester, JobDigester
 from env_helper import (
     CI,
     GITHUB_JOB_API_URL,
-    GITHUB_REPOSITORY,
     GITHUB_RUN_URL,
-    GITHUB_UPSTREAM_REPOSITORY,
     REPO_COPY,
     REPORT_PATH,
     S3_BUILDS_BUCKET,
@@ -56,7 +52,6 @@ from github_helper import GitHub
 from pr_info import PRInfo
 from report import ERROR, SUCCESS, BuildResult, JobReport
 from s3_helper import S3Helper
-from synchronizer_utils import SYNC_BRANCH_PREFIX
 from version_helper import get_version_from_repo
 
 # pylint: disable=too-many-lines
@@ -891,9 +886,9 @@ class CiOptions:
                     for job in job_with_parents:
                         if job in jobs_to_do and job not in jobs_to_do_requested:
                             jobs_to_do_requested.append(job)
-            assert (
-                jobs_to_do_requested
-            ), f"Include tags are set but no job configured - Invalid tags, probably [{self.include_keywords}]"
+            print(
+                f"WARNING: Include tags are set but no job configured - Invalid tags, probably [{self.include_keywords}]"
+            )
             if JobNames.STYLE_CHECK not in jobs_to_do_requested:
                 # Style check must not be omitted
                 jobs_to_do_requested.append(JobNames.STYLE_CHECK)
@@ -903,7 +898,7 @@ class CiOptions:
         if self.ci_sets:
             for tag in self.ci_sets:
                 label_config = CI_CONFIG.get_label_config(tag)
-                assert label_config, f"Unknonwn tag [{tag}]"
+                assert label_config, f"Unknown tag [{tag}]"
                 print(
                     f"NOTE: CI Set's tag: [{tag}], add jobs: [{label_config.run_jobs}]"
                 )
@@ -2189,39 +2184,6 @@ def main() -> int:
                     pr_info,
                     dump_to_file=True,
                 )
-                if not pr_info.is_merge_queue:
-                    # in the merge queue mergeable status must be set only in FinishCheck (last job in wf)
-                    mergeable_status = update_mergeable_check(
-                        commit,
-                        pr_info,
-                        job_report.check_name or _get_ext_check_name(args.job_name),
-                    )
-
-                    # Process upstream StatusNames.SYNC
-                    if (
-                        pr_info.head_ref.startswith(f"{SYNC_BRANCH_PREFIX}/pr/")
-                        and mergeable_status
-                        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, mergeable_status
-                        )
-                        prepared_events = prepare_tests_results_for_clickhouse(
-                            pr_info,
-                            [],
-                            job_report.status,
-                            0,
-                            job_report.start_time,
-                            f"https://github.com/ClickHouse/ClickHouse/pull/{upstream_pr_number}",
-                            StatusNames.SYNC,
-                        )
-                        prepared_events[0]["test_context_raw"] = args.job_name
-                        ch_helper.insert_events_into(
-                            db="default", table="checks", events=prepared_events
-                        )
 
             print(f"Job report url: [{check_url}]")
             prepared_events = prepare_tests_results_for_clickhouse(

tests/ci/ci_config.py

@@ -26,6 +26,7 @@ class CIStages(metaclass=WithIter):
     BUILDS_2 = "Builds_2"
     TESTS_1 = "Tests_1"
     TESTS_2 = "Tests_2"
+    TESTS_3 = "Tests_3"
 
 
 class Runners(metaclass=WithIter):
@@ -581,7 +582,6 @@ class CIConfig:
         elif job_name == JobNames.BUILD_CHECK_SPECIAL:
             stage_type = CIStages.TESTS_2
         elif self.is_test_job(job_name):
-            stage_type = CIStages.TESTS_1
             if job_name in CI_CONFIG.test_configs:
                 required_build = CI_CONFIG.test_configs[job_name].required_build
                 assert required_build
@@ -593,6 +593,8 @@ class CIConfig:
                     stage_type = CIStages.TESTS_2
             else:
                 stage_type = CIStages.TESTS_1
+            if job_name not in REQUIRED_CHECKS:
+                stage_type = CIStages.TESTS_3
         assert stage_type, f"BUG [{job_name}]"
         return stage_type
 

tests/ci/commit_status_helper.py

@@ -447,9 +447,7 @@ def set_mergeable_check(
     )
 
 
-def update_mergeable_check(
-    commit: Commit, pr_info: PRInfo, check_name: str
-) -> Optional[CommitStatus]:
+def update_mergeable_check(commit: Commit, pr_info: PRInfo, check_name: str) -> None:
     "check if the check_name in REQUIRED_CHECKS and then trigger update"
     not_run = (
         pr_info.labels.intersection({Labels.SKIP_MERGEABLE_CHECK, Labels.RELEASE})
@@ -460,17 +458,21 @@ def update_mergeable_check(
 
     if not_run:
         # Let's avoid unnecessary work
-        return None
+        return
 
     logging.info("Update Mergeable Check by %s", check_name)
 
     statuses = get_commit_filtered_statuses(commit)
-    return trigger_mergeable_check(commit, statuses)
+    trigger_mergeable_check(commit, statuses)
 
 
 def trigger_mergeable_check(
-    commit: Commit, statuses: CommitStatuses, hide_url: bool = False
-) -> CommitStatus:
+    commit: Commit,
+    statuses: CommitStatuses,
+    hide_url: bool = False,
+    set_if_green: bool = False,
+    workflow_failed: bool = False,
+) -> StatusType:
     """calculate and update StatusNames.MERGEABLE"""
     required_checks = [status for status in statuses if is_required(status.context)]
 
@@ -498,19 +500,27 @@ def trigger_mergeable_check(
     if fail:
         description = "failed: " + ", ".join(fail)
         state = FAILURE
+    elif workflow_failed:
+        description = "check workflow failures"
+        state = FAILURE
     description = format_description(description)
 
-    if mergeable_status is None or mergeable_status.description != description:
-        return set_mergeable_check(commit, description, state, hide_url)
+    if not set_if_green and state == SUCCESS:
+        # do not set green Mergeable Check status
+        pass
+    else:
+        if mergeable_status is None or mergeable_status.description != description:
+            set_mergeable_check(commit, description, state, hide_url)
 
-    return mergeable_status
+    return state
 
 
 def update_upstream_sync_status(
     upstream_pr_number: int,
     sync_pr_number: int,
     gh: Github,
-    mergeable_status: CommitStatus,
+    state: StatusType,
+    can_set_green_mergeable_status: bool = False,
 ) -> None:
     upstream_repo = gh.get_repo(GITHUB_UPSTREAM_REPOSITORY)
     upstream_pr = upstream_repo.get_pull(upstream_pr_number)
@@ -518,46 +528,41 @@ def update_upstream_sync_status(
     sync_pr = sync_repo.get_pull(sync_pr_number)
     # Find the commit that is in both repos, upstream and cloud
     sync_commits = sync_pr.get_commits().reversed
-    upstream_commits = upstream_pr.get_commits()
+    upstream_commits = upstream_pr.get_commits().reversed
     # Github objects are compared by _url attribute. We can't compare them directly and
     # should compare commits by SHA1
-    upstream_shas = [uc.sha for uc in upstream_commits]
+    upstream_shas = [c.sha for c in upstream_commits]
     logging.info("Commits in upstream PR:\n %s", ", ".join(upstream_shas))
-    sync_shas = [uc.sha for uc in upstream_commits]
+    sync_shas = [c.sha for c in sync_commits]
     logging.info("Commits in sync PR:\n %s", ", ".join(reversed(sync_shas)))
-    found = False
-    for commit in sync_commits:
-        try:
-            idx = upstream_shas.index(commit.sha)
-            found = True
-            upstream_commit = upstream_commits[idx]
+
+    # find latest synced commit
+    last_synced_upstream_commit = None
+    for commit in upstream_commits:
+        if commit.sha in sync_shas:
+            last_synced_upstream_commit = commit
             break
-        except ValueError:
-            continue
 
-    if not found:
-        logging.info(
-            "There's no same commits in upstream and sync PRs, probably force-push"
-        )
-        return
+    assert last_synced_upstream_commit
 
-    sync_status = get_status(mergeable_status.state)
+    sync_status = get_status(state)
     logging.info(
         "Using commit %s to post the %s status `%s`: [%s]",
-        upstream_commit.sha,
+        last_synced_upstream_commit.sha,
         sync_status,
         StatusNames.SYNC,
-        mergeable_status.description,
+        "",
     )
     post_commit_status(
-        upstream_commit,
+        last_synced_upstream_commit,
         sync_status,
         "",  # let's won't expose any urls from cloud
-        mergeable_status.description,
+        "",
         StatusNames.SYNC,
     )
     trigger_mergeable_check(
-        upstream_commit,
-        get_commit_filtered_statuses(upstream_commit),
+        last_synced_upstream_commit,
+        get_commit_filtered_statuses(last_synced_upstream_commit),
         True,
+        set_if_green=can_set_green_mergeable_status,
     )

tests/ci/finish_check.py

@@ -11,10 +11,13 @@ from commit_status_helper import (
     post_commit_status,
     set_mergeable_check,
     trigger_mergeable_check,
+    update_upstream_sync_status,
 )
 from get_robot_token import get_best_robot_token
 from pr_info import PRInfo
 from report import PENDING, SUCCESS
+from synchronizer_utils import SYNC_BRANCH_PREFIX
+from env_helper import GITHUB_REPOSITORY, GITHUB_UPSTREAM_REPOSITORY
 
 
 def main():
@@ -40,7 +43,21 @@ def main():
             set_mergeable_check(commit, "workflow passed", "success")
     else:
         statuses = get_commit_filtered_statuses(commit)
-        trigger_mergeable_check(commit, statuses)
+        state = trigger_mergeable_check(commit, statuses, set_if_green=True)
+
+        # 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 = [s for s in statuses if s.context == StatusNames.CI]
         if not statuses:

tests/ci/merge_pr.py

@@ -13,7 +13,11 @@ from github.PaginatedList import PaginatedList
 from github.PullRequestReview import PullRequestReview
 from github.WorkflowRun import WorkflowRun
 
-from commit_status_helper import get_commit_filtered_statuses
+from commit_status_helper import (
+    get_commit_filtered_statuses,
+    get_commit,
+    trigger_mergeable_check,
+)
 from get_robot_token import get_best_robot_token
 from github_helper import GitHub, NamedUser, PullRequest, Repository
 from pr_info import PRInfo
@@ -173,6 +177,17 @@ def parse_args() -> argparse.Namespace:
         action="store_true",
         help="if set, the script won't merge the PR, just check the conditions",
     )
+    parser.add_argument(
+        "--set-ci-status",
+        action="store_true",
+        help="if set, only update/set Mergeable Check status",
+    )
+    parser.add_argument(
+        "--wf-status",
+        type=str,
+        default="",
+        help="overall workflow status [success|failure]. used with --set-ci-status only",
+    )
     parser.add_argument(
         "--check-approved",
         action="store_true",
@@ -226,6 +241,21 @@ def main():
     token = args.token or get_best_robot_token()
     gh = GitHub(token)
     repo = gh.get_repo(args.repo)
+
+    if args.set_ci_status:
+        assert args.wf_status in ("failure", "success")
+        # set mergeable check status and exit
+        commit = get_commit(gh, args.pr_info.sha)
+        statuses = get_commit_filtered_statuses(commit)
+        trigger_mergeable_check(
+            commit,
+            statuses,
+            hide_url=False,
+            set_if_green=True,
+            workflow_failed=(args.wf_status != "success"),
+        )
+        return
+
     # An ugly and not nice fix to patch the wrong organization URL,
     # see https://github.com/PyGithub/PyGithub/issues/2395#issuecomment-1378629710
     # pylint: disable=protected-access