Skip to content

[JENKINS-54078] JEP-210 compatibility #74

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
dwnusbaum merged 10 commits into from
Oct 26, 2018
Merged

[JENKINS-54078] JEP-210 compatibility #74

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
2ad8c70
[JENKINS-26521] Prove that Channel.export is needed for JEP-210 compa…
jglick Oct 8, 2018
11c9999
Updated test to match change in logging format.
jglick Oct 9, 2018
746f455
[JENKINS-54078] Reproduced error in test.
jglick Oct 16, 2018
f66bb29
May as well use nonbeta versions of JEP-210 plugins.
jglick Oct 16, 2018
128f7d2
[JENKINS-54078] Do not rely on Channel.export as we have little contr…
jglick Oct 16, 2018
5d8c464
https://github.com/jenkinsci/workflow-durable-task-step-plugin/pull/7…
jglick Oct 23, 2018
0918918
Handle deserialization of old ConsoleLogFilterImpl with null id.
jglick Oct 23, 2018
46433e1
Minimize the number of ResetTimer calls made.
jglick Oct 25, 2018
3af06c9
If the Tick sees an idle stream, reschedule sooner to avoid a false t…
jglick Oct 25, 2018
066e961
Retain the original ConsoleLogFilterImpl for derialization, but do no…
jglick Oct 25, 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
47 changes: 39 additions & 8 deletions pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
<parent>
<groupId>org.jenkins-ci.plugins</groupId>
<artifactId>plugin</artifactId>
<version>3.19</version>
<version>3.25</version>
Copy link
Member Author

@jglick jglick Oct 8, 2018

Choose a reason for hiding this comment

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

Various changes to pick up JEP-210 binaries.

<relativePath />
</parent>
<groupId>org.jenkins-ci.plugins.workflow</groupId>
Expand Down Expand Up @@ -67,10 +67,12 @@
<jenkins.version>2.121.1</jenkins.version>
<java.level>8</java.level>
<workflow-step-api-plugin.version>2.15</workflow-step-api-plugin.version>
<workflow-cps-plugin.version>2.32</workflow-cps-plugin.version>
<workflow-support-plugin.version>2.14</workflow-support-plugin.version>
<workflow-api-plugin.version>2.28</workflow-api-plugin.version>
<workflow-cps-plugin.version>2.58</workflow-cps-plugin.version>
<workflow-support-plugin.version>2.21</workflow-support-plugin.version>
<workflow-api-plugin.version>2.30</workflow-api-plugin.version>
<useBeta>true</useBeta>
<git-plugin.version>4.0.0-beta3</git-plugin.version>
<scm-api-plugin.version>2.2.6</scm-api-plugin.version>
</properties>
<dependencies>
<dependency>
Expand Down Expand Up @@ -98,7 +100,7 @@
<dependency>
<groupId>org.jenkins-ci.plugins</groupId>
<artifactId>structs</artifactId>
<version>1.14</version>
<version>1.17</version>
</dependency>
<dependency>
<groupId>org.jenkins-ci.plugins</groupId>
Expand All @@ -121,13 +123,13 @@
<dependency>
<groupId>org.jenkins-ci.plugins.workflow</groupId>
<artifactId>workflow-job</artifactId>
<version>2.10</version>
<version>2.26</version>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.jenkins-ci.plugins.workflow</groupId>
<artifactId>workflow-durable-task-step</artifactId>
<version>2.3</version>
<version>2.24</version>
Copy link
Member Author

@jglick jglick Oct 24, 2018

Choose a reason for hiding this comment

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

FTR this was to pick up the fix of JENKINS-28822.

