Refactor labels in CI, move them to shared package

This commit is contained in:
Mikhail f. Shiryaev 2024-03-15 16:57:59 +01:00
parent 38a0edde75
commit 2ba5b7fd23
No known key found for this signature in database
GPG Key ID: 4B02ED204C7D93F4
8 changed files with 81 additions and 62 deletions

View File

@ -1,31 +1,27 @@
#!/usr/bin/env python3 #!/usr/bin/env python3
import argparse import argparse
from pathlib import Path
from typing import Tuple
import subprocess
import logging import logging
import subprocess
import sys import sys
import time import time
from pathlib import Path
from typing import Tuple
from ci_config import CI_CONFIG, BuildConfig
from env_helper import (
REPO_COPY,
S3_BUILDS_BUCKET,
TEMP_PATH,
)
from git_helper import Git
from pr_info import PRInfo
from report import FAILURE, JobReport, StatusType, SUCCESS
from tee_popen import TeePopen
import docker_images_helper import docker_images_helper
from ci_config import CI_CONFIG, BuildConfig
from env_helper import REPO_COPY, S3_BUILDS_BUCKET, TEMP_PATH
from git_helper import Git
from lambda_shared_package.lambda_shared.pr import Labels
from pr_info import PRInfo
from report import FAILURE, SUCCESS, JobReport, StatusType
from stopwatch import Stopwatch
from tee_popen import TeePopen
from version_helper import ( from version_helper import (
ClickHouseVersion, ClickHouseVersion,
get_version_from_repo, get_version_from_repo,
update_version_local, update_version_local,
) )
from stopwatch import Stopwatch
IMAGE_NAME = "clickhouse/binary-builder" IMAGE_NAME = "clickhouse/binary-builder"
BUILD_LOG_NAME = "build_log.log" BUILD_LOG_NAME = "build_log.log"
@ -111,6 +107,10 @@ def build_clickhouse(
return build_log_path, SUCCESS if success else FAILURE return build_log_path, SUCCESS if success else FAILURE
def is_release_pr(pr_info: PRInfo) -> bool:
return Labels.RELEASE in pr_info.labels or Labels.RELEASE_LTS in pr_info.labels
def get_release_or_pr(pr_info: PRInfo, version: ClickHouseVersion) -> Tuple[str, str]: def get_release_or_pr(pr_info: PRInfo, version: ClickHouseVersion) -> Tuple[str, str]:
"Return prefixes for S3 artifacts paths" "Return prefixes for S3 artifacts paths"
# FIXME performance # FIXME performance
@ -119,7 +119,7 @@ def get_release_or_pr(pr_info: PRInfo, version: ClickHouseVersion) -> Tuple[str,
# It should be fixed in performance-comparison image eventually # It should be fixed in performance-comparison image eventually
# For performance tests we always set PRs prefix # For performance tests we always set PRs prefix
performance_pr = "PRs/0" performance_pr = "PRs/0"
if "release" in pr_info.labels or "release-lts" in pr_info.labels: if is_release_pr(pr_info):
# for release pull requests we use branch names prefixes, not pr numbers # for release pull requests we use branch names prefixes, not pr numbers
return pr_info.head_ref, performance_pr return pr_info.head_ref, performance_pr
if pr_info.number == 0: if pr_info.number == 0:
@ -163,7 +163,7 @@ def main():
official_flag = pr_info.number == 0 official_flag = pr_info.number == 0
version_type = "testing" version_type = "testing"
if "release" in pr_info.labels or "release-lts" in pr_info.labels: if is_release_pr(pr_info):
version_type = "stable" version_type = "stable"
official_flag = True official_flag = True

View File

@ -9,7 +9,7 @@ from threading import Thread
from typing import Any, Dict, List, Optional from typing import Any, Dict, List, Optional
import requests import requests
from lambda_shared.pr import check_pr_description from lambda_shared.pr import Labels, check_pr_description
from lambda_shared.token import get_cached_access_token from lambda_shared.token import get_cached_access_token
NEED_RERUN_OR_CANCELL_WORKFLOWS = { NEED_RERUN_OR_CANCELL_WORKFLOWS = {
@ -261,7 +261,7 @@ def main(event):
print("Freshly opened PR, nothing to do") print("Freshly opened PR, nothing to do")
return return
if action == "closed" or label == "do not test": if action == "closed" or label == Labels.DO_NOT_TEST:
print("PR merged/closed or manually labeled 'do not test', will kill workflows") print("PR merged/closed or manually labeled 'do not test', will kill workflows")
workflow_descriptions = get_workflows_description_for_pull_request( workflow_descriptions = get_workflows_description_for_pull_request(
pull_request, token pull_request, token
@ -281,7 +281,7 @@ def main(event):
exec_workflow_url(urls_to_cancel, token) exec_workflow_url(urls_to_cancel, token)
return return
if label == "can be tested": if label == Labels.CAN_BE_TESTED:
print("PR marked with can be tested label, rerun workflow") print("PR marked with can be tested label, rerun workflow")
workflow_descriptions = get_workflows_description_for_pull_request( workflow_descriptions = get_workflows_description_for_pull_request(
pull_request, token pull_request, token

View File

@ -37,19 +37,10 @@ from env_helper import TEMP_PATH
from get_robot_token import get_best_robot_token from get_robot_token import get_best_robot_token
from git_helper import git_runner, is_shallow from git_helper import git_runner, is_shallow
from github_helper import GitHub, PullRequest, PullRequests, Repository from github_helper import GitHub, PullRequest, PullRequests, Repository
from lambda_shared_package.lambda_shared.pr import Labels
from ssh import SSHKey from ssh import SSHKey
class Labels:
MUST_BACKPORT = "pr-must-backport"
MUST_BACKPORT_CLOUD = "pr-must-backport-cloud"
BACKPORT = "pr-backport"
BACKPORTS_CREATED = "pr-backports-created"
BACKPORTS_CREATED_CLOUD = "pr-backports-created-cloud"
CHERRYPICK = "pr-cherrypick"
DO_NOT_TEST = "do not test"
class ReleaseBranch: class ReleaseBranch:
CHERRYPICK_DESCRIPTION = """Original pull-request #{pr_number} CHERRYPICK_DESCRIPTION = """Original pull-request #{pr_number}
@ -99,7 +90,7 @@ close it.
name: str, name: str,
pr: PullRequest, pr: PullRequest,
repo: Repository, repo: Repository,
backport_created_label: str = Labels.BACKPORTS_CREATED, backport_created_label: str = Labels.PR_BACKPORTS_CREATED,
): ):
self.name = name self.name = name
self.pr = pr self.pr = pr
@ -247,12 +238,12 @@ close it.
pr_number=self.pr.number, pr_number=self.pr.number,
pr_url=self.pr.html_url, pr_url=self.pr.html_url,
backport_created_label=self.backport_created_label, backport_created_label=self.backport_created_label,
label_cherrypick=Labels.CHERRYPICK, label_cherrypick=Labels.PR_CHERRYPICK,
), ),
base=self.backport_branch, base=self.backport_branch,
head=self.cherrypick_branch, head=self.cherrypick_branch,
) )
self.cherrypick_pr.add_to_labels(Labels.CHERRYPICK) self.cherrypick_pr.add_to_labels(Labels.PR_CHERRYPICK)
self.cherrypick_pr.add_to_labels(Labels.DO_NOT_TEST) self.cherrypick_pr.add_to_labels(Labels.DO_NOT_TEST)
self._assign_new_pr(self.cherrypick_pr) self._assign_new_pr(self.cherrypick_pr)
# update cherrypick PR to get the state for PR.mergable # update cherrypick PR to get the state for PR.mergable
@ -288,7 +279,7 @@ close it.
base=self.name, base=self.name,
head=self.backport_branch, head=self.backport_branch,
) )
self.backport_pr.add_to_labels(Labels.BACKPORT) self.backport_pr.add_to_labels(Labels.PR_BACKPORT)
self._assign_new_pr(self.backport_pr) self._assign_new_pr(self.backport_pr)
def ping_cherry_pick_assignees(self, dry_run: bool) -> None: def ping_cherry_pick_assignees(self, dry_run: bool) -> None:
@ -518,7 +509,7 @@ class Backport:
) )
bp_cp_prs = self.gh.get_pulls_from_search( bp_cp_prs = self.gh.get_pulls_from_search(
query=f"type:pr repo:{self._repo_name} {query_suffix}", query=f"type:pr repo:{self._repo_name} {query_suffix}",
label=f"{Labels.BACKPORT},{Labels.CHERRYPICK}", label=f"{Labels.PR_BACKPORT},{Labels.PR_CHERRYPICK}",
) )
for br in branches: for br in branches:
br.pop_prs(bp_cp_prs) br.pop_prs(bp_cp_prs)
@ -588,8 +579,8 @@ def parse_args():
) )
parser.add_argument( parser.add_argument(
"--backport-created-label", "--backport-created-label",
default=Labels.BACKPORTS_CREATED, default=Labels.PR_BACKPORTS_CREATED,
choices=(Labels.BACKPORTS_CREATED, Labels.BACKPORTS_CREATED_CLOUD), choices=(Labels.PR_BACKPORTS_CREATED, Labels.PR_BACKPORTS_CREATED_CLOUD),
help="label to mark PRs as backported", help="label to mark PRs as backported",
) )
parser.add_argument( parser.add_argument(

View File

@ -19,7 +19,8 @@ from github.Repository import Repository
from ci_config import CHECK_DESCRIPTIONS, REQUIRED_CHECKS, CheckDescription from ci_config import CHECK_DESCRIPTIONS, REQUIRED_CHECKS, CheckDescription
from env_helper import GITHUB_REPOSITORY, GITHUB_RUN_URL, TEMP_PATH from env_helper import GITHUB_REPOSITORY, GITHUB_RUN_URL, TEMP_PATH
from pr_info import SKIP_MERGEABLE_CHECK_LABEL, PRInfo from lambda_shared_package.lambda_shared.pr import Labels
from pr_info import PRInfo
from report import ( from report import (
ERROR, ERROR,
FAILURE, FAILURE,
@ -438,7 +439,7 @@ def set_mergeable_check(
def update_mergeable_check(commit: Commit, pr_info: PRInfo, check_name: str) -> None: def update_mergeable_check(commit: Commit, pr_info: PRInfo, check_name: str) -> None:
"check if the check_name in REQUIRED_CHECKS and then trigger update" "check if the check_name in REQUIRED_CHECKS and then trigger update"
not_run = ( not_run = (
pr_info.labels.intersection({SKIP_MERGEABLE_CHECK_LABEL, "release"}) pr_info.labels.intersection({Labels.SKIP_MERGEABLE_CHECK, Labels.RELEASE})
or check_name not in REQUIRED_CHECKS or check_name not in REQUIRED_CHECKS
or pr_info.release_pr or pr_info.release_pr
or pr_info.number == 0 or pr_info.number == 0

View File

@ -48,11 +48,35 @@ TRUSTED_CONTRIBUTORS = {
] ]
} }
class Labels:
CAN_BE_TESTED = "can be tested"
DO_NOT_TEST = "do not test"
MUST_BACKPORT = "pr-must-backport"
MUST_BACKPORT_CLOUD = "pr-must-backport-cloud"
JEPSEN_TEST = "jepsen-test"
SKIP_MERGEABLE_CHECK = "skip mergeable check"
PR_BACKPORT = "pr-backport"
PR_BACKPORTS_CREATED = "pr-backports-created"
PR_BACKPORTS_CREATED_CLOUD = "pr-backports-created-cloud"
PR_CHERRYPICK = "pr-cherrypick"
PR_CI = "pr-ci"
PR_FEATURE = "pr-feature"
PR_SYNCED_TO_CLOUD = "pr-synced-to-cloud"
PR_SYNC_UPSTREAM = "pr-sync-upstream"
RELEASE = "release"
RELEASE_LTS = "release-lts"
SUBMODULE_CHANGED = "submodule changed"
# pr-bugfix autoport can lead to issues in releases, let's do ci fixes only
AUTO_BACKPORT = {"pr-ci"}
# Descriptions are used in .github/PULL_REQUEST_TEMPLATE.md, keep comments there # Descriptions are used in .github/PULL_REQUEST_TEMPLATE.md, keep comments there
# updated accordingly # updated accordingly
# The following lists are append only, try to avoid editing them # The following lists are append only, try to avoid editing them
# They still could be cleaned out after the decent time though. # They still could be cleaned out after the decent time though.
LABELS = { LABEL_CATEGORIES = {
"pr-backward-incompatible": ["Backward Incompatible Change"], "pr-backward-incompatible": ["Backward Incompatible Change"],
"pr-bugfix": [ "pr-bugfix": [
"Bug Fix", "Bug Fix",
@ -81,7 +105,9 @@ LABELS = {
"pr-ci": ["CI Fix or Improvement (changelog entry is not required)"], "pr-ci": ["CI Fix or Improvement (changelog entry is not required)"],
} }
CATEGORY_TO_LABEL = {c: lb for lb, categories in LABELS.items() for c in categories} CATEGORY_TO_LABEL = {
c: lb for lb, categories in LABEL_CATEGORIES.items() for c in categories
}
def check_pr_description(pr_body: str, repo_name: str) -> Tuple[str, str]: def check_pr_description(pr_body: str, repo_name: str) -> Tuple[str, str]:
@ -93,7 +119,7 @@ def check_pr_description(pr_body: str, repo_name: str) -> Tuple[str, str]:
# Check if body contains "Reverts ClickHouse/ClickHouse#36337" # Check if body contains "Reverts ClickHouse/ClickHouse#36337"
if [True for line in lines if re.match(rf"\AReverts {repo_name}#[\d]+\Z", line)]: if [True for line in lines if re.match(rf"\AReverts {repo_name}#[\d]+\Z", line)]:
return "", LABELS["pr-not-for-changelog"][0] return "", LABEL_CATEGORIES["pr-not-for-changelog"][0]
category = "" category = ""
entry = "" entry = ""

View File

@ -15,8 +15,8 @@ from env_helper import (
GITHUB_RUN_URL, GITHUB_RUN_URL,
GITHUB_SERVER_URL, GITHUB_SERVER_URL,
) )
from lambda_shared_package.lambda_shared.pr import Labels
SKIP_MERGEABLE_CHECK_LABEL = "skip mergeable check"
NeedsDataType = Dict[str, Dict[str, Union[str, Dict[str, str]]]] NeedsDataType = Dict[str, Dict[str, Union[str, Dict[str, str]]]]
DIFF_IN_DOCUMENTATION_EXT = [ DIFF_IN_DOCUMENTATION_EXT = [
@ -252,7 +252,7 @@ class PRInfo:
self.head_ref = pull_request["head"]["ref"] self.head_ref = pull_request["head"]["ref"]
self.head_name = pull_request["head"]["repo"]["full_name"] self.head_name = pull_request["head"]["repo"]["full_name"]
self.pr_html_url = pull_request["html_url"] self.pr_html_url = pull_request["html_url"]
if "pr-backport" in self.labels: if Labels.PR_BACKPORT in self.labels:
# head1...head2 gives changes in head2 since merge base # head1...head2 gives changes in head2 since merge base
# Thag's why we need {self.head_ref}...master to get # Thag's why we need {self.head_ref}...master to get
# files changed in upstream AND master...{self.head_ref} # files changed in upstream AND master...{self.head_ref}
@ -275,7 +275,7 @@ class PRInfo:
] ]
else: else:
self.diff_urls.append(self.compare_pr_url(pull_request)) self.diff_urls.append(self.compare_pr_url(pull_request))
if "release" in self.labels: if Labels.RELEASE in self.labels:
# For release PRs we must get not only files changed in the PR # For release PRs we must get not only files changed in the PR
# itself, but as well files changed since we branched out # itself, but as well files changed since we branched out
self.diff_urls.append( self.diff_urls.append(

View File

@ -25,6 +25,7 @@ from contextlib import contextmanager
from typing import Any, Final, Iterator, List, Optional, Tuple from typing import Any, Final, Iterator, List, Optional, Tuple
from git_helper import Git, commit, release_branch from git_helper import Git, commit, release_branch
from lambda_shared_package.lambda_shared.pr import Labels
from report import SUCCESS from report import SUCCESS
from version_helper import ( from version_helper import (
FILE_WITH_VERSION_PATH, FILE_WITH_VERSION_PATH,
@ -407,9 +408,9 @@ class Release:
self._git.update() self._git.update()
new_version = self.version.patch_update() new_version = self.version.patch_update()
version_type = self.get_stable_release_type() version_type = self.get_stable_release_type()
pr_labels = "--label release" pr_labels = f"--label {Labels.RELEASE}"
if version_type == VersionType.LTS: if version_type == VersionType.LTS:
pr_labels += " --label release-lts" pr_labels += f" --label {Labels.RELEASE_LTS}"
new_version.with_description(version_type) new_version.with_description(version_type)
self._update_cmake_contributors(new_version) self._update_cmake_contributors(new_version)
self._commit_cmake_contributors(new_version) self._commit_cmake_contributors(new_version)

View File

@ -20,6 +20,7 @@ from get_robot_token import get_best_robot_token
from lambda_shared_package.lambda_shared.pr import ( from lambda_shared_package.lambda_shared.pr import (
CATEGORY_TO_LABEL, CATEGORY_TO_LABEL,
TRUSTED_CONTRIBUTORS, TRUSTED_CONTRIBUTORS,
Labels,
check_pr_description, check_pr_description,
) )
from pr_info import PRInfo from pr_info import PRInfo
@ -29,13 +30,8 @@ TRUSTED_ORG_IDS = {
54801242, # clickhouse 54801242, # clickhouse
} }
OK_SKIP_LABELS = {"release", "pr-backport", "pr-cherrypick"} OK_SKIP_LABELS = {Labels.RELEASE, Labels.PR_BACKPORT, Labels.PR_CHERRYPICK}
CAN_BE_TESTED_LABEL = "can be tested"
FEATURE_LABEL = "pr-feature"
SUBMODULE_CHANGED_LABEL = "submodule changed"
PR_CHECK = "PR Check" PR_CHECK = "PR Check"
# pr-bugfix autoport can lead to issues in releases, let's do ci fixes only
AUTO_BACKPORT_LABELS = ["pr-ci"]
def pr_is_by_trusted_user(pr_user_login, pr_user_orgs): def pr_is_by_trusted_user(pr_user_login, pr_user_orgs):
@ -68,11 +64,12 @@ def should_run_ci_for_pr(pr_info: PRInfo) -> Tuple[bool, str]:
if OK_SKIP_LABELS.intersection(pr_info.labels): if OK_SKIP_LABELS.intersection(pr_info.labels):
return True, "Don't try new checks for release/backports/cherry-picks" return True, "Don't try new checks for release/backports/cherry-picks"
if CAN_BE_TESTED_LABEL not in pr_info.labels and not pr_is_by_trusted_user( if Labels.CAN_BE_TESTED not in pr_info.labels and not pr_is_by_trusted_user(
pr_info.user_login, pr_info.user_orgs pr_info.user_login, pr_info.user_orgs
): ):
print( print(
f"PRs by untrusted users need the '{CAN_BE_TESTED_LABEL}' label - please contact a member of the core team" f"PRs by untrusted users need the '{Labels.CAN_BE_TESTED}' label - "
"please contact a member of the core team"
) )
return False, "Needs 'can be tested' label" return False, "Needs 'can be tested' label"
@ -116,11 +113,11 @@ def main():
pr_labels_to_remove.append(label) pr_labels_to_remove.append(label)
if pr_info.has_changes_in_submodules(): if pr_info.has_changes_in_submodules():
pr_labels_to_add.append(SUBMODULE_CHANGED_LABEL) pr_labels_to_add.append(Labels.SUBMODULE_CHANGED)
elif SUBMODULE_CHANGED_LABEL in pr_info.labels: elif Labels.SUBMODULE_CHANGED in pr_info.labels:
pr_labels_to_remove.append(SUBMODULE_CHANGED_LABEL) pr_labels_to_remove.append(Labels.SUBMODULE_CHANGED)
if any(label in AUTO_BACKPORT_LABELS for label in pr_labels_to_add): if any(label in Labels.AUTO_BACKPORT for label in pr_labels_to_add):
backport_labels = [Labels.MUST_BACKPORT, Labels.MUST_BACKPORT_CLOUD] backport_labels = [Labels.MUST_BACKPORT, Labels.MUST_BACKPORT_CLOUD]
pr_labels_to_add += [ pr_labels_to_add += [
label for label in backport_labels if label not in pr_info.labels label for label in backport_labels if label not in pr_info.labels
@ -160,16 +157,19 @@ def main():
) )
sys.exit(1) sys.exit(1)
if FEATURE_LABEL in pr_info.labels and not pr_info.has_changes_in_documentation(): if (
Labels.PR_FEATURE in pr_info.labels
and not pr_info.has_changes_in_documentation()
):
print( print(
f"The '{FEATURE_LABEL}' in the labels, " f"The '{Labels.PR_FEATURE}' in the labels, "
"but there's no changed documentation" "but there's no changed documentation"
) )
post_commit_status( post_commit_status(
commit, commit,
FAILURE, FAILURE,
"", "",
f"expect adding docs for {FEATURE_LABEL}", f"expect adding docs for {Labels.PR_FEATURE}",
PR_CHECK, PR_CHECK,
pr_info, pr_info,
) )