Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
37 commits
Select commit Hold shift + click to select a range
b945cf2
[JENKINS-38381] Controller.watch API.
jglick Feb 9, 2018
27f957f
Reminder to pick up https://github.com/jenkinsci/jenkins/pull/3272.
jglick Feb 13, 2018
cc6029c
Merge branch 'UTF-8-JENKINS-31096' into watch-plus-UTF-8-JENKINS-31096
jglick Feb 13, 2018
5d7ea7f
Integrating transcoding into watches.
jglick Feb 13, 2018
6141407
Merge branch 'UTF-8-JENKINS-31096' into watch-plus-UTF-8-JENKINS-31096
jglick Feb 13, 2018
deb425d
Completing test coverage.
jglick Feb 13, 2018
625da89
Merge branch 'UTF-8-JENKINS-31096' into watch-plus-UTF-8-JENKINS-31096
jglick Feb 19, 2018
814047d
Merge with #66.
jglick Mar 6, 2018
d3e2afb
Merge branch 'watch-JENKINS-38381' into watch-plus-UTF-8-JENKINS-31096
jglick Mar 6, 2018
768bf09
Merge branch 'UTF-8-JENKINS-31096' into watch-plus-UTF-8-JENKINS-31096
jglick Mar 6, 2018
9f6e2ec
Merge branch 'incrementals' into watch-JENKINS-38381
jglick Jun 8, 2018
5ac713d
Suggestions from @oleg-nenashev.
jglick Jun 8, 2018
6974ae2
Decided that unresponsiveness to kill is better handled by having Dur…
jglick Jun 8, 2018
d699ee0
Merge branch 'UTF-8-JENKINS-31096' into watch-plus-UTF-8-JENKINS-31096
jglick Jun 9, 2018
e219d47
Merge branch 'watch-JENKINS-38381' into watch-plus-UTF-8-JENKINS-31096
jglick Jun 9, 2018
c910214
Merge branch 'writeLog-JENKINS-37575' into watch-JENKINS-38381
jglick Jun 25, 2018
129d6b7
Merge branch 'writeLog-JENKINS-37575' into watch-JENKINS-38381
jglick Jun 25, 2018
9b56686
Merge branch 'UTF-8-JENKINS-31096' into watch-plus-UTF-8-JENKINS-31096
jglick Jun 25, 2018
e0fced2
Merge branch 'watch-JENKINS-38381' into watch-plus-UTF-8-JENKINS-31096
jglick Jun 25, 2018
8c7ea34
Merge branch 'writeLog-JENKINS-37575' into watch-JENKINS-38381
jglick Jul 9, 2018
20c9649
Merge branch 'UTF-8-JENKINS-31096' into watch-plus-UTF-8-JENKINS-31096
jglick Jul 9, 2018
a2a3d11
Merge branch 'watch-JENKINS-38381' into watch-plus-UTF-8-JENKINS-31096
jglick Jul 9, 2018
2e9d565
Merge branch 'parent' into watch-JENKINS-38381
jglick Aug 6, 2018
6267445
Merge branch 'watch-JENKINS-38381' into watch-plus-UTF-8-JENKINS-31096
jglick Aug 6, 2018
bd9232f
Merge branch 'UTF-8-JENKINS-31096' into watch-plus-UTF-8-JENKINS-31096
jglick Aug 6, 2018
95f9062
Fixed the lastLocation vs. transcoding bug also for watch mode.
jglick Aug 6, 2018
a330852
Merge branch 'master' into watch-plus-UTF-8-JENKINS-31096
jglick Aug 7, 2018
b40fbed
Merge branch 'master' into watch-JENKINS-38381
jglick Aug 7, 2018
c46fc88
Split out a parameterized EncodingTest for clarity.
jglick Aug 7, 2018
147acaf
Adding comments to clarify discussion with @oleg-nenashev.
jglick Aug 7, 2018
399e3eb
[JENKINS-37575] As in #74, last-location.txt should record only what …
jglick Aug 7, 2018
3ba5eb8
merge
jglick Aug 8, 2018
e3119d5
@dwnusbaum noticed that code could be simplified a bit by dropping a …
jglick Aug 8, 2018
8feca39
Merge branch 'master' into watch-JENKINS-38381
jglick Aug 8, 2018
b077ce3
Reverting 399e3eb7deb75f611a2cf1c6d00a5011e2dcff22; seems to be bette…
jglick Aug 10, 2018
dccb646
Comments.
jglick Aug 10, 2018
c1ee660
Merge branch 'master' into watch-JENKINS-38381
jglick Sep 4, 2018
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -206,9 +206,8 @@ private FilePath pidFile(FilePath ws) throws IOException, InterruptedException {
return controlDir(ws).child("pid");
}

