diff --git a/snapshot_manager/snapshot_manager/github_util.py b/snapshot_manager/snapshot_manager/github_util.py index 0411926f..0ffec08b 100644 --- a/snapshot_manager/snapshot_manager/github_util.py +++ b/snapshot_manager/snapshot_manager/github_util.py @@ -100,9 +100,13 @@ def initial_comment(self) -> str: {self.config.update_marker} -

Last updated: {datetime.datetime.now().isoformat()}

+{self.last_updated_html()} """ + @classmethod + def last_updated_html(cls) -> str: + return f"

Last updated: {datetime.datetime.now().isoformat()}

" + def create_or_get_todays_github_issue( self, maintainer_handle: str, @@ -269,10 +273,10 @@ def remove_labels_safe( issue (github.Issue.Issue): The issue from which to remove the labels label_names_to_be_removed (list[str]): A list of label names that shall be removed if they exist on the issue. """ - current_label_names = [label.name for label in issue.get_labels()] - label_names_to_be_removed = set(current_label_names).intersection( - label_names_to_be_removed - ) - for label in label_names_to_be_removed: + current_set = {label.name for label in issue.get_labels()} + + remove_set = set(label_names_to_be_removed) + intersection = current_set.intersection(remove_set) + for label in intersection: logging.info(f"Removing label '{label}' from issue: {issue.title}") issue.remove_from_labels(label) diff --git a/snapshot_manager/snapshot_manager/snapshot_manager.py b/snapshot_manager/snapshot_manager/snapshot_manager.py index 43168766..1a600068 100755 --- a/snapshot_manager/snapshot_manager/snapshot_manager.py +++ b/snapshot_manager/snapshot_manager/snapshot_manager.py @@ -27,11 +27,11 @@ def check_todays_builds(self): maintainer_handle=self.config.maintainer_handle, creator=self.config.creator_handle, ) - if issue.state == "closed": - logging.info( - f"Issue {issue.html_url} was already closed. Not doing anything." - ) - return + # if issue.state == "closed": + # logging.info( + # f"Issue {issue.html_url} was already closed. Not doing anything." + # ) + # return all_chroots = self.copr.get_copr_chroots() @@ -52,14 +52,13 @@ def check_todays_builds(self): build_states=states, ) - logging.info("Append ordered list of errors to the issue's body comment") + logging.info("Get ordered list of errors to the issue's body comment") errors = build_status.list_only_errors(states=states) - logging.info( - "Extract and sanitize testing-farm information out of the last comment body." - ) - testing_farm_requests = tf.TestingFarmRequest.parse(comment_body) - logging.info(testing_farm_requests) + logging.info("Recover testing-farm requests") + requests = tf.TestingFarmRequest.parse(comment_body) + if requests is None: + requests = dict() # logging.info(f"Update the issue comment body") # # See https://github.com/fedora-llvm-team/llvm-snapshots/issues/205#issuecomment-1902057639 @@ -70,101 +69,63 @@ def check_todays_builds(self): # f"Github only allows {max_length} characters on a comment body and we have reached {len(comment_body)} characters." # ) - logging.info("Gather labels based on the errors we've found") - error_labels = list({f"error/{err.err_cause}" for err in errors}) - project_labels = list({f"project/{err.package_name}" for err in errors}) - os_labels = list({f"os/{err.os}" for err in errors}) - arch_labels = list({f"arch/{err.arch}" for err in errors}) - strategy_labels = [f"strategy/{self.config.build_strategy}"] - other_labels: list[str] = [] - if errors is None and len(errors) > 0: - other_labels.append("broken_snapshot_detected") - - logging.info("Create labels") - self.github._create_labels( - labels=["broken_snapshot_detected"], color="F46696", prefix="" - ) - self.github.create_labels_for_error_causes(error_labels) - self.github.create_labels_for_oses(os_labels) - self.github.create_labels_for_projects(project_labels) - self.github.create_labels_for_archs(arch_labels) - self.github.create_labels_for_strategies(strategy_labels) - self.github.create_labels_for_in_testing(all_chroots) - self.github.create_labels_for_tested_on(all_chroots) - self.github.create_labels_for_failed_on(all_chroots) - - # Remove old labels from issue if they no longer apply. This is greate - # for restarted builds for example to make all builds green and be able - # to promote this snapshot. - - labels_to_be_removed: list[str] = [] - old_error_labels = self.github.get_error_labels_on_issue(issue=issue) - old_project_labels = self.github.get_project_labels_on_issue(issue=issue) - old_arch_labels = self.github.get_arch_labels_on_issue(issue=issue) - - labels_to_be_removed.extend(set(old_error_labels) - set(error_labels)) - labels_to_be_removed.extend(set(old_project_labels) - set(project_labels)) - labels_to_be_removed.extend(set(old_arch_labels) - set(arch_labels)) - - for label in labels_to_be_removed: - logging.info(f"Removing label that no longer applies: {label}") - issue.remove_from_labels(label=label) - - # Labels must be added or removed manually in order to not remove manually added labels :/ - labels_to_add = ( - error_labels - + project_labels - + os_labels - + arch_labels - + strategy_labels - + other_labels - ) - logging.info(f"Adding label: {labels_to_add}") - issue.add_to_labels(labels_to_add) + self.handle_labels(issue=issue, all_chroots=all_chroots, errors=errors) failed_test_cases: list[tf.FailedTestCase] = [] for chroot in all_chroots: - # Define some label names - in_testing = f"{self.config.label_prefix_in_testing}{chroot}" - tested_on = f"{self.config.label_prefix_tested_on}{chroot}" - failed_on = f"{self.config.label_prefix_tested_on}{chroot}" + # Create or update a comment for each chroot that has errors and render + errors_for_this_chroot = [ + error for error in errors if error.chroot == chroot + ] + if errors_for_this_chroot is not None and len(errors_for_this_chroot) > 0: + comment = self.github.create_or_update_comment( + issue=issue, + marker=f"", + comment_body=f""" +

