Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

script to clean up tarballs of jobs given a PR number #217

Merged
merged 3 commits into from
Oct 3, 2023

Conversation

trz42
Copy link
Contributor

@trz42 trz42 commented Sep 30, 2023

To delete all major tarballs (temporary storage and build artefacts such as software installations) of PR NUMBER, simply run with

./cleanup_pr.sh NUMBER

from bot directory that contains app.cfg (used to determine base directory for jobs, e.g., setting jobs_base_dir).

  • Add -D or --dry-run to check what the script would do.
  • Add -b DIR or --jobs-base-dir DIR where DIR points to the base directory of jobs.

@trz42 trz42 added the feature label Sep 30, 2023
cleanup_pr.sh Outdated
# restore potentially parsed filename(s) into $*
set -- "${POSITIONAL_ARGS[@]}"

if [[ $# -lt 1 ]]; then
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't we want exactly 1 PR number here?

If so, use $# -ne 1, and adjust the error message, like "ERROR: Exactly one PR number should be provided".

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in 1f9c0da

cleanup_pr.sh Outdated
if ${dry_run} = true ; then
echo "DRY_RUN: rm '$f' ($(ls -lh $f | awk '{print $5}'))"
else
rm $f
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe also emit a message when removing a file?

echo "Removing $f..."`

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in 1f9c0da

@boegel
Copy link
Contributor

boegel commented Oct 2, 2023

@trz42 Next step (maybe in follow-up PR) could be to run this script every time a PR is closed/merged?

@trz42
Copy link
Contributor Author

trz42 commented Oct 2, 2023

@trz42 Next step (maybe in follow-up PR) could be to run this script every time a PR is closed/merged?

Good idea.

We could implement this as follows:

  • When PR is closed/merged, submit a new job that executes cleanup_pr.sh (or scripts/bot-cleanup_pr.slurm) with some additional parameters.
  • Additional parameters could be
    • -p|--cleanup-policy POLICY with policies all -- cleans up all directories for the PR (doesn't run target repository-specific cleanup script), repo_defined -- for each job runs bot/cleanup_pr.sh provided by the target repository, tarballs -- for each job removes all tarballs (suffix .tar.gz or .tgz), (possibly) distinguish different types of tarballs for what was built or what contains temporary storage, none -- doesn't clean up anything; policy should be defined in app.cfg, default policy could be none or all
    • -j|--jobid JOBID only cleans up directory for the given jobid
  • The target repository could provide a bot/cleanup_pr.sh which cleans up a directory for a given directory (or current directory).
  • (Optionally) Having these scripts would make the implementation of a command such as bot: cleanup job:JOBID or just bot: cleanup or bot: cleanup storage:tmp relatively easy.

@boegel
Copy link
Contributor

boegel commented Oct 3, 2023

@trz42 Should we move the script into the scripts subdirectory?

@trz42
Copy link
Contributor Author

trz42 commented Oct 3, 2023

@trz42 Should we move the script into the scripts subdirectory?

Done in 8d4b937

@boegel boegel merged commit 37f2891 into EESSI:develop Oct 3, 2023
6 checks passed
@boegel boegel added this to the 0.1.1 milestone Nov 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants