Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
[JENKINS-38514] Retain CauseOfBlockage from JobOffer (#2651)
* Converted to JenkinsRule.

* Improved messages from Node.canTake.

* [FIXED JENKINS-38514] BuildableItem needs to retain information from JobOffer about why it is neither blocked nor building.

* Converted to JenkinsRule.

* Found an existing usage of BecauseNodeIsNotAcceptingTasks.

* Original JENKINS-6598 test was checking behavior we want amended by JENKINS-38514.

* Ensure that a BuildableItem which is simply waiting for a free executor reports that as its CauseOfBlockage.

* Review comments from @oleg-nenashev.
  • Loading branch information
jglick authored and oleg-nenashev committed Dec 9, 2016
1 parent 9172bca commit 8d23041
Show file tree
Hide file tree
Showing 8 changed files with 306 additions and 59 deletions.
8 changes: 4 additions & 4 deletions core/src/main/java/hudson/model/Node.java
Expand Up @@ -372,7 +372,7 @@ public CauseOfBlockage canTake(Task task) {
public CauseOfBlockage canTake(Queue.BuildableItem item) {
Label l = item.getAssignedLabel();
if(l!=null && !l.contains(this))
return CauseOfBlockage.fromMessage(Messages._Node_LabelMissing(getNodeName(),l)); // the task needs to be executed on label that this node doesn't have.
return CauseOfBlockage.fromMessage(Messages._Node_LabelMissing(getDisplayName(), l)); // the task needs to be executed on label that this node doesn't have.

if(l==null && getMode()== Mode.EXCLUSIVE) {
// flyweight tasks need to get executed somewhere, if every node
Expand All @@ -381,14 +381,14 @@ public CauseOfBlockage canTake(Queue.BuildableItem item) {
|| Jenkins.getInstance().getNumExecutors() < 1
|| Jenkins.getInstance().getMode() == Mode.EXCLUSIVE)
)) {
return CauseOfBlockage.fromMessage(Messages._Node_BecauseNodeIsReserved(getNodeName())); // this node is reserved for tasks that are tied to it
return CauseOfBlockage.fromMessage(Messages._Node_BecauseNodeIsReserved(getDisplayName())); // this node is reserved for tasks that are tied to it
}
}

Authentication identity = item.authenticate();
if (!getACL().hasPermission(identity,Computer.BUILD)) {
// doesn't have a permission
return CauseOfBlockage.fromMessage(Messages._Node_LackingBuildPermission(identity.getName(),getNodeName()));
return CauseOfBlockage.fromMessage(Messages._Node_LackingBuildPermission(identity.getName(), getDisplayName()));
}

// Check each NodeProperty to see whether they object to this node
Expand All @@ -399,7 +399,7 @@ public CauseOfBlockage canTake(Queue.BuildableItem item) {
}

if (!isAcceptingTasks()) {
return CauseOfBlockage.fromMessage(Messages._Node_BecauseNodeIsNotAcceptingTasks(getNodeName()));
return new CauseOfBlockage.BecauseNodeIsNotAcceptingTasks(this);
}

// Looks like we can take the task
Expand Down
76 changes: 60 additions & 16 deletions core/src/main/java/hudson/model/Queue.java
Expand Up @@ -61,6 +61,7 @@
import hudson.model.queue.CauseOfBlockage.BecauseLabelIsOffline;
import hudson.model.queue.CauseOfBlockage.BecauseNodeIsBusy;
import hudson.model.queue.WorkUnitContext;
import hudson.security.ACL;
import hudson.security.AccessControlled;
import jenkins.security.QueueItemAuthenticatorProvider;
import jenkins.util.Timer;
Expand Down Expand Up @@ -122,10 +123,10 @@
import com.thoughtworks.xstream.XStream;
import com.thoughtworks.xstream.converters.basic.AbstractSingleValueConverter;
import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
import jenkins.util.SystemProperties;
import javax.annotation.CheckForNull;
import javax.annotation.Nonnegative;
import jenkins.model.queue.AsynchronousExecution;
import jenkins.model.queue.CompositeCauseOfBlockage;
import org.kohsuke.stapler.QueryParameter;
import org.kohsuke.stapler.interceptor.RequirePOST;

Expand Down Expand Up @@ -219,7 +220,7 @@ public class Queue extends ResourceController implements Saveable {
* to assign a work. Once a work is assigned, the executor actually gets
* started to carry out the task in question.
*/
public class JobOffer extends MappingWorksheet.ExecutorSlot {
public static class JobOffer extends MappingWorksheet.ExecutorSlot {
public final Executor executor;

/**
Expand All @@ -246,20 +247,44 @@ public Executor getExecutor() {
}

/**
* Verifies that the {@link Executor} represented by this object is capable of executing the given task.
* @deprecated discards information; prefer {@link #getCauseOfBlockage}
*/
@Deprecated
public boolean canTake(BuildableItem item) {
Node node = getNode();
if (node==null) return false; // this executor is about to die

if(node.canTake(item)!=null)
return false; // this node is not able to take the task

for (QueueTaskDispatcher d : QueueTaskDispatcher.all())
if (d.canTake(node,item)!=null)
return false;
return getCauseOfBlockage(item) == null;
}

return isAvailable();
/**
* Checks whether the {@link Executor} represented by this object is capable of executing the given task.
* @return a reason why it cannot, or null if it could
* @since FIXME
*/
public @CheckForNull CauseOfBlockage getCauseOfBlockage(BuildableItem item) {
Node node = getNode();
if (node == null) {
return CauseOfBlockage.fromMessage(Messages._Queue_node_has_been_removed_from_configuration(executor.getOwner().getDisplayName()));
}
CauseOfBlockage reason = node.canTake(item);
if (reason != null) {
return reason;
}
for (QueueTaskDispatcher d : QueueTaskDispatcher.all()) {
reason = d.canTake(node, item);
if (reason != null) {
return reason;
}
}
// inlining isAvailable:
if (workUnit != null) { // unlikely in practice (should not have even found this executor if so)
return CauseOfBlockage.fromMessage(Messages._Queue_executor_slot_already_in_use());
}
if (executor.getOwner().isOffline()) {
return new CauseOfBlockage.BecauseNodeIsOffline(node);
}
if (!executor.getOwner().isAcceptingTasks()) { // Node.canTake (above) does not consider RetentionStrategy.isAcceptingTasks
return new CauseOfBlockage.BecauseNodeIsNotAcceptingTasks(node);
}
return null;
}

/**
Expand Down Expand Up @@ -1521,13 +1546,18 @@ public void maintain() {
}
} else {

List<JobOffer> candidates = new ArrayList<JobOffer>(parked.size());
List<JobOffer> candidates = new ArrayList<>(parked.size());
List<CauseOfBlockage> reasons = new ArrayList<>(parked.size());
for (JobOffer j : parked.values()) {
if (j.canTake(p)) {
CauseOfBlockage reason = j.getCauseOfBlockage(p);
if (reason == null) {
LOGGER.log(Level.FINEST,
"{0} is a potential candidate for task {1}",
new Object[]{j.executor.getDisplayName(), taskDisplayName});
new Object[]{j, taskDisplayName});
candidates.add(j);
} else {
LOGGER.log(Level.FINEST, "{0} rejected {1}: {2}", new Object[] {j, taskDisplayName, reason});
reasons.add(reason);
}
}

Expand All @@ -1539,6 +1569,7 @@ public void maintain() {
// check if we can execute other projects
LOGGER.log(Level.FINER, "Failed to map {0} to executors. candidates={1} parked={2}",
new Object[]{p, candidates, parked.values()});
p.transientCausesOfBlockage = reasons.isEmpty() ? null : reasons;
continue;
}

Expand Down Expand Up @@ -2465,6 +2496,12 @@ public final static class BuildableItem extends NotWaitingItem {
*/
private boolean isPending;

/**
* Reasons why the last call to {@link #maintain} left this buildable (but not blocked or executing).
* May be null but not empty.
*/
private transient volatile @CheckForNull List<CauseOfBlockage> transientCausesOfBlockage;

public BuildableItem(WaitingItem wi) {
super(wi);
}
Expand All @@ -2478,6 +2515,8 @@ public CauseOfBlockage getCauseOfBlockage() {
if(isBlockedByShutdown(task))
return CauseOfBlockage.fromMessage(Messages._Queue_HudsonIsAboutToShutDown());

List<CauseOfBlockage> causesOfBlockage = transientCausesOfBlockage;

Label label = getAssignedLabel();
List<Node> allNodes = jenkins.getNodes();
if (allNodes.isEmpty())
Expand All @@ -2489,9 +2528,14 @@ public CauseOfBlockage getCauseOfBlockage() {
if (nodes.size() != 1) return new BecauseLabelIsOffline(label);
else return new BecauseNodeIsOffline(nodes.iterator().next());
} else {
if (causesOfBlockage != null && label.getIdleExecutors() > 0) {
return new CompositeCauseOfBlockage(causesOfBlockage);
}
if (nodes.size() != 1) return new BecauseLabelIsBusy(label);
else return new BecauseNodeIsBusy(nodes.iterator().next());
}
} else if (causesOfBlockage != null && new ComputerSet().getIdleExecutors() > 0) {
return new CompositeCauseOfBlockage(causesOfBlockage);
} else {
return CauseOfBlockage.createNeedsMoreExecutor(Messages._Queue_WaitingForNextAvailableExecutor());
}
Expand Down
33 changes: 32 additions & 1 deletion core/src/main/java/hudson/model/queue/CauseOfBlockage.java
@@ -1,12 +1,14 @@
package hudson.model.queue;

import hudson.console.ModelHyperlinkNote;
import hudson.model.Computer;
import hudson.model.Queue.Task;
import hudson.model.Node;
import hudson.model.Messages;
import hudson.model.Label;
import hudson.model.TaskListener;
import hudson.slaves.Cloud;
import javax.annotation.Nonnull;
import org.jvnet.localizer.Localizable;

/**
Expand Down Expand Up @@ -42,8 +44,10 @@ public void print(TaskListener listener) {
/**
* Obtains a simple implementation backed by {@link Localizable}.
*/
public static CauseOfBlockage fromMessage(final Localizable l) {
public static CauseOfBlockage fromMessage(@Nonnull final Localizable l) {
l.getKey(); // null check
return new CauseOfBlockage() {
@Override
public String getShortDescription() {
return l.toString();
}
Expand Down Expand Up @@ -103,6 +107,33 @@ public void print(TaskListener listener) {
}
}

/**
* Build is blocked because a node (or its retention strategy) is not accepting tasks.
* @since FIXME
*/
public static final class BecauseNodeIsNotAcceptingTasks extends CauseOfBlockage implements NeedsMoreExecutor {

public final Node node;

public BecauseNodeIsNotAcceptingTasks(Node node) {
this.node = node;
}

@Override
public String getShortDescription() {
Computer computer = node.toComputer();
String name = computer != null ? computer.getDisplayName() : node.getDisplayName();
return Messages.Node_BecauseNodeIsNotAcceptingTasks(name);
}

@Override
public void print(TaskListener listener) {
listener.getLogger().println(
Messages.Node_BecauseNodeIsNotAcceptingTasks(ModelHyperlinkNote.encodeTo(node)));
}

}

/**
* Build is blocked because all the nodes that match a given label is offline.
*/
Expand Down
@@ -0,0 +1,63 @@
/*
* 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 jenkins.model.queue;

import hudson.model.TaskListener;
import hudson.model.queue.CauseOfBlockage;
import java.util.List;
import java.util.Map;
import java.util.TreeMap;
import org.apache.commons.lang.StringUtils;
import org.kohsuke.accmod.Restricted;
import org.kohsuke.accmod.restrictions.NoExternalUse;

/**
* Represents the fact that there was at least one {@link hudson.model.Queue.JobOffer} which rejected a task.
*/
@Restricted(NoExternalUse.class)
public class CompositeCauseOfBlockage extends CauseOfBlockage {

public final Map<String, CauseOfBlockage> uniqueReasons;

public CompositeCauseOfBlockage(List<CauseOfBlockage> delegates) {
uniqueReasons = new TreeMap<>();
for (CauseOfBlockage delegate : delegates) {
uniqueReasons.put(delegate.getShortDescription(), delegate);
}
}

@Override
public String getShortDescription() {
return StringUtils.join(uniqueReasons.keySet(), "; ");
}

@Override
public void print(TaskListener listener) {
for (CauseOfBlockage delegate : uniqueReasons.values()) {
delegate.print(listener);
}
}

}
4 changes: 3 additions & 1 deletion core/src/main/resources/hudson/model/Messages.properties
Expand Up @@ -194,6 +194,8 @@ Queue.Unknown=???
Queue.WaitingForNextAvailableExecutor=Waiting for next available executor
Queue.WaitingForNextAvailableExecutorOn=Waiting for next available executor on {0}
Queue.init=Restoring the build queue
Queue.node_has_been_removed_from_configuration={0} has been removed from configuration
Queue.executor_slot_already_in_use=Executor slot already in use

ResultTrend.Aborted=Aborted
ResultTrend.Failure=Failure
Expand Down Expand Up @@ -335,7 +337,7 @@ PasswordParameterDefinition.DisplayName=Password Parameter
Node.BecauseNodeIsReserved={0} is reserved for jobs with matching label expression
Node.BecauseNodeIsNotAcceptingTasks={0} is not accepting tasks
Node.LabelMissing={0} doesn\u2019t have label {1}
Node.LackingBuildPermission={0} doesn\u2019t have a permission to run on {1}
Node.LackingBuildPermission={0} lacks permission to run on {1}
Node.Mode.NORMAL=Use this node as much as possible
Node.Mode.EXCLUSIVE=Only build jobs with label expressions matching this node

Expand Down
@@ -0,0 +1,32 @@
<?xml version="1.0" encoding="UTF-8"?>
<!--
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.
-->

<?jelly escape-by-default='true'?>
<j:jelly xmlns:j="jelly:core" xmlns:st="jelly:stapler">
<j:forEach var="delegate" items="${it.uniqueReasons.values()}" indexVar="index">
<j:if test="${index > 0}">; <wbr/></j:if>
<st:include it="${delegate}" page="summary"/>
</j:forEach>
</j:jelly>

0 comments on commit 8d23041

Please sign in to comment.