From c2bb888e9a52e3428ed018b470da0457968a4a81 Mon Sep 17 00:00:00 2001 From: Feng Huang Date: Tue, 21 Jan 2025 22:16:55 -0500 Subject: [PATCH] add verify_ondemend_tests for gitlab-housekeeping Signed-off-by: Feng Huang --- reconcile/gitlab_housekeeping.py | 106 ++++++++++++- reconcile/gql_definitions/introspection.json | 20 +++ reconcile/queries.py | 1 + reconcile/test/test_gitlab_housekeeping.py | 155 +++++++++++++++++++ 4 files changed, 279 insertions(+), 3 deletions(-) diff --git a/reconcile/gitlab_housekeeping.py b/reconcile/gitlab_housekeeping.py index 7795d6e5bb..534e639848 100644 --- a/reconcile/gitlab_housekeeping.py +++ b/reconcile/gitlab_housekeeping.py @@ -1,6 +1,7 @@ import logging from collections.abc import ( Iterable, + Mapping, Set, ) from contextlib import suppress @@ -49,6 +50,7 @@ prioritized_approval_label, ) from reconcile.utils.sharding import is_in_shard +from reconcile.utils.state import State, init_state MERGE_LABELS_PRIORITY = [ prioritized_approval_label(p.value) for p in ChangeTypePriority @@ -71,7 +73,6 @@ DATE_FORMAT = "%Y-%m-%dT%H:%M:%S.%fZ" EXPIRATION_DATE_FORMAT = "%Y-%m-%d" - merged_merge_requests = Counter( name="qontract_reconcile_merged_merge_requests", documentation="Number of merge requests that have been successfully merged in a repository", @@ -179,6 +180,71 @@ def clean_pipelines( ) +def verify_ondemend_tests( + dry_run: bool, + mr: ProjectMergeRequest, + must_pass: Iterable[str], + gl: GitLabApi, + gl_instance: Mapping[str, Any], + gl_settings: Mapping[str, Any], + state: State, +) -> bool: + pipelines = gl.get_merge_request_pipelines(mr) + running_pipelines = [p for p in pipelines if p["status"] == "running"] + if running_pipelines: + # wait for pipeline complate + return False + + gl_fork = GitLabApi( + instance=gl_instance, + project_id=mr.source_project_id, + settings=gl_settings, + ) + commit = next(mr.commits()) + statuses = gl_fork.project.commits.get(commit.id).statuses.list() + test_state = {} + for s in statuses: + test_state[s.name] = s.status + state_key = f"{gl.project.path_with_namespace}/{mr.iid}/{commit.id}" + remain_tests = [t for t in must_pass if test_state.get(t) != "success"] + # only add comment when state changes + state_change = state.get(state_key, None) != remain_tests + + if remain_tests: + logging.info([ + "ondemend tests", + "add comment", + gl.project.name, + mr.iid, + commit.id, + ]) + if not dry_run and state_change: + markdown_report = ( + f"Ondemend Tests: \n\n For latest [commit]({commit.web_url}) You will need to pass following test jobs to get this MR merged.\n\n" + f"Add comment with `/test [test_name]` to trigger the tests.\n\n" + ) + markdown_report += f"* {', '.join(remain_tests)}\n" + gl.delete_merge_request_comments(mr, startswith="Ondemend Tests:") + gl.add_comment_to_merge_request(mr, markdown_report) + state.add(state_key, remain_tests, force=True) + return False + else: + # no remain_tests, pass the check + logging.info([ + "ondemend tests", + "check pass", + gl.project.name, + mr.iid, + commit.id, + ]) + if not dry_run and state_change: + markdown_report = f"Ondemend Tests: \n\n All necessary tests have paased for latest [commit]({commit.web_url})\n" + gl.delete_merge_request_comments(mr, startswith="Ondemend Tests:") + gl.add_comment_to_merge_request(mr, markdown_report) + state.add(state_key, remain_tests, force=True) + return True + + def close_item( dry_run: bool, gl: GitLabApi, @@ -291,6 +357,7 @@ def is_rebased(mr, gl: GitLabApi) -> bool: def get_merge_requests( dry_run: bool, gl: GitLabApi, + state: State, users_allowed_to_label: Iterable[str] | None = None, ) -> list[dict[str, Any]]: mrs = gl.get_merge_requests(state=MRState.OPENED) @@ -298,6 +365,7 @@ def get_merge_requests( dry_run=dry_run, gl=gl, project_merge_requests=mrs, + state=state, users_allowed_to_label=users_allowed_to_label, ) @@ -306,10 +374,17 @@ def preprocess_merge_requests( dry_run: bool, gl: GitLabApi, project_merge_requests: list[ProjectMergeRequest], + state: State, users_allowed_to_label: Iterable[str] | None = None, + must_pass: Iterable[str] = [], + gl_instance: Mapping[str, Any] = {}, + gl_settings: Mapping[str, Any] = {}, ) -> list[dict[str, Any]]: results = [] for mr in project_merge_requests: + # if mr.iid != 7: + # continue + if mr.merge_status in { MRStatus.CANNOT_BE_MERGED, MRStatus.CANNOT_BE_MERGED_RECHECK, @@ -320,6 +395,11 @@ def preprocess_merge_requests( if len(mr.commits()) == 0: continue + if must_pass and not verify_ondemend_tests( + dry_run, mr, must_pass, gl, gl_instance, gl_settings, state + ): + continue + labels = set(mr.labels) if not labels: continue @@ -405,10 +485,12 @@ def rebase_merge_requests( gl_instance=None, gl_settings=None, users_allowed_to_label=None, + state=None, ): rebases = 0 merge_requests = [ - item["mr"] for item in get_merge_requests(dry_run, gl, users_allowed_to_label) + item["mr"] + for item in get_merge_requests(dry_run, gl, state, users_allowed_to_label) ] for mr in merge_requests: if is_rebased(mr, gl): @@ -477,12 +559,21 @@ def merge_merge_requests( gl_instance=None, gl_settings=None, users_allowed_to_label=None, + must_pass=None, + state=None, ): merges = 0 if reload_toggle.reload: project_merge_requests = gl.get_merge_requests(state=MRState.OPENED) merge_requests = preprocess_merge_requests( - dry_run, gl, project_merge_requests, users_allowed_to_label + dry_run, + gl, + project_merge_requests, + state, + users_allowed_to_label, + must_pass, + gl_instance, + gl_settings, ) merge_requests_waiting.labels(gl.project.id).set(len(merge_requests)) @@ -582,10 +673,13 @@ def run(dry_run, wait_for_pipeline): repos = queries.get_repos_gitlab_housekeeping(server=instance["url"]) repos = [r for r in repos if is_in_shard(r["url"])] app_sre_usernames: Set[str] = set() + state = init_state(QONTRACT_INTEGRATION) for repo in repos: hk = repo["housekeeping"] project_url = repo["url"] + # if project_url != 'https://gitlab.cee.redhat.com/app-sre/stage-ci-test': + # continue days_interval = hk.get("days_interval") or default_days_interval enable_closing = hk.get("enable_closing") or default_enable_closing limit = hk.get("limit") or default_limit @@ -624,6 +718,7 @@ def run(dry_run, wait_for_pipeline): ] reload_toggle = ReloadToggle(reload=False) rebase = hk.get("rebase") + must_pass = hk.get("must_pass") try: merge_merge_requests( dry_run, @@ -639,6 +734,8 @@ def run(dry_run, wait_for_pipeline): gl_instance=instance, gl_settings=settings, users_allowed_to_label=users_allowed_to_label, + must_pass=must_pass, + state=state, ) except Exception: logging.error( @@ -657,6 +754,8 @@ def run(dry_run, wait_for_pipeline): gl_instance=instance, gl_settings=settings, users_allowed_to_label=users_allowed_to_label, + must_pass=must_pass, + state=state, ) if rebase: rebase_merge_requests( @@ -668,4 +767,5 @@ def run(dry_run, wait_for_pipeline): gl_instance=instance, gl_settings=settings, users_allowed_to_label=users_allowed_to_label, + state=state, ) diff --git a/reconcile/gql_definitions/introspection.json b/reconcile/gql_definitions/introspection.json index cb40efbe44..11fb66a63e 100644 --- a/reconcile/gql_definitions/introspection.json +++ b/reconcile/gql_definitions/introspection.json @@ -20519,6 +20519,26 @@ }, "isDeprecated": false, "deprecationReason": null + }, + { + "name": "must_pass", + "description": null, + "args": [], + "type": { + "kind": "LIST", + "name": null, + "ofType": { + "kind": "NON_NULL", + "name": null, + "ofType": { + "kind": "SCALAR", + "name": "String", + "ofType": null + } + } + }, + "isDeprecated": false, + "deprecationReason": null } ], "inputFields": null, diff --git a/reconcile/queries.py b/reconcile/queries.py index 7febf9419a..864cf91a03 100644 --- a/reconcile/queries.py +++ b/reconcile/queries.py @@ -1648,6 +1648,7 @@ def get_environments(): } } } + must_pass } jira { serverUrl diff --git a/reconcile/test/test_gitlab_housekeeping.py b/reconcile/test/test_gitlab_housekeeping.py index e79eea96bc..c54d1aa107 100644 --- a/reconcile/test/test_gitlab_housekeeping.py +++ b/reconcile/test/test_gitlab_housekeeping.py @@ -14,6 +14,7 @@ Project, ProjectCommit, ProjectCommitManager, + ProjectCommitStatusManager, ProjectIssue, ProjectMergeRequest, ProjectMergeRequestResourceLabelEvent, @@ -24,6 +25,7 @@ from reconcile.test.fixtures import Fixtures from reconcile.utils.gitlab_api import GitLabApi from reconcile.utils.secret_reader import SecretReader +from reconcile.utils.state import State DATE_FORMAT = "%Y-%m-%dT%H:%M:%S.%fZ" @@ -157,6 +159,7 @@ def test_dry_run( mocked_gitlab_api = mocker.patch( "reconcile.gitlab_housekeeping.GitLabApi", autospec=True ).return_value.__enter__.return_value + mocker.patch("reconcile.gitlab_housekeeping.init_state", autospec=True) gl_h.run(True, False) @@ -331,3 +334,155 @@ def test_close_item_without_enable_closing( 1, ]) mocked_logging.info.assert_not_called() + + +@pytest.fixture +def merge_request() -> Mock: + mr = create_autospec(ProjectMergeRequest) + commit = create_autospec(ProjectCommit) + commit.id = "abc" + commit.web_url = "a.b" + mr.commits.return_value = iter([commit]) + mr.iid = 1 + mr.source_project_id = 4 + return mr + + +@pytest.fixture +def gitlab_api() -> Mock: + gl = create_autospec(GitLabApi) + project = create_autospec(Project) + project.name = "b" + project.path_with_namespace = "a/b" + gl.project = project + return gl + + +@pytest.fixture +def fork_project() -> Mock: + project = create_autospec(Project) + project.commits = create_autospec(ProjectCommitManager) + commit = create_autospec(ProjectCommit) + commit.statuses = create_autospec(ProjectCommitStatusManager) + project.commits.get.return_value = commit + return project + + +@pytest.fixture +def state() -> Mock: + state = create_autospec(State) + return state + + +class StatueMock: + def __init__(self, name, status): + self.name = name + self.status = status + + +def test_verify_ondemend_tests_running( + merge_request: Mock, + gitlab_api: Mock, + state: Mock, +) -> None: + must_pass = ["pr-check", "e2e"] + state.get.return_value = {"pr-check": "success"} + gitlab_api.get_merge_request_pipelines.return_value = [{"status": "running"}] + + assert not gl_h.verify_ondemend_tests( + False, merge_request, must_pass, gitlab_api, {}, {}, state + ) + state.add.assert_not_called() + + +def test_verify_ondemend_tests_state_fail( + mocker: MockerFixture, + merge_request: Mock, + gitlab_api: Mock, + state: Mock, + fork_project: Mock, +) -> None: + must_pass = ["pr-check", "e2e"] + state.get.return_value = ["e2e"] + mocked_gl_fork = mocker.patch( + "reconcile.gitlab_housekeeping.GitLabApi", autospec=True + ) + mocked_gl_fork.return_value.project = fork_project + fork_project.commits.get.return_value.statuses.list.return_value = [ + StatueMock("pr-check", "success") + ] + + assert not gl_h.verify_ondemend_tests( + False, merge_request, must_pass, gitlab_api, {}, {}, state + ) + state.add.assert_not_called() + + +def test_verify_ondemend_tests_state_pass( + mocker: MockerFixture, + merge_request: Mock, + gitlab_api: Mock, + state: Mock, + fork_project: Mock, +) -> None: + must_pass = ["pr-check", "e2e"] + state.get.return_value = [] + mocked_gl_fork = mocker.patch( + "reconcile.gitlab_housekeeping.GitLabApi", autospec=True + ) + mocked_gl_fork.return_value.project = fork_project + fork_project.commits.get.return_value.statuses.list.return_value = [ + StatueMock("pr-check", "success"), + StatueMock("e2e", "success"), + ] + + assert gl_h.verify_ondemend_tests( + False, merge_request, must_pass, gitlab_api, {}, {}, state + ) + state.add.assert_not_called() + + +def test_verify_ondemend_tests_fail( + mocker: MockerFixture, + merge_request: Mock, + gitlab_api: Mock, + state: Mock, + fork_project: Mock, +) -> None: + must_pass = ["pr-check", "e2e"] + state.get.return_value = None + mocked_gl_fork = mocker.patch( + "reconcile.gitlab_housekeeping.GitLabApi", autospec=True + ) + mocked_gl_fork.return_value.project = fork_project + fork_project.commits.get.return_value.statuses.list.return_value = [ + StatueMock("pr-check", "success") + ] + + assert not gl_h.verify_ondemend_tests( + False, merge_request, must_pass, gitlab_api, {}, {}, state + ) + state.add.assert_called_once_with("a/b/1/abc", {"pr-check": "success"}, force=True) + + +def test_verify_ondemend_tests_pass( + mocker: MockerFixture, + merge_request: Mock, + gitlab_api: Mock, + state: Mock, + fork_project: Mock, +) -> None: + must_pass = ["pr-check", "e2e"] + state.get.return_value = ["e2e"] + mocked_gl_fork = mocker.patch( + "reconcile.gitlab_housekeeping.GitLabApi", autospec=True + ) + mocked_gl_fork.return_value.project = fork_project + fork_project.commits.get.return_value.statuses.list.return_value = [ + StatueMock("pr-check", "success"), + StatueMock("e2e", "success"), + ] + + assert gl_h.verify_ondemend_tests( + False, merge_request, must_pass, gitlab_api, {}, {}, state + )