Errors found in {chroot}

+{self.github.last_updated_html()} +{build_status.render_as_markdown(errors_for_this_chroot)} +""", + ) + build_status_matrix = build_status_matrix.replace( + chroot, + f'{chroot}
:x: Copr build(s) failed', + ) - if not tf.is_chroot_supported(chroot): + for chroot in all_chroots: + # Check if we can ignore the chroot because it is not supported by testing-farm + if not tf.TestingFarmRequest.is_chroot_supported(chroot): # see https://docs.testing-farm.io/Testing%20Farm/0.1/test-environment.html#_supported_architectures logging.debug( f"Ignoring chroot {chroot} because testing-farm doesn't support it." ) continue + in_testing = f"{self.config.label_prefix_in_testing}{chroot}" + tested_on = f"{self.config.label_prefix_tested_on}{chroot}" + failed_on = f"{self.config.label_prefix_tested_on}{chroot}" + # Gather build IDs associated with this chroot. + # We'll attach them a new testing-farm request, and for a recovered + # request we'll check if they still match the current ones. current_copr_build_ids = [ state.build_id for state in states if state.chroot == chroot ] - def flip_test_label(issue: github.Issue.Issue, new_label: str | None): - all_states = [in_testing, tested_on, failed_on] - labels_to_be_removed = all_states - if new_label is not None: - labels_to_be_removed = [set(all_states).difference(new_label)] - self.github.remove_labels_safe(issue, labels_to_be_removed) - issue.add_to_labels(new_label) - # Check if we need to invalidate a recovered testing-farm requests. # Background: It can be that we have old testing-farm request IDs in the issue comment. # But if a package was re-build and failed, the old request ID for that chroot is invalid. # To compensate for this scenario that we saw on April 1st 2024 btw., we're gonna # delete any request that has a different set of Copr build IDs associated with it. - if chroot in testing_farm_requests: - recovered_request = testing_farm_requests[chroot] + if chroot in requests: + recovered_request = requests[chroot] if set(recovered_request.copr_build_ids) != set(current_copr_build_ids): logging.info( - "The recovered testing-farm request no longer applies because build IDs have changed" + f"Recovered request ({recovered_request.request_id}) invalid (build IDs changed):\\nRecovered: {recovered_request.copr_build_ids}\\nCurrent: {current_copr_build_ids}" ) - flip_test_label(issue, None) - del testing_farm_requests[chroot] - - tf.TestingFarmRequest(chroot=chroot) + self.flip_test_label(issue=issue, chroot=chroot, new_label=None) + del requests[chroot] logging.info(f"Check if all builds in chroot {chroot} have succeeded") builds_succeeded = self.copr.has_all_good_builds( @@ -181,8 +142,8 @@ def flip_test_label(issue: github.Issue.Issue, new_label: str | None): logging.info(f"All builds in chroot {chroot} have succeeded!") # Check for current status of testing-farm request - if chroot in testing_farm_requests: - request = testing_farm_requests[chroot] + if chroot in requests: + request = requests[chroot] watch_result, artifacts_url = request.watch() html = tf.render_html(request, watch_result, artifacts_url) @@ -191,32 +152,31 @@ def flip_test_label(issue: github.Issue.Issue, new_label: str | None): f"{chroot}
{html}", ) - # Fetch all failed tests for this request - if watch_result.is_error: - failed_test_cases.extend( - request.fetch_failed_test_cases(artifacts_url=artifacts_url) - ) - logging.info( f"Chroot {chroot} testing-farm watch result: {watch_result} (URL: {artifacts_url})" ) if watch_result.is_error: - flip_test_label(issue, failed_on) + # Fetch all failed test cases for this request + failed_test_cases.extend( + request.fetch_failed_test_cases(artifacts_url=artifacts_url) + ) + self.flip_test_label(issue, chroot, failed_on) elif watch_result == tf.TestingFarmWatchResult.TESTS_PASSED: - flip_test_label(issue, tested_on) + self.flip_test_label(issue, chroot, tested_on) else: logging.info(f"Starting tests for chroot {chroot}") - request_id = tf.TestingFarmRequest.make( + request = tf.TestingFarmRequest.make( chroot=chroot, config=self.config, issue=issue, copr_build_ids=current_copr_build_ids, ) - logging.info(f"Request ID: {request_id}") - testing_farm_requests[chroot] = request_id - flip_test_label(issue, in_testing) + logging.info(f"Request ID: {request.request_id}") + requests[chroot] = request + self.flip_test_label(issue, chroot, in_testing) + # Create or update a comment for testing-farm results display if len(failed_test_cases) > 0: self.github.create_or_update_comment( issue=issue, @@ -226,12 +186,11 @@ def flip_test_label(issue: github.Issue.Issue, new_label: str | None): ), ) - logging.info("Reconstructing issue comment body") + logging.info("Constructing issue comment body") comment_body = f""" {self.github.initial_comment} {build_status_matrix} -{build_status.render_as_markdown(errors)} -{tf.TestingFarmRequest.dict_to_html_comment(testing_farm_requests)} +{tf.TestingFarmRequest.dict_to_html_comment(requests)} """ issue.edit(body=comment_body) @@ -245,7 +204,7 @@ def flip_test_label(issue: github.Issue.Issue, new_label: str | None): required_chroot_abels = [ "{self.config.label_prefix_tested_on}{chroot}" for chroot in all_chroots - if tf.is_chroot_supported(chroot) + if tf.TestingFarmRequest.is_chroot_supported(chroot) ] if set(tested_chroot_labels) == set(required_chroot_abels): msg = f"@{self.config.maintainer_handle}, all required packages have been successfully built and tested on all required chroots. We'll close this issue for you now as completed. Congratulations!" @@ -257,3 +216,90 @@ def flip_test_label(issue: github.Issue.Issue, new_label: str | None): logging.info("Cannot close issue yet.") logging.info(f"Updated today's issue: {issue.html_url}") + + def handle_labels( + self, + issue: github.Issue.Issue, + all_chroots: list[str], + errors: build_status.BuildStateList, + ): + logging.info("Gather labels based on the errors we've found") + error_labels = list({f"error/{err.err_cause}" for err in errors}) + project_labels = list({f"project/{err.package_name}" for err in errors}) + os_labels = list({f"os/{err.os}" for err in errors}) + arch_labels = list({f"arch/{err.arch}" for err in errors}) + strategy_labels = [f"strategy/{self.config.build_strategy}"] + other_labels: list[str] = [] + if errors is None and len(errors) > 0: + other_labels.append("broken_snapshot_detected") + + logging.info("Create labels") + self.github._create_labels( + labels=["broken_snapshot_detected"], color="F46696", prefix="" + ) + self.github.create_labels_for_error_causes(error_labels) + self.github.create_labels_for_oses(os_labels) + self.github.create_labels_for_projects(project_labels) + self.github.create_labels_for_archs(arch_labels) + self.github.create_labels_for_strategies(strategy_labels) + self.github.create_labels_for_in_testing(all_chroots) + self.github.create_labels_for_tested_on(all_chroots) + self.github.create_labels_for_failed_on(all_chroots) + + # Remove old labels from issue if they no longer apply. This is great + # for restarted builds for example to make all builds green and be able + # to promote this snapshot. + + labels_to_be_removed: list[str] = [] + old_error_labels = self.github.get_error_labels_on_issue(issue=issue) + old_project_labels = self.github.get_project_labels_on_issue(issue=issue) + old_arch_labels = self.github.get_arch_labels_on_issue(issue=issue) + + labels_to_be_removed.extend(set(old_error_labels) - set(error_labels)) + labels_to_be_removed.extend(set(old_project_labels) - set(project_labels)) + labels_to_be_removed.extend(set(old_arch_labels) - set(arch_labels)) + + for label in labels_to_be_removed: + logging.info(f"Removing label that no longer applies: {label}") + issue.remove_from_labels(label=label) + + # Labels must be added or removed manually in order to not remove manually added labels :/ + labels_to_add = ( + error_labels + + project_labels + + os_labels + + arch_labels + + strategy_labels + + other_labels + ) + logging.info(f"Adding label: {labels_to_add}") + issue.add_to_labels(*labels_to_add) + + def flip_test_label( + self, issue: github.Issue.Issue, chroot: str, new_label: str | None + ): + """Let's you change the label on an issue for a specific chroot. + + If `new_label` is `None`, then all test labels will be removed. + + Args: + issue (github.Issue.Issue): The issue to modify + chroot (str): The chroot for which you want to flip the test label + new_label (str | None): The new label or `None`. + """ + in_testing = f"{self.config.label_prefix_in_testing}{chroot}" + tested_on = f"{self.config.label_prefix_tested_on}{chroot}" + failed_on = f"{self.config.label_prefix_tested_on}{chroot}" + + all_states = [in_testing, tested_on, failed_on] + labels_to_be_removed = all_states + + if new_label is not None: + labels_to_be_removed = list(set(all_states).difference(set([new_label]))) + self.github.remove_labels_safe(issue, labels_to_be_removed) + + # Check if new_label is already set + if new_label is not None and new_label not in [ + label.name for label in issue.labels + ]: + issue.add_to_labels(new_label) diff --git a/snapshot_manager/snapshot_manager/testing_farm_util.py b/snapshot_manager/snapshot_manager/testing_farm_util.py index 82e3f379..388f2155 100644 --- a/snapshot_manager/snapshot_manager/testing_farm_util.py +++ b/snapshot_manager/snapshot_manager/testing_farm_util.py @@ -107,14 +107,15 @@ def parse(cls, string: str) -> dict[str, "TestingFarmRequest"]: >>> keys dict_keys(['fedora-rawhide-x86_64', 'fedora-39-x86_64', 'fedora-40-x86_64']) >>> requests['fedora-rawhide-x86_64'] - TestingFarmRequest(request_id=UUID('271a79e8-fc9a-4e1d-95fe-567cc9d62ad4'), chroot='fedora-rawhide-x86_64', copr_build_ids=['1', '2', '3']) + TestingFarmRequest(request_id=UUID('271a79e8-fc9a-4e1d-95fe-567cc9d62ad4'), chroot='fedora-rawhide-x86_64', copr_build_ids=[1, 2, 3]) >>> requests['fedora-39-x86_64'] - TestingFarmRequest(request_id=UUID('22222222-fc9a-4e1d-95fe-567cc9d62ad4'), chroot='fedora-39-x86_64', copr_build_ids=['5', '6', '7']) + TestingFarmRequest(request_id=UUID('22222222-fc9a-4e1d-95fe-567cc9d62ad4'), chroot='fedora-39-x86_64', copr_build_ids=[5, 6, 7]) >>> requests['fedora-40-x86_64'] - TestingFarmRequest(request_id=UUID('33333333-fc9a-4e1d-95fe-567cc9d62ad4'), chroot='fedora-40-x86_64', copr_build_ids=['12', '13', '14']) + TestingFarmRequest(request_id=UUID('33333333-fc9a-4e1d-95fe-567cc9d62ad4'), chroot='fedora-40-x86_64', copr_build_ids=[12, 13, 14]) """ matches = re.findall(r"", string) if not matches: + logging.info("No testing-farm requests found to recover.") return None res: dict[str, TestingFarmRequest] = {} @@ -124,12 +125,14 @@ def parse(cls, string: str) -> dict[str, "TestingFarmRequest"]: tfr = TestingFarmRequest( chroot=chroot, request_id=sanitize_request_id(str(match[1])), - copr_build_ids=[item.strip() for item in match[2].split(",")], + copr_build_ids=[int(item.strip()) for item in match[2].split(",")], ) res[chroot] = tfr logging.info(f"ADDED testing-farm request: {tfr}") except ValueError as e: logging.info(f"ignoring: {match} : {str(e)}") + + logging.info(f"Recovered testing-farm-requests: {res}") return res @classmethod @@ -166,7 +169,7 @@ def make( ranch = cls.select_ranch(chroot) - if not cls._is_chroot_supported(chroot=chroot, ranch=ranch): + if not cls.is_chroot_supported(chroot=chroot, ranch=ranch): raise ValueError( f"Chroot {chroot} has an unsupported architecture on ranch {ranch}" ) @@ -586,11 +589,11 @@ def render_html( return f'{title}{vpn}' -def sanitize_request_id(request_id: str) -> uuid.UUID: - """Sanitizes a testing-farm request ID by ensuring that it matches a pattern. +def sanitize_request_id(request_id: str | uuid.UUID) -> uuid.UUID: + """Sanitizes a testing-farm request ID by ensuring that it is a UUID. Args: - request_id (str): A testing-farm request ID + request_id (str | uuid.UUID): A testing-farm request ID Raises: ValueError: if the given string is not in the right format @@ -603,11 +606,17 @@ def sanitize_request_id(request_id: str) -> uuid.UUID: >>> sanitize_request_id(request_id="271a79e8-fc9a-4e1d-95fe-567cc9d62ad4") UUID('271a79e8-fc9a-4e1d-95fe-567cc9d62ad4') + >>> import uuid + >>> sanitize_request_id(uuid.UUID('271a79e8-fc9a-4e1d-95fe-567cc9d62ad5')) + UUID('271a79e8-fc9a-4e1d-95fe-567cc9d62ad5') + >>> sanitize_request_id(request_id="; cat /etc/passwd") Traceback (most recent call last): ... ValueError: string is not a valid testing-farm request ID: badly formed hexadecimal UUID string """ + if isinstance(request_id, uuid.UUID): + return request_id res: uuid.UUID = None try: res = uuid.UUID(request_id) @@ -675,7 +684,7 @@ def render_list_as_markdown(cls, test_cases: list["FailedTestCase"]) -> str:

