Merge pull request #1656 from lachmanfrantisek/new-job-names

Deprecate production_build in favour of upstream_koji_build Deprecate also the build alias since it's too generic. Fixes: packit/packit#1658 Requires: packit/packit#1728 Documentation: packit/packit.dev#527 RELEASE NOTES BEGIN There are two changes in the naming of the service jobs: The build job type name has been deprecated. It aimed to be an alias when Packit supported just one build type. There are currently more types of builds and just build can be misleading. Please, be explicit and use copr_build instead. 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. Users will get a neutral status describing the change when the old names are in use. The status will become a warning starting in November and the old names will be removed by the end of the year. RELEASE NOTES END Reviewed-by: Tomas Tomecek <[email protected]> Reviewed-by: Laura Barcziová <None>
packit · Oct 11, 2022 · 85b02fc · 85b02fc
2 parents 911f4a2 + 5dfbef9
commit 85b02fc
Show file tree

Hide file tree

Showing 12 changed files with 144 additions and 52 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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,