diff --git a/packit_service/constants.py b/packit_service/constants.py index 317544033..87c6892be 100644 --- a/packit_service/constants.py +++ b/packit_service/constants.py @@ -5,6 +5,7 @@ CONTACTS_URL = "https://packit.dev/#contact" DOCS_URL = "https://packit.dev/docs" +DOCS_CONFIGURATION_URL = f"{DOCS_URL}/configuration" DOCS_FAQ_URL = f"{DOCS_URL}/faq" DOCS_HOW_TO_RETRIGGER_URL = ( f"{DOCS_URL}/guide/#how-to-re-trigger-packit-actions-in-your-pull-request" diff --git a/packit_service/trigger_mapping.py b/packit_service/trigger_mapping.py index ddea7f4f4..2a9e8a8eb 100644 --- a/packit_service/trigger_mapping.py +++ b/packit_service/trigger_mapping.py @@ -8,4 +8,8 @@ def are_job_types_same(first: JobType, second: JobType) -> bool: """ We need to treat `build` alias in a special way. """ - return first == second or {first, second} == {JobType.build, JobType.copr_build} + return ( + first == second + or {first, second} == {JobType.build, JobType.copr_build} + or {first, second} == {JobType.production_build, JobType.upstream_koji_build} + ) diff --git a/packit_service/worker/events/github.py b/packit_service/worker/events/github.py index dbe55833c..03df1a2c0 100644 --- a/packit_service/worker/events/github.py +++ b/packit_service/worker/events/github.py @@ -228,10 +228,7 @@ def __init__( @property def build_targets_override(self) -> Optional[Set[str]]: - if ( - self.check_name_job == "rpm-build" - or self.check_name_job == "production-build" - ): + if self.check_name_job in {"rpm-build", "production-build", "koji-build"}: return {self.check_name_target} return None diff --git a/packit_service/worker/handlers/koji.py b/packit_service/worker/handlers/koji.py index 646dc74fc..7480fb796 100644 --- a/packit_service/worker/handlers/koji.py +++ b/packit_service/worker/handlers/koji.py @@ -56,8 +56,11 @@ @configured_as(job_type=JobType.production_build) +@configured_as(job_type=JobType.upstream_koji_build) @run_for_comment(command="production-build") +@run_for_comment(command="upstream-koji-build") @run_for_check_rerun(prefix="production-build") +@run_for_check_rerun(prefix="koji-build") @reacts_to(ReleaseEvent) @reacts_to(PullRequestGithubEvent) @reacts_to(PushGitHubEvent) @@ -147,6 +150,7 @@ def pre_check(self) -> bool: @configured_as(job_type=JobType.production_build) +@configured_as(job_type=JobType.upstream_koji_build) @reacts_to(event=KojiTaskEvent) class KojiTaskReportHandler(JobHandler): task_name = TaskName.upstream_koji_build_report diff --git a/packit_service/worker/helpers/build/koji_build.py b/packit_service/worker/helpers/build/koji_build.py index 399f5f1c5..64c32e287 100644 --- a/packit_service/worker/helpers/build/koji_build.py +++ b/packit_service/worker/helpers/build/koji_build.py @@ -29,7 +29,7 @@ class KojiBuildJobHelper(BaseBuildJobHelper): job_type_build = JobType.production_build job_type_test = None - status_name_build: str = "production-build" + status_name_build: str = "koji-build" status_name_test: str = None def __init__( @@ -55,7 +55,7 @@ def __init__( ) self.msg_retrigger: str = MSG_RETRIGGER.format( job="build", - command="production-build", + command="upstream-koji-build", place="pull request", packit_comment_command_prefix=self.service_config.comment_command_prefix, ) diff --git a/packit_service/worker/jobs.py b/packit_service/worker/jobs.py index 8691db89c..30d419e07 100644 --- a/packit_service/worker/jobs.py +++ b/packit_service/worker/jobs.py @@ -13,8 +13,10 @@ 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.constants import ( + DOCS_CONFIGURATION_URL, TASK_ACCEPTED, COMMENT_REACTION, PACKIT_VERIFY_FAS_COMMAND, @@ -245,7 +247,7 @@ def report_task_accepted(self, handler: JobHandler, job_config: JobConfig): job_config: Job config that is being used. """ number_of_build_targets = None - if isinstance( + if not isinstance( handler, ( CoprBuildHandler, @@ -254,30 +256,29 @@ def report_task_accepted(self, handler: JobHandler, job_config: JobConfig): ProposeDownstreamHandler, ), ): - job_helper = self.initialize_job_helper(handler, job_config) - reporting_method = None + # no reporting, no metrics + return - if isinstance(job_helper, ProposeDownstreamJobHelper): - reporting_method = job_helper.report_status_to_all + job_helper = self.initialize_job_helper(handler, job_config) + reporting_method = None - elif isinstance(job_helper, BaseBuildJobHelper): - reporting_method = ( - job_helper.report_status_to_tests - if isinstance(handler, TestingFarmHandler) - else job_helper.report_status_to_build - ) - number_of_build_targets = len(job_helper.build_targets) + if isinstance(job_helper, ProposeDownstreamJobHelper): + reporting_method = job_helper.report_status_to_all - task_accepted_time = datetime.now(timezone.utc) - reporting_method( - description=TASK_ACCEPTED, - state=BaseCommitStatus.pending, - url="", + elif isinstance(job_helper, BaseBuildJobHelper): + reporting_method = ( + job_helper.report_status_to_tests + if isinstance(handler, TestingFarmHandler) + else job_helper.report_status_to_build ) + number_of_build_targets = len(job_helper.build_targets) - else: - # no reporting, no metrics - return + task_accepted_time = datetime.now(timezone.utc) + reporting_method( + description=TASK_ACCEPTED, + state=BaseCommitStatus.pending, + url="", + ) self.push_initial_metrics(task_accepted_time, handler, number_of_build_targets) @@ -427,6 +428,19 @@ def should_task_be_created_for_job_config_and_handler( # e.g. We don't allow using internal TF for external contributors. return False + if deprecation_msg := DEPRECATED_JOB_TYPES.get(job_config.type): + job_helper = self.initialize_job_helper(handler, job_config) + job_helper.status_reporter.report( + state=BaseCommitStatus.neutral, # TODO: change to warning in Nov 2022 + description=f"Job name `{job_config.type.name}` deprecated.", + url=f"{DOCS_CONFIGURATION_URL}/#supported-jobs", + check_names=f"config-deprecation-{job_config.type.name}", + markdown_content=f"{deprecation_msg}\n\n" + "This status will be switched to a warning since November " + "and the support for the old name will be removed " + "by the end of the year.", + ) + self.report_task_accepted(handler=handler, job_config=job_config) return True diff --git a/tests/integration/test_check_rerun.py b/tests/integration/test_check_rerun.py index 4a29537f2..694ed1edf 100644 --- a/tests/integration/test_check_rerun.py +++ b/tests/integration/test_check_rerun.py @@ -72,7 +72,7 @@ def check_rerun_event_koji_build(): event = json.loads( (DATA_DIR / "webhooks" / "github" / "checkrun_rerequested.json").read_text() ) - event["check_run"]["name"] = "production-build:f34" + event["check_run"]["name"] = "koji-build:f34" return event @@ -342,7 +342,7 @@ def test_check_rerun_pr_testing_farm_handler( [ { "trigger": "pull_request", - "job": "production_build", + "job": "upstream_koji_build", "metadata": {"targets": "fedora-all", "scratch": "true"}, } ] @@ -367,7 +367,79 @@ def test_check_rerun_pr_koji_build_handler( flexmock(StatusReporterGithubChecks).should_receive("set_status").with_args( state=BaseCommitStatus.pending, description=TASK_ACCEPTED, - check_name="production-build:f34", + check_name="koji-build:f34", + url="", + links_to_external_services=None, + markdown_content=None, + ).once() + flexmock(Signature).should_receive("apply_async").once() + flexmock(Pushgateway).should_receive("push").twice().and_return() + + processing_results = SteveJobs().process_message(check_rerun_event_koji_build) + event_dict, job, job_config, package_config = get_parameters_from_results( + processing_results + ) + assert json.dumps(event_dict) + assert event_dict["build_targets_override"] == ["f34"] + + results = run_koji_build_handler( + package_config=package_config, + event=event_dict, + job_config=job_config, + ) + assert first_dict_value(results["job"])["success"] + + +@pytest.mark.parametrize( + "mock_pr_functionality", + ( + [ + [ + { + "trigger": "pull_request", + "job": "production_build", + "metadata": {"targets": "fedora-all", "scratch": "true"}, + } + ] + ] + ), + indirect=True, +) +def test_check_rerun_pr_koji_build_handler_old_job_name( + mock_pr_functionality, check_rerun_event_koji_build +): + flexmock(KojiBuildJobHelper).should_receive("run_koji_build").and_return( + TaskResults(success=True, details={}) + ) + flexmock(GithubProject).should_receive("get_files").and_return(["foo.spec"]) + flexmock(GithubProject).should_receive("get_web_url").and_return( + "https://github.com/the-namespace/the-repo" + ) + flexmock(GithubProject).should_receive("is_private").and_return(False) + flexmock(koji_build).should_receive("get_koji_targets").and_return( + {"rawhide", "f34"} + ) + flexmock(StatusReporterGithubChecks).should_receive("set_status").with_args( + state=BaseCommitStatus.neutral, + description="Job name `production_build` deprecated.", + check_name="config-deprecation-production_build", + url="https://packit.dev/docs/configuration/#supported-jobs", + links_to_external_services=None, + markdown_content="The `production_build` name for upstream Koji build is misleading " + "because it is not used to run production/non-scratch builds and " + "because it can be confused with " + "the `koji_build` job that is triggered for dist-git commits. " + "(The `koji_build` job can trigger both scratch and " + "non-scratch/production builds.) " + "To be explicit, use `upstream_koji_build` for builds triggered in upstream and " + "`koji_build` for builds triggered in downstream.\n\n" + "This status will be switched to a warning since November and " + "the support for the old name will be removed by the end of the year.", + ).once() + flexmock(StatusReporterGithubChecks).should_receive("set_status").with_args( + state=BaseCommitStatus.pending, + description=TASK_ACCEPTED, + check_name="koji-build:f34", url="", links_to_external_services=None, markdown_content=None, @@ -526,7 +598,7 @@ def test_check_rerun_push_testing_farm_handler( [ { "trigger": "commit", - "job": "production_build", + "job": "upstream_koji_build", "metadata": {"targets": "fedora-all", "scratch": "true"}, } ] @@ -551,7 +623,7 @@ def test_check_rerun_push_koji_build_handler( flexmock(StatusReporterGithubChecks).should_receive("set_status").with_args( state=BaseCommitStatus.pending, description=TASK_ACCEPTED, - check_name="production-build:f34", + check_name="koji-build:f34", url="", links_to_external_services=None, markdown_content=None, @@ -650,7 +722,7 @@ def test_check_rerun_release_copr_build_handler( [ { "trigger": "release", - "job": "production_build", + "job": "upstream_koji_build", "metadata": {"targets": "fedora-all", "scratch": "true"}, } ] @@ -675,7 +747,7 @@ def test_check_rerun_release_koji_build_handler( flexmock(StatusReporterGithubChecks).should_receive("set_status").with_args( state=BaseCommitStatus.pending, description=TASK_ACCEPTED, - check_name="production-build:f34", + check_name="koji-build:f34", url="", links_to_external_services=None, markdown_content=None, diff --git a/tests/integration/test_handler.py b/tests/integration/test_handler.py index 6afe14dd6..fd389fc56 100644 --- a/tests/integration/test_handler.py +++ b/tests/integration/test_handler.py @@ -213,7 +213,7 @@ def test_precheck_koji_build_non_scratch(github_pr_event): flexmock(StatusReporterGithubChecks).should_receive("set_status").with_args( state=BaseCommitStatus.neutral, description="Non-scratch builds not possible from upstream.", - check_name="production-build:bright-future", + check_name="koji-build:bright-future", url=KOJI_PRODUCTION_BUILDS_ISSUE, links_to_external_services=None, markdown_content=None, diff --git a/tests/integration/test_listen_to_fedmsg.py b/tests/integration/test_listen_to_fedmsg.py index 30bbd79b6..31dbf3cff 100644 --- a/tests/integration/test_listen_to_fedmsg.py +++ b/tests/integration/test_listen_to_fedmsg.py @@ -123,7 +123,7 @@ def pc_koji_build_pr(): specfile_path="test.spec", jobs=[ JobConfig( - type=JobType.production_build, + type=JobType.upstream_koji_build, trigger=JobConfigTriggerType.pull_request, _targets=["fedora-all"], ) @@ -1093,7 +1093,7 @@ def test_koji_build_start(koji_build_scratch_start, pc_koji_build_pr, koji_build state=BaseCommitStatus.running, description="RPM build is in progress...", url=url, - check_names="production-build:rawhide", + check_names="koji-build:rawhide", markdown_content=None, ).once() flexmock(Signature).should_receive("apply_async").once() @@ -1155,7 +1155,7 @@ def test_koji_build_end(koji_build_scratch_end, pc_koji_build_pr, koji_build_pr) state=BaseCommitStatus.success, description="RPM build succeeded.", url=url, - check_names="production-build:rawhide", + check_names="koji-build:rawhide", markdown_content=None, ).once() flexmock(Signature).should_receive("apply_async").once() diff --git a/tests/integration/test_pr_comment.py b/tests/integration/test_pr_comment.py index 68d0c20b9..2ed01ad80 100644 --- a/tests/integration/test_pr_comment.py +++ b/tests/integration/test_pr_comment.py @@ -450,7 +450,7 @@ def test_pr_comment_production_build_handler(pr_production_build_comment_event): "jobs": [ { "trigger": "pull_request", - "job": "production_build", + "job": "upstream_koji_build", "metadata": {"targets": "fedora-rawhide-x86_64", "scratch": "true"}, } ], diff --git a/tests/unit/test_copr_build.py b/tests/unit/test_copr_build.py index af135cc84..c1b71dc29 100644 --- a/tests/unit/test_copr_build.py +++ b/tests/unit/test_copr_build.py @@ -1004,7 +1004,7 @@ def test_copr_build_for_release(release_event): # - Building SRPM ... # - Starting RPM build... branch_build_job = JobConfig( - type=JobType.build, + type=JobType.copr_build, trigger=JobConfigTriggerType.release, _targets=DEFAULT_TARGETS, owner="nobody", @@ -2397,7 +2397,7 @@ def test_run_copr_build_from_source_script_github_outage_retry( flexmock(StatusReporterGithubChecks).should_receive("set_status").with_args( state=BaseCommitStatus.error, description=f"Submit of the build failed: {exc}", - check_name="rpm-build:bright-fugure-x86_64", + check_name="rpm-build:bright-future-x86_64", url="", links_to_external_services=None, markdown_content=None, diff --git a/tests/unit/test_koji_build.py b/tests/unit/test_koji_build.py index 25dd869a8..844e4cdc7 100644 --- a/tests/unit/test_koji_build.py +++ b/tests/unit/test_koji_build.py @@ -71,7 +71,7 @@ def build_helper( jobs = jobs or [] jobs.append( JobConfig( - type=JobType.production_build, + type=JobType.upstream_koji_build, trigger=trigger or JobConfigTriggerType.pull_request, _targets=_targets, owner=owner, @@ -122,7 +122,7 @@ def test_koji_build_check_names(github_pr_event): flexmock(StatusReporter).should_receive("set_status").with_args( state=BaseCommitStatus.running, description="Building SRPM ...", - check_name="production-build:bright-future", + check_name="koji-build:bright-future", url="", links_to_external_services=None, markdown_content=None, @@ -130,7 +130,7 @@ def test_koji_build_check_names(github_pr_event): flexmock(StatusReporter).should_receive("set_status").with_args( state=BaseCommitStatus.running, description="Building RPM ...", - check_name="production-build:bright-future", + check_name="koji-build:bright-future", url=koji_build_url, links_to_external_services=None, markdown_content=None, @@ -197,7 +197,7 @@ def test_koji_build_failed_kerberos(github_pr_event): flexmock(StatusReporter).should_receive("set_status").with_args( state=BaseCommitStatus.running, description="Building SRPM ...", - check_name="production-build:bright-future", + check_name="koji-build:bright-future", url="", links_to_external_services=None, markdown_content=None, @@ -205,7 +205,7 @@ def test_koji_build_failed_kerberos(github_pr_event): flexmock(StatusReporter).should_receive("set_status").with_args( state=BaseCommitStatus.error, description="Kerberos authentication error: the bad authentication error", - check_name="production-build:bright-future", + check_name="koji-build:bright-future", url=get_srpm_build_info_url(1), links_to_external_services=None, markdown_content=None, @@ -273,7 +273,7 @@ def test_koji_build_target_not_supported(github_pr_event): flexmock(StatusReporter).should_receive("set_status").with_args( state=BaseCommitStatus.running, description="Building SRPM ...", - check_name="production-build:nonexisting-target", + check_name="koji-build:nonexisting-target", url="", links_to_external_services=None, markdown_content=None, @@ -281,7 +281,7 @@ def test_koji_build_target_not_supported(github_pr_event): flexmock(StatusReporter).should_receive("set_status").with_args( state=BaseCommitStatus.error, description="Target not supported: nonexisting-target", - check_name="production-build:nonexisting-target", + check_name="koji-build:nonexisting-target", url=get_srpm_build_info_url(1), links_to_external_services=None, markdown_content=None, @@ -409,7 +409,7 @@ def test_koji_build_failed(github_pr_event): flexmock(StatusReporter).should_receive("set_status").with_args( state=BaseCommitStatus.running, description="Building SRPM ...", - check_name="production-build:bright-future", + check_name="koji-build:bright-future", url="", links_to_external_services=None, markdown_content=None, @@ -419,7 +419,7 @@ def test_koji_build_failed(github_pr_event): flexmock(StatusReporter).should_receive("set_status").with_args( state=BaseCommitStatus.error, description="Submit of the build failed: some error", - check_name="production-build:bright-future", + check_name="koji-build:bright-future", url=srpm_build_url, links_to_external_services=None, markdown_content=None, @@ -479,7 +479,7 @@ def test_koji_build_failed_srpm(github_pr_event): flexmock(StatusReporter).should_receive("set_status").with_args( state=BaseCommitStatus.running, description="Building SRPM ...", - check_name="production-build:bright-future", + check_name="koji-build:bright-future", url="", links_to_external_services=None, markdown_content=None, @@ -487,7 +487,7 @@ def test_koji_build_failed_srpm(github_pr_event): flexmock(StatusReporter).should_receive("set_status").with_args( state=BaseCommitStatus.failure, description="SRPM build failed, check the logs for details.", - check_name="production-build:bright-future", + check_name="koji-build:bright-future", url=srpm_build_url, links_to_external_services=None, markdown_content=None,