Failed testing-farm test cases

-{"".join([ test_case.render_as_markdown for test_case in test_cases ])} +{"".join([ test_case.render_as_markdown() for test_case in test_cases ])} """ diff --git a/snapshot_manager/snapshot_manager/util.py b/snapshot_manager/snapshot_manager/util.py index 9526806c..09046f10 100644 --- a/snapshot_manager/snapshot_manager/util.py +++ b/snapshot_manager/snapshot_manager/util.py @@ -115,7 +115,7 @@ def run_cmd(cmd: str, timeout_secs: int = 5) -> tuple[int, str, str]: stderr = proc.stderr.decode() exit_code = proc.returncode if exit_code != 0: - logging.info( + logging.debug( f"exit code: {proc.returncode} for cmd: {cmd}\n\nstdout={stdout}\n\nstderr={stderr}" ) diff --git a/snapshot_manager/tests/snapshot_manager_test.py b/snapshot_manager/tests/snapshot_manager_test.py index 786beccc..ba479905 100644 --- a/snapshot_manager/tests/snapshot_manager_test.py +++ b/snapshot_manager/tests/snapshot_manager_test.py @@ -11,8 +11,8 @@ class TestSnapshotManager(base_test.TestBase): def test_check_todays_builds(self): # cfg = self.config # cfg.copr_ownername = "@fedora-llvm-team" - # cfg.copr_project_tpl = "llvm-snapshots-incubator-20240329" - # cfg.datetime = datetime.date(year=2024, month=3, day=29) + # cfg.copr_project_tpl = "llvm-snapshots-incubator-20240405" + # cfg.datetime = datetime.date(year=2024, month=4, day=5) # cfg.strategy = "standalone" # cfg.maintainer_handle = "kwk" # cfg.creator_handle = "kwk"