From 1baf0162472304ce9d98004bb6e5489ff524d924 Mon Sep 17 00:00:00 2001 From: Jiri Kyjovsky Date: Wed, 14 Sep 2022 13:26:42 +0200 Subject: [PATCH] Feat(bodhi): Retrigger bodhi update via dist-git PR command --- packit_service/worker/handlers/bodhi.py | 141 +++++++++++++++--------- tests/conftest.py | 6 + tests/integration/test_pr_comment.py | 93 ++++++++++++++++ tests/unit/test_bodhi_update.py | 48 ++++++++ tests/unit/test_events.py | 5 - 5 files changed, 238 insertions(+), 55 deletions(-) create mode 100644 tests/unit/test_bodhi_update.py diff --git a/packit_service/worker/handlers/bodhi.py b/packit_service/worker/handlers/bodhi.py index 452196f4e..f19f41d97 100644 --- a/packit_service/worker/handlers/bodhi.py +++ b/packit_service/worker/handlers/bodhi.py @@ -36,6 +36,7 @@ configured_as, reacts_to, RetriableJobHandler, + run_for_comment, ) from packit_service.worker.result import TaskResults @@ -51,6 +52,8 @@ class KojiBuildEventWrapper: @configured_as(job_type=JobType.bodhi_update) +@run_for_comment(command="create-update") +@reacts_to(event=PullRequestCommentPagureEvent) @reacts_to(event=KojiBuildEvent) class CreateBodhiUpdateHandler(RetriableJobHandler): """ @@ -75,7 +78,9 @@ def __init__( ) # lazy properties - self._koji_build_event_data: Optional[KojiBuildEvent] = None + self._koji_build_event_data: Optional[KojiBuildEventWrapper] = None + self._packit_api: Optional[PackitAPI] = None + self._local_project: Optional[LocalProject] = None def _build_from_event_dict(self) -> KojiBuildEventWrapper: koji_build_event = KojiBuildEvent.from_event_dict(self.data.event_dict) @@ -114,6 +119,52 @@ def koji_build_event_data(self) -> Optional[KojiBuildEventWrapper]: return self._koji_build_event_data + @property + def packit_api(self) -> Optional[PackitAPI]: + if self._packit_api is None: + self._packit_api = PackitAPI( + self.service_config, + self.job_config, + downstream_local_project=self.local_project, + ) + + return self._packit_api + + @property + def local_project(self) -> LocalProject: + if self._local_project is None: + self._local_project = LocalProject( + git_project=self.project, + working_dir=self.service_config.command_handler_work_dir, + cache=RepositoryCache( + cache_path=self.service_config.repository_cache, + add_new=self.service_config.add_repositories_to_repository_cache, + ) + if self.service_config.repository_cache + else None, + ) + + return self._local_project + + def _write_access_to_dist_git_repo_is_not_needed_or_satisfied(self) -> bool: + if self.data.event_type != PullRequestCommentPagureEvent.__name__: + # not needed + return True + + user = self.data.actor + logger.debug( + f"Bodhi update will be re-triggered via dist-git PR comment by user {user}." + ) + + if not self.project.has_write_access(user=user): + 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: {user}." + ) + return False + + return True + def pre_check(self) -> bool: """ We react only on finished builds (=KojiBuildState.complete) @@ -121,15 +172,15 @@ def pre_check(self) -> bool: By default, we use `fedora-stable` alias. (Rawhide updates are already created automatically.) """ - if self.koji_build_event.state != KojiBuildState.complete: + if self.koji_build_event_data.state != KojiBuildState.complete: logger.debug( - f"Skipping build '{self.koji_build_event.build_id}' " - f"on '{self.koji_build_event.git_ref}'. " + f"Skipping build '{self.koji_build_event_data.build_id}' " + f"on '{self.koji_build_event_data.dist_git_branch}'. " f"Build not finished yet." ) return False - if self.koji_build_event.git_ref not in ( + if self.koji_build_event_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 @@ -140,68 +191,58 @@ def pre_check(self) -> bool: f"Bodhi update configured only for '{configured_branches}'." ) return False - return True - def run(self) -> TaskResults: - self.local_project = LocalProject( - git_project=self.project, - working_dir=self.service_config.command_handler_work_dir, - cache=RepositoryCache( - cache_path=self.service_config.repository_cache, - add_new=self.service_config.add_repositories_to_repository_cache, - ) - if self.service_config.repository_cache - else None, + return self._write_access_to_dist_git_repo_is_not_needed_or_satisfied() + + def _error_message_for_auth_error(self, packit_exception: PackitException) -> str: + body = ( + f"Bodhi update creation failed for `{self.koji_build_event_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" ) - packit_api = PackitAPI( - self.service_config, - self.job_config, - downstream_local_project=self.local_project, + + body += ( + f"*Try {self.celery_task.retries + 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"{RETRY_INTERVAL_IN_MINUTES_WHEN_USER_ACTION_IS_NEEDED} minutes.*" + ) + self.celery_task.retry( + delay=RETRY_INTERVAL_IN_MINUTES_WHEN_USER_ACTION_IS_NEEDED * 60, + ex=packit_exception, + ) + else: + body += "*" + + return body + + def run(self) -> TaskResults: try: - packit_api.create_update( - dist_git_branch=self.koji_build_event.git_ref, + self.packit_api.create_update( + dist_git_branch=self.koji_build_event_data.dist_git_branch, update_type="enhancement", update_notes=DEFAULT_BODHI_NOTE, koji_builds=[ - self.koji_build_event.nvr # it accepts NVRs, not build IDs + self.koji_build_event_data.nvr # it accepts NVRs, not build IDs ], ) except PackitException as ex: logger.debug(f"Bodhi update failed to be created: {ex}") if isinstance(ex.__cause__, AuthError): - body = ( - f"Bodhi update creation failed for `{self.koji_build_event.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" - ) - - body += ( - f"*Try {self.celery_task.retries + 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"{RETRY_INTERVAL_IN_MINUTES_WHEN_USER_ACTION_IS_NEEDED} minutes.*" - ) - self.celery_task.retry( - delay=RETRY_INTERVAL_IN_MINUTES_WHEN_USER_ACTION_IS_NEEDED * 60, - ex=ex, - ) - else: - body += "*" - + body = self._error_message_for_auth_error(packit_exception=ex) notify = True known_error = True else: body = ( - f"Bodhi update creation failed for `{self.koji_build_event.nvr}`:\n" + f"Bodhi update creation failed for `{self.koji_build_event_data.nvr}`:\n" "```\n" f"{ex}\n" "```" diff --git a/tests/conftest.py b/tests/conftest.py index ba9b5102f..0207180b1 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -411,3 +411,9 @@ def koji_build_completed_f35(): def koji_build_completed_epel8(): with open(DATA_DIR / "fedmsg" / "koji_build_completed_epel8.json", "r") as outfile: return load_the_message_from_file(outfile) + + +@pytest.fixture() +def pagure_pr_comment_added(): + with open(DATA_DIR / "fedmsg" / "pagure_pr_comment.json") as outfile: + return json.load(outfile) diff --git a/tests/integration/test_pr_comment.py b/tests/integration/test_pr_comment.py index 808dccccc..f195a4e07 100644 --- a/tests/integration/test_pr_comment.py +++ b/tests/integration/test_pr_comment.py @@ -8,11 +8,14 @@ from celery.canvas import Signature from flexmock import flexmock from github import Github +from ogr.services.pagure import PagureProject import packit_service.service.urls as urls from ogr.services.github import GithubProject from ogr.utils import RequestResponse +from packit.api import PackitAPI from packit.config import JobConfigTriggerType +from packit.constants import DEFAULT_BODHI_NOTE from packit.exceptions import PackitConfigException from packit.local_project import LocalProject from packit_service.config import ServiceConfig @@ -24,6 +27,7 @@ PG_BUILD_STATUS_SUCCESS, TASK_ACCEPTED, PG_BUILD_STATUS_FAILURE, + KojiBuildState, ) from packit_service.models import ( CoprBuildTargetModel, @@ -40,6 +44,10 @@ from packit_service.worker.allowlist import Allowlist from packit_service.worker.events.event import AbstractForgeIndependentEvent from packit_service.worker.handlers.abstract import CeleryTask +from packit_service.worker.handlers.bodhi import ( + CreateBodhiUpdateHandler, + KojiBuildEventWrapper, +) from packit_service.worker.helpers.build import copr_build from packit_service.worker.helpers.build.copr_build import CoprBuildJobHelper from packit_service.worker.helpers.build.koji_build import KojiBuildJobHelper @@ -53,6 +61,7 @@ run_copr_build_handler, run_koji_build_handler, run_testing_farm_handler, + run_bodhi_update, ) from tests.spellbook import DATA_DIR, first_dict_value, get_parameters_from_results @@ -2150,3 +2159,87 @@ def test_invalid_packit_command_without_config( processing_result["details"]["msg"] == "No packit config found in the repository." ) + + +class TestBodhiRetriggering: + def test_bodhi_update_retrigger_via_dist_git_pr_comment( + self, pagure_pr_comment_added + ): + pagure_pr_comment_added["pullrequest"]["comments"][0][ + "comment" + ] = "/packit create-update" + project = pagure_pr_comment_added["pullrequest"]["project"] + project["full_url"] = "https://src.fedoraproject.org/rpms/jouduv-dort" + project["fullname"] = "rpms/jouduv-dort" + project["name"] = "jouduv-dort" + project["url_path"] = "rpms/jouduv-dort" + + packit_yaml = ( + "{'specfile_path': 'jouduv-dort.spec', 'synced_files': []," + "'jobs': [{'trigger': 'commit', 'job': 'bodhi_update'}]," + "'downstream_package_name': 'jouduv-dort'}" + ) + + trigger = flexmock( + job_config_trigger_type=JobConfigTriggerType.pull_request, id=123 + ) + flexmock(AddPullRequestDbTrigger).should_receive("db_trigger").and_return( + trigger + ) + flexmock(PullRequestModel).should_receive("get_by_id").with_args( + 123 + ).and_return(trigger) + + pagure_project = flexmock( + PagureProject, + full_repo_name="rpms/jouduv-dort", + get_web_url=lambda: "https://src.fedoraproject.org/rpms/jouduv-dort", + default_branch="main", + ) + + pagure_project.should_receive("get_files").with_args( + ref="beaf90bcecc51968a46663f8d6f092bfdc92e682", filter_regex=r".+\.spec$" + ).and_return(["jouduv-dort.spec"]) + pagure_project.should_receive("get_file_content").with_args( + path=".distro/source-git.yaml", + ref="beaf90bcecc51968a46663f8d6f092bfdc92e682", + ).and_raise(FileNotFoundError, "Not found.") + pagure_project.should_receive("get_file_content").with_args( + path=".packit.yaml", ref="beaf90bcecc51968a46663f8d6f092bfdc92e682" + ).and_return(packit_yaml) + + flexmock(CreateBodhiUpdateHandler).should_receive("pre_check").and_return(True) + + koji_event_data = KojiBuildEventWrapper( + build_id=123, + state=KojiBuildState.complete, + dist_git_branch="f36", + nvr="name-version-release", + ) + flexmock(CreateBodhiUpdateHandler).should_receive( + "_build_koji_event_data" + ).and_return(koji_event_data) + + flexmock(LocalProject, refresh_the_arguments=lambda: None) + flexmock(Signature).should_receive("apply_async").once() + flexmock(PackitAPI).should_receive("create_update").with_args( + dist_git_branch=koji_event_data.dist_git_branch, + update_type="enhancement", + update_notes=DEFAULT_BODHI_NOTE, + koji_builds=[koji_event_data.nvr], + ).once() + + flexmock(Pushgateway).should_receive("push").once().and_return() + + processing_results = SteveJobs().process_message(pagure_pr_comment_added) + event_dict, job, job_config, package_config = get_parameters_from_results( + processing_results + ) + assert json.dumps(event_dict) + results = run_bodhi_update( + event=event_dict, + package_config=package_config, + job_config=job_config, + ) + + assert first_dict_value(results["job"])["success"] diff --git a/tests/unit/test_bodhi_update.py b/tests/unit/test_bodhi_update.py new file mode 100644 index 000000000..b2ad390a3 --- /dev/null +++ b/tests/unit/test_bodhi_update.py @@ -0,0 +1,48 @@ +import pytest + +from flexmock import flexmock + +from packit_service.worker.handlers.bodhi import CreateBodhiUpdateHandler + + +class TestBodhiHandler: + @pytest.mark.parametrize( + "event_type, has_write_access, result", + [ + pytest.param( + "PullRequestCommentPagureEvent", + True, + True, + ), + pytest.param( + "PullRequestCommentPagureEvent", + False, + False, + ), + pytest.param( + "TestingFarmHandler", + True, + True, + ), + ], + ) + def test_write_access_to_dist_git_repo_is_not_needed_or_satisfied( + self, event_type: str, has_write_access: bool, result: bool + ): + mock_data = flexmock( + event_type=event_type, + actor="happy-packit-user", + pr_id=123, + ) + mock_project = flexmock( + has_write_access=lambda user: has_write_access, + repo="playground-for-pencils", + ) + mock_self = flexmock(data=mock_data, project=mock_project) + + assert ( + result + == CreateBodhiUpdateHandler._write_access_to_dist_git_repo_is_not_needed_or_satisfied( + mock_self + ) + ) diff --git a/tests/unit/test_events.py b/tests/unit/test_events.py index 9c87f888c..69088a5e7 100644 --- a/tests/unit/test_events.py +++ b/tests/unit/test_events.py @@ -189,11 +189,6 @@ def pagure_pr_flag_updated(self): with open(DATA_DIR / "fedmsg" / "pagure_pr_flag_updated.json") as outfile: return json.load(outfile) - @pytest.fixture() - def pagure_pr_comment_added(self): - with open(DATA_DIR / "fedmsg" / "pagure_pr_comment.json") as outfile: - return json.load(outfile) - @pytest.fixture() def distgit_commit(self): with open(DATA_DIR / "fedmsg" / "distgit_commit.json") as outfile: