From 8114138f29f99cb8232a284e5423ef27d3ff10f5 Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Thu, 17 Aug 2023 19:33:38 -0400 Subject: [PATCH 1/5] More efficient `lastLocation` handling in watch mode --- .../durabletask/FileMonitoringTask.java | 20 ++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/src/main/java/org/jenkinsci/plugins/durabletask/FileMonitoringTask.java b/src/main/java/org/jenkinsci/plugins/durabletask/FileMonitoringTask.java index 218842fe..b5a85d4d 100644 --- a/src/main/java/org/jenkinsci/plugins/durabletask/FileMonitoringTask.java +++ b/src/main/java/org/jenkinsci/plugins/durabletask/FileMonitoringTask.java @@ -602,6 +602,7 @@ private static class Watcher implements Runnable { private final Handler handler; private final TaskListener listener; private final @CheckForNull Charset cs; + private long lastLocation = -1; Watcher(FileMonitoringController controller, FilePath workspace, Handler handler, TaskListener listener) { LOGGER.log(Level.FINE, "starting " + this, new Throwable()); @@ -616,10 +617,17 @@ private static class Watcher implements Runnable { @Override public void run() { try { Integer exitStatus = controller.exitStatus(workspace, listener); // check before collecting output, in case the process is just now finishing - long lastLocation = 0; - FilePath lastLocationFile = controller.getLastLocationFile(workspace); - if (lastLocationFile.exists()) { - lastLocation = Long.parseLong(lastLocationFile.readToString()); + if (lastLocation == -1) { + FilePath lastLocationFile = controller.getLastLocationFile(workspace); + if (lastLocationFile.exists()) { + lastLocation = Long.parseLong(lastLocationFile.readToString()); + LOGGER.finest(() -> "Loaded lastLocation=" + lastLocation); + } else { + lastLocation = 0; + LOGGER.finest("New watch, lastLocation=0"); + } + } else { + LOGGER.finest(() -> "Using cached lastLocation=" + lastLocation); } FilePath logFile = controller.getLogFile(workspace); long len = logFile.length(); @@ -630,9 +638,11 @@ private static class Watcher implements Runnable { InputStream utf8EncodedStream = cs == null ? locallyEncodedStream : new ReaderInputStream(new InputStreamReader(locallyEncodedStream, cs), StandardCharsets.UTF_8); handler.output(utf8EncodedStream); long newLocation = ch.position(); - lastLocationFile.write(Long.toString(newLocation), null); + // TODO use AtomicFileWriter or equivalent? + controller.getLastLocationFile(workspace).write(Long.toString(newLocation), null); long delta = newLocation - lastLocation; LOGGER.finer(() -> this + " copied " + delta + " bytes from " + logFile); + lastLocation = newLocation; } } if (exitStatus != null) { From e2eb5c4925db58a35d1a2872b1d2b0cd06d5898d Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Fri, 18 Aug 2023 13:14:34 -0400 Subject: [PATCH 2/5] Adding on-controller test mode to `BourneShellScriptTest`. Though `.watch` actually ran decent test coverage of push mode, JaCoCo did not show `Watcher` since all the tests ran on agents in another JVM. Adding a mode to run on-controller since people do sometimes do that, and it should show up in JaCoCo, and log messages from the watcher go to test output which is helpful. --- .../durabletask/BourneShellScriptTest.java | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/src/test/java/org/jenkinsci/plugins/durabletask/BourneShellScriptTest.java b/src/test/java/org/jenkinsci/plugins/durabletask/BourneShellScriptTest.java index fd2d99a8..fb18bf91 100644 --- a/src/test/java/org/jenkinsci/plugins/durabletask/BourneShellScriptTest.java +++ b/src/test/java/org/jenkinsci/plugins/durabletask/BourneShellScriptTest.java @@ -35,6 +35,7 @@ import hudson.Launcher; import hudson.Platform; import hudson.Proc; +import hudson.model.Node; import hudson.model.Slave; import hudson.plugins.sshslaves.SSHLauncher; import hudson.remoting.Channel; @@ -86,7 +87,7 @@ import org.jvnet.hudson.test.SimpleCommandLauncher; enum TestPlatform { - NATIVE, ALPINE, CENTOS, UBUNTU, NO_INIT, UBUNTU_NO_BINARY, SLIM + ON_CONTROLLER, NATIVE, ALPINE, CENTOS, UBUNTU, NO_INIT, UBUNTU_NO_BINARY, SLIM } @RunWith(Parameterized.class) @@ -113,11 +114,11 @@ static void assumeDocker() throws Exception { assumeThat("Docker must be at least 1.13.0 for this test (uses --init)", new VersionNumber(baos.toString().trim()), greaterThanOrEqualTo(new VersionNumber("1.13.0"))); } - @Rule public LoggerRule logging = new LoggerRule().recordPackage(BourneShellScript.class, Level.FINE); + @Rule public LoggerRule logging = new LoggerRule().recordPackage(BourneShellScript.class, Level.FINEST); private TestPlatform platform; private StreamTaskListener listener; - private Slave s; + private Node s; private FilePath ws; private Launcher launcher; @@ -128,6 +129,9 @@ public BourneShellScriptTest(TestPlatform platform) throws Exception { @Before public void prepareAgentForPlatform() throws Exception { switch (platform) { + case ON_CONTROLLER: + s = j.jenkins; + break; case NATIVE: BourneShellScript.USE_BINARY_WRAPPER = true; s = j.createOnlineSlave(); @@ -141,13 +145,15 @@ public BourneShellScriptTest(TestPlatform platform) throws Exception { case UBUNTU_NO_BINARY: assumeDocker(); s = prepareAgentDocker(); - j.jenkins.addNode(s); - j.waitOnline(s); + if (s instanceof Slave) { + j.jenkins.addNode(s); + j.waitOnline((Slave) s); + } break; default: throw new AssertionError(platform); } - ws = s.getWorkspaceRoot().child("ws"); + ws = (s instanceof Slave ? ((Slave) s).getWorkspaceRoot() : j.jenkins.getRootPath()).child("ws"); launcher = s.createLauncher(listener); } From b3cba3b1bd34d91c31281bbb768bb92e28bb1af7 Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Fri, 18 Aug 2023 14:28:28 -0400 Subject: [PATCH 3/5] Work around https://github.com/jenkinsci/plugin-pom/pull/560#issuecomment-1656232490 so it is possible to run e.g. `mvn test -Dtest="BourneShellScriptTest#binaryCaching[0: ON_CONTROLLER]"` --- pom.xml | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/pom.xml b/pom.xml index 272bd3e0..576620bf 100644 --- a/pom.xml +++ b/pom.xml @@ -73,4 +73,22 @@ + + + + + org.apache.maven.plugins + maven-surefire-plugin + + + + org.apache.maven.surefire + surefire-junit47 + ${maven-surefire-plugin.version} + + + + + + From 67e67f02050565aa425e23c0c861f6d765888e77 Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Fri, 18 Aug 2023 14:30:12 -0400 Subject: [PATCH 4/5] `binaryCaching` was failing `ON_CONTROLLER` due to a fragile calculation of `binaryPath` --- .../jenkinsci/plugins/durabletask/BourneShellScriptTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/java/org/jenkinsci/plugins/durabletask/BourneShellScriptTest.java b/src/test/java/org/jenkinsci/plugins/durabletask/BourneShellScriptTest.java index fb18bf91..17c7f901 100644 --- a/src/test/java/org/jenkinsci/plugins/durabletask/BourneShellScriptTest.java +++ b/src/test/java/org/jenkinsci/plugins/durabletask/BourneShellScriptTest.java @@ -513,7 +513,7 @@ static class MockHandler extends Handler { String version = j.getPluginManager().getPlugin("durable-task").getVersion(); version = StringUtils.substringBefore(version, "-"); String binaryName = "durable_task_monitor_" + version + "_" + os + "_" + architecture; - FilePath binaryPath = ws.getParent().getParent().child("caches/durable-task/" + binaryName); + FilePath binaryPath = s.getRootPath().child("caches/durable-task/" + binaryName); assertFalse(binaryPath.exists()); BourneShellScript script = new BourneShellScript("echo hello"); From 9445f80dd923af630c7cd648e6219e509a17d75e Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Fri, 18 Aug 2023 14:30:45 -0400 Subject: [PATCH 5/5] Reviewed `switch`es on `platform` and aligned `ON_CONTROLLER` with `NATIVE` --- .../jenkinsci/plugins/durabletask/BourneShellScriptTest.java | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/test/java/org/jenkinsci/plugins/durabletask/BourneShellScriptTest.java b/src/test/java/org/jenkinsci/plugins/durabletask/BourneShellScriptTest.java index 17c7f901..c1d7d306 100644 --- a/src/test/java/org/jenkinsci/plugins/durabletask/BourneShellScriptTest.java +++ b/src/test/java/org/jenkinsci/plugins/durabletask/BourneShellScriptTest.java @@ -130,6 +130,7 @@ public BourneShellScriptTest(TestPlatform platform) throws Exception { @Before public void prepareAgentForPlatform() throws Exception { switch (platform) { case ON_CONTROLLER: + BourneShellScript.USE_BINARY_WRAPPER = true; s = j.jenkins; break; case NATIVE: @@ -212,6 +213,7 @@ private Slave prepareDockerPlatforms() throws Exception { @Test public void smokeTest() throws Exception { int sleepSeconds; switch (platform) { + case ON_CONTROLLER: case NATIVE: sleepSeconds = 0; break; @@ -438,6 +440,7 @@ static class MockHandler extends Handler { @Test public void backgroundLaunch() throws IOException, InterruptedException { int sleepSeconds; switch (platform) { + case ON_CONTROLLER: case NATIVE: case CENTOS: case UBUNTU: @@ -483,6 +486,7 @@ static class MockHandler extends Handler { String os; String architecture; switch (platform) { + case ON_CONTROLLER: case NATIVE: if (Platform.isDarwin()) { os = "darwin"; @@ -608,6 +612,7 @@ private String psOut(String psFormat) throws InterruptedException, IOException { private String setPsFormat() { String cmdCol = null; switch (platform) { + case ON_CONTROLLER: case NATIVE: cmdCol = Platform.isDarwin() ? "comm" : "cmd"; break;