Skip to content
Permalink
Browse files

[JENKINS-21316] PrioritySorter wrongly assumes Queue.Task is Job

 * Updated fix provided for [JENKINS-21310] to exit early
 * Updated javadoc to clearify responsibilities
  • Loading branch information...
emsa23 committed Jan 10, 2014
1 parent 9c3fc00 commit 5b6e7c68d3cde206608e7bfc4f5b975470af64f7
@@ -71,8 +71,7 @@
* @since 2.0
*/
@Extension
public class PriorityConfiguration extends Descriptor<PriorityConfiguration> implements RootAction,
Describable<PriorityConfiguration> {
public class PriorityConfiguration extends Descriptor<PriorityConfiguration> implements RootAction, Describable<PriorityConfiguration> {

private final static Logger LOGGER = Logger.getLogger(PriorityConfiguration.class.getName());

@@ -119,13 +118,13 @@ public String getUrlName() {
}
return "advanced-build-queue";
}

private boolean checkActive() {
PrioritySorterConfiguration configuration = PrioritySorterConfiguration.get();
if(configuration.getLegacyMode()) {
if (configuration.getLegacyMode()) {
return false;
}
if(configuration.getOnlyAdminsMayEditPriorityConfiguration()) {
if (configuration.getOnlyAdminsMayEditPriorityConfiguration()) {
return Jenkins.getInstance().getACL().hasPermission(Jenkins.ADMINISTER);
}
return true;
@@ -192,7 +191,7 @@ public FormValidation doCheckJobPattern(@QueryParameter String value) throws IOE
}
return FormValidation.ok();
}

public PriorityConfigurationCallback getPriority(Queue.Item item, PriorityConfigurationCallback priorityCallback) {
SecurityContext saveCtx = ACL.impersonate(ACL.SYSTEM);
try {
@@ -201,32 +200,38 @@ public PriorityConfigurationCallback getPriority(Queue.Item item, PriorityConfig
SecurityContextHolder.setContext(saveCtx);
}
}

private PriorityConfigurationCallback getPriorityInternal(Queue.Item item,
PriorityConfigurationCallback priorityCallback) {
Queue.Task job = item.task;

private PriorityConfigurationCallback getPriorityInternal(Queue.Item item, PriorityConfigurationCallback priorityCallback) {

if (!(item.task instanceof Job)) {
// Not a job generally this mean that this is a lightweight task so
// priority doesn't really matter - returning default priority
priorityCallback.addDecisionLog(0, "Queue.Item is not a Job - Assigning global default priority");
return priorityCallback.setPrioritySelection(PrioritySorterConfiguration.get().getStrategy().getDefaultPriority());
}

Job<?, ?> job = (Job<?, ?>) item.task;

// [JENKINS-8597]
// For MatrixConfiguration use the latest assigned Priority from the MatrixProject
// For MatrixConfiguration use the latest assigned Priority from the
// MatrixProject
if (job instanceof MatrixConfiguration) {
MatrixProject matrixProject = ((MatrixConfiguration) job).getParent();
priorityCallback.addDecisionLog(0, "Job is MatrixConfiguration [" + matrixProject.getName() + "] ...");
ItemInfo itemInfo = QueueItemCache.get().getItem(matrixProject.getName());
// Can be null (for example) at startup when the MatrixBuild got lost (was running at
// Can be null (for example) at startup when the MatrixBuild got
// lost (was running at
// restart)
if (itemInfo != null) {
priorityCallback.addDecisionLog(0, "MatrixProject found in cache, using priority from queue-item ["
+ itemInfo.getItemId() + "]");
return priorityCallback.setPrioritySelection(itemInfo.getPriority(), itemInfo.getJobGroupId(),
itemInfo.getPriorityStrategy());
priorityCallback.addDecisionLog(0, "MatrixProject found in cache, using priority from queue-item [" + itemInfo.getItemId() + "]");
return priorityCallback.setPrioritySelection(itemInfo.getPriority(), itemInfo.getJobGroupId(), itemInfo.getPriorityStrategy());
}
priorityCallback.addDecisionLog(0, "MatrixProject not found in cache, assigning global default priority");
return priorityCallback.setPrioritySelection(PrioritySorterConfiguration.get().getStrategy()
.getDefaultPriority());
return priorityCallback.setPrioritySelection(PrioritySorterConfiguration.get().getStrategy().getDefaultPriority());
}

if (job instanceof Job && PrioritySorterConfiguration.get().getAllowPriorityOnJobs()) {
AdvancedQueueSorterJobProperty priorityProperty = ((Job<?,?>) job).getProperty(AdvancedQueueSorterJobProperty.class);
if (PrioritySorterConfiguration.get().getAllowPriorityOnJobs()) {
AdvancedQueueSorterJobProperty priorityProperty = ((Job<?, ?>) job).getProperty(AdvancedQueueSorterJobProperty.class);
if (priorityProperty != null && priorityProperty.getUseJobPriority()) {
int priority = priorityProperty.priority;
if (priority == PriorityCalculationsUtil.getUseDefaultPriorityPriority()) {
@@ -243,11 +248,11 @@ private PriorityConfigurationCallback getPriorityInternal(Queue.Item item,
}
//
priorityCallback.addDecisionLog(0, "Assigning global default priority");
return priorityCallback.setPrioritySelection(PrioritySorterConfiguration.get().getStrategy()
.getDefaultPriority());
return priorityCallback.setPrioritySelection(PrioritySorterConfiguration.get().getStrategy().getDefaultPriority());
}

// TODO a simple jobName will not work for jobs in folders, and would be meaningless for non-Job Queue.Task’s
// TODO a simple jobName will not work for jobs in folders, and would be
// meaningless for non-Job Queue.Task’s
public JobGroup getJobGroup(PriorityConfigurationCallback priorityCallback, String jobName) {
for (JobGroup jobGroup : jobGroups) {
priorityCallback.addDecisionLog(0, "Evaluating JobGroup [" + jobGroup.getId() + "] ...");
@@ -291,22 +296,19 @@ public JobGroup getJobGroup(PriorityConfigurationCallback priorityCallback, Stri
return null;
}

private PriorityConfigurationCallback getPriorityForJobGroup(PriorityConfigurationCallback priorityCallback,
JobGroup jobGroup, Queue.Item item) {
private PriorityConfigurationCallback getPriorityForJobGroup(PriorityConfigurationCallback priorityCallback, JobGroup jobGroup, Queue.Item item) {
int priority = jobGroup.getPriority();
PriorityStrategy reason = null;
if (jobGroup.isUsePriorityStrategies()) {
priorityCallback.addDecisionLog(2, "Evaluating strategies ...");
List<JobGroup.PriorityStrategyHolder> priorityStrategies = jobGroup.getPriorityStrategies();
for (JobGroup.PriorityStrategyHolder priorityStrategy : priorityStrategies) {
PriorityStrategy strategy = priorityStrategy.getPriorityStrategy();
priorityCallback.addDecisionLog(3, "Evaluating strategy ["
+ strategy.getDescriptor().getDisplayName() + "] ...");
priorityCallback.addDecisionLog(3, "Evaluating strategy [" + strategy.getDescriptor().getDisplayName() + "] ...");
if (strategy.isApplicable(item)) {
priorityCallback.addDecisionLog(4, "Strategy is applicable");
int foundPriority = strategy.getPriority(item);
if (foundPriority > 0
&& foundPriority <= PrioritySorterConfiguration.get().getStrategy().getNumberOfPriorities()) {
if (foundPriority > 0 && foundPriority <= PrioritySorterConfiguration.get().getStrategy().getNumberOfPriorities()) {
priority = foundPriority;
reason = strategy;
break;
@@ -27,7 +27,9 @@
import hudson.ExtensionPoint;
import hudson.model.Describable;
import hudson.model.Descriptor;
import hudson.model.Job;
import hudson.model.Queue;
import hudson.model.Queue.Item;
import jenkins.model.Jenkins;

/**
@@ -36,8 +38,25 @@
*/
public abstract class PriorityStrategy implements ExtensionPoint, Describable<PriorityStrategy> {

/**
* Method that checks if strategy can assign a priority to the provided {@link Item}
*
* The caller garanties that the {@link Item#task} is a {@link Job}
*
* @param item the {@link Item} to check
* @return <code>true</code> if the {@link PriorityStrategy} is applicable else <code>false</code>
*/
abstract public boolean isApplicable(Queue.Item item);

/**
* Method that that return the priority that should be used for this {@link Item}, this method is only called id
* {@link PriorityStrategy#isApplicable(Item)} returned true
*
* The caller garanties that the {@link Item#task} is a {@link Job}
*
* @param item the {@link Item} to check
* @return the priority to be used by the provided {@link Item}
*/
abstract public int getPriority(Queue.Item item);

abstract public void numberPrioritiesUpdates(int oldNumberOfPriorities, int newNumberOfPriorities);

0 comments on commit 5b6e7c6

Please sign in to comment.
You can’t perform that action at this time.