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

logclean: Fix cleanup for multiple MASTERNAMEs #1157

Open
delphij opened this issue Jun 5, 2024 · 0 comments
Open

logclean: Fix cleanup for multiple MASTERNAMEs #1157

delphij opened this issue Jun 5, 2024 · 0 comments
Assignees
Labels
Milestone

Comments

@delphij
Copy link
Member

delphij commented Jun 5, 2024

Describe the bug

When multiple jails (on my system there were neptune-amd64, saturn-amd64 and sirius-amd64) are used, the first logclean run would fail at "Removing empty build log directories" step with, e.g.:

[00:00:00] Removing empty build log directories...find: neptune-amd64-default saturn-amd64-default sirius-amd64-default/latest-per-pkg: No such file or directory
Error: (77076) /usr/local/share/poudriere/logclean.sh:422: set -e error: status = 1

How to reproduce

Steps to reproduce the behavior:

  1. Use poudriere to generate logs, this can be done by using multiple jails to perform an empty bulk build.
  2. Run poudriere logclean -ay

Expected behavior

The whole process should complete without error.

Environment

  • Host OS: 14.1 amd64
  • Jail OS: various, but mainly 14.1 amd64 and 15.0 amd64
  • Browser: N/A
  • Poudriere Version: poudriere-devel-3.4.99.20240424 from ports tree
  • Ports branch and revision: main

Additional context

I would propose the following change:

diff --git a/src/share/poudriere/logclean.sh b/src/share/poudriere/logclean.sh
index f28ecc44..99bcf920 100755
--- a/src/share/poudriere/logclean.sh
+++ b/src/share/poudriere/logclean.sh
@@ -370,11 +370,11 @@ if [ ${logs_deleted} -eq 1 ]; then
 
        msg_n "Removing empty build log directories..."
        if lock_have "logs_latest-per-pkg"; then
-               echo "${MASTERNAMES_LOCKED:?}" | sed -e 's,$,/latest-per-pkg,' | \
-                   tr '\n' '\000' | \
-                   xargs -0 -J % find -x % -mindepth 0 -maxdepth 0 -empty | \
+               for MASTERNAME in ${MASTERNAMES_LOCKED:?}; do
+                   find -x "${MASTERNAME}/latest-per-pkg" -mindepth 0 -maxdepth 0 -empty | \
                    sed -e 's,$,/..,' | xargs realpath | tr '\n' '\000' | \
                    xargs -0 rm -rf
+               done
                echo " done"
        else
                echo " skipped (locked by another process)"

Basically, MASTERNAMES_LOCKED was a set of strings separated by blank space. For all other usages, for MASTERNAME in ${MASTERNAMES_LOCKED:?}; idiom was used, and for this one case the sed seems to be both redundant and incorrect (because it adds /latest-per-pkg to the end of the list, and the code seems to be expecting the list to be \n separated and it is not).

I think it makes more sense to simply avoid these sed's and manipulate the string in shell.

@delphij delphij added the bug label Jun 5, 2024
@bdrewery bdrewery self-assigned this Aug 8, 2024
@bdrewery bdrewery added this to the 3.5.0 milestone Aug 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants