From d29fe3e968de0421d4353805803cea042023f36c Mon Sep 17 00:00:00 2001 From: tomasmatus Date: Wed, 4 Jun 2025 11:00:08 +0200 Subject: [PATCH 1/6] github.py: add methods to approve PR and get author info --- task/github.py | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/task/github.py b/task/github.py index 4236c5a3c0..775750f7ec 100644 --- a/task/github.py +++ b/task/github.py @@ -413,10 +413,29 @@ def issue_comments(self, number: int) -> Sequence[JsonObject]: count = len(comments) return result + def get_pr_info(self, pr: int) -> JsonObject: + return self.get_obj(f"pulls/{pr}", {}) + def get_head(self, pr: int) -> str | None: - pull = self.get_obj(f"pulls/{pr}", {}) + pull = self.get_pr_info(pr) return get_str(get_dict(pull, "head", {}), "sha", None) + def get_author(self, pr: int) -> JsonObject: + pull = self.get_pr_info(pr) + return get_dict(pull, "user", {}) + + def approve_pr(self, pr: int, sha: str) -> None: + # https://docs.github.com/en/rest/pulls/reviews?apiVersion=2022-11-28#create-a-review-for-a-pull-request + + data = { + 'commit_id': sha, + 'event': 'APPROVE', + 'comments': 'So cool' + } + self.post(f'pulls/{pr}/reviews', data) + + # let's not write the merge code yet :) + class Checklist: # NB: GitHub sends `body: null` for issues with empty bodies From 9ad919ed94caff8c3feae798b6be7a02395e9bc1 Mon Sep 17 00:00:00 2001 From: tomasmatus Date: Wed, 4 Jun 2025 11:00:41 +0200 Subject: [PATCH 2/6] add initial auto-merge capability --- lib/bots_automerge.py | 79 +++++++++++++++++++++++++++++++++++++++++++ run-queue | 34 ++++++++++++++----- task/github.py | 1 - 3 files changed, 104 insertions(+), 10 deletions(-) create mode 100644 lib/bots_automerge.py diff --git a/lib/bots_automerge.py b/lib/bots_automerge.py new file mode 100644 index 0000000000..d2604e5c07 --- /dev/null +++ b/lib/bots_automerge.py @@ -0,0 +1,79 @@ +# This file is part of Cockpit. +# +# Copyright (C) 2025 Red Hat, Inc. +# +# Cockpit is free software; you can redistribute it and/or modify it +# under the terms of the GNU Lesser General Public License as published by +# the Free Software Foundation; either version 2.1 of the License, or +# (at your option) any later version. +# +# Cockpit is distributed in the hope that it will be useful, but +# WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +# Lesser General Public License for more details. +# +# You should have received a copy of the GNU Lesser General Public License +# along with Cockpit; If not, see . + +import logging + +from lib.aio.jsonutil import get_int, get_str +from task import github + +logger = logging.getLogger(__name__) + +# TODO: verify if this is always the same +GITHUB_CI = { + 'login': 'github-actions[bot]', + 'id': 41898282, +} + +COCKPITUOUS = { + 'login': 'cockpituous', + 'id': 14330603, +} + + +def is_ci_bot(api: github.GitHub, pr: int) -> bool: + author = api.get_author(pr) + login = get_str(author, 'login') + login_id = get_int(author, 'id') + + return ((login == GITHUB_CI['login'] and login_id == GITHUB_CI['id']) or + (login == COCKPITUOUS['login'] and login_id == COCKPITUOUS['id'])) + + +def all_checks_pass(api: github.GitHub, commit_hash: str) -> bool: + statuses = api.statuses(commit_hash) + + logger.info("Checking statuses:") + if len(statuses) == 0: + logger.info("No statuses found for commit %s", commit_hash) + return False + + for context in statuses: + status = statuses[context] + status_state = get_str(status, 'state') + logger.info("Status for context '%s': %s", context, status_state) + if status_state != 'success': + return False + + return True + + +def auto_merge_bots_pr(repo: str, pr: int, sha: str) -> None: + api = github.GitHub(repo=repo) + + # Make sure that the PR was made by cockpituous or github actions + # if not is_ci_bot(api, pr_num): + # logger.info("PR not made by CI bot, skipping automerge") + # return + + # check that all checks are green + if not all_checks_pass(api, sha): + logger.info("Not every check has passed, skipping automerge") + return + + logger.info("All checks green, can automerge") + # merge the PR + api.approve_pr(pr, sha) diff --git a/run-queue b/run-queue index 729aeb6d43..4dfd8cc52f 100755 --- a/run-queue +++ b/run-queue @@ -31,6 +31,8 @@ from collections.abc import Sequence import pika +from lib.aio.jsonutil import JsonObject, get_int, get_str +from lib.bots_automerge import auto_merge_bots_pr from lib.directories import get_images_data_dir from lib.network import redhat_network from lib.stores import LOG_STORE @@ -43,7 +45,9 @@ statistics_queue = os.environ.get("RUN_STATISTICS_QUEUE") # as per pika docs DeliveryTag = int -ConsumeResult = tuple[Sequence[str] | str | None, DeliveryTag | None] +JobSubject = JsonObject + +ConsumeResult = tuple[Sequence[str] | str | None, DeliveryTag | None, JobSubject | None] # Returns a command argv to execute and the delivery tag needed to ack the message @@ -52,7 +56,7 @@ def consume_webhook_queue(dq: distributed_queue.DistributedQueue) -> ConsumeResu # call tests-scan or issue-scan appropriately method_frame, _header_frame, message = dq.channel.basic_get(queue='webhook') if not method_frame or not message: - return None, None + return None, None, None body = json.loads(message) event = body['event'] @@ -97,9 +101,9 @@ def consume_webhook_queue(dq: distributed_queue.DistributedQueue) -> ConsumeResu cmd = ['./issue-scan', '--issues-data', json.dumps(request), '--amqp', dq.address] else: logging.error('Unkown event type in the webhook queue') - return None, None + return None, None, None - return cmd, method_frame.delivery_tag + return cmd, method_frame.delivery_tag, None # Returns a command to execute and the delivery tag needed to ack the message @@ -119,18 +123,20 @@ def consume_task_queue(dq: distributed_queue.DistributedQueue) -> ConsumeResult: queue = ['public', 'rhel'][random.randrange(2)] else: # nothing to do - return None, None + return None, None, None method_frame, _header_frame, message = dq.channel.basic_get(queue=queue) if not method_frame or not message: - return None, None + return None, None, None body = json.loads(message) if job := body.get('job'): command = ['./job-runner', 'json', json.dumps(job)] + job_subject = job.get('subject') else: command = body['command'] - return command, method_frame.delivery_tag + job_subject = None + return command, method_frame.delivery_tag, job_subject def mail_notification(body: str) -> None: @@ -159,14 +165,14 @@ def main() -> int: opts = parser.parse_args() with distributed_queue.DistributedQueue(opts.amqp, ['webhook', 'rhel', 'public', 'statistics']) as dq: - cmd, delivery_tag = consume_webhook_queue(dq) + cmd, delivery_tag, job_subj = consume_webhook_queue(dq) if not cmd and delivery_tag: logging.info("Webhook message interpretation generated no command") dq.channel.basic_ack(delivery_tag) return 0 if not cmd: - cmd, delivery_tag = consume_task_queue(dq) + cmd, delivery_tag, job_subj = consume_task_queue(dq) if not cmd: logging.info("All queues are empty") return 1 @@ -191,6 +197,16 @@ failed with exit code %i. Please check the container logs for details.""" % (cmd if delivery_tag is not None: dq.channel.basic_ack(delivery_tag) + if job_subj is not None: + repo = get_str(job_subj, 'repo') + pull = get_int(job_subj, 'pull') + sha = get_str(job_subj, 'sha') + # skip automerge if jobs don't run against a PR + if repo is not None and pull is not None and sha is not None: + auto_merge_bots_pr(repo, pull, sha) + else: + logging.info("Skipping automerge for job: %s", job_subj) + return 0 diff --git a/task/github.py b/task/github.py index 775750f7ec..b558159ac5 100644 --- a/task/github.py +++ b/task/github.py @@ -426,7 +426,6 @@ def get_author(self, pr: int) -> JsonObject: def approve_pr(self, pr: int, sha: str) -> None: # https://docs.github.com/en/rest/pulls/reviews?apiVersion=2022-11-28#create-a-review-for-a-pull-request - data = { 'commit_id': sha, 'event': 'APPROVE', From cd0681713286caf0051f3b19a882b5a6158de7a4 Mon Sep 17 00:00:00 2001 From: tomasmatus Date: Tue, 1 Jul 2025 11:21:43 +0200 Subject: [PATCH 3/6] debug --- run-queue | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/run-queue b/run-queue index 4dfd8cc52f..79dc960bda 100755 --- a/run-queue +++ b/run-queue @@ -131,9 +131,11 @@ def consume_task_queue(dq: distributed_queue.DistributedQueue) -> ConsumeResult: body = json.loads(message) if job := body.get('job'): + print(f"\njob: {job}") command = ['./job-runner', 'json', json.dumps(job)] job_subject = job.get('subject') else: + print("\nNO JOB :(") command = body['command'] job_subject = None return command, method_frame.delivery_tag, job_subject @@ -197,10 +199,12 @@ failed with exit code %i. Please check the container logs for details.""" % (cmd if delivery_tag is not None: dq.channel.basic_ack(delivery_tag) + print(f"\n\n\njob_sub: {job_subj}") if job_subj is not None: repo = get_str(job_subj, 'repo') pull = get_int(job_subj, 'pull') sha = get_str(job_subj, 'sha') + print(f"\nrepo: {repo},\npull: {pull},\nsha {sha}") # skip automerge if jobs don't run against a PR if repo is not None and pull is not None and sha is not None: auto_merge_bots_pr(repo, pull, sha) From 7847b7bab3e8ee46d5ac8ac62647a518be93a6fa Mon Sep 17 00:00:00 2001 From: tomasmatus Date: Tue, 1 Jul 2025 12:50:53 +0200 Subject: [PATCH 4/6] get PR info from run-queue structure run-queue uses different structure than job-runner, this fixes the code --- run-queue | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/run-queue b/run-queue index 79dc960bda..0451131bb4 100755 --- a/run-queue +++ b/run-queue @@ -31,6 +31,7 @@ from collections.abc import Sequence import pika +from lib.aio.base import SubjectSpecification from lib.aio.jsonutil import JsonObject, get_int, get_str from lib.bots_automerge import auto_merge_bots_pr from lib.directories import get_images_data_dir @@ -45,9 +46,7 @@ statistics_queue = os.environ.get("RUN_STATISTICS_QUEUE") # as per pika docs DeliveryTag = int -JobSubject = JsonObject - -ConsumeResult = tuple[Sequence[str] | str | None, DeliveryTag | None, JobSubject | None] +ConsumeResult = tuple[Sequence[str] | str | None, DeliveryTag | None, SubjectSpecification | None] # Returns a command argv to execute and the delivery tag needed to ack the message @@ -133,7 +132,7 @@ def consume_task_queue(dq: distributed_queue.DistributedQueue) -> ConsumeResult: if job := body.get('job'): print(f"\njob: {job}") command = ['./job-runner', 'json', json.dumps(job)] - job_subject = job.get('subject') + job_subject = SubjectSpecification(job) else: print("\nNO JOB :(") command = body['command'] @@ -201,13 +200,9 @@ failed with exit code %i. Please check the container logs for details.""" % (cmd print(f"\n\n\njob_sub: {job_subj}") if job_subj is not None: - repo = get_str(job_subj, 'repo') - pull = get_int(job_subj, 'pull') - sha = get_str(job_subj, 'sha') - print(f"\nrepo: {repo},\npull: {pull},\nsha {sha}") # skip automerge if jobs don't run against a PR - if repo is not None and pull is not None and sha is not None: - auto_merge_bots_pr(repo, pull, sha) + if job_subj.repo is not None and job_subj.pull is not None and job_subj.sha is not None: + auto_merge_bots_pr(job_subj.repo, job_subj.pull, job_subj.sha) else: logging.info("Skipping automerge for job: %s", job_subj) From 9604f3918dcd6861d95b9ef86cea46e044c6cb29 Mon Sep 17 00:00:00 2001 From: tomasmatus Date: Tue, 1 Jul 2025 12:58:46 +0200 Subject: [PATCH 5/6] more debug --- lib/bots_automerge.py | 5 ++++- run-queue | 3 ++- task/github.py | 3 ++- 3 files changed, 8 insertions(+), 3 deletions(-) diff --git a/lib/bots_automerge.py b/lib/bots_automerge.py index d2604e5c07..afedc93452 100644 --- a/lib/bots_automerge.py +++ b/lib/bots_automerge.py @@ -64,16 +64,19 @@ def all_checks_pass(api: github.GitHub, commit_hash: str) -> bool: def auto_merge_bots_pr(repo: str, pr: int, sha: str) -> None: api = github.GitHub(repo=repo) + print(f"is_cu_bot: {is_ci_bot(api, pr)}") # Make sure that the PR was made by cockpituous or github actions - # if not is_ci_bot(api, pr_num): + # if not is_ci_bot(api, pr): # logger.info("PR not made by CI bot, skipping automerge") # return # check that all checks are green + print(f"all_checks_pass: {all_checks_pass(api, sha)}") if not all_checks_pass(api, sha): logger.info("Not every check has passed, skipping automerge") return logger.info("All checks green, can automerge") + print("All checks green, can automerge") # merge the PR api.approve_pr(pr, sha) diff --git a/run-queue b/run-queue index 0451131bb4..41e43631ee 100755 --- a/run-queue +++ b/run-queue @@ -198,10 +198,11 @@ failed with exit code %i. Please check the container logs for details.""" % (cmd if delivery_tag is not None: dq.channel.basic_ack(delivery_tag) - print(f"\n\n\njob_sub: {job_subj}") if job_subj is not None: + print(f"\n\n\nrepo: {job_subj.repo}\n, pull: {job_subj.pull}\n, sha: {job_subj.sha}\n") # skip automerge if jobs don't run against a PR if job_subj.repo is not None and job_subj.pull is not None and job_subj.sha is not None: + print("starting automerge") auto_merge_bots_pr(job_subj.repo, job_subj.pull, job_subj.sha) else: logging.info("Skipping automerge for job: %s", job_subj) diff --git a/task/github.py b/task/github.py index b558159ac5..e1ffa35bc1 100644 --- a/task/github.py +++ b/task/github.py @@ -431,7 +431,8 @@ def approve_pr(self, pr: int, sha: str) -> None: 'event': 'APPROVE', 'comments': 'So cool' } - self.post(f'pulls/{pr}/reviews', data) + rw = self.post(f'pulls/{pr}/reviews', data) + print(f"post {rw}") # let's not write the merge code yet :) From 4e0a820f2c4bacf885361c3e32fed99200104dda Mon Sep 17 00:00:00 2001 From: tomasmatus Date: Tue, 1 Jul 2025 16:27:32 +0200 Subject: [PATCH 6/6] add mock for statuses --- lib/bots_automerge.py | 15 +++++++++------ lib/testmap.py | 1 + task/github.py | 4 ++++ 3 files changed, 14 insertions(+), 6 deletions(-) diff --git a/lib/bots_automerge.py b/lib/bots_automerge.py index afedc93452..325cf91736 100644 --- a/lib/bots_automerge.py +++ b/lib/bots_automerge.py @@ -36,6 +36,7 @@ def is_ci_bot(api: github.GitHub, pr: int) -> bool: author = api.get_author(pr) + print(author) login = get_str(author, 'login') login_id = get_int(author, 'id') @@ -45,16 +46,17 @@ def is_ci_bot(api: github.GitHub, pr: int) -> bool: def all_checks_pass(api: github.GitHub, commit_hash: str) -> bool: statuses = api.statuses(commit_hash) + print(f"STATUSES: {statuses}") - logger.info("Checking statuses:") + print("Checking statuses:") if len(statuses) == 0: - logger.info("No statuses found for commit %s", commit_hash) + print("No statuses found for commit %s", commit_hash) return False for context in statuses: status = statuses[context] status_state = get_str(status, 'state') - logger.info("Status for context '%s': %s", context, status_state) + print("Status for context '%s': %s", context, status_state) if status_state != 'success': return False @@ -71,9 +73,10 @@ def auto_merge_bots_pr(repo: str, pr: int, sha: str) -> None: # return # check that all checks are green - print(f"all_checks_pass: {all_checks_pass(api, sha)}") - if not all_checks_pass(api, sha): - logger.info("Not every check has passed, skipping automerge") + all_pass = all_checks_pass(api, sha) + print(f"all_checks_pass: {all_pass}") + if not all_pass: + print("Not every check has passed, skipping automerge") return logger.info("All checks green, can automerge") diff --git a/lib/testmap.py b/lib/testmap.py index 7dbe6dd506..972486d559 100644 --- a/lib/testmap.py +++ b/lib/testmap.py @@ -302,6 +302,7 @@ def is_valid_context(context: str, repo: str) -> bool: image = image_scenario.split('/')[0] # if the context specifies a repo, use that one instead branch_contexts = tests_for_project(context_repo or repo) + print(f"CONTEXTS: {branch_contexts}") if context_repo: # if the context specifies a repo, only look at that particular branch try: diff --git a/task/github.py b/task/github.py index e1ffa35bc1..db9e25b648 100644 --- a/task/github.py +++ b/task/github.py @@ -336,8 +336,11 @@ def statuses(self, revision: str) -> Mapping[str, JsonObject]: data = self.get_obj(f"commits/{revision}/status?page={page}&per_page={count}") count = 0 page += 1 + print(f"DATA: {data}") for status in get_dictv(data, "statuses", ()): context = get_str(status, "context") + print(f"CONTEXT: {context}") + print(f"REPO: {self.repo}") if is_valid_context(context, self.repo) and context not in result: result[context] = status count += 1 @@ -422,6 +425,7 @@ def get_head(self, pr: int) -> str | None: def get_author(self, pr: int) -> JsonObject: pull = self.get_pr_info(pr) + print(f"PR INFO: {pull}") return get_dict(pull, "user", {}) def approve_pr(self, pr: int, sha: str) -> None: