From 1e8f3b7d39f6fdfb4a5324cb83b5914bff0bcd12 Mon Sep 17 00:00:00 2001 From: Maja Massarini Date: Thu, 1 Dec 2022 13:42:56 +0100 Subject: [PATCH 01/10] Retrigger koji and bodhi services by issue comment --- packit_service/worker/checker/bodhi.py | 109 ++++++++++++++++------ packit_service/worker/checker/distgit.py | 29 +++++- packit_service/worker/handlers/bodhi.py | 10 +- packit_service/worker/handlers/distgit.py | 7 +- packit_service/worker/jobs.py | 13 +++ tests/unit/test_bodhi_update_checks.py | 7 +- tests/unit/test_jobs.py | 38 ++++++++ 7 files changed, 174 insertions(+), 39 deletions(-) diff --git a/packit_service/worker/checker/bodhi.py b/packit_service/worker/checker/bodhi.py index 6c549433a..a938941cf 100644 --- a/packit_service/worker/checker/bodhi.py +++ b/packit_service/worker/checker/bodhi.py @@ -7,7 +7,10 @@ from packit_service.constants import KojiBuildState -from packit_service.worker.checker.abstract import ActorChecker, Checker +from packit_service.worker.checker.abstract import ( + ActorChecker, + Checker, +) from packit_service.worker.handlers.mixin import ( GetKojiBuildData, GetKojiBuildDataFromKojiBuildEventMixin, @@ -18,6 +21,12 @@ ConfigFromEventMixin, PackitAPIWithDownstreamMixin, ) +from packit_service.worker.events import ( + PullRequestCommentPagureEvent, + IssueCommentEvent, +) + +from packit_service.worker.events.koji import KojiBuildEvent logger = logging.getLogger(__name__) @@ -29,25 +38,31 @@ def pre_check(self) -> bool: By default, we use `fedora-stable` alias. (Rawhide updates are already created automatically.) """ - if self.state != KojiBuildState.complete: - logger.debug( - f"Skipping build '{self.build_id}' " - f"on '{self.dist_git_branch}'. " - f"Build not finished yet." - ) - return False - if self.dist_git_branch not in ( - configured_branches := get_branches( - *(self.job_config.dist_git_branches or {"fedora-stable"}), - default_dg_branch="rawhide", # Koji calls it rawhide, not main - ) + if self.data.event_type in ( + PullRequestCommentPagureEvent.__name__, + KojiBuildEvent.__name__, ): - logger.info( - f"Skipping build on '{self.dist_git_branch}'. " - f"Bodhi update configured only for '{configured_branches}'." - ) - return False + if self.state != KojiBuildState.complete: + logger.debug( + f"Skipping build '{self.build_id}' " + f"on '{self.dist_git_branch}'. " + f"Build not finished yet." + ) + return False + + if self.dist_git_branch not in ( + configured_branches := get_branches( + *(self.job_config.dist_git_branches or {"fedora-stable"}), + default_dg_branch="rawhide", # Koji calls it rawhide, not main + ) + ): + logger.info( + f"Skipping build on '{self.dist_git_branch}'. " + f"Bodhi update configured only for '{configured_branches}'." + ) + return False + return True @@ -65,14 +80,45 @@ class IsKojiBuildCompleteAndBranchConfiguredCheckService( ... -class HasAuthorWriteAccess(ActorChecker, ConfigFromEventMixin): +class HasIssueCommenterRetriggeringPermissions(ActorChecker, ConfigFromEventMixin): + """To be able to retrigger a Bodhi update the issue commenter should + have write permission on the project. + """ + def _pre_check(self) -> bool: - if not self.project.has_write_access(user=self.actor): - logger.info( - f"Re-triggering Bodhi update via dist-git comment in PR#{self.data.pr_id}" - f" and project {self.project.repo} is not allowed for the user: {self.actor}." + has_write_access = self.project.has_write_access(user=self.actor) + if self.data.event_type in (IssueCommentEvent.__name__,): + logger.debug( + f"Re-triggering Bodhi update through comment in " + f"repo {self.project.repo} and issue {self.data.issue_id} " + f"by {self.actor}." ) - return False + if not has_write_access: + logger.warning( + f"Re-triggering Bodhi update through comment in " + f"repo {self.project.repo} and issue {self.data.issue_id} " + f"is not allowed for the user {self.actor} " + f"which has not write permissions on the project." + ) + return False + + return True + if self.data.event_type in (PullRequestCommentPagureEvent.__name__,): + logger.debug( + f"Re-triggering Bodhi update via dist-git comment in " + f"repo {self.project.repo} and #PR {self.data.pr_id} " + f"by {self.actor}." + ) + if not has_write_access: + logger.warning( + f"Re-triggering Bodhi update via dist-git comment in " + f"PR#{self.data.pr_id} and project {self.project.repo} " + f"is not allowed for the user {self.actor} " + f"which has not write permissions on the project." + ) + return False + + return True return True @@ -80,12 +126,13 @@ def _pre_check(self) -> bool: class IsAuthorAPackager(ActorChecker, PackitAPIWithDownstreamMixin): def _pre_check(self) -> bool: - if not self.is_packager(user=self.actor): - logger.info( - f"Re-triggering Bodhi update via dist-git comment in PR#{self.data.pr_id}" - f" and project {self.project.repo} is not allowed, user {self.actor} " - "is not a packager." - ) - return False + if self.data.event_type in (PullRequestCommentPagureEvent.__name__,): + if not self.is_packager(user=self.actor): + logger.info( + f"Re-triggering Bodhi update via dist-git comment in PR#{self.data.pr_id}" + f" and project {self.project.repo} is not allowed, user {self.actor} " + "is not a packager." + ) + return False return True diff --git a/packit_service/worker/checker/distgit.py b/packit_service/worker/checker/distgit.py index 5a37f6f90..434b203eb 100644 --- a/packit_service/worker/checker/distgit.py +++ b/packit_service/worker/checker/distgit.py @@ -5,9 +5,10 @@ from packit.config.aliases import get_branches -from packit_service.worker.checker.abstract import Checker +from packit_service.worker.checker.abstract import Checker, ActorChecker from packit_service.worker.events import ( PushPagureEvent, + IssueCommentEvent, ) from packit_service.worker.events.pagure import PullRequestCommentPagureEvent from packit_service.worker.handlers.mixin import GetProjectToSyncMixin @@ -70,6 +71,32 @@ def pre_check(self) -> bool: return True +class HasIssueCommenterRetriggeringPermissions(ActorChecker): + """To be able to retrigger a koji-build the issue commenter should + have write permission on the project. + """ + + def _pre_check(self) -> bool: + if self.data.event_type in (IssueCommentEvent.__name__,): + logger.debug( + f"Re-triggering downstream koji-build through comment in " + f"repo {self.project.repo} and issue {self.data.issue_id} " + f"by {self.actor}." + ) + if not self.project.has_write_access(user=self.actor): + logger.warning( + f"Re-triggering downstream koji-build through comment in " + f"repo {self.project.repo} and issue {self.data.issue_id} " + f"done by {self.actor} which has not write permissions " + "on the project." + ) + return False + + return True + + return True + + class IsProjectOk(Checker, GetProjectToSyncMixin): def pre_check(self) -> bool: return self.project_to_sync is not None diff --git a/packit_service/worker/handlers/bodhi.py b/packit_service/worker/handlers/bodhi.py index a256eef82..4676075ff 100644 --- a/packit_service/worker/handlers/bodhi.py +++ b/packit_service/worker/handlers/bodhi.py @@ -18,12 +18,15 @@ ) from packit_service.worker.checker.abstract import Checker from packit_service.worker.checker.bodhi import ( - HasAuthorWriteAccess, IsAuthorAPackager, + HasIssueCommenterRetriggeringPermissions, IsKojiBuildCompleteAndBranchConfiguredCheckEvent, IsKojiBuildCompleteAndBranchConfiguredCheckService, ) -from packit_service.worker.events import PullRequestCommentPagureEvent +from packit_service.worker.events import ( + PullRequestCommentPagureEvent, + IssueCommentEvent, +) from packit_service.worker.events.koji import KojiBuildEvent from packit_service.worker.handlers.abstract import ( TaskName, @@ -167,6 +170,7 @@ def get_checkers() -> Tuple[Type[Checker], ...]: @configured_as(job_type=JobType.bodhi_update) @reacts_to(event=PullRequestCommentPagureEvent) +@reacts_to(event=IssueCommentEvent) @run_for_comment(command="create-update") class RetriggerBodhiUpdateHandler( BodhiUpdateHandler, GetKojiBuildDataFromKojiServiceMixin @@ -184,7 +188,7 @@ def get_checkers() -> Tuple[Type[Checker], ...]: """ logger.debug("Bodhi update will be re-triggered via dist-git PR comment.") return ( - HasAuthorWriteAccess, IsAuthorAPackager, + HasIssueCommenterRetriggeringPermissions, IsKojiBuildCompleteAndBranchConfiguredCheckService, ) diff --git a/packit_service/worker/handlers/distgit.py b/packit_service/worker/handlers/distgit.py index ac2c46418..cadc7f56a 100644 --- a/packit_service/worker/handlers/distgit.py +++ b/packit_service/worker/handlers/distgit.py @@ -35,7 +35,8 @@ IsProjectOk, PermissionOnDistgit, ValidInformationForPullFromUpstream, -) + HasIssueCommenterRetriggeringPermissions, + ) from packit_service.worker.events import ( PushPagureEvent, ReleaseEvent, @@ -43,6 +44,7 @@ AbstractIssueCommentEvent, CheckRerunReleaseEvent, PullRequestCommentPagureEvent, + IssueCommentEvent, ) from packit_service.worker.events.new_hotness import NewHotnessUpdateEvent from packit_service.worker.handlers.abstract import ( @@ -443,6 +445,7 @@ def _report_errors_for_each_branch(self, message: str) -> None: @configured_as(job_type=JobType.koji_build) @run_for_comment(command="koji-build") @reacts_to(event=PushPagureEvent) +@reacts_to(event=IssueCommentEvent) @reacts_to(event=PullRequestCommentPagureEvent) class DownstreamKojiBuildHandler( RetriableJobHandler, @@ -476,7 +479,7 @@ def __init__( @staticmethod def get_checkers() -> Tuple[Type[Checker], ...]: - return (PermissionOnDistgit,) + return (PermissionOnDistgit, HasIssueCommenterRetriggeringPermissions) def run(self) -> TaskResults: branch = ( diff --git a/packit_service/worker/jobs.py b/packit_service/worker/jobs.py index 5040eb029..a8af75fbb 100644 --- a/packit_service/worker/jobs.py +++ b/packit_service/worker/jobs.py @@ -489,6 +489,19 @@ def check_explicit_matching(self) -> List[JobConfig]: # A koji_build job with comment trigger # can be re-triggered by a Pagure comment in a PR matching_jobs.append(job) + elif isinstance(self.event, IssueCommentEvent): + for job in self.event.package_config.jobs: + if ( + job.type in (JobType.koji_build, JobType.bodhi_update) + and job.trigger == JobConfigTriggerType.commit + and self.event.job_config_trigger_type + == JobConfigTriggerType.release + ): + # A koji_build/bodhi_update can be re-triggered by a + # GitHub comment in an issue + # after a failed release event + # (which has created the issue) + matching_jobs.append(job) return matching_jobs diff --git a/tests/unit/test_bodhi_update_checks.py b/tests/unit/test_bodhi_update_checks.py index 981b8004e..4efabe63a 100644 --- a/tests/unit/test_bodhi_update_checks.py +++ b/tests/unit/test_bodhi_update_checks.py @@ -2,7 +2,10 @@ from flexmock import flexmock -from packit_service.worker.checker.bodhi import HasAuthorWriteAccess, IsAuthorAPackager +from packit_service.worker.checker.bodhi import ( + HasIssueCommenterRetriggeringPermissions, + IsAuthorAPackager, +) from packit_service.worker.mixin import PackitAPIWithDownstreamMixin from packit.config import ( JobConfig, @@ -73,7 +76,7 @@ def test_check_has_author_write_access( repo="playground-for-pencils", ) - checker = HasAuthorWriteAccess(package_config, job_config, data) + checker = HasIssueCommenterRetriggeringPermissions(package_config, job_config, data) checker._project = project assert checker.pre_check() == result diff --git a/tests/unit/test_jobs.py b/tests/unit/test_jobs.py index c2aad6dfb..5c2d8ecda 100644 --- a/tests/unit/test_jobs.py +++ b/tests/unit/test_jobs.py @@ -2985,6 +2985,44 @@ def __init__(self): ], {}, ), + pytest.param( + IssueCommentEvent, + JobConfigTriggerType.release, + [ + JobConfig( + type=JobType.koji_build, + trigger=JobConfigTriggerType.commit, + packages={"package": CommonPackageConfig()}, + ), + ], + [ + JobConfig( + type=JobType.koji_build, + trigger=JobConfigTriggerType.commit, + packages={"package": CommonPackageConfig()}, + ), + ], + {"issue_id": 1}, + ), + pytest.param( + IssueCommentEvent, + JobConfigTriggerType.release, + [ + JobConfig( + type=JobType.bodhi_update, + trigger=JobConfigTriggerType.commit, + packages={"package": CommonPackageConfig()}, + ), + ], + [ + JobConfig( + type=JobType.bodhi_update, + trigger=JobConfigTriggerType.commit, + packages={"package": CommonPackageConfig()}, + ), + ], + {"issue_id": 1}, + ), ], ) def test_get_jobs_matching_trigger( From 170429a91e069ecdc045e0e35456f5b994b32da8 Mon Sep 17 00:00:00 2001 From: Maja Massarini Date: Mon, 12 Dec 2022 09:29:13 +0100 Subject: [PATCH 02/10] Pre-commit style fix --- packit_service/worker/handlers/distgit.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packit_service/worker/handlers/distgit.py b/packit_service/worker/handlers/distgit.py index cadc7f56a..0b2d9f1a1 100644 --- a/packit_service/worker/handlers/distgit.py +++ b/packit_service/worker/handlers/distgit.py @@ -36,7 +36,7 @@ PermissionOnDistgit, ValidInformationForPullFromUpstream, HasIssueCommenterRetriggeringPermissions, - ) +) from packit_service.worker.events import ( PushPagureEvent, ReleaseEvent, From 227ee51741015ad9752bdd41009cc3beda28c06c Mon Sep 17 00:00:00 2001 From: Maja Massarini Date: Mon, 12 Dec 2022 15:07:21 +0100 Subject: [PATCH 03/10] Give user feedback for failed Bodhi checks --- packit_service/config.py | 6 +-- packit_service/constants.py | 4 ++ packit_service/worker/checker/bodhi.py | 61 ++++++++++++++++++----- packit_service/worker/checker/distgit.py | 43 +++++++++++++--- packit_service/worker/handlers/bodhi.py | 2 + packit_service/worker/handlers/distgit.py | 2 + packit_service/worker/jobs.py | 9 ++-- tests/unit/test_distgit.py | 11 +++- 8 files changed, 111 insertions(+), 27 deletions(-) diff --git a/packit_service/config.py b/packit_service/config.py index 7cc13f936..08480234c 100644 --- a/packit_service/config.py +++ b/packit_service/config.py @@ -279,17 +279,17 @@ def create_issue_if_needed( ) -> Optional[Issue]: # TODO: Improve filtering issues = project.get_issue_list() - title = f"[packit] {title}" + packit_title = f"[packit] {title}" for issue in issues: - if title in issue.title: + if packit_title in issue.title or title in issue.title: if comment_to_existing: issue.comment(body=comment_to_existing) logger.debug(f"Issue #{issue.id} updated: {issue.url}") return None # TODO: store in DB - issue = project.create_issue(title=title, body=message) + issue = project.create_issue(title=packit_title, body=message) logger.debug(f"Issue #{issue.id} created: {issue.url}") return issue diff --git a/packit_service/constants.py b/packit_service/constants.py index 7dd0341a4..c103d93b0 100644 --- a/packit_service/constants.py +++ b/packit_service/constants.py @@ -26,6 +26,10 @@ TESTING_FARM_INSTALLABILITY_TEST_URL = "https://gitlab.com/testing-farm/tests" TESTING_FARM_INSTALLABILITY_TEST_REF = "main" +MSG_GET_IN_TOUCH = ( + f"\n\n---\n\n*Get in [touch with us]({CONTACTS_URL}) if you need some help.*" +) + MSG_RETRIGGER = ( "You can retrigger the {job} by adding a comment (`{packit_comment_command_prefix} {command}`) " "into this {place}." diff --git a/packit_service/worker/checker/bodhi.py b/packit_service/worker/checker/bodhi.py index a938941cf..727c60858 100644 --- a/packit_service/worker/checker/bodhi.py +++ b/packit_service/worker/checker/bodhi.py @@ -5,7 +5,7 @@ from packit.config.aliases import get_branches -from packit_service.constants import KojiBuildState +from packit_service.constants import KojiBuildState, MSG_GET_IN_TOUCH from packit_service.worker.checker.abstract import ( ActorChecker, @@ -24,9 +24,11 @@ from packit_service.worker.events import ( PullRequestCommentPagureEvent, IssueCommentEvent, + IssueCommentGitlabEvent, ) from packit_service.worker.events.koji import KojiBuildEvent +from packit_service.worker.reporting import report_in_issue_repository logger = logging.getLogger(__name__) @@ -87,35 +89,57 @@ class HasIssueCommenterRetriggeringPermissions(ActorChecker, ConfigFromEventMixi def _pre_check(self) -> bool: has_write_access = self.project.has_write_access(user=self.actor) - if self.data.event_type in (IssueCommentEvent.__name__,): + if self.data.event_type in ( + IssueCommentEvent.__name__, + IssueCommentGitlabEvent.__name__, + ): logger.debug( f"Re-triggering Bodhi update through comment in " - f"repo {self.project.repo} and issue {self.data.issue_id} " + f"repo {self.project_url} and issue {self.data.issue_id} " f"by {self.actor}." ) if not has_write_access: - logger.warning( + msg = ( f"Re-triggering Bodhi update through comment in " - f"repo {self.project.repo} and issue {self.data.issue_id} " - f"is not allowed for the user {self.actor} " + f"repo **{self.project_url}** and issue **{self.data.issue_id}** " + f"is not allowed for the user *{self.actor}* " f"which has not write permissions on the project." ) + logger.info(msg) + issue = self.project.get_issue(self.data.issue_id) + report_in_issue_repository( + issue_repository=self.job_config.issue_repository, + service_config=self.service_config, + title=issue.title, + message=msg + MSG_GET_IN_TOUCH, + comment_to_existing=msg, + ) return False return True if self.data.event_type in (PullRequestCommentPagureEvent.__name__,): + logger.debug( f"Re-triggering Bodhi update via dist-git comment in " - f"repo {self.project.repo} and #PR {self.data.pr_id} " + f"repo {self.project_url} and #PR {self.data.pr_id} " f"by {self.actor}." ) if not has_write_access: - logger.warning( + msg = ( f"Re-triggering Bodhi update via dist-git comment in " - f"PR#{self.data.pr_id} and project {self.project.repo} " - f"is not allowed for the user {self.actor} " + f"**PR#{self.data.pr_id}** and project **{self.project.repo}** " + f"is not allowed for the user *{self.actor}* " f"which has not write permissions on the project." ) + logger.info(msg) + title = "Re-triggering Bodhi update through comment in issue failed" + report_in_issue_repository( + issue_repository=self.job_config.issue_repository, + service_config=self.service_config, + title=title, + message=msg + MSG_GET_IN_TOUCH, + comment_to_existing=msg, + ) return False return True @@ -128,11 +152,22 @@ def _pre_check(self) -> bool: if self.data.event_type in (PullRequestCommentPagureEvent.__name__,): if not self.is_packager(user=self.actor): - logger.info( - f"Re-triggering Bodhi update via dist-git comment in PR#{self.data.pr_id}" - f" and project {self.project.repo} is not allowed, user {self.actor} " + title = ( + "Re-triggering Bodhi update through dist-git comment in PR failed" + ) + msg = ( + f"Re-triggering Bodhi update via dist-git comment in **PR#{self.data.pr_id}**" + f" and project **{self.project_url}** is not allowed, user *{self.actor}* " "is not a packager." ) + logger.info(msg) + report_in_issue_repository( + issue_repository=self.job_config.issue_repository, + service_config=self.service_config, + title=title, + message=msg + MSG_GET_IN_TOUCH, + comment_to_existing=msg, + ) return False return True diff --git a/packit_service/worker/checker/distgit.py b/packit_service/worker/checker/distgit.py index 434b203eb..ed8df4064 100644 --- a/packit_service/worker/checker/distgit.py +++ b/packit_service/worker/checker/distgit.py @@ -5,10 +5,13 @@ from packit.config.aliases import get_branches +from packit_service.constants import MSG_GET_IN_TOUCH + from packit_service.worker.checker.abstract import Checker, ActorChecker from packit_service.worker.events import ( PushPagureEvent, IssueCommentEvent, + IssueCommentGitlabEvent, ) from packit_service.worker.events.pagure import PullRequestCommentPagureEvent from packit_service.worker.handlers.mixin import GetProjectToSyncMixin @@ -62,10 +65,23 @@ def pre_check(self) -> bool: f"Triggering downstream koji build through comment by: {commenter}" ) if not self.is_packager(commenter): - logger.info( - f"koji-build retrigger comment event on PR identifier {self.data.pr_id} " + msg = ( + f"koji-build retriggering through comment " + f"on PR identifier {self.data.pr_id} " + f"and project {self.data.project_url} " f"done by {commenter} which is not a packager." ) + logger.info(msg) + report_in_issue_repository( + issue_repository=self.job_config.issue_repository, + service_config=self.service_config, + title=( + "Re-triggering downstream koji build " + "through comment in dist-git PR failed" + ), + message=msg + MSG_GET_IN_TOUCH, + comment_to_existing=msg, + ) return False return True @@ -77,19 +93,32 @@ class HasIssueCommenterRetriggeringPermissions(ActorChecker): """ def _pre_check(self) -> bool: - if self.data.event_type in (IssueCommentEvent.__name__,): + if self.data.event_type in ( + IssueCommentEvent.__name__, + IssueCommentGitlabEvent.__name__, + ): logger.debug( f"Re-triggering downstream koji-build through comment in " f"repo {self.project.repo} and issue {self.data.issue_id} " f"by {self.actor}." ) if not self.project.has_write_access(user=self.actor): - logger.warning( + msg = ( f"Re-triggering downstream koji-build through comment in " - f"repo {self.project.repo} and issue {self.data.issue_id} " - f"done by {self.actor} which has not write permissions " - "on the project." + f"repo **{self.project_url}** and issue **{self.data.issue_id}** " + f"is not allowed for the user *{self.actor}* " + f"which has not write permissions on the project." + ) + logger.info(msg) + issue = self.project.get_issue(self.data.issue_id) + report_in_issue_repository( + issue_repository=self.job_config.issue_repository, + service_config=self.service_config, + title=issue.title, + message=msg + MSG_GET_IN_TOUCH, + comment_to_existing=msg, ) + return False return True diff --git a/packit_service/worker/handlers/bodhi.py b/packit_service/worker/handlers/bodhi.py index 4676075ff..289a74101 100644 --- a/packit_service/worker/handlers/bodhi.py +++ b/packit_service/worker/handlers/bodhi.py @@ -26,6 +26,7 @@ from packit_service.worker.events import ( PullRequestCommentPagureEvent, IssueCommentEvent, + IssueCommentGitlabEvent, ) from packit_service.worker.events.koji import KojiBuildEvent from packit_service.worker.handlers.abstract import ( @@ -171,6 +172,7 @@ def get_checkers() -> Tuple[Type[Checker], ...]: @configured_as(job_type=JobType.bodhi_update) @reacts_to(event=PullRequestCommentPagureEvent) @reacts_to(event=IssueCommentEvent) +@reacts_to(event=IssueCommentGitlabEvent) @run_for_comment(command="create-update") class RetriggerBodhiUpdateHandler( BodhiUpdateHandler, GetKojiBuildDataFromKojiServiceMixin diff --git a/packit_service/worker/handlers/distgit.py b/packit_service/worker/handlers/distgit.py index 0b2d9f1a1..cc66d4764 100644 --- a/packit_service/worker/handlers/distgit.py +++ b/packit_service/worker/handlers/distgit.py @@ -45,6 +45,7 @@ CheckRerunReleaseEvent, PullRequestCommentPagureEvent, IssueCommentEvent, + IssueCommentGitlabEvent, ) from packit_service.worker.events.new_hotness import NewHotnessUpdateEvent from packit_service.worker.handlers.abstract import ( @@ -446,6 +447,7 @@ def _report_errors_for_each_branch(self, message: str) -> None: @run_for_comment(command="koji-build") @reacts_to(event=PushPagureEvent) @reacts_to(event=IssueCommentEvent) +@reacts_to(event=IssueCommentGitlabEvent) @reacts_to(event=PullRequestCommentPagureEvent) class DownstreamKojiBuildHandler( RetriableJobHandler, diff --git a/packit_service/worker/jobs.py b/packit_service/worker/jobs.py index a8af75fbb..06f2feacb 100644 --- a/packit_service/worker/jobs.py +++ b/packit_service/worker/jobs.py @@ -31,7 +31,10 @@ CheckRerunEvent, IssueCommentEvent, ) -from packit_service.worker.events.comment import AbstractCommentEvent +from packit_service.worker.events.comment import ( + AbstractCommentEvent, + AbstractIssueCommentEvent, +) from packit_service.worker.handlers import ( CoprBuildHandler, GithubAppInstallationHandler, @@ -489,7 +492,7 @@ def check_explicit_matching(self) -> List[JobConfig]: # A koji_build job with comment trigger # can be re-triggered by a Pagure comment in a PR matching_jobs.append(job) - elif isinstance(self.event, IssueCommentEvent): + elif isinstance(self.event, AbstractIssueCommentEvent): for job in self.event.package_config.jobs: if ( job.type in (JobType.koji_build, JobType.bodhi_update) @@ -498,7 +501,7 @@ def check_explicit_matching(self) -> List[JobConfig]: == JobConfigTriggerType.release ): # A koji_build/bodhi_update can be re-triggered by a - # GitHub comment in an issue + # comment in a issue in the repository issues # after a failed release event # (which has created the issue) matching_jobs.append(job) diff --git a/tests/unit/test_distgit.py b/tests/unit/test_distgit.py index fd82f1e13..6f210d77f 100644 --- a/tests/unit/test_distgit.py +++ b/tests/unit/test_distgit.py @@ -12,6 +12,7 @@ DownstreamKojiBuildHandler, ) from packit_service.worker.events.event import EventData +from packit_service.config import PackageConfigGetter def test_create_one_issue_for_pr(): @@ -84,5 +85,13 @@ def test_retrigger_downstream_koji_build_pre_check(user_groups, data, check_pass "list_user_groups" ).and_return(lambda username: user_groups) - result = DownstreamKojiBuildHandler.pre_check(None, None, data_dict) + flexmock(DownstreamKojiBuildHandler).should_receive("service_config").and_return( + flexmock() + ) + if not check_passed: + flexmock(PackageConfigGetter).should_receive("create_issue_if_needed").once() + + result = DownstreamKojiBuildHandler.pre_check( + None, flexmock(issue_repository=flexmock()), data_dict + ) assert result == check_passed From 206a25de09a267dc768e6fe3f5ab815bfd6b4d1c Mon Sep 17 00:00:00 2001 From: Maja Massarini Date: Tue, 20 Dec 2022 11:25:59 +0100 Subject: [PATCH 04/10] Add url to dist-git repo in created issue When the propose-downstream fails it creates an issue and the issue could be in any git forge. Add a url (into the issue) to the dist-git source repo. --- packit_service/worker/handlers/distgit.py | 3 ++- tests/integration/test_release_event.py | 5 ++++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/packit_service/worker/handlers/distgit.py b/packit_service/worker/handlers/distgit.py index cc66d4764..123ffae38 100644 --- a/packit_service/worker/handlers/distgit.py +++ b/packit_service/worker/handlers/distgit.py @@ -328,7 +328,8 @@ def run(self) -> TaskResults: err_without_new_lines = err.replace("\n", " ") branch_errors += f"| `{branch}` | `{err_without_new_lines}` |\n" body_msg = ( - f"Packit failed on creating pull-requests in dist-git:\n\n" + f"Packit failed on creating pull-requests in dist-git " + f"({self.packit_api.dg.local_project.git_url}):\n\n" f"| dist-git branch | error |\n" f"| --------------- | ----- |\n" f"{branch_errors}\n\n" diff --git a/tests/integration/test_release_event.py b/tests/integration/test_release_event.py index d05ff3ee6..b885f3fa1 100644 --- a/tests/integration/test_release_event.py +++ b/tests/integration/test_release_event.py @@ -488,7 +488,8 @@ def test_dist_git_push_release_handle_all_failed( .should_receive("create_issue") .with_args( title="[packit] Propose downstream failed for release 0.3.0", - body="Packit failed on creating pull-requests in dist-git:\n\n" + body="Packit failed on creating pull-requests in dist-git " + "(https://src.fedoraproject.org/rpms/hello-world.git):\n\n" "| dist-git branch | error |\n" "| --------------- | ----- |\n" f"{table_content}\n\n" @@ -502,6 +503,7 @@ def test_dist_git_push_release_handle_all_failed( project.should_receive("get_issue_list").and_return([]) lp = flexmock(LocalProject, refresh_the_arguments=lambda: None) lp.git_project = project + lp.git_url = "https://src.fedoraproject.org/rpms/hello-world.git" lp.working_dir = "" flexmock(DistGit).should_receive("local_project").and_return(lp) # reset of the upstream repo @@ -726,6 +728,7 @@ def test_dont_retry_propose_downstream_task( lp = flexmock(LocalProject, refresh_the_arguments=lambda: None) lp.git_project = project + lp.git_url = "https://src.fedoraproject.org/rpms/hello-world.git" lp.working_dir = "" flexmock(DistGit).should_receive("local_project").and_return(lp) From c0676c579bb532c3ea202f84986fc3d090e48693 Mon Sep 17 00:00:00 2001 From: Maja Massarini Date: Wed, 21 Dec 2022 09:32:17 +0100 Subject: [PATCH 05/10] Look for a packit config file in dist-git --- packit_service/worker/jobs.py | 32 +++++++++++++++++++++++--------- 1 file changed, 23 insertions(+), 9 deletions(-) diff --git a/packit_service/worker/jobs.py b/packit_service/worker/jobs.py index 06f2feacb..c5ae292a8 100644 --- a/packit_service/worker/jobs.py +++ b/packit_service/worker/jobs.py @@ -8,13 +8,14 @@ from datetime import datetime, timezone from typing import Any, Optional, Union from typing import List, Set, Type +from re import match import celery from ogr.exceptions import GithubAppNotInstalledError from packit.config import JobConfig, JobType, JobConfigTriggerType from packit.config.job_config import DEPRECATED_JOB_TYPES -from packit_service.config import ServiceConfig +from packit_service.config import PackageConfigGetter, ServiceConfig from packit_service.constants import ( DOCS_CONFIGURATION_URL, TASK_ACCEPTED, @@ -309,19 +310,32 @@ def is_packit_config_present(self) -> bool: return True + def search_config_in_distgit(self): + issue = self.event.project.get_issue(self.event.issue_id) + if m := match(r"[\w\s-]+dist-git \((\S+)\):", issue.description): + project = self.service_config.get_project(url=m[1]) + package_config = PackageConfigGetter.get_package_config_from_repo( + project=project, + fail_when_missing=False, + ) + return package_config + def process_jobs(self) -> List[TaskResults]: """ Create Celery tasks for a job handler (if trigger matches) for every job defined in config. """ if not self.is_packit_config_present(): - return [ - TaskResults.create_from( - success=True, - msg="No packit config found in the repository.", - job_config=None, - event=self.event, - ) - ] + if dist_git_package_config := self.search_config_in_distgit(): + self.event._package_config = dist_git_package_config + else: + return [ + TaskResults.create_from( + success=True, + msg="No packit config found in the repository.", + job_config=None, + event=self.event, + ) + ] handler_classes = self.get_handlers_for_event() From 18f7a529b752650e6f5f4a372a409e6a87182779 Mon Sep 17 00:00:00 2001 From: Maja Massarini Date: Wed, 21 Dec 2022 09:51:17 +0100 Subject: [PATCH 06/10] Create new handlers for re-triggering through an issue The koji builds, bodhi updates, propose-downstream jobs can be retriggered through comments in a issue (in the issues repository) and the issue repository does not need the packit yaml. When re-triggering a bodhi update or koji build through a comment in a issue (in the issues repository) do not use local_project or project_url from event. They reference the issue repository and not the distgit repo. Packit API have to be built using the distgit url found in the issue description and saved in a new property inside the AbstractIssueCommentEvent. All the failed branches (written inside the issue description) will be retriggered by a single re-trigger comment. --- packit_service/worker/checker/bodhi.py | 39 ++++---- packit_service/worker/events/comment.py | 4 + packit_service/worker/events/github.py | 2 + packit_service/worker/events/gitlab.py | 2 + packit_service/worker/handlers/abstract.py | 6 +- packit_service/worker/handlers/bodhi.py | 60 +++++++++--- packit_service/worker/handlers/distgit.py | 93 ++++++++++++++----- packit_service/worker/handlers/forges.py | 3 +- packit_service/worker/handlers/koji.py | 5 +- packit_service/worker/handlers/mixin.py | 83 +++++++++++++++-- .../worker/handlers/testing_farm.py | 3 +- packit_service/worker/jobs.py | 53 ++++++----- packit_service/worker/mixin.py | 58 +++++++++++- packit_service/worker/tasks.py | 38 ++++++++ tests/integration/test_handler.py | 6 +- tests/integration/test_issue_comment.py | 2 +- tests/integration/test_pr_comment.py | 2 +- 17 files changed, 364 insertions(+), 95 deletions(-) diff --git a/packit_service/worker/checker/bodhi.py b/packit_service/worker/checker/bodhi.py index 727c60858..e6efa1983 100644 --- a/packit_service/worker/checker/bodhi.py +++ b/packit_service/worker/checker/bodhi.py @@ -45,25 +45,26 @@ def pre_check(self) -> bool: PullRequestCommentPagureEvent.__name__, KojiBuildEvent.__name__, ): - if self.state != KojiBuildState.complete: - logger.debug( - f"Skipping build '{self.build_id}' " - f"on '{self.dist_git_branch}'. " - f"Build not finished yet." - ) - return False - - if self.dist_git_branch not in ( - configured_branches := get_branches( - *(self.job_config.dist_git_branches or {"fedora-stable"}), - default_dg_branch="rawhide", # Koji calls it rawhide, not main - ) - ): - logger.info( - f"Skipping build on '{self.dist_git_branch}'. " - f"Bodhi update configured only for '{configured_branches}'." - ) - return False + for koji_build_data in self: + if koji_build_data.state != KojiBuildState.complete: + logger.debug( + f"Skipping build '{koji_build_data.build_id}' " + f"on '{koji_build_data.dist_git_branch}'. " + f"Build not finished yet." + ) + return False + + if koji_build_data.dist_git_branch not in ( + configured_branches := get_branches( + *(self.job_config.dist_git_branches or {"fedora-stable"}), + default_dg_branch="rawhide", # Koji calls it rawhide, not main + ) + ): + logger.info( + f"Skipping build on '{koji_build_data.dist_git_branch}'. " + f"Bodhi update configured only for '{configured_branches}'." + ) + return False return True diff --git a/packit_service/worker/events/comment.py b/packit_service/worker/events/comment.py index 90444b885..91eef3008 100644 --- a/packit_service/worker/events/comment.py +++ b/packit_service/worker/events/comment.py @@ -130,6 +130,7 @@ def __init__( comment_id: int, tag_name: str = "", comment_object: Optional[Comment] = None, + dist_git_project_url=None, ) -> None: super().__init__( project_url=project_url, @@ -141,6 +142,9 @@ def __init__( self.repo_namespace = repo_namespace self.repo_name = repo_name + # issue description link to dist-git + self.dist_git_project_url = dist_git_project_url + # Lazy properties self._tag_name = tag_name self._commit_sha: Optional[str] = None diff --git a/packit_service/worker/events/github.py b/packit_service/worker/events/github.py index 03df1a2c0..f06d42961 100644 --- a/packit_service/worker/events/github.py +++ b/packit_service/worker/events/github.py @@ -179,6 +179,7 @@ def __init__( str ] = "master", # default is master when working with issues comment_object: Optional[Comment] = None, + dist_git_project_url=None, ) -> None: super().__init__( issue_id=issue_id, @@ -189,6 +190,7 @@ def __init__( comment_id=comment_id, tag_name=tag_name, comment_object=comment_object, + dist_git_project_url=dist_git_project_url, ) self.action = action self.actor = actor diff --git a/packit_service/worker/events/gitlab.py b/packit_service/worker/events/gitlab.py index fe2c9ad8f..1d2315fbd 100644 --- a/packit_service/worker/events/gitlab.py +++ b/packit_service/worker/events/gitlab.py @@ -157,6 +157,7 @@ def __init__( comment_id: int, tag_name: str = "", comment_object: Optional[Comment] = None, + dist_git_project_url=None, ): super().__init__( issue_id=issue_id, @@ -167,6 +168,7 @@ def __init__( comment_id=comment_id, tag_name=tag_name, comment_object=comment_object, + dist_git_project_url=dist_git_project_url, ) self.action = action self.actor = actor diff --git a/packit_service/worker/handlers/abstract.py b/packit_service/worker/handlers/abstract.py index 048aa6ed8..b44e2e4c4 100644 --- a/packit_service/worker/handlers/abstract.py +++ b/packit_service/worker/handlers/abstract.py @@ -32,7 +32,7 @@ from packit_service.worker.checker.abstract import Checker from packit_service.worker.mixin import ( - ConfigFromEventMixin, + Config, PackitAPIWithDownstreamMixin, ) @@ -205,19 +205,21 @@ class TaskName(str, enum.Enum): upstream_koji_build = "task.run_koji_build_handler" upstream_koji_build_report = "task.run_koji_build_report_handler" downstream_koji_build = "task.run_downstream_koji_build_handler" + retrigger_downstream_koji_build = "task.run_retrigger_downstream_koji_build_handler" downstream_koji_build_report = "task.run_downstream_koji_build_report_handler" # Fedora notification is ok for now # downstream_koji_build_report = "task.run_downstream_koji_build_report_handler" sync_from_downstream = "task.run_sync_from_downstream_handler" bodhi_update = "task.bodhi_update" retrigger_bodhi_update = "task.retrigger_bodhi_update" + issue_comment_retrigger_bodhi_update = "task.issue_comment_retrigger_bodhi_update" github_fas_verification = "task.github_fas_verification" vm_image_build = "task.run_vm_image_build_handler" vm_image_build_result = "task.run_vm_image_build_result_handler" pull_from_upstream = "pull_from_upstream" -class Handler(ConfigFromEventMixin, PackitAPIWithDownstreamMixin): +class Handler(PackitAPIWithDownstreamMixin, Config): def run(self) -> TaskResults: raise NotImplementedError("This should have been implemented.") diff --git a/packit_service/worker/handlers/bodhi.py b/packit_service/worker/handlers/bodhi.py index 289a74101..b65d98c7f 100644 --- a/packit_service/worker/handlers/bodhi.py +++ b/packit_service/worker/handlers/bodhi.py @@ -37,12 +37,16 @@ run_for_comment, ) from packit_service.worker.handlers.mixin import ( - GetKojiBuildData, + GetKojiBuildDataFromKojiServiceMultipleBranches, GetKojiBuildDataFromKojiBuildEventMixin, GetKojiBuildDataFromKojiServiceMixin, GetKojiBuildEventMixin, + GetKojiBuildData, + KojiBuildData, ) from packit_service.worker.mixin import ( + ConfigFromDistGitUrlMixin, + GetBranchesFromIssueMixin, PackitAPIWithDownstreamMixin, ) from packit_service.worker.reporting import report_in_issue_repository @@ -72,21 +76,28 @@ def __init__( def run(self) -> TaskResults: try: - self.packit_api.create_update( - dist_git_branch=self.dist_git_branch, - update_type="enhancement", - koji_builds=[self.nvr], # it accepts NVRs, not build IDs - ) + branches = [] + for koji_build_data in self: + branches.append(koji_build_data.dist_git_branch) + logger.debug( + f"Create update for dist-git branch: {koji_build_data.dist_git_branch}" + ) + self.packit_api.create_update( + dist_git_branch=koji_build_data.dist_git_branch, + update_type="enhancement", + koji_builds=[koji_build_data.nvr], # it accepts NVRs, not build IDs + ) + logger.debug(f"Iterated over all dist-git branches: {branches}") except PackitException as ex: logger.debug(f"Bodhi update failed to be created: {ex}") if isinstance(ex.__cause__, AuthError): - body = self._error_message_for_auth_error(ex) + body = self._error_message_for_auth_error(ex, koji_build_data) notify = True known_error = True else: body = ( - f"Bodhi update creation failed for `{self.nvr}`:\n" + f"Bodhi update creation failed for `{koji_build_data.nvr}`:\n" "```\n" f"{ex}\n" "```" @@ -116,9 +127,11 @@ def run(self) -> TaskResults: # Sentry issue will be created otherwise. return TaskResults(success=True, details={}) - def _error_message_for_auth_error(self, ex: PackitException) -> str: + def _error_message_for_auth_error( + self, ex: PackitException, koji_build_data: KojiBuildData + ) -> str: body = ( - f"Bodhi update creation failed for `{self.nvr}` " + f"Bodhi update creation failed for `{koji_build_data.nvr}` " f"because of the missing permissions.\n\n" f"Please, give {self.service_config.fas_user} user `commit` rights in the " f"[dist-git settings]({self.data.project_url}/adduser).\n\n" @@ -171,8 +184,6 @@ def get_checkers() -> Tuple[Type[Checker], ...]: @configured_as(job_type=JobType.bodhi_update) @reacts_to(event=PullRequestCommentPagureEvent) -@reacts_to(event=IssueCommentEvent) -@reacts_to(event=IssueCommentGitlabEvent) @run_for_comment(command="create-update") class RetriggerBodhiUpdateHandler( BodhiUpdateHandler, GetKojiBuildDataFromKojiServiceMixin @@ -194,3 +205,28 @@ def get_checkers() -> Tuple[Type[Checker], ...]: HasIssueCommenterRetriggeringPermissions, IsKojiBuildCompleteAndBranchConfiguredCheckService, ) + + +@configured_as(job_type=JobType.bodhi_update) +@reacts_to(event=IssueCommentEvent) +@reacts_to(event=IssueCommentGitlabEvent) +@run_for_comment(command="create-update") +class IssueCommentRetriggerBodhiUpdateHandler( + BodhiUpdateHandler, + ConfigFromDistGitUrlMixin, + GetBranchesFromIssueMixin, + GetKojiBuildDataFromKojiServiceMultipleBranches, +): + """ + This handler can re-trigger a bodhi update if any successful Koji build. + """ + + task_name = TaskName.issue_comment_retrigger_bodhi_update + + @staticmethod + def get_checkers() -> Tuple[Type[Checker], ...]: + """We react only on finished builds (=KojiBuildState.complete) + and configured branches. + """ + logger.debug("Bodhi update will be re-triggered via dist-git PR comment.") + return (HasIssueCommenterRetriggeringPermissions,) diff --git a/packit_service/worker/handlers/distgit.py b/packit_service/worker/handlers/distgit.py index 123ffae38..73633ab06 100644 --- a/packit_service/worker/handlers/distgit.py +++ b/packit_service/worker/handlers/distgit.py @@ -6,8 +6,9 @@ """ import logging import shutil +import abc from datetime import datetime -from typing import Optional, Tuple, Type +from typing import Optional, Tuple, Type, List from celery import Task from ogr.abstract import PullRequest @@ -67,9 +68,12 @@ from packit_service.worker.helpers.sync_release.sync_release import SyncReleaseHelper from packit_service.worker.mixin import ( LocalProjectMixin, + ConfigFromEventMixin, + GetBranchesFromIssueMixin, + ConfigFromDistGitUrlMixin, + GetPagurePullRequestMixin, PackitAPIWithUpstreamMixin, PackitAPIWithDownstreamMixin, - GetPagurePullRequestMixin, ) from packit_service.worker.reporting import BaseCommitStatus, report_in_issue_repository from packit_service.worker.result import TaskResults @@ -80,7 +84,11 @@ @configured_as(job_type=JobType.sync_from_downstream) @reacts_to(event=PushPagureEvent) class SyncFromDownstream( - JobHandler, GetProjectToSyncMixin, LocalProjectMixin, PackitAPIWithUpstreamMixin + JobHandler, + GetProjectToSyncMixin, + ConfigFromEventMixin, + LocalProjectMixin, + PackitAPIWithUpstreamMixin, ): """Sync new specfile changes to upstream after a new git push in the dist-git.""" @@ -121,7 +129,10 @@ def run(self) -> TaskResults: class AbstractSyncReleaseHandler( - PackitAPIWithUpstreamMixin, LocalProjectMixin, RetriableJobHandler + RetriableJobHandler, + ConfigFromEventMixin, + PackitAPIWithUpstreamMixin, + LocalProjectMixin, ): helper_kls: type[SyncReleaseHelper] sync_release_job_type: SyncReleaseJobType @@ -444,17 +455,9 @@ def _report_errors_for_each_branch(self, message: str) -> None: ) -@configured_as(job_type=JobType.koji_build) -@run_for_comment(command="koji-build") -@reacts_to(event=PushPagureEvent) -@reacts_to(event=IssueCommentEvent) -@reacts_to(event=IssueCommentGitlabEvent) -@reacts_to(event=PullRequestCommentPagureEvent) -class DownstreamKojiBuildHandler( +class AbstractDownstreamKojiBuildHandler( + abc.ABC, RetriableJobHandler, - LocalProjectMixin, - PackitAPIWithDownstreamMixin, - GetPagurePullRequestMixin, ): """ This handler can submit a build in Koji from a dist-git. @@ -484,19 +487,20 @@ def __init__( def get_checkers() -> Tuple[Type[Checker], ...]: return (PermissionOnDistgit, HasIssueCommenterRetriggeringPermissions) + @abc.abstractmethod + def get_branches(self) -> List[str]: + """Get a list of branch (names) to be built in koji""" + def run(self) -> TaskResults: - branch = ( - self.project.get_pr(self.data.pr_id).target_branch - if self.data.event_type in (PullRequestCommentPagureEvent.__name__,) - else self.dg_branch - ) + branch = None try: - self.packit_api.build( - dist_git_branch=branch, - scratch=self.job_config.scratch, - nowait=True, - from_upstream=False, - ) + for branch in self.get_branches(): + self.packit_api.build( + dist_git_branch=branch, + scratch=self.job_config.scratch, + nowait=True, + from_upstream=False, + ) except PackitException as ex: if self.celery_task and not self.celery_task.is_last_try(): logger.debug( @@ -518,3 +522,42 @@ def run(self) -> TaskResults: raise ex return TaskResults(success=True, details={}) + + +@configured_as(job_type=JobType.koji_build) +@run_for_comment(command="koji-build") +@reacts_to(event=PushPagureEvent) +@reacts_to(event=PullRequestCommentPagureEvent) +class DownstreamKojiBuildHandler( + AbstractDownstreamKojiBuildHandler, + ConfigFromEventMixin, + LocalProjectMixin, + PackitAPIWithDownstreamMixin, + GetPagurePullRequestMixin, +): + task_name = TaskName.downstream_koji_build + + def get_branches(self) -> List[str]: + branch = ( + self.project.get_pr(self.data.pr_id).target_branch + if self.data.event_type in (PullRequestCommentPagureEvent.__name__,) + else self.dg_branch + ) + return [branch] + + +@configured_as(job_type=JobType.koji_build) +@run_for_comment(command="koji-build") +@reacts_to(event=IssueCommentEvent) +@reacts_to(event=IssueCommentGitlabEvent) +class RetriggerDownstreamKojiBuildHandler( + AbstractDownstreamKojiBuildHandler, + ConfigFromDistGitUrlMixin, + LocalProjectMixin, + PackitAPIWithDownstreamMixin, + GetBranchesFromIssueMixin, +): + task_name = TaskName.retrigger_downstream_koji_build + + def get_branches(self) -> List[str]: + return self.branches diff --git a/packit_service/worker/handlers/forges.py b/packit_service/worker/handlers/forges.py index b2f8b78ac..278a91c9f 100644 --- a/packit_service/worker/handlers/forges.py +++ b/packit_service/worker/handlers/forges.py @@ -13,6 +13,7 @@ Deployment, ) from packit.config.package_config import PackageConfig +from packit_service.worker.mixin import ConfigFromEventMixin from packit_service.constants import CONTACTS_URL, DOCS_APPROVAL_URL, NOTIFICATION_REPO from packit_service.models import ( GithubInstallationModel, @@ -37,7 +38,7 @@ @reacts_to(event=InstallationEvent) -class GithubAppInstallationHandler(JobHandler): +class GithubAppInstallationHandler(JobHandler, ConfigFromEventMixin): task_name = TaskName.installation # https://developer.github.com/v3/activity/events/types/#events-api-payload-28 diff --git a/packit_service/worker/handlers/koji.py b/packit_service/worker/handlers/koji.py index ad07f9f3d..3c0738e7d 100644 --- a/packit_service/worker/handlers/koji.py +++ b/packit_service/worker/handlers/koji.py @@ -19,6 +19,7 @@ KojiBuildState, ) from packit_service.constants import KojiTaskState +from packit_service.worker.mixin import ConfigFromEventMixin from packit_service.models import AbstractTriggerDbType, KojiBuildTargetModel from packit_service.service.urls import ( get_koji_build_info_url, @@ -99,7 +100,7 @@ def run(self) -> TaskResults: @configured_as(job_type=JobType.production_build) @configured_as(job_type=JobType.upstream_koji_build) @reacts_to(event=KojiTaskEvent) -class KojiTaskReportHandler(JobHandler): +class KojiTaskReportHandler(JobHandler, ConfigFromEventMixin): task_name = TaskName.upstream_koji_build_report def __init__( @@ -219,7 +220,7 @@ def run(self): @configured_as(job_type=JobType.koji_build) @configured_as(job_type=JobType.bodhi_update) @reacts_to(event=KojiBuildEvent) -class KojiBuildReportHandler(JobHandler): +class KojiBuildReportHandler(JobHandler, ConfigFromEventMixin): task_name = TaskName.downstream_koji_build_report def __init__( diff --git a/packit_service/worker/handlers/mixin.py b/packit_service/worker/handlers/mixin.py index f4d47d776..60246f06d 100644 --- a/packit_service/worker/handlers/mixin.py +++ b/packit_service/worker/handlers/mixin.py @@ -3,7 +3,8 @@ import logging from abc import abstractmethod -from typing import Any, Optional, Protocol +from typing import Any, Optional, Protocol, Iterator +from dataclasses import dataclass from packit.exceptions import PackitException from packit.config import PackageConfig, JobConfig from packit.utils.koji_helper import KojiHelper @@ -25,7 +26,7 @@ from packit_service.worker.helpers.build.koji_build import KojiBuildJobHelper from packit_service.worker.helpers.testing_farm import TestingFarmJobHelper -from packit_service.worker.mixin import ConfigFromEventMixin +from packit_service.worker.mixin import Config, ConfigFromEventMixin, GetBranches from packit_service.worker.events.koji import KojiBuildEvent from packit_service.worker.monitoring import Pushgateway @@ -82,7 +83,53 @@ def koji_build_helper(self) -> KojiBuildJobHelper: return self._koji_build_helper -class GetKojiBuildData(Protocol): +@dataclass +class KojiBuildData: + """Koji build data associated with + a selected dist-git branch. + """ + + dist_git_branch: str + build_id: int + nvr: str + state: KojiBuildState + + +class GetKojiBuildData(Iterator, Protocol): + """Get the Koji build data associated with + the selected dist-git branch. + """ + + _branch_index: int = 0 + + def __iter__(self) -> Iterator[KojiBuildData]: + self._branch_index = 0 + return self + + @property + @abstractmethod + def num_of_branches(self): + ... + + def __next__(self) -> KojiBuildData: + """Iterate over all available dist-git branches. + Change internal pointer to the next dist-git branch. + + Returns: + A new set of Koji Build Data associated + with the next available dist_git_branch + """ + if self._branch_index < self.num_of_branches: + koji_build_data = KojiBuildData( + dist_git_branch=self.dist_git_branch, + build_id=self.build_id, + nvr=self.nvr, + state=self.state, + ) + self._branch_index += 1 + return koji_build_data + raise StopIteration + @property @abstractmethod def nvr(self) -> str: @@ -121,8 +168,12 @@ def dist_git_branch(self) -> str: def state(self) -> KojiBuildState: return self.koji_build_event.state + @property + def num_of_branches(self): + return 1 # just a branch in the event + -class GetKojiBuildDataFromKojiServiceMixin(ConfigFromEventMixin, GetKojiBuildData): +class GetKojiBuildDataFromKojiService(Config, GetKojiBuildData): """See https://koji.fedoraproject.org/koji/api method listBuilds for a detailed description of a Koji build map. """ @@ -157,13 +208,33 @@ def nvr(self) -> str: def build_id(self) -> int: return self.build["build_id"] + @property + def state(self) -> KojiBuildState: + return KojiBuildState.from_number(self.build["state"]) + + +class GetKojiBuildDataFromKojiServiceMixin( + ConfigFromEventMixin, GetKojiBuildDataFromKojiService +): @property def dist_git_branch(self) -> str: return self.project.get_pr(self.data.pr_id).target_branch @property - def state(self) -> KojiBuildState: - return KojiBuildState.from_number(self.build["state"]) + def num_of_branches(self): + return 1 # just a branch in the event + + +class GetKojiBuildDataFromKojiServiceMultipleBranches( + GetKojiBuildDataFromKojiService, GetBranches +): + @property + def dist_git_branch(self) -> str: + return self.branches[self._branch_index] + + @property + def num_of_branches(self): + return len(self.branches) class GetCoprBuildEvent(Protocol): diff --git a/packit_service/worker/handlers/testing_farm.py b/packit_service/worker/handlers/testing_farm.py index de194a3bc..c71cff640 100644 --- a/packit_service/worker/handlers/testing_farm.py +++ b/packit_service/worker/handlers/testing_farm.py @@ -25,6 +25,7 @@ get_testing_farm_info_url, get_copr_build_info_url, ) +from packit_service.worker.mixin import ConfigFromEventMixin from packit_service.utils import dump_job_config, dump_package_config, elapsed_seconds from packit_service.worker.checker.abstract import Checker from packit_service.worker.checker.testing_farm import ( @@ -260,7 +261,7 @@ def run(self) -> TaskResults: @configured_as(job_type=JobType.tests) @reacts_to(event=TestingFarmResultsEvent) -class TestingFarmResultsHandler(JobHandler): +class TestingFarmResultsHandler(JobHandler, ConfigFromEventMixin): __test__ = False task_name = TaskName.testing_farm_results diff --git a/packit_service/worker/jobs.py b/packit_service/worker/jobs.py index c5ae292a8..aeddc5ffa 100644 --- a/packit_service/worker/jobs.py +++ b/packit_service/worker/jobs.py @@ -287,6 +287,17 @@ def report_task_accepted( task_accepted_time, handler_kls, number_of_build_targets ) + def search_distgit_config_in_issue(self): + issue = self.event.project.get_issue(self.event.issue_id) + if m := match(r"[\w\s-]+dist-git \((\S+)\):", issue.description): + url = m[1] + project = self.service_config.get_project(url=url) + package_config = PackageConfigGetter.get_package_config_from_repo( + project=project, + fail_when_missing=False, + ) + return url, package_config + def is_packit_config_present(self) -> bool: """ Set fail_when_config_file_missing if we handle comment events so that @@ -301,7 +312,18 @@ def is_packit_config_present(self) -> bool: packit_comment_command_prefix=self.service_config.comment_command_prefix, ): # we require packit config file when event is triggered by /packit command - self.event.fail_when_config_file_missing = True + # but not when it is triggered through an issue in the issues repository + dist_git_package_config = None + if isinstance(self.event, AbstractIssueCommentEvent): + if dist_git_package_config := self.search_distgit_config_in_issue(): + ( + self.event.dist_git_project_url, + self.event._package_config, + ) = dist_git_package_config + return True + + if not dist_git_package_config: + self.event.fail_when_config_file_missing = True if not self.event.package_config: # this happens when service receives events for repos which don't have packit config @@ -310,32 +332,19 @@ def is_packit_config_present(self) -> bool: return True - def search_config_in_distgit(self): - issue = self.event.project.get_issue(self.event.issue_id) - if m := match(r"[\w\s-]+dist-git \((\S+)\):", issue.description): - project = self.service_config.get_project(url=m[1]) - package_config = PackageConfigGetter.get_package_config_from_repo( - project=project, - fail_when_missing=False, - ) - return package_config - def process_jobs(self) -> List[TaskResults]: """ Create Celery tasks for a job handler (if trigger matches) for every job defined in config. """ if not self.is_packit_config_present(): - if dist_git_package_config := self.search_config_in_distgit(): - self.event._package_config = dist_git_package_config - else: - return [ - TaskResults.create_from( - success=True, - msg="No packit config found in the repository.", - job_config=None, - event=self.event, - ) - ] + return [ + TaskResults.create_from( + success=True, + msg="No packit config found in the repository.", + job_config=None, + event=self.event, + ) + ] handler_classes = self.get_handlers_for_event() diff --git a/packit_service/worker/mixin.py b/packit_service/worker/mixin.py index 75a76c98b..793fdc045 100644 --- a/packit_service/worker/mixin.py +++ b/packit_service/worker/mixin.py @@ -3,7 +3,8 @@ from abc import abstractmethod import logging -from typing import Optional, Protocol, Union +import re +from typing import Optional, Protocol, Union, List from fasjson_client import Client from fasjson_client.errors import APIError @@ -94,6 +95,29 @@ def project_url(self) -> str: return self._project_url +class ConfigFromDistGitUrlMixin(Config): + _project: Optional[GitProject] = None + _service_config: Optional[ServiceConfig] = None + _project_url: str + data: EventData + + @property + def service_config(self) -> ServiceConfig: + if not self._service_config: + self._service_config = ServiceConfig.get_service_config() + return self._service_config + + @property + def project(self) -> Optional[GitProject]: + if not self._project and self.data.event_dict["dist_git_project_url"]: + self._project = self.service_config.get_project(url=self.project_url) + return self._project + + @property + def project_url(self) -> str: + return self.data.event_dict["dist_git_project_url"] + + class PackitAPIProtocol(Config): local_project: Optional[LocalProject] = None @@ -171,7 +195,7 @@ def clean_api(self) -> None: self._packit_api.clean() -class LocalProjectMixin(ConfigFromEventMixin): +class LocalProjectMixin(Config): _local_project: Optional[LocalProject] = None @property @@ -242,6 +266,36 @@ def issue(self): return self._issue +class GetBranches(Protocol): + @property + @abstractmethod + def branches(self) -> List[str]: + ... + + +class GetBranchesFromIssueMixin(Config, GetBranches): + @property + def branches(self) -> List[str]: + """Get branches names from an issue comment like the following: + + + Packit failed on creating pull-requests in dist-git (https://src.fedoraproject.org/rpms/python-teamcity-messages): # noqa + + | dist-git branch | error | + | --------------- | ----- | + | `f37` | `` | + + + You can retrigger the update by adding a comment (`/packit propose-downstream`) into this issue. + """ + branches = [] + issue = self.data.project.get_issue(self.data.issue_id) + for line in issue.description.splitlines(): + if m := re.match(r"\| `(\S+)` \|", line): + branches.append(m[1]) + return branches + + class GetVMImageBuilder(Protocol): @property @abstractmethod diff --git a/packit_service/worker/tasks.py b/packit_service/worker/tasks.py index aa2e62550..7e295f425 100644 --- a/packit_service/worker/tasks.py +++ b/packit_service/worker/tasks.py @@ -46,9 +46,11 @@ from packit_service.worker.handlers.bodhi import ( CreateBodhiUpdateHandler, RetriggerBodhiUpdateHandler, + IssueCommentRetriggerBodhiUpdateHandler, ) from packit_service.worker.handlers.distgit import ( DownstreamKojiBuildHandler, + RetriggerDownstreamKojiBuildHandler, PullFromUpstreamHandler, ) from packit_service.worker.handlers.forges import GithubFasVerificationHandler @@ -340,6 +342,24 @@ def run_downstream_koji_build( return get_handlers_task_results(handler.run_job(), event) +@celery_app.task( + bind=True, + name=TaskName.retrigger_downstream_koji_build, + base=HandlerTaskWithRetry, + queue="long-running", +) +def run_retrigger_downstream_koji_build( + self, event: dict, package_config: dict, job_config: dict +): + handler = RetriggerDownstreamKojiBuildHandler( + package_config=load_package_config(package_config), + job_config=load_job_config(job_config), + event=event, + celery_task=self, + ) + return get_handlers_task_results(handler.run_job(), event) + + @celery_app.task(name=TaskName.downstream_koji_build_report, base=HandlerTaskWithRetry) def run_downstream_koji_build_report( event: dict, package_config: dict, job_config: dict @@ -386,6 +406,24 @@ def run_retrigger_bodhi_update( return get_handlers_task_results(handler.run_job(), event) +@celery_app.task( + bind=True, + name=TaskName.issue_comment_retrigger_bodhi_update, + base=HandlerTaskWithRetry, + queue="long-running", +) +def run_issue_comment_retrigger_bodhi_update( + self, event: dict, package_config: dict, job_config: dict +): + handler = IssueCommentRetriggerBodhiUpdateHandler( + package_config=load_package_config(package_config), + job_config=load_job_config(job_config), + event=event, + celery_task=self, + ) + return get_handlers_task_results(handler.run_job(), event) + + @celery_app.task( bind=True, name=TaskName.vm_image_build, diff --git a/tests/integration/test_handler.py b/tests/integration/test_handler.py index a59ee08fe..17a7a8bdc 100644 --- a/tests/integration/test_handler.py +++ b/tests/integration/test_handler.py @@ -22,6 +22,7 @@ JobTriggerModel, JobTriggerModelType, ) +from packit_service.worker.mixin import ConfigFromEventMixin from packit_service.worker.handlers import ( JobHandler, CoprBuildHandler, @@ -39,6 +40,9 @@ def trick_p_s_with_k8s(): def test_handler_cleanup(tmp_path, trick_p_s_with_k8s): + class TestJobHandler(JobHandler, ConfigFromEventMixin): + pass + tmp_path.joinpath("a").mkdir() tmp_path.joinpath("b").write_text("a") tmp_path.joinpath("c").symlink_to("b") @@ -56,7 +60,7 @@ def test_handler_cleanup(tmp_path, trick_p_s_with_k8s): trigger=JobConfigTriggerType.pull_request, packages={"package": CommonPackageConfig()}, ) - j = JobHandler( + j = TestJobHandler( package_config=pc, job_config=jc, event={}, diff --git a/tests/integration/test_issue_comment.py b/tests/integration/test_issue_comment.py index 9f07538ba..33412d4d0 100644 --- a/tests/integration/test_issue_comment.py +++ b/tests/integration/test_issue_comment.py @@ -87,7 +87,7 @@ def mock_comment(request): .with_args(author) .and_return(True) ) - issue = flexmock() + issue = flexmock(description="") flexmock(project_class).should_receive("get_issue").and_return(issue) comment = flexmock() flexmock(issue).should_receive("get_comment").and_return(comment) diff --git a/tests/integration/test_pr_comment.py b/tests/integration/test_pr_comment.py index 2266d5e0c..8333c2710 100644 --- a/tests/integration/test_pr_comment.py +++ b/tests/integration/test_pr_comment.py @@ -2351,7 +2351,7 @@ def test_bodhi_update_retrigger_via_dist_git_pr_comment(pagure_pr_comment_added) ) flexmock(KojiHelper).should_receive("get_latest_build_in_tag").and_return( - {"nvr": "123"} + {"nvr": "123", "build_id": 321, "state": 0} ) pagure_project.should_receive("get_files").with_args( From 23cf23aac247938ef39c0fea28512db25eb8f318 Mon Sep 17 00:00:00 2001 From: Maja Massarini Date: Mon, 9 Jan 2023 15:40:01 +0100 Subject: [PATCH 07/10] Add tests for mixin classes --- packit_service/worker/handlers/mixin.py | 36 +++++------ packit_service/worker/mixin.py | 2 +- tests/unit/test_handler_mixin.py | 68 ++++++++++++++++--- tests/unit/test_mixin.py | 86 +++++++++++++++++++++++++ 4 files changed, 165 insertions(+), 27 deletions(-) diff --git a/packit_service/worker/handlers/mixin.py b/packit_service/worker/handlers/mixin.py index 60246f06d..0bdebada3 100644 --- a/packit_service/worker/handlers/mixin.py +++ b/packit_service/worker/handlers/mixin.py @@ -121,10 +121,10 @@ def __next__(self) -> KojiBuildData: """ if self._branch_index < self.num_of_branches: koji_build_data = KojiBuildData( - dist_git_branch=self.dist_git_branch, - build_id=self.build_id, - nvr=self.nvr, - state=self.state, + dist_git_branch=self._dist_git_branch, + build_id=self._build_id, + nvr=self._nvr, + state=self._state, ) self._branch_index += 1 return koji_build_data @@ -132,40 +132,40 @@ def __next__(self) -> KojiBuildData: @property @abstractmethod - def nvr(self) -> str: + def _nvr(self) -> str: ... @property @abstractmethod - def build_id(self) -> int: + def _build_id(self) -> int: ... @property @abstractmethod - def dist_git_branch(self) -> str: + def _dist_git_branch(self) -> str: ... @property @abstractmethod - def state(self) -> KojiBuildState: + def _state(self) -> KojiBuildState: ... class GetKojiBuildDataFromKojiBuildEventMixin(GetKojiBuildData, GetKojiBuildEvent): @property - def nvr(self) -> str: + def _nvr(self) -> str: return self.koji_build_event.nvr @property - def build_id(self) -> int: + def _build_id(self) -> int: return self.koji_build_event.build_id @property - def dist_git_branch(self) -> str: + def _dist_git_branch(self) -> str: return self.koji_build_event.git_ref @property - def state(self) -> KojiBuildState: + def _state(self) -> KojiBuildState: return self.koji_build_event.state @property @@ -192,7 +192,7 @@ def build(self): if not self._build: self._build = self.koji_helper.get_latest_build_in_tag( package=self.project.repo, - tag=self.dist_git_branch, + tag=self._dist_git_branch, ) if not self._build: raise PackitException( @@ -201,15 +201,15 @@ def build(self): return self._build @property - def nvr(self) -> str: + def _nvr(self) -> str: return self.build["nvr"] @property - def build_id(self) -> int: + def _build_id(self) -> int: return self.build["build_id"] @property - def state(self) -> KojiBuildState: + def _state(self) -> KojiBuildState: return KojiBuildState.from_number(self.build["state"]) @@ -217,7 +217,7 @@ class GetKojiBuildDataFromKojiServiceMixin( ConfigFromEventMixin, GetKojiBuildDataFromKojiService ): @property - def dist_git_branch(self) -> str: + def _dist_git_branch(self) -> str: return self.project.get_pr(self.data.pr_id).target_branch @property @@ -229,7 +229,7 @@ class GetKojiBuildDataFromKojiServiceMultipleBranches( GetKojiBuildDataFromKojiService, GetBranches ): @property - def dist_git_branch(self) -> str: + def _dist_git_branch(self) -> str: return self.branches[self._branch_index] @property diff --git a/packit_service/worker/mixin.py b/packit_service/worker/mixin.py index 793fdc045..73c32328e 100644 --- a/packit_service/worker/mixin.py +++ b/packit_service/worker/mixin.py @@ -291,7 +291,7 @@ def branches(self) -> List[str]: branches = [] issue = self.data.project.get_issue(self.data.issue_id) for line in issue.description.splitlines(): - if m := re.match(r"\| `(\S+)` \|", line): + if m := re.match(r"\s*\| `(\S+)` \|", line): branches.append(m[1]) return branches diff --git a/tests/unit/test_handler_mixin.py b/tests/unit/test_handler_mixin.py index 0094e372d..dc7378c9f 100644 --- a/tests/unit/test_handler_mixin.py +++ b/tests/unit/test_handler_mixin.py @@ -1,12 +1,17 @@ # Copyright Contributors to the Packit project. # SPDX-License-Identifier: MIT +from typing import Optional from flexmock import flexmock + +from ogr.abstract import GitProject from packit.utils.koji_helper import KojiHelper +from packit_service.config import ServiceConfig from packit_service.constants import KojiBuildState from packit_service.worker.handlers.mixin import ( GetKojiBuildDataFromKojiServiceMixin, GetKojiBuildDataFromKojiBuildEventMixin, + GetKojiBuildDataFromKojiServiceMultipleBranches, ) @@ -22,10 +27,15 @@ def __init__(self): package="a_repo", tag="a_branch" ).and_return({"nvr": "1.0.0", "state": 1, "build_id": 123}) mixin = Test() - assert mixin.nvr == "1.0.0" - assert mixin.build_id == 123 - assert mixin.state == KojiBuildState.complete - assert mixin.dist_git_branch == "a_branch" + data = [] + for koji_build_data in mixin: + data.append(koji_build_data) + assert koji_build_data.nvr == "1.0.0" + assert koji_build_data.build_id == 123 + assert koji_build_data.state == KojiBuildState.complete + assert koji_build_data.dist_git_branch == "a_branch" + assert mixin.num_of_branches == 1 + assert len(data) == 1 def test_GetKojiBuildDataFromKojiBuildEventMixin(): @@ -43,7 +53,49 @@ def koji_build_event(self): ) mixin = Test() - assert mixin.nvr == "1.0.0" - assert mixin.build_id == 123 - assert mixin.state == KojiBuildState.complete - assert mixin.dist_git_branch == "a_branch" + data = [] + for koji_build_data in mixin: + data.append(koji_build_data) + assert koji_build_data.nvr == "1.0.0" + assert koji_build_data.build_id == 123 + assert koji_build_data.state == KojiBuildState.complete + assert koji_build_data.dist_git_branch == "a_branch" + assert mixin.num_of_branches == 1 + assert len(data) == 1 + + +def test_GetKojiBuildDataFromKojiServiceMultipleBranches(): + class Test(GetKojiBuildDataFromKojiServiceMultipleBranches): + @property + def service_config(self) -> ServiceConfig: + return flexmock(ServiceConfig) + + @property + def project(self) -> Optional[GitProject]: + return flexmock(repo="a repo") + + @property + def project_url(self) -> str: + return "" + + @property + def branches(self): + return ["f37", "f38"] + + flexmock(KojiHelper).should_receive("get_latest_build_in_tag").with_args( + package="a repo", tag="f37" + ).and_return({"nvr": "1.0.1", "state": 1, "build_id": 123}) + flexmock(KojiHelper).should_receive("get_latest_build_in_tag").with_args( + package="a repo", tag="f38" + ).and_return({"nvr": "1.0.2", "state": 1, "build_id": 1234}) + + mixin = Test() + data = [] + for koji_build_data in mixin: + data.append(koji_build_data) + assert koji_build_data.nvr in ("1.0.1", "1.0.2") + assert koji_build_data.build_id in (123, 1234) + assert koji_build_data.state == KojiBuildState.complete + assert koji_build_data.dist_git_branch in ("f37", "f38") + assert mixin.num_of_branches == 2 + assert len(data) == 2 diff --git a/tests/unit/test_mixin.py b/tests/unit/test_mixin.py index a3bb3d75f..0ccc3038c 100644 --- a/tests/unit/test_mixin.py +++ b/tests/unit/test_mixin.py @@ -1,13 +1,25 @@ # Copyright Contributors to the Packit project. # SPDX-License-Identifier: MIT +import pytest + +from flexmock import flexmock +from typing import Optional + from packit.vm_image_build import ImageBuilder +from ogr.abstract import GitProject +from packit_service.config import ServiceConfig from packit_service.worker.mixin import ( + GetBranchesFromIssueMixin, + ConfigFromDistGitUrlMixin, GetVMImageBuilderMixin, ConfigFromEventMixin, GetVMImageDataMixin, ) +from packit_service.worker.events import EventData +from packit_service.worker.events.comment import AbstractIssueCommentEvent + def test_GetVMImageBuilderMixin(): class Test(ConfigFromEventMixin, GetVMImageBuilderMixin): @@ -43,3 +55,77 @@ def __init__(self) -> None: "upload_request": {"type": "aws", "options": {}}, } assert mixin.image_customizations == {"packages": ["python-knx-stack"]} + + +@pytest.mark.parametrize( + "desc,branches", + [ + ( + """ + | dist-git branch | error | + | --------------- | ----- | + | `f37` | `` | + | `f38` | `` | + """, + ["f37", "f38"], + ), + ( + """ +| dist-git branch | error | +| --------------- | ----- | +| `f37` | `` | +| `f38` | `` | + """, + ["f37", "f38"], + ), + ( + "", + [], + ), + ], +) +def test_GetBranchesFromIssueMixin(desc, branches): + class Test(GetBranchesFromIssueMixin): + def __init__(self) -> None: + project = ( + flexmock() + .should_receive("get_issue") + .and_return(flexmock(description=desc)) + .mock() + ) + self.data = flexmock(project=project, issue_id=1) + + @property + def service_config(self) -> ServiceConfig: + return flexmock(ServiceConfig) + + @property + def project(self) -> Optional[GitProject]: + return None + + @property + def project_url(self) -> str: + return "" + + mixin = Test() + assert mixin.branches == branches + + +def test_ConfigFromDistGitUrlMixin(): + class Test(ConfigFromDistGitUrlMixin): + def __init__(self) -> None: + event = AbstractIssueCommentEvent( + issue_id=1, + repo_namespace="a namespace", + repo_name="a repo name", + project_url="upstream project url", + comment="probably an issue opened by the propose downstream", + comment_id=1, + ) + event.dist_git_project_url = "url to distgit" + self.data = EventData.from_event_dict( + flexmock(event, tag_name="a tag", commit_sha="aebdf").get_dict() + ) + + mixin = Test() + assert mixin.project_url == "url to distgit" From 4cb0f9d4e07aa17d2840230eab99ec7ce5f6a4bb Mon Sep 17 00:00:00 2001 From: Maja Massarini Date: Tue, 10 Jan 2023 12:18:03 +0100 Subject: [PATCH 08/10] Add integration tests for new handlers --- packit_service/worker/handlers/mixin.py | 13 + ..._issue_comment_retrigger_bodhi_update.json | 231 ++++++++++++++++++ ...ry_issue_comment_retrigger_koji_build.json | 231 ++++++++++++++++++ tests/integration/test_issue_comment.py | 143 ++++++++++- 4 files changed, 617 insertions(+), 1 deletion(-) create mode 100644 tests/data/webhooks/github/repository_issue_comment_retrigger_bodhi_update.json create mode 100644 tests/data/webhooks/github/repository_issue_comment_retrigger_koji_build.json diff --git a/packit_service/worker/handlers/mixin.py b/packit_service/worker/handlers/mixin.py index 0bdebada3..656685ab5 100644 --- a/packit_service/worker/handlers/mixin.py +++ b/packit_service/worker/handlers/mixin.py @@ -232,6 +232,19 @@ class GetKojiBuildDataFromKojiServiceMultipleBranches( def _dist_git_branch(self) -> str: return self.branches[self._branch_index] + @property + def build(self): + # call it every time since dist_git_branch reference can change + build = self.koji_helper.get_latest_build_in_tag( + package=self.project.repo, + tag=self._dist_git_branch, + ) + if not build: + raise PackitException( + f"No build found for package={self.project.repo} and tag={self.dist_git_branch}" + ) + return build + @property def num_of_branches(self): return len(self.branches) diff --git a/tests/data/webhooks/github/repository_issue_comment_retrigger_bodhi_update.json b/tests/data/webhooks/github/repository_issue_comment_retrigger_bodhi_update.json new file mode 100644 index 000000000..28a87f623 --- /dev/null +++ b/tests/data/webhooks/github/repository_issue_comment_retrigger_bodhi_update.json @@ -0,0 +1,231 @@ +{ + "action": "created", + "issue": { + "url": "https://api.github.com/repos/majamassarini/issues-repo/issues/1", + "repository_url": "https://api.github.com/repos/majamassarini/issues-repo", + "labels_url": "https://api.github.com/repos/majamassarini/issues-repo/issues/1/labels{/name}", + "comments_url": "https://api.github.com/repos/majamassarini/issues-repo/issues/1/comments", + "events_url": "https://api.github.com/repos/majamassarini/issues-repo/issues/1/events", + "html_url": "https://github.com/majamassarini/issues-repo/issues/1", + "id": 1502729797, + "node_id": "I_kwDOIoNLcc5ZkdZF", + "number": 1, + "title": "[packit] Propose downstream failed for release x.y.z", + "user": { + "login": "majamassarini", + "id": 2678400, + "node_id": "MDQ6VXNlcjI2Nzg0MDA=", + "avatar_url": "https://avatars.githubusercontent.com/u/2678400?v=4", + "gravatar_id": "", + "url": "https://api.github.com/users/majamassarini", + "html_url": "https://github.com/majamassarini", + "followers_url": "https://api.github.com/users/majamassarini/followers", + "following_url": "https://api.github.com/users/majamassarini/following{/other_user}", + "gists_url": "https://api.github.com/users/majamassarini/gists{/gist_id}", + "starred_url": "https://api.github.com/users/majamassarini/starred{/owner}{/repo}", + "subscriptions_url": "https://api.github.com/users/majamassarini/subscriptions", + "organizations_url": "https://api.github.com/users/majamassarini/orgs", + "repos_url": "https://api.github.com/users/majamassarini/repos", + "events_url": "https://api.github.com/users/majamassarini/events{/privacy}", + "received_events_url": "https://api.github.com/users/majamassarini/received_events", + "type": "User", + "site_admin": false + }, + "labels": [], + "state": "open", + "locked": false, + "assignee": null, + "assignees": [], + "milestone": null, + "comments": 3, + "created_at": "2022-12-19T10:56:17Z", + "updated_at": "2022-12-20T12:46:34Z", + "closed_at": null, + "author_association": "OWNER", + "active_lock_reason": null, + "body": "Packit failed on creating pull-requests in dist-git (https://src.fedoraproject.org/rpms/python-teamcity-messages):| dist-git branch | error || --------------- | ----- || `f37` | `` |You can retrigger the update by adding a comment (`/packit propose-downstream`) into this issue.", + "reactions": { + "url": "https://api.github.com/repos/majamassarini/issues-repo/issues/1/reactions", + "total_count": 0, + "+1": 0, + "-1": 0, + "laugh": 0, + "hooray": 0, + "confused": 0, + "heart": 0, + "rocket": 0, + "eyes": 0 + }, + "timeline_url": "https://api.github.com/repos/majamassarini/issues-repo/issues/1/timeline", + "performed_via_github_app": null, + "state_reason": null + }, + "comment": { + "url": "https://api.github.com/repos/majamassarini/issues-repo/issues/comments/1359303806", + "html_url": "https://github.com/majamassarini/issues-repo/issues/1#issuecomment-1359303806", + "issue_url": "https://api.github.com/repos/majamassarini/issues-repo/issues/1", + "id": 1359303806, + "node_id": "IC_kwDOIoNLcc5RBVR-", + "user": { + "login": "majamassarini", + "id": 2678400, + "node_id": "MDQ6VXNlcjI2Nzg0MDA=", + "avatar_url": "https://avatars.githubusercontent.com/u/2678400?v=4", + "gravatar_id": "", + "url": "https://api.github.com/users/majamassarini", + "html_url": "https://github.com/majamassarini", + "followers_url": "https://api.github.com/users/majamassarini/followers", + "following_url": "https://api.github.com/users/majamassarini/following{/other_user}", + "gists_url": "https://api.github.com/users/majamassarini/gists{/gist_id}", + "starred_url": "https://api.github.com/users/majamassarini/starred{/owner}{/repo}", + "subscriptions_url": "https://api.github.com/users/majamassarini/subscriptions", + "organizations_url": "https://api.github.com/users/majamassarini/orgs", + "repos_url": "https://api.github.com/users/majamassarini/repos", + "events_url": "https://api.github.com/users/majamassarini/events{/privacy}", + "received_events_url": "https://api.github.com/users/majamassarini/received_events", + "type": "User", + "site_admin": false + }, + "created_at": "2022-12-20T12:46:34Z", + "updated_at": "2022-12-20T12:46:34Z", + "author_association": "OWNER", + "body": "/packit create-update", + "reactions": { + "url": "https://api.github.com/repos/majamassarini/issues-repo/issues/comments/1359303806/reactions", + "total_count": 0, + "+1": 0, + "-1": 0, + "laugh": 0, + "hooray": 0, + "confused": 0, + "heart": 0, + "rocket": 0, + "eyes": 0 + }, + "performed_via_github_app": null + }, + "repository": { + "id": 579029873, + "node_id": "R_kgDOIoNLcQ", + "name": "issues-repo", + "full_name": "majamassarini/issues-repo", + "private": false, + "owner": { + "login": "majamassarini", + "id": 2678400, + "node_id": "MDQ6VXNlcjI2Nzg0MDA=", + "avatar_url": "https://avatars.githubusercontent.com/u/2678400?v=4", + "gravatar_id": "", + "url": "https://api.github.com/users/majamassarini", + "html_url": "https://github.com/majamassarini", + "followers_url": "https://api.github.com/users/majamassarini/followers", + "following_url": "https://api.github.com/users/majamassarini/following{/other_user}", + "gists_url": "https://api.github.com/users/majamassarini/gists{/gist_id}", + "starred_url": "https://api.github.com/users/majamassarini/starred{/owner}{/repo}", + "subscriptions_url": "https://api.github.com/users/majamassarini/subscriptions", + "organizations_url": "https://api.github.com/users/majamassarini/orgs", + "repos_url": "https://api.github.com/users/majamassarini/repos", + "events_url": "https://api.github.com/users/majamassarini/events{/privacy}", + "received_events_url": "https://api.github.com/users/majamassarini/received_events", + "type": "User", + "site_admin": false + }, + "html_url": "https://github.com/majamassarini/issues-repo", + "description": "Python team city messages downstream releases issues automatically created by Packit", + "fork": false, + "url": "https://api.github.com/repos/majamassarini/issues-repo", + "forks_url": "https://api.github.com/repos/majamassarini/issues-repo/forks", + "keys_url": "https://api.github.com/repos/majamassarini/issues-repo/keys{/key_id}", + "collaborators_url": "https://api.github.com/repos/majamassarini/issues-repo/collaborators{/collaborator}", + "teams_url": "https://api.github.com/repos/majamassarini/issues-repo/teams", + "hooks_url": "https://api.github.com/repos/majamassarini/issues-repo/hooks", + "issue_events_url": "https://api.github.com/repos/majamassarini/issues-repo/issues/events{/number}", + "events_url": "https://api.github.com/repos/majamassarini/issues-repo/events", + "assignees_url": "https://api.github.com/repos/majamassarini/issues-repo/assignees{/user}", + "branches_url": "https://api.github.com/repos/majamassarini/issues-repo/branches{/branch}", + "tags_url": "https://api.github.com/repos/majamassarini/issues-repo/tags", + "blobs_url": "https://api.github.com/repos/majamassarini/issues-repo/git/blobs{/sha}", + "git_tags_url": "https://api.github.com/repos/majamassarini/issues-repo/git/tags{/sha}", + "git_refs_url": "https://api.github.com/repos/majamassarini/issues-repo/git/refs{/sha}", + "trees_url": "https://api.github.com/repos/majamassarini/issues-repo/git/trees{/sha}", + "statuses_url": "https://api.github.com/repos/majamassarini/issues-repo/statuses/{sha}", + "languages_url": "https://api.github.com/repos/majamassarini/issues-repo/languages", + "stargazers_url": "https://api.github.com/repos/majamassarini/issues-repo/stargazers", + "contributors_url": "https://api.github.com/repos/majamassarini/issues-repo/contributors", + "subscribers_url": "https://api.github.com/repos/majamassarini/issues-repo/subscribers", + "subscription_url": "https://api.github.com/repos/majamassarini/issues-repo/subscription", + "commits_url": "https://api.github.com/repos/majamassarini/issues-repo/commits{/sha}", + "git_commits_url": "https://api.github.com/repos/majamassarini/issues-repo/git/commits{/sha}", + "comments_url": "https://api.github.com/repos/majamassarini/issues-repo/comments{/number}", + "issue_comment_url": "https://api.github.com/repos/majamassarini/issues-repo/issues/comments{/number}", + "contents_url": "https://api.github.com/repos/majamassarini/issues-repo/contents/{+path}", + "compare_url": "https://api.github.com/repos/majamassarini/issues-repo/compare/{base}...{head}", + "merges_url": "https://api.github.com/repos/majamassarini/issues-repo/merges", + "archive_url": "https://api.github.com/repos/majamassarini/issues-repo/{archive_format}{/ref}", + "downloads_url": "https://api.github.com/repos/majamassarini/issues-repo/downloads", + "issues_url": "https://api.github.com/repos/majamassarini/issues-repo/issues{/number}", + "pulls_url": "https://api.github.com/repos/majamassarini/issues-repo/pulls{/number}", + "milestones_url": "https://api.github.com/repos/majamassarini/issues-repo/milestones{/number}", + "notifications_url": "https://api.github.com/repos/majamassarini/issues-repo/notifications{?since,all,participating}", + "labels_url": "https://api.github.com/repos/majamassarini/issues-repo/labels{/name}", + "releases_url": "https://api.github.com/repos/majamassarini/issues-repo/releases{/id}", + "deployments_url": "https://api.github.com/repos/majamassarini/issues-repo/deployments", + "created_at": "2022-12-16T13:37:26Z", + "updated_at": "2022-12-19T10:55:21Z", + "pushed_at": "2022-12-16T13:37:26Z", + "git_url": "git://github.com/majamassarini/issues-repo.git", + "ssh_url": "git@github.com:majamassarini/issues-repo.git", + "clone_url": "https://github.com/majamassarini/issues-repo.git", + "svn_url": "https://github.com/majamassarini/issues-repo", + "homepage": null, + "size": 0, + "stargazers_count": 0, + "watchers_count": 0, + "language": null, + "has_issues": true, + "has_projects": true, + "has_downloads": true, + "has_wiki": true, + "has_pages": false, + "has_discussions": false, + "forks_count": 0, + "mirror_url": null, + "archived": false, + "disabled": false, + "open_issues_count": 2, + "license": null, + "allow_forking": true, + "is_template": false, + "web_commit_signoff_required": false, + "topics": [], + "visibility": "public", + "forks": 0, + "open_issues": 2, + "watchers": 0, + "default_branch": "main" + }, + "sender": { + "login": "majamassarini", + "id": 2678400, + "node_id": "MDQ6VXNlcjI2Nzg0MDA=", + "avatar_url": "https://avatars.githubusercontent.com/u/2678400?v=4", + "gravatar_id": "", + "url": "https://api.github.com/users/majamassarini", + "html_url": "https://github.com/majamassarini", + "followers_url": "https://api.github.com/users/majamassarini/followers", + "following_url": "https://api.github.com/users/majamassarini/following{/other_user}", + "gists_url": "https://api.github.com/users/majamassarini/gists{/gist_id}", + "starred_url": "https://api.github.com/users/majamassarini/starred{/owner}{/repo}", + "subscriptions_url": "https://api.github.com/users/majamassarini/subscriptions", + "organizations_url": "https://api.github.com/users/majamassarini/orgs", + "repos_url": "https://api.github.com/users/majamassarini/repos", + "events_url": "https://api.github.com/users/majamassarini/events{/privacy}", + "received_events_url": "https://api.github.com/users/majamassarini/received_events", + "type": "User", + "site_admin": false + }, + "installation": { + "id": 28742069, + "node_id": "MDIzOkludGVncmF0aW9uSW5zdGFsbGF0aW9uMjg3NDIwNjk=" + } +} diff --git a/tests/data/webhooks/github/repository_issue_comment_retrigger_koji_build.json b/tests/data/webhooks/github/repository_issue_comment_retrigger_koji_build.json new file mode 100644 index 000000000..21ebce33d --- /dev/null +++ b/tests/data/webhooks/github/repository_issue_comment_retrigger_koji_build.json @@ -0,0 +1,231 @@ +{ + "action": "created", + "issue": { + "url": "https://api.github.com/repos/majamassarini/issues-repo/issues/1", + "repository_url": "https://api.github.com/repos/majamassarini/issues-repo", + "labels_url": "https://api.github.com/repos/majamassarini/issues-repo/issues/1/labels{/name}", + "comments_url": "https://api.github.com/repos/majamassarini/issues-repo/issues/1/comments", + "events_url": "https://api.github.com/repos/majamassarini/issues-repo/issues/1/events", + "html_url": "https://github.com/majamassarini/issues-repo/issues/1", + "id": 1502729797, + "node_id": "I_kwDOIoNLcc5ZkdZF", + "number": 1, + "title": "[packit] Propose downstream failed for release x.y.z", + "user": { + "login": "majamassarini", + "id": 2678400, + "node_id": "MDQ6VXNlcjI2Nzg0MDA=", + "avatar_url": "https://avatars.githubusercontent.com/u/2678400?v=4", + "gravatar_id": "", + "url": "https://api.github.com/users/majamassarini", + "html_url": "https://github.com/majamassarini", + "followers_url": "https://api.github.com/users/majamassarini/followers", + "following_url": "https://api.github.com/users/majamassarini/following{/other_user}", + "gists_url": "https://api.github.com/users/majamassarini/gists{/gist_id}", + "starred_url": "https://api.github.com/users/majamassarini/starred{/owner}{/repo}", + "subscriptions_url": "https://api.github.com/users/majamassarini/subscriptions", + "organizations_url": "https://api.github.com/users/majamassarini/orgs", + "repos_url": "https://api.github.com/users/majamassarini/repos", + "events_url": "https://api.github.com/users/majamassarini/events{/privacy}", + "received_events_url": "https://api.github.com/users/majamassarini/received_events", + "type": "User", + "site_admin": false + }, + "labels": [], + "state": "open", + "locked": false, + "assignee": null, + "assignees": [], + "milestone": null, + "comments": 3, + "created_at": "2022-12-19T10:56:17Z", + "updated_at": "2022-12-20T12:46:34Z", + "closed_at": null, + "author_association": "OWNER", + "active_lock_reason": null, + "body": "Packit failed on creating pull-requests in dist-git (https://src.fedoraproject.org/rpms/python-teamcity-messages):| dist-git branch | error || --------------- | ----- || `f37` | `` |You can retrigger the update by adding a comment (`/packit propose-downstream`) into this issue.", + "reactions": { + "url": "https://api.github.com/repos/majamassarini/issues-repo/issues/1/reactions", + "total_count": 0, + "+1": 0, + "-1": 0, + "laugh": 0, + "hooray": 0, + "confused": 0, + "heart": 0, + "rocket": 0, + "eyes": 0 + }, + "timeline_url": "https://api.github.com/repos/majamassarini/issues-repo/issues/1/timeline", + "performed_via_github_app": null, + "state_reason": null + }, + "comment": { + "url": "https://api.github.com/repos/majamassarini/issues-repo/issues/comments/1359303806", + "html_url": "https://github.com/majamassarini/issues-repo/issues/1#issuecomment-1359303806", + "issue_url": "https://api.github.com/repos/majamassarini/issues-repo/issues/1", + "id": 1359303806, + "node_id": "IC_kwDOIoNLcc5RBVR-", + "user": { + "login": "majamassarini", + "id": 2678400, + "node_id": "MDQ6VXNlcjI2Nzg0MDA=", + "avatar_url": "https://avatars.githubusercontent.com/u/2678400?v=4", + "gravatar_id": "", + "url": "https://api.github.com/users/majamassarini", + "html_url": "https://github.com/majamassarini", + "followers_url": "https://api.github.com/users/majamassarini/followers", + "following_url": "https://api.github.com/users/majamassarini/following{/other_user}", + "gists_url": "https://api.github.com/users/majamassarini/gists{/gist_id}", + "starred_url": "https://api.github.com/users/majamassarini/starred{/owner}{/repo}", + "subscriptions_url": "https://api.github.com/users/majamassarini/subscriptions", + "organizations_url": "https://api.github.com/users/majamassarini/orgs", + "repos_url": "https://api.github.com/users/majamassarini/repos", + "events_url": "https://api.github.com/users/majamassarini/events{/privacy}", + "received_events_url": "https://api.github.com/users/majamassarini/received_events", + "type": "User", + "site_admin": false + }, + "created_at": "2022-12-20T12:46:34Z", + "updated_at": "2022-12-20T12:46:34Z", + "author_association": "OWNER", + "body": "/packit koji-build", + "reactions": { + "url": "https://api.github.com/repos/majamassarini/issues-repo/issues/comments/1359303806/reactions", + "total_count": 0, + "+1": 0, + "-1": 0, + "laugh": 0, + "hooray": 0, + "confused": 0, + "heart": 0, + "rocket": 0, + "eyes": 0 + }, + "performed_via_github_app": null + }, + "repository": { + "id": 579029873, + "node_id": "R_kgDOIoNLcQ", + "name": "issues-repo", + "full_name": "majamassarini/issues-repo", + "private": false, + "owner": { + "login": "majamassarini", + "id": 2678400, + "node_id": "MDQ6VXNlcjI2Nzg0MDA=", + "avatar_url": "https://avatars.githubusercontent.com/u/2678400?v=4", + "gravatar_id": "", + "url": "https://api.github.com/users/majamassarini", + "html_url": "https://github.com/majamassarini", + "followers_url": "https://api.github.com/users/majamassarini/followers", + "following_url": "https://api.github.com/users/majamassarini/following{/other_user}", + "gists_url": "https://api.github.com/users/majamassarini/gists{/gist_id}", + "starred_url": "https://api.github.com/users/majamassarini/starred{/owner}{/repo}", + "subscriptions_url": "https://api.github.com/users/majamassarini/subscriptions", + "organizations_url": "https://api.github.com/users/majamassarini/orgs", + "repos_url": "https://api.github.com/users/majamassarini/repos", + "events_url": "https://api.github.com/users/majamassarini/events{/privacy}", + "received_events_url": "https://api.github.com/users/majamassarini/received_events", + "type": "User", + "site_admin": false + }, + "html_url": "https://github.com/majamassarini/issues-repo", + "description": "Python team city messages downstream releases issues automatically created by Packit", + "fork": false, + "url": "https://api.github.com/repos/majamassarini/issues-repo", + "forks_url": "https://api.github.com/repos/majamassarini/issues-repo/forks", + "keys_url": "https://api.github.com/repos/majamassarini/issues-repo/keys{/key_id}", + "collaborators_url": "https://api.github.com/repos/majamassarini/issues-repo/collaborators{/collaborator}", + "teams_url": "https://api.github.com/repos/majamassarini/issues-repo/teams", + "hooks_url": "https://api.github.com/repos/majamassarini/issues-repo/hooks", + "issue_events_url": "https://api.github.com/repos/majamassarini/issues-repo/issues/events{/number}", + "events_url": "https://api.github.com/repos/majamassarini/issues-repo/events", + "assignees_url": "https://api.github.com/repos/majamassarini/issues-repo/assignees{/user}", + "branches_url": "https://api.github.com/repos/majamassarini/issues-repo/branches{/branch}", + "tags_url": "https://api.github.com/repos/majamassarini/issues-repo/tags", + "blobs_url": "https://api.github.com/repos/majamassarini/issues-repo/git/blobs{/sha}", + "git_tags_url": "https://api.github.com/repos/majamassarini/issues-repo/git/tags{/sha}", + "git_refs_url": "https://api.github.com/repos/majamassarini/issues-repo/git/refs{/sha}", + "trees_url": "https://api.github.com/repos/majamassarini/issues-repo/git/trees{/sha}", + "statuses_url": "https://api.github.com/repos/majamassarini/issues-repo/statuses/{sha}", + "languages_url": "https://api.github.com/repos/majamassarini/issues-repo/languages", + "stargazers_url": "https://api.github.com/repos/majamassarini/issues-repo/stargazers", + "contributors_url": "https://api.github.com/repos/majamassarini/issues-repo/contributors", + "subscribers_url": "https://api.github.com/repos/majamassarini/issues-repo/subscribers", + "subscription_url": "https://api.github.com/repos/majamassarini/issues-repo/subscription", + "commits_url": "https://api.github.com/repos/majamassarini/issues-repo/commits{/sha}", + "git_commits_url": "https://api.github.com/repos/majamassarini/issues-repo/git/commits{/sha}", + "comments_url": "https://api.github.com/repos/majamassarini/issues-repo/comments{/number}", + "issue_comment_url": "https://api.github.com/repos/majamassarini/issues-repo/issues/comments{/number}", + "contents_url": "https://api.github.com/repos/majamassarini/issues-repo/contents/{+path}", + "compare_url": "https://api.github.com/repos/majamassarini/issues-repo/compare/{base}...{head}", + "merges_url": "https://api.github.com/repos/majamassarini/issues-repo/merges", + "archive_url": "https://api.github.com/repos/majamassarini/issues-repo/{archive_format}{/ref}", + "downloads_url": "https://api.github.com/repos/majamassarini/issues-repo/downloads", + "issues_url": "https://api.github.com/repos/majamassarini/issues-repo/issues{/number}", + "pulls_url": "https://api.github.com/repos/majamassarini/issues-repo/pulls{/number}", + "milestones_url": "https://api.github.com/repos/majamassarini/issues-repo/milestones{/number}", + "notifications_url": "https://api.github.com/repos/majamassarini/issues-repo/notifications{?since,all,participating}", + "labels_url": "https://api.github.com/repos/majamassarini/issues-repo/labels{/name}", + "releases_url": "https://api.github.com/repos/majamassarini/issues-repo/releases{/id}", + "deployments_url": "https://api.github.com/repos/majamassarini/issues-repo/deployments", + "created_at": "2022-12-16T13:37:26Z", + "updated_at": "2022-12-19T10:55:21Z", + "pushed_at": "2022-12-16T13:37:26Z", + "git_url": "git://github.com/majamassarini/issues-repo.git", + "ssh_url": "git@github.com:majamassarini/issues-repo.git", + "clone_url": "https://github.com/majamassarini/issues-repo.git", + "svn_url": "https://github.com/majamassarini/issues-repo", + "homepage": null, + "size": 0, + "stargazers_count": 0, + "watchers_count": 0, + "language": null, + "has_issues": true, + "has_projects": true, + "has_downloads": true, + "has_wiki": true, + "has_pages": false, + "has_discussions": false, + "forks_count": 0, + "mirror_url": null, + "archived": false, + "disabled": false, + "open_issues_count": 2, + "license": null, + "allow_forking": true, + "is_template": false, + "web_commit_signoff_required": false, + "topics": [], + "visibility": "public", + "forks": 0, + "open_issues": 2, + "watchers": 0, + "default_branch": "main" + }, + "sender": { + "login": "majamassarini", + "id": 2678400, + "node_id": "MDQ6VXNlcjI2Nzg0MDA=", + "avatar_url": "https://avatars.githubusercontent.com/u/2678400?v=4", + "gravatar_id": "", + "url": "https://api.github.com/users/majamassarini", + "html_url": "https://github.com/majamassarini", + "followers_url": "https://api.github.com/users/majamassarini/followers", + "following_url": "https://api.github.com/users/majamassarini/following{/other_user}", + "gists_url": "https://api.github.com/users/majamassarini/gists{/gist_id}", + "starred_url": "https://api.github.com/users/majamassarini/starred{/owner}{/repo}", + "subscriptions_url": "https://api.github.com/users/majamassarini/subscriptions", + "organizations_url": "https://api.github.com/users/majamassarini/orgs", + "repos_url": "https://api.github.com/users/majamassarini/repos", + "events_url": "https://api.github.com/users/majamassarini/events{/privacy}", + "received_events_url": "https://api.github.com/users/majamassarini/received_events", + "type": "User", + "site_admin": false + }, + "installation": { + "id": 28742069, + "node_id": "MDIzOkludGVncmF0aW9uSW5zdGFsbGF0aW9uMjg3NDIwNjk=" + } +} diff --git a/tests/integration/test_issue_comment.py b/tests/integration/test_issue_comment.py index 33412d4d0..d020a93a1 100644 --- a/tests/integration/test_issue_comment.py +++ b/tests/integration/test_issue_comment.py @@ -18,6 +18,7 @@ from packit.config import JobConfigTriggerType from packit.distgit import DistGit from packit.local_project import LocalProject +from packit.utils.koji_helper import KojiHelper from packit_service.constants import COMMENT_REACTION, TASK_ACCEPTED from packit_service.models import ( IssueModel, @@ -35,10 +36,17 @@ from packit_service.worker.helpers.sync_release.propose_downstream import ( ProposeDownstreamJobHelper, ) +from packit_service.worker.handlers.distgit import ( + RetriggerDownstreamKojiBuildHandler, +) from packit_service.worker.jobs import SteveJobs from packit_service.worker.monitoring import Pushgateway from packit_service.worker.reporting import BaseCommitStatus -from packit_service.worker.tasks import run_propose_downstream_handler +from packit_service.worker.tasks import ( + run_propose_downstream_handler, + run_retrigger_downstream_koji_build, + run_issue_comment_retrigger_bodhi_update, +) from tests.spellbook import DATA_DIR, first_dict_value, get_parameters_from_results @@ -250,3 +258,136 @@ def test_issue_comment_propose_downstream_handler( ) assert first_dict_value(results["job"])["success"] + + +@pytest.fixture() +def mock_repository_issue_retriggering(): + flexmock(GithubProject).should_receive("is_private").and_return(False) + + issue = flexmock( + description=""" +Packit failed on creating pull-requests in dist-git (https://src.fedoraproject.org/rpms/python-teamcity-messages): # noqa +| dist-git branch | error | +| --------------- | ----- | +| `f37` | `` | +| `f38` | `` | +You can retrigger the update by adding a comment (`/packit propose-downstream`) into this issue. + """ + ) + project = ( + flexmock(GithubProject).should_receive("get_issue").and_return(issue).mock() + ) + project.should_receive("get_latest_release").and_return(flexmock(tag_name="123")) + project.should_receive("get_sha_from_tag").and_return("abcdef") + project.should_receive("has_write_access").and_return(True) + db_trigger = flexmock( + id=123, + job_config_trigger_type=JobConfigTriggerType.release, + job_trigger_model_type=JobTriggerModelType.issue, + ) + flexmock(IssueCommentEvent).should_receive("db_trigger").and_return(db_trigger) + comment = flexmock() + flexmock(issue).should_receive("get_comment").and_return(comment) + flexmock(comment).should_receive("add_reaction").with_args(COMMENT_REACTION).once() + flexmock(Allowlist, check_and_report=True) + flexmock(Signature).should_receive("apply_async").once() + flexmock(Pushgateway).should_receive("push").and_return() + + +@pytest.fixture() +def github_repository_issue_comment_retrigger_bodhi_update(): + return json.loads( + ( + DATA_DIR + / "webhooks" + / "github" + / "repository_issue_comment_retrigger_bodhi_update.json" + ).read_text() + ) + + +def test_issue_comment_retrigger_bodhi_update_handler( + mock_repository_issue_retriggering, + github_repository_issue_comment_retrigger_bodhi_update, +): + processing_results = SteveJobs().process_message( + github_repository_issue_comment_retrigger_bodhi_update + ) + event_dict, _, job_config, package_config = get_parameters_from_results( + processing_results + ) + assert json.dumps(event_dict) + + flexmock(PackitAPI).should_receive("create_update").with_args( + dist_git_branch="f38", + update_type="enhancement", + koji_builds=["python-teamcity-messages.f38"], + ) + flexmock(KojiHelper).should_receive("get_latest_build_in_tag").with_args( + package="python-teamcity-messages", tag="f38" + ).and_return({"nvr": "python-teamcity-messages.f38", "build_id": 2, "state": 1}) + flexmock(PackitAPI).should_receive("create_update").with_args( + dist_git_branch="f37", + update_type="enhancement", + koji_builds=["python-teamcity-messages.f37"], + ) + flexmock(KojiHelper).should_receive("get_latest_build_in_tag").with_args( + package="python-teamcity-messages", tag="f37" + ).and_return({"nvr": "python-teamcity-messages.f37", "build_id": 1, "state": 1}) + + results = run_issue_comment_retrigger_bodhi_update( + package_config=package_config, + event=event_dict, + job_config=job_config, + ) + + assert first_dict_value(results["job"])["success"] + + +@pytest.fixture() +def github_repository_issue_comment_retrigger_koji_build(): + return json.loads( + ( + DATA_DIR + / "webhooks" + / "github" + / "repository_issue_comment_retrigger_koji_build.json" + ).read_text() + ) + + +def test_issue_comment_retrigger_koji_build_handler( + mock_repository_issue_retriggering, + github_repository_issue_comment_retrigger_koji_build, +): + processing_results = SteveJobs().process_message( + github_repository_issue_comment_retrigger_koji_build + ) + event_dict, _, job_config, package_config = get_parameters_from_results( + processing_results + ) + assert json.dumps(event_dict) + + flexmock(PackitAPI).should_receive("build").with_args( + dist_git_branch="f37", + scratch=False, + nowait=True, + from_upstream=False, + ) + flexmock(PackitAPI).should_receive("build").with_args( + dist_git_branch="f38", + scratch=False, + nowait=True, + from_upstream=False, + ) + flexmock(RetriggerDownstreamKojiBuildHandler).should_receive( + "local_project" + ).and_return(flexmock()) + + results = run_retrigger_downstream_koji_build( + package_config=package_config, + event=event_dict, + job_config=job_config, + ) + + assert first_dict_value(results["job"])["success"] From cc18ada36e41d8ed4757ef6087eb494e0122008a Mon Sep 17 00:00:00 2001 From: Maja Massarini Date: Wed, 11 Jan 2023 09:50:21 +0100 Subject: [PATCH 09/10] Add docstrings to new method --- packit_service/worker/jobs.py | 40 ++++++++++++++++++++++++----------- 1 file changed, 28 insertions(+), 12 deletions(-) diff --git a/packit_service/worker/jobs.py b/packit_service/worker/jobs.py index aeddc5ffa..36f97af23 100644 --- a/packit_service/worker/jobs.py +++ b/packit_service/worker/jobs.py @@ -7,7 +7,7 @@ import logging from datetime import datetime, timezone from typing import Any, Optional, Union -from typing import List, Set, Type +from typing import List, Set, Type, Tuple from re import match import celery @@ -15,7 +15,7 @@ from ogr.exceptions import GithubAppNotInstalledError from packit.config import JobConfig, JobType, JobConfigTriggerType from packit.config.job_config import DEPRECATED_JOB_TYPES -from packit_service.config import PackageConfigGetter, ServiceConfig +from packit_service.config import PackageConfig, PackageConfigGetter, ServiceConfig from packit_service.constants import ( DOCS_CONFIGURATION_URL, TASK_ACCEPTED, @@ -287,16 +287,32 @@ def report_task_accepted( task_accepted_time, handler_kls, number_of_build_targets ) - def search_distgit_config_in_issue(self): - issue = self.event.project.get_issue(self.event.issue_id) - if m := match(r"[\w\s-]+dist-git \((\S+)\):", issue.description): - url = m[1] - project = self.service_config.get_project(url=url) - package_config = PackageConfigGetter.get_package_config_from_repo( - project=project, - fail_when_missing=False, - ) - return url, package_config + def search_distgit_config_in_issue(self) -> Optional[Tuple[str, PackageConfig]]: + """Get a tuple (dist-git repo url, package config loaded from dist-git yaml file). + Look up for a dist-git repo url inside + the issue description for the issue comment event. + The issue description should have a format like the following: + + Packit failed on creating pull-requests in dist-git (https://src.fedoraproject.org/rpms/python-teamcity-messages): # noqa + | dist-git branch | error | + | --------------- | ----- | + | `f37` | `` | + You can retrigger the update by adding a comment (`/packit propose-downstream`) into this issue. + + Returns: + A tuple (dist_git_repo_url, dist_git_package_config) or None + """ + if isinstance(self.event, AbstractIssueCommentEvent): + issue = self.event.project.get_issue(self.event.issue_id) + if m := match(r"[\w\s-]+dist-git \((\S+)\):", issue.description): + url = m[1] + project = self.service_config.get_project(url=url) + package_config = PackageConfigGetter.get_package_config_from_repo( + project=project, + fail_when_missing=False, + ) + return url, package_config + return None def is_packit_config_present(self) -> bool: """ From 901f33fbcba0070254508c77528958407e91a53f Mon Sep 17 00:00:00 2001 From: Maja Massarini Date: Wed, 11 Jan 2023 15:52:27 +0100 Subject: [PATCH 10/10] Create issues with a similar format MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A propose-downstream, a Bodhi update or a Koji build job should create issues with similar format to be parsable by any job reacting to an AbstractIssueCommentEvent. Co-authored-by: Laura Barcziová <49026743+lbarcziova@users.noreply.github.com> --- packit_service/constants.py | 7 + packit_service/worker/handlers/bodhi.py | 94 ++++++-- packit_service/worker/handlers/distgit.py | 77 ++++-- tests/integration/test_bodhi_update.py | 27 ++- tests/integration/test_issue_comment.py | 56 +++++ tests/integration/test_koji_build.py | 68 ++++++ tests/unit/test_bodhi_update_error_msgs.py | 263 +++++++++++++++++++++ 7 files changed, 545 insertions(+), 47 deletions(-) create mode 100644 tests/unit/test_bodhi_update_error_msgs.py diff --git a/packit_service/constants.py b/packit_service/constants.py index c103d93b0..e6c3d2446 100644 --- a/packit_service/constants.py +++ b/packit_service/constants.py @@ -26,6 +26,13 @@ TESTING_FARM_INSTALLABILITY_TEST_URL = "https://gitlab.com/testing-farm/tests" TESTING_FARM_INSTALLABILITY_TEST_REF = "main" +MSG_DOWNSTREAM_JOB_ERROR_HEADER = ( + "Packit failed on creating {object} in dist-git " + "({dist_git_url}):\n\n" + "| dist-git branch | error |\n" + "| --------------- | ----- |\n" +) + MSG_GET_IN_TOUCH = ( f"\n\n---\n\n*Get in [touch with us]({CONTACTS_URL}) if you need some help.*" ) diff --git a/packit_service/worker/handlers/bodhi.py b/packit_service/worker/handlers/bodhi.py index b65d98c7f..6ac84975a 100644 --- a/packit_service/worker/handlers/bodhi.py +++ b/packit_service/worker/handlers/bodhi.py @@ -5,6 +5,7 @@ This file defines classes for job handlers related to Bodhi """ import logging +import abc from typing import Tuple, Type from celery import Task @@ -13,7 +14,9 @@ from packit.config import JobConfig, JobType, PackageConfig from packit.exceptions import PackitException from packit_service.constants import ( - CONTACTS_URL, + MSG_RETRIGGER, + MSG_GET_IN_TOUCH, + MSG_DOWNSTREAM_JOB_ERROR_HEADER, RETRY_INTERVAL_IN_MINUTES_WHEN_USER_ACTION_IS_NEEDED, ) from packit_service.worker.checker.abstract import Checker @@ -76,18 +79,17 @@ def __init__( def run(self) -> TaskResults: try: - branches = [] + koji_build_data = None for koji_build_data in self: - branches.append(koji_build_data.dist_git_branch) logger.debug( - f"Create update for dist-git branch: {koji_build_data.dist_git_branch}" + f"Create update for dist-git branch: {koji_build_data.dist_git_branch} " + f"and nvr: {koji_build_data.nvr}." ) self.packit_api.create_update( dist_git_branch=koji_build_data.dist_git_branch, update_type="enhancement", koji_builds=[koji_build_data.nvr], # it accepts NVRs, not build IDs ) - logger.debug(f"Iterated over all dist-git branches: {branches}") except PackitException as ex: logger.debug(f"Bodhi update failed to be created: {ex}") @@ -96,24 +98,14 @@ def run(self) -> TaskResults: notify = True known_error = True else: - body = ( - f"Bodhi update creation failed for `{koji_build_data.nvr}`:\n" - "```\n" - f"{ex}\n" - "```" - ) + body = f"``` {ex} ```" # Notify user just on the last run. notify = self.celery_task.is_last_try() known_error = False if notify: - report_in_issue_repository( - issue_repository=self.job_config.issue_repository, - service_config=self.service_config, - title="Fedora Bodhi update failed to be created", - message=body - + f"\n\n---\n\n*Get in [touch with us]({CONTACTS_URL}) if you need some help.*", - comment_to_existing=body, + self.report_in_issue_repository( + koji_build_data=koji_build_data, error=body ) else: logger.debug("User will not be notified about the failure.") @@ -127,26 +119,66 @@ def run(self) -> TaskResults: # Sentry issue will be created otherwise. return TaskResults(success=True, details={}) + @abc.abstractmethod + def get_trigger_type_description(self, koji_build_data: KojiBuildData) -> str: + """Describe the user's action which triggered the Bodhi update + + Args: + koji_build_data: koji build data associated with the + retriggered Bodhi update + """ + + def report_in_issue_repository( + self, koji_build_data: KojiBuildData, error: str + ) -> None: + + body = MSG_DOWNSTREAM_JOB_ERROR_HEADER.format( + object="Bodhi update", dist_git_url=self.packit_api.dg.local_project.git_url + ) + if koji_build_data: + body += f"| `{koji_build_data.dist_git_branch}` | {error} |\n" + else: + body += f"| | {error} |\n" + + msg_retrigger = MSG_RETRIGGER.format( + job="update", + command="create-update", + place="issue", + packit_comment_command_prefix=self.service_config.comment_command_prefix, + ) + trigger_type_description = self.get_trigger_type_description(koji_build_data) + body_msg = ( + f"{body}\n{trigger_type_description}\n\n{msg_retrigger}{MSG_GET_IN_TOUCH}\n" + ) + + report_in_issue_repository( + issue_repository=self.job_config.issue_repository, + service_config=self.service_config, + title="Fedora Bodhi update failed to be created", + message=body_msg, + comment_to_existing=body_msg, + ) + def _error_message_for_auth_error( self, ex: PackitException, koji_build_data: KojiBuildData ) -> str: body = ( f"Bodhi update creation failed for `{koji_build_data.nvr}` " - f"because of the missing permissions.\n\n" + f"because of the missing permissions. " f"Please, give {self.service_config.fas_user} user `commit` rights in the " - f"[dist-git settings]({self.data.project_url}/adduser).\n\n" + f"[dist-git settings]({self.data.project_url}/adduser). " ) body += ( f"*Try {self.celery_task.retries + 1}/" - f"{self.celery_task.get_retry_limit() + 1}" + f"{self.celery_task.get_retry_limit() + 1}." ) # Notify user on each task run and set a more generous retry interval # to let the user fix this issue in the meantime. if not self.celery_task.is_last_try(): body += ( - f": Task will be retried in " + f" Task will be retried in " f"{RETRY_INTERVAL_IN_MINUTES_WHEN_USER_ACTION_IS_NEEDED} minutes.*" ) self.celery_task.retry( @@ -181,6 +213,12 @@ def get_checkers() -> Tuple[Type[Checker], ...]: logger.debug("Bodhi update will be re-triggered via dist-git PR comment.") return (IsKojiBuildCompleteAndBranchConfiguredCheckEvent,) + def get_trigger_type_description(self, koji_build_data: KojiBuildData) -> str: + return ( + f"Fedora Bodhi update was triggered by " + f"Koji build {koji_build_data.nvr}." + ) + @configured_as(job_type=JobType.bodhi_update) @reacts_to(event=PullRequestCommentPagureEvent) @@ -206,6 +244,12 @@ def get_checkers() -> Tuple[Type[Checker], ...]: IsKojiBuildCompleteAndBranchConfiguredCheckService, ) + def get_trigger_type_description(self, _: KojiBuildData) -> str: + return ( + f"Fedora Bodhi update was re-triggered " + f"by comment in dist-git PR with id {self.data.pr_id}." + ) + @configured_as(job_type=JobType.bodhi_update) @reacts_to(event=IssueCommentEvent) @@ -230,3 +274,9 @@ def get_checkers() -> Tuple[Type[Checker], ...]: """ logger.debug("Bodhi update will be re-triggered via dist-git PR comment.") return (HasIssueCommenterRetriggeringPermissions,) + + def get_trigger_type_description(self, _: KojiBuildData) -> str: + return ( + f"Fedora Bodhi update was re-triggered by " + f"comment in issue {self.data.issue_id}." + ) diff --git a/packit_service/worker/handlers/distgit.py b/packit_service/worker/handlers/distgit.py index 73633ab06..4df53c122 100644 --- a/packit_service/worker/handlers/distgit.py +++ b/packit_service/worker/handlers/distgit.py @@ -21,6 +21,8 @@ from packit_service.constants import ( CONTACTS_URL, MSG_RETRIGGER, + MSG_GET_IN_TOUCH, + MSG_DOWNSTREAM_JOB_ERROR_HEADER, ) from packit_service.models import ( SyncReleaseTargetStatus, @@ -338,13 +340,11 @@ def run(self) -> TaskResults: ): err_without_new_lines = err.replace("\n", " ") branch_errors += f"| `{branch}` | `{err_without_new_lines}` |\n" - body_msg = ( - f"Packit failed on creating pull-requests in dist-git " - f"({self.packit_api.dg.local_project.git_url}):\n\n" - f"| dist-git branch | error |\n" - f"| --------------- | ----- |\n" - f"{branch_errors}\n\n" + body_msg = MSG_DOWNSTREAM_JOB_ERROR_HEADER.format( + object="pull-requests", + dist_git_url=self.packit_api.dg.local_project.git_url, ) + body_msg += f"{branch_errors}\n\n" self._report_errors_for_each_branch(body_msg) sync_release_run_model.set_status(status=SyncReleaseStatus.error) return TaskResults( @@ -492,8 +492,8 @@ def get_branches(self) -> List[str]: """Get a list of branch (names) to be built in koji""" def run(self) -> TaskResults: - branch = None try: + branch = None for branch in self.get_branches(): self.packit_api.build( dist_git_branch=branch, @@ -508,21 +508,42 @@ def run(self) -> TaskResults: ) raise ex - body = f"Koji build on `{branch}` branch failed:\n" "```\n" f"{ex}\n" "```" - - report_in_issue_repository( - issue_repository=self.job_config.issue_repository, - service_config=self.service_config, - title="Fedora Koji build failed to be triggered", - message=body - + f"\n\n*Get in [touch with us]({CONTACTS_URL}) if you need some help.*", - comment_to_existing=body, - ) - + self.report_in_issue_repository(branch, ex) raise ex return TaskResults(success=True, details={}) + @abc.abstractmethod + def get_trigger_type_description(self) -> str: + """Describe the user's action which triggered the Koji build""" + + def report_in_issue_repository(self, branch: str, ex: PackitException) -> None: + + body = MSG_DOWNSTREAM_JOB_ERROR_HEADER.format( + object="Koji build", dist_git_url=self.packit_api.dg.local_project.git_url + ) + body += f"| `{branch}` | ```{ex}``` |\n" + + msg_retrigger = MSG_RETRIGGER.format( + job="build", + command="koji-build", + place="issue", + packit_comment_command_prefix=self.service_config.comment_command_prefix, + ) + + trigger_type_description = self.get_trigger_type_description() + body_msg = ( + f"{body}\n{trigger_type_description}\n\n{msg_retrigger}{MSG_GET_IN_TOUCH}\n" + ) + + report_in_issue_repository( + issue_repository=self.job_config.issue_repository, + service_config=self.service_config, + title="Fedora Koji build failed to be triggered", + message=body_msg, + comment_to_existing=body_msg, + ) + @configured_as(job_type=JobType.koji_build) @run_for_comment(command="koji-build") @@ -545,6 +566,20 @@ def get_branches(self) -> List[str]: ) return [branch] + def get_trigger_type_description(self) -> str: + trigger_type_description = "" + if self.data.event_type == PullRequestCommentPagureEvent.__name__: + trigger_type_description += ( + f"Fedora Koji build was re-triggered " + f"by comment in dist-git PR id {self.data.pr_id}." + ) + elif self.data.event_type == PushPagureEvent.__name__: + trigger_type_description += ( + f"Fedora Koji build was triggered " + f"by push with sha {self.data.commit_sha}." + ) + return trigger_type_description + @configured_as(job_type=JobType.koji_build) @run_for_comment(command="koji-build") @@ -561,3 +596,9 @@ class RetriggerDownstreamKojiBuildHandler( def get_branches(self) -> List[str]: return self.branches + + def get_trigger_type_description(self) -> str: + return ( + f"Fedora Koji build was re-triggered " + f"by comment in issue {self.data.issue_id}." + ) diff --git a/tests/integration/test_bodhi_update.py b/tests/integration/test_bodhi_update.py index 48af6e56e..398137777 100644 --- a/tests/integration/test_bodhi_update.py +++ b/tests/integration/test_bodhi_update.py @@ -205,6 +205,8 @@ def test_bodhi_update_for_unknown_koji_build_failed_issue_created( flexmock(LocalProject, refresh_the_arguments=lambda: None) # 1*CreateBodhiUpdateHandler + 1*KojiBuildReportHandler flexmock(Signature).should_receive("apply_async").times(2) + dg = flexmock(local_project=flexmock(git_url="an url")) + flexmock(PackitAPI).should_receive("dg").and_return(dg) flexmock(PackitAPI).should_receive("create_update").with_args( dist_git_branch="rawhide", update_type="enhancement", @@ -288,6 +290,8 @@ def test_bodhi_update_for_unknown_koji_build_failed_issue_comment( flexmock(LocalProject, refresh_the_arguments=lambda: None) # 1*CreateBodhiUpdateHandler + 1*KojiBuildReportHandler flexmock(Signature).should_receive("apply_async").times(2) + dg = flexmock(local_project=flexmock(git_url="an url")) + flexmock(PackitAPI).should_receive("dg").and_return(dg) flexmock(PackitAPI).should_receive("create_update").with_args( dist_git_branch="rawhide", update_type="enhancement", @@ -486,18 +490,27 @@ def test_bodhi_update_auth_error( update_type="enhancement", koji_builds=["packit-0.43.0-1.fc36"], ).and_raise(bodhi_exception) + dg = flexmock(local_project=flexmock(git_url="https://src.fedoraproject.org/rpms")) + flexmock(PackitAPI).should_receive("dg").and_return(dg) issue_project_mock = flexmock(GithubProject) issue_project_mock.should_receive("get_issue_list").and_return([]).once() issue_project_mock.should_receive("create_issue").with_args( title="[packit] Fedora Bodhi update failed to be created", - body="Bodhi update creation failed for `packit-0.43.0-1.fc36` " - "because of the missing permissions.\n\n" - "Please, give packit user `commit` rights " - "in the [dist-git settings](https://src.fedoraproject.org/rpms/packit/adduser).\n\n" - "*Try 1/6: Task will be retried in 10 minutes.*\n\n" - "---\n\n" - "*Get in [touch with us](https://packit.dev/#contact) if you need some help.*", + body=( + "Packit failed on creating Bodhi update in dist-git " + "(https://src.fedoraproject.org/rpms):\n\n" + "| dist-git branch | error |\n| --------------- | ----- |\n" + "| `rawhide` | Bodhi update creation failed for `packit-0.43.0-1.fc36` " + "because of the missing permissions. " + "Please, give packit user `commit` rights in the [dist-git settings]" + "(https://src.fedoraproject.org/rpms/packit/adduser). *Try 1/6. " + "Task will be retried in 10 minutes.* |\n\n" + "Fedora Bodhi update was triggered by Koji build packit-0.43.0-1.fc36.\n\n" + "You can retrigger the update by adding a comment (`/packit create-update`) " + "into this issue.\n\n---\n\n*" + "Get in [touch with us](https://packit.dev/#contact) if you need some help.*\n" + ), ).and_return( flexmock(id=3, url="https://github.com/namespace/project/issues/3") ).once() diff --git a/tests/integration/test_issue_comment.py b/tests/integration/test_issue_comment.py index d020a93a1..fe64d61a7 100644 --- a/tests/integration/test_issue_comment.py +++ b/tests/integration/test_issue_comment.py @@ -17,8 +17,10 @@ from packit.api import PackitAPI from packit.config import JobConfigTriggerType from packit.distgit import DistGit +from packit.exceptions import PackitException from packit.local_project import LocalProject from packit.utils.koji_helper import KojiHelper +from packit_service.config import ServiceConfig from packit_service.constants import COMMENT_REACTION, TASK_ACCEPTED from packit_service.models import ( IssueModel, @@ -32,10 +34,12 @@ ) from packit_service.service.urls import get_propose_downstream_info_url from packit_service.worker.allowlist import Allowlist +from packit_service.worker.celery_task import CeleryTask from packit_service.worker.events import IssueCommentEvent, IssueCommentGitlabEvent from packit_service.worker.helpers.sync_release.propose_downstream import ( ProposeDownstreamJobHelper, ) +from packit_service.worker.handlers import distgit from packit_service.worker.handlers.distgit import ( RetriggerDownstreamKojiBuildHandler, ) @@ -391,3 +395,55 @@ def test_issue_comment_retrigger_koji_build_handler( ) assert first_dict_value(results["job"])["success"] + + +def test_issue_comment_retrigger_koji_build_error_msg( + mock_repository_issue_retriggering, + github_repository_issue_comment_retrigger_koji_build, +): + processing_results = SteveJobs().process_message( + github_repository_issue_comment_retrigger_koji_build + ) + event_dict, _, job_config, package_config = get_parameters_from_results( + processing_results + ) + assert json.dumps(event_dict) + + flexmock(CeleryTask).should_receive("is_last_try").and_return(True) + error_msg = "error abc" + dg = flexmock(local_project=flexmock(git_url="an url")) + packit_api = ( + flexmock(dg=dg) + .should_receive("build") + .and_raise(PackitException, error_msg) + .mock() + ) + # flexmock(JobConfig).should_receive("issue_repository").and_return( + # "a repo" + # ) + flexmock(RetriggerDownstreamKojiBuildHandler).should_receive( + "packit_api" + ).and_return(packit_api) + msg = ( + "Packit failed on creating Koji build in dist-git (an url):" + "\n\n| dist-git branch | error |\n| --------------- | ----- |\n" + "| `f37` | ```error abc``` |\n\n" + "Fedora Koji build was re-triggered by comment in issue 1.\n\n" + "You can retrigger the build by adding a comment " + "(`/packit koji-build`) into this issue.\n\n" + "---\n\n*Get in [touch with us](https://packit.dev/#contact) if you need some help.*\n" + ) + flexmock(distgit).should_receive("report_in_issue_repository").with_args( + issue_repository=None, + service_config=ServiceConfig, + title=("Fedora Koji build failed to be triggered"), + message=msg, + comment_to_existing=msg, + ).once() + + with pytest.raises(PackitException): + run_retrigger_downstream_koji_build( + package_config=package_config, + event=event_dict, + job_config=job_config, + ) diff --git a/tests/integration/test_koji_build.py b/tests/integration/test_koji_build.py index 66bccae41..06650a958 100644 --- a/tests/integration/test_koji_build.py +++ b/tests/integration/test_koji_build.py @@ -7,14 +7,28 @@ from celery.canvas import Signature from flexmock import flexmock +from ogr.services.github import GithubProject from ogr.services.pagure import PagureProject +from packit.exceptions import PackitException from packit.config import JobConfigTriggerType +from packit_service.config import PackageConfigGetter from packit_service.models import GitBranchModel, KojiBuildTargetModel, PipelineModel from packit_service.worker.jobs import SteveJobs from packit_service.worker.monitoring import Pushgateway +from packit_service.worker.celery_task import CeleryTask from packit_service.worker.tasks import ( + run_downstream_koji_build, run_downstream_koji_build_report, ) +from packit_service.worker.handlers.distgit import ( + DownstreamKojiBuildHandler, +) +from packit_service.models import ( + JobTriggerModelType, +) +from packit_service.worker.events.pagure import ( + PushPagureEvent, +) from tests.conftest import koji_build_completed_rawhide, koji_build_start_rawhide from tests.spellbook import first_dict_value, get_parameters_from_results @@ -179,3 +193,57 @@ def test_downstream_koji_build_report_unknown_build(koji_build_fixture, request) ) assert first_dict_value(results["job"])["success"] + + +def test_koji_build_error_msg(distgit_push_packit): + db_trigger = flexmock( + id=123, + job_config_trigger_type=JobConfigTriggerType.commit, + job_trigger_model_type=JobTriggerModelType.release, + ) + flexmock(PushPagureEvent).should_receive("db_trigger").and_return(db_trigger) + flexmock(DownstreamKojiBuildHandler).should_receive("pre_check").and_return(True) + flexmock(Signature).should_receive("apply_async").once() + + processing_results = SteveJobs().process_message(distgit_push_packit) + event_dict, _, job_config, package_config = get_parameters_from_results( + processing_results + ) + assert json.dumps(event_dict) + + flexmock(CeleryTask).should_receive("is_last_try").and_return(True) + error_msg = "error abc" + dg = flexmock(local_project=flexmock(git_url="an url")) + packit_api = ( + flexmock(dg=dg) + .should_receive("build") + .and_raise(PackitException, error_msg) + .mock() + ) + flexmock(DownstreamKojiBuildHandler).should_receive("packit_api").and_return( + packit_api + ) + msg = ( + "Packit failed on creating Koji build in dist-git (an url):" + "\n\n| dist-git branch | error |\n| --------------- | ----- |" + "\n| `f36` | ```error abc``` |\n\n" + "Fedora Koji build was triggered by push " + "with sha ad0c308af91da45cf40b253cd82f07f63ea9cbbf." + "\n\nYou can retrigger the build by adding a comment " + "(`/packit koji-build`) into this issue." + "\n\n---\n\n*Get in [touch with us]" + "(https://packit.dev/#contact) if you need some help.*\n" + ) + flexmock(PackageConfigGetter).should_receive("create_issue_if_needed").with_args( + project=GithubProject, + title=("Fedora Koji build failed to be triggered"), + message=msg, + comment_to_existing=msg, + ).once() + + with pytest.raises(PackitException): + run_downstream_koji_build( + package_config=package_config, + event=event_dict, + job_config=job_config, + ) diff --git a/tests/unit/test_bodhi_update_error_msgs.py b/tests/unit/test_bodhi_update_error_msgs.py new file mode 100644 index 000000000..1d1257469 --- /dev/null +++ b/tests/unit/test_bodhi_update_error_msgs.py @@ -0,0 +1,263 @@ +import pytest + +from flexmock import flexmock +from fedora.client import AuthError + +from packit.exceptions import PackitException +from packit.config import ( + JobConfig, + JobConfigTriggerType, + JobType, + PackageConfig, + CommonPackageConfig, +) +from packit_service.config import ServiceConfig +from packit_service.worker.handlers import bodhi +from packit_service.worker.celery_task import CeleryTask +from packit_service.worker.events.enums import PullRequestAction +from packit_service.worker.events import ( + PullRequestCommentPagureEvent, + IssueCommentEvent, +) +from packit_service.worker.handlers.bodhi import ( + RetriggerBodhiUpdateHandler, + IssueCommentRetriggerBodhiUpdateHandler, +) +from packit_service.worker.handlers.mixin import KojiBuildData + + +@pytest.fixture(scope="module") +def package_config__job_config(): + package_config = PackageConfig( + packages={ + "package": CommonPackageConfig( + identifier="first", + ) + }, + jobs=[ + JobConfig( + type=JobType.bodhi_update, + trigger=JobConfigTriggerType.pull_request, + packages={ + "package": CommonPackageConfig( + identifier="first", + ) + }, + ), + ], + ) + job_config = JobConfig( + type=JobType.bodhi_update, + trigger=JobConfigTriggerType.pull_request, + packages={ + "package": CommonPackageConfig( + identifier="first", + ) + }, + ) + return package_config, job_config + + +@pytest.fixture(scope="module") +def package_config__job_config__pull_request_event(package_config__job_config): + package_config, job_config = package_config__job_config + flexmock(PullRequestCommentPagureEvent).should_receive("commit_sha").and_return( + "abcdef" + ) + data = PullRequestCommentPagureEvent( + pr_id=123, + action=PullRequestAction.opened, + base_repo_namespace="a_namespace", + base_repo_name="a_repo_name", + base_repo_owner="a_owner", + target_repo="a_target", + project_url="projec_url", + user_login="usr_login", + comment="/packit creat-update", + comment_id=321, + base_ref="abcdef", + ).get_dict() + return package_config, job_config, data + + +def test_pull_request_retrigger_bodhi_update_no_koji_data( + package_config__job_config__pull_request_event, +): + package_config, job_config, data = package_config__job_config__pull_request_event + + msg = ( + "Packit failed on creating Bodhi update " + "in dist-git (an url):\n\n" + "| dist-git branch | error |\n" + "| --------------- | ----- |\n" + "| | ``` error abc ``` |\n\n" + "Fedora Bodhi update was re-triggered by comment in dist-git PR with id 123.\n\n" + "You can retrigger the update by adding a comment (`/packit create-update`) " + "into this issue.\n\n---\n\n" + "*Get in [touch with us](https://packit.dev/#contact) if you need some help.*\n" + ) + flexmock(bodhi).should_receive("report_in_issue_repository").with_args( + issue_repository=None, + service_config=ServiceConfig, + title=("Fedora Bodhi update failed to be created"), + message=msg, + comment_to_existing=msg, + ).once() + + error_msg = "error abc" + dg = flexmock(local_project=flexmock(git_url="an url")) + packit_api = flexmock(dg=dg) + flexmock(RetriggerBodhiUpdateHandler).should_receive("packit_api").and_return( + packit_api + ) + flexmock(RetriggerBodhiUpdateHandler).should_receive("__next__").and_raise( + PackitException, error_msg + ) + flexmock(CeleryTask).should_receive("is_last_try").and_return(True) + handler = RetriggerBodhiUpdateHandler(package_config, job_config, data, flexmock()) + with pytest.raises(PackitException): + handler.run() + + +def test_pull_request_retrigger_bodhi_update_with_koji_data( + package_config__job_config__pull_request_event, +): + package_config, job_config, data = package_config__job_config__pull_request_event + + msg = ( + "Packit failed on creating Bodhi update " + "in dist-git (an url):\n\n" + "| dist-git branch | error |\n" + "| --------------- | ----- |\n" + "| `f36` | ``` error abc ``` |\n\n" + "Fedora Bodhi update was re-triggered by comment in dist-git PR with id 123.\n\n" + "You can retrigger the update by adding a comment (`/packit create-update`) " + "into this issue.\n\n---\n\n" + "*Get in [touch with us](https://packit.dev/#contact) if you need some help.*\n" + ) + flexmock(bodhi).should_receive("report_in_issue_repository").with_args( + issue_repository=None, + service_config=ServiceConfig, + title=("Fedora Bodhi update failed to be created"), + message=msg, + comment_to_existing=msg, + ).once() + + error_msg = "error abc" + dg = flexmock(local_project=flexmock(git_url="an url")) + packit_api = ( + flexmock(dg=dg) + .should_receive("create_update") + .and_raise(PackitException, error_msg) + .mock() + ) + flexmock(RetriggerBodhiUpdateHandler).should_receive("packit_api").and_return( + packit_api + ) + flexmock(RetriggerBodhiUpdateHandler).should_receive("__next__").and_return( + KojiBuildData(dist_git_branch="f36", build_id=1, nvr="a_package_1.f36", state=1) + ) + flexmock(CeleryTask).should_receive("is_last_try").and_return(True) + handler = RetriggerBodhiUpdateHandler(package_config, job_config, data, flexmock()) + with pytest.raises(PackitException): + handler.run() + + +def test_pull_request_retrigger_bodhi_update_auth_err( + package_config__job_config__pull_request_event, +): + package_config, job_config, data = package_config__job_config__pull_request_event + + msg = ( + "Packit failed on creating Bodhi update in dist-git (an url):\n\n" + "| dist-git branch | error |\n| --------------- | ----- |\n" + "| `f36` | Bodhi update creation failed for `a_package_1.f36` " + "because of the missing permissions. Please, give packit user `commit`" + " rights in the [dist-git settings](projec_url/adduser). *Try 2/2.* |\n\n" + "Fedora Bodhi update was re-triggered by comment in dist-git PR with id 123.\n\n" + "You can retrigger the update by adding a comment (`/packit create-update`) " + "into this issue.\n\n---\n\n" + "*Get in [touch with us](https://packit.dev/#contact) if you need some help.*\n" + ) + flexmock(bodhi).should_receive("report_in_issue_repository").with_args( + issue_repository=None, + service_config=ServiceConfig, + title=("Fedora Bodhi update failed to be created"), + message=msg, + comment_to_existing=msg, + ).once() + + error_msg = "error abc" + dg = flexmock(local_project=flexmock(git_url="an url")) + packit_api = ( + flexmock(dg=dg) + .should_receive("create_update") + .and_raise(PackitException, error_msg) + .mock() + ) + flexmock(RetriggerBodhiUpdateHandler).should_receive("packit_api").and_return( + packit_api + ) + flexmock(RetriggerBodhiUpdateHandler).should_receive("__next__").and_return( + KojiBuildData(dist_git_branch="f36", build_id=1, nvr="a_package_1.f36", state=1) + ) + flexmock(CeleryTask).should_receive("is_last_try").and_return(True) + flexmock(CeleryTask).should_receive("retries").and_return(1) + flexmock(CeleryTask).should_receive("get_retry_limit").and_return(1) + flexmock(PackitException).should_receive("__cause__").and_return( + AuthError("another_error") + ) + handler = RetriggerBodhiUpdateHandler(package_config, job_config, data, flexmock()) + handler.run() + + +def test_issue_comment_retrigger_bodhi_update_no_koji_data(package_config__job_config): + package_config, job_config = package_config__job_config + flexmock(IssueCommentEvent).should_receive("tag_name").and_return("1") + flexmock(IssueCommentEvent).should_receive("commit_sha").and_return("abcdef") + data = IssueCommentEvent( + issue_id=123, + action=PullRequestAction.opened, + repo_namespace="a_namespace", + repo_name="a_repo_name", + target_repo="a_target", + project_url="projec_url", + actor="actor", + comment="/packit creat-update", + comment_id=321, + ).get_dict() + + msg = ( + "Packit failed on creating Bodhi update " + "in dist-git (an url):\n\n" + "| dist-git branch | error |\n" + "| --------------- | ----- |\n" + "| | ``` error abc ``` |\n\n" + "Fedora Bodhi update was re-triggered by comment in issue 123.\n\n" + "You can retrigger the update by adding a comment (`/packit create-update`) " + "into this issue.\n\n---\n\n" + "*Get in [touch with us](https://packit.dev/#contact) if you need some help.*\n" + ) + flexmock(bodhi).should_receive("report_in_issue_repository").with_args( + issue_repository=None, + service_config=ServiceConfig, + title=("Fedora Bodhi update failed to be created"), + message=msg, + comment_to_existing=msg, + ).once() + + error_msg = "error abc" + dg = flexmock(local_project=flexmock(git_url="an url")) + packit_api = flexmock(dg=dg) + flexmock(IssueCommentRetriggerBodhiUpdateHandler).should_receive( + "packit_api" + ).and_return(packit_api) + flexmock(IssueCommentRetriggerBodhiUpdateHandler).should_receive( + "__next__" + ).and_raise(PackitException, error_msg) + flexmock(CeleryTask).should_receive("is_last_try").and_return(True) + handler = IssueCommentRetriggerBodhiUpdateHandler( + package_config, job_config, data, flexmock() + ) + with pytest.raises(PackitException): + handler.run()