<scope>test</scope>
</dependency>
<dependency>
Expand Down Expand Up @@ -182,7 +184,7 @@
<dependency>
<groupId>org.jenkins-ci.plugins</groupId>
<artifactId>script-security</artifactId>
<version>1.28</version>
<version>1.46</version>
<scope>test</scope>
</dependency>
<dependency>
Expand All @@ -204,5 +206,34 @@
<classifier>tests</classifier>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.jenkins-ci.plugins</groupId>
<artifactId>git</artifactId>
<version>${git-plugin.version}</version>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.jenkins-ci.plugins</groupId>
<artifactId>scm-api</artifactId>
<version>${scm-api-plugin.version}</version>
<classifier>tests</classifier>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.jenkins-ci.plugins</groupId>
<artifactId>git</artifactId>
<version>${git-plugin.version}</version>
<classifier>tests</classifier>
<scope>test</scope>
</dependency>
</dependencies>
<dependencyManagement>
<dependencies>
<dependency>
<groupId>org.jenkins-ci.plugins</groupId>
<artifactId>scm-api</artifactId>
<version>${scm-api-plugin.version}</version>
</dependency>
</dependencies>
</dependencyManagement>
</project>
137 changes: 134 additions & 3 deletions src/main/java/org/jenkinsci/plugins/workflow/steps/TimeoutStepExecution.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,17 +10,25 @@
import hudson.model.Result;
import hudson.model.Run;
import hudson.model.TaskListener;
import hudson.remoting.Callable;
import hudson.remoting.Channel;
import java.io.IOException;
import java.io.OutputStream;
import java.io.Serializable;
import java.lang.ref.Reference;
import java.lang.ref.WeakReference;
import java.util.List;
import java.util.UUID;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.ScheduledFuture;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.logging.Level;
import java.util.logging.Logger;
import javax.annotation.CheckForNull;
import javax.annotation.Nonnull;
import jenkins.model.CauseOfInterruption;
import jenkins.security.SlaveToMasterCallable;
import jenkins.util.Timer;
import org.jenkinsci.plugins.workflow.flow.FlowExecution;
import org.jenkinsci.plugins.workflow.graph.FlowNode;
Expand All @@ -37,7 +45,7 @@ public class TimeoutStepExecution extends AbstractStepExecutionImpl {
private BodyExecution body;
private transient ScheduledFuture<?> killer;

private long timeout = 0;
private final long timeout;
private long end = 0;

/** Used to track whether this is timing out on inactivity without needing to reference {@link #step}. */
Expand All @@ -46,10 +54,15 @@ public class TimeoutStepExecution extends AbstractStepExecutionImpl {
/** whether we are forcing the body to end */
private boolean forcible;

/** Token for {@link #activity} callbacks. */
private final String id;

TimeoutStepExecution(TimeoutStep step, StepContext context) {
super(context);
this.step = step;
this.activity = step.isActivity();
id = activity ? UUID.randomUUID().toString() : null;
timeout = step.getUnit().toMillis(step.getTime());
}

@Override
Expand All @@ -62,13 +75,12 @@ public boolean start() throws Exception {
bodyInvoker = bodyInvoker.withContext(
BodyInvoker.mergeConsoleLogFilters(
context.get(ConsoleLogFilter.class),
new ConsoleLogFilterImpl(new ResetCallbackImpl())
new ConsoleLogFilterImpl2(id, timeout)
)
);
}

body = bodyInvoker.start();
timeout = step.getUnit().toMillis(step.getTime());
resetTimer();
return false; // execution is asynchronous
}
Expand Down Expand Up @@ -230,17 +242,136 @@ public String getShortDescription() {
}
}

private static final class ResetTimer extends SlaveToMasterCallable<Void, RuntimeException> {

private static final long serialVersionUID = 1L;

private final @Nonnull String id;

ResetTimer(String id) {
this.id = id;
}

@Override public Void call() throws RuntimeException {
StepExecution.applyAll(TimeoutStepExecution.class, e -> {
if (id.equals(e.id)) {
e.resetTimer();
}
return null;
});
return null;
}

}