// TODO run as one big MasterToSlaveCallable<Integer> to avoid extra network roundtrips
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(was added in #66 but is obsolete here)

@Override public Integer exitStatus(FilePath workspace, Launcher launcher, TaskListener listener) throws IOException, InterruptedException {
Integer status = super.exitStatus(workspace, launcher, listener);
@Override protected Integer exitStatus(FilePath workspace, TaskListener listener) throws IOException, InterruptedException {
Integer status = super.exitStatus(workspace, listener);
if (status != null) {
LOGGER.log(Level.FINE, "found exit code {0} in {1}", new Object[] {status, controlDir});
return status;
Expand Down
20 changes: 18 additions & 2 deletions src/main/java/org/jenkinsci/plugins/durabletask/Controller.java
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
import hudson.Launcher;
import hudson.Util;
import hudson.model.TaskListener;
import hudson.remoting.ChannelClosedException;
import hudson.util.LogTaskListener;
import java.io.IOException;
import java.io.OutputStream;
Expand All @@ -43,6 +44,21 @@
*/
public abstract class Controller implements Serializable {

/**
* Begins watching the process asynchronously, so that the master may receive notification when output is available or the process has exited.
* This should be called as soon as the process is launched, and thereafter whenever reconnecting to the agent.
* You should not call {@link #writeLog} or {@link #cleanup} in this case; you do not need to call {@link #exitStatus(FilePath, Launcher)} frequently,
* though it is advisable to still call it occasionally to verify that the process is still running.
* @param workspace the workspace in use
* @param handler a remotable callback
* @param listener a remotable destination for messages
* @throws IOException if initiating the watch fails, for example with a {@link ChannelClosedException}
* @throws UnsupportedOperationException when this mode is not available, so you must fall back to polling {@link #writeLog} and {@link #exitStatus(FilePath, Launcher)}
*/
public void watch(@Nonnull FilePath workspace, @Nonnull Handler handler, @Nonnull TaskListener listener) throws IOException, InterruptedException, UnsupportedOperationException {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Speculative, but what's the expected behavior in the face of an InterruptedException here?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

During DurableTaskStep.Execution.start: the sh step would fail. During getWorkspace: we retry later.

throw new UnsupportedOperationException("Asynchronous mode is not implemented in " + getClass().getName());
}

/**
* Obtains any new task log output.
* Could use a serializable field to keep track of how much output has been previously written.
Expand All @@ -57,7 +73,7 @@ public abstract class Controller implements Serializable {
/**
* Checks whether the task has finished.
* @param workspace the workspace in use
* @param launcher a way to start processes
* @param launcher a way to start processes (currently unused)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The method is overridable, and the downstream implementation may actually use it but inherit Javadocs. So I would rather not touch it

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The parameter is no longer used in any existing implementation and it likely never will be, but harms nothing to leave it there.

* @param logger a way to report special messages
* @return an exit code (zero is successful), or null if the task appears to still be running
*/
Expand Down Expand Up @@ -88,7 +104,7 @@ public abstract class Controller implements Serializable {
* Intended for use after {@link #exitStatus(FilePath, Launcher)} has returned a non-null status.
* The result is undefined if {@link DurableTask#captureOutput} was not called before launch; generally an {@link IOException} will result.
* @param workspace the workspace in use
* @param launcher a way to start processes
* @param launcher a way to start processes (currently unused)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(ditto)

* @return the output of the process as raw bytes (may be empty but not null)
* @see DurableTask#charset
* @see DurableTask#defaultCharset
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,8 +59,9 @@ public abstract class DurableTask extends AbstractDescribableImpl<DurableTask> i
public abstract Controller launch(EnvVars env, FilePath workspace, Launcher launcher, TaskListener listener) throws IOException, InterruptedException;

/**
* Requests that standard output of the task be captured rather than streamed to {@link Controller#writeLog}.
* If so, you may call {@link Controller#getOutput}.
* Requests that standard output of the task be captured rather than streamed.
* If you use {@link Controller#watch}, standard output will not be sent to {@link Handler#output}; it will be included in {@link Handler#exited} instead.
* Otherwise (using polling mode), standard output will not be sent to {@link Controller#writeLog}; call {@link Controller#getOutput} to collect.
* Standard error should still be streamed to the log.
* Should be called prior to {@link #launch} to take effect.
* @throws UnsupportedOperationException if this implementation does not support that mode
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,25 +31,36 @@
import hudson.Util;
import hudson.model.TaskListener;
import hudson.remoting.Channel;
import hudson.remoting.DaemonThreadFactory;
import hudson.remoting.NamingThreadFactory;
import hudson.remoting.RemoteOutputStream;
import hudson.remoting.VirtualChannel;
import hudson.slaves.WorkspaceList;
import hudson.util.StreamTaskListener;
import java.io.File;
import java.io.IOException;
import java.io.InputStream;
import java.io.InputStreamReader;
import java.io.OutputStream;
import java.io.OutputStreamWriter;
import java.io.RandomAccessFile;
import java.io.StringWriter;
import java.nio.ByteBuffer;
import java.nio.channels.Channels;
import java.nio.channels.FileChannel;
import java.nio.charset.Charset;
import java.nio.charset.CharsetDecoder;
import java.nio.charset.CodingErrorAction;
import java.nio.charset.StandardCharsets;
import java.nio.file.Paths;
import java.nio.file.StandardOpenOption;
import java.util.Collections;
import java.util.Map;
import java.util.TreeMap;
import java.util.UUID;
import java.util.concurrent.ScheduledExecutorService;
import java.util.concurrent.ScheduledThreadPoolExecutor;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicReference;
import java.util.logging.Level;
import java.util.logging.Logger;
Expand All @@ -58,6 +69,7 @@
import jenkins.MasterToSlaveFileCallable;
import jenkins.security.MasterToSlaveCallable;
import org.apache.commons.io.FileUtils;
import org.apache.commons.io.input.ReaderInputStream;
import org.apache.commons.io.output.CountingOutputStream;
import org.apache.commons.io.output.WriterOutputStream;

Expand Down Expand Up @@ -125,7 +137,11 @@ protected static Map<String, String> escape(EnvVars envVars) {
return m;
}

protected static class FileMonitoringController extends Controller {
/**
* Tails a log file and watches for an exit status file.
* Must be remotable so that {@link #watch} can transfer the implementation.
*/
protected static class FileMonitoringController extends Controller { // TODO implements Remotable when available (*not* SerializableOnlyOverRemoting)

/** Absolute path of {@link #controlDir(FilePath)}. */
String controlDir;
Expand All @@ -137,6 +153,7 @@ protected static class FileMonitoringController extends Controller {

/**
* Byte offset in the file that has been reported thus far.
* Only used if {@link #writeLog(FilePath, OutputStream)} is used; not used for {@link #watch}.
*/
private long lastLocation;

Expand Down Expand Up @@ -251,11 +268,25 @@ public Integer invoke(File f, VirtualChannel channel) throws IOException, Interr
static final StatusCheck STATUS_CHECK_INSTANCE = new StatusCheck();

@Override public Integer exitStatus(FilePath workspace, Launcher launcher, TaskListener listener) throws IOException, InterruptedException {
return exitStatus(workspace, listener);
}

/**
* Like {@link #exitStatus(FilePath, Launcher, TaskListener)} but not requesting a {@link Launcher}, which would not be available in {@link #watch} mode anyway.
*/
protected @CheckForNull Integer exitStatus(FilePath workspace, TaskListener listener) throws IOException, InterruptedException {
FilePath status = getResultFile(workspace);
return status.act(STATUS_CHECK_INSTANCE);
}

@Override public byte[] getOutput(FilePath workspace, Launcher launcher) throws IOException, InterruptedException {
return getOutput(workspace);
}

/**
* Like {@link #getOutput(FilePath, Launcher)} but not requesting a {@link Launcher}, which would not be available in {@link #watch} mode anyway.
*/
protected byte[] getOutput(FilePath workspace) throws IOException, InterruptedException {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe it's better to introduce a new method with optional Launcher so that it's not ignored in the implementations?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure I follow what you are saying.

To be clear, this overload needed to be introduced so it can be called from the watch task, which cannot produce a (usable) Launcher; the original overload was not using its launcher parameter anyway.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we're going to rework the way output is done, I'd really like to return a stream-able data implementation rather than raw byte[] - this ensures we're not allocating giant wads of memory to collect all the data if something produces logs quickly. Whether it's a Stream, Channel, whatever I do not care as much.

Consider also something like 'writeOutput' that internally feeds data to a sink.

It's also just plain more elegant and easier to work with in general.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually... we could still count location that way using a CountingOutputStream I believe.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was already discussed earlier. This method is only used when you specify returnStdout: true, a fairly specialized case which is anyway only suited for small amounts of output—in the case of a workflow-cps caller, the entire resulting String gets saved to program.dat.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Damnit, we've gone in circles on these PRs so long that I'm repeating myself -- at least I'm consistent though! 😆

return getOutputFile(workspace).act(new MasterToSlaveFileCallable<byte[]>() {
@Override public byte[] invoke(File f, VirtualChannel channel) throws IOException, InterruptedException {
byte[] buf = FileUtils.readFileToByteArray(f);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sigh, so much wish for streaming use.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ditto, unnecessary here.

Expand Down Expand Up @@ -367,7 +398,125 @@ public FilePath getOutputFile(FilePath workspace) throws IOException, Interrupte
}
}

@Override public void watch(FilePath workspace, Handler handler, TaskListener listener) throws IOException, InterruptedException, ClassCastException {
workspace.actAsync(new StartWatching(this, handler, listener));
LOGGER.log(Level.FINE, "started asynchronous watch in {0}", controlDir);
}

/**
* File in which a last-read position is stored if {@link #watch} is used.
*/
public FilePath getLastLocationFile(FilePath workspace) throws IOException, InterruptedException {
return controlDir(workspace).child("last-location.txt");
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@svanoort noted that there should be a test for the upgrade scenario, either using @LocalData or a switched flag in the plugin to simulate an upgrade. Either could also simulate a dirty shutdown.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Anyway would be a topic for jenkinsci/workflow-durable-task-step-plugin#63 not this PR.

}

private static final long serialVersionUID = 1L;
}

private static ScheduledExecutorService watchService;
private synchronized static ScheduledExecutorService watchService() {
if (watchService == null) {
// TODO 2.105+ use ClassLoaderSanityThreadFactory
watchService = new /*ErrorLogging*/ScheduledThreadPoolExecutor(5, new NamingThreadFactory(new DaemonThreadFactory(), "FileMonitoringTask watcher"));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wish we could use a thread pool that grows as needed for load (with an upper limit) but still supports scheduling. Sigh.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

( I know, I know, not part of the standard library )

}
return watchService;
}

private static class StartWatching extends MasterToSlaveFileCallable<Void> {

private static final long serialVersionUID = 1L;

private final FileMonitoringController controller;
private final Handler handler;
private final TaskListener listener;

StartWatching(FileMonitoringController controller, Handler handler, TaskListener listener) {
this.controller = controller;
this.handler = handler;
this.listener = listener;
}

@Override public Void invoke(File workspace, VirtualChannel channel) throws IOException, InterruptedException {
watchService().submit(new Watcher(controller, new FilePath(workspace), handler, listener));
return null;
}

}

private static class Watcher implements Runnable {

private final FileMonitoringController controller;
private final FilePath workspace;
private final Handler handler;
private final TaskListener listener;
private final @CheckForNull Charset cs;

Watcher(FileMonitoringController controller, FilePath workspace, Handler handler, TaskListener listener) {
this.controller = controller;
this.workspace = workspace;
this.handler = handler;
this.listener = listener;
cs = FileMonitoringController.transcodingCharset(controller.charset);
}

@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());
}
FilePath logFile = controller.getLogFile(workspace);
long len = logFile.length();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe check the file's existence just in case? otherwise the watcher will give up immediately (IOException ). But IIUC the code it may want to wait to avoid race conditions. Or not?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the file does not exist (because the wrapper script is still launching, or failed to launch), length() will be zero, thus nothing will happen on this iteration and we check again in another 100ms.

if (len > lastLocation) {
assert !logFile.isRemote();
try (FileChannel ch = FileChannel.open(Paths.get(logFile.getRemote()), StandardOpenOption.READ)) {
InputStream locallyEncodedStream = Channels.newInputStream(ch.position(lastLocation));
InputStream utf8EncodedStream = cs == null ? locallyEncodedStream : new ReaderInputStream(new InputStreamReader(locallyEncodedStream, cs), StandardCharsets.UTF_8);
handler.output(utf8EncodedStream);
lastLocationFile.write(Long.toString(ch.position()), null);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think a comment explaining that this is intentionally not in a finally block so that we duplicate data rather than skip it when an error occurs would be helpful.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just added that.

}
}
if (exitStatus != null) {
byte[] output;
if (controller.getOutputFile(workspace).exists()) {
output = controller.getOutput(workspace);
} else {
output = null;
}
handler.exited(exitStatus, output);
controller.cleanup(workspace);
} else {
if (!controller.controlDir(workspace).isDirectory()) {
LOGGER.log(Level.WARNING, "giving up on watching nonexistent {0}", controller.controlDir);
return;
}
// Could use an adaptive timeout as in DurableTaskStep.Execution in polling mode,
// though less relevant here since there is no network overhead to the check.
watchService().schedule(this, 100, TimeUnit.MILLISECONDS);
}
} catch (Exception x) {
// note that LOGGER here is going to the agent log, not master log
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so Runtime exceptions will be non-recoverable here... Looks fine assuming that we want Handler implementations to be written properly.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If there is an error because the channel is closed, then if and when a new agent is started, a new watcher should be created; so, fine. If there is some other kind of error, then yes this is fatal—we stop sending status updates from this process.

Copy link
Member

@dwnusbaum dwnusbaum Aug 10, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If there is a non-remoting error, then it looks like the build will keep running until controller.exitStatus returns null in DurableTaskStep.Execution#check. Would it helpful/possible to add some way to indicate to DurableTaskStep that logging appears to be broken? A passing build with broken logs would be strange, although failing the build because logging is broken doesn't seem any better.

If the channel is closed, then everything should retry in DurableTask#getWorkspace during the next call to DurableTaskStep.Execution#check so that seems fine to me, although the 5 minute recurrence period seems a little long.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it helpful/possible to add some way to indicate to DurableTaskStep that logging appears to be broken?

Possibly. I will think about another method in Handler for this purpose. As you note, in the case of a closed channel, the agent likely cannot communicate that error by definition, and getWorkspace is responsible for fixing things up a few minutes later. But in the case of a cloudy log implementation it could be useful to indicate to the master that some other error occurred, logs may be faulty, and we should rewatch when possible. Whether anything else in the build gets successfully logged at that point, who knows.

A passing build with broken logs would be strange, although failing the build because logging is broken doesn't seem any better.

Yeah it seems a bit of a toss-up. My general expectation is that if something strange happens without a clear recovery procedure, we should err on the side of just failing the build.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

jenkinsci/workflow-durable-task-step-plugin@c859378 may produce better outcomes for cloudy log sinks. Without a test case it is hard to predict exactly what the failure modes will be, much less what response a user would actually find helpful.

ExecutorStepTest.shellOutputAcrossRestart does now exercise the code path under discussion in the case of the stock Remoting sink. The aforementioned commit does not change the behavior in that test, because it is indeed a ChannelClosedException that you get when the agent is disconnected from the master side. (If you just kill the agent JVM via either SIGTERM or SIGKILL, then the watcher task dies with it and you are also OK.)

LOGGER.log(Level.WARNING, "giving up on watching " + controller.controlDir, x);
// Typically this will have been inside Handler.output, e.g.:
// hudson.remoting.ChannelClosedException: channel is already closed
// at hudson.remoting.Channel.send(Channel.java:667)
// at hudson.remoting.ProxyOutputStream.write(ProxyOutputStream.java:143)
// at hudson.remoting.RemoteOutputStream.write(RemoteOutputStream.java:110)
// at org.apache.commons.io.IOUtils.copyLarge(IOUtils.java:1793)
// at org.apache.commons.io.IOUtils.copyLarge(IOUtils.java:1769)
// at org.apache.commons.io.IOUtils.copy(IOUtils.java:1744)
// at org.jenkinsci.plugins.workflow.steps.durable_task.DurableTaskStep$HandlerImpl.output(DurableTaskStep.java:503)
// at org.jenkinsci.plugins.durabletask.FileMonitoringTask$Watcher.run(FileMonitoringTask.java:477)
// Thus we assume the log sink is hopeless and the Watcher task dies.
// If and when the agent is reconnected, a new watch call will be made and we will resume streaming.
// last-location.txt will record the last successfully written block of output;
// we cannot know reliably how much of the problematic block was actually received by the sink,
// so we err on the side of possibly duplicating text rather than losing text.
}
}

}

}
67 changes: 67 additions & 0 deletions src/main/java/org/jenkinsci/plugins/durabletask/Handler.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
/*
* The MIT License
*
* Copyright 2016 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.durabletask;

import hudson.FilePath;
import hudson.Launcher;
import hudson.remoting.VirtualChannel;
import java.io.InputStream;
import java.io.Serializable;
import javax.annotation.Nonnull;
import javax.annotation.Nullable;

/**
* A remote handler which may be sent to an agent and handle process output and results.
* If it needs to communicate with the master, you may use {@link VirtualChannel#export}.
* @see Controller#watch
*/
public abstract class Handler implements Serializable { // TODO 2.107+ SerializableOnlyOverRemoting

/**
* Notification that new process output is available.
* <p>Should only be called when at least one byte is available.
* Whatever bytes are actually read will not be offered on the next call, if there is one; there is no need to close the stream.
* <p>There is no guarantee that output is offered in the form of complete lines of text,
* though in the typical case of line-oriented output it is likely that it will end in a newline.
* <p>Buffering is the responsibility of the caller, and {@link InputStream#markSupported} may be false.
* @param stream a way to read process output which has not already been handled
* @throws Exception if anything goes wrong, this watch is deactivated
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would rather use a more narrow exception type in throws so that Handler implementers do not start throwing RuntimeExceptions. But you will have to catch them when invoking Handler methods anyway...

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do not really care what type is thrown. If anything is caught, the watch task shuts down.

*/
public abstract void output(@Nonnull InputStream stream) throws Exception;

/**
* Notification that the process has exited or vanished.
* {@link #output} should have been called with any final uncollected output.
* <p>Any metadata associated with the process may be deleted after this call completes, rendering subsequent {@link Controller} calls unsatisfiable.
* <p>Note that unlike {@link Controller#exitStatus(FilePath, Launcher)}, no specialized {@link Launcher} is available on the agent,
* so if there are specialized techniques for determining process liveness they will not be considered here;
* you still need to occasionally poll for an exit status from the master.
* @param code the exit code, if known (0 conventionally represents success); may be negative for anomalous conditions such as a missing process
* @param output standard output captured, if {@link DurableTask#captureOutput} was called; else null
* @throws Exception if anything goes wrong, this watch is deactivated
*/
public abstract void exited(int code, @Nullable byte[] output) throws Exception;

}
Loading