From 323ad6a4a165a92f06e3ec2bfc75947c4541d9e0 Mon Sep 17 00:00:00 2001 From: Allan Burdajewicz Date: Sun, 14 Apr 2024 17:31:42 +1000 Subject: [PATCH] =?UTF-8?q?[JENKINS-65829]=20Fix=20WorkspaceCleanupThread?= =?UTF-8?q?=20to=20consider=20suffixed=20works=E2=80=A6=20(#9083)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * [JENKINS-65829] Fix WorkspaceCleanupThread to consider suffixed workspaces even if original is inexistent * [JENKINS-65829] Make sur controller system properties is used across all nodes * [JENKINS-65829] Adapt test for tmp + rename test for libs --- .../hudson/model/WorkspaceCleanupThread.java | 89 +++++++++++++++---- .../model/WorkspaceCleanupThreadTest.java | 24 +++++ 2 files changed, 94 insertions(+), 19 deletions(-) diff --git a/core/src/main/java/hudson/model/WorkspaceCleanupThread.java b/core/src/main/java/hudson/model/WorkspaceCleanupThread.java index 00ef65c4a92e..d17970bb6a45 100644 --- a/core/src/main/java/hudson/model/WorkspaceCleanupThread.java +++ b/core/src/main/java/hudson/model/WorkspaceCleanupThread.java @@ -24,6 +24,8 @@ package hudson.model; +import static hudson.Util.fileToPath; + import edu.umd.cs.findbugs.annotations.NonNull; import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; import hudson.Extension; @@ -31,12 +33,19 @@ import hudson.FilePath; import hudson.Functions; import hudson.Util; +import hudson.remoting.VirtualChannel; +import hudson.slaves.WorkspaceList; +import java.io.File; +import java.io.FileFilter; import java.io.IOException; +import java.io.Serializable; +import java.nio.file.Path; import java.util.ArrayList; import java.util.Date; import java.util.List; import java.util.logging.Level; import java.util.logging.Logger; +import jenkins.MasterToSlaveFileCallable; import jenkins.model.Jenkins; import jenkins.model.ModifiableTopLevelItemGroup; import jenkins.util.SystemProperties; @@ -90,8 +99,7 @@ public static void invoke() { if (check) { listener.getLogger().println("Deleting " + ws + " on " + node.getDisplayName()); try { - ws.deleteSuffixesRecursive(); - ws.deleteRecursive(); + ws.act(new CleanupOldWorkspaces(retainForDays)); } catch (IOException | InterruptedException x) { Functions.printStackTrace(x, listener.error("Failed to delete " + ws + " on " + node.getDisplayName())); } @@ -101,21 +109,6 @@ public static void invoke() { } private boolean shouldBeDeleted(@NonNull TopLevelItem item, FilePath dir, @NonNull Node n) throws IOException, InterruptedException { - // TODO: the use of remoting is not optimal. - // One remoting can execute "exists", "lastModified", and "delete" all at once. - // (Could even invert master loop so that one FileCallable takes care of all known items.) - if (!dir.exists()) { - LOGGER.log(Level.FINE, "Directory {0} does not exist", dir); - return false; - } - - // if younger than a month, keep it - long now = new Date().getTime(); - if (dir.lastModified() + retainForDays * DAY > now) { - LOGGER.log(Level.FINE, "Directory {0} is only {1} old, so not deleting", new Object[] {dir, Util.getTimeSpanString(now - dir.lastModified())}); - return false; - } - // TODO could also be good to add checkbox that lets users configure a workspace to never be auto-cleaned. // TODO check instead for SCMTriggerItem: @@ -143,11 +136,69 @@ private boolean shouldBeDeleted(@NonNull TopLevelItem item, FilePath dir, @NonNu return false; } } - - LOGGER.log(Level.FINER, "Going to delete directory {0}", dir); return true; } + private static class CleanupOldWorkspaces extends MasterToSlaveFileCallable { + + private final int retentionInDays; + + public CleanupOldWorkspaces(int retentionInDays) { + this.retentionInDays = retentionInDays; + } + + @Override + public Void invoke(File f, VirtualChannel channel) throws IOException, InterruptedException { + File[] workspaces = null; + File parentWs = f.getParentFile(); + if (parentWs != null) { + workspaces = parentWs.listFiles(new ShouldBeDeletedFilter(this.retentionInDays, f.getName())); + } + + if (workspaces != null) { + for (File workspace : workspaces) { + LOGGER.log(Level.FINER, "Going to delete directory {0}", workspace); + Util.deleteRecursive(fileToPath(workspace), Path::toFile); + } + } + return null; + } + } + + private static class ShouldBeDeletedFilter implements FileFilter, Serializable { + + private final int retentionInDays; + + private final String workspaceBaseName; + + public ShouldBeDeletedFilter(int retentionInDays, String workspaceBaseName) { + this.retentionInDays = retentionInDays; + this.workspaceBaseName = workspaceBaseName; + } + + @Override + public boolean accept(File dir) { + + if (!dir.isDirectory()) { + return false; + } + + // if not the workspace or a workspace suffix + if (!dir.getName().equals(workspaceBaseName) && !dir.getName().startsWith(workspaceBaseName + WorkspaceList.COMBINATOR)) { + return false; + } + + // if younger than a month, keep it + long now = new Date().getTime(); + if (dir.lastModified() + this.retentionInDays * DAY > now) { + LOGGER.log(Level.FINE, "Directory {0} is only {1} old, so not deleting", new Object[] {dir, Util.getTimeSpanString(now - dir.lastModified())}); + return false; + } + + return true; + } + } + private static final Logger LOGGER = Logger.getLogger(WorkspaceCleanupThread.class.getName()); /** diff --git a/test/src/test/java/hudson/model/WorkspaceCleanupThreadTest.java b/test/src/test/java/hudson/model/WorkspaceCleanupThreadTest.java index c62fe7742f72..57f4946e7978 100644 --- a/test/src/test/java/hudson/model/WorkspaceCleanupThreadTest.java +++ b/test/src/test/java/hudson/model/WorkspaceCleanupThreadTest.java @@ -40,6 +40,7 @@ import java.util.concurrent.TimeUnit; import java.util.logging.Level; import jenkins.MasterToSlaveFileCallable; +import jenkins.model.Jenkins; import org.junit.Assume; import org.junit.Rule; import org.junit.Test; @@ -181,12 +182,28 @@ public void deleteTemporaryDirectory() throws Exception { FilePath ws = createOldWorkspaceOn(r.jenkins, p); FilePath tmp = WorkspaceList.tempDir(ws); tmp.child("stuff").write("content", null); + tmp.act(new Touch(0)); createOldWorkspaceOn(r.createOnlineSlave(), p); performCleanup(); assertFalse(ws.exists()); assertFalse("temporary directory should be cleaned up as well", tmp.exists()); } + @Issue("JENKINS-65829") + @Test + public void deleteSoleLibsDirectory() throws Exception { + FreeStyleProject p = r.createFreeStyleProject(); + FilePath jobWs = Jenkins.get().getWorkspaceFor(p); + FilePath libsWs = jobWs.withSuffix(WorkspaceList.COMBINATOR + "libs"); + libsWs.child("test-libs").write("content", null); + libsWs.act(new Touch(0)); + assertFalse(jobWs.exists()); + assertTrue(libsWs.exists()); + performCleanup(); + assertFalse(jobWs.exists()); + assertFalse("libs directory should be cleaned up as well", libsWs.exists()); + } + private FilePath createOldWorkspaceOn(Node slave, FreeStyleProject p) throws Exception { p.setAssignedNode(slave); FreeStyleBuild b1 = r.buildAndAssertSuccess(p); @@ -197,6 +214,13 @@ private FilePath createOldWorkspaceOn(Node slave, FreeStyleProject p) throws Exc return ws; } + private FilePath createOldLibsWorkspace(FreeStyleProject p) throws IOException, InterruptedException { + FilePath libsWs = Jenkins.get().getWorkspaceFor(p).withSuffix(WorkspaceList.COMBINATOR + "libs"); + libsWs.child("test-libs").write("content", null); + libsWs.act(new Touch(0)); + return libsWs; + } + private void performCleanup() throws InterruptedException, IOException { new WorkspaceCleanupThread().execute(StreamTaskListener.fromStdout()); }