private static class ConsoleLogFilterImpl2 extends ConsoleLogFilter implements /* TODO Remotable */ Serializable {
private static final long serialVersionUID = 1L;

private final @Nonnull String id;
private final long timeout;
private transient @CheckForNull Channel channel;

ConsoleLogFilterImpl2(String id, long timeout) {
this.id = id;
this.timeout = timeout;
}

private Object readResolve() {
channel = Channel.current();
return this;
}

@Override
public OutputStream decorateLogger(@SuppressWarnings("rawtypes") Run build, final OutputStream logger)
throws IOException, InterruptedException {
// TODO if channel == null, we can safely ResetTimer.call synchronously from eol and skip the Tick
AtomicBoolean active = new AtomicBoolean();
OutputStream decorated = new LineTransformationOutputStream() {
@Override
protected void eol(byte[] b, int len) throws IOException {
logger.write(b, 0, len);
active.set(true);
}

@Override
public void flush() throws IOException {
super.flush();
logger.flush();
}

@Override
public void close() throws IOException {
super.close();
logger.close();
}
};
new Tick(active, new WeakReference<>(decorated), timeout, channel, id).schedule();
return decorated;
}
}

private static final class Tick implements Runnable {
private final AtomicBoolean active;
private final Reference<?> stream;
private final long timeout;
private final @CheckForNull Channel channel;
private final @Nonnull String id;
Tick(AtomicBoolean active, Reference<?> stream, long timeout, Channel channel, String id) {
this.active = active;
this.stream = stream;
this.timeout = timeout;
this.channel = channel;
this.id = id;
}
@Override
public void run() {
if (stream.get() == null) {
// Not only idle but gone—stop the timer.
return;
}
boolean currentlyActive = active.getAndSet(false);
if (currentlyActive) {
Callable<Void, RuntimeException> resetTimer = new ResetTimer(id);
if (channel != null) {
try {
channel.call(resetTimer);
} catch (Exception x) {
LOGGER.log(Level.WARNING, null, x);
}
} else {
resetTimer.call();
}
schedule();
} else {
// Idle at the moment, but check well before the timeout expires in case new output appears.
schedule(timeout / 10);
}
}
void schedule() {
schedule(timeout / 2); // less than the full timeout, to give some grace period, but in the same ballpark to avoid overhead
}
private void schedule(long delay) {
Timer.get().schedule(this, delay, TimeUnit.MILLISECONDS);
}
}

/** @deprecated only here for serial compatibility */
@Deprecated
public interface ResetCallback extends Serializable {
void logWritten();
}

/** @deprecated only here for serial compatibility */
@Deprecated
private class ResetCallbackImpl implements ResetCallback {
private static final long serialVersionUID = 1L;
@Override public void logWritten() {
resetTimer();
}
}

/** @deprecated only here for serial compatibility */
@Deprecated
private static class ConsoleLogFilterImpl extends ConsoleLogFilter implements /* TODO Remotable */ Serializable {
private static final long serialVersionUID = 1L;

Expand Down
8 changes: 4 additions & 4 deletions src/test/java/org/jenkinsci/plugins/workflow/steps/EnvStepTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,18 +90,18 @@ public class EnvStepTest {
WorkflowJob p = story.j.jenkins.createProject(WorkflowJob.class, "p");
p.setDefinition(new CpsFlowDefinition(
"parallel a: {\n" +
" node {withEnv(['TOOL=aloc']) {semaphore 'a'; isUnix() ? sh('echo TOOL=$TOOL') : bat('echo TOOL=%TOOL%')}}\n" +
" node {withEnv(['TOOL=aloc']) {semaphore 'a'; isUnix() ? sh('echo a TOOL=$TOOL') : bat('echo a TOOL=%TOOL%')}}\n" +
"}, b: {\n" +
" node {withEnv(['TOOL=bloc']) {semaphore 'b'; isUnix() ? sh('echo TOOL=$TOOL') : bat('echo TOOL=%TOOL%')}}\n" +
" node {withEnv(['TOOL=bloc']) {semaphore 'b'; isUnix() ? sh('echo b TOOL=$TOOL') : bat('echo b TOOL=%TOOL%')}}\n" +
"}", true));
WorkflowRun b = p.scheduleBuild2(0).getStartCondition().get();
SemaphoreStep.waitForStart("a/1", b);
SemaphoreStep.waitForStart("b/1", b);
SemaphoreStep.success("a/1", null);
SemaphoreStep.success("b/1", null);
story.j.assertBuildStatusSuccess(story.j.waitForCompletion(b));
story.j.assertLogContains("[a] TOOL=aloc", b);
story.j.assertLogContains("[b] TOOL=bloc", b);
story.j.assertLogContains("a TOOL=aloc", b);
story.j.assertLogContains("b TOOL=bloc", b);
}
});
}
Expand Down
41 changes: 41 additions & 0 deletions src/test/java/org/jenkinsci/plugins/workflow/steps/TimeoutStepTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@

