From 81a964c3486592a35caf7249702c746111ac0b1d Mon Sep 17 00:00:00 2001 From: Neves-P Date: Wed, 15 Nov 2023 17:27:22 +0100 Subject: [PATCH 01/50] download_pr error handling --- tasks/build.py | 33 +++++++++++++++++++++++++-------- 1 file changed, 25 insertions(+), 8 deletions(-) diff --git a/tasks/build.py b/tasks/build.py index 7750da02..0a597406 100644 --- a/tasks/build.py +++ b/tasks/build.py @@ -345,21 +345,28 @@ def download_pr(repo_name, branch_name, pr, arch_job_dir): # - 'curl' diff for pull request # - 'git apply' diff file git_clone_cmd = ' '.join(['git clone', f'https://github.com/{repo_name}', arch_job_dir]) - clone_output, clone_error, clone_exit_code = run_cmd(git_clone_cmd, "Clone repo", arch_job_dir) + clone_output, clone_error, clone_exit_code = run_cmd(git_clone_cmd, "Clone repo", arch_job_dir, raise_on_error=False) + if clone_exit_code != 0: + return clone_output, clone_error, clone_exit_code git_checkout_cmd = ' '.join([ 'git checkout', branch_name, ]) checkout_output, checkout_err, checkout_exit_code = run_cmd(git_checkout_cmd, - "checkout branch '%s'" % branch_name, arch_job_dir) + "checkout branch '%s'" % branch_name, arch_job_dir, raise_on_error=False) + if checkout_exit_code != 0: + return checkout_output, checkout_err, checkout_exit_code curl_cmd = f'curl -L https://github.com/{repo_name}/pull/{pr.number}.diff > {pr.number}.diff' - curl_output, curl_error, curl_exit_code = run_cmd(curl_cmd, "Obtain patch", arch_job_dir) - + curl_output, curl_error, curl_exit_code = run_cmd(curl_cmd, "Obtain patch", arch_job_dir, raise_on_error=False) + if curl_exit_code != 0: + return curl_output, curl_error, curl_exit_code + git_apply_cmd = f'git apply {pr.number}.diff' - git_apply_output, git_apply_error, git_apply_exit_code = run_cmd(git_apply_cmd, "Apply patch", arch_job_dir) - + git_apply_output, git_apply_error, git_apply_exit_code = run_cmd(git_apply_cmd, "Apply patch", arch_job_dir, raise_on_error=False) + if git_apply_exit_code != 0: + return git_apply_output, git_apply_error, git_apply_exit_code def apply_cvmfs_customizations(cvmfs_customizations, arch_job_dir): """ @@ -453,7 +460,17 @@ def prepare_jobs(pr, cfg, event_info, action_filter): log(f"{fn}(): job_dir '{job_dir}'") # TODO optimisation? download once, copy and cleanup initial copy? - download_pr(base_repo_name, base_branch_name, pr, job_dir) + download_pr_output, download_pr_error, download_pr_exit_code = download_pr(base_repo_name, base_branch_name, + pr, job_dir) + + if download_pr_exit_code != 0: + download_comment = f"Download failed with error: {download_pr_error}" + download_comment = pr_comments.create_comment(repo_name=base_repo_name,pr_number=pr.number,comment=download_comment) + if download_comment: + log(f"{fn}(): created PR issue comment with id {download_comment.id}") + else: + log(f"{fn}(): failed to create PR issue comment ") + raise RuntimeError(download_pr_error) # prepare job configuration file 'job.cfg' in directory /cfg cpu_target = '/'.join(arch.split('/')[1:]) @@ -491,7 +508,7 @@ def prepare_job_cfg(job_dir, build_env_cfg, repos_cfg, repo_id, software_subdir, jobcfg_dir = os.path.join(job_dir, 'cfg') # create ini file job.cfg with entries: - # [site_config] + # [site_config # local_tmp = LOCAL_TMP_VALUE # shared_fs_path = SHARED_FS_PATH # build_logs_dir = BUILD_LOGS_DIR From db69f09ea86165e0382f2c55a5723562933a314a Mon Sep 17 00:00:00 2001 From: Neves-P Date: Thu, 16 Nov 2023 10:37:17 +0100 Subject: [PATCH 02/50] Improve message and fix typo --- tasks/build.py | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/tasks/build.py b/tasks/build.py index 0a597406..682f3081 100644 --- a/tasks/build.py +++ b/tasks/build.py @@ -464,13 +464,15 @@ def prepare_jobs(pr, cfg, event_info, action_filter): pr, job_dir) if download_pr_exit_code != 0: - download_comment = f"Download failed with error: {download_pr_error}" - download_comment = pr_comments.create_comment(repo_name=base_repo_name,pr_number=pr.number,comment=download_comment) + download_comment = f"""Unable to download or merge changes between source branch and the destination branch. Error: \n + {download_pr_error}\n + Tip: This can usually be resolved by syncing your branch""" + download_comment = pr_comments.create_comment(repo_name=base_repo_name, pr_number=pr.number, comment=download_comment) if download_comment: log(f"{fn}(): created PR issue comment with id {download_comment.id}") else: - log(f"{fn}(): failed to create PR issue comment ") - raise RuntimeError(download_pr_error) + log(f"{fn}(): failed to create PR issue comment") + raise RuntimeError(download_pr_error) # prepare job configuration file 'job.cfg' in directory /cfg cpu_target = '/'.join(arch.split('/')[1:]) @@ -508,7 +510,7 @@ def prepare_job_cfg(job_dir, build_env_cfg, repos_cfg, repo_id, software_subdir, jobcfg_dir = os.path.join(job_dir, 'cfg') # create ini file job.cfg with entries: - # [site_config + # [site_config] # local_tmp = LOCAL_TMP_VALUE # shared_fs_path = SHARED_FS_PATH # build_logs_dir = BUILD_LOGS_DIR From b688574a01ddb09beb79269ed41d0ac44cb4ebb7 Mon Sep 17 00:00:00 2001 From: Neves-P Date: Thu, 16 Nov 2023 10:40:11 +0100 Subject: [PATCH 03/50] Yet more comment formatting --- tasks/build.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tasks/build.py b/tasks/build.py index 682f3081..f3a4c1f6 100644 --- a/tasks/build.py +++ b/tasks/build.py @@ -464,9 +464,9 @@ def prepare_jobs(pr, cfg, event_info, action_filter): pr, job_dir) if download_pr_exit_code != 0: - download_comment = f"""Unable to download or merge changes between source branch and the destination branch. Error: \n - {download_pr_error}\n - Tip: This can usually be resolved by syncing your branch""" + download_comment = f"""{download_pr_error} + Unable to download or merge changes between source branch and the destination branch. + Tip: This can usually be resolved by syncing your branch""" download_comment = pr_comments.create_comment(repo_name=base_repo_name, pr_number=pr.number, comment=download_comment) if download_comment: log(f"{fn}(): created PR issue comment with id {download_comment.id}") From 1b65a3a188361e626c8079382e070e1c6f508b2d Mon Sep 17 00:00:00 2001 From: Neves-P Date: Thu, 16 Nov 2023 10:45:15 +0100 Subject: [PATCH 04/50] More comment improvements --- tasks/build.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tasks/build.py b/tasks/build.py index f3a4c1f6..fd8dc240 100644 --- a/tasks/build.py +++ b/tasks/build.py @@ -464,9 +464,9 @@ def prepare_jobs(pr, cfg, event_info, action_filter): pr, job_dir) if download_pr_exit_code != 0: - download_comment = f"""{download_pr_error} - Unable to download or merge changes between source branch and the destination branch. - Tip: This can usually be resolved by syncing your branch""" + download_comment = (f"{download_pr_error}" + f"\nUnable to download or merge changes between source branch and the destination branch.\n" + f"\nTip: This can usually be resolved by syncing your branch and solving any merge conflics") download_comment = pr_comments.create_comment(repo_name=base_repo_name, pr_number=pr.number, comment=download_comment) if download_comment: log(f"{fn}(): created PR issue comment with id {download_comment.id}") From e3657a912f929290c3bc26c8daeb9d09733ed4f8 Mon Sep 17 00:00:00 2001 From: Neves-P Date: Thu, 16 Nov 2023 11:06:43 +0100 Subject: [PATCH 05/50] Tiny formatting change --- tasks/build.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tasks/build.py b/tasks/build.py index fd8dc240..6419d70e 100644 --- a/tasks/build.py +++ b/tasks/build.py @@ -464,9 +464,9 @@ def prepare_jobs(pr, cfg, event_info, action_filter): pr, job_dir) if download_pr_exit_code != 0: - download_comment = (f"{download_pr_error}" - f"\nUnable to download or merge changes between source branch and the destination branch.\n" - f"\nTip: This can usually be resolved by syncing your branch and solving any merge conflics") + download_comment = (f"`{download_pr_error}`" + f"\nUnable to download or merge changes between the source branch and the destination branch.\n" + f"\nTip: This can usually be resolved by syncing your branch and solving any merge conflics.") download_comment = pr_comments.create_comment(repo_name=base_repo_name, pr_number=pr.number, comment=download_comment) if download_comment: log(f"{fn}(): created PR issue comment with id {download_comment.id}") From e7c6d659709d5b4b3a59f30f0e4514d763ad812a Mon Sep 17 00:00:00 2001 From: Neves-P Date: Wed, 29 Nov 2023 09:35:42 +0100 Subject: [PATCH 06/50] Comment changes --- tasks/build.py | 45 ++++++++++++++++++++-------- tests/test_task_build.py | 64 +++++++++++++++++++++++++++++++++++++++- 2 files changed, 96 insertions(+), 13 deletions(-) diff --git a/tasks/build.py b/tasks/build.py index 6419d70e..b1da385e 100644 --- a/tasks/build.py +++ b/tasks/build.py @@ -345,6 +345,7 @@ def download_pr(repo_name, branch_name, pr, arch_job_dir): # - 'curl' diff for pull request # - 'git apply' diff file git_clone_cmd = ' '.join(['git clone', f'https://github.com/{repo_name}', arch_job_dir]) + print(f'cloning with command {git_clone_cmd}') clone_output, clone_error, clone_exit_code = run_cmd(git_clone_cmd, "Clone repo", arch_job_dir, raise_on_error=False) if clone_exit_code != 0: return clone_output, clone_error, clone_exit_code @@ -353,20 +354,51 @@ def download_pr(repo_name, branch_name, pr, arch_job_dir): 'git checkout', branch_name, ]) + print(f'checking out with command {git_checkout_cmd}') checkout_output, checkout_err, checkout_exit_code = run_cmd(git_checkout_cmd, "checkout branch '%s'" % branch_name, arch_job_dir, raise_on_error=False) if checkout_exit_code != 0: return checkout_output, checkout_err, checkout_exit_code curl_cmd = f'curl -L https://github.com/{repo_name}/pull/{pr.number}.diff > {pr.number}.diff' + print(f'curl with command {curl_cmd}') curl_output, curl_error, curl_exit_code = run_cmd(curl_cmd, "Obtain patch", arch_job_dir, raise_on_error=False) if curl_exit_code != 0: return curl_output, curl_error, curl_exit_code git_apply_cmd = f'git apply {pr.number}.diff' + print(f'git apply with command {git_apply_cmd}') git_apply_output, git_apply_error, git_apply_exit_code = run_cmd(git_apply_cmd, "Apply patch", arch_job_dir, raise_on_error=False) if git_apply_exit_code != 0: return git_apply_output, git_apply_error, git_apply_exit_code + + +def comment_download_pr(base_repo_name, pr, download_pr_exit_code, download_pr_error): + """ + Handle download_pr() exit code and write helpful comment to PR in case of failure + + Args: + base_repo_name (string): name of the repository (format USER_OR_ORGANISATION/REPOSITORY) + pr (github.PullRequest.PullRequest): instance representing the pull request + download_pr_exit_code (int): exit code from download_pr(). 0 if all tasks were successful, + otherwise it corresponds to the error codes of git clone, git checkout, git apply, or curl. + download_pr_error (string): none, or the output of stderr from git clone, git checkout, git apply or curl. + + Return: + None (implicitly). A comment is created in the appropriate PR. + + """ + if download_pr_exit_code != 0: + fn = sys._getframe().f_code.co_name + download_comment = (f"`{download_pr_error}`" + f"\nUnable to download or merge changes between the source branch and the destination branch.\n" + f"\nTip: This can usually be resolved by syncing your branch and resolving any merge conflicts.") + download_comment = pr_comments.create_comment(repo_name=base_repo_name, pr_number=pr.number, comment=download_comment) + if download_comment: + log(f"{fn}(): created PR issue comment with id {download_comment.id}") + else: + log(f"{fn}(): failed to create PR issue comment") + raise ValueError("Unable to download PR and/or sync changes") def apply_cvmfs_customizations(cvmfs_customizations, arch_job_dir): """ @@ -462,18 +494,7 @@ def prepare_jobs(pr, cfg, event_info, action_filter): # TODO optimisation? download once, copy and cleanup initial copy? download_pr_output, download_pr_error, download_pr_exit_code = download_pr(base_repo_name, base_branch_name, pr, job_dir) - - if download_pr_exit_code != 0: - download_comment = (f"`{download_pr_error}`" - f"\nUnable to download or merge changes between the source branch and the destination branch.\n" - f"\nTip: This can usually be resolved by syncing your branch and solving any merge conflics.") - download_comment = pr_comments.create_comment(repo_name=base_repo_name, pr_number=pr.number, comment=download_comment) - if download_comment: - log(f"{fn}(): created PR issue comment with id {download_comment.id}") - else: - log(f"{fn}(): failed to create PR issue comment") - raise RuntimeError(download_pr_error) - + comment_download_pr(base_repo_name, pr, download_pr_exit_code, download_pr_error) # prepare job configuration file 'job.cfg' in directory /cfg cpu_target = '/'.join(arch.split('/')[1:]) os_type = arch.split('/')[0] diff --git a/tests/test_task_build.py b/tests/test_task_build.py index d1dd415d..67d1b57f 100644 --- a/tests/test_task_build.py +++ b/tests/test_task_build.py @@ -18,6 +18,7 @@ import re import shutil from unittest.mock import patch +from unittest import TestCase # Third party imports (anything installed into the local Python environment) from collections import namedtuple @@ -25,7 +26,7 @@ import pytest # Local application imports (anything from EESSI/eessi-bot-software-layer) -from tasks.build import Job, create_pr_comment +from tasks.build import Job, create_pr_comment, comment_download_pr, download_pr from tools import run_cmd, run_subprocess from tools.job_metadata import create_metadata_file, read_metadata_file from tools.pr_comments import PRComment, get_submitted_job_comment @@ -474,3 +475,64 @@ def test_create_read_metadata_file(mocked_github, tmpdir): job_id5 = "555" with pytest.raises(TypeError): create_metadata_file(job5, job_id5, pr_comment) + +def test_comment_download_pr(tmpdir): + """Tests for function comment_download_pr.""" + + base_repo_name = "EESSI/software-layer" + pr = MockPullRequest(999) + download_pr_exit_code = 1 + download_pr_error = "Cannot apply patch" + + # Test exception is raised if there was an error + with patch('tools.pr_comments.create_comment') as mock_download_error: + mock_download_error = "error" + with pytest.raises(Exception): + comment_download_pr(base_repo_name, pr, download_pr_exit_code, download_pr_error) + + +def test_download_pr(tmpdir): + """Tests for function download_pr.""" + + base_repo_name = "EESSI/software-layer" + pr = MockPullRequest(12) + # Test git clone error is returned properly + download_pr_exit_code = 1 + invalid_url_part = "invalid_clone_url" + output, error, exit_code = download_pr(repo_name = base_repo_name, + branch_name = invalid_url_part, pr = pr, + arch_job_dir = tmpdir) + assert exit_code == download_pr_exit_code + assert error == "error: pathspec 'invalid_clone_url' did not match any file(s) known to git\n" + + # Test git checkout error is returned properly + shutil.copyfile("tests/1.diff", os.path.join(tmpdir, "1.diff")) + pr = MockPullRequest(12) + download_pr_exit_code = 1 + branch_name = "develop" + + + class CustomMock(object): + calls = 0 + def run_cmd(self, arg): + self.calls += 1 + if self.calls != 2: + return None + else: + return DEFAULT + + run_cmd.side_effect = CustomMock().run_cmd + + + + output, error, exit_code = download_pr(repo_name = base_repo_name, + branch_name = branch_name, pr = pr, + arch_job_dir = tmpdir) + + # assert exit_code == 1 + # assert error == "error: pathspec 'invalid_clone_url' did not match any file(s) known to git\n" + + # Test curl error is returned properly + + # Test git apply error is returned properly + From 1ef2e2feb64cdf994501e02f39adfa1111383129 Mon Sep 17 00:00:00 2001 From: Thomas Roeblitz Date: Sat, 10 Feb 2024 00:31:27 +0100 Subject: [PATCH 07/50] add function that reads PR section from job metadata and use it in job manager --- eessi_bot_job_manager.py | 11 +++-------- tools/job_metadata.py | 25 +++++++++++++++++++++++++ 2 files changed, 28 insertions(+), 8 deletions(-) diff --git a/eessi_bot_job_manager.py b/eessi_bot_job_manager.py index 4830c1e3..ddf90722 100644 --- a/eessi_bot_job_manager.py +++ b/eessi_bot_job_manager.py @@ -42,7 +42,7 @@ from connections import github from tools import config, run_cmd from tools.args import job_manager_parse -from tools.job_metadata import read_metadata_file +from tools.job_metadata import read_job_metadata_from_file, read_metadata_file from tools.pr_comments import get_submitted_job_comment, update_comment @@ -253,7 +253,7 @@ def determine_finished_jobs(self, known_jobs, current_jobs): def read_job_pr_metadata(self, job_metadata_path): """ - Read job metadata file and return the contents of the 'PR' section. + Determine metadata od a job Args: job_metadata_path (string): path to job metadata file @@ -262,12 +262,7 @@ def read_job_pr_metadata(self, job_metadata_path): (ConfigParser): instance of ConfigParser corresponding to the 'PR' section or None """ - # reuse function from module tools.job_metadata to read metadata file - metadata = read_metadata_file(job_metadata_path, self.logfile) - if metadata and "PR" in metadata: - return metadata["PR"] - else: - return None + return read_job_metadata_from_file(job_metadata_path, self.logfile) def read_job_result(self, job_result_file_path): """ diff --git a/tools/job_metadata.py b/tools/job_metadata.py index a90cad90..56116b04 100644 --- a/tools/job_metadata.py +++ b/tools/job_metadata.py @@ -80,3 +80,28 @@ def read_metadata_file(metadata_path, log_file=None): else: log(f"No metadata file found at {metadata_path}.", log_file) return None + + +def read_job_metadata_from_file(filepath, log_file=None):$ + """ + Read job metadata from file + + Args: + filepath (string): path to job metadata file + log_file (string): path to log file + + Returns: + job_metadata (dict): dictionary containing job metadata or None + """ + + metadata = read_metadata_file(filepath, log_file=log_file) + if metadata: + # get PR section + if "PR" in metadata: + metadata_pr = metadata["PR"] + else: + metadata_pr = {} + return metadata_pr + else: + log(f"Metadata file '{filepath}' does not exist or could not be read") + return None From 4f743a2569811bf981d92193c2eae4b0c98daff1 Mon Sep 17 00:00:00 2001 From: Thomas Roeblitz Date: Sat, 10 Feb 2024 00:37:11 +0100 Subject: [PATCH 08/50] removing trailing $ --- tools/job_metadata.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/job_metadata.py b/tools/job_metadata.py index 56116b04..b8cd4f0d 100644 --- a/tools/job_metadata.py +++ b/tools/job_metadata.py @@ -82,7 +82,7 @@ def read_metadata_file(metadata_path, log_file=None): return None -def read_job_metadata_from_file(filepath, log_file=None):$ +def read_job_metadata_from_file(filepath, log_file=None): """ Read job metadata from file From 7ed3d8d32b24da271bbd0ac0e7b513f1e6ee2b9e Mon Sep 17 00:00:00 2001 From: Thomas Roeblitz Date: Sat, 10 Feb 2024 18:58:50 +0100 Subject: [PATCH 09/50] removed method read_job_pr_metadata - call read_job_metadata_from_file from tools/job_metadata.py - moved one test from tests for EESSIBotSoftwareLayerJobManager to tests for tools/job_metadata.py --- eessi_bot_job_manager.py | 19 +++------------- tests/test_eessi_bot_job_manager.py | 24 -------------------- tests/test_tools_job_metadata.py | 34 +++++++++++++++++++++++++++++ 3 files changed, 37 insertions(+), 40 deletions(-) create mode 100644 tests/test_tools_job_metadata.py diff --git a/eessi_bot_job_manager.py b/eessi_bot_job_manager.py index ddf90722..7d40b545 100644 --- a/eessi_bot_job_manager.py +++ b/eessi_bot_job_manager.py @@ -251,19 +251,6 @@ def determine_finished_jobs(self, known_jobs, current_jobs): return finished_jobs - def read_job_pr_metadata(self, job_metadata_path): - """ - Determine metadata od a job - - Args: - job_metadata_path (string): path to job metadata file - - Returns: - (ConfigParser): instance of ConfigParser corresponding to the 'PR' - section or None - """ - return read_job_metadata_from_file(job_metadata_path, self.logfile) - def read_job_result(self, job_result_file_path): """ Read job result file and return the contents of the 'RESULT' section. @@ -345,7 +332,7 @@ def process_new_job(self, new_job): # assuming that a bot job's working directory contains a metadata # file, its existence is used to check if the job belongs to the bot - metadata_pr = self.read_job_pr_metadata(job_metadata_path) + metadata_pr = read_job_metadata_from_file(job_metadata_path, self.logfile) if metadata_pr is None: log(f"No metadata file found at {job_metadata_path} for job {job_id}, so skipping it", @@ -441,7 +428,7 @@ def process_running_jobs(self, running_job): job_metadata_path = os.path.join(job_dir, metadata_file) # check if metadata file exist - metadata_pr = self.read_job_pr_metadata(job_metadata_path) + metadata_pr = read_job_metadata_from_file(job_metadata_path, self.logfile) if metadata_pr is None: raise Exception("Unable to find metadata file") @@ -586,7 +573,7 @@ def process_finished_job(self, finished_job): # obtain id of PR comment to be updated (from file '_bot_jobID.metadata') metadata_file = f"_bot_job{job_id}.metadata" job_metadata_path = os.path.join(new_symlink, metadata_file) - metadata_pr = self.read_job_pr_metadata(job_metadata_path) + metadata_pr = read_job_metadata_from_file(job_metadata_path, self.logfile) if metadata_pr is None: raise Exception("Unable to find metadata file ... skip updating PR comment") diff --git a/tests/test_eessi_bot_job_manager.py b/tests/test_eessi_bot_job_manager.py index c04033b5..2843069a 100644 --- a/tests/test_eessi_bot_job_manager.py +++ b/tests/test_eessi_bot_job_manager.py @@ -10,34 +10,10 @@ # # license: GPLv2 # -import os -import shutil from eessi_bot_job_manager import EESSIBotSoftwareLayerJobManager -def test_read_job_pr_metadata(tmpdir): - # copy needed app.cfg from tests directory - shutil.copyfile("tests/test_app.cfg", "app.cfg") - - # if metadata file does not exist, we should get None as return value - job_manager = EESSIBotSoftwareLayerJobManager() - path = os.path.join(tmpdir, 'test.metadata') - assert job_manager.read_job_pr_metadata(path) is None - - with open(path, 'w') as fp: - fp.write('''[PR] - repo=test - pr_number=12345''') - - metadata_pr = job_manager.read_job_pr_metadata(path) - expected = { - "repo": "test", - "pr_number": "12345", - } - assert metadata_pr == expected - - def test_determine_running_jobs(): job_manager = EESSIBotSoftwareLayerJobManager() diff --git a/tests/test_tools_job_metadata.py b/tests/test_tools_job_metadata.py new file mode 100644 index 00000000..d667cae0 --- /dev/null +++ b/tests/test_tools_job_metadata.py @@ -0,0 +1,34 @@ +# Tests for 'tools/job_metadata.py' of the EESSI build-and-deploy bot, +# see https://github.com/EESSI/eessi-bot-software-layer +# +# The bot helps with requests to add software installations to the +# EESSI software layer, see https://github.com/EESSI/software-layer +# +# author: Thomas Roeblitz (@trz42) +# +# license: GPLv2 +# + +import os +import shutil + +from tools.job_metadata import read_job_metadata_from_file + + +def test_read_job_metadata_from_file(tmpdir): + logfile = os.path.join(tmpdir, 'test_read_job_metadata_from_file.log') + # if metadata file does not exist, we should get None as return value + path = os.path.join(tmpdir, 'test.metadata') + assert read_job_metadata_from_file(path, logfile) is None + + with open(path, 'w') as fp: + fp.write('''[PR] + repo=test + pr_number=12345''') + + metadata_pr = read_job_metadata_from_file(path, logfile) + expected = { + "repo": "test", + "pr_number": "12345", + } + assert metadata_pr == expected From 03891c9a2f1ab200a6b307ffdff50d9e19caa11c Mon Sep 17 00:00:00 2001 From: Thomas Roeblitz Date: Sat, 10 Feb 2024 19:02:57 +0100 Subject: [PATCH 10/50] remove unused import --- tests/test_tools_job_metadata.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/test_tools_job_metadata.py b/tests/test_tools_job_metadata.py index d667cae0..f5542c6f 100644 --- a/tests/test_tools_job_metadata.py +++ b/tests/test_tools_job_metadata.py @@ -10,7 +10,6 @@ # import os -import shutil from tools.job_metadata import read_job_metadata_from_file From ec2a31719bc7087fae2f52243341a6c2ad899b45 Mon Sep 17 00:00:00 2001 From: Thomas Roeblitz Date: Sat, 10 Feb 2024 19:09:36 +0100 Subject: [PATCH 11/50] still need app.cfg in some tests --- tests/test_eessi_bot_job_manager.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tests/test_eessi_bot_job_manager.py b/tests/test_eessi_bot_job_manager.py index 2843069a..bd492919 100644 --- a/tests/test_eessi_bot_job_manager.py +++ b/tests/test_eessi_bot_job_manager.py @@ -11,10 +11,15 @@ # license: GPLv2 # +import shutil + from eessi_bot_job_manager import EESSIBotSoftwareLayerJobManager def test_determine_running_jobs(): + # copy needed app.cfg from tests directory + shutil.copyfile("tests/test_app.cfg", "app.cfg") + job_manager = EESSIBotSoftwareLayerJobManager() assert job_manager.determine_running_jobs({}) == [] From f1ac9a4c2b7f7b6cb16cf472b03f9d9098d10ffb Mon Sep 17 00:00:00 2001 From: Pedro Santos Neves <10762799+Neves-P@users.noreply.github.com> Date: Sun, 11 Feb 2024 17:15:39 +0100 Subject: [PATCH 12/50] Revert changes to test_task_build.py --- tests/test_task_build.py | 48 +--------------------------------------- 1 file changed, 1 insertion(+), 47 deletions(-) diff --git a/tests/test_task_build.py b/tests/test_task_build.py index b2516a02..ec6c4b2b 100644 --- a/tests/test_task_build.py +++ b/tests/test_task_build.py @@ -26,7 +26,7 @@ import pytest # Local application imports (anything from EESSI/eessi-bot-software-layer) -from tasks.build import Job, create_pr_comment, comment_download_pr, download_pr +from tasks.build import Job, create_pr_comment from tools import run_cmd, run_subprocess from tools.job_metadata import create_metadata_file, read_metadata_file from tools.pr_comments import PRComment, get_submitted_job_comment @@ -490,49 +490,3 @@ def test_comment_download_pr(tmpdir): with pytest.raises(Exception): comment_download_pr(base_repo_name, pr, download_pr_exit_code, download_pr_error) - -def test_download_pr(tmpdir): - """Tests for function download_pr.""" - - base_repo_name = "EESSI/software-layer" - pr = MockPullRequest(12) - # Test git clone error is returned properly - download_pr_exit_code = 1 - invalid_url_part = "invalid_clone_url" - output, error, exit_code = download_pr(repo_name = base_repo_name, - branch_name = invalid_url_part, pr = pr, - arch_job_dir = tmpdir) - assert exit_code == download_pr_exit_code - assert error == "error: pathspec 'invalid_clone_url' did not match any file(s) known to git\n" - - # Test git checkout error is returned properly - shutil.copyfile("tests/1.diff", os.path.join(tmpdir, "1.diff")) - pr = MockPullRequest(12) - download_pr_exit_code = 1 - branch_name = "develop" - - - class CustomMock(object): - calls = 0 - def run_cmd(self, arg): - self.calls += 1 - if self.calls != 2: - return None - else: - return DEFAULT - - run_cmd.side_effect = CustomMock().run_cmd - - - - output, error, exit_code = download_pr(repo_name = base_repo_name, - branch_name = branch_name, pr = pr, - arch_job_dir = tmpdir) - - # assert exit_code == 1 - # assert error == "error: pathspec 'invalid_clone_url' did not match any file(s) known to git\n" - - # Test curl error is returned properly - - # Test git apply error is returned properly - From 5a96c4f01493b91e3af40f016b3657d1937ae0ec Mon Sep 17 00:00:00 2001 From: Pedro Santos Neves <10762799+Neves-P@users.noreply.github.com> Date: Sun, 11 Feb 2024 17:16:38 +0100 Subject: [PATCH 13/50] Revert changes to test_task_build.py --- tests/test_task_build.py | 17 +---------------- 1 file changed, 1 insertion(+), 16 deletions(-) diff --git a/tests/test_task_build.py b/tests/test_task_build.py index ec6c4b2b..60ba9adf 100644 --- a/tests/test_task_build.py +++ b/tests/test_task_build.py @@ -26,7 +26,7 @@ import pytest # Local application imports (anything from EESSI/eessi-bot-software-layer) -from tasks.build import Job, create_pr_comment +from tasks.build import Job from tools import run_cmd, run_subprocess from tools.job_metadata import create_metadata_file, read_metadata_file from tools.pr_comments import PRComment, get_submitted_job_comment @@ -475,18 +475,3 @@ def test_create_read_metadata_file(mocked_github, tmpdir): job_id5 = "555" with pytest.raises(TypeError): create_metadata_file(job5, job_id5, pr_comment) - -def test_comment_download_pr(tmpdir): - """Tests for function comment_download_pr.""" - - base_repo_name = "EESSI/software-layer" - pr = MockPullRequest(999) - download_pr_exit_code = 1 - download_pr_error = "Cannot apply patch" - - # Test exception is raised if there was an error - with patch('tools.pr_comments.create_comment') as mock_download_error: - mock_download_error = "error" - with pytest.raises(Exception): - comment_download_pr(base_repo_name, pr, download_pr_exit_code, download_pr_error) - From b82cc0b267dd0e206128175b33dd4282d800f440 Mon Sep 17 00:00:00 2001 From: Pedro Santos Neves <10762799+Neves-P@users.noreply.github.com> Date: Sun, 11 Feb 2024 17:22:16 +0100 Subject: [PATCH 14/50] Revert changes to test_task_build.py --- tests/test_task_build.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/test_task_build.py b/tests/test_task_build.py index 60ba9adf..ea0a6692 100644 --- a/tests/test_task_build.py +++ b/tests/test_task_build.py @@ -18,7 +18,6 @@ import re import shutil from unittest.mock import patch -from unittest import TestCase # Third party imports (anything installed into the local Python environment) from collections import namedtuple @@ -26,7 +25,7 @@ import pytest # Local application imports (anything from EESSI/eessi-bot-software-layer) -from tasks.build import Job +from tasks.build import Job, create_pr_comment from tools import run_cmd, run_subprocess from tools.job_metadata import create_metadata_file, read_metadata_file from tools.pr_comments import PRComment, get_submitted_job_comment From 33128c38194f9038e591f491c59d7271b827ddd3 Mon Sep 17 00:00:00 2001 From: Thomas Roeblitz Date: Mon, 12 Feb 2024 13:29:16 +0100 Subject: [PATCH 15/50] add function to determine comment id for a job and use it - adds function `determine_pr_comment_id` which reads the attribute `pr_comment_id` from the job's metadata file (all current bot instances already provide that attribute when creating the job's metadata file) - the function is then used when successful jobs are determined (in function `determine_successful_jobs`) and the comment id is added to a dictionary that contains information for each job - another function (`determine_tarballs_to_deploy`) adds the comment id to a dictionary storing information about jobs/tarballs to deploy --- tasks/deploy.py | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/tasks/deploy.py b/tasks/deploy.py index 39b85a7b..02f940b0 100644 --- a/tasks/deploy.py +++ b/tasks/deploy.py @@ -26,6 +26,7 @@ from tasks.build import CFG_DIRNAME, JOB_CFG_FILENAME, JOB_REPO_ID, JOB_REPOSITORY from tasks.build import get_build_env_cfg from tools import config, pr_comments, run_cmd +from tools.job_metadata import read_job_metadata_from_file BUCKET_NAME = "bucket_name" @@ -74,6 +75,26 @@ def determine_job_dirs(pr_number): return job_directories +def determine_pr_comment_id(job_dir): + """ + Determines pr_comment_id by reading _bot_job{JOBID}.metadata in job_dir. + + Args: + job_dir (string): working directory of the job + + Returns: + (int): id of comment corresponding to job in pull request or -1 + """ + # assumes that last part of job_dir encodes the job's id + job_id = os.path.basename(os.path.normpath(job_dir)) + job_metadata_file = os.path.join(job_dir, f"_bot_job{job_id}.metadata") + job_metadata = read_job_metadata_from_file(job_metadata_file) + if job_metadata and "pr_comment_id" in job_metadata: + return int(job_metadata["pr_comment_id"]) + else: + return -1 + + def determine_slurm_out(job_dir): """ Determine path to job stdout/err output file for a given job directory. @@ -371,10 +392,13 @@ def determine_successful_jobs(job_dirs): for job_dir in job_dirs: slurm_out = determine_slurm_out(job_dir) eessi_tarballs = determine_eessi_tarballs(job_dir) + pr_comment_id = determine_pr_comment_id(job_dir) + if check_build_status(slurm_out, eessi_tarballs): log(f"{funcname}(): SUCCESSFUL build in '{job_dir}'") successes.append({'job_dir': job_dir, 'slurm_out': slurm_out, + 'pr_comment_id': pr_comment_id, 'eessi_tarballs': eessi_tarballs}) else: log(f"{funcname}(): FAILED build in '{job_dir}'") @@ -448,6 +472,7 @@ def determine_tarballs_to_deploy(successes, upload_policy): if deploy: to_be_deployed[build_target] = {"job_dir": s["job_dir"], + "pr_comment_id": job["pr_comment_id"], "timestamp": timestamp} return to_be_deployed From c3b2d6da351c2b01da9add8d38570381be552405 Mon Sep 17 00:00:00 2001 From: Thomas Roeblitz Date: Mon, 12 Feb 2024 13:40:40 +0100 Subject: [PATCH 16/50] replaced one letter variable name with more descriptive name --- tasks/deploy.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tasks/deploy.py b/tasks/deploy.py index 02f940b0..367c5785 100644 --- a/tasks/deploy.py +++ b/tasks/deploy.py @@ -427,9 +427,9 @@ def determine_tarballs_to_deploy(successes, upload_policy): log(f"{funcname}(): num successful jobs {len(successes)}") to_be_deployed = {} - for s in successes: + for job in successes: # all tarballs for successful job - tarballs = s["eessi_tarballs"] + tarballs = job["eessi_tarballs"] log(f"{funcname}(): num tarballs {len(tarballs)}") # full path to first tarball for successful job @@ -462,7 +462,7 @@ def determine_tarballs_to_deploy(successes, upload_policy): else: deploy = True elif upload_policy == "once": - uploaded = uploaded_before(build_target, s["job_dir"]) + uploaded = uploaded_before(build_target, job["job_dir"]) if uploaded is None: deploy = True else: @@ -471,7 +471,7 @@ def determine_tarballs_to_deploy(successes, upload_policy): f"{indent_fname}has been uploaded through '{uploaded}'") if deploy: - to_be_deployed[build_target] = {"job_dir": s["job_dir"], + to_be_deployed[build_target] = {"job_dir": job["job_dir"], "pr_comment_id": job["pr_comment_id"], "timestamp": timestamp} From e35f462844589b9c4d892d9632b8ceeb22a21ec6 Mon Sep 17 00:00:00 2001 From: Neves-P Date: Mon, 12 Feb 2024 15:56:07 +0100 Subject: [PATCH 17/50] Replace `print()` with `log()` where appropriate --- tasks/build.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tasks/build.py b/tasks/build.py index 86a18b58..5df959fc 100644 --- a/tasks/build.py +++ b/tasks/build.py @@ -346,7 +346,7 @@ def download_pr(repo_name, branch_name, pr, arch_job_dir): # - 'curl' diff for pull request # - 'git apply' diff file git_clone_cmd = ' '.join(['git clone', f'https://github.com/{repo_name}', arch_job_dir]) - print(f'cloning with command {git_clone_cmd}') + log(f'cloning with command {git_clone_cmd}') clone_output, clone_error, clone_exit_code = run_cmd(git_clone_cmd, "Clone repo", arch_job_dir, raise_on_error=False) if clone_exit_code != 0: return clone_output, clone_error, clone_exit_code @@ -355,20 +355,20 @@ def download_pr(repo_name, branch_name, pr, arch_job_dir): 'git checkout', branch_name, ]) - print(f'checking out with command {git_checkout_cmd}') + log(f'checking out with command {git_checkout_cmd}') checkout_output, checkout_err, checkout_exit_code = run_cmd(git_checkout_cmd, "checkout branch '%s'" % branch_name, arch_job_dir, raise_on_error=False) if checkout_exit_code != 0: return checkout_output, checkout_err, checkout_exit_code curl_cmd = f'curl -L https://github.com/{repo_name}/pull/{pr.number}.diff > {pr.number}.diff' - print(f'curl with command {curl_cmd}') + log(f'curl with command {curl_cmd}') curl_output, curl_error, curl_exit_code = run_cmd(curl_cmd, "Obtain patch", arch_job_dir, raise_on_error=False) if curl_exit_code != 0: return curl_output, curl_error, curl_exit_code git_apply_cmd = f'git apply {pr.number}.diff' - print(f'git apply with command {git_apply_cmd}') + log(f'git apply with command {git_apply_cmd}') git_apply_output, git_apply_error, git_apply_exit_code = run_cmd(git_apply_cmd, "Apply patch", arch_job_dir, raise_on_error=False) if git_apply_exit_code != 0: return git_apply_output, git_apply_error, git_apply_exit_code From d92e1da79b9b961b1b696cdeae688f8442bd0f80 Mon Sep 17 00:00:00 2001 From: Neves-P Date: Mon, 12 Feb 2024 16:31:33 +0100 Subject: [PATCH 18/50] Lint code --- tasks/build.py | 44 ++++++++++++++++++++++++++++---------------- 1 file changed, 28 insertions(+), 16 deletions(-) diff --git a/tasks/build.py b/tasks/build.py index 5df959fc..17ea3ad2 100644 --- a/tasks/build.py +++ b/tasks/build.py @@ -347,7 +347,9 @@ def download_pr(repo_name, branch_name, pr, arch_job_dir): # - 'git apply' diff file git_clone_cmd = ' '.join(['git clone', f'https://github.com/{repo_name}', arch_job_dir]) log(f'cloning with command {git_clone_cmd}') - clone_output, clone_error, clone_exit_code = run_cmd(git_clone_cmd, "Clone repo", arch_job_dir, raise_on_error=False) + clone_output, clone_error, clone_exit_code = run_cmd( + git_clone_cmd, "Clone repo", arch_job_dir, raise_on_error=False + ) if clone_exit_code != 0: return clone_output, clone_error, clone_exit_code @@ -356,23 +358,28 @@ def download_pr(repo_name, branch_name, pr, arch_job_dir): branch_name, ]) log(f'checking out with command {git_checkout_cmd}') - checkout_output, checkout_err, checkout_exit_code = run_cmd(git_checkout_cmd, - "checkout branch '%s'" % branch_name, arch_job_dir, raise_on_error=False) + checkout_output, checkout_err, checkout_exit_code = run_cmd( + git_checkout_cmd, "checkout branch '%s'" % branch_name, arch_job_dir, raise_on_error=False + ) if checkout_exit_code != 0: - return checkout_output, checkout_err, checkout_exit_code + return checkout_output, checkout_err, checkout_exit_code curl_cmd = f'curl -L https://github.com/{repo_name}/pull/{pr.number}.diff > {pr.number}.diff' log(f'curl with command {curl_cmd}') - curl_output, curl_error, curl_exit_code = run_cmd(curl_cmd, "Obtain patch", arch_job_dir, raise_on_error=False) + curl_output, curl_error, curl_exit_code = run_cmd( + curl_cmd, "Obtain patch", arch_job_dir, raise_on_error=False + ) if curl_exit_code != 0: - return curl_output, curl_error, curl_exit_code - + return curl_output, curl_error, curl_exit_code + git_apply_cmd = f'git apply {pr.number}.diff' log(f'git apply with command {git_apply_cmd}') - git_apply_output, git_apply_error, git_apply_exit_code = run_cmd(git_apply_cmd, "Apply patch", arch_job_dir, raise_on_error=False) + git_apply_output, git_apply_error, git_apply_exit_code = run_cmd( + git_apply_cmd, "Apply patch", arch_job_dir, raise_on_error=False + ) if git_apply_exit_code != 0: return git_apply_output, git_apply_error, git_apply_exit_code - + def comment_download_pr(base_repo_name, pr, download_pr_exit_code, download_pr_error): """ @@ -381,26 +388,31 @@ def comment_download_pr(base_repo_name, pr, download_pr_exit_code, download_pr_e Args: base_repo_name (string): name of the repository (format USER_OR_ORGANISATION/REPOSITORY) pr (github.PullRequest.PullRequest): instance representing the pull request - download_pr_exit_code (int): exit code from download_pr(). 0 if all tasks were successful, - otherwise it corresponds to the error codes of git clone, git checkout, git apply, or curl. - download_pr_error (string): none, or the output of stderr from git clone, git checkout, git apply or curl. + download_pr_exit_code (int): exit code from download_pr(). 0 if all tasks were successful, + otherwise it corresponds to the error codes of git clone, git checkout, git apply, or curl. + download_pr_error (string): none, or the output of stderr from git clone, git checkout, git apply or curl. Return: None (implicitly). A comment is created in the appropriate PR. - + """ if download_pr_exit_code != 0: fn = sys._getframe().f_code.co_name download_comment = (f"`{download_pr_error}`" - f"\nUnable to download or merge changes between the source branch and the destination branch.\n" - f"\nTip: This can usually be resolved by syncing your branch and resolving any merge conflicts.") - download_comment = pr_comments.create_comment(repo_name=base_repo_name, pr_number=pr.number, comment=download_comment) + f"\nUnable to download or merge changes between the source" + " branch and the destination branch.\n" + f"\nTip: This can usually be resolved by syncing your" + " branch and resolving any merge conflicts.") + download_comment = pr_comments.create_comment( + repo_name=base_repo_name, pr_number=pr.number, comment=download_comment + ) if download_comment: log(f"{fn}(): created PR issue comment with id {download_comment.id}") else: log(f"{fn}(): failed to create PR issue comment") raise ValueError("Unable to download PR and/or sync changes") + def apply_cvmfs_customizations(cvmfs_customizations, arch_job_dir): """ Apply cvmfs_customizations to job From 3213fbe3d23dd81768ca459119005077452080c3 Mon Sep 17 00:00:00 2001 From: Neves-P Date: Mon, 12 Feb 2024 17:07:28 +0100 Subject: [PATCH 19/50] Handle each error stage individually --- tasks/build.py | 54 +++++++++++++++++++++++++++++++++++++------------- 1 file changed, 40 insertions(+), 14 deletions(-) diff --git a/tasks/build.py b/tasks/build.py index 17ea3ad2..e5845737 100644 --- a/tasks/build.py +++ b/tasks/build.py @@ -337,7 +337,9 @@ def download_pr(repo_name, branch_name, pr, arch_job_dir): arch_job_dir (string): working directory of the job to be submitted Returns: - None (implicitly) + None (implicitly), in case an error is caught in the git clone, git checkout, curl, + or git apply commands, returns the output, stderror, exit code and a string + stating which of these commands failed. """ # download pull request to arch_job_dir # - 'git clone' repository into arch_job_dir (NOTE 'git clone' requires that @@ -351,7 +353,8 @@ def download_pr(repo_name, branch_name, pr, arch_job_dir): git_clone_cmd, "Clone repo", arch_job_dir, raise_on_error=False ) if clone_exit_code != 0: - return clone_output, clone_error, clone_exit_code + error_stage = 'git clone' + return clone_output, clone_error, clone_exit_code, error_stage git_checkout_cmd = ' '.join([ 'git checkout', @@ -362,7 +365,8 @@ def download_pr(repo_name, branch_name, pr, arch_job_dir): git_checkout_cmd, "checkout branch '%s'" % branch_name, arch_job_dir, raise_on_error=False ) if checkout_exit_code != 0: - return checkout_output, checkout_err, checkout_exit_code + error_stage = 'git checkout' + return checkout_output, checkout_err, checkout_exit_code, error_stage curl_cmd = f'curl -L https://github.com/{repo_name}/pull/{pr.number}.diff > {pr.number}.diff' log(f'curl with command {curl_cmd}') @@ -370,7 +374,8 @@ def download_pr(repo_name, branch_name, pr, arch_job_dir): curl_cmd, "Obtain patch", arch_job_dir, raise_on_error=False ) if curl_exit_code != 0: - return curl_output, curl_error, curl_exit_code + error_stage = "curl" + return curl_output, curl_error, curl_exit_code, error_stage git_apply_cmd = f'git apply {pr.number}.diff' log(f'git apply with command {git_apply_cmd}') @@ -378,10 +383,11 @@ def download_pr(repo_name, branch_name, pr, arch_job_dir): git_apply_cmd, "Apply patch", arch_job_dir, raise_on_error=False ) if git_apply_exit_code != 0: - return git_apply_output, git_apply_error, git_apply_exit_code + error_stage = 'git apply' + return git_apply_output, git_apply_error, git_apply_exit_code, error_stage -def comment_download_pr(base_repo_name, pr, download_pr_exit_code, download_pr_error): +def comment_download_pr(base_repo_name, pr, download_pr_exit_code, download_pr_error, error_stage): """ Handle download_pr() exit code and write helpful comment to PR in case of failure @@ -391,6 +397,8 @@ def comment_download_pr(base_repo_name, pr, download_pr_exit_code, download_pr_e download_pr_exit_code (int): exit code from download_pr(). 0 if all tasks were successful, otherwise it corresponds to the error codes of git clone, git checkout, git apply, or curl. download_pr_error (string): none, or the output of stderr from git clone, git checkout, git apply or curl. + error_stage (string): a string informing the stage where download_pr() failed. Can be 'git clone', + 'git checkout', 'curl', or 'git apply'. Return: None (implicitly). A comment is created in the appropriate PR. @@ -398,11 +406,28 @@ def comment_download_pr(base_repo_name, pr, download_pr_exit_code, download_pr_e """ if download_pr_exit_code != 0: fn = sys._getframe().f_code.co_name - download_comment = (f"`{download_pr_error}`" - f"\nUnable to download or merge changes between the source" - " branch and the destination branch.\n" - f"\nTip: This can usually be resolved by syncing your" - " branch and resolving any merge conflicts.") + if error_stage == 'git clone': + download_comment = (f"`{download_pr_error}`" + f"\nUnable to clone the target repository." + f"\n_Tip: This could be a connection failure." + f" Try again and if the issue remains check if the address is correct_.") + elif error_stage == 'git checkout': + download_comment = (f"`{download_pr_error}`" + f"\nUnable to checkout to the correct branch." + f"\n_Tip: Ensure that the branch name is correct and the target" + " branch is available._") + elif error_stage == 'curl': + download_comment = (f"`{download_pr_error}`" + f"\nUnable to download the .diff file." + f"\n_Tip: This could be a connection failure." + f" Try again and if the issue remains check if the address is correct_") + elif error_stage == 'git apply': + download_comment = (f"`{download_pr_error}`" + f"\nUnable to download or merge changes between the source" + f" branch and the destination branch.\n" + f"\n_Tip: This can usually be resolved by syncing your" + f" branch and resolving any merge conflicts._") + download_comment = pr_comments.create_comment( repo_name=base_repo_name, pr_number=pr.number, comment=download_comment ) @@ -505,9 +530,10 @@ def prepare_jobs(pr, cfg, event_info, action_filter): log(f"{fn}(): job_dir '{job_dir}'") # TODO optimisation? download once, copy and cleanup initial copy? - download_pr_output, download_pr_error, download_pr_exit_code = download_pr(base_repo_name, base_branch_name, - pr, job_dir) - comment_download_pr(base_repo_name, pr, download_pr_exit_code, download_pr_error) + download_pr_output, download_pr_error, download_pr_exit_code, error_stage = download_pr( + base_repo_name, base_branch_name, pr, job_dir + ) + comment_download_pr(base_repo_name, pr, download_pr_exit_code, download_pr_error, error_stage) # prepare job configuration file 'job.cfg' in directory /cfg cpu_target = '/'.join(arch.split('/')[1:]) os_type = arch.split('/')[0] From 68e739d4f9aad3644dff1f0700cc2e5b1a9df3fc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bob=20Dr=C3=B6ge?= Date: Tue, 13 Feb 2024 09:38:02 +0100 Subject: [PATCH 20/50] determines -> determine (to be consistent with other docstrings) --- tasks/deploy.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tasks/deploy.py b/tasks/deploy.py index 367c5785..1287e2a5 100644 --- a/tasks/deploy.py +++ b/tasks/deploy.py @@ -77,7 +77,7 @@ def determine_job_dirs(pr_number): def determine_pr_comment_id(job_dir): """ - Determines pr_comment_id by reading _bot_job{JOBID}.metadata in job_dir. + Determine pr_comment_id by reading _bot_job{JOBID}.metadata in job_dir. Args: job_dir (string): working directory of the job From 27831a8805add8b047bef7159e538e50bb2a3f4e Mon Sep 17 00:00:00 2001 From: Thomas Roeblitz Date: Tue, 13 Feb 2024 12:08:59 +0100 Subject: [PATCH 21/50] add function that returns an IssueComment instance --- tools/pr_comments.py | 36 ++++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/tools/pr_comments.py b/tools/pr_comments.py index 5c5e6e9b..ee1a537f 100644 --- a/tools/pr_comments.py +++ b/tools/pr_comments.py @@ -16,6 +16,7 @@ # Standard library imports from collections import namedtuple import re +import sys # Third party imports (anything installed into the local Python environment) from pyghee.utils import log @@ -48,6 +49,41 @@ def create_comment(repo_name, pr_number, comment): return pull_request.create_issue_comment(comment) +def determine_issue_comment(pull_request, pr_comment_id, search_pattern=None): + """ + Determine issue comment for a given id or using a search pattern. + + Args: + pull_request (github.PullRequest.PullRequest): instance representing the pull request + pr_comment_id (int): number of the comment to the pull request to be returned + search_pattern (string): pattern used to determine the comment to the pull request to be returned + + Returns: + github.IssueComment.IssueComment instance or None (note, github refers to + PyGithub, not the github from the internal connections module) + """ + + fn = sys._getframe().f_code.co_name + + if pr_comment_id != -1: + return pull_request.get_issue_comment(pr_comment_id) + else: + # use search pattern to determine issue comment + comments = pull_request.get_issue_comments() + for comment in comments: + # NOTE + # adjust search string if format changed by event handler + # (separate process running eessi_bot_event_handler.py) + re_pattern = f".*{search_pattern}.*" + comment_match = re.search(re_pattern, comment.body) + + if comment_match: + log(f"{fn}(): found comment with id {comment.id}") + return comment + + return None + + @retry(Exception, tries=5, delay=1, backoff=2, max_delay=30) def get_comment(pr, search_pattern): """ From e8cf2198e7dcb77dfffd178f2797ef7a842f8110 Mon Sep 17 00:00:00 2001 From: Thomas Roeblitz Date: Tue, 13 Feb 2024 12:28:22 +0100 Subject: [PATCH 22/50] use determine_issue_comment when updating comment --- tasks/deploy.py | 36 +++++++++--------------------------- 1 file changed, 9 insertions(+), 27 deletions(-) diff --git a/tasks/deploy.py b/tasks/deploy.py index 1287e2a5..b51acde9 100644 --- a/tasks/deploy.py +++ b/tasks/deploy.py @@ -188,9 +188,9 @@ def check_build_status(slurm_out, eessi_tarballs): return False -def update_pr_comment(tarball, repo_name, pr_number, state, msg): +def update_pr_comment(tarball, repo_name, pr_number, pr_comment_id, state, msg): """ - Update pull request comment which contains specific tarball name. + Update pull request comment for the given comment id or tarball name Args: tarball (string): name of tarball that is looked for in a PR comment @@ -202,36 +202,18 @@ def update_pr_comment(tarball, repo_name, pr_number, state, msg): Returns: None (implicitly) """ - funcname = sys._getframe().f_code.co_name - gh = github.get_instance() repo = gh.get_repo(repo_name) pull_request = repo.get_pull(pr_number) - # TODO does this always return all comments? - comments = pull_request.get_issue_comments() - for comment in comments: - # NOTE - # adjust search string if format changed by event handler - # (separate process running eessi_bot_event_handler.py) - re_tarball = f".*{tarball}.*" - comment_match = re.search(re_tarball, comment.body) - - if comment_match: - log(f"{funcname}(): found comment with id {comment.id}") - - issue_comment = pull_request.get_issue_comment(int(comment.id)) - - dt = datetime.now(timezone.utc) - comment_update = (f"\n|{dt.strftime('%b %d %X %Z %Y')}|{state}|" - f"transfer of `{tarball}` to S3 bucket {msg}|") - - # append update to existing comment - issue_comment.edit(issue_comment.body + comment_update) + issue_comment = pr_comments.determine_issue_comment(pull_request, pr_comment_id, tarball) + if issue_comment: + dt = datetime.now(timezone.utc) + comment_update = (f"\n|{dt.strftime('%b %d %X %Z %Y')}|{state}|" + f"transfer of `{tarball}` to S3 bucket {msg}|") - # leave 'for' loop (only update one comment, because tarball - # should only be referenced in one comment) - break + # append update to existing comment + issue_comment.edit(issue_comment.body + comment_update) def append_tarball_to_upload_log(tarball, job_dir): From 8311a5d6406be5c64c732ca69d65c5ed6bde9826 Mon Sep 17 00:00:00 2001 From: Thomas Roeblitz Date: Tue, 13 Feb 2024 12:30:55 +0100 Subject: [PATCH 23/50] simpliify determine_issue_comment by reusing existing function get_comment --- tools/pr_comments.py | 14 +------------- 1 file changed, 1 insertion(+), 13 deletions(-) diff --git a/tools/pr_comments.py b/tools/pr_comments.py index ee1a537f..890a611c 100644 --- a/tools/pr_comments.py +++ b/tools/pr_comments.py @@ -69,19 +69,7 @@ def determine_issue_comment(pull_request, pr_comment_id, search_pattern=None): return pull_request.get_issue_comment(pr_comment_id) else: # use search pattern to determine issue comment - comments = pull_request.get_issue_comments() - for comment in comments: - # NOTE - # adjust search string if format changed by event handler - # (separate process running eessi_bot_event_handler.py) - re_pattern = f".*{search_pattern}.*" - comment_match = re.search(re_pattern, comment.body) - - if comment_match: - log(f"{fn}(): found comment with id {comment.id}") - return comment - - return None + return get_comment(pull_request, search_pattern) @retry(Exception, tries=5, delay=1, backoff=2, max_delay=30) From ed5500a843a21cfceab8c5b39c02b1798854b45e Mon Sep 17 00:00:00 2001 From: Thomas Roeblitz Date: Tue, 13 Feb 2024 12:37:12 +0100 Subject: [PATCH 24/50] removed unused variable and import --- tools/pr_comments.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/tools/pr_comments.py b/tools/pr_comments.py index 890a611c..1b391ed7 100644 --- a/tools/pr_comments.py +++ b/tools/pr_comments.py @@ -16,7 +16,6 @@ # Standard library imports from collections import namedtuple import re -import sys # Third party imports (anything installed into the local Python environment) from pyghee.utils import log @@ -63,8 +62,6 @@ def determine_issue_comment(pull_request, pr_comment_id, search_pattern=None): PyGithub, not the github from the internal connections module) """ - fn = sys._getframe().f_code.co_name - if pr_comment_id != -1: return pull_request.get_issue_comment(pr_comment_id) else: From f0157a90ba3a155cc6a8b1d03d52b76c6aacf727 Mon Sep 17 00:00:00 2001 From: vsc46128 vscuser Date: Tue, 13 Feb 2024 15:28:46 +0100 Subject: [PATCH 25/50] fix for was calling wrong values --- tasks/build.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tasks/build.py b/tasks/build.py index 1ce8e956..e6f493b9 100644 --- a/tasks/build.py +++ b/tasks/build.py @@ -794,7 +794,7 @@ def request_bot_build_issue_comments(repo_name, pr_number): first_line = comment['body'].split('\n')[0] arch_map = get_architecture_targets(cfg) for arch in arch_map.keys(): - target_arch = '/'.join(arch.split('/')[1:]) + target_arch = '/'.join(arch.split('/')[-1:]) if target_arch in first_line: status_table['arch'].append(target_arch) @@ -826,7 +826,7 @@ def request_bot_build_issue_comments(repo_name, pr_number): status_table['url'].append(comment['html_url']) if 'FAILURE' in row['comment']: status_table['result'].append(':cry: FAILURE') - elif 'SUCCESS' in value['comment']: + elif 'SUCCESS' in row['comment']: status_table['result'].append(':grin: SUCCESS') elif 'UNKNOWN' in row['comment']: status_table['result'].append(':shrug: UNKNOWN') From 0dc36501e903d6bc29f4db81ffe511f5c20c24dd Mon Sep 17 00:00:00 2001 From: Lara Ramona Peeters <49882639+laraPPr@users.noreply.github.com> Date: Tue, 13 Feb 2024 17:28:25 +0100 Subject: [PATCH 26/50] Update tasks/build.py MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Thomas Röblitz --- tasks/build.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tasks/build.py b/tasks/build.py index e6f493b9..e35b3044 100644 --- a/tasks/build.py +++ b/tasks/build.py @@ -794,7 +794,7 @@ def request_bot_build_issue_comments(repo_name, pr_number): first_line = comment['body'].split('\n')[0] arch_map = get_architecture_targets(cfg) for arch in arch_map.keys(): - target_arch = '/'.join(arch.split('/')[-1:]) + target_arch = '/'.join(arch.split('/')[-1]) if target_arch in first_line: status_table['arch'].append(target_arch) From 92edf24385cd4d51f2eb2692794b20823f60631e Mon Sep 17 00:00:00 2001 From: Thomas Roeblitz Date: Tue, 13 Feb 2024 18:26:11 +0100 Subject: [PATCH 27/50] pass along and use pr_comment_id in two more functions --- tasks/deploy.py | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/tasks/deploy.py b/tasks/deploy.py index b51acde9..42ce66e7 100644 --- a/tasks/deploy.py +++ b/tasks/deploy.py @@ -235,7 +235,7 @@ def append_tarball_to_upload_log(tarball, job_dir): upload_log.write(f"{job_plus_tarball}\n") -def upload_tarball(job_dir, build_target, timestamp, repo_name, pr_number): +def upload_tarball(job_dir, build_target, timestamp, repo_name, pr_number, pr_comment_id): """ Upload built tarball to an S3 bucket. @@ -245,6 +245,7 @@ def upload_tarball(job_dir, build_target, timestamp, repo_name, pr_number): timestamp (int): timestamp of the tarball repo_name (string): repository of the pull request pr_number (int): number of the pull request + pr_comment_id (int): id of the pull request comment Returns: None (implicitly) @@ -277,13 +278,13 @@ def upload_tarball(job_dir, build_target, timestamp, repo_name, pr_number): # bucket spec may be a mapping of target repo id to bucket name bucket_name = bucket_spec.get(target_repo_id) if bucket_name is None: - update_pr_comment(tarball, repo_name, pr_number, "not uploaded", + update_pr_comment(tarball, repo_name, pr_number, pr_comment_id, "not uploaded", f"failed (no bucket specified for {target_repo_id})") return else: log(f"Using bucket for {target_repo_id}: {bucket_name}") else: - update_pr_comment(tarball, repo_name, pr_number, "not uploaded", + update_pr_comment(tarball, repo_name, pr_number, pr_comment_id, "not uploaded", f"failed (incorrect bucket spec: {bucket_spec})") return @@ -310,11 +311,11 @@ def upload_tarball(job_dir, build_target, timestamp, repo_name, pr_number): # add file to 'job_dir/../uploaded.txt' append_tarball_to_upload_log(tarball, job_dir) # update pull request comment - update_pr_comment(tarball, repo_name, pr_number, "uploaded", + update_pr_comment(tarball, repo_name, pr_number, pr_comment_id, "uploaded", "succeeded") else: # update pull request comment - update_pr_comment(tarball, repo_name, pr_number, "not uploaded", + update_pr_comment(tarball, repo_name, pr_number, pr_comment_id, "not uploaded", "failed") @@ -525,4 +526,5 @@ def deploy_built_artefacts(pr, event_info): for target, job in to_be_deployed.items(): job_dir = job['job_dir'] timestamp = job['timestamp'] - upload_tarball(job_dir, target, timestamp, repo_name, pr.number) + pr_comment_id = job['pr_comment_id'] + upload_tarball(job_dir, target, timestamp, repo_name, pr.number, pr_comment_id) From 570217f8238924f03227ac6902f511830475ffe0 Mon Sep 17 00:00:00 2001 From: Neves-P Date: Wed, 14 Feb 2024 16:01:20 +0100 Subject: [PATCH 28/50] Use globaly defined string constants for errors --- app.cfg.example | 2 ++ tasks/build.py | 20 ++++++++++++-------- 2 files changed, 14 insertions(+), 8 deletions(-) diff --git a/app.cfg.example b/app.cfg.example index 7a6a1073..57d09bbb 100644 --- a/app.cfg.example +++ b/app.cfg.example @@ -219,3 +219,5 @@ no_matching_tarball = No tarball matching `{tarball_pattern}` found in job dir. multiple_tarballs = Found {num_tarballs} tarballs in job dir - only 1 matching `{tarball_pattern}` expected. job_result_unknown_fmt =
:shrug: UNKNOWN _(click triangle for detailed information)_
  • Job results file `{filename}` does not exist in job directory, or parsing it failed.
  • No artefacts were found/reported.
