diff --git a/src/main/java/testingbot/TestingBotBuildWrapper.java b/src/main/java/testingbot/TestingBotBuildWrapper.java index e23a3fb..d319241 100644 --- a/src/main/java/testingbot/TestingBotBuildWrapper.java +++ b/src/main/java/testingbot/TestingBotBuildWrapper.java @@ -22,9 +22,13 @@ import hudson.model.BuildableItemWithBuildWrappers; import hudson.model.Job; import hudson.model.Item; +import hudson.model.Computer; +import hudson.model.Node; import hudson.model.listeners.ItemListener; +import hudson.remoting.VirtualChannel; import hudson.security.ACL; import hudson.util.DescribableList; +import testingbot.pipeline.TestingBotTunnelStep; import java.util.Map; import hudson.util.ListBoxModel; import java.util.ArrayList; @@ -92,24 +96,45 @@ public Environment setUp(AbstractBuild build, Launcher launcher, BuildListener l if (this.useTunnel) { if (credentials == null) { listener.getLogger().println("No TestingBot key/secret found while trying to start a TestingBot Tunnel"); - return new TestingBotBuildEnvironment(null, null, null); + return new TestingBotBuildEnvironment(null, buildId, null, null); } + // Run the tunnel on the build's node (not the controller) so that a test connecting to + // localhost reaches the tunnel. The identifier keeps parallel builds on one node isolated. + String tunnelIdentifier = TunnelManager.generateTunnelIdentifier(build.getParent().getFullName(), build.getNumber()); + String key = credentials.getKey(); + String secret = credentials.getDecryptedSecret(); + VirtualChannel channel = launcher.getChannel(); + + if (channel != null) { + try { + TunnelManager.startOnChannel(channel, key, secret, "", tunnelIdentifier, listener); + } catch (InterruptedException ie) { + TunnelManager.stopOnChannel(channel, tunnelIdentifier, listener); + throw ie; + } catch (Exception ex) { + // Do not continue the build with a broken tunnel: clean up and fail. + TunnelManager.stopOnChannel(channel, tunnelIdentifier, listener); + throw new IOException("Failed to start TestingBot tunnel", ex); + } + return new TestingBotBuildEnvironment(credentials, buildId, tunnelIdentifier, null); + } + + // No agent channel available (unusual launcher); fall back to a controller-local tunnel. final App app = new App(); try { - TunnelManager.start(app, credentials.getKey(), credentials.getDecryptedSecret(), "", null, listener); + TunnelManager.start(app, key, secret, "", tunnelIdentifier, listener); } catch (InterruptedException ie) { TunnelManager.stop(app, listener); throw ie; } catch (Exception ex) { - // Do not continue the build with a broken tunnel: clean up and fail. TunnelManager.stop(app, listener); throw new IOException("Failed to start TestingBot tunnel", ex); } - return new TestingBotBuildEnvironment(credentials, app, buildId); + return new TestingBotBuildEnvironment(credentials, buildId, tunnelIdentifier, app); } - return new TestingBotBuildEnvironment(credentials, null, buildId); + return new TestingBotBuildEnvironment(credentials, buildId, null, null); } /** @@ -151,14 +176,19 @@ public ListBoxModel doFillCredentialsIdItems(@AncestorInPath final Item context) private class TestingBotBuildEnvironment extends BuildWrapper.Environment { - private final App app; private final TestingBotCredentials credentials; private final String buildId; + /** Non-null when a tunnel was started for this build. */ + private final String tunnelIdentifier; + /** Controller-local tunnel handle, used only in the no-channel fallback (otherwise null → runs on the agent). */ + private final App localApp; - public TestingBotBuildEnvironment(TestingBotCredentials credentials, @Nullable App app, @Nullable String buildId) { + public TestingBotBuildEnvironment(TestingBotCredentials credentials, @Nullable String buildId, + @Nullable String tunnelIdentifier, @Nullable App localApp) { this.credentials = credentials; - this.app = app; this.buildId = buildId; + this.tunnelIdentifier = tunnelIdentifier; + this.localApp = localApp; } @Override @@ -171,8 +201,9 @@ public void buildEnvVars(Map env) { env.put(TestingBotBuildReportAction.TESTINGBOT_BUILD, buildId); } - if (app != null) { - env.put(TESTINGBOT_TUNNEL,"true"); + if (tunnelIdentifier != null) { + env.put(TESTINGBOT_TUNNEL, "true"); + env.put(TestingBotTunnelStep.TESTINGBOT_TUNNEL_IDENTIFIER, tunnelIdentifier); } super.buildEnvVars(env); @@ -180,10 +211,27 @@ public void buildEnvVars(Map env) { @Override public boolean tearDown(AbstractBuild build, BuildListener listener) throws IOException, InterruptedException { - TunnelManager.stop(app, listener); + if (tunnelIdentifier == null) { + return true; + } + if (localApp != null) { + // Controller-local fallback tunnel. + TunnelManager.stop(localApp, listener); + } else { + // Resolve the agent's live channel now rather than holding a reference captured in + // setUp, which could be stale if the agent reconnected during the build. + TunnelManager.stopOnChannel(currentChannel(build), tunnelIdentifier, listener); + } return true; } } + + /** The live channel of the node the build ran on, or null if it is offline/unavailable. */ + private static VirtualChannel currentChannel(AbstractBuild build) { + Node node = build.getBuiltOn(); + Computer computer = node == null ? null : node.toComputer(); + return computer == null ? null : computer.getChannel(); + } protected boolean migrateCredentials(AbstractProject project) { Logger.getLogger(TestingBotBuildWrapper.class.getName()).log(Level.INFO, "TestingBot Plugin: migrateCredentials: " + this.credentialsId); diff --git a/src/main/java/testingbot/TunnelManager.java b/src/main/java/testingbot/TunnelManager.java index c3bc2e0..169b7f4 100644 --- a/src/main/java/testingbot/TunnelManager.java +++ b/src/main/java/testingbot/TunnelManager.java @@ -4,12 +4,16 @@ import com.testingbot.tunnel.Api; import com.testingbot.tunnel.App; import hudson.model.TaskListener; +import hudson.remoting.VirtualChannel; import java.io.IOException; import java.util.ArrayList; import java.util.List; import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.atomic.AtomicInteger; import java.util.regex.Matcher; import java.util.regex.Pattern; +import jenkins.security.MasterToSlaveCallable; /** * Shared TestingBot Tunnel lifecycle helper used by both the freestyle @@ -203,4 +207,138 @@ public static void stop(App app, TaskListener listener) { app.stop(); } } + + // ------------------------------------------------------------------------------------------ + // Remote (agent) tunnel lifecycle. + // + // The tunnel must boot on the same node where the tests run, so that a test connecting to + // {@code localhost} reaches the tunnel. Both the freestyle build wrapper and the pipeline step + // therefore run the tunnel inside a MasterToSlaveCallable on the build node's channel. The live + // App is kept in a per-JVM static registry (co-located with the tunnel on the agent) so teardown + // can find and stop it again without serializing the handle back across the channel. + // ------------------------------------------------------------------------------------------ + + /** Tunnels running in this JVM, keyed by tunnel identifier. Lives in the agent's static space. */ + private static final ConcurrentHashMap RUNNING_TUNNELS = new ConcurrentHashMap<>(); + + /** Monotonic suffix so multiple generated identifiers within one run stay distinct. */ + private static final AtomicInteger TUNNEL_SEQ = new AtomicInteger(); + + /** + * Generates a tunnel identifier unique to a build (and to each tunnel within it), so that + * parallel tunnels on the same node stay isolated. + */ + public static String generateTunnelIdentifier(String jobFullName, int buildNumber) { + return "jenkins-" + jobFullName.replace('/', '-') + "-" + buildNumber + "-" + TUNNEL_SEQ.incrementAndGet(); + } + + /** + * Boots a tunnel on the given channel's node and registers it under {@code tunnelIdentifier}. + * Credentials are passed as already-decrypted strings (only plaintext crosses the encrypted + * remoting link, never the credential object). + */ + public static void startOnChannel(VirtualChannel channel, String key, String secret, String options, + String tunnelIdentifier, TaskListener listener) throws Exception { + channel.call(new StartTunnelHandler(key, secret, options, tunnelIdentifier, listener)); + } + + /** + * Stops the tunnel registered under {@code tunnelIdentifier} on the given channel's node. + * Best-effort: a {@code null} channel (offline agent) or a remoting failure is tolerated so a + * teardown never fails the build; an offline agent is reported to the log. + */ + public static void stopOnChannel(VirtualChannel channel, String tunnelIdentifier, TaskListener listener) { + if (channel == null) { + RUNNING_TUNNELS.remove(tunnelIdentifier); + if (listener != null) { + listener.getLogger().println("[TestingBot] Agent offline: remote teardown of tunnel " + + tunnelIdentifier + " could not be performed. It may still be running on the agent " + + "and should stop when the agent process exits (best-effort cleanup)."); + } + return; + } + try { + channel.call(new StopTunnelHandler(tunnelIdentifier, listener)); + } catch (InterruptedException ie) { + // Restore the interrupt status; teardown stays best-effort and does not rethrow. + Thread.currentThread().interrupt(); + if (listener != null) { + listener.getLogger().println("[TestingBot] Interrupted while stopping tunnel " + tunnelIdentifier); + } + } catch (Exception e) { + // Best-effort, but do not silently discard the failure — record why teardown could not complete. + if (listener != null) { + listener.getLogger().println("[TestingBot] Failed to stop tunnel " + tunnelIdentifier + ": " + e); + } + } + } + + private static final class StartTunnelHandler extends MasterToSlaveCallable { + + private static final long serialVersionUID = 1L; + private final String key; + private final String secret; + private final String tunnelOptions; + private final String tunnelIdentifier; + private final TaskListener listener; + + StartTunnelHandler(String key, String secret, String tunnelOptions, String tunnelIdentifier, TaskListener listener) { + this.key = key; + this.secret = secret; + this.tunnelOptions = tunnelOptions; + this.tunnelIdentifier = tunnelIdentifier; + this.listener = listener; + } + + @Override + public Void call() throws Exception { + App app = new App(); + // Reserve the identifier before booting: a duplicate identifier is rejected rather than + // silently overwriting (and leaking) an existing tunnel, and teardown can find and stop + // the App even while it is still booting. + App previous = RUNNING_TUNNELS.putIfAbsent(tunnelIdentifier, app); + if (previous != null) { + try { + app.stop(); + } catch (Exception ignored) { + // best effort + } + throw new IllegalStateException( + "A TestingBot tunnel with identifier '" + tunnelIdentifier + "' is already running"); + } + try { + start(app, key, secret, tunnelOptions, tunnelIdentifier, listener); + } catch (Exception e) { + // Startup failed: remove only our own registry entry (leaving any concurrent owner + // untouched) and stop the App so we don't leak a half-booted tunnel. + RUNNING_TUNNELS.remove(tunnelIdentifier, app); + try { + app.stop(); + } catch (Exception ignored) { + // best effort + } + throw e; + } + return null; + } + } + + private static final class StopTunnelHandler extends MasterToSlaveCallable { + + private static final long serialVersionUID = 1L; + private final String tunnelIdentifier; + private final TaskListener listener; + + StopTunnelHandler(String tunnelIdentifier, TaskListener listener) { + this.tunnelIdentifier = tunnelIdentifier; + this.listener = listener; + } + + @Override + public Void call() { + App app = RUNNING_TUNNELS.remove(tunnelIdentifier); + stop(app, listener); + return null; + } + } } diff --git a/src/main/java/testingbot/pipeline/TestingBotTunnelStep.java b/src/main/java/testingbot/pipeline/TestingBotTunnelStep.java index ce98eb4..b5b7c19 100644 --- a/src/main/java/testingbot/pipeline/TestingBotTunnelStep.java +++ b/src/main/java/testingbot/pipeline/TestingBotTunnelStep.java @@ -20,10 +20,7 @@ import java.util.ArrayList; import java.util.HashMap; import java.util.Set; -import java.util.concurrent.ConcurrentHashMap; -import java.util.concurrent.atomic.AtomicInteger; import jenkins.model.Jenkins; -import jenkins.security.MasterToSlaveCallable; import org.jenkinsci.plugins.workflow.steps.BodyExecution; import org.jenkinsci.plugins.workflow.steps.BodyExecutionCallback; import org.jenkinsci.plugins.workflow.steps.BodyInvoker; @@ -46,16 +43,6 @@ public class TestingBotTunnelStep extends Step { /** Env var exposing the (possibly auto-generated) tunnel identifier to the build. */ public static final String TESTINGBOT_TUNNEL_IDENTIFIER = "TESTINGBOT_TUNNEL_IDENTIFIER"; - /** - * Tunnels currently running in this JVM, keyed by tunnel identifier. Replaces the - * previous single static {@code App}, which caused concurrent executions on the same - * node to corrupt and tear down each other's tunnel. - */ - private static final ConcurrentHashMap RUNNING_TUNNELS = new ConcurrentHashMap<>(); - - /** Monotonic suffix so multiple tunnels within one run get distinct generated identifiers. */ - private static final AtomicInteger TUNNEL_SEQ = new AtomicInteger(); - private String options; private String credentialsId; @@ -125,59 +112,6 @@ public ListBoxModel doFillCredentialsIdItems(final @AncestorInPath Item context) } } - private static final class TbStartTunnelHandler extends MasterToSlaveCallable { - - private final String key; - private final String secret; - private final String tunnelOptions; - private final String tunnelIdentifier; - private final TaskListener listener; - - TbStartTunnelHandler(String key, String secret, String tunnelOptions, String tunnelIdentifier, TaskListener listener) { - this.key = key; - this.secret = secret; - this.tunnelOptions = tunnelOptions; - this.tunnelIdentifier = tunnelIdentifier; - this.listener = listener; - } - - @Override - public Void call() throws Exception { - App app = new App(); - try { - TunnelManager.start(app, key, secret, tunnelOptions, tunnelIdentifier, listener); - } catch (Exception e) { - // Startup failed after the App was created; stop it so we don't leak a half-booted tunnel. - try { - app.stop(); - } catch (Exception ignored) { - // best effort - } - throw e; - } - RUNNING_TUNNELS.put(tunnelIdentifier, app); - return null; - } - } - - private static final class TbStopTunnelHandler extends MasterToSlaveCallable { - - private final String tunnelIdentifier; - private final TaskListener listener; - - TbStopTunnelHandler(String tunnelIdentifier, TaskListener listener) { - this.tunnelIdentifier = tunnelIdentifier; - this.listener = listener; - } - - @Override - public Void call() { - App app = RUNNING_TUNNELS.remove(tunnelIdentifier); - TunnelManager.stop(app, listener); - return null; - } - } - public static class TestingBotTunnelStepExecution extends StepExecution { private static final long serialVersionUID = 1L; @@ -228,7 +162,7 @@ public boolean start() throws Exception { String userIdentifier = TunnelManager.extractTunnelIdentifier(options); tunnelIdentifier = (userIdentifier != null && !userIdentifier.isEmpty()) ? userIdentifier - : "jenkins-" + run.getParent().getFullName().replace('/', '-') + "-" + run.getNumber() + "-" + TUNNEL_SEQ.incrementAndGet(); + : TunnelManager.generateTunnelIdentifier(run.getParent().getFullName(), run.getNumber()); // Resolve secrets on the controller; only plaintext crosses the channel (over the encrypted remoting link). String key = tbCredentials.getKey(); @@ -249,7 +183,7 @@ public boolean start() throws Exception { env.put("SELENIUM_HOST", hubHost); env.put("SELENIUM_PORT", hubPort); - requireChannel(computer).call(new TbStartTunnelHandler(key, secret, options, tunnelIdentifier, listener)); + TunnelManager.startOnChannel(requireChannel(computer), key, secret, options, tunnelIdentifier, listener); try { body = getContext().newBodyInvoker() @@ -278,21 +212,7 @@ private static VirtualChannel requireChannel(Computer computer) throws Exception /** Best-effort tunnel teardown that tolerates an offline agent (no channel). */ private static void stopTunnelQuietly(Computer computer, String tunnelIdentifier, TaskListener listener) { - VirtualChannel channel = computer == null ? null : computer.getChannel(); - if (channel == null) { - RUNNING_TUNNELS.remove(tunnelIdentifier); - if (listener != null) { - listener.getLogger().println("[TestingBot] Agent offline: remote teardown of tunnel " - + tunnelIdentifier + " could not be performed. It may still be running on the agent " - + "and should stop when the agent process exits (best-effort cleanup)."); - } - return; - } - try { - channel.call(new TbStopTunnelHandler(tunnelIdentifier, listener)); - } catch (Exception ignored) { - // best effort - } + TunnelManager.stopOnChannel(computer == null ? null : computer.getChannel(), tunnelIdentifier, listener); } @Override