package org.jenkinsci.plugins.workflow.steps;

import hudson.Functions;
import hudson.model.Result;
import hudson.model.TaskListener;
import hudson.model.listeners.RunListener;
Expand All @@ -33,6 +34,7 @@
import java.util.concurrent.TimeUnit;
import jenkins.model.CauseOfInterruption;
import jenkins.model.InterruptedBuildAction;
import jenkins.plugins.git.GitSampleRepoRule;
import org.jenkinsci.plugins.workflow.actions.ErrorAction;
import org.jenkinsci.plugins.workflow.cps.CpsFlowDefinition;
import org.jenkinsci.plugins.workflow.cps.nodes.StepAtomNode;
Expand All @@ -43,6 +45,7 @@
import org.jenkinsci.plugins.workflow.test.steps.SemaphoreStep;
import org.junit.*;
import static org.junit.Assert.assertEquals;
import static org.junit.Assume.*;
import org.junit.runners.model.Statement;
import org.jvnet.hudson.test.BuildWatcher;
import org.jvnet.hudson.test.Issue;
Expand All @@ -57,6 +60,8 @@ public class TimeoutStepTest extends Assert {

@Rule public RestartableJenkinsRule story = new RestartableJenkinsRule();

@Rule public GitSampleRepoRule git = new GitSampleRepoRule();

@Test public void configRoundTrip() {
story.addStep(new Statement() {
@Override public void evaluate() throws Throwable {
Expand Down Expand Up @@ -239,6 +244,42 @@ public void evaluate() throws Throwable {
});
}

@Test
public void activityRemote() {
assumeFalse(Functions.isWindows()); // TODO create analogue using bat
story.then(r -> {
r.createSlave();
WorkflowJob p = r.jenkins.createProject(WorkflowJob.class, "p");
p.setDefinition(new CpsFlowDefinition("" +
"node('!master') {\n" +
" timeout(time:5, unit:'SECONDS', activity: true) {\n" +
" sh 'set +x; echo NotHere; sleep 3; echo NotHereYet; sleep 3; echo JustHere; sleep 10; echo ShouldNot'\n" +
Copy link
Member Author

@jglick jglick Oct 8, 2018

Choose a reason for hiding this comment

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

Cannot use echo steps as in the other tests, because we need to demonstrate serialization of the ConsoleLogFilter impl, which only happens if a nested step actually remotes its TaskListener.

" }\n" +
"}\n", true));
WorkflowRun b = r.assertBuildStatus(Result.ABORTED, p.scheduleBuild2(0));
story.j.assertLogContains("JustHere", b);
story.j.assertLogNotContains("ShouldNot", b);
});
}

@Issue("JENKINS-54078")
@Test public void activityGit() {
story.then(r -> {
r.createSlave();
git.init();
git.write("file", "content");
git.git("commit", "--all", "--message=init");
WorkflowJob p = r.jenkins.createProject(WorkflowJob.class, "p");
p.setDefinition(new CpsFlowDefinition("" +
"node('!master') {\n" +
" timeout(time: 5, unit: 'MINUTES', activity: true) {\n" +
" git($/" + git + "/$)\n" +
" }\n" +
"}\n", true));
r.buildAndAssertSuccess(p);
});
}

@Issue("JENKINS-26163")
@Test
public void restarted() throws Exception {
Expand Down