job_test_unknown_fmt =
:shrug: UNKNOWN _(click triangle for detailed information)_
  • Job test file `{filename}` does not exist in job directory, or parsing it failed.
+ +[download_pr_comments] diff --git a/tasks/build.py b/tasks/build.py index e5845737..87f0cd2d 100644 --- a/tasks/build.py +++ b/tasks/build.py @@ -42,6 +42,10 @@ CONTAINER_CACHEDIR = "container_cachedir" CVMFS_CUSTOMIZATIONS = "cvmfs_customizations" DEFAULT_JOB_TIME_LIMIT = "24:00:00" +ERROR_CURL = "curl" +ERROR_GIT_APPLY = "git apply" +ERROR_GIT_CHECKOUT = "git checkout" +ERROR_GIT_CLONE = "curl" GITHUB = "github" HTTPS_PROXY = "https_proxy" HTTP_PROXY = "http_proxy" @@ -353,7 +357,7 @@ def download_pr(repo_name, branch_name, pr, arch_job_dir): git_clone_cmd, "Clone repo", arch_job_dir, raise_on_error=False ) if clone_exit_code != 0: - error_stage = 'git clone' + error_stage = ERROR_GIT_CLONE return clone_output, clone_error, clone_exit_code, error_stage git_checkout_cmd = ' '.join([ @@ -365,7 +369,7 @@ def download_pr(repo_name, branch_name, pr, arch_job_dir): git_checkout_cmd, "checkout branch '%s'" % branch_name, arch_job_dir, raise_on_error=False ) if checkout_exit_code != 0: - error_stage = 'git checkout' + error_stage = ERROR_GIT_CHECKOUT return checkout_output, checkout_err, checkout_exit_code, error_stage curl_cmd = f'curl -L https://github.com/{repo_name}/pull/{pr.number}.diff > {pr.number}.diff' @@ -374,7 +378,7 @@ def download_pr(repo_name, branch_name, pr, arch_job_dir): curl_cmd, "Obtain patch", arch_job_dir, raise_on_error=False ) if curl_exit_code != 0: - error_stage = "curl" + error_stage = ERROR_CURL return curl_output, curl_error, curl_exit_code, error_stage git_apply_cmd = f'git apply {pr.number}.diff' @@ -383,7 +387,7 @@ def download_pr(repo_name, branch_name, pr, arch_job_dir): git_apply_cmd, "Apply patch", arch_job_dir, raise_on_error=False ) if git_apply_exit_code != 0: - error_stage = 'git apply' + error_stage = ERROR_GIT_APPLY return git_apply_output, git_apply_error, git_apply_exit_code, error_stage @@ -406,22 +410,22 @@ def comment_download_pr(base_repo_name, pr, download_pr_exit_code, download_pr_e """ if download_pr_exit_code != 0: fn = sys._getframe().f_code.co_name - if error_stage == 'git clone': + if error_stage == ERROR_GIT_CLONE: download_comment = (f"`{download_pr_error}`" f"\nUnable to clone the target repository." f"\n_Tip: This could be a connection failure." f" Try again and if the issue remains check if the address is correct_.") - elif error_stage == 'git checkout': + elif error_stage == ERROR_GIT_CHECKOUT: download_comment = (f"`{download_pr_error}`" f"\nUnable to checkout to the correct branch." f"\n_Tip: Ensure that the branch name is correct and the target" " branch is available._") - elif error_stage == 'curl': + elif error_stage == ERROR_CURL: download_comment = (f"`{download_pr_error}`" f"\nUnable to download the .diff file." f"\n_Tip: This could be a connection failure." f" Try again and if the issue remains check if the address is correct_") - elif error_stage == 'git apply': + elif error_stage == ERROR_GIT_APPLY: download_comment = (f"`{download_pr_error}`" f"\nUnable to download or merge changes between the source" f" branch and the destination branch.\n" From fd6cda1239cbf48bf7d5fa250b8f2c99dd80738a Mon Sep 17 00:00:00 2001 From: Neves-P Date: Wed, 14 Feb 2024 16:36:09 +0100 Subject: [PATCH 29/50] Move text snippets to cfg_file --- app.cfg.example | 8 ++++++++ tasks/build.py | 32 +++++++++++++++++++------------- 2 files changed, 27 insertions(+), 13 deletions(-) diff --git a/app.cfg.example b/app.cfg.example index 57d09bbb..1888e499 100644 --- a/app.cfg.example +++ b/app.cfg.example @@ -221,3 +221,11 @@ job_result_unknown_fmt =
:shrug: UNKNOWN _(click triangle for job_test_unknown_fmt =
:shrug: UNKNOWN _(click triangle for detailed information)_
  • Job test file `{filename}` does not exist in job directory, or parsing it failed.
[download_pr_comments] +git_clone_failure = Unable to clone the target repository. +git_clone_tip = _Tip: This could be a connection failure. Try again and if the issue remains check if the address is correct_. +git_checkout_failure = Unable to checkout to the correct branch. +git_checkout_tip = _Tip: Ensure that the branch name is correct and the target branch is available._ +curl_failure = Unable to download the `.diff` file. +curl_tip = _Tip: This could be a connection failure. Try again and if the issue remains check if the address is correct_ +git_apply_failure = Unable to download or merge changes between the source branch and the destination branch. +git_apply_tip = _Tip: This can usually be resolved by syncing your branch and resolving any merge conflicts._ diff --git a/tasks/build.py b/tasks/build.py index 87f0cd2d..473599b6 100644 --- a/tasks/build.py +++ b/tasks/build.py @@ -40,13 +40,22 @@ BUILD_PERMISSION = "build_permission" CFG_DIRNAME = "cfg" CONTAINER_CACHEDIR = "container_cachedir" +CURL_FAILURE = "curl_failure" +CURL_TIP = "curl_tip" CVMFS_CUSTOMIZATIONS = "cvmfs_customizations" DEFAULT_JOB_TIME_LIMIT = "24:00:00" +DOWNLOAD_PR_COMMENTS = "download_pr_comments" ERROR_CURL = "curl" ERROR_GIT_APPLY = "git apply" ERROR_GIT_CHECKOUT = "git checkout" ERROR_GIT_CLONE = "curl" GITHUB = "github" +GIT_CLONE_FAILURE = "git_clone_failure" +GIT_CLONE_TIP = "git_clone_tip" +GIT_CHECKOUT_FAILURE = "git_checkout_failure" +GIT_CHECKOUT_TIP = "git_checkout_tip" +GIT_APPLY_FAILURE = "git_apply_failure" +GIT_APPLY_TIP = "git_apply_tip" HTTPS_PROXY = "https_proxy" HTTP_PROXY = "http_proxy" INITIAL_COMMENT = "initial_comment" @@ -410,27 +419,24 @@ def comment_download_pr(base_repo_name, pr, download_pr_exit_code, download_pr_e """ if download_pr_exit_code != 0: fn = sys._getframe().f_code.co_name + + download_pr_comments_cfg = config.read_config()[DOWNLOAD_PR_COMMENTS] if error_stage == ERROR_GIT_CLONE: download_comment = (f"`{download_pr_error}`" - f"\nUnable to clone the target repository." - f"\n_Tip: This could be a connection failure." - f" Try again and if the issue remains check if the address is correct_.") + f"{download_pr_comments_cfg[GIT_CLONE_FAILURE]}" + f"{download_pr_comments_cfg[GIT_CLONE_TIP]}") elif error_stage == ERROR_GIT_CHECKOUT: download_comment = (f"`{download_pr_error}`" - f"\nUnable to checkout to the correct branch." - f"\n_Tip: Ensure that the branch name is correct and the target" - " branch is available._") + f"{download_pr_comments_cfg[GIT_CHECKOUT_FAILURE]}" + f"{download_pr_comments_cfg[GIT_CHECKOUT_TIP]}") elif error_stage == ERROR_CURL: download_comment = (f"`{download_pr_error}`" - f"\nUnable to download the .diff file." - f"\n_Tip: This could be a connection failure." - f" Try again and if the issue remains check if the address is correct_") + f"{download_pr_comments_cfg[CURL_FAILURE]}" + f"{download_pr_comments_cfg[CURL_TIP]}") elif error_stage == ERROR_GIT_APPLY: download_comment = (f"`{download_pr_error}`" - f"\nUnable to download or merge changes between the source" - f" branch and the destination branch.\n" - f"\n_Tip: This can usually be resolved by syncing your" - f" branch and resolving any merge conflicts._") + f"{download_pr_comments_cfg[GIT_APPLY_FAILURE]}" + f"{download_pr_comments_cfg[GIT_APPLY_TIP]}") download_comment = pr_comments.create_comment( repo_name=base_repo_name, pr_number=pr.number, comment=download_comment From fb9e4d95f27936b84911a31b8fe17862e3d74af0 Mon Sep 17 00:00:00 2001 From: Thomas Roeblitz Date: Wed, 14 Feb 2024 22:58:24 +0100 Subject: [PATCH 30/50] add PR comment id to uploaded metadata - passes PR comment id to script used for uploading tarball and metadata file to S3 bucket - script extended to use additional argument and to add the PR comment id to the uploaded metadata file --- scripts/eessi-upload-to-staging | 22 ++++++++++++++++++---- tasks/deploy.py | 1 + 2 files changed, 19 insertions(+), 4 deletions(-) diff --git a/scripts/eessi-upload-to-staging b/scripts/eessi-upload-to-staging index 16b9231e..3638eeb2 100755 --- a/scripts/eessi-upload-to-staging +++ b/scripts/eessi-upload-to-staging @@ -42,6 +42,7 @@ function create_metadata_file _url=$2 _repository=$3 _pull_request=$4 + _pull_request_comment_id=$5 _tmpfile=$(mktemp) @@ -56,10 +57,11 @@ function create_metadata_file --arg url "${_url}" \ --arg repo "${_repository}" \ --arg pr "${_pull_request}" \ + --arg pr_comment_id "${_pull_request_comment_id}" \ '{ uploader: {username: $un, ip: $ip, hostname: $hn}, payload: {filename: $fn, size: $sz, ctime: $ct, sha256sum: $sha256, url: $url}, - link2pr: {repo: $repo, pr: $pr}, + link2pr: {repo: $repo, pr: $pr, pr_comment_id: $pr_commend_id}, }' > "${_tmpfile}" echo "${_tmpfile}" @@ -70,6 +72,10 @@ function display_help echo "Usage: $0 [OPTIONS] " >&2 echo " -e | --endpoint-url URL - endpoint url (needed for non AWS S3)" >&2 echo " -h | --help - display this usage information" >&2 + echo " -i | --pr-comment-id - identifier of a PR comment; may be" >&2 + echo " used to efficiently determine the PR" >&2 + echo " comment to be updated during the" >&2 + echo " ingestion procedure" >&2 echo " -n | --bucket-name BUCKET - bucket name (same as BUCKET above)" >&2 echo " -p | --pull-request NUMBER - a pull request NUMBER; used to" >&2 echo " link the upload to a PR" >&2 @@ -97,8 +103,11 @@ bucket_name="eessi-staging" # provided via options in the bot's config file app.cfg endpoint_url= -pull_request= -repository= + +# provided via command line arguments +pr_comment_id="none" +pull_request="none" +repository="EESSI/software-layer" while [[ $# -gt 0 ]]; do case $1 in @@ -110,6 +119,10 @@ while [[ $# -gt 0 ]]; do display_help exit 0 ;; + -i|--pr-comment-id) + pr_comment_id="$2" + shift 2 + ;; -n|--bucket-name) bucket_name="$2" shift 2 @@ -161,7 +174,8 @@ for file in "$*"; do echo "Creating metadata file" url="${bucket_base}/${aws_path}/${aws_file}" metadata_file=$(create_metadata_file "${file}" "${url}" \ - "${repository}" "${pull_request}") + "${repository}" "${pull_request}" \ + "${pr_comment_id}") echo "metadata:" cat ${metadata_file} diff --git a/tasks/deploy.py b/tasks/deploy.py index 42ce66e7..34a287e2 100644 --- a/tasks/deploy.py +++ b/tasks/deploy.py @@ -301,6 +301,7 @@ def upload_tarball(job_dir, build_target, timestamp, repo_name, pr_number, pr_co cmd_args.extend(['--endpoint-url', endpoint_url]) cmd_args.extend(['--repository', repo_name]) cmd_args.extend(['--pull-request', str(pr_number)]) + cmd_args.extend(['--pr-comment-id', str(pr_comment_id)]) cmd_args.append(abs_path) upload_cmd = ' '.join(cmd_args) From 5759b0da200db267567a4cf67edb7fb0c7ae5ac8 Mon Sep 17 00:00:00 2001 From: Thomas Roeblitz Date: Fri, 16 Feb 2024 09:44:47 +0100 Subject: [PATCH 31/50] let upload script support configurable target directories - adds command line args to specify upload target directories/directory pattern - adds a command line arg to show variables that could be used in a directory pattern - makes the legacy path accessible (i.e., can be used in the directory pattern) - slighly modifies some variable names to make their purpose more clear - fixes small typo - makes script a bit more 'noisy' to let it tell what it does (should facilitate debugging; as some actions are more configurable it's desirable to have a bit more information about what is going on) --- scripts/eessi-upload-to-staging | 109 +++++++++++++++++++++++++------- 1 file changed, 86 insertions(+), 23 deletions(-) diff --git a/scripts/eessi-upload-to-staging b/scripts/eessi-upload-to-staging index 3638eeb2..45e52fbf 100755 --- a/scripts/eessi-upload-to-staging +++ b/scripts/eessi-upload-to-staging @@ -41,7 +41,7 @@ function create_metadata_file _tarball=$1 _url=$2 _repository=$3 - _pull_request=$4 + _pull_request_number=$4 _pull_request_comment_id=$5 _tmpfile=$(mktemp) @@ -56,12 +56,12 @@ function create_metadata_file --arg sha256 "$(sha256sum "${_tarball}" | awk '{print $1}')" \ --arg url "${_url}" \ --arg repo "${_repository}" \ - --arg pr "${_pull_request}" \ + --arg pr "${_pull_request_number}" \ --arg pr_comment_id "${_pull_request_comment_id}" \ '{ uploader: {username: $un, ip: $ip, hostname: $hn}, payload: {filename: $fn, size: $sz, ctime: $ct, sha256sum: $sha256, url: $url}, - link2pr: {repo: $repo, pr: $pr, pr_comment_id: $pr_commend_id}, + link2pr: {repo: $repo, pr: $pr, pr_comment_id: $pr_comment_id}, }' > "${_tmpfile}" echo "${_tmpfile}" @@ -69,18 +69,30 @@ function create_metadata_file function display_help { - echo "Usage: $0 [OPTIONS] " >&2 - echo " -e | --endpoint-url URL - endpoint url (needed for non AWS S3)" >&2 - echo " -h | --help - display this usage information" >&2 - echo " -i | --pr-comment-id - identifier of a PR comment; may be" >&2 - echo " used to efficiently determine the PR" >&2 - echo " comment to be updated during the" >&2 - echo " ingestion procedure" >&2 - echo " -n | --bucket-name BUCKET - bucket name (same as BUCKET above)" >&2 - echo " -p | --pull-request NUMBER - a pull request NUMBER; used to" >&2 - echo " link the upload to a PR" >&2 - echo " -r | --repository FULL_NAME - a repository name ACCOUNT/REPONAME;" >&2 - echo " used to link the upload to a PR" >&2 + echo "Usage: $0 [OPTIONS] " >&2 + echo " -e | --endpoint-url URL - endpoint url (needed for non AWS S3)" >&2 + echo " -h | --help - display this usage information" >&2 + echo " -i | --pr-comment-id - identifier of a PR comment; may be" >&2 + echo " used to efficiently determine the PR" >&2 + echo " comment to be updated during the" >&2 + echo " ingestion procedure" >&2 + echo " -l | --list-variables - list variables that are available" >&2 + echo " for expansion" >&2 + echo " -m | --metadata-prefix PREFIX - a directory to which the metadata" >&2 + echo " file shall be uploaded; BASH variable" >&2 + echo " expansion will be applied; arg '-l'" >&2 + echo " lists variables that are defined at" >&2 + echo " the time of expansion" >&2 + echo " -n | --bucket-name BUCKET - bucket name (same as BUCKET above)" >&2 + echo " -p | --pull-request-number INT - a pull request number (INT); used to" >&2 + echo " link the upload to a PR" >&2 + echo " -r | --repository FULL_NAME - a repository name ACCOUNT/REPONAME;" >&2 + echo " used to link the upload to a PR" >&2 + echo " -t | --tarball-prefix PREFIX - a directory to which the tarball" >&2 + echo " shall be uploaded; BASH variable" >&2 + echo " expansion will be applied; arg '-l'" >&2 + echo " lists variables that are defined at" >&2 + echo " the time of expansion" >&2 } if [[ $# -lt 1 ]]; then @@ -106,8 +118,16 @@ endpoint_url= # provided via command line arguments pr_comment_id="none" -pull_request="none" -repository="EESSI/software-layer" +pull_request_number="none" +github_repository="EESSI/software-layer" + +# provided via options in the bot's config file app.cfg and/or command line argument +metadata_prefix= +tarball_prefix= + +# other variables +legacy_aws_path= +variables="github_repository legacy_aws_path pull_request_number" while [[ $# -gt 0 ]]; do case $1 in @@ -119,20 +139,36 @@ while [[ $# -gt 0 ]]; do display_help exit 0 ;; + -l|--list-variables) + echo "variables that will be expanded: name (default value)" + for var in ${variables} + do + echo " ${var} (${!var:-unset})" + done + exit 0 + ;; -i|--pr-comment-id) pr_comment_id="$2" shift 2 ;; + -m|--metadata-prefix) + metadata_prefix="$2" + shift 2 + ;; -n|--bucket-name) bucket_name="$2" shift 2 ;; - -p|--pull-request) - pull_request="$2" + -p|--pull-request-number) + pull_request_number="$2" shift 2 ;; -r|--repository) - repository="$2" + github_repository="$2" + shift 2 + ;; + -t|--tarball-prefix) + tarball_prefix="$2" shift 2 ;; -*|--*) @@ -168,23 +204,50 @@ for file in "$*"; do basefile=$( basename ${file} ) if check_file_name ${basefile}; then if tar tf "${file}" | head -n1 > /dev/null; then - aws_path=$(basename ${file} | tr -s '-' '/' \ + # 'legacy_aws_path' might be used in tarball_prefix or metadata_prefix + # its purpose is to support the old/legacy method to derive the location + # where to store the tarball and metadata file + export legacy_aws_path=$(basename ${file} | tr -s '-' '/' \ | perl -pe 's/^eessi.//;' | perl -pe 's/\.tar\.gz$//;' ) + if [ -z ${tarball_prefix} ]; then + aws_path=${legacy_aws_path} + else + export pull_request_number + export github_repository + aws_path=$(envsubst <<< "${tarball_prefix}") + fi aws_file=$(basename ${file}) echo "Creating metadata file" url="${bucket_base}/${aws_path}/${aws_file}" - metadata_file=$(create_metadata_file "${file}" "${url}" \ - "${repository}" "${pull_request}" \ + echo "create_metadata_file file=${file} \ + url=${url} \ + github_repository=${github_repository} \ + pull_request_number=${pull_request_number} \ + pr_comment_id=${pr_comment_id}" + metadata_file=$(create_metadata_file "${file}" \ + "${url}" \ + "${github_repository}" \ + "${pull_request_number}" \ "${pr_comment_id}") echo "metadata:" cat ${metadata_file} echo Uploading to "${url}" + echo " store tarball at ${aws_path}/${aws_file}" upload_to_staging_bucket \ "${file}" \ "${bucket_name}" \ "${aws_path}/${aws_file}" \ "${endpoint_url}" + + if [ -z ${metadata_prefix} ]; then + aws_path=${legacy_aws_path} + else + export pull_request_number + export github_repository + aws_path=$(envsubst <<< "${metadata_prefix}") + fi + echo " store metadata file at ${aws_path}/${aws_file}.meta.txt" upload_to_staging_bucket \ "${metadata_file}" \ "${bucket_name}" \ From ccf5230064c6a9abc52b69e32090d049f96cf345 Mon Sep 17 00:00:00 2001 From: Thomas Roeblitz Date: Fri, 16 Feb 2024 10:20:44 +0100 Subject: [PATCH 32/50] adds settings and description for them to example cfg file --- app.cfg.example | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/app.cfg.example b/app.cfg.example index 1888e499..f01c2be9 100644 --- a/app.cfg.example +++ b/app.cfg.example @@ -147,6 +147,28 @@ deploy_permission = # template for comment when user who set a label has no permission to trigger deploying tarballs no_deploy_permission_comment = Label `bot:deploy` has been set by user `{deploy_labeler}`, but this person does not have permission to trigger deployments +# settings for where (directory) in the S3 bucket to store the metadata file and +# the tarball +# - Can be a string value to always use the same 'prefix' regardless of the target +# CVMFS repository, or can be a mapping of a target repository id (see also +# repo_target_map) to a prefix. +# - The prefix itself can use some (environment) variables that are set within +# the script. Currently those are: +# * 'github_repository' (which would be expanded to the full name of the GitHub +# repository, e.g., 'EESSI/software-layer'), +# * 'legacy_aws_path' (which expands to the legacy/old prefix being used for +# storing tarballs/metadata files) and +# * 'pull_request_number' (which would be expanded to the number of the pull +# request from which the tarball originates). +# - The list of supported variables can be shown by running +# `scripts/eessi-upload-to-staging --list-variables`. +# - Examples: +# metadata_prefix = {"eessi.io-2023.06": "new/${github_repository}/${pull_request_number}"} +# tarball_prefix = {"eessi-pilot-2023.06": "", "eessi.io-2023.06": "new/${github_repository}/${pull_request_number}"} +# If left empty, the old/legacy prefix is being used. +metadata_prefix = +tarball_prefix = + [architecturetargets] # defines both for which architectures the bot will build From 1fd7b6009ee1c3754ca7dd89b93c08b3bea7673a Mon Sep 17 00:00:00 2001 From: Thomas Roeblitz Date: Fri, 16 Feb 2024 11:47:42 +0100 Subject: [PATCH 33/50] pass *_prefix settings to upload script - read settings from config - parse settings - pass settings to upload script - adjust messages used for reporting via GitHub PR comments --- tasks/deploy.py | 52 ++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 51 insertions(+), 1 deletion(-) diff --git a/tasks/deploy.py b/tasks/deploy.py index 34a287e2..8fc8d708 100644 --- a/tasks/deploy.py +++ b/tasks/deploy.py @@ -34,7 +34,9 @@ DEPLOY_PERMISSION = "deploy_permission" ENDPOINT_URL = "endpoint_url" JOBS_BASE_DIR = "jobs_base_dir" +METADATA_PREFIX = "metadata_prefix" NO_DEPLOY_PERMISSION_COMMENT = "no_deploy_permission_comment" +TARBALL_PREFIX = "tarball_prefix" TARBALL_UPLOAD_SCRIPT = "tarball_upload_script" UPLOAD_POLICY = "upload_policy" @@ -262,11 +264,21 @@ def upload_tarball(job_dir, build_target, timestamp, repo_name, pr_number, pr_co tarball_upload_script = deploycfg.get(TARBALL_UPLOAD_SCRIPT) endpoint_url = deploycfg.get(ENDPOINT_URL) or '' bucket_spec = deploycfg.get(BUCKET_NAME) + metadata_prefix = deploycfg.get(METADATA_PREFIX) + tarball_prefix = deploycfg.get(TARBALL_PREFIX) # if bucket_spec value looks like a dict, try parsing it as such if bucket_spec.lstrip().startswith('{'): bucket_spec = json.loads(bucket_spec) + # if metadata_prefix value looks like a dict, try parsing it as such + if metadata_prefix.lstrip().startswith('{'): + metadata_prefix = json.loads(metadata_prefix) + + # if tarball_prefix value looks like a dict, try parsing it as such + if tarball_prefix.lstrip().startswith('{'): + tarball_prefix = json.loads(tarball_prefix) + jobcfg_path = os.path.join(job_dir, CFG_DIRNAME, JOB_CFG_FILENAME) jobcfg = config.read_config(jobcfg_path) target_repo_id = jobcfg[JOB_REPOSITORY][JOB_REPO_ID] @@ -288,6 +300,40 @@ def upload_tarball(job_dir, build_target, timestamp, repo_name, pr_number, pr_co f"failed (incorrect bucket spec: {bucket_spec})") return + if isinstance(metadata_prefix, str): + metadata_prefix_arg = metadata_prefix + log(f"Using specified metadata prefix: {metadata_prefix_arg}") + elif isinstance(metadata_prefix, dict): + # metadata prefix spec may be a mapping of target repo id to metadata prefix + metadata_prefix_arg = metadata_prefix.get(target_repo_id) + if metadata_prefix_arg is None: + update_pr_comment(tarball, repo_name, pr_number, pr_comment_id, "not uploaded", + f"failed (no metadata prefix specified for {target_repo_id})") + return + else: + log(f"Using metadata prefix for {target_repo_id}: {metadata_prefix_arg}") + else: + update_pr_comment(tarball, repo_name, pr_number, pr_comment_id, "not uploaded", + f"failed (incorrect metadata prefix spec: {metadata_prefix_arg})") + return + + if isinstance(tarball_prefix, str): + tarball_prefix_arg = tarball_prefix + log(f"Using specified tarball prefix: {tarball_prefix_arg}") + elif isinstance(tarball_prefix, dict): + # tarball prefix spec may be a mapping of target repo id to tarball prefix + tarball_prefix_arg = tarball_prefix.get(target_repo_id) + if tarball_prefix_arg is None: + update_pr_comment(tarball, repo_name, pr_number, pr_comment_id, "not uploaded", + f"failed (no tarball prefix specified for {target_repo_id})") + return + else: + log(f"Using tarball prefix for {target_repo_id}: {tarball_prefix_arg}") + else: + update_pr_comment(tarball, repo_name, pr_number, pr_comment_id, "not uploaded", + f"failed (incorrect tarball prefix spec: {tarball_prefix_arg})") + return + # run 'eessi-upload-to-staging {abs_path}' # (1) construct command line # script assumes a few defaults: @@ -299,9 +345,13 @@ def upload_tarball(job_dir, build_target, timestamp, repo_name, pr_number, pr_co cmd_args.extend(['--bucket-name', bucket_name]) if len(endpoint_url) > 0: cmd_args.extend(['--endpoint-url', endpoint_url]) + if len(metadata_prefix_arg) > 0: + cmd_args.extend(['--metadata-prefix', metadata_prefix_arg]) cmd_args.extend(['--repository', repo_name]) - cmd_args.extend(['--pull-request', str(pr_number)]) + cmd_args.extend(['--pull-request-number', str(pr_number)]) cmd_args.extend(['--pr-comment-id', str(pr_comment_id)]) + if len(tarball_prefix_arg) > 0: + cmd_args.extend(['--tarball-prefix', tarball_prefix_arg]) cmd_args.append(abs_path) upload_cmd = ' '.join(cmd_args) From 3a2df5af805025785131df9fc76967bf3f6b6e2f Mon Sep 17 00:00:00 2001 From: Thomas Roeblitz Date: Tue, 20 Feb 2024 07:01:47 +0100 Subject: [PATCH 34/50] return status of PR download in case of no ERROR --- tasks/build.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tasks/build.py b/tasks/build.py index d660e016..ad90a314 100644 --- a/tasks/build.py +++ b/tasks/build.py @@ -49,6 +49,7 @@ ERROR_GIT_APPLY = "git apply" ERROR_GIT_CHECKOUT = "git checkout" ERROR_GIT_CLONE = "curl" +ERROR_NONE = "none" GITHUB = "github" GIT_CLONE_FAILURE = "git_clone_failure" GIT_CLONE_TIP = "git_clone_tip" @@ -399,6 +400,8 @@ def download_pr(repo_name, branch_name, pr, arch_job_dir): error_stage = ERROR_GIT_APPLY return git_apply_output, git_apply_error, git_apply_exit_code, error_stage + # need to return four items also in case everything went fine + return 'downloading PR succeeded', 'no error while downloading PR', 0, ERROR_NONE def comment_download_pr(base_repo_name, pr, download_pr_exit_code, download_pr_error, error_stage): """ From bbfb21bec6109f4c7b8b65443f33284bc7c5c6dc Mon Sep 17 00:00:00 2001 From: Thomas Roeblitz Date: Tue, 20 Feb 2024 07:06:37 +0100 Subject: [PATCH 35/50] add a bit log info + fixing obtaining target_arch --- tasks/build.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/tasks/build.py b/tasks/build.py index ad90a314..8c1a8415 100644 --- a/tasks/build.py +++ b/tasks/build.py @@ -865,6 +865,8 @@ def request_bot_build_issue_comments(repo_name, pr_number): status_table (dict): dictionary with 'arch', 'date', 'status', 'url' and 'result' for all the finished builds; """ + fn = sys._getframe().f_code.co_name + status_table = {'arch': [], 'date': [], 'status': [], 'url': [], 'result': []} cfg = config.read_config() @@ -885,9 +887,12 @@ def request_bot_build_issue_comments(repo_name, pr_number): first_line = comment['body'].split('\n')[0] arch_map = get_architecture_targets(cfg) for arch in arch_map.keys(): - target_arch = '/'.join(arch.split('/')[-1]) + # drop the first element in arch (which names the OS type) and join the remaining items with '-' + target_arch = '-'.join(arch.split('/')[1:]) if target_arch in first_line: status_table['arch'].append(target_arch) + else: + log(f"{fn}(): target_arch '{target_arch}' not found in first line '{first_line}'") # get date, status, url and result from the markdown table comment_table = comment['body'][comment['body'].find('|'):comment['body'].rfind('|')+1] From 75c00bbaeb07ebd79adc2cc8657e3ca6be8d2268 Mon Sep 17 00:00:00 2001 From: vsc46128 vscuser Date: Wed, 21 Feb 2024 12:56:03 +0100 Subject: [PATCH 36/50] fix for job manager crash: Unable to contact slurm controller --- eessi_bot_job_manager.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/eessi_bot_job_manager.py b/eessi_bot_job_manager.py index 7d40b545..58f68fbe 100644 --- a/eessi_bot_job_manager.py +++ b/eessi_bot_job_manager.py @@ -113,8 +113,14 @@ def get_current_jobs(self): squeue_cmd, "get_current_jobs(): squeue command", log_file=self.logfile, + raise_on_error=False, ) + if squeue_exitcode != 0: + current_jobs = {} + log("The squeue command failed will try again in {} seconds".format(config.read_config()["job_manager"].get("poll_interval"))) + return current_jobs + # create dictionary of jobs from output of 'squeue_cmd' # with the following information per job: jobid, state, # nodelist_reason From d458da17fd20a1aaed17132e929c96cb847b2ca5 Mon Sep 17 00:00:00 2001 From: vsc46128 vscuser Date: Wed, 21 Feb 2024 13:59:46 +0100 Subject: [PATCH 37/50] fix long line --- eessi_bot_job_manager.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/eessi_bot_job_manager.py b/eessi_bot_job_manager.py index 58f68fbe..ebb00fd6 100644 --- a/eessi_bot_job_manager.py +++ b/eessi_bot_job_manager.py @@ -118,7 +118,8 @@ def get_current_jobs(self): if squeue_exitcode != 0: current_jobs = {} - log("The squeue command failed will try again in {} seconds".format(config.read_config()["job_manager"].get("poll_interval"))) + poll_interval = config.read_config()["job_manager"].get("poll_interval") + log("The squeue command failed will try again in {} seconds".format(poll_interval)) return current_jobs # create dictionary of jobs from output of 'squeue_cmd' From 307c62c85c2eada4a1be7c93ea4130294874685d Mon Sep 17 00:00:00 2001 From: vsc46128 vscuser Date: Thu, 22 Feb 2024 13:09:12 +0100 Subject: [PATCH 38/50] move poll_interval and skip loop if job_manager.get_current_jobs() fails --- eessi_bot_job_manager.py | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/eessi_bot_job_manager.py b/eessi_bot_job_manager.py index ebb00fd6..288b8014 100644 --- a/eessi_bot_job_manager.py +++ b/eessi_bot_job_manager.py @@ -674,6 +674,13 @@ def main(): if max_iter != 0: known_jobs = job_manager.get_known_jobs() while max_iter < 0 or i < max_iter: + # sleep poll_interval seconds (not for the first iteration) + if i != 0 and (max_iter < 0 or i < max_iter): + log( + "job manager main loop: sleep %d seconds" % poll_interval, + job_manager.logfile, + ) + time.sleep(poll_interval) log("job manager main loop: iteration %d" % i, job_manager.logfile) log( "job manager main loop: known_jobs='%s'" % ",".join( @@ -681,7 +688,12 @@ def main(): job_manager.logfile, ) - current_jobs = job_manager.get_current_jobs() + try: + current_jobs = job_manager.get_current_jobs() + except: + i = i + 1 + continue + log( "job manager main loop: current_jobs='%s'" % ",".join( current_jobs.keys()), @@ -736,13 +748,7 @@ def main(): known_jobs = current_jobs - # sleep poll_interval seconds (only if at least one more iteration) - if max_iter < 0 or i + 1 < max_iter: - log( - "job manager main loop: sleep %d seconds" % poll_interval, - job_manager.logfile, - ) - time.sleep(poll_interval) + # add one iteration to the loop i = i + 1 From 43ab1a6493e7583bbdaa8b99fe2afd105f276302 Mon Sep 17 00:00:00 2001 From: vsc46128 vscuser Date: Thu, 22 Feb 2024 14:07:16 +0100 Subject: [PATCH 39/50] remove other fix --- eessi_bot_job_manager.py | 7 ------- 1 file changed, 7 deletions(-) diff --git a/eessi_bot_job_manager.py b/eessi_bot_job_manager.py index 288b8014..558060b4 100644 --- a/eessi_bot_job_manager.py +++ b/eessi_bot_job_manager.py @@ -113,15 +113,8 @@ def get_current_jobs(self): squeue_cmd, "get_current_jobs(): squeue command", log_file=self.logfile, - raise_on_error=False, ) - if squeue_exitcode != 0: - current_jobs = {} - poll_interval = config.read_config()["job_manager"].get("poll_interval") - log("The squeue command failed will try again in {} seconds".format(poll_interval)) - return current_jobs - # create dictionary of jobs from output of 'squeue_cmd' # with the following information per job: jobid, state, # nodelist_reason From f7b38df9792e327686229347de9051b56c0065ca Mon Sep 17 00:00:00 2001 From: vsc46128 vscuser Date: Thu, 22 Feb 2024 14:11:32 +0100 Subject: [PATCH 40/50] fix except --- eessi_bot_job_manager.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/eessi_bot_job_manager.py b/eessi_bot_job_manager.py index 558060b4..224fc201 100644 --- a/eessi_bot_job_manager.py +++ b/eessi_bot_job_manager.py @@ -683,7 +683,7 @@ def main(): try: current_jobs = job_manager.get_current_jobs() - except: + except Exception as err: i = i + 1 continue From e73bfe62ea39bb67cbd4d140896aab85ce024d18 Mon Sep 17 00:00:00 2001 From: vsc46128 vscuser Date: Thu, 22 Feb 2024 14:15:51 +0100 Subject: [PATCH 41/50] implement suggested changes --- eessi_bot_job_manager.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/eessi_bot_job_manager.py b/eessi_bot_job_manager.py index 224fc201..5c475898 100644 --- a/eessi_bot_job_manager.py +++ b/eessi_bot_job_manager.py @@ -668,7 +668,7 @@ def main(): known_jobs = job_manager.get_known_jobs() while max_iter < 0 or i < max_iter: # sleep poll_interval seconds (not for the first iteration) - if i != 0 and (max_iter < 0 or i < max_iter): + if i != 0: log( "job manager main loop: sleep %d seconds" % poll_interval, job_manager.logfile, @@ -683,7 +683,7 @@ def main(): try: current_jobs = job_manager.get_current_jobs() - except Exception as err: + except RuntimeError: i = i + 1 continue From 7ef1e4bd362ff8c157e89eb3f28cf0f26d7e2abd Mon Sep 17 00:00:00 2001 From: Thomas Roeblitz Date: Fri, 23 Feb 2024 20:40:35 +0100 Subject: [PATCH 42/50] add new/missing bot settings to README.md - added new settings `metadata_prefix` and `tarball_prefix` - added new settings for reporting about pull request download failures - added missing settings for build permissions and reporting about no permissions --- README.md | 98 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 98 insertions(+) diff --git a/README.md b/README.md index d855ef43..1cbc7402 100644 --- a/README.md +++ b/README.md @@ -404,6 +404,20 @@ submit_command = /usr/bin/sbatch ``` `submit_command` is the full path to the Slurm job submission command used for submitting batch jobs. You may want to verify if `sbatch` is provided at that path or determine its actual location (using `which sbatch`). +``` +build_permission = GH_ACCOUNT_1 GH_ACCOUNT_2 ... +``` +`build_permission` defines which GitHub accounts have the permission to trigger +the build, i.e., the bot acts on `bot: build ...` commands. If the value is left +empty everyone can trigger build jobs. + +``` +no_build_permission_comment = Label `bot:build` has been set by user `{build_labeler}`, but this person does not have permission to trigger builds +``` +`no_build_permission_comment` defines a comment (template) that is used when +the account trying to trigger build jobs has no permission to do so. + + #### `[bot_control]` section The `[bot_control]` section contains settings for configuring the feature to @@ -485,6 +499,43 @@ This defines a message that is added to the status table in a PR comment corresponding to a job whose tarball should have been uploaded (e.g., after setting the `bot:deploy` label). + +``` +metadata_prefix = LOCATION_WHERE_METADATA_FILE_GETS_DEPOSITED +tarball_prefix = LOCATION_WHERE_TARBALL_GETS_DEPOSITED +``` + +These two settings are used to define where (which directory) in the S3 bucket +(see `bucket_name` above) the metadata file and the tarball will be stored. The +value `LOCATION...` can be a string value to always use the same 'prefix' +regardless of the target CVMFS repository, or can be a mapping of a target +repository id (see also `repo_target_map` below) to a prefix. + +The prefix itself can use some (environment) variables that are set within +the upload script (see `tarball_upload_script` above). Currently those are: + * `'${github_repository}'` (which would be expanded to the full name of the GitHub + repository, e.g., `EESSI/software-layer`), + * `'${legacy_aws_path}'` (which expands to the legacy/old prefix being used for + storing tarballs/metadata files, the old prefix is + `EESSI_VERSION/TARBALL_TYPE/OS_TYPE/CPU_ARCHITECTURE/TIMESTAMP/`), _and_ + * `'${pull_request_number}'` (which would be expanded to the number of the pull + request from which the tarball originates). +Note, it's important to single-quote (`'`) the variables as shown above, because +they may likely not be defined when the bot calls the upload script. + +The list of supported variables can be shown by running +`scripts/eessi-upload-to-staging --list-variables`. + +**Examples:** +``` +metadata_prefix = {"eessi.io-2023.06": "new/${github_repository}/${pull_request_number}"} +tarball_prefix = { + "eessi-pilot-2023.06": "", + "eessi.io-2023.06": "new/${github_repository}/${pull_request_number}" + } +``` +If left empty, the old/legacy prefix is being used. + #### `[architecturetargets]` section The section `[architecturetargets]` defines for which targets (OS/SUBDIR), (for example `linux/x86_64/amd/zen2`) the EESSI bot should submit jobs, and which additional `sbatch` parameters will be used for requesting a compute node with the CPU microarchitecture needed to build the software stack. @@ -657,6 +708,53 @@ job_test_unknown_fmt =
:shrug: UNKNOWN _(click triangle for de `job_test_unknown_fmt` is used in case no test file (produced by `bot/check-test.sh` provided by target repository) was found. + +#### `[download_pr_comments]` section + +The `[download_pr_comments]` section sets templates for messages related to +downloading the contents of a pull request. +``` +git_clone_failure = Unable to clone the target repository. +``` +`git_clone_failure` is shown when `git clone` failed. + +``` +git_clone_tip = _Tip: This could be a connection failure. Try again and if the issue remains check if the address is correct_. +``` +`git_clone_tip` should contain some hint on how to deal with the issue. It is shown when `git clone` failed. + +``` +git_checkout_failure = Unable to checkout to the correct branch. +``` +`git_checkout_failure` is shown when `git checkout` failed. + +``` +git_checkout_tip = _Tip: Ensure that the branch name is correct and the target branch is available._ +``` +`git_checkout_tip` should contain some hint on how to deal with the failure. It +is shown when `git checkout` failed. + +``` +curl_failure = Unable to download the `.diff` file. +``` +`curl_failure` is shown when downloading the `PR_NUMBER.diff` +``` +curl_tip = _Tip: This could be a connection failure. Try again and if the issue remains check if the address is correct_ +``` +`curl_tip` should help in how to deal with failing downloads of the `.diff` file. + +``` +git_apply_failure = Unable to download or merge changes between the source branch and the destination branch. +``` +`git_apply_failure` is shown when applying the `.diff` file with `git apply` +failed. + +``` +git_apply_tip = _Tip: This can usually be resolved by syncing your branch and resolving any merge conflicts._ +``` +`git_apply_tip` should guide the contributor/maintainer about resolving the cause +of `git apply` failing. + # Instructions to run the bot components The bot consists of three components: From 2eb7f0ac3ff72ff127494e2af98682ceba6b4b18 Mon Sep 17 00:00:00 2001 From: Thomas Roeblitz Date: Fri, 23 Feb 2024 21:53:04 +0100 Subject: [PATCH 43/50] fix flake8 test issue --- tasks/build.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tasks/build.py b/tasks/build.py index 8c1a8415..36cc08d1 100644 --- a/tasks/build.py +++ b/tasks/build.py @@ -403,6 +403,7 @@ def download_pr(repo_name, branch_name, pr, arch_job_dir): # need to return four items also in case everything went fine return 'downloading PR succeeded', 'no error while downloading PR', 0, ERROR_NONE + def comment_download_pr(base_repo_name, pr, download_pr_exit_code, download_pr_error, error_stage): """ Handle download_pr() exit code and write helpful comment to PR in case of failure From 8b09beca061c325b15077204c1e931ae6d5aabcf Mon Sep 17 00:00:00 2001 From: Thomas Roeblitz Date: Mon, 26 Feb 2024 16:44:02 +0100 Subject: [PATCH 44/50] updating README.md to implement suggestions --- README.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index 1cbc7402..ca96d2eb 100644 --- a/README.md +++ b/README.md @@ -408,11 +408,11 @@ submit_command = /usr/bin/sbatch build_permission = GH_ACCOUNT_1 GH_ACCOUNT_2 ... ``` `build_permission` defines which GitHub accounts have the permission to trigger -the build, i.e., the bot acts on `bot: build ...` commands. If the value is left -empty everyone can trigger build jobs. +build jobs, i.e., for which accounts the bot acts on `bot: build ...` commands. +If the value is left empty, everyone can trigger build jobs. ``` -no_build_permission_comment = Label `bot:build` has been set by user `{build_labeler}`, but this person does not have permission to trigger builds +no_build_permission_comment = The `bot: build ...` command has been used by user `{build_labeler}`, but this person does not have permission to trigger builds. ``` `no_build_permission_comment` defines a comment (template) that is used when the account trying to trigger build jobs has no permission to do so. From 2cf73ac79865df1c4e23132db8d0436d5ec673f8 Mon Sep 17 00:00:00 2001 From: Thomas Roeblitz Date: Mon, 26 Feb 2024 21:19:15 +0100 Subject: [PATCH 45/50] prevent bot from responding on comments without a bot command --- eessi_bot_event_handler.py | 34 +++++++++++++++++++++------------- tools/commands.py | 13 +++++++++++++ 2 files changed, 34 insertions(+), 13 deletions(-) diff --git a/eessi_bot_event_handler.py b/eessi_bot_event_handler.py index 024615ce..60df7a3a 100644 --- a/eessi_bot_event_handler.py +++ b/eessi_bot_event_handler.py @@ -31,7 +31,8 @@ from tasks.deploy import deploy_built_artefacts from tools import config from tools.args import event_handler_parse -from tools.commands import EESSIBotCommand, EESSIBotCommandError, get_bot_command +from tools.commands import EESSIBotCommand, EESSIBotCommandError, + contains_any_bot_command, get_bot_command from tools.permissions import check_command_permission from tools.pr_comments import create_comment @@ -113,7 +114,25 @@ def handle_issue_comment_event(self, event_info, log_file=None): # currently, only commands in new comments are supported # - commands have the syntax 'bot: COMMAND [ARGS*]' - # first check if sender is authorized to send any command + # only scan for commands in newly created comments + if action == 'created': + comment_received = request_body['comment']['body'] + self.log(f"comment action '{action}' is handled") + else: + # NOTE we do not respond to an updated PR comment with yet another + # new PR comment, because it would make the bot very noisy or + # worse could result in letting the bot enter an endless loop + self.log(f"comment action '{action}' not handled") + return + # at this point we know that we are handling a new comment + + # check if comment does not contain a bot command + if not contains_any_bot_command(comment_received): + self.log(f"comment does not contain a bot comment; not processing it further") + return + # at this point we know that the comment contains a bot command + + # check if sender is authorized to send any command # - this serves a double purpose: # 1. check permission # 2. skip any comment updates that were done by the bot itself @@ -150,17 +169,6 @@ def handle_issue_comment_event(self, event_info, log_file=None): else: self.log(f"account `{sender}` has permission to send commands to bot") - # only scan for commands in newly created comments - if action == 'created': - comment_received = request_body['comment']['body'] - self.log(f"comment action '{action}' is handled") - else: - # NOTE we do not respond to an updated PR comment with yet another - # new PR comment, because it would make the bot very noisy or - # worse could result in letting the bot enter an endless loop - self.log(f"comment action '{action}' not handled") - return - # search for commands in comment comment_response = '' commands = [] diff --git a/tools/commands.py b/tools/commands.py index cdc2c8f7..5db8f7f7 100644 --- a/tools/commands.py +++ b/tools/commands.py @@ -20,6 +20,19 @@ from tools.filter import EESSIBotActionFilter, EESSIBotActionFilterError +def contains_any_bot_command(body): + """ + Checks if argument contains any bot command. + + Args: + body (string): possibly multi-line string that may contain a bot command + + Returns: + (bool): True if bot command found, False otherwise + """ + return any(map(get_bot_command, body.split('\n'))) + + def get_bot_command(line): """ Retrieve bot command from a line. From 6be82dd5fe4edcada083c3bdec0a3b1e00c10e2d Mon Sep 17 00:00:00 2001 From: Thomas Roeblitz Date: Mon, 26 Feb 2024 21:30:45 +0100 Subject: [PATCH 46/50] fix import syntax error --- eessi_bot_event_handler.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/eessi_bot_event_handler.py b/eessi_bot_event_handler.py index 60df7a3a..c0e6cd32 100644 --- a/eessi_bot_event_handler.py +++ b/eessi_bot_event_handler.py @@ -31,7 +31,7 @@ from tasks.deploy import deploy_built_artefacts from tools import config from tools.args import event_handler_parse -from tools.commands import EESSIBotCommand, EESSIBotCommandError, +from tools.commands import EESSIBotCommand, EESSIBotCommandError, \ contains_any_bot_command, get_bot_command from tools.permissions import check_command_permission from tools.pr_comments import create_comment From e7e69b6d887ec99ac862dff85f15aa025ea15561 Mon Sep 17 00:00:00 2001 From: Thomas Roeblitz Date: Tue, 27 Feb 2024 17:35:00 +0100 Subject: [PATCH 47/50] use plain string instead of f-string --- eessi_bot_event_handler.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/eessi_bot_event_handler.py b/eessi_bot_event_handler.py index c0e6cd32..be48306f 100644 --- a/eessi_bot_event_handler.py +++ b/eessi_bot_event_handler.py @@ -128,7 +128,7 @@ def handle_issue_comment_event(self, event_info, log_file=None): # check if comment does not contain a bot command if not contains_any_bot_command(comment_received): - self.log(f"comment does not contain a bot comment; not processing it further") + self.log("comment does not contain a bot comment; not processing it further") return # at this point we know that the comment contains a bot command From bd3cc235927000d8c8b868516bec36778a9d2609 Mon Sep 17 00:00:00 2001 From: Thomas Roeblitz Date: Tue, 27 Feb 2024 19:20:37 +0100 Subject: [PATCH 48/50] release notes for v0.4.0 --- RELEASE_NOTES | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/RELEASE_NOTES b/RELEASE_NOTES index b8d09e8c..6182443d 100644 --- a/RELEASE_NOTES +++ b/RELEASE_NOTES @@ -1,6 +1,22 @@ This file contains a description of the major changes to the EESSI build-and-deploy bot. For more detailed information, please see the git log. +v0.4.0 (28 February 2024) +-------------------------- + +This is a minor release of the EESSI build-and-deploy bot. + +Bug fixes: +* fixes issue using wrong values when using the `bot: status` command (#251) + +Improvements: +* the bot report when downloading the pull request's contents failed (#248) +* adding the pull request comment id to the metadata file that is uploded to the + the S3 bucket (#247, #249, #250, #253) +* enabling configurable upload directories for tarball and metadata file (#254) +* the bot only responds to pull request comments that contain a bot command (#257) + + v0.3.0 (30 January 2024) -------------------------- From 2018925079996b22325d3e97be2e3ed273607b65 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20R=C3=B6blitz?= Date: Wed, 28 Feb 2024 14:23:45 +0100 Subject: [PATCH 49/50] Update RELEASE_NOTES spell correction Co-authored-by: Lara Ramona Peeters <49882639+laraPPr@users.noreply.github.com> --- RELEASE_NOTES | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/RELEASE_NOTES b/RELEASE_NOTES index 6182443d..984d0bcf 100644 --- a/RELEASE_NOTES +++ b/RELEASE_NOTES @@ -11,7 +11,7 @@ Bug fixes: Improvements: * the bot report when downloading the pull request's contents failed (#248) -* adding the pull request comment id to the metadata file that is uploded to the +* adding the pull request comment id to the metadata file that is uploaded to the the S3 bucket (#247, #249, #250, #253) * enabling configurable upload directories for tarball and metadata file (#254) * the bot only responds to pull request comments that contain a bot command (#257) From 352c510dab3ba7660bd87e2f5c4016ed305f0179 Mon Sep 17 00:00:00 2001 From: Thomas Roeblitz Date: Wed, 28 Feb 2024 16:25:59 +0100 Subject: [PATCH 50/50] improve release notes for v0.4.0 --- RELEASE_NOTES | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/RELEASE_NOTES b/RELEASE_NOTES index 984d0bcf..bfc562ae 100644 --- a/RELEASE_NOTES +++ b/RELEASE_NOTES @@ -10,11 +10,11 @@ Bug fixes: * fixes issue using wrong values when using the `bot: status` command (#251) Improvements: -* the bot report when downloading the pull request's contents failed (#248) +* make bot report when preparing the job working directory failed, for example due to merge conflict in a pull request (#248) * adding the pull request comment id to the metadata file that is uploaded to the the S3 bucket (#247, #249, #250, #253) * enabling configurable upload directories for tarball and metadata file (#254) -* the bot only responds to pull request comments that contain a bot command (#257) +* only make bot respond to pull request comments that contain a bot command (#257) v0.3.0 (30 January 2024)