Overhaul of SlaveLaunchLogs

src/main/java/com/cloudbees/jenkins/support/api/LaunchLogsFileContent.java

@@ -1,6 +1,7 @@
 package com.cloudbees.jenkins.support.api;
 
 import com.cloudbees.jenkins.support.filter.PasswordRedactor;
+import com.cloudbees.jenkins.support.impl.SlaveLaunchLogs;
 import java.io.ByteArrayInputStream;
 import java.io.File;
 import java.io.FileInputStream;
@@ -11,6 +12,9 @@
 import java.util.stream.Collectors;
 import org.apache.commons.io.IOUtils;
 
+/**
+ * @see SlaveLaunchLogs
+ */
 public class LaunchLogsFileContent extends FileContent {
 
     public LaunchLogsFileContent(String name, String[] filterableParameters, File file, long maxSize) {

src/main/java/com/cloudbees/jenkins/support/impl/LogFilenameAgentFilter.java

@@ -32,6 +32,7 @@
 
 /**
  * Matches agent log files in an interval of time
+ * @see SlaveLogs
  */
 class LogFilenameAgentFilter implements FilenameFilter, Serializable {
 

src/main/java/com/cloudbees/jenkins/support/impl/NodeRemoteDirectoryComponent.java

@@ -45,6 +45,9 @@
             return;
         }
 
+        // TODO this is not necessarily the right root for workDir.
+        // Engine.workDir says that the workDir “should” be the agent root but this is not enforced.
+        // Safer to use Engine.current().internalDir.
         FilePath rootPath = node.getRootPath();
         if (rootPath == null) {
             LOGGER.log(Level.WARNING, "Node " + node.getDisplayName() + " seems to be offline");

src/main/java/com/cloudbees/jenkins/support/impl/SlaveLogs.java

@@ -210,6 +210,12 @@
             tasks.add(new java.util.concurrent.Callable<List<FileContent>>() {
                 public List<FileContent> call() throws Exception {
                     List<FileContent> result = new ArrayList<FileContent>();
+                    // TODO presumes that WinSW’s %BASE% would be the remoteFS as in
+                    // https://docs.cloudbees.com/docs/cloudbees-ci-kb/latest/client-and-managed-controllers/how-to-install-windows-agents-as-a-service
+                    // and that
+                    // https://github.com/winsw/winsw/blob/6cf303c1d3fbe1069d95af230b8efa117d29cdf2/src/WinSW.Core/Configuration/XmlServiceConfig.cs#L273
+                    // is not overridden from e.g.
+                    // https://github.com/winsw/winsw/blob/e4cf507bae5981363a9cdc0f7301c1aa892af401/samples/shared-directory-mapper.xml#L8
                     final Map<String, File> logFiles = logFetcher.forNode(node).getLogFiles(rootPath);
                     for (Map.Entry<String, File> entry : logFiles.entrySet()) {
                         result.add(new FileContent(

src/main/java/com/cloudbees/jenkins/support/impl/SmartLogCleaner.java

@@ -1,3 +1,4 @@
+// TODO delete?
 package com.cloudbees.jenkins.support.impl;
 
 import com.cloudbees.jenkins.support.SupportPlugin;

src/main/java/com/cloudbees/jenkins/support/impl/SmartLogFetcher.java

@@ -1,3 +1,4 @@
+// TODO delete? (9aeca363ce4812f5f6c084d673e8d8bcdb266498)
 package com.cloudbees.jenkins.support.impl;
 
 import com.cloudbees.jenkins.support.SupportPlugin;

src/main/java/com/cloudbees/jenkins/support/impl/WinswLogfileFilter.java

@@ -7,7 +7,8 @@
 /**
  * Matches log files from winsw.
  *
- * @see <a href="https://github.com/kohsuke/winsw/blob/master/LogAppenders.cs">LogAppenders.cs</a>
+ * @see <a href="https://github.com/winsw/winsw/blob/e4cf507bae5981363a9cdc0f7301c1aa892af401/src/WinSW.Core/LogAppenders.cs#L169-L170">LogAppenders.cs</a>
+ * @see SlaveLogs
  * @author Kohsuke Kawaguchi
  */
 class WinswLogfileFilter implements FilenameFilter, Serializable {

src/test/java/com/cloudbees/jenkins/support/impl/SlaveLaunchLogsTest.java

@@ -1,36 +1,76 @@
 package com.cloudbees.jenkins.support.impl;
 
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.allOf;
+import static org.hamcrest.Matchers.containsString;
 import static org.hamcrest.Matchers.hasKey;
+import static org.hamcrest.Matchers.notNullValue;
 
 import com.cloudbees.jenkins.support.SupportTestUtils;
 import hudson.ExtensionList;
-import hudson.slaves.DumbSlave;
-import java.util.Map;
-import org.hamcrest.MatcherAssert;
 import org.junit.Rule;
 import org.junit.Test;
+import org.jvnet.hudson.test.InboundAgentRule;
 import org.jvnet.hudson.test.JenkinsRule;
 
 public class SlaveLaunchLogsTest {
 
     @Rule
     public JenkinsRule j = new JenkinsRule();
 
+    @Rule
+    public InboundAgentRule inboundAgents = new InboundAgentRule();
+
+    @Test
+    public void onlineOutboundAgent() throws Exception {
+        var s = j.createOnlineSlave();
+        assertThat(
+                "reflects JenkinsRule.createComputerLauncher command & SlaveComputer.setChannel",
+                SupportTestUtils.invokeComponentToMap(ExtensionList.lookupSingleton(SlaveLaunchLogs.class))
+                        .get("nodes/slave/" + s.getNodeName() + "/launchLogs/slave.log"),
+                allOf(
+                        notNullValue(),
+                        containsString("-XX:+PrintCommandLineFlags"),
+                        containsString("Remoting version: ")));
+    }
+
     @Test
-    public void smokesRoot() throws Exception {
-        DumbSlave s = j.createSlave();
-        Map<String, String> output =
-                SupportTestUtils.invokeComponentToMap(ExtensionList.lookupSingleton(SlaveLaunchLogs.class));
-        String key = "nodes/slave/" + s.getNodeName() + "/launchLogs/slave.log";
-        MatcherAssert.assertThat(output, hasKey(key));
+    public void onlineInboundAgent() throws Exception {
+        inboundAgents.createAgent(j, "remote");
+        assertThat(
+                "reflects DefaultJnlpSlaveReceiver.beforeChannel & SlaveComputer.setChannel",
+                SupportTestUtils.invokeComponentToMap(ExtensionList.lookupSingleton(SlaveLaunchLogs.class))
+                        .get("nodes/slave/remote/launchLogs/slave.log"),
+                allOf(
+                        notNullValue(),
+                        containsString("Inbound agent connected from"),
+                        containsString("Communication Protocol: JNLP4-connect")));
     }
 
     @Test
-    public void smokesComputer() throws Exception {
-        DumbSlave s = j.createSlave();
-        Map<String, String> output = SupportTestUtils.invokeComponentToMap(
-                ExtensionList.lookupSingleton(SlaveLaunchLogs.class), s.toComputer());
-        String key = "nodes/slave/" + s.getNodeName() + "/launchLogs/slave.log";
-        MatcherAssert.assertThat(output, hasKey(key));
+    public void component() throws Exception {
+        var s = j.createSlave();
+        assertThat(
+                SupportTestUtils.invokeComponentToMap(
+                        ExtensionList.lookupSingleton(SlaveLaunchLogs.class), s.toComputer()),
+                hasKey("nodes/slave/" + s.getNodeName() + "/launchLogs/slave.log"));
     }
+
+    @Test
+    public void offlineAgent() throws Exception {
+        var s = j.createOnlineSlave();
+        var name = s.getNodeName();
+        s.toComputer().disconnect(null).get();
+        assertThat(
+                "still includes something",
+                SupportTestUtils.invokeComponentToMap(ExtensionList.lookupSingleton(SlaveLaunchLogs.class))
+                        .get("nodes/slave/" + name + "/launchLogs/slave.log"),
+                allOf(notNullValue(), containsString("Remoting version: ")));
+    }
+
+    // TODO logs from a deleted agent
+    // TODO logs from multiple launch attempts of same agent
+    // TODO honor SafeTimerTask.getLogsRoot (if applicable)
+    // TODO rotation of old or excessively long logs
+
 }

src/test/java/com/cloudbees/jenkins/support/impl/SlaveLogsTest.java

@@ -0,0 +1,45 @@
+/*
+ * The MIT License
+ *
+ * Copyright 2024 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.impl;
+
+import org.junit.Rule;
+import org.jvnet.hudson.test.JenkinsRule;
+
+public final class SlaveLogsTest {
+
+    @Rule
+    public JenkinsRule j = new JenkinsRule();
+
+    // TODO INFO messages from online agent after connection
+    // TODO messages from online agent prior to connection (if -workDir set)
+    // TODO messages from disconnected agent
+    // TODO messages from deleted agent
+    // TODO messages from reconnected agent
+    // TODO FINE messages
+    // TODO winsw logs
+    // TODO honor SafeTimerTask.getLogsRoot (if applicable)
+    // TODO rotation of old or excessively long logs
+
+}