diff --git a/Jenkinsfile b/Jenkinsfile index 9ceab421..ed1f8d49 100644 --- a/Jenkinsfile +++ b/Jenkinsfile @@ -1,4 +1,4 @@ -buildPlugin(useContainerAgent: true, configurations: [ +buildPlugin(useContainerAgent: true, forkCount: '1C', configurations: [ [platform: 'linux', jdk: 17], [platform: 'windows', jdk: 11], ]) diff --git a/pom.xml b/pom.xml index f8e46131..faea9298 100644 --- a/pom.xml +++ b/pom.xml @@ -67,6 +67,8 @@ 999999-SNAPSHOT 2.361.4 + + 2042.v787a_641a_9b_26 true jenkinsci/${project.artifactId}-plugin 2.40 @@ -76,10 +78,26 @@ io.jenkins.tools.bom bom-2.361.x - 1654.vcb_69d035fa_20 + 2081.v85885a_d2e5c5 import pom + + + org.jenkins-ci.plugins + durable-task + 513.vc48a_a_075a_d93 + + + org.jenkins-ci.plugins.workflow + workflow-api + 1248.v4b_91043341d2 + + + org.jenkins-ci.plugins + credentials-binding + 621.v58c0fb_d285a_c + diff --git a/src/main/java/org/jenkinsci/plugins/workflow/steps/durable_task/DurableTaskStep.java b/src/main/java/org/jenkinsci/plugins/workflow/steps/durable_task/DurableTaskStep.java index fea45e67..0057eaad 100644 --- a/src/main/java/org/jenkinsci/plugins/workflow/steps/durable_task/DurableTaskStep.java +++ b/src/main/java/org/jenkinsci/plugins/workflow/steps/durable_task/DurableTaskStep.java @@ -30,17 +30,21 @@ import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; import hudson.AbortException; import hudson.EnvVars; +import hudson.Extension; import hudson.FilePath; import hudson.Functions; import hudson.Launcher; import hudson.Util; import hudson.init.Terminator; +import hudson.model.Computer; import hudson.model.Node; import hudson.model.Result; import hudson.model.Run; import hudson.model.TaskListener; import hudson.remoting.Channel; import hudson.remoting.ChannelClosedException; +import hudson.slaves.ComputerListener; +import hudson.slaves.OfflineCause; import hudson.util.DaemonThreadFactory; import hudson.util.FormValidation; import hudson.util.LogTaskListener; @@ -51,8 +55,6 @@ import java.io.InputStream; import java.io.OutputStream; import java.io.PrintStream; -import java.io.UnsupportedEncodingException; -import java.lang.reflect.Field; import java.nio.charset.Charset; import java.nio.charset.StandardCharsets; import java.util.Set; @@ -72,7 +74,9 @@ import org.jenkinsci.plugins.durabletask.Handler; import org.jenkinsci.plugins.workflow.FilePathUtils; import org.jenkinsci.plugins.workflow.actions.LabelAction; +import org.jenkinsci.plugins.workflow.flow.FlowExecutionList; import org.jenkinsci.plugins.workflow.graph.FlowNode; +import org.jenkinsci.plugins.workflow.log.OutputStreamTaskListener; import org.jenkinsci.plugins.workflow.steps.AbstractStepExecutionImpl; import org.jenkinsci.plugins.workflow.steps.FlowInterruptedException; import org.jenkinsci.plugins.workflow.steps.Step; @@ -437,26 +441,23 @@ private void getWorkspaceProblem(Exception x) { } /** - * Interprets {@link PrintStream#close} as a signal to end a final newline if necessary. + * Interprets {@link OutputStream#close} as a signal to end a final newline if necessary. */ - private static final class NewlineSafeTaskListener implements TaskListener { + private static final class NewlineSafeTaskListener extends OutputStreamTaskListener.Default { private static final long serialVersionUID = 1; private final TaskListener delegate; - private transient PrintStream logger; + private transient OutputStream out; NewlineSafeTaskListener(TaskListener delegate) { this.delegate = delegate; } // Similar to DecoratedTaskListener: - @NonNull - @Override public synchronized PrintStream getLogger() { - if (logger == null) { - LOGGER.fine("creating filtered stream"); - OutputStream base = delegate.getLogger(); - OutputStream filtered = new FilterOutputStream(base) { + @Override public synchronized OutputStream getOutputStream() { + if (out == null) { + out = new FilterOutputStream(OutputStreamTaskListener.getOutputStream(delegate)) { boolean nl = true; // empty string does not need a newline @Override public void write(int b) throws IOException { super.write(b); @@ -475,14 +476,12 @@ private static final class NewlineSafeTaskListener implements TaskListener { } flush(); // do *not* call base.close() here, unlike super.close() } + @Override public String toString() { + return "NewlineSafeTaskListener.output[" + out + "]"; + } }; - try { - logger = new PrintStream(filtered, false, "UTF-8"); - } catch (UnsupportedEncodingException x) { - throw new AssertionError(x); - } } - return logger; + return out; } } @@ -589,9 +588,12 @@ private void check() { Integer exitCode = controller.exitStatus(workspace, launcher(), listener); if (exitCode == null) { LOGGER.log(Level.FINE, "still running in {0} on {1}", new Object[] {remote, node}); + } else if (recurrencePeriod == 0) { + LOGGER.fine(() -> "late check in " + remote + " on " + node + " ignored"); } else if (awaitingAsynchExit) { recurrencePeriod = 0; - getContext().onFailure(new AbortException("script apparently exited with code " + exitCode + " but asynchronous notification was lost")); + listener.getLogger().println("script apparently exited with code " + exitCode + " but asynchronous notification was lost"); + handleExit(exitCode, () -> controller.getOutput(workspace, launcher())); } else { LOGGER.log(Level.FINE, "exited with {0} in {1} on {2}; expect asynchronous exit soon", new Object[] {exitCode, remote, node}); awaitingAsynchExit = true; @@ -627,6 +629,7 @@ private void check() { // called remotely from HandlerImpl @Override public void exited(int exitCode, byte[] output) throws Exception { + recurrencePeriod = 0; try { getContext().get(TaskListener.class); } catch (IOException | InterruptedException x) { @@ -643,7 +646,6 @@ private void check() { getContext().onFailure(new IllegalStateException("did not expect output but got some")); return; } - recurrencePeriod = 0; handleExit(exitCode, () -> output); } @@ -657,11 +659,11 @@ private void handleExit(int exitCode, OutputSupplier output) throws IOException, getContext().onSuccess(returnStatus ? exitCode : returnStdout ? new String(output.produce(), StandardCharsets.UTF_8) : null); } else { if (returnStdout) { - listener().getLogger().write(output.produce()); // diagnostic + _listener().getLogger().write(output.produce()); // diagnostic } if (originalCause != null) { // JENKINS-28822: Use the previous cause instead of throwing a new AbortException - listener().getLogger().println("script returned exit code " + exitCode); + _listener().getLogger().println("script returned exit code " + exitCode); getContext().onFailure(originalCause); } else { getContext().onFailure(new AbortException("script returned exit code " + exitCode)); @@ -694,21 +696,6 @@ private void setupTimer(long initialRecurrencePeriod) { private static class HandlerImpl extends Handler { - private static final Field printStreamDelegate; - static { - try { - printStreamDelegate = FilterOutputStream.class.getDeclaredField("out"); - } catch (NoSuchFieldException x) { - // Defined in Java Platform and protected, so should not happen. - throw new ExceptionInInitializerError(x); - } - try { - printStreamDelegate.setAccessible(true); - } catch (/* TODO Java 11+ InaccessibleObjectException */RuntimeException x) { - LOGGER.log(Level.WARNING, "On Java 17 error handling is degraded unless `--add-opens java.base/java.io=ALL-UNNAMED` is passed to the agent", x); - } - } - private static final long serialVersionUID = 1L; private final ExecutionRemotable execution; @@ -721,34 +708,24 @@ private static class HandlerImpl extends Handler { @Override public void output(@NonNull InputStream stream) throws Exception { PrintStream ps = listener.getLogger(); + OutputStream os = OutputStreamTaskListener.getOutputStream(listener); try { - if (ps.getClass() == PrintStream.class) { - // Try to extract the underlying stream, since swallowing exceptions is undesirable and PrintStream.checkError is useless. - OutputStream os = ps; - try { - os = (OutputStream) printStreamDelegate.get(ps); - } catch (IllegalAccessException x) { - LOGGER.log(Level.FINE, "using PrintStream rather than underlying FilterOutputStream.out", x); - } - if (os == null) { // like PrintStream.ensureOpen - throw new IOException("Stream closed"); - } - synchronized (ps) { // like PrintStream.write overloads do - IOUtils.copy(stream, os); - } - } else { - // A subclass. Who knows why, but trust any write(…) overrides it may have. - IOUtils.copy(stream, ps); + synchronized (ps) { // like PrintStream.write overloads do + IOUtils.copy(stream, os); } + LOGGER.finest(() -> "print to " + os + " succeeded"); } catch (ChannelClosedException x) { + LOGGER.log(Level.FINE, null, x); // We are giving up on this watch. Wait for some call to getWorkspace to rewatch. throw x; } catch (Exception x) { + LOGGER.log(Level.FINE, null, x); // Try to report it to the controller. try { execution.problem(x); // OK, printed to log on controller side, we may have lost some text but could continue. } catch (Exception x2) { // e.g., RemotingSystemException + LOGGER.log(Level.FINE, null, x2); // No, channel seems to be broken, give up on this watch. throw x; } @@ -762,4 +739,35 @@ private static class HandlerImpl extends Handler { } + @Extension public static final class AgentReconnectionListener extends ComputerListener { + + @Override public void onOffline(Computer c, OfflineCause cause) { + if (Jenkins.get().isTerminating()) { + LOGGER.fine(() -> "Skipping check on " + c.getName() + " during shutdown"); + return; + } + check(c); + } + + @Override public void onOnline(Computer c, TaskListener listener) throws IOException, InterruptedException { + if (!FlowExecutionList.get().isResumptionComplete()) { + LOGGER.fine(() -> "Skipping check on " + c.getName() + " before builds are ready"); + return; + } + check(c); + } + + private void check(Computer c) { + String name = c.getName(); + StepExecution.applyAll(Execution.class, exec -> { + if (exec.watching && exec.node.equals(name)) { + LOGGER.fine(() -> "Online/offline event on " + name + ", checking current status of " + exec.remote + " soon"); + threadPool().schedule(exec::check, 15, TimeUnit.SECONDS); + } + return null; + }); + } + + } + } diff --git a/src/main/java/org/jenkinsci/plugins/workflow/support/steps/AgentErrorCondition.java b/src/main/java/org/jenkinsci/plugins/workflow/support/steps/AgentErrorCondition.java index 72e70d28..129e7b09 100644 --- a/src/main/java/org/jenkinsci/plugins/workflow/support/steps/AgentErrorCondition.java +++ b/src/main/java/org/jenkinsci/plugins/workflow/support/steps/AgentErrorCondition.java @@ -61,7 +61,7 @@ public final class AgentErrorCondition extends ErrorCondition { c -> c instanceof ExecutorStepExecution.RemovedNodeCause || c instanceof ExecutorStepExecution.QueueTaskCancelled)) { return true; } - if (isClosedChannel(t)) { + if (isClosedChannelException(t)) { return true; } if (t instanceof MissingContextVariableException) { @@ -75,7 +75,8 @@ public final class AgentErrorCondition extends ErrorCondition { return false; } - private static boolean isClosedChannel(Throwable t) { + // TODO https://github.com/jenkinsci/remoting/pull/657 + private static boolean isClosedChannelException(Throwable t) { if (t instanceof ClosedChannelException) { return true; } else if (t instanceof ChannelClosedException) { @@ -85,7 +86,7 @@ private static boolean isClosedChannel(Throwable t) { } else if (t == null) { return false; } else { - return isClosedChannel(t.getCause()) || Stream.of(t.getSuppressed()).anyMatch(AgentErrorCondition::isClosedChannel); + return isClosedChannelException(t.getCause()) || Stream.of(t.getSuppressed()).anyMatch(AgentErrorCondition::isClosedChannelException); } } diff --git a/src/test/java/org/jenkinsci/plugins/workflow/steps/durable_task/DurableTaskStepTest.java b/src/test/java/org/jenkinsci/plugins/workflow/steps/durable_task/DurableTaskStepTest.java new file mode 100644 index 00000000..5358e820 --- /dev/null +++ b/src/test/java/org/jenkinsci/plugins/workflow/steps/durable_task/DurableTaskStepTest.java @@ -0,0 +1,127 @@ +/* + * The MIT License + * + * Copyright 2023 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 org.jenkinsci.plugins.workflow.steps.durable_task; + +import hudson.ExtensionList; +import hudson.Functions; +import hudson.model.ParametersAction; +import hudson.model.ParametersDefinitionProperty; +import hudson.model.StringParameterDefinition; +import hudson.model.StringParameterValue; +import hudson.slaves.AbstractCloudSlave; +import hudson.slaves.ComputerListener; +import java.io.File; +import java.time.Duration; +import java.util.List; +import java.util.logging.Level; +import java.util.logging.Logger; +import jenkins.slaves.restarter.JnlpSlaveRestarterInstaller; +import static org.awaitility.Awaitility.await; +import org.jenkinsci.plugins.durabletask.FileMonitoringTask; +import org.jenkinsci.plugins.workflow.cps.CpsFlowDefinition; +import org.jenkinsci.plugins.workflow.job.WorkflowJob; +import org.jenkinsci.plugins.workflow.log.FileLogStorage; +import org.junit.Rule; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; +import org.jvnet.hudson.test.InboundAgentRule; +import org.jvnet.hudson.test.JenkinsRule; +import org.jvnet.hudson.test.PrefixedOutputStream; +import org.jvnet.hudson.test.RealJenkinsRule; +import org.jvnet.hudson.test.TailLog; + +@RunWith(Parameterized.class) +public final class DurableTaskStepTest { + + @Parameterized.Parameters(name = "watching={0}") public static List data() { + return List.of(false, true); + } + + @Parameterized.Parameter public boolean useWatching; + + private static final Logger LOGGER = Logger.getLogger(DurableTaskStepTest.class.getName()); + + @Rule public RealJenkinsRule rr = new RealJenkinsRule(). + javaOptions("-Dorg.jenkinsci.plugins.workflow.support.pickles.ExecutorPickle.timeoutForNodeMillis=" + Duration.ofMinutes(5).toMillis()). // reconnection could be >15s esp. on Windows + withColor(PrefixedOutputStream.Color.BLUE). + withLogger(DurableTaskStep.class, Level.FINE). + withLogger(FileMonitoringTask.class, Level.FINE); + + @Rule public InboundAgentRule inboundAgents = new InboundAgentRule(); + + @Test public void scriptExitingAcrossRestart() throws Throwable { + rr.javaOptions("-D" + DurableTaskStep.class.getName() + ".USE_WATCHING=" + useWatching); + rr.startJenkins(); + rr.runRemotely(DurableTaskStepTest::disableJnlpSlaveRestarterInstaller); + inboundAgents.createAgent(rr, InboundAgentRule.Options.newBuilder(). + color(PrefixedOutputStream.Color.MAGENTA). + label("remote"). + withLogger(FileMonitoringTask.class, Level.FINER). + withLogger(DurableTaskStep.class, Level.FINEST). + withPackageLogger(FileLogStorage.class, Level.FINE). + build()); + try (var tailLog = new TailLog(rr, "p", 1).withColor(PrefixedOutputStream.Color.YELLOW)) { + rr.runRemotely(DurableTaskStepTest::scriptExitingAcrossRestart1); + rr.stopJenkins(); + var f = new File(rr.getHome(), "f"); + LOGGER.info(() -> "Waiting for " + f + " to be written…"); + await().until(f::isFile); + LOGGER.info("…done."); + rr.startJenkins(); + rr.runRemotely(DurableTaskStepTest::scriptExitingAcrossRestart2); + tailLog.waitForCompletion(); + } + } + + /** + * Simulate {@link AbstractCloudSlave} as in https://github.com/jenkinsci/jenkins/pull/7693. + * Most users should be using cloud agents, + * and this lets us preserve {@link InboundAgentRule.Options.Builder#withLogger(Class, Level)}. + */ + private static void disableJnlpSlaveRestarterInstaller(JenkinsRule r) throws Throwable { + ComputerListener.all().remove(ExtensionList.lookupSingleton(JnlpSlaveRestarterInstaller.class)); + } + + private static void scriptExitingAcrossRestart1(JenkinsRule r) throws Throwable { + var p = r.createProject(WorkflowJob.class, "p"); + var f = new File(r.jenkins.getRootDir(), "f"); + p.addProperty(new ParametersDefinitionProperty(new StringParameterDefinition("F"))); + p.setDefinition(new CpsFlowDefinition("node('remote') {if (isUnix()) {sh 'sleep 5 && touch \"$F\"'} else {bat 'ping -n 5 localhost && copy nul \"%F%\" && dir \"%F%\"'}}", true)); + var b = p.scheduleBuild2(0, new ParametersAction(new StringParameterValue("F", f.getAbsolutePath()))).waitForStart(); + r.waitForMessage(Functions.isWindows() ? ">ping -n 5 localhost" : "+ sleep 5", b); + r.jenkins.doQuietDown(true, 0, null); + } + + private static void scriptExitingAcrossRestart2(JenkinsRule r) throws Throwable { + var p = (WorkflowJob) r.jenkins.getItem("p"); + var b = p.getLastBuild(); + r.assertBuildStatusSuccess(r.waitForCompletion(b)); + // In the case of Bourne shell, `+ touch …` is printed when the command actually runs. + // In the case of batch shell, the whole command is printed immediately, so we need to assert that the _output_ of `dir` is there. + r.assertLogContains(Functions.isWindows() ? "Directory of " + r.jenkins.getRootDir() : "+ touch " + new File(r.jenkins.getRootDir(), "f"), b); + } + +} diff --git a/src/test/java/org/jenkinsci/plugins/workflow/steps/durable_task/ShellStepTest.java b/src/test/java/org/jenkinsci/plugins/workflow/steps/durable_task/ShellStepTest.java index 302476af..b8c7ac12 100644 --- a/src/test/java/org/jenkinsci/plugins/workflow/steps/durable_task/ShellStepTest.java +++ b/src/test/java/org/jenkinsci/plugins/workflow/steps/durable_task/ShellStepTest.java @@ -1,13 +1,23 @@ package org.jenkinsci.plugins.workflow.steps.durable_task; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.*; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertSame; +import static org.junit.Assert.assertTrue; +import static org.junit.Assume.assumeFalse; +import static org.junit.Assume.assumeTrue; + import com.cloudbees.plugins.credentials.CredentialsProvider; import com.cloudbees.plugins.credentials.CredentialsScope; import com.cloudbees.plugins.credentials.domains.Domain; import com.cloudbees.plugins.credentials.impl.UsernamePasswordCredentialsImpl; import edu.umd.cs.findbugs.annotations.CheckForNull; import edu.umd.cs.findbugs.annotations.NonNull; -import hudson.FilePath; import hudson.EnvVars; +import hudson.FilePath; import hudson.Functions; import hudson.Launcher; import hudson.LauncherDecorator; @@ -18,19 +28,15 @@ import hudson.console.ConsoleNote; import hudson.console.LineTransformationOutputStream; import hudson.model.BallColor; -import hudson.model.BooleanParameterDefinition; -import hudson.model.BooleanParameterValue; import hudson.model.BuildListener; import hudson.model.Descriptor; import hudson.model.FreeStyleProject; import hudson.model.Node; -import hudson.model.ParametersAction; -import hudson.model.ParametersDefinitionProperty; import hudson.model.Result; import hudson.model.Run; -import hudson.remoting.Channel; import hudson.model.Slave; import hudson.model.TaskListener; +import hudson.remoting.Channel; import hudson.slaves.DumbSlave; import hudson.slaves.EnvironmentVariablesNodeProperty; import hudson.tasks.BatchFile; @@ -42,7 +48,6 @@ import java.io.File; import java.io.IOException; import java.io.OutputStream; -import java.io.PrintStream; import java.io.Serializable; import java.io.StringWriter; import java.io.UncheckedIOException; @@ -62,12 +67,8 @@ import jenkins.tasks.filters.EnvVarsFilterGlobalConfiguration; import jenkins.util.JenkinsJVM; import org.apache.commons.lang.StringUtils; - -import static org.hamcrest.Matchers.*; - import org.hamcrest.MatcherAssert; import org.jenkinsci.plugins.durabletask.FileMonitoringTask; - import org.jenkinsci.plugins.workflow.actions.ArgumentsAction; import org.jenkinsci.plugins.workflow.cps.CpsFlowDefinition; import org.jenkinsci.plugins.workflow.cps.CpsStepContext; @@ -82,6 +83,7 @@ import org.jenkinsci.plugins.workflow.log.FileLogStorage; import org.jenkinsci.plugins.workflow.log.LogStorage; import org.jenkinsci.plugins.workflow.log.LogStorageFactory; +import org.jenkinsci.plugins.workflow.log.OutputStreamTaskListener; import org.jenkinsci.plugins.workflow.steps.BodyExecutionCallback; import org.jenkinsci.plugins.workflow.steps.BodyInvoker; import org.jenkinsci.plugins.workflow.steps.Step; @@ -92,21 +94,15 @@ import org.jenkinsci.plugins.workflow.support.steps.ExecutorStepExecution; import org.jenkinsci.plugins.workflow.support.visualization.table.FlowGraphTable; import org.jenkinsci.plugins.workflow.support.visualization.table.FlowGraphTable.Row; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertNull; -import static org.junit.Assert.assertSame; -import static org.hamcrest.MatcherAssert.assertThat; -import static org.junit.Assert.assertTrue; import org.junit.Assume; -import static org.junit.Assume.assumeFalse; import org.junit.ClassRule; import org.junit.Rule; import org.junit.Test; import org.junit.rules.ErrorCollector; import org.junit.rules.TemporaryFolder; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; import org.jvnet.hudson.test.BuildWatcher; -import org.jvnet.hudson.test.FlagRule; import org.jvnet.hudson.test.Issue; import org.jvnet.hudson.test.JenkinsRule; import org.jvnet.hudson.test.LoggerRule; @@ -115,8 +111,19 @@ import org.kohsuke.stapler.DataBoundConstructor; import org.kohsuke.stapler.DataBoundSetter; +@RunWith(Parameterized.class) public class ShellStepTest { + @Parameterized.Parameters(name = "watching={0}") public static List data() { + return List.of(false, true); + } + + @Parameterized.BeforeParam public static void useWatching(boolean x) { + DurableTaskStep.USE_WATCHING = x; + } + + @Parameterized.Parameter public boolean useWatching; + @ClassRule public static BuildWatcher buildWatcher = new BuildWatcher(); @@ -124,7 +131,6 @@ public class ShellStepTest { @Rule public TemporaryFolder tmp = new TemporaryFolder(); @Rule public ErrorCollector errors = new ErrorCollector(); @Rule public LoggerRule logging = new LoggerRule(); - @Rule public FlagRule useWatching = new FlagRule<>(() -> DurableTaskStep.USE_WATCHING, x -> DurableTaskStep.USE_WATCHING = x); /** * Failure in the shell script should mark the step as red @@ -275,7 +281,7 @@ private static class Decorator extends LauncherDecorator implements Serializable return launcher.decorateByPrefix("nice"); } } - @TestExtension("launcherDecorator") public static class DescriptorImpl extends StepDescriptor { + @TestExtension({"launcherDecorator[watching=false]", "launcherDecorator[watching=true]"}) public static class DescriptorImpl extends StepDescriptor { @Override public Set> getRequiredContext() { return Collections.emptySet(); } @@ -382,8 +388,8 @@ private static class Decorator extends LauncherDecorator implements Serializable @Issue("JENKINS-38381") @Test public void remoteLogger() throws Exception { + assumeTrue(useWatching); logging.record(DurableTaskStep.class, Level.FINE).record(FileMonitoringTask.class, Level.FINE); - DurableTaskStep.USE_WATCHING = true; assumeFalse(Functions.isWindows()); // TODO create Windows equivalent final String credentialsId = "creds"; final String username = "bob"; @@ -414,7 +420,7 @@ private static class Decorator extends LauncherDecorator implements Serializable j.assertLogNotContains(password.toUpperCase(Locale.ENGLISH), b); j.assertLogContains("CURL -U **** HTTP://SERVER/ [master → remote]", b); } - @TestExtension("remoteLogger") public static class LogFile implements LogStorageFactory { + @TestExtension("remoteLogger[watching=true]") public static class LogFile implements LogStorageFactory { @Override public LogStorage forBuild(FlowExecutionOwner b) { final LogStorage base; try { @@ -442,13 +448,13 @@ private static class Decorator extends LauncherDecorator implements Serializable }; } } - private static class RemotableBuildListener implements BuildListener { + private static class RemotableBuildListener extends OutputStreamTaskListener.Default implements BuildListener { private static final long serialVersionUID = 1; /** actual implementation */ private final TaskListener delegate; /** records allocation & deserialization history; e.g., {@code master → agent} */ private final String id; - private transient PrintStream logger; + private transient OutputStream out; RemotableBuildListener(TaskListener delegate) { this(delegate, "master"); } @@ -457,10 +463,10 @@ private RemotableBuildListener(TaskListener delegate, String id) { this.id = id; } @NonNull - @Override public PrintStream getLogger() { - if (logger == null) { - final OutputStream os = delegate.getLogger(); - logger = new PrintStream(new LineTransformationOutputStream() { + @Override public OutputStream getOutputStream() { + if (out == null) { + final OutputStream os = OutputStreamTaskListener.getOutputStream(delegate); + out = new LineTransformationOutputStream() { @Override protected void eol(byte[] b, int len) throws IOException { for (int i = 0; i < len - 1; i++) { // all but NL os.write(id.equals("master") ? b[i] : Character.toUpperCase(b[i])); @@ -475,9 +481,9 @@ private RemotableBuildListener(TaskListener delegate, String id) { super.close(); os.close(); } - }, true); + }; } - return logger; + return out; } private Object writeReplace() { /* To see serialization happening from BourneShellScript.launchWithCookie & FileMonitoringController.watch: @@ -490,7 +496,7 @@ private Object writeReplace() { @Issue("JENKINS-54133") @Test public void remoteConsoleNotes() throws Exception { - DurableTaskStep.USE_WATCHING = true; + assumeTrue(useWatching); assumeFalse(Functions.isWindows()); // TODO create Windows equivalent j.createSlave("remote", null, null); WorkflowJob p = j.jenkins.createProject(WorkflowJob.class, "p"); @@ -628,29 +634,25 @@ private static final class HelloNote extends ConsoleNote> { logging.record(DurableTaskStep.class, Level.FINE).record(FileMonitoringTask.class, Level.FINE); j.showAgentLogs(s, logging); WorkflowJob p = j.jenkins.createProject(WorkflowJob.class, "p"); - p.addProperty(new ParametersDefinitionProperty(new BooleanParameterDefinition("WATCHING", false, null))); String builtInNodeLabel = j.jenkins.getSelfLabel().getName(); // compatibility with 2.307+ p.setDefinition(new CpsFlowDefinition( "['" + builtInNodeLabel + "', 'remote'].each {label ->\n" + " node(label) {\n" + " withCredentials([usernameColonPassword(variable: 'USERPASS', credentialsId: '" + credentialsId + "')]) {\n" + - " sh 'set +x; echo \"with final newline node=$NODE_NAME watching=$WATCHING\"'\n" + - " sh 'set +x; printf \"missing final newline node=$NODE_NAME watching=$WATCHING\"'\n" + + " sh 'set +x; echo \"node=$NODE_NAME with final newline\"'\n" + + " sh 'set +x; printf \"node=$NODE_NAME missing final newline\"'\n" + " }\n" + " }\n" + "}", true)); - for (boolean watching : new boolean[] {false, true}) { - DurableTaskStep.USE_WATCHING = watching; - String log = JenkinsRule.getLog(j.assertBuildStatusSuccess(p.scheduleBuild2(0, new ParametersAction(new BooleanParameterValue("WATCHING", watching))))); - for (String node : new String[] {builtInNodeLabel, "remote"}) { - for (String mode : new String[] {"with", "missing"}) { - errors.checkThat(log, containsString(mode + " final newline node=" + node + " watching=" + watching)); - } - } - errors.checkThat("no blank lines with watching=" + watching, log, not(containsString("\n\n"))); - errors.checkThat(log, not(containsString("watching=false[Pipeline]"))); - errors.checkThat(log, not(containsString("watching=true[Pipeline]"))); + String log = JenkinsRule.getLog(j.buildAndAssertSuccess(p)); + for (String node : new String[] {builtInNodeLabel, "remote"}) { + for (String mode : new String[] {"with", "missing"}) { + errors.checkThat(log, containsString("node=" + node + " " + mode + " final newline")); } + } + errors.checkThat("no blank lines", log, not(containsString("\n\n"))); + // TODO more robust to assert against /missing final newline./ in line mode + errors.checkThat(log, not(containsString("missing final newline[Pipeline]"))); } @Issue("JENKINS-34021") @@ -769,7 +771,7 @@ private void ensureForWhile(int timeout, T o, Predicate predicate) throws } } - @TestExtension(value = "shouldInvokeLauncherDecoratorForShellStep") + @TestExtension({"shouldInvokeLauncherDecoratorForShellStep[watching=false]", "shouldInvokeLauncherDecoratorForShellStep[watching=true]"}) public static final class MyNodeLauncherDecorator extends LauncherDecorator { @NonNull diff --git a/src/test/java/org/jenkinsci/plugins/workflow/support/steps/ExecutorStepTest.java b/src/test/java/org/jenkinsci/plugins/workflow/support/steps/ExecutorStepTest.java index a9a0f0bb..c8f54b48 100644 --- a/src/test/java/org/jenkinsci/plugins/workflow/support/steps/ExecutorStepTest.java +++ b/src/test/java/org/jenkinsci/plugins/workflow/support/steps/ExecutorStepTest.java @@ -24,7 +24,16 @@ package org.jenkinsci.plugins.workflow.support.steps; -import org.htmlunit.Page; +import static org.awaitility.Awaitility.await; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.*; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotEquals; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; + import com.google.common.base.Predicate; import edu.umd.cs.findbugs.annotations.Nullable; import hudson.FilePath; @@ -70,8 +79,6 @@ import java.util.regex.Matcher; import java.util.regex.Pattern; import java.util.stream.Collectors; - -import hudson.util.VersionNumber; import jenkins.model.Jenkins; import jenkins.security.QueueItemAuthenticator; import jenkins.security.QueueItemAuthenticatorConfiguration; @@ -80,8 +87,7 @@ import net.sf.json.groovy.JsonSlurper; import org.acegisecurity.Authentication; import org.apache.commons.io.IOUtils; -import static org.awaitility.Awaitility.await; -import static org.hamcrest.Matchers.*; +import org.htmlunit.Page; import org.jenkinsci.plugins.durabletask.FileMonitoringTask; import org.jenkinsci.plugins.workflow.actions.LogAction; import org.jenkinsci.plugins.workflow.actions.QueueItemAction; @@ -100,31 +106,37 @@ import org.jenkinsci.plugins.workflow.steps.durable_task.DurableTaskStep; import org.jenkinsci.plugins.workflow.steps.durable_task.Messages; import org.jenkinsci.plugins.workflow.test.steps.SemaphoreStep; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertNotEquals; -import static org.junit.Assert.assertNotNull; -import static org.hamcrest.MatcherAssert.assertThat; -import static org.junit.Assert.assertTrue; -import static org.junit.Assert.fail; import org.junit.Assume; import org.junit.ClassRule; import org.junit.Rule; import org.junit.Test; import org.junit.rules.TemporaryFolder; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; import org.jvnet.hudson.test.BuildWatcher; import org.jvnet.hudson.test.InboundAgentRule; import org.jvnet.hudson.test.Issue; import org.jvnet.hudson.test.JenkinsRule; +import org.jvnet.hudson.test.JenkinsSessionRule; import org.jvnet.hudson.test.LoggerRule; import org.jvnet.hudson.test.MockAuthorizationStrategy; import org.jvnet.hudson.test.MockFolder; -import org.jvnet.hudson.test.JenkinsSessionRule; import org.jvnet.hudson.test.TestExtension; /** Tests pertaining to {@code node} and {@code sh} steps. */ +@RunWith(Parameterized.class) public class ExecutorStepTest { + @Parameterized.Parameters(name = "watching={0}") public static List data() { + return List.of(false, true); + } + + @Parameterized.BeforeParam public static void useWatching(boolean x) { + DurableTaskStep.USE_WATCHING = x; + } + + @Parameterized.Parameter public boolean useWatching; + private static final Logger LOGGER = Logger.getLogger(ExecutorStepTest.class.getName()); @ClassRule public static BuildWatcher buildWatcher = new BuildWatcher();