diff --git a/packit_service/constants.py b/packit_service/constants.py index 1c617c191..52adffc3d 100644 --- a/packit_service/constants.py +++ b/packit_service/constants.py @@ -262,3 +262,15 @@ def from_number(number: int): # shared sandcastle dir SANDCASTLE_DG_REPO_DIR = "dist-git" SANDCASTLE_LOCAL_PROJECT_DIR = "local-project" + +FAILURE_COMMENT_MESSAGE_VARIABLES = { + # placeholder name in the user customized failure message: + # default value for placeholder if not given + placeholder: f"{{no entry for {placeholder}}}" + for placeholder in ( + "commit_sha", + "packit_dashboard_url", + "external_dashboard_url", + "logs_url", + ) +} diff --git a/packit_service/worker/handlers/copr.py b/packit_service/worker/handlers/copr.py index 0fb830513..4565ecd61 100644 --- a/packit_service/worker/handlers/copr.py +++ b/packit_service/worker/handlers/copr.py @@ -304,14 +304,19 @@ def run(self): # https://pagure.io/copr/copr/blob/master/f/common/copr_common/enums.py#_42 if self.copr_event.status != COPR_API_SUCC_STATE: failed_msg = "RPMs failed to be built." + packit_dashboard_url = get_copr_build_info_url(self.build.id) self.copr_build_helper.report_status_to_all_for_chroot( state=BaseCommitStatus.failure, description=failed_msg, - url=get_copr_build_info_url(self.build.id), + url=packit_dashboard_url, chroot=self.copr_event.chroot, ) self.measure_time_after_reporting() - self.copr_build_helper.notify_about_failure_if_configured() + self.copr_build_helper.notify_about_failure_if_configured( + packit_dashboard_url=packit_dashboard_url, + external_dashboard_url=self.build.copr_web_url, + logs_url=self.build.build_logs_url, + ) self.build.set_status(BuildStatus.failure) return TaskResults(success=False, details={"msg": failed_msg}) @@ -371,7 +376,11 @@ def handle_srpm_end(self): description=failed_msg, url=url, ) - self.copr_build_helper.notify_about_failure_if_configured() + self.copr_build_helper.notify_about_failure_if_configured( + packit_dashboard_url=url, + external_dashboard_url=self.build.copr_web_url, + logs_url=self.build.build_logs_url, + ) self.build.set_status(BuildStatus.failure) self.copr_build_helper.monitor_not_submitted_copr_builds( len(self.copr_build_helper.build_targets), "srpm_failure" diff --git a/packit_service/worker/handlers/koji.py b/packit_service/worker/handlers/koji.py index 1db2de203..9da075fd4 100644 --- a/packit_service/worker/handlers/koji.py +++ b/packit_service/worker/handlers/koji.py @@ -225,19 +225,23 @@ def run(self): url=url, chroot=build.target, ) - if self.koji_task_event.state == KojiTaskState.failed: - build_job_helper.notify_about_failure_if_configured() + koji_build_logs = KojiTaskEvent.get_koji_build_logs_url( + rpm_build_task_id=int(build.build_id), + koji_logs_url=self.service_config.koji_logs_url, + ) + build.set_build_logs_url(koji_build_logs) + koji_rpm_task_web_url = KojiTaskEvent.get_koji_rpm_build_web_url( + rpm_build_task_id=int(build.build_id), + koji_web_url=self.service_config.koji_web_url, + ) + build.set_web_url(koji_rpm_task_web_url) - koji_build_logs = KojiTaskEvent.get_koji_build_logs_url( - rpm_build_task_id=int(build.build_id), - koji_logs_url=self.service_config.koji_logs_url, - ) - build.set_build_logs_url(koji_build_logs) - koji_rpm_task_web_url = KojiTaskEvent.get_koji_rpm_build_web_url( - rpm_build_task_id=int(build.build_id), - koji_web_url=self.service_config.koji_web_url, - ) - build.set_web_url(koji_rpm_task_web_url) + if self.koji_task_event.state == KojiTaskState.failed: + build_job_helper.notify_about_failure_if_configured( + packit_dashboard_url=url, + external_dashboard_url=koji_rpm_task_web_url, + logs_url=koji_build_logs, + ) msg = ( f"Build on {build.target} in koji changed state " diff --git a/packit_service/worker/handlers/testing_farm.py b/packit_service/worker/handlers/testing_farm.py index 7241280c4..621af55f8 100644 --- a/packit_service/worker/handlers/testing_farm.py +++ b/packit_service/worker/handlers/testing_farm.py @@ -423,18 +423,19 @@ def run(self) -> TaskResults: self.pushgateway.test_run_finished_time.observe(test_run_time) test_run_model.set_web_url(self.log_url) - + url = get_testing_farm_info_url(test_run_model.id) if test_run_model else None self.testing_farm_job_helper.report_status_to_tests_for_test_target( state=status, description=summary, target=test_run_model.target, - url=get_testing_farm_info_url(test_run_model.id) - if test_run_model - else self.log_url, + url=url if url else self.log_url, links_to_external_services={"Testing Farm": self.log_url}, ) if failure: - self.testing_farm_job_helper.notify_about_failure_if_configured() + self.testing_farm_job_helper.notify_about_failure_if_configured( + packit_dashboard_url=url, + logs_url=self.log_url, + ) test_run_model.set_status(self.result, created=self.created) diff --git a/packit_service/worker/helpers/build/build_helper.py b/packit_service/worker/helpers/build/build_helper.py index 50421434e..cb925e732 100644 --- a/packit_service/worker/helpers/build/build_helper.py +++ b/packit_service/worker/helpers/build/build_helper.py @@ -1,5 +1,6 @@ # Copyright Contributors to the Packit project. # SPDX-License-Identifier: MIT +import copy import datetime import logging import re @@ -26,6 +27,7 @@ GitBranchModel, ProjectEventModel, ) +from packit_service.constants import FAILURE_COMMENT_MESSAGE_VARIABLES from packit_service.service.urls import get_srpm_build_info_url from packit_service.trigger_mapping import are_job_types_same from packit_service.worker.events import EventData @@ -755,7 +757,7 @@ def report_status_to_configured_job( update_feedback_time=update_feedback_time, ) - def notify_about_failure_if_configured(self): + def notify_about_failure_if_configured(self, **kwargs): """ If there is a failure_comment_message configured for the job, post a comment and include the configured message. Do not post @@ -766,9 +768,11 @@ def notify_about_failure_if_configured(self): ): return - formatted_message = configured_message.format( - commit_sha=self.db_project_event.commit_sha - ) + all_kwargs = copy.copy(FAILURE_COMMENT_MESSAGE_VARIABLES) + all_kwargs["commit_sha"] = self.db_project_event.commit_sha + all_kwargs.update(kwargs) + formatted_message = configured_message.format(**all_kwargs) + self.status_reporter.comment( formatted_message, duplicate_check=DuplicateCheckMode.check_last_comment ) diff --git a/tests/conftest.py b/tests/conftest.py index 87794f530..8e7b29627 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -132,6 +132,8 @@ def srpm_build_model( set_build_logs_url=lambda x: None, url=None, build_start_time=None, + build_logs_url=None, + copr_web_url=None, ) flexmock(ProjectEventModel).should_receive("get_or_create").with_args( diff --git a/tests/unit/test_build_helper.py b/tests/unit/test_build_helper.py index 23f672fde..20fdb782c 100644 --- a/tests/unit/test_build_helper.py +++ b/tests/unit/test_build_helper.py @@ -2865,7 +2865,53 @@ def test_local_project_not_called_when_initializing_api(): assert copr_build_helper.api.copr_helper -def test_notify_about_failure_if_configured(): +@pytest.mark.parametrize( + "failure_comment,kwargs,result_comment", + [ + pytest.param( + ("One of the Copr builds failed for " "commit {commit_sha}, ping @admin"), + {}, + "One of the Copr builds failed for commit 123, ping @admin", + id="only commit_sha", + ), + pytest.param( + ( + "One of the Copr builds failed for " + "commit {commit_sha}, ping @admin, copr build logs {logs_url}" + ), + {"logs_url": "jghfkgjfd"}, + "One of the Copr builds failed for commit 123, ping @admin, copr build logs jghfkgjfd", + id="commit_sha and logs url", + ), + pytest.param( + ( + "One of the Copr builds failed for " + "commit {commit_sha}, ping @admin, copr build logs {logs_url} " + "and {packit_dashboard_url}" + ), + {}, + ( + "One of the Copr builds failed for commit 123, ping @admin, " + "copr build logs {no entry for logs_url} and " + "{no entry for packit_dashboard_url}" + ), + id="commit_sha and no logs and packit dashboard url", + ), + pytest.param( + ( + "One of the Copr builds failed for " + "commit {commit_sha}, ping @admin, copr build logs {logs_url}" + ), + { + "logs_url": "jghfkgjfd", + "packit_dashboard_url": "jghfkgjfd", + }, + "One of the Copr builds failed for commit 123, ping @admin, copr build logs jghfkgjfd", + id="commit_sha, copr build logs url and packit dashboard url", + ), + ], +) +def test_notify_about_failure_if_configured(failure_comment, kwargs, result_comment): jobs = [ JobConfig( type=JobType.copr_build, @@ -2874,8 +2920,7 @@ def test_notify_about_failure_if_configured(): "packages": CommonPackageConfig( notifications=NotificationsConfig( failure_comment=FailureCommentNotificationsConfig( - "One of the Copr builds failed for " - "commit {commit_sha}, ping @admin" + failure_comment ) ) ) @@ -2897,7 +2942,7 @@ def test_notify_about_failure_if_configured(): ) flexmock(StatusReporter).should_receive("comment").with_args( - "One of the Copr builds failed for commit 123, ping @admin", + result_comment, duplicate_check=DuplicateCheckMode.check_last_comment, ) - copr_build_helper.notify_about_failure_if_configured() + copr_build_helper.notify_about_failure_if_configured(**kwargs)