From 8ecc1f32d6d073ce138ace4173fd339e82a8f4ea Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Fri, 14 Feb 2025 15:11:34 -0500 Subject: [PATCH 01/17] Stop agents before exiting test cases --- .../com/cloudbees/jenkins/support/CheckFilterTest.java | 9 +++++++++ .../com/cloudbees/jenkins/support/Security2186Test.java | 9 +++++++++ .../com/cloudbees/jenkins/support/SupportActionTest.java | 9 +++++++++ .../support/actions/SupportComputerActionTest.java | 9 +++++++++ .../support/configfiles/AgentsConfigFileTest.java | 9 +++++++++ .../support/filter/SensitiveContentFilterTest.java | 9 +++++++++ .../cloudbees/jenkins/support/impl/AboutJenkinsTest.java | 9 +++++++++ .../jenkins/support/impl/DumpExportTableTest.java | 9 +++++++++ .../jenkins/support/impl/FileDescriptorLimitTest.java | 9 +++++++++ .../jenkins/support/impl/NodeExecutorsTest.java | 9 +++++++++ .../support/impl/NodeRemoteDirectoryComponentTest.java | 9 +++++++++ .../jenkins/support/impl/SlaveCommandStatisticsTest.java | 9 +++++++++ .../jenkins/support/impl/SlaveLaunchLogsRestartTest.java | 4 ++++ .../jenkins/support/impl/SlaveLaunchLogsTest.java | 9 +++++++++ .../jenkins/support/impl/SmartLogCleanerTest.java | 9 +++++++++ 15 files changed, 130 insertions(+) diff --git a/src/test/java/com/cloudbees/jenkins/support/CheckFilterTest.java b/src/test/java/com/cloudbees/jenkins/support/CheckFilterTest.java index 3c336b14a..bac3b0875 100644 --- a/src/test/java/com/cloudbees/jenkins/support/CheckFilterTest.java +++ b/src/test/java/com/cloudbees/jenkins/support/CheckFilterTest.java @@ -51,6 +51,7 @@ import java.util.zip.ZipEntry; import java.util.zip.ZipFile; import jenkins.model.Jenkins; +import org.junit.After; import org.junit.Assert; import org.junit.Rule; import org.junit.Test; @@ -75,6 +76,14 @@ public class CheckFilterTest { @Rule public LoggerRule logging = new LoggerRule().record(AsyncResultCache.class, Level.FINER); + @After + public void stopAgents() throws Exception { + for (var agent : j.jenkins.getNodes()) { + System.err.println("Stopping " + agent); + agent.toComputer().disconnect(null).get(); + } + } + @Test public void checkFilterTest() throws Exception { // Create the files to check diff --git a/src/test/java/com/cloudbees/jenkins/support/Security2186Test.java b/src/test/java/com/cloudbees/jenkins/support/Security2186Test.java index 9a290f955..e374f35e4 100644 --- a/src/test/java/com/cloudbees/jenkins/support/Security2186Test.java +++ b/src/test/java/com/cloudbees/jenkins/support/Security2186Test.java @@ -27,6 +27,7 @@ import java.util.List; import java.util.zip.ZipEntry; import java.util.zip.ZipFile; +import org.junit.After; import org.junit.Before; import org.junit.Rule; import org.junit.Test; @@ -51,6 +52,14 @@ public void setup() throws Exception { setupAgent(); } + @After + public void stopAgents() throws Exception { + for (var agent : j.jenkins.getNodes()) { + System.err.println("Stopping " + agent); + agent.toComputer().disconnect(null).get(); + } + } + @Test @Issue("SECURITY-2186") public void secretsFilterWhenSystemPropertyContainsPasswordThenValueRedacted() throws Exception { diff --git a/src/test/java/com/cloudbees/jenkins/support/SupportActionTest.java b/src/test/java/com/cloudbees/jenkins/support/SupportActionTest.java index 338c50946..e61e27668 100644 --- a/src/test/java/com/cloudbees/jenkins/support/SupportActionTest.java +++ b/src/test/java/com/cloudbees/jenkins/support/SupportActionTest.java @@ -52,6 +52,7 @@ import org.htmlunit.html.HtmlButton; import org.htmlunit.html.HtmlForm; import org.htmlunit.html.HtmlPage; +import org.junit.After; import org.junit.Assume; import org.junit.Before; import org.junit.Rule; @@ -84,6 +85,14 @@ public void setUp() { root = ExtensionList.lookupSingleton(SupportAction.class); } + @After + public void stopAgents() throws Exception { + for (var agent : j.jenkins.getNodes()) { + System.err.println("Stopping " + agent); + agent.toComputer().disconnect(null).get(); + } + } + @Test public void download() throws IOException, SAXException { downloadBundle("/download?json={\"components\":1}"); diff --git a/src/test/java/com/cloudbees/jenkins/support/actions/SupportComputerActionTest.java b/src/test/java/com/cloudbees/jenkins/support/actions/SupportComputerActionTest.java index 6aac3815c..6743156c5 100644 --- a/src/test/java/com/cloudbees/jenkins/support/actions/SupportComputerActionTest.java +++ b/src/test/java/com/cloudbees/jenkins/support/actions/SupportComputerActionTest.java @@ -14,6 +14,7 @@ import java.util.stream.Stream; import java.util.zip.ZipFile; import jenkins.model.Jenkins; +import org.junit.After; import org.junit.Rule; import org.junit.Test; import org.jvnet.hudson.test.JenkinsRule; @@ -26,6 +27,14 @@ public class SupportComputerActionTest { @Rule public JenkinsRule j = new JenkinsRule(); + @After + public void stopAgents() throws Exception { + for (var agent : j.jenkins.getNodes()) { + System.err.println("Stopping " + agent); + agent.toComputer().disconnect(null).get(); + } + } + @Test public void onlyAdminCanSeeAction() throws Exception { diff --git a/src/test/java/com/cloudbees/jenkins/support/configfiles/AgentsConfigFileTest.java b/src/test/java/com/cloudbees/jenkins/support/configfiles/AgentsConfigFileTest.java index e026727ad..09b136764 100644 --- a/src/test/java/com/cloudbees/jenkins/support/configfiles/AgentsConfigFileTest.java +++ b/src/test/java/com/cloudbees/jenkins/support/configfiles/AgentsConfigFileTest.java @@ -33,6 +33,7 @@ import hudson.EnvVars; import java.util.Map; import org.hamcrest.MatcherAssert; +import org.junit.After; import org.junit.Rule; import org.junit.Test; import org.jvnet.hudson.test.Issue; @@ -45,6 +46,14 @@ public class AgentsConfigFileTest { @Rule public JenkinsRule j = new JenkinsRule(); + @After + public void stopAgents() throws Exception { + for (var agent : j.jenkins.getNodes()) { + System.err.println("Stopping " + agent); + agent.toComputer().disconnect(null).get(); + } + } + @Test public void agentsConfigFile() throws Exception { j.createSlave("node1", "node1", new EnvVars()); diff --git a/src/test/java/com/cloudbees/jenkins/support/filter/SensitiveContentFilterTest.java b/src/test/java/com/cloudbees/jenkins/support/filter/SensitiveContentFilterTest.java index d52fabf78..73b29c77e 100644 --- a/src/test/java/com/cloudbees/jenkins/support/filter/SensitiveContentFilterTest.java +++ b/src/test/java/com/cloudbees/jenkins/support/filter/SensitiveContentFilterTest.java @@ -29,6 +29,7 @@ import hudson.model.ListView; import hudson.model.User; import java.io.IOException; +import org.junit.After; import org.junit.ClassRule; import org.junit.Test; import org.jvnet.hudson.test.Issue; @@ -39,6 +40,14 @@ public class SensitiveContentFilterTest { @ClassRule public static JenkinsRule j = new JenkinsRule(); + @After + public void stopAgents() throws Exception { + for (var agent : j.jenkins.getNodes()) { + System.err.println("Stopping " + agent); + agent.toComputer().disconnect(null).get(); + } + } + @Issue("JENKINS-21670") @Test public void anonymizeAgentsAndLabels() throws Exception { diff --git a/src/test/java/com/cloudbees/jenkins/support/impl/AboutJenkinsTest.java b/src/test/java/com/cloudbees/jenkins/support/impl/AboutJenkinsTest.java index 763bdf8a5..48b4eb6c5 100644 --- a/src/test/java/com/cloudbees/jenkins/support/impl/AboutJenkinsTest.java +++ b/src/test/java/com/cloudbees/jenkins/support/impl/AboutJenkinsTest.java @@ -18,6 +18,7 @@ import jenkins.model.Jenkins; import jenkins.model.identity.IdentityRootAction; import jenkins.slaves.RemotingVersionInfo; +import org.junit.After; import org.junit.Rule; import org.junit.Test; import org.jvnet.hudson.test.Issue; @@ -31,6 +32,14 @@ public class AboutJenkinsTest { @Rule public JenkinsRule j = new JenkinsRule(); + @After + public void stopAgents() throws Exception { + for (var agent : j.jenkins.getNodes()) { + System.err.println("Stopping " + agent); + agent.toComputer().disconnect(null).get(); + } + } + @Test @Issue("JENKINS-56245") public void testAboutJenkinsContent() { diff --git a/src/test/java/com/cloudbees/jenkins/support/impl/DumpExportTableTest.java b/src/test/java/com/cloudbees/jenkins/support/impl/DumpExportTableTest.java index 8776147de..51f5c8f48 100644 --- a/src/test/java/com/cloudbees/jenkins/support/impl/DumpExportTableTest.java +++ b/src/test/java/com/cloudbees/jenkins/support/impl/DumpExportTableTest.java @@ -14,6 +14,7 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.List; +import org.junit.After; import org.junit.Rule; import org.junit.Test; import org.jvnet.hudson.test.JenkinsRule; @@ -23,6 +24,14 @@ public class DumpExportTableTest { @Rule public JenkinsRule j = new JenkinsRule(); + @After + public void stopAgents() throws Exception { + for (var agent : j.jenkins.getNodes()) { + System.err.println("Stopping " + agent); + agent.toComputer().disconnect(null).get(); + } + } + @Test public void testAddContents() throws Exception { // Given diff --git a/src/test/java/com/cloudbees/jenkins/support/impl/FileDescriptorLimitTest.java b/src/test/java/com/cloudbees/jenkins/support/impl/FileDescriptorLimitTest.java index 387c0ccc3..7f224f877 100644 --- a/src/test/java/com/cloudbees/jenkins/support/impl/FileDescriptorLimitTest.java +++ b/src/test/java/com/cloudbees/jenkins/support/impl/FileDescriptorLimitTest.java @@ -23,6 +23,7 @@ import java.util.zip.ZipFile; import org.apache.commons.io.IOUtils; import org.hamcrest.MatcherAssert; +import org.junit.After; import org.junit.Assume; import org.junit.Rule; import org.junit.Test; @@ -44,6 +45,14 @@ public class FileDescriptorLimitTest { @Rule public LoggerRule logging = new LoggerRule(); + @After + public void stopAgents() throws Exception { + for (var agent : j.jenkins.getNodes()) { + System.err.println("Stopping " + agent); + agent.toComputer().disconnect(null).get(); + } + } + @Test public void addContents() throws Exception { Assume.assumeTrue(!Functions.isWindows()); diff --git a/src/test/java/com/cloudbees/jenkins/support/impl/NodeExecutorsTest.java b/src/test/java/com/cloudbees/jenkins/support/impl/NodeExecutorsTest.java index 9cd8a0153..5e5c4561a 100644 --- a/src/test/java/com/cloudbees/jenkins/support/impl/NodeExecutorsTest.java +++ b/src/test/java/com/cloudbees/jenkins/support/impl/NodeExecutorsTest.java @@ -21,6 +21,7 @@ import org.jenkinsci.plugins.workflow.job.WorkflowJob; import org.jenkinsci.plugins.workflow.job.WorkflowRun; import org.jenkinsci.plugins.workflow.test.steps.SemaphoreStep; +import org.junit.After; import org.junit.ClassRule; import org.junit.Rule; import org.junit.Test; @@ -38,6 +39,14 @@ public class NodeExecutorsTest { @ClassRule public static BuildWatcher bw = new BuildWatcher(); + @After + public void stopAgents() throws Exception { + for (var agent : j.jenkins.getNodes()) { + System.err.println("Stopping " + agent); + agent.toComputer().disconnect(null).get(); + } + } + @Test public void addContents() throws Exception { DumbSlave agent = j.createOnlineSlave(Label.parseExpression("test"), null); diff --git a/src/test/java/com/cloudbees/jenkins/support/impl/NodeRemoteDirectoryComponentTest.java b/src/test/java/com/cloudbees/jenkins/support/impl/NodeRemoteDirectoryComponentTest.java index 729eec67f..6f5c0e92c 100644 --- a/src/test/java/com/cloudbees/jenkins/support/impl/NodeRemoteDirectoryComponentTest.java +++ b/src/test/java/com/cloudbees/jenkins/support/impl/NodeRemoteDirectoryComponentTest.java @@ -7,6 +7,7 @@ import hudson.model.Label; import hudson.slaves.DumbSlave; import java.util.Map; +import org.junit.After; import org.junit.Rule; import org.junit.Test; import org.jvnet.hudson.test.JenkinsRule; @@ -19,6 +20,14 @@ public class NodeRemoteDirectoryComponentTest { @Rule public JenkinsRule j = new JenkinsRule(); + @After + public void stopAgents() throws Exception { + for (var agent : j.jenkins.getNodes()) { + System.err.println("Stopping " + agent); + agent.toComputer().disconnect(null).get(); + } + } + /* * Test adding agent remote directory content with the defaults. */ diff --git a/src/test/java/com/cloudbees/jenkins/support/impl/SlaveCommandStatisticsTest.java b/src/test/java/com/cloudbees/jenkins/support/impl/SlaveCommandStatisticsTest.java index 3f4ad6e73..a329272f3 100644 --- a/src/test/java/com/cloudbees/jenkins/support/impl/SlaveCommandStatisticsTest.java +++ b/src/test/java/com/cloudbees/jenkins/support/impl/SlaveCommandStatisticsTest.java @@ -42,6 +42,7 @@ import java.io.IOException; import java.util.logging.Level; import jenkins.MasterToSlaveFileCallable; +import org.junit.After; import org.junit.Rule; import org.junit.Test; import org.jvnet.hudson.test.Issue; @@ -57,6 +58,14 @@ public class SlaveCommandStatisticsTest { @Rule public LoggerRule logging = new LoggerRule().record(SlaveComputer.class, Level.FINEST); + @After + public void stopAgents() throws Exception { + for (var agent : j.jenkins.getNodes()) { + System.err.println("Stopping " + agent); + agent.toComputer().disconnect(null).get(); + } + } + @Test public void smokes() throws Exception { DumbSlave s = j.createSlave(); diff --git a/src/test/java/com/cloudbees/jenkins/support/impl/SlaveLaunchLogsRestartTest.java b/src/test/java/com/cloudbees/jenkins/support/impl/SlaveLaunchLogsRestartTest.java index 1355488a7..aacdf4feb 100644 --- a/src/test/java/com/cloudbees/jenkins/support/impl/SlaveLaunchLogsRestartTest.java +++ b/src/test/java/com/cloudbees/jenkins/support/impl/SlaveLaunchLogsRestartTest.java @@ -59,6 +59,10 @@ public void twoAgents() throws Throwable { allOf( containsString("Z agent1] Remoting version: "), containsString("Z agent2] Remoting version: "))); + for (var agent : r.jenkins.getNodes()) { + System.err.println("Stopping " + agent); + agent.toComputer().disconnect(null).get(); + } }); } } diff --git a/src/test/java/com/cloudbees/jenkins/support/impl/SlaveLaunchLogsTest.java b/src/test/java/com/cloudbees/jenkins/support/impl/SlaveLaunchLogsTest.java index f00b4a4b3..bbf085cff 100644 --- a/src/test/java/com/cloudbees/jenkins/support/impl/SlaveLaunchLogsTest.java +++ b/src/test/java/com/cloudbees/jenkins/support/impl/SlaveLaunchLogsTest.java @@ -20,6 +20,7 @@ import java.util.TreeMap; import java.util.logging.Level; import jenkins.slaves.StandardOutputSwapper; +import org.junit.After; import org.junit.Rule; import org.junit.Test; import org.jvnet.hudson.test.InboundAgentRule; @@ -43,6 +44,14 @@ public class SlaveLaunchLogsTest { @Rule public LoggerRule logging = new LoggerRule().record(SlaveLaunchLogs.class, Level.FINE); + @After + public void stopAgents() throws Exception { + for (var agent : j.jenkins.getNodes()) { + System.err.println("Stopping " + agent); + agent.toComputer().disconnect(null).get(); + } + } + @Test public void onlineOutboundAgent() throws Exception { var s = j.createOnlineSlave(); diff --git a/src/test/java/com/cloudbees/jenkins/support/impl/SmartLogCleanerTest.java b/src/test/java/com/cloudbees/jenkins/support/impl/SmartLogCleanerTest.java index e330cc6cc..29f118844 100644 --- a/src/test/java/com/cloudbees/jenkins/support/impl/SmartLogCleanerTest.java +++ b/src/test/java/com/cloudbees/jenkins/support/impl/SmartLogCleanerTest.java @@ -15,6 +15,7 @@ import java.util.Collections; import java.util.List; import java.util.zip.ZipFile; +import org.junit.After; import org.junit.Rule; import org.junit.Test; import org.junit.rules.TemporaryFolder; @@ -28,6 +29,14 @@ public class SmartLogCleanerTest { @Rule public TemporaryFolder temp = new TemporaryFolder(); + @After + public void stopAgents() throws Exception { + for (var agent : j.jenkins.getNodes()) { + System.err.println("Stopping " + agent); + agent.toComputer().disconnect(null).get(); + } + } + @Test public void cleanUp() throws Exception { File cacheDir = new File(SupportPlugin.getLogsDirectory(), "winsw"); From 7fcc00b4544597652056b0d7d28fb15ac32fa969 Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Tue, 18 Feb 2025 15:16:39 -0500 Subject: [PATCH 02/17] Trying harder to delete `agent-work-dirs` --- .../jenkins/support/CheckFilterTest.java | 5 +-- .../jenkins/support/Security2186Test.java | 5 +-- .../jenkins/support/SupportActionTest.java | 5 +-- .../jenkins/support/SupportTestUtils.java | 37 +++++++++++++++++++ .../actions/SupportComputerActionTest.java | 5 +-- .../configfiles/AgentsConfigFileTest.java | 5 +-- .../filter/SensitiveContentFilterTest.java | 6 +-- .../support/impl/AboutJenkinsTest.java | 5 +-- .../support/impl/DumpExportTableTest.java | 5 +-- .../support/impl/FileDescriptorLimitTest.java | 5 +-- .../support/impl/NodeExecutorsTest.java | 5 +-- .../NodeRemoteDirectoryComponentTest.java | 5 +-- .../impl/SlaveCommandStatisticsTest.java | 5 +-- .../impl/SlaveLaunchLogsRestartTest.java | 5 +-- .../support/impl/SlaveLaunchLogsTest.java | 5 +-- .../support/impl/SmartLogCleanerTest.java | 6 +-- 16 files changed, 54 insertions(+), 60 deletions(-) diff --git a/src/test/java/com/cloudbees/jenkins/support/CheckFilterTest.java b/src/test/java/com/cloudbees/jenkins/support/CheckFilterTest.java index bac3b0875..2290aef12 100644 --- a/src/test/java/com/cloudbees/jenkins/support/CheckFilterTest.java +++ b/src/test/java/com/cloudbees/jenkins/support/CheckFilterTest.java @@ -78,10 +78,7 @@ public class CheckFilterTest { @After public void stopAgents() throws Exception { - for (var agent : j.jenkins.getNodes()) { - System.err.println("Stopping " + agent); - agent.toComputer().disconnect(null).get(); - } + SupportTestUtils.stopAgents(); } @Test diff --git a/src/test/java/com/cloudbees/jenkins/support/Security2186Test.java b/src/test/java/com/cloudbees/jenkins/support/Security2186Test.java index e374f35e4..ef56304da 100644 --- a/src/test/java/com/cloudbees/jenkins/support/Security2186Test.java +++ b/src/test/java/com/cloudbees/jenkins/support/Security2186Test.java @@ -54,10 +54,7 @@ public void setup() throws Exception { @After public void stopAgents() throws Exception { - for (var agent : j.jenkins.getNodes()) { - System.err.println("Stopping " + agent); - agent.toComputer().disconnect(null).get(); - } + SupportTestUtils.stopAgents(); } @Test diff --git a/src/test/java/com/cloudbees/jenkins/support/SupportActionTest.java b/src/test/java/com/cloudbees/jenkins/support/SupportActionTest.java index e61e27668..78b7bce91 100644 --- a/src/test/java/com/cloudbees/jenkins/support/SupportActionTest.java +++ b/src/test/java/com/cloudbees/jenkins/support/SupportActionTest.java @@ -87,10 +87,7 @@ public void setUp() { @After public void stopAgents() throws Exception { - for (var agent : j.jenkins.getNodes()) { - System.err.println("Stopping " + agent); - agent.toComputer().disconnect(null).get(); - } + SupportTestUtils.stopAgents(); } @Test diff --git a/src/test/java/com/cloudbees/jenkins/support/SupportTestUtils.java b/src/test/java/com/cloudbees/jenkins/support/SupportTestUtils.java index c5393d819..f080b9b2b 100644 --- a/src/test/java/com/cloudbees/jenkins/support/SupportTestUtils.java +++ b/src/test/java/com/cloudbees/jenkins/support/SupportTestUtils.java @@ -20,7 +20,10 @@ import java.io.ByteArrayOutputStream; import java.io.File; import java.io.IOException; +import java.nio.file.DirectoryStream; import java.nio.file.Files; +import java.nio.file.LinkOption; +import java.nio.file.Path; import java.util.Map; import java.util.Objects; import java.util.Set; @@ -48,6 +51,7 @@ * @since 2.26 */ public class SupportTestUtils { + private static final Logger LOGGER = Logger.getLogger(SupportTestUtils.class.getName()); /** * Invoke a component, and return the component contents as a String. @@ -337,4 +341,37 @@ public static void testPermissionToDisplayAction( public static boolean isJava8OrBelow() { return System.getProperty("java.specification.version").startsWith("1."); } + + public static void stopAgents() throws Exception { + for (var agent : Jenkins.get().getNodes()) { + LOGGER.info("stopping " + agent); + agent.toComputer().disconnect(null).get(); + } + var dir = Jenkins.get().getRootDir().toPath().resolve("agent-work-dirs"); + for (int i = 0; i < 60; i++) { + try { + delete(dir); + LOGGER.info("successfully deleted " + dir); + return; + } catch (IOException x) { + LOGGER.log(Level.WARNING, "failed to delete " + dir + " on attempt #" + i, x); + Thread.sleep(1000); + } + } + LOGGER.warning("never managed to delete " + dir); + } + + private static void delete(Path p) throws IOException { + LOGGER.info(() -> "deleting " + p); + if (Files.isDirectory(p, LinkOption.NOFOLLOW_LINKS)) { + try (DirectoryStream children = Files.newDirectoryStream(p)) { + for (Path child : children) { + delete(child); + } + } + } + Files.deleteIfExists(p); + } + + private SupportTestUtils() {} } diff --git a/src/test/java/com/cloudbees/jenkins/support/actions/SupportComputerActionTest.java b/src/test/java/com/cloudbees/jenkins/support/actions/SupportComputerActionTest.java index 6743156c5..832bd94d0 100644 --- a/src/test/java/com/cloudbees/jenkins/support/actions/SupportComputerActionTest.java +++ b/src/test/java/com/cloudbees/jenkins/support/actions/SupportComputerActionTest.java @@ -29,10 +29,7 @@ public class SupportComputerActionTest { @After public void stopAgents() throws Exception { - for (var agent : j.jenkins.getNodes()) { - System.err.println("Stopping " + agent); - agent.toComputer().disconnect(null).get(); - } + SupportTestUtils.stopAgents(); } @Test diff --git a/src/test/java/com/cloudbees/jenkins/support/configfiles/AgentsConfigFileTest.java b/src/test/java/com/cloudbees/jenkins/support/configfiles/AgentsConfigFileTest.java index 09b136764..c913dcc8e 100644 --- a/src/test/java/com/cloudbees/jenkins/support/configfiles/AgentsConfigFileTest.java +++ b/src/test/java/com/cloudbees/jenkins/support/configfiles/AgentsConfigFileTest.java @@ -48,10 +48,7 @@ public class AgentsConfigFileTest { @After public void stopAgents() throws Exception { - for (var agent : j.jenkins.getNodes()) { - System.err.println("Stopping " + agent); - agent.toComputer().disconnect(null).get(); - } + SupportTestUtils.stopAgents(); } @Test diff --git a/src/test/java/com/cloudbees/jenkins/support/filter/SensitiveContentFilterTest.java b/src/test/java/com/cloudbees/jenkins/support/filter/SensitiveContentFilterTest.java index 73b29c77e..987a727aa 100644 --- a/src/test/java/com/cloudbees/jenkins/support/filter/SensitiveContentFilterTest.java +++ b/src/test/java/com/cloudbees/jenkins/support/filter/SensitiveContentFilterTest.java @@ -25,6 +25,7 @@ import static org.assertj.core.api.Assertions.assertThat; +import com.cloudbees.jenkins.support.SupportTestUtils; import hudson.model.FreeStyleProject; import hudson.model.ListView; import hudson.model.User; @@ -42,10 +43,7 @@ public class SensitiveContentFilterTest { @After public void stopAgents() throws Exception { - for (var agent : j.jenkins.getNodes()) { - System.err.println("Stopping " + agent); - agent.toComputer().disconnect(null).get(); - } + SupportTestUtils.stopAgents(); } @Issue("JENKINS-21670") diff --git a/src/test/java/com/cloudbees/jenkins/support/impl/AboutJenkinsTest.java b/src/test/java/com/cloudbees/jenkins/support/impl/AboutJenkinsTest.java index 48b4eb6c5..265198f0f 100644 --- a/src/test/java/com/cloudbees/jenkins/support/impl/AboutJenkinsTest.java +++ b/src/test/java/com/cloudbees/jenkins/support/impl/AboutJenkinsTest.java @@ -34,10 +34,7 @@ public class AboutJenkinsTest { @After public void stopAgents() throws Exception { - for (var agent : j.jenkins.getNodes()) { - System.err.println("Stopping " + agent); - agent.toComputer().disconnect(null).get(); - } + SupportTestUtils.stopAgents(); } @Test diff --git a/src/test/java/com/cloudbees/jenkins/support/impl/DumpExportTableTest.java b/src/test/java/com/cloudbees/jenkins/support/impl/DumpExportTableTest.java index 51f5c8f48..c4639ef2e 100644 --- a/src/test/java/com/cloudbees/jenkins/support/impl/DumpExportTableTest.java +++ b/src/test/java/com/cloudbees/jenkins/support/impl/DumpExportTableTest.java @@ -26,10 +26,7 @@ public class DumpExportTableTest { @After public void stopAgents() throws Exception { - for (var agent : j.jenkins.getNodes()) { - System.err.println("Stopping " + agent); - agent.toComputer().disconnect(null).get(); - } + SupportTestUtils.stopAgents(); } @Test diff --git a/src/test/java/com/cloudbees/jenkins/support/impl/FileDescriptorLimitTest.java b/src/test/java/com/cloudbees/jenkins/support/impl/FileDescriptorLimitTest.java index 7f224f877..8d33e60c3 100644 --- a/src/test/java/com/cloudbees/jenkins/support/impl/FileDescriptorLimitTest.java +++ b/src/test/java/com/cloudbees/jenkins/support/impl/FileDescriptorLimitTest.java @@ -47,10 +47,7 @@ public class FileDescriptorLimitTest { @After public void stopAgents() throws Exception { - for (var agent : j.jenkins.getNodes()) { - System.err.println("Stopping " + agent); - agent.toComputer().disconnect(null).get(); - } + SupportTestUtils.stopAgents(); } @Test diff --git a/src/test/java/com/cloudbees/jenkins/support/impl/NodeExecutorsTest.java b/src/test/java/com/cloudbees/jenkins/support/impl/NodeExecutorsTest.java index 5e5c4561a..304314c40 100644 --- a/src/test/java/com/cloudbees/jenkins/support/impl/NodeExecutorsTest.java +++ b/src/test/java/com/cloudbees/jenkins/support/impl/NodeExecutorsTest.java @@ -41,10 +41,7 @@ public class NodeExecutorsTest { @After public void stopAgents() throws Exception { - for (var agent : j.jenkins.getNodes()) { - System.err.println("Stopping " + agent); - agent.toComputer().disconnect(null).get(); - } + SupportTestUtils.stopAgents(); } @Test diff --git a/src/test/java/com/cloudbees/jenkins/support/impl/NodeRemoteDirectoryComponentTest.java b/src/test/java/com/cloudbees/jenkins/support/impl/NodeRemoteDirectoryComponentTest.java index 6f5c0e92c..471e2f655 100644 --- a/src/test/java/com/cloudbees/jenkins/support/impl/NodeRemoteDirectoryComponentTest.java +++ b/src/test/java/com/cloudbees/jenkins/support/impl/NodeRemoteDirectoryComponentTest.java @@ -22,10 +22,7 @@ public class NodeRemoteDirectoryComponentTest { @After public void stopAgents() throws Exception { - for (var agent : j.jenkins.getNodes()) { - System.err.println("Stopping " + agent); - agent.toComputer().disconnect(null).get(); - } + SupportTestUtils.stopAgents(); } /* diff --git a/src/test/java/com/cloudbees/jenkins/support/impl/SlaveCommandStatisticsTest.java b/src/test/java/com/cloudbees/jenkins/support/impl/SlaveCommandStatisticsTest.java index a329272f3..72388714b 100644 --- a/src/test/java/com/cloudbees/jenkins/support/impl/SlaveCommandStatisticsTest.java +++ b/src/test/java/com/cloudbees/jenkins/support/impl/SlaveCommandStatisticsTest.java @@ -60,10 +60,7 @@ public class SlaveCommandStatisticsTest { @After public void stopAgents() throws Exception { - for (var agent : j.jenkins.getNodes()) { - System.err.println("Stopping " + agent); - agent.toComputer().disconnect(null).get(); - } + SupportTestUtils.stopAgents(); } @Test diff --git a/src/test/java/com/cloudbees/jenkins/support/impl/SlaveLaunchLogsRestartTest.java b/src/test/java/com/cloudbees/jenkins/support/impl/SlaveLaunchLogsRestartTest.java index aacdf4feb..97a7913e5 100644 --- a/src/test/java/com/cloudbees/jenkins/support/impl/SlaveLaunchLogsRestartTest.java +++ b/src/test/java/com/cloudbees/jenkins/support/impl/SlaveLaunchLogsRestartTest.java @@ -59,10 +59,7 @@ public void twoAgents() throws Throwable { allOf( containsString("Z agent1] Remoting version: "), containsString("Z agent2] Remoting version: "))); - for (var agent : r.jenkins.getNodes()) { - System.err.println("Stopping " + agent); - agent.toComputer().disconnect(null).get(); - } + SupportTestUtils.stopAgents(); }); } } diff --git a/src/test/java/com/cloudbees/jenkins/support/impl/SlaveLaunchLogsTest.java b/src/test/java/com/cloudbees/jenkins/support/impl/SlaveLaunchLogsTest.java index bbf085cff..38aa0c672 100644 --- a/src/test/java/com/cloudbees/jenkins/support/impl/SlaveLaunchLogsTest.java +++ b/src/test/java/com/cloudbees/jenkins/support/impl/SlaveLaunchLogsTest.java @@ -46,10 +46,7 @@ public class SlaveLaunchLogsTest { @After public void stopAgents() throws Exception { - for (var agent : j.jenkins.getNodes()) { - System.err.println("Stopping " + agent); - agent.toComputer().disconnect(null).get(); - } + SupportTestUtils.stopAgents(); } @Test diff --git a/src/test/java/com/cloudbees/jenkins/support/impl/SmartLogCleanerTest.java b/src/test/java/com/cloudbees/jenkins/support/impl/SmartLogCleanerTest.java index 29f118844..66a4d0a4d 100644 --- a/src/test/java/com/cloudbees/jenkins/support/impl/SmartLogCleanerTest.java +++ b/src/test/java/com/cloudbees/jenkins/support/impl/SmartLogCleanerTest.java @@ -4,6 +4,7 @@ import static org.junit.Assert.assertNotNull; import com.cloudbees.jenkins.support.SupportPlugin; +import com.cloudbees.jenkins.support.SupportTestUtils; import com.cloudbees.jenkins.support.api.Component; import com.cloudbees.jenkins.support.filter.ContentFilters; import hudson.ExtensionList; @@ -31,10 +32,7 @@ public class SmartLogCleanerTest { @After public void stopAgents() throws Exception { - for (var agent : j.jenkins.getNodes()) { - System.err.println("Stopping " + agent); - agent.toComputer().disconnect(null).get(); - } + SupportTestUtils.stopAgents(); } @Test From 06190e8f42c05bd0b23a3e0ac40ac850a66545e4 Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Tue, 18 Feb 2025 17:10:18 -0500 Subject: [PATCH 03/17] https://github.com/jenkinsci/jenkins-test-harness/pull/921 should suffice --- pom.xml | 2 ++ .../jenkins/support/SupportTestUtils.java | 27 ------------------- 2 files changed, 2 insertions(+), 27 deletions(-) diff --git a/pom.xml b/pom.xml index 8f630a2f0..e9304bed3 100644 --- a/pom.xml +++ b/pom.xml @@ -70,6 +70,8 @@ 2.479 ${jenkins.baseline}.1 + + 2398.vd015803ca_4d3 jenkinsci/${project.artifactId}-plugin false diff --git a/src/test/java/com/cloudbees/jenkins/support/SupportTestUtils.java b/src/test/java/com/cloudbees/jenkins/support/SupportTestUtils.java index f080b9b2b..75fc9ab64 100644 --- a/src/test/java/com/cloudbees/jenkins/support/SupportTestUtils.java +++ b/src/test/java/com/cloudbees/jenkins/support/SupportTestUtils.java @@ -20,10 +20,7 @@ import java.io.ByteArrayOutputStream; import java.io.File; import java.io.IOException; -import java.nio.file.DirectoryStream; import java.nio.file.Files; -import java.nio.file.LinkOption; -import java.nio.file.Path; import java.util.Map; import java.util.Objects; import java.util.Set; @@ -347,30 +344,6 @@ public static void stopAgents() throws Exception { LOGGER.info("stopping " + agent); agent.toComputer().disconnect(null).get(); } - var dir = Jenkins.get().getRootDir().toPath().resolve("agent-work-dirs"); - for (int i = 0; i < 60; i++) { - try { - delete(dir); - LOGGER.info("successfully deleted " + dir); - return; - } catch (IOException x) { - LOGGER.log(Level.WARNING, "failed to delete " + dir + " on attempt #" + i, x); - Thread.sleep(1000); - } - } - LOGGER.warning("never managed to delete " + dir); - } - - private static void delete(Path p) throws IOException { - LOGGER.info(() -> "deleting " + p); - if (Files.isDirectory(p, LinkOption.NOFOLLOW_LINKS)) { - try (DirectoryStream children = Files.newDirectoryStream(p)) { - for (Path child : children) { - delete(child); - } - } - } - Files.deleteIfExists(p); } private SupportTestUtils() {} From 201f59893099a9c3f0c1cf1c0eb2516211f81b30 Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Tue, 18 Feb 2025 18:09:46 -0500 Subject: [PATCH 04/17] Revert "https://github.com/jenkinsci/jenkins-test-harness/pull/921 should suffice" This reverts commit 06190e8f42c05bd0b23a3e0ac40ac850a66545e4. It seemed to mostly work, but not in one case. --- pom.xml | 2 -- .../jenkins/support/SupportTestUtils.java | 27 +++++++++++++++++++ 2 files changed, 27 insertions(+), 2 deletions(-) diff --git a/pom.xml b/pom.xml index e9304bed3..8f630a2f0 100644 --- a/pom.xml +++ b/pom.xml @@ -70,8 +70,6 @@ 2.479 ${jenkins.baseline}.1 - - 2398.vd015803ca_4d3 jenkinsci/${project.artifactId}-plugin false diff --git a/src/test/java/com/cloudbees/jenkins/support/SupportTestUtils.java b/src/test/java/com/cloudbees/jenkins/support/SupportTestUtils.java index 75fc9ab64..f080b9b2b 100644 --- a/src/test/java/com/cloudbees/jenkins/support/SupportTestUtils.java +++ b/src/test/java/com/cloudbees/jenkins/support/SupportTestUtils.java @@ -20,7 +20,10 @@ import java.io.ByteArrayOutputStream; import java.io.File; import java.io.IOException; +import java.nio.file.DirectoryStream; import java.nio.file.Files; +import java.nio.file.LinkOption; +import java.nio.file.Path; import java.util.Map; import java.util.Objects; import java.util.Set; @@ -344,6 +347,30 @@ public static void stopAgents() throws Exception { LOGGER.info("stopping " + agent); agent.toComputer().disconnect(null).get(); } + var dir = Jenkins.get().getRootDir().toPath().resolve("agent-work-dirs"); + for (int i = 0; i < 60; i++) { + try { + delete(dir); + LOGGER.info("successfully deleted " + dir); + return; + } catch (IOException x) { + LOGGER.log(Level.WARNING, "failed to delete " + dir + " on attempt #" + i, x); + Thread.sleep(1000); + } + } + LOGGER.warning("never managed to delete " + dir); + } + + private static void delete(Path p) throws IOException { + LOGGER.info(() -> "deleting " + p); + if (Files.isDirectory(p, LinkOption.NOFOLLOW_LINKS)) { + try (DirectoryStream children = Files.newDirectoryStream(p)) { + for (Path child : children) { + delete(child); + } + } + } + Files.deleteIfExists(p); } private SupportTestUtils() {} From 7449467f3d20490847df4ccc897f2b0c6b4ffc6c Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Tue, 18 Feb 2025 20:44:50 -0500 Subject: [PATCH 05/17] Maybe https://github.com/jenkinsci/jenkins-test-harness/pull/922 is all I needed? --- pom.xml | 2 + .../jenkins/support/CheckFilterTest.java | 6 --- .../jenkins/support/Security2186Test.java | 6 --- .../jenkins/support/SupportActionTest.java | 6 --- .../jenkins/support/SupportTestUtils.java | 37 ------------------- .../actions/SupportComputerActionTest.java | 6 --- .../configfiles/AgentsConfigFileTest.java | 6 --- .../filter/SensitiveContentFilterTest.java | 7 ---- .../support/impl/AboutJenkinsTest.java | 6 --- .../support/impl/DumpExportTableTest.java | 6 --- .../support/impl/FileDescriptorLimitTest.java | 6 --- .../support/impl/NodeExecutorsTest.java | 6 --- .../NodeRemoteDirectoryComponentTest.java | 6 --- .../impl/SlaveCommandStatisticsTest.java | 6 --- .../impl/SlaveLaunchLogsRestartTest.java | 1 - .../support/impl/SlaveLaunchLogsTest.java | 6 --- .../support/impl/SmartLogCleanerTest.java | 7 ---- 17 files changed, 2 insertions(+), 124 deletions(-) diff --git a/pom.xml b/pom.xml index 8f630a2f0..896ccf40c 100644 --- a/pom.xml +++ b/pom.xml @@ -70,6 +70,8 @@ 2.479 ${jenkins.baseline}.1 + + 2397.v7a_7075012def jenkinsci/${project.artifactId}-plugin false diff --git a/src/test/java/com/cloudbees/jenkins/support/CheckFilterTest.java b/src/test/java/com/cloudbees/jenkins/support/CheckFilterTest.java index 2290aef12..3c336b14a 100644 --- a/src/test/java/com/cloudbees/jenkins/support/CheckFilterTest.java +++ b/src/test/java/com/cloudbees/jenkins/support/CheckFilterTest.java @@ -51,7 +51,6 @@ import java.util.zip.ZipEntry; import java.util.zip.ZipFile; import jenkins.model.Jenkins; -import org.junit.After; import org.junit.Assert; import org.junit.Rule; import org.junit.Test; @@ -76,11 +75,6 @@ public class CheckFilterTest { @Rule public LoggerRule logging = new LoggerRule().record(AsyncResultCache.class, Level.FINER); - @After - public void stopAgents() throws Exception { - SupportTestUtils.stopAgents(); - } - @Test public void checkFilterTest() throws Exception { // Create the files to check diff --git a/src/test/java/com/cloudbees/jenkins/support/Security2186Test.java b/src/test/java/com/cloudbees/jenkins/support/Security2186Test.java index ef56304da..9a290f955 100644 --- a/src/test/java/com/cloudbees/jenkins/support/Security2186Test.java +++ b/src/test/java/com/cloudbees/jenkins/support/Security2186Test.java @@ -27,7 +27,6 @@ import java.util.List; import java.util.zip.ZipEntry; import java.util.zip.ZipFile; -import org.junit.After; import org.junit.Before; import org.junit.Rule; import org.junit.Test; @@ -52,11 +51,6 @@ public void setup() throws Exception { setupAgent(); } - @After - public void stopAgents() throws Exception { - SupportTestUtils.stopAgents(); - } - @Test @Issue("SECURITY-2186") public void secretsFilterWhenSystemPropertyContainsPasswordThenValueRedacted() throws Exception { diff --git a/src/test/java/com/cloudbees/jenkins/support/SupportActionTest.java b/src/test/java/com/cloudbees/jenkins/support/SupportActionTest.java index 78b7bce91..338c50946 100644 --- a/src/test/java/com/cloudbees/jenkins/support/SupportActionTest.java +++ b/src/test/java/com/cloudbees/jenkins/support/SupportActionTest.java @@ -52,7 +52,6 @@ import org.htmlunit.html.HtmlButton; import org.htmlunit.html.HtmlForm; import org.htmlunit.html.HtmlPage; -import org.junit.After; import org.junit.Assume; import org.junit.Before; import org.junit.Rule; @@ -85,11 +84,6 @@ public void setUp() { root = ExtensionList.lookupSingleton(SupportAction.class); } - @After - public void stopAgents() throws Exception { - SupportTestUtils.stopAgents(); - } - @Test public void download() throws IOException, SAXException { downloadBundle("/download?json={\"components\":1}"); diff --git a/src/test/java/com/cloudbees/jenkins/support/SupportTestUtils.java b/src/test/java/com/cloudbees/jenkins/support/SupportTestUtils.java index f080b9b2b..c5393d819 100644 --- a/src/test/java/com/cloudbees/jenkins/support/SupportTestUtils.java +++ b/src/test/java/com/cloudbees/jenkins/support/SupportTestUtils.java @@ -20,10 +20,7 @@ import java.io.ByteArrayOutputStream; import java.io.File; import java.io.IOException; -import java.nio.file.DirectoryStream; import java.nio.file.Files; -import java.nio.file.LinkOption; -import java.nio.file.Path; import java.util.Map; import java.util.Objects; import java.util.Set; @@ -51,7 +48,6 @@ * @since 2.26 */ public class SupportTestUtils { - private static final Logger LOGGER = Logger.getLogger(SupportTestUtils.class.getName()); /** * Invoke a component, and return the component contents as a String. @@ -341,37 +337,4 @@ public static void testPermissionToDisplayAction( public static boolean isJava8OrBelow() { return System.getProperty("java.specification.version").startsWith("1."); } - - public static void stopAgents() throws Exception { - for (var agent : Jenkins.get().getNodes()) { - LOGGER.info("stopping " + agent); - agent.toComputer().disconnect(null).get(); - } - var dir = Jenkins.get().getRootDir().toPath().resolve("agent-work-dirs"); - for (int i = 0; i < 60; i++) { - try { - delete(dir); - LOGGER.info("successfully deleted " + dir); - return; - } catch (IOException x) { - LOGGER.log(Level.WARNING, "failed to delete " + dir + " on attempt #" + i, x); - Thread.sleep(1000); - } - } - LOGGER.warning("never managed to delete " + dir); - } - - private static void delete(Path p) throws IOException { - LOGGER.info(() -> "deleting " + p); - if (Files.isDirectory(p, LinkOption.NOFOLLOW_LINKS)) { - try (DirectoryStream children = Files.newDirectoryStream(p)) { - for (Path child : children) { - delete(child); - } - } - } - Files.deleteIfExists(p); - } - - private SupportTestUtils() {} } diff --git a/src/test/java/com/cloudbees/jenkins/support/actions/SupportComputerActionTest.java b/src/test/java/com/cloudbees/jenkins/support/actions/SupportComputerActionTest.java index 832bd94d0..6aac3815c 100644 --- a/src/test/java/com/cloudbees/jenkins/support/actions/SupportComputerActionTest.java +++ b/src/test/java/com/cloudbees/jenkins/support/actions/SupportComputerActionTest.java @@ -14,7 +14,6 @@ import java.util.stream.Stream; import java.util.zip.ZipFile; import jenkins.model.Jenkins; -import org.junit.After; import org.junit.Rule; import org.junit.Test; import org.jvnet.hudson.test.JenkinsRule; @@ -27,11 +26,6 @@ public class SupportComputerActionTest { @Rule public JenkinsRule j = new JenkinsRule(); - @After - public void stopAgents() throws Exception { - SupportTestUtils.stopAgents(); - } - @Test public void onlyAdminCanSeeAction() throws Exception { diff --git a/src/test/java/com/cloudbees/jenkins/support/configfiles/AgentsConfigFileTest.java b/src/test/java/com/cloudbees/jenkins/support/configfiles/AgentsConfigFileTest.java index c913dcc8e..e026727ad 100644 --- a/src/test/java/com/cloudbees/jenkins/support/configfiles/AgentsConfigFileTest.java +++ b/src/test/java/com/cloudbees/jenkins/support/configfiles/AgentsConfigFileTest.java @@ -33,7 +33,6 @@ import hudson.EnvVars; import java.util.Map; import org.hamcrest.MatcherAssert; -import org.junit.After; import org.junit.Rule; import org.junit.Test; import org.jvnet.hudson.test.Issue; @@ -46,11 +45,6 @@ public class AgentsConfigFileTest { @Rule public JenkinsRule j = new JenkinsRule(); - @After - public void stopAgents() throws Exception { - SupportTestUtils.stopAgents(); - } - @Test public void agentsConfigFile() throws Exception { j.createSlave("node1", "node1", new EnvVars()); diff --git a/src/test/java/com/cloudbees/jenkins/support/filter/SensitiveContentFilterTest.java b/src/test/java/com/cloudbees/jenkins/support/filter/SensitiveContentFilterTest.java index 987a727aa..d52fabf78 100644 --- a/src/test/java/com/cloudbees/jenkins/support/filter/SensitiveContentFilterTest.java +++ b/src/test/java/com/cloudbees/jenkins/support/filter/SensitiveContentFilterTest.java @@ -25,12 +25,10 @@ import static org.assertj.core.api.Assertions.assertThat; -import com.cloudbees.jenkins.support.SupportTestUtils; import hudson.model.FreeStyleProject; import hudson.model.ListView; import hudson.model.User; import java.io.IOException; -import org.junit.After; import org.junit.ClassRule; import org.junit.Test; import org.jvnet.hudson.test.Issue; @@ -41,11 +39,6 @@ public class SensitiveContentFilterTest { @ClassRule public static JenkinsRule j = new JenkinsRule(); - @After - public void stopAgents() throws Exception { - SupportTestUtils.stopAgents(); - } - @Issue("JENKINS-21670") @Test public void anonymizeAgentsAndLabels() throws Exception { diff --git a/src/test/java/com/cloudbees/jenkins/support/impl/AboutJenkinsTest.java b/src/test/java/com/cloudbees/jenkins/support/impl/AboutJenkinsTest.java index 265198f0f..763bdf8a5 100644 --- a/src/test/java/com/cloudbees/jenkins/support/impl/AboutJenkinsTest.java +++ b/src/test/java/com/cloudbees/jenkins/support/impl/AboutJenkinsTest.java @@ -18,7 +18,6 @@ import jenkins.model.Jenkins; import jenkins.model.identity.IdentityRootAction; import jenkins.slaves.RemotingVersionInfo; -import org.junit.After; import org.junit.Rule; import org.junit.Test; import org.jvnet.hudson.test.Issue; @@ -32,11 +31,6 @@ public class AboutJenkinsTest { @Rule public JenkinsRule j = new JenkinsRule(); - @After - public void stopAgents() throws Exception { - SupportTestUtils.stopAgents(); - } - @Test @Issue("JENKINS-56245") public void testAboutJenkinsContent() { diff --git a/src/test/java/com/cloudbees/jenkins/support/impl/DumpExportTableTest.java b/src/test/java/com/cloudbees/jenkins/support/impl/DumpExportTableTest.java index c4639ef2e..8776147de 100644 --- a/src/test/java/com/cloudbees/jenkins/support/impl/DumpExportTableTest.java +++ b/src/test/java/com/cloudbees/jenkins/support/impl/DumpExportTableTest.java @@ -14,7 +14,6 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.List; -import org.junit.After; import org.junit.Rule; import org.junit.Test; import org.jvnet.hudson.test.JenkinsRule; @@ -24,11 +23,6 @@ public class DumpExportTableTest { @Rule public JenkinsRule j = new JenkinsRule(); - @After - public void stopAgents() throws Exception { - SupportTestUtils.stopAgents(); - } - @Test public void testAddContents() throws Exception { // Given diff --git a/src/test/java/com/cloudbees/jenkins/support/impl/FileDescriptorLimitTest.java b/src/test/java/com/cloudbees/jenkins/support/impl/FileDescriptorLimitTest.java index 8d33e60c3..387c0ccc3 100644 --- a/src/test/java/com/cloudbees/jenkins/support/impl/FileDescriptorLimitTest.java +++ b/src/test/java/com/cloudbees/jenkins/support/impl/FileDescriptorLimitTest.java @@ -23,7 +23,6 @@ import java.util.zip.ZipFile; import org.apache.commons.io.IOUtils; import org.hamcrest.MatcherAssert; -import org.junit.After; import org.junit.Assume; import org.junit.Rule; import org.junit.Test; @@ -45,11 +44,6 @@ public class FileDescriptorLimitTest { @Rule public LoggerRule logging = new LoggerRule(); - @After - public void stopAgents() throws Exception { - SupportTestUtils.stopAgents(); - } - @Test public void addContents() throws Exception { Assume.assumeTrue(!Functions.isWindows()); diff --git a/src/test/java/com/cloudbees/jenkins/support/impl/NodeExecutorsTest.java b/src/test/java/com/cloudbees/jenkins/support/impl/NodeExecutorsTest.java index 304314c40..9cd8a0153 100644 --- a/src/test/java/com/cloudbees/jenkins/support/impl/NodeExecutorsTest.java +++ b/src/test/java/com/cloudbees/jenkins/support/impl/NodeExecutorsTest.java @@ -21,7 +21,6 @@ import org.jenkinsci.plugins.workflow.job.WorkflowJob; import org.jenkinsci.plugins.workflow.job.WorkflowRun; import org.jenkinsci.plugins.workflow.test.steps.SemaphoreStep; -import org.junit.After; import org.junit.ClassRule; import org.junit.Rule; import org.junit.Test; @@ -39,11 +38,6 @@ public class NodeExecutorsTest { @ClassRule public static BuildWatcher bw = new BuildWatcher(); - @After - public void stopAgents() throws Exception { - SupportTestUtils.stopAgents(); - } - @Test public void addContents() throws Exception { DumbSlave agent = j.createOnlineSlave(Label.parseExpression("test"), null); diff --git a/src/test/java/com/cloudbees/jenkins/support/impl/NodeRemoteDirectoryComponentTest.java b/src/test/java/com/cloudbees/jenkins/support/impl/NodeRemoteDirectoryComponentTest.java index 471e2f655..729eec67f 100644 --- a/src/test/java/com/cloudbees/jenkins/support/impl/NodeRemoteDirectoryComponentTest.java +++ b/src/test/java/com/cloudbees/jenkins/support/impl/NodeRemoteDirectoryComponentTest.java @@ -7,7 +7,6 @@ import hudson.model.Label; import hudson.slaves.DumbSlave; import java.util.Map; -import org.junit.After; import org.junit.Rule; import org.junit.Test; import org.jvnet.hudson.test.JenkinsRule; @@ -20,11 +19,6 @@ public class NodeRemoteDirectoryComponentTest { @Rule public JenkinsRule j = new JenkinsRule(); - @After - public void stopAgents() throws Exception { - SupportTestUtils.stopAgents(); - } - /* * Test adding agent remote directory content with the defaults. */ diff --git a/src/test/java/com/cloudbees/jenkins/support/impl/SlaveCommandStatisticsTest.java b/src/test/java/com/cloudbees/jenkins/support/impl/SlaveCommandStatisticsTest.java index 72388714b..3f4ad6e73 100644 --- a/src/test/java/com/cloudbees/jenkins/support/impl/SlaveCommandStatisticsTest.java +++ b/src/test/java/com/cloudbees/jenkins/support/impl/SlaveCommandStatisticsTest.java @@ -42,7 +42,6 @@ import java.io.IOException; import java.util.logging.Level; import jenkins.MasterToSlaveFileCallable; -import org.junit.After; import org.junit.Rule; import org.junit.Test; import org.jvnet.hudson.test.Issue; @@ -58,11 +57,6 @@ public class SlaveCommandStatisticsTest { @Rule public LoggerRule logging = new LoggerRule().record(SlaveComputer.class, Level.FINEST); - @After - public void stopAgents() throws Exception { - SupportTestUtils.stopAgents(); - } - @Test public void smokes() throws Exception { DumbSlave s = j.createSlave(); diff --git a/src/test/java/com/cloudbees/jenkins/support/impl/SlaveLaunchLogsRestartTest.java b/src/test/java/com/cloudbees/jenkins/support/impl/SlaveLaunchLogsRestartTest.java index 97a7913e5..1355488a7 100644 --- a/src/test/java/com/cloudbees/jenkins/support/impl/SlaveLaunchLogsRestartTest.java +++ b/src/test/java/com/cloudbees/jenkins/support/impl/SlaveLaunchLogsRestartTest.java @@ -59,7 +59,6 @@ public void twoAgents() throws Throwable { allOf( containsString("Z agent1] Remoting version: "), containsString("Z agent2] Remoting version: "))); - SupportTestUtils.stopAgents(); }); } } diff --git a/src/test/java/com/cloudbees/jenkins/support/impl/SlaveLaunchLogsTest.java b/src/test/java/com/cloudbees/jenkins/support/impl/SlaveLaunchLogsTest.java index 38aa0c672..f00b4a4b3 100644 --- a/src/test/java/com/cloudbees/jenkins/support/impl/SlaveLaunchLogsTest.java +++ b/src/test/java/com/cloudbees/jenkins/support/impl/SlaveLaunchLogsTest.java @@ -20,7 +20,6 @@ import java.util.TreeMap; import java.util.logging.Level; import jenkins.slaves.StandardOutputSwapper; -import org.junit.After; import org.junit.Rule; import org.junit.Test; import org.jvnet.hudson.test.InboundAgentRule; @@ -44,11 +43,6 @@ public class SlaveLaunchLogsTest { @Rule public LoggerRule logging = new LoggerRule().record(SlaveLaunchLogs.class, Level.FINE); - @After - public void stopAgents() throws Exception { - SupportTestUtils.stopAgents(); - } - @Test public void onlineOutboundAgent() throws Exception { var s = j.createOnlineSlave(); diff --git a/src/test/java/com/cloudbees/jenkins/support/impl/SmartLogCleanerTest.java b/src/test/java/com/cloudbees/jenkins/support/impl/SmartLogCleanerTest.java index 66a4d0a4d..e330cc6cc 100644 --- a/src/test/java/com/cloudbees/jenkins/support/impl/SmartLogCleanerTest.java +++ b/src/test/java/com/cloudbees/jenkins/support/impl/SmartLogCleanerTest.java @@ -4,7 +4,6 @@ import static org.junit.Assert.assertNotNull; import com.cloudbees.jenkins.support.SupportPlugin; -import com.cloudbees.jenkins.support.SupportTestUtils; import com.cloudbees.jenkins.support.api.Component; import com.cloudbees.jenkins.support.filter.ContentFilters; import hudson.ExtensionList; @@ -16,7 +15,6 @@ import java.util.Collections; import java.util.List; import java.util.zip.ZipFile; -import org.junit.After; import org.junit.Rule; import org.junit.Test; import org.junit.rules.TemporaryFolder; @@ -30,11 +28,6 @@ public class SmartLogCleanerTest { @Rule public TemporaryFolder temp = new TemporaryFolder(); - @After - public void stopAgents() throws Exception { - SupportTestUtils.stopAgents(); - } - @Test public void cleanUp() throws Exception { File cacheDir = new File(SupportPlugin.getLogsDirectory(), "winsw"); From 6aafeb399fa21dcf400f0d99d8c4fc76b78fc3d5 Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Tue, 18 Feb 2025 21:43:02 -0500 Subject: [PATCH 06/17] Bump --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index 896ccf40c..f9eb41581 100644 --- a/pom.xml +++ b/pom.xml @@ -71,7 +71,7 @@ 2.479 ${jenkins.baseline}.1 - 2397.v7a_7075012def + 2400.v86a_2b_c9d99d6 jenkinsci/${project.artifactId}-plugin false From 173a6c4305fcec650ff3ab176c319b76543cf2c2 Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Tue, 18 Feb 2025 22:15:17 -0500 Subject: [PATCH 07/17] Another attempt at closing log handlers before exit, this time not relying on channel events --- .../cloudbees/jenkins/support/SupportPlugin.java | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/src/main/java/com/cloudbees/jenkins/support/SupportPlugin.java b/src/main/java/com/cloudbees/jenkins/support/SupportPlugin.java index 3e57d01a3..bf8256f1b 100644 --- a/src/main/java/com/cloudbees/jenkins/support/SupportPlugin.java +++ b/src/main/java/com/cloudbees/jenkins/support/SupportPlugin.java @@ -862,6 +862,15 @@ public Void call() { // avoid double installation of the handler. JNLP agents can reconnect to the controller multiple times // and each connection gets a different RemoteClassLoader, so we need to evict them by class name, // not by their identity. + closeAll(); + Runtime.getRuntime().addShutdownHook(new Thread(LogInitializer::closeAll, "close log handlers")); + LogHolder.AGENT_LOG_HANDLER.setLevel(level); + LogHolder.AGENT_LOG_HANDLER.setDirectory(new File(rootPath.getRemote(), SUPPORT_DIRECTORY_NAME), "all"); + ROOT_LOGGER.addHandler(LogHolder.AGENT_LOG_HANDLER); + return null; + } + + private static void closeAll() { for (Handler h : ROOT_LOGGER.getHandlers()) { if (h.getClass() .getName() @@ -874,10 +883,6 @@ public Void call() { } } } - LogHolder.AGENT_LOG_HANDLER.setLevel(level); - LogHolder.AGENT_LOG_HANDLER.setDirectory(new File(rootPath.getRemote(), SUPPORT_DIRECTORY_NAME), "all"); - ROOT_LOGGER.addHandler(LogHolder.AGENT_LOG_HANDLER); - return null; } } From b70d8db6f8f0a0674021a63edea0de61af2d6eb4 Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Wed, 19 Feb 2025 09:52:18 -0500 Subject: [PATCH 08/17] Trying to get a diagnostic log of what is going on --- .../jenkins/support/SupportLogHandler.java | 3 ++ .../jenkins/support/SupportPlugin.java | 37 ++++++++++++++++++- .../NodeRemoteDirectoryComponentTest.java | 19 ++++++++++ 3 files changed, 58 insertions(+), 1 deletion(-) diff --git a/src/main/java/com/cloudbees/jenkins/support/SupportLogHandler.java b/src/main/java/com/cloudbees/jenkins/support/SupportLogHandler.java index 842b34d6e..652e56ad5 100644 --- a/src/main/java/com/cloudbees/jenkins/support/SupportLogHandler.java +++ b/src/main/java/com/cloudbees/jenkins/support/SupportLogHandler.java @@ -177,10 +177,12 @@ public void flush() { @Override public void close() throws SecurityException { + SupportPlugin.safeLog("close " + this.logDirectry + " " + this.logFilePrefix); outputLock.lock(); try { if (writer != null) { StreamUtils.closeQuietly(writer); + SupportPlugin.safeLog("closed " + writer); writer = null; } } finally { @@ -243,6 +245,7 @@ private void setFile(File file) throws FileNotFoundException { bos = new BufferedOutputStream(fos); writer = new OutputStreamWriter(bos, StandardCharsets.UTF_8); setWriter(writer); + SupportPlugin.safeLog("setWriter " + file); fileCount = 0; success = true; } finally { diff --git a/src/main/java/com/cloudbees/jenkins/support/SupportPlugin.java b/src/main/java/com/cloudbees/jenkins/support/SupportPlugin.java index bf8256f1b..4a6ba8373 100644 --- a/src/main/java/com/cloudbees/jenkins/support/SupportPlugin.java +++ b/src/main/java/com/cloudbees/jenkins/support/SupportPlugin.java @@ -80,8 +80,10 @@ import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.nio.file.Path; +import java.nio.file.StandardOpenOption; import java.text.MessageFormat; import java.text.SimpleDateFormat; +import java.time.Instant; import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; @@ -108,6 +110,7 @@ import jenkins.model.GlobalConfiguration; import jenkins.model.Jenkins; import jenkins.security.MasterToSlaveCallable; +import jenkins.util.JenkinsJVM; import net.sf.json.JSONObject; import org.apache.commons.io.output.CountingOutputStream; import org.apache.commons.lang.StringUtils; @@ -859,11 +862,13 @@ public LogInitializer(FilePath rootPath, Level level) { } public Void call() { + safeLog("LogInitializer on " + rootPath); // avoid double installation of the handler. JNLP agents can reconnect to the controller multiple times // and each connection gets a different RemoteClassLoader, so we need to evict them by class name, // not by their identity. closeAll(); Runtime.getRuntime().addShutdownHook(new Thread(LogInitializer::closeAll, "close log handlers")); + safeLog("initialized hook"); LogHolder.AGENT_LOG_HANDLER.setLevel(level); LogHolder.AGENT_LOG_HANDLER.setDirectory(new File(rootPath.getRemote(), SUPPORT_DIRECTORY_NAME), "all"); ROOT_LOGGER.addHandler(LogHolder.AGENT_LOG_HANDLER); @@ -871,6 +876,7 @@ public Void call() { } private static void closeAll() { + safeLog("closeAll"); for (Handler h : ROOT_LOGGER.getHandlers()) { if (h.getClass() .getName() @@ -878,14 +884,38 @@ private static void closeAll() { ROOT_LOGGER.removeHandler(h); try { h.close(); + safeLog("removed " + h); } catch (Throwable t) { - // ignore + safeLog("close failure: " + t); } } } } } + public static final Path safeLogFile; + + static { + // Cannot use System.getProperty("java.io.tmpdir") since this has different values + // for test/controller JVM vs. agent (due to override in plugin POM). + try { + var tmp = Files.createTempFile(null, null); + Files.delete(tmp); + safeLogFile = tmp.getParent().resolve("safe.log"); + } catch (IOException x) { + throw new AssertionError(x); + } + } + + static void safeLog(String message) { + // TODO FileChannel.open + .lock if necessary to synchronize access + try (var w = Files.newBufferedWriter(safeLogFile, StandardOpenOption.CREATE, StandardOpenOption.APPEND)) { + w.write(Instant.now() + " [" + (JenkinsJVM.isJenkinsJVM() ? "J" : "A") + "] " + message + "\n"); + } catch (IOException x) { + x.printStackTrace(); + } + } + public static class LogFetcher extends MasterToSlaveCallable, RuntimeException> { private static final long serialVersionUID = 1L; @@ -923,7 +953,12 @@ public void onOnline(Computer c, TaskListener listener) throws IOException, Inte if (channel != null) { final FilePath rootPath = node.getRootPath(); if (rootPath != null) { + safeLog("will call on " + rootPath); CallAsyncWrapper.callAsync(channel, new LogInitializer(rootPath, getLogLevel())); + // Note that this may or may not run before the test completes; to force it to run: + // Thread.sleep(3000); + // alternately: + // channel.call(new LogInitializer(rootPath, getLogLevel())); } } } catch (IOException e) { diff --git a/src/test/java/com/cloudbees/jenkins/support/impl/NodeRemoteDirectoryComponentTest.java b/src/test/java/com/cloudbees/jenkins/support/impl/NodeRemoteDirectoryComponentTest.java index 729eec67f..a2e1315c9 100644 --- a/src/test/java/com/cloudbees/jenkins/support/impl/NodeRemoteDirectoryComponentTest.java +++ b/src/test/java/com/cloudbees/jenkins/support/impl/NodeRemoteDirectoryComponentTest.java @@ -3,10 +3,15 @@ import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; +import com.cloudbees.jenkins.support.SupportPlugin; import com.cloudbees.jenkins.support.SupportTestUtils; import hudson.model.Label; import hudson.slaves.DumbSlave; +import java.nio.file.Files; +import java.nio.file.StandardOpenOption; import java.util.Map; +import org.junit.AfterClass; +import org.junit.BeforeClass; import org.junit.Rule; import org.junit.Test; import org.jvnet.hudson.test.JenkinsRule; @@ -19,6 +24,20 @@ public class NodeRemoteDirectoryComponentTest { @Rule public JenkinsRule j = new JenkinsRule(); + @BeforeClass + public static void clearLog() throws Exception { + try (var os = Files.newOutputStream( + SupportPlugin.safeLogFile, StandardOpenOption.CREATE, StandardOpenOption.TRUNCATE_EXISTING)) {} + System.err.println("Cleared " + SupportPlugin.safeLogFile); + } + + @AfterClass + public static void showLog() throws Exception { + System.err.println("---%<--- " + SupportPlugin.safeLogFile); + Files.copy(SupportPlugin.safeLogFile, System.err); + System.err.println("--->%--- "); + } + /* * Test adding agent remote directory content with the defaults. */ From bbc4d5ba30be6d8160c0fc70c64a69161a03f9e9 Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Wed, 19 Feb 2025 10:33:56 -0500 Subject: [PATCH 09/17] Extracted `SafeLog` to a separate class safe to load on agents --- .../cloudbees/jenkins/support/SafeLog.java | 60 +++++++++++++++++++ .../jenkins/support/SupportLogHandler.java | 6 +- .../jenkins/support/SupportPlugin.java | 38 ++---------- .../NodeRemoteDirectoryComponentTest.java | 12 ++-- 4 files changed, 75 insertions(+), 41 deletions(-) create mode 100644 src/main/java/com/cloudbees/jenkins/support/SafeLog.java diff --git a/src/main/java/com/cloudbees/jenkins/support/SafeLog.java b/src/main/java/com/cloudbees/jenkins/support/SafeLog.java new file mode 100644 index 000000000..7de474287 --- /dev/null +++ b/src/main/java/com/cloudbees/jenkins/support/SafeLog.java @@ -0,0 +1,60 @@ +/* + * The MIT License + * + * Copyright 2025 CloudBees, Inc. + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN + * THE SOFTWARE. + */ + +package com.cloudbees.jenkins.support; + +import java.io.BufferedWriter; +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.StandardOpenOption; +import java.time.Instant; +import jenkins.util.JenkinsJVM; + +public class SafeLog { + public static final Path file; + + static { + // Cannot use System.getProperty("java.io.tmpdir") since this has different values + // for test/controller JVM vs. agent (due to override in plugin POM). + try { + var tmp = Files.createTempFile(null, null); + Files.delete(tmp); + file = tmp.getParent().resolve("safe.log"); + } catch (IOException x) { + throw new AssertionError(x); + } + } + + public static void print(String message) { + // TODO FileChannel.open + .lock if necessary to synchronize access + try (BufferedWriter w = Files.newBufferedWriter(file, StandardOpenOption.CREATE, StandardOpenOption.APPEND)) { + w.write(Instant.now() + " [" + (JenkinsJVM.isJenkinsJVM() ? "J" : "A") + "] " + message + "\n"); + } catch (IOException x) { + x.printStackTrace(); + } + } + + private SafeLog() {} +} diff --git a/src/main/java/com/cloudbees/jenkins/support/SupportLogHandler.java b/src/main/java/com/cloudbees/jenkins/support/SupportLogHandler.java index 652e56ad5..1efa01848 100644 --- a/src/main/java/com/cloudbees/jenkins/support/SupportLogHandler.java +++ b/src/main/java/com/cloudbees/jenkins/support/SupportLogHandler.java @@ -177,12 +177,12 @@ public void flush() { @Override public void close() throws SecurityException { - SupportPlugin.safeLog("close " + this.logDirectry + " " + this.logFilePrefix); + SafeLog.print("close " + this.logDirectry + " " + this.logFilePrefix); outputLock.lock(); try { if (writer != null) { StreamUtils.closeQuietly(writer); - SupportPlugin.safeLog("closed " + writer); + SafeLog.print("closed " + writer); writer = null; } } finally { @@ -245,7 +245,7 @@ private void setFile(File file) throws FileNotFoundException { bos = new BufferedOutputStream(fos); writer = new OutputStreamWriter(bos, StandardCharsets.UTF_8); setWriter(writer); - SupportPlugin.safeLog("setWriter " + file); + SafeLog.print("setWriter " + file); fileCount = 0; success = true; } finally { diff --git a/src/main/java/com/cloudbees/jenkins/support/SupportPlugin.java b/src/main/java/com/cloudbees/jenkins/support/SupportPlugin.java index 4a6ba8373..dbac4750e 100644 --- a/src/main/java/com/cloudbees/jenkins/support/SupportPlugin.java +++ b/src/main/java/com/cloudbees/jenkins/support/SupportPlugin.java @@ -80,10 +80,8 @@ import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.nio.file.Path; -import java.nio.file.StandardOpenOption; import java.text.MessageFormat; import java.text.SimpleDateFormat; -import java.time.Instant; import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; @@ -110,7 +108,6 @@ import jenkins.model.GlobalConfiguration; import jenkins.model.Jenkins; import jenkins.security.MasterToSlaveCallable; -import jenkins.util.JenkinsJVM; import net.sf.json.JSONObject; import org.apache.commons.io.output.CountingOutputStream; import org.apache.commons.lang.StringUtils; @@ -862,13 +859,13 @@ public LogInitializer(FilePath rootPath, Level level) { } public Void call() { - safeLog("LogInitializer on " + rootPath); + SafeLog.print("LogInitializer on " + rootPath); // avoid double installation of the handler. JNLP agents can reconnect to the controller multiple times // and each connection gets a different RemoteClassLoader, so we need to evict them by class name, // not by their identity. closeAll(); Runtime.getRuntime().addShutdownHook(new Thread(LogInitializer::closeAll, "close log handlers")); - safeLog("initialized hook"); + SafeLog.print("initialized hook"); LogHolder.AGENT_LOG_HANDLER.setLevel(level); LogHolder.AGENT_LOG_HANDLER.setDirectory(new File(rootPath.getRemote(), SUPPORT_DIRECTORY_NAME), "all"); ROOT_LOGGER.addHandler(LogHolder.AGENT_LOG_HANDLER); @@ -876,7 +873,7 @@ public Void call() { } private static void closeAll() { - safeLog("closeAll"); + SafeLog.print("closeAll"); for (Handler h : ROOT_LOGGER.getHandlers()) { if (h.getClass() .getName() @@ -884,38 +881,15 @@ private static void closeAll() { ROOT_LOGGER.removeHandler(h); try { h.close(); - safeLog("removed " + h); + SafeLog.print("removed " + h); } catch (Throwable t) { - safeLog("close failure: " + t); + SafeLog.print("close failure: " + t); } } } } } - public static final Path safeLogFile; - - static { - // Cannot use System.getProperty("java.io.tmpdir") since this has different values - // for test/controller JVM vs. agent (due to override in plugin POM). - try { - var tmp = Files.createTempFile(null, null); - Files.delete(tmp); - safeLogFile = tmp.getParent().resolve("safe.log"); - } catch (IOException x) { - throw new AssertionError(x); - } - } - - static void safeLog(String message) { - // TODO FileChannel.open + .lock if necessary to synchronize access - try (var w = Files.newBufferedWriter(safeLogFile, StandardOpenOption.CREATE, StandardOpenOption.APPEND)) { - w.write(Instant.now() + " [" + (JenkinsJVM.isJenkinsJVM() ? "J" : "A") + "] " + message + "\n"); - } catch (IOException x) { - x.printStackTrace(); - } - } - public static class LogFetcher extends MasterToSlaveCallable, RuntimeException> { private static final long serialVersionUID = 1L; @@ -953,7 +927,7 @@ public void onOnline(Computer c, TaskListener listener) throws IOException, Inte if (channel != null) { final FilePath rootPath = node.getRootPath(); if (rootPath != null) { - safeLog("will call on " + rootPath); + SafeLog.print("will call on " + rootPath); CallAsyncWrapper.callAsync(channel, new LogInitializer(rootPath, getLogLevel())); // Note that this may or may not run before the test completes; to force it to run: // Thread.sleep(3000); diff --git a/src/test/java/com/cloudbees/jenkins/support/impl/NodeRemoteDirectoryComponentTest.java b/src/test/java/com/cloudbees/jenkins/support/impl/NodeRemoteDirectoryComponentTest.java index a2e1315c9..5a13b5ce1 100644 --- a/src/test/java/com/cloudbees/jenkins/support/impl/NodeRemoteDirectoryComponentTest.java +++ b/src/test/java/com/cloudbees/jenkins/support/impl/NodeRemoteDirectoryComponentTest.java @@ -3,7 +3,7 @@ import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; -import com.cloudbees.jenkins.support.SupportPlugin; +import com.cloudbees.jenkins.support.SafeLog; import com.cloudbees.jenkins.support.SupportTestUtils; import hudson.model.Label; import hudson.slaves.DumbSlave; @@ -26,15 +26,15 @@ public class NodeRemoteDirectoryComponentTest { @BeforeClass public static void clearLog() throws Exception { - try (var os = Files.newOutputStream( - SupportPlugin.safeLogFile, StandardOpenOption.CREATE, StandardOpenOption.TRUNCATE_EXISTING)) {} - System.err.println("Cleared " + SupportPlugin.safeLogFile); + try (var os = + Files.newOutputStream(SafeLog.file, StandardOpenOption.CREATE, StandardOpenOption.TRUNCATE_EXISTING)) {} + System.err.println("Cleared " + SafeLog.file); } @AfterClass public static void showLog() throws Exception { - System.err.println("---%<--- " + SupportPlugin.safeLogFile); - Files.copy(SupportPlugin.safeLogFile, System.err); + System.err.println("---%<--- " + SafeLog.file); + Files.copy(SafeLog.file, System.err); System.err.println("--->%--- "); } From cb46c860d75ca0f54549cd690195b2e3f288cb27 Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Wed, 19 Feb 2025 10:36:38 -0500 Subject: [PATCH 10/17] Use both `@After` and `@AfterClass` since Surefire/Jenkins output seems to skip `@AfterClass` output? --- .../support/impl/NodeRemoteDirectoryComponentTest.java | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/test/java/com/cloudbees/jenkins/support/impl/NodeRemoteDirectoryComponentTest.java b/src/test/java/com/cloudbees/jenkins/support/impl/NodeRemoteDirectoryComponentTest.java index 5a13b5ce1..9afe3fd68 100644 --- a/src/test/java/com/cloudbees/jenkins/support/impl/NodeRemoteDirectoryComponentTest.java +++ b/src/test/java/com/cloudbees/jenkins/support/impl/NodeRemoteDirectoryComponentTest.java @@ -10,6 +10,7 @@ import java.nio.file.Files; import java.nio.file.StandardOpenOption; import java.util.Map; +import org.junit.After; import org.junit.AfterClass; import org.junit.BeforeClass; import org.junit.Rule; @@ -31,8 +32,15 @@ public static void clearLog() throws Exception { System.err.println("Cleared " + SafeLog.file); } + @After + public void showLog() throws Exception { + System.err.println("---%<--- " + SafeLog.file); + Files.copy(SafeLog.file, System.err); + System.err.println("--->%--- "); + } + @AfterClass - public static void showLog() throws Exception { + public static void showFinalLog() throws Exception { System.err.println("---%<--- " + SafeLog.file); Files.copy(SafeLog.file, System.err); System.err.println("--->%--- "); From 2a9b6e13c348a1ca52894c999e54d5d5bef34791 Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Wed, 19 Feb 2025 10:40:29 -0500 Subject: [PATCH 11/17] `JenkinsSessionRule` seems to solve that issue --- .../NodeRemoteDirectoryComponentTest.java | 100 +++++++++--------- 1 file changed, 50 insertions(+), 50 deletions(-) diff --git a/src/test/java/com/cloudbees/jenkins/support/impl/NodeRemoteDirectoryComponentTest.java b/src/test/java/com/cloudbees/jenkins/support/impl/NodeRemoteDirectoryComponentTest.java index 9afe3fd68..753be5526 100644 --- a/src/test/java/com/cloudbees/jenkins/support/impl/NodeRemoteDirectoryComponentTest.java +++ b/src/test/java/com/cloudbees/jenkins/support/impl/NodeRemoteDirectoryComponentTest.java @@ -11,11 +11,10 @@ import java.nio.file.StandardOpenOption; import java.util.Map; import org.junit.After; -import org.junit.AfterClass; -import org.junit.BeforeClass; +import org.junit.Before; import org.junit.Rule; import org.junit.Test; -import org.jvnet.hudson.test.JenkinsRule; +import org.jvnet.hudson.test.JenkinsSessionRule; /** * Tests for the {@link NodeRemoteDirectoryComponent} @@ -23,24 +22,17 @@ public class NodeRemoteDirectoryComponentTest { @Rule - public JenkinsRule j = new JenkinsRule(); + public JenkinsSessionRule r = new JenkinsSessionRule(); - @BeforeClass - public static void clearLog() throws Exception { + @Before + public void clearLog() throws Throwable { try (var os = Files.newOutputStream(SafeLog.file, StandardOpenOption.CREATE, StandardOpenOption.TRUNCATE_EXISTING)) {} System.err.println("Cleared " + SafeLog.file); } @After - public void showLog() throws Exception { - System.err.println("---%<--- " + SafeLog.file); - Files.copy(SafeLog.file, System.err); - System.err.println("--->%--- "); - } - - @AfterClass - public static void showFinalLog() throws Exception { + public void showLog() throws Throwable { System.err.println("---%<--- " + SafeLog.file); Files.copy(SafeLog.file, System.err); System.err.println("--->%--- "); @@ -50,61 +42,69 @@ public static void showFinalLog() throws Exception { * Test adding agent remote directory content with the defaults. */ @Test - public void addContents() throws Exception { - DumbSlave agent = j.createOnlineSlave(Label.parseExpression("test"), null); + public void addContents() throws Throwable { + r.then(j -> { + DumbSlave agent = j.createOnlineSlave(Label.parseExpression("test"), null); - Map output = - SupportTestUtils.invokeComponentToMap(new NodeRemoteDirectoryComponent(), agent.toComputer()); + Map output = + SupportTestUtils.invokeComponentToMap(new NodeRemoteDirectoryComponent(), agent.toComputer()); - String prefix = "nodes/slave/" + agent.getNodeName() + "/remote"; - assertTrue(output.keySet().stream().anyMatch(key -> key.matches(prefix + "/support/.*.log"))); + String prefix = "nodes/slave/" + agent.getNodeName() + "/remote"; + assertTrue(output.keySet().stream().anyMatch(key -> key.matches(prefix + "/support/.*.log"))); + }); } /* * Test adding agent remote directory content with excludes pattern(s). */ @Test - public void addContentsWithExcludes() throws Exception { - DumbSlave agent = j.createSlave("agent1", "test", null); - agent.getComputer().connect(false).get(); - j.waitOnline(agent); - - Map output = SupportTestUtils.invokeComponentToMap( - new NodeRemoteDirectoryComponent("", "**/*.log", true, 10), agent.toComputer()); - - String prefix = "nodes/slave/" + agent.getNodeName() + "/remote"; - assertFalse(output.keySet().stream().anyMatch(key -> key.matches(prefix + ".*/.*.log"))); + public void addContentsWithExcludes() throws Throwable { + r.then(j -> { + DumbSlave agent = j.createSlave("agent1", "test", null); + agent.getComputer().connect(false).get(); + j.waitOnline(agent); + + Map output = SupportTestUtils.invokeComponentToMap( + new NodeRemoteDirectoryComponent("", "**/*.log", true, 10), agent.toComputer()); + + String prefix = "nodes/slave/" + agent.getNodeName() + "/remote"; + assertFalse(output.keySet().stream().anyMatch(key -> key.matches(prefix + ".*/.*.log"))); + }); } /* * Test adding agent remote directory content with includes pattern(s). */ @Test - public void addContentsWithIncludes() throws Exception { - DumbSlave agent = j.createSlave("agent1", "test", null); - agent.getComputer().connect(false).get(); - j.waitOnline(agent); - - Map output = SupportTestUtils.invokeComponentToMap( - new NodeRemoteDirectoryComponent("support/*.log", "", true, 10), agent.toComputer()); - - String prefix = "nodes/slave/" + agent.getNodeName() + "/remote"; - assertTrue(output.keySet().stream().anyMatch(key -> key.matches(prefix + "/support/.*.log"))); + public void addContentsWithIncludes() throws Throwable { + r.then(j -> { + DumbSlave agent = j.createSlave("agent1", "test", null); + agent.getComputer().connect(false).get(); + j.waitOnline(agent); + + Map output = SupportTestUtils.invokeComponentToMap( + new NodeRemoteDirectoryComponent("support/*.log", "", true, 10), agent.toComputer()); + + String prefix = "nodes/slave/" + agent.getNodeName() + "/remote"; + assertTrue(output.keySet().stream().anyMatch(key -> key.matches(prefix + "/support/.*.log"))); + }); } /* * Test adding agent remote directory content with includes pattern(s). */ @Test - public void addContentsWithMaxDepth() throws Exception { - DumbSlave agent = j.createSlave("agent1", "test", null); - agent.getComputer().connect(false).get(); - j.waitOnline(agent); - - Map output = SupportTestUtils.invokeComponentToMap( - new NodeRemoteDirectoryComponent("", "", true, 1), agent.toComputer()); - - String prefix = "nodes/slave/" + agent.getNodeName() + "/remote"; - assertFalse(output.keySet().stream().anyMatch(key -> key.matches(prefix + "/support/.*.log"))); + public void addContentsWithMaxDepth() throws Throwable { + r.then(j -> { + DumbSlave agent = j.createSlave("agent1", "test", null); + agent.getComputer().connect(false).get(); + j.waitOnline(agent); + + Map output = SupportTestUtils.invokeComponentToMap( + new NodeRemoteDirectoryComponent("", "", true, 1), agent.toComputer()); + + String prefix = "nodes/slave/" + agent.getNodeName() + "/remote"; + assertFalse(output.keySet().stream().anyMatch(key -> key.matches(prefix + "/support/.*.log"))); + }); } } From e04d1dde8f0903dce252b45015e6c85fbfbc0904 Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Wed, 19 Feb 2025 10:45:31 -0500 Subject: [PATCH 12/17] Observed NCDFE on `StreamUtils` --- .../java/com/cloudbees/jenkins/support/SupportLogHandler.java | 1 + 1 file changed, 1 insertion(+) diff --git a/src/main/java/com/cloudbees/jenkins/support/SupportLogHandler.java b/src/main/java/com/cloudbees/jenkins/support/SupportLogHandler.java index 1efa01848..1e5a6a898 100644 --- a/src/main/java/com/cloudbees/jenkins/support/SupportLogHandler.java +++ b/src/main/java/com/cloudbees/jenkins/support/SupportLogHandler.java @@ -236,6 +236,7 @@ private void setFile(File file) throws FileNotFoundException { parentFile.mkdirs(); } + StreamUtils.closeQuietly(null); // ensure class is loaded so close() can succeed boolean success = false; FileOutputStream fos = null; BufferedOutputStream bos = null; From 122ee279becad9b8a52db1964548ba93d25ee520 Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Wed, 19 Feb 2025 11:12:49 -0500 Subject: [PATCH 13/17] SpotBugs getting in the way --- src/main/java/com/cloudbees/jenkins/support/SafeLog.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/main/java/com/cloudbees/jenkins/support/SafeLog.java b/src/main/java/com/cloudbees/jenkins/support/SafeLog.java index 7de474287..e7b0e9719 100644 --- a/src/main/java/com/cloudbees/jenkins/support/SafeLog.java +++ b/src/main/java/com/cloudbees/jenkins/support/SafeLog.java @@ -24,6 +24,7 @@ package com.cloudbees.jenkins.support; +import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; import java.io.BufferedWriter; import java.io.IOException; import java.nio.file.Files; @@ -32,6 +33,7 @@ import java.time.Instant; import jenkins.util.JenkinsJVM; +@SuppressFBWarnings(value = "NP_NULL_ON_SOME_PATH_FROM_RETURN_VALUE", justification = "shut the front door") public class SafeLog { public static final Path file; From e23e0bae8e5f6a0ad15789205ad975b5076c5f1b Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Wed, 19 Feb 2025 12:24:12 -0500 Subject: [PATCH 14/17] Also logging in `NodeExecutorsTest` :fingerscrossed: --- .../java/com/cloudbees/jenkins/support/SafeLog.java | 11 +++++++++++ .../jenkins/support/impl/NodeExecutorsTest.java | 13 +++++++++++++ .../impl/NodeRemoteDirectoryComponentTest.java | 10 ++-------- 3 files changed, 26 insertions(+), 8 deletions(-) diff --git a/src/main/java/com/cloudbees/jenkins/support/SafeLog.java b/src/main/java/com/cloudbees/jenkins/support/SafeLog.java index e7b0e9719..fc63899e0 100644 --- a/src/main/java/com/cloudbees/jenkins/support/SafeLog.java +++ b/src/main/java/com/cloudbees/jenkins/support/SafeLog.java @@ -58,5 +58,16 @@ public static void print(String message) { } } + public static void clear() throws Exception { + try (var os = Files.newOutputStream(file, StandardOpenOption.CREATE, StandardOpenOption.TRUNCATE_EXISTING)) {} + System.err.println("Cleared " + file); + } + + public static void show() throws Exception { + System.err.println("---%<--- " + file); + Files.copy(file, System.err); + System.err.println("--->%--- "); + } + private SafeLog() {} } diff --git a/src/test/java/com/cloudbees/jenkins/support/impl/NodeExecutorsTest.java b/src/test/java/com/cloudbees/jenkins/support/impl/NodeExecutorsTest.java index 9cd8a0153..b5932fbe9 100644 --- a/src/test/java/com/cloudbees/jenkins/support/impl/NodeExecutorsTest.java +++ b/src/test/java/com/cloudbees/jenkins/support/impl/NodeExecutorsTest.java @@ -5,6 +5,7 @@ import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertTrue; +import com.cloudbees.jenkins.support.SafeLog; import com.cloudbees.jenkins.support.SupportPlugin; import com.cloudbees.jenkins.support.SupportTestUtils; import com.cloudbees.jenkins.support.filter.ContentFilter; @@ -21,6 +22,8 @@ import org.jenkinsci.plugins.workflow.job.WorkflowJob; import org.jenkinsci.plugins.workflow.job.WorkflowRun; import org.jenkinsci.plugins.workflow.test.steps.SemaphoreStep; +import org.junit.After; +import org.junit.Before; import org.junit.ClassRule; import org.junit.Rule; import org.junit.Test; @@ -38,6 +41,16 @@ public class NodeExecutorsTest { @ClassRule public static BuildWatcher bw = new BuildWatcher(); + @Before + public void clearLog() throws Throwable { + SafeLog.clear(); + } + + @After + public void showLog() throws Throwable { + SafeLog.show(); + } + @Test public void addContents() throws Exception { DumbSlave agent = j.createOnlineSlave(Label.parseExpression("test"), null); diff --git a/src/test/java/com/cloudbees/jenkins/support/impl/NodeRemoteDirectoryComponentTest.java b/src/test/java/com/cloudbees/jenkins/support/impl/NodeRemoteDirectoryComponentTest.java index 753be5526..ee9092f02 100644 --- a/src/test/java/com/cloudbees/jenkins/support/impl/NodeRemoteDirectoryComponentTest.java +++ b/src/test/java/com/cloudbees/jenkins/support/impl/NodeRemoteDirectoryComponentTest.java @@ -7,8 +7,6 @@ import com.cloudbees.jenkins.support.SupportTestUtils; import hudson.model.Label; import hudson.slaves.DumbSlave; -import java.nio.file.Files; -import java.nio.file.StandardOpenOption; import java.util.Map; import org.junit.After; import org.junit.Before; @@ -26,16 +24,12 @@ public class NodeRemoteDirectoryComponentTest { @Before public void clearLog() throws Throwable { - try (var os = - Files.newOutputStream(SafeLog.file, StandardOpenOption.CREATE, StandardOpenOption.TRUNCATE_EXISTING)) {} - System.err.println("Cleared " + SafeLog.file); + SafeLog.clear(); } @After public void showLog() throws Throwable { - System.err.println("---%<--- " + SafeLog.file); - Files.copy(SafeLog.file, System.err); - System.err.println("--->%--- "); + SafeLog.show(); } /* From 6e736cf9b39273a4a2847cddfed3e21eefc3ab43 Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Wed, 19 Feb 2025 12:55:53 -0500 Subject: [PATCH 15/17] Removing diagnostic code --- .../cloudbees/jenkins/support/SafeLog.java | 73 -------------- .../jenkins/support/SupportLogHandler.java | 3 - .../jenkins/support/SupportPlugin.java | 10 -- .../support/impl/NodeExecutorsTest.java | 13 --- .../NodeRemoteDirectoryComponentTest.java | 97 ++++++++----------- 5 files changed, 38 insertions(+), 158 deletions(-) delete mode 100644 src/main/java/com/cloudbees/jenkins/support/SafeLog.java diff --git a/src/main/java/com/cloudbees/jenkins/support/SafeLog.java b/src/main/java/com/cloudbees/jenkins/support/SafeLog.java deleted file mode 100644 index fc63899e0..000000000 --- a/src/main/java/com/cloudbees/jenkins/support/SafeLog.java +++ /dev/null @@ -1,73 +0,0 @@ -/* - * The MIT License - * - * Copyright 2025 CloudBees, Inc. - * - * Permission is hereby granted, free of charge, to any person obtaining a copy - * of this software and associated documentation files (the "Software"), to deal - * in the Software without restriction, including without limitation the rights - * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell - * copies of the Software, and to permit persons to whom the Software is - * furnished to do so, subject to the following conditions: - * - * The above copyright notice and this permission notice shall be included in - * all copies or substantial portions of the Software. - * - * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR - * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, - * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE - * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER - * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, - * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN - * THE SOFTWARE. - */ - -package com.cloudbees.jenkins.support; - -import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; -import java.io.BufferedWriter; -import java.io.IOException; -import java.nio.file.Files; -import java.nio.file.Path; -import java.nio.file.StandardOpenOption; -import java.time.Instant; -import jenkins.util.JenkinsJVM; - -@SuppressFBWarnings(value = "NP_NULL_ON_SOME_PATH_FROM_RETURN_VALUE", justification = "shut the front door") -public class SafeLog { - public static final Path file; - - static { - // Cannot use System.getProperty("java.io.tmpdir") since this has different values - // for test/controller JVM vs. agent (due to override in plugin POM). - try { - var tmp = Files.createTempFile(null, null); - Files.delete(tmp); - file = tmp.getParent().resolve("safe.log"); - } catch (IOException x) { - throw new AssertionError(x); - } - } - - public static void print(String message) { - // TODO FileChannel.open + .lock if necessary to synchronize access - try (BufferedWriter w = Files.newBufferedWriter(file, StandardOpenOption.CREATE, StandardOpenOption.APPEND)) { - w.write(Instant.now() + " [" + (JenkinsJVM.isJenkinsJVM() ? "J" : "A") + "] " + message + "\n"); - } catch (IOException x) { - x.printStackTrace(); - } - } - - public static void clear() throws Exception { - try (var os = Files.newOutputStream(file, StandardOpenOption.CREATE, StandardOpenOption.TRUNCATE_EXISTING)) {} - System.err.println("Cleared " + file); - } - - public static void show() throws Exception { - System.err.println("---%<--- " + file); - Files.copy(file, System.err); - System.err.println("--->%--- "); - } - - private SafeLog() {} -} diff --git a/src/main/java/com/cloudbees/jenkins/support/SupportLogHandler.java b/src/main/java/com/cloudbees/jenkins/support/SupportLogHandler.java index 1e5a6a898..fde60d5c4 100644 --- a/src/main/java/com/cloudbees/jenkins/support/SupportLogHandler.java +++ b/src/main/java/com/cloudbees/jenkins/support/SupportLogHandler.java @@ -177,12 +177,10 @@ public void flush() { @Override public void close() throws SecurityException { - SafeLog.print("close " + this.logDirectry + " " + this.logFilePrefix); outputLock.lock(); try { if (writer != null) { StreamUtils.closeQuietly(writer); - SafeLog.print("closed " + writer); writer = null; } } finally { @@ -246,7 +244,6 @@ private void setFile(File file) throws FileNotFoundException { bos = new BufferedOutputStream(fos); writer = new OutputStreamWriter(bos, StandardCharsets.UTF_8); setWriter(writer); - SafeLog.print("setWriter " + file); fileCount = 0; success = true; } finally { diff --git a/src/main/java/com/cloudbees/jenkins/support/SupportPlugin.java b/src/main/java/com/cloudbees/jenkins/support/SupportPlugin.java index dbac4750e..e67a3d98d 100644 --- a/src/main/java/com/cloudbees/jenkins/support/SupportPlugin.java +++ b/src/main/java/com/cloudbees/jenkins/support/SupportPlugin.java @@ -859,13 +859,11 @@ public LogInitializer(FilePath rootPath, Level level) { } public Void call() { - SafeLog.print("LogInitializer on " + rootPath); // avoid double installation of the handler. JNLP agents can reconnect to the controller multiple times // and each connection gets a different RemoteClassLoader, so we need to evict them by class name, // not by their identity. closeAll(); Runtime.getRuntime().addShutdownHook(new Thread(LogInitializer::closeAll, "close log handlers")); - SafeLog.print("initialized hook"); LogHolder.AGENT_LOG_HANDLER.setLevel(level); LogHolder.AGENT_LOG_HANDLER.setDirectory(new File(rootPath.getRemote(), SUPPORT_DIRECTORY_NAME), "all"); ROOT_LOGGER.addHandler(LogHolder.AGENT_LOG_HANDLER); @@ -873,7 +871,6 @@ public Void call() { } private static void closeAll() { - SafeLog.print("closeAll"); for (Handler h : ROOT_LOGGER.getHandlers()) { if (h.getClass() .getName() @@ -881,9 +878,7 @@ private static void closeAll() { ROOT_LOGGER.removeHandler(h); try { h.close(); - SafeLog.print("removed " + h); } catch (Throwable t) { - SafeLog.print("close failure: " + t); } } } @@ -927,12 +922,7 @@ public void onOnline(Computer c, TaskListener listener) throws IOException, Inte if (channel != null) { final FilePath rootPath = node.getRootPath(); if (rootPath != null) { - SafeLog.print("will call on " + rootPath); CallAsyncWrapper.callAsync(channel, new LogInitializer(rootPath, getLogLevel())); - // Note that this may or may not run before the test completes; to force it to run: - // Thread.sleep(3000); - // alternately: - // channel.call(new LogInitializer(rootPath, getLogLevel())); } } } catch (IOException e) { diff --git a/src/test/java/com/cloudbees/jenkins/support/impl/NodeExecutorsTest.java b/src/test/java/com/cloudbees/jenkins/support/impl/NodeExecutorsTest.java index b5932fbe9..9cd8a0153 100644 --- a/src/test/java/com/cloudbees/jenkins/support/impl/NodeExecutorsTest.java +++ b/src/test/java/com/cloudbees/jenkins/support/impl/NodeExecutorsTest.java @@ -5,7 +5,6 @@ import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertTrue; -import com.cloudbees.jenkins.support.SafeLog; import com.cloudbees.jenkins.support.SupportPlugin; import com.cloudbees.jenkins.support.SupportTestUtils; import com.cloudbees.jenkins.support.filter.ContentFilter; @@ -22,8 +21,6 @@ import org.jenkinsci.plugins.workflow.job.WorkflowJob; import org.jenkinsci.plugins.workflow.job.WorkflowRun; import org.jenkinsci.plugins.workflow.test.steps.SemaphoreStep; -import org.junit.After; -import org.junit.Before; import org.junit.ClassRule; import org.junit.Rule; import org.junit.Test; @@ -41,16 +38,6 @@ public class NodeExecutorsTest { @ClassRule public static BuildWatcher bw = new BuildWatcher(); - @Before - public void clearLog() throws Throwable { - SafeLog.clear(); - } - - @After - public void showLog() throws Throwable { - SafeLog.show(); - } - @Test public void addContents() throws Exception { DumbSlave agent = j.createOnlineSlave(Label.parseExpression("test"), null); diff --git a/src/test/java/com/cloudbees/jenkins/support/impl/NodeRemoteDirectoryComponentTest.java b/src/test/java/com/cloudbees/jenkins/support/impl/NodeRemoteDirectoryComponentTest.java index ee9092f02..729eec67f 100644 --- a/src/test/java/com/cloudbees/jenkins/support/impl/NodeRemoteDirectoryComponentTest.java +++ b/src/test/java/com/cloudbees/jenkins/support/impl/NodeRemoteDirectoryComponentTest.java @@ -3,16 +3,13 @@ import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; -import com.cloudbees.jenkins.support.SafeLog; import com.cloudbees.jenkins.support.SupportTestUtils; import hudson.model.Label; import hudson.slaves.DumbSlave; import java.util.Map; -import org.junit.After; -import org.junit.Before; import org.junit.Rule; import org.junit.Test; -import org.jvnet.hudson.test.JenkinsSessionRule; +import org.jvnet.hudson.test.JenkinsRule; /** * Tests for the {@link NodeRemoteDirectoryComponent} @@ -20,85 +17,67 @@ public class NodeRemoteDirectoryComponentTest { @Rule - public JenkinsSessionRule r = new JenkinsSessionRule(); - - @Before - public void clearLog() throws Throwable { - SafeLog.clear(); - } - - @After - public void showLog() throws Throwable { - SafeLog.show(); - } + public JenkinsRule j = new JenkinsRule(); /* * Test adding agent remote directory content with the defaults. */ @Test - public void addContents() throws Throwable { - r.then(j -> { - DumbSlave agent = j.createOnlineSlave(Label.parseExpression("test"), null); + public void addContents() throws Exception { + DumbSlave agent = j.createOnlineSlave(Label.parseExpression("test"), null); - Map output = - SupportTestUtils.invokeComponentToMap(new NodeRemoteDirectoryComponent(), agent.toComputer()); + Map output = + SupportTestUtils.invokeComponentToMap(new NodeRemoteDirectoryComponent(), agent.toComputer()); - String prefix = "nodes/slave/" + agent.getNodeName() + "/remote"; - assertTrue(output.keySet().stream().anyMatch(key -> key.matches(prefix + "/support/.*.log"))); - }); + String prefix = "nodes/slave/" + agent.getNodeName() + "/remote"; + assertTrue(output.keySet().stream().anyMatch(key -> key.matches(prefix + "/support/.*.log"))); } /* * Test adding agent remote directory content with excludes pattern(s). */ @Test - public void addContentsWithExcludes() throws Throwable { - r.then(j -> { - DumbSlave agent = j.createSlave("agent1", "test", null); - agent.getComputer().connect(false).get(); - j.waitOnline(agent); - - Map output = SupportTestUtils.invokeComponentToMap( - new NodeRemoteDirectoryComponent("", "**/*.log", true, 10), agent.toComputer()); - - String prefix = "nodes/slave/" + agent.getNodeName() + "/remote"; - assertFalse(output.keySet().stream().anyMatch(key -> key.matches(prefix + ".*/.*.log"))); - }); + public void addContentsWithExcludes() throws Exception { + DumbSlave agent = j.createSlave("agent1", "test", null); + agent.getComputer().connect(false).get(); + j.waitOnline(agent); + + Map output = SupportTestUtils.invokeComponentToMap( + new NodeRemoteDirectoryComponent("", "**/*.log", true, 10), agent.toComputer()); + + String prefix = "nodes/slave/" + agent.getNodeName() + "/remote"; + assertFalse(output.keySet().stream().anyMatch(key -> key.matches(prefix + ".*/.*.log"))); } /* * Test adding agent remote directory content with includes pattern(s). */ @Test - public void addContentsWithIncludes() throws Throwable { - r.then(j -> { - DumbSlave agent = j.createSlave("agent1", "test", null); - agent.getComputer().connect(false).get(); - j.waitOnline(agent); - - Map output = SupportTestUtils.invokeComponentToMap( - new NodeRemoteDirectoryComponent("support/*.log", "", true, 10), agent.toComputer()); - - String prefix = "nodes/slave/" + agent.getNodeName() + "/remote"; - assertTrue(output.keySet().stream().anyMatch(key -> key.matches(prefix + "/support/.*.log"))); - }); + public void addContentsWithIncludes() throws Exception { + DumbSlave agent = j.createSlave("agent1", "test", null); + agent.getComputer().connect(false).get(); + j.waitOnline(agent); + + Map output = SupportTestUtils.invokeComponentToMap( + new NodeRemoteDirectoryComponent("support/*.log", "", true, 10), agent.toComputer()); + + String prefix = "nodes/slave/" + agent.getNodeName() + "/remote"; + assertTrue(output.keySet().stream().anyMatch(key -> key.matches(prefix + "/support/.*.log"))); } /* * Test adding agent remote directory content with includes pattern(s). */ @Test - public void addContentsWithMaxDepth() throws Throwable { - r.then(j -> { - DumbSlave agent = j.createSlave("agent1", "test", null); - agent.getComputer().connect(false).get(); - j.waitOnline(agent); - - Map output = SupportTestUtils.invokeComponentToMap( - new NodeRemoteDirectoryComponent("", "", true, 1), agent.toComputer()); - - String prefix = "nodes/slave/" + agent.getNodeName() + "/remote"; - assertFalse(output.keySet().stream().anyMatch(key -> key.matches(prefix + "/support/.*.log"))); - }); + public void addContentsWithMaxDepth() throws Exception { + DumbSlave agent = j.createSlave("agent1", "test", null); + agent.getComputer().connect(false).get(); + j.waitOnline(agent); + + Map output = SupportTestUtils.invokeComponentToMap( + new NodeRemoteDirectoryComponent("", "", true, 1), agent.toComputer()); + + String prefix = "nodes/slave/" + agent.getNodeName() + "/remote"; + assertFalse(output.keySet().stream().anyMatch(key -> key.matches(prefix + "/support/.*.log"))); } } From f9aa9e6b2bfffe1761e2b4ee452dc99c01594af5 Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Wed, 19 Feb 2025 14:25:31 -0500 Subject: [PATCH 16/17] https://github.com/jenkinsci/jenkins-test-harness/pull/922 released --- pom.xml | 4 ++-- .../java/com/cloudbees/jenkins/support/SupportPlugin.java | 1 + 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/pom.xml b/pom.xml index f9eb41581..605dba7c6 100644 --- a/pom.xml +++ b/pom.xml @@ -70,8 +70,8 @@ 2.479 ${jenkins.baseline}.1 - - 2400.v86a_2b_c9d99d6 + + 2403.v256947ecb_c8a_ jenkinsci/${project.artifactId}-plugin false diff --git a/src/main/java/com/cloudbees/jenkins/support/SupportPlugin.java b/src/main/java/com/cloudbees/jenkins/support/SupportPlugin.java index e67a3d98d..bf8256f1b 100644 --- a/src/main/java/com/cloudbees/jenkins/support/SupportPlugin.java +++ b/src/main/java/com/cloudbees/jenkins/support/SupportPlugin.java @@ -879,6 +879,7 @@ private static void closeAll() { try { h.close(); } catch (Throwable t) { + // ignore } } } From d576db684a3d40bd106deb21f58922f35eb3532a Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Wed, 19 Feb 2025 15:13:01 -0500 Subject: [PATCH 17/17] `SmartLogCleanerTest` flake; `Jenkins.removeNode` does not force immediate disconnection --- .../com/cloudbees/jenkins/support/impl/SmartLogCleanerTest.java | 1 + 1 file changed, 1 insertion(+) diff --git a/src/test/java/com/cloudbees/jenkins/support/impl/SmartLogCleanerTest.java b/src/test/java/com/cloudbees/jenkins/support/impl/SmartLogCleanerTest.java index e330cc6cc..8f6afc190 100644 --- a/src/test/java/com/cloudbees/jenkins/support/impl/SmartLogCleanerTest.java +++ b/src/test/java/com/cloudbees/jenkins/support/impl/SmartLogCleanerTest.java @@ -49,6 +49,7 @@ public void cleanUp() throws Exception { } assertEquals(cacheDir.list().length, 2); + agent2.toComputer().disconnect(null).get(); j.getInstance().removeNode(agent2); generateBundle();