Skip to content

Commit

Permalink
Improve lockable-resources build (job) page (#673)
Browse files Browse the repository at this point in the history 
* NPE during the release of a lock
* Show LRM actions on build page
* logs
  • Loading branch information
@mPokornyETM
mPokornyETM authored Sep 17, 2024
1 parent c0a40b7 commit e218743
Show file tree
Hide file tree
Showing 11 changed files with 215 additions and 210 deletions.
42 changes: 26 additions & 16 deletions src/main/java/org/jenkins/plugins/lockableresources/LockStepExecution.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,25 +41,28 @@ public LockStepExecution(LockStep step, StepContext context) {

@Override
public boolean start() throws Exception {
step.validate();

// normally it might raise a exception, but we check it in the function .validate()
// therefore we can skip the try-catch here.
ResourceSelectStrategy resourceSelectStrategy =
ResourceSelectStrategy.valueOf(step.resourceSelectStrategy.toUpperCase(Locale.ENGLISH));

getContext().get(FlowNode.class).addAction(new PauseAction("Lock"));
PrintStream logger = getContext().get(TaskListener.class).getLogger();

Run<?, ?> run = getContext().get(Run.class);
LockableResourcesManager.printLogs("Trying to acquire lock on [" + step + "]", Level.FINE, LOGGER, logger);

List<LockableResourcesStruct> resourceHolderList = new ArrayList<>();

LockableResourcesManager lrm = LockableResourcesManager.get();
List<LockableResource> available = null;
LinkedHashMap<String, List<LockableResourceProperty>> resourceNames = new LinkedHashMap<>();
LinkedHashMap<String, List<LockableResourceProperty>> lockedResources = new LinkedHashMap<>();
LockableResourcesManager lrm = LockableResourcesManager.get();
synchronized (lrm.syncResources) {
step.validate();

LockableResourcesManager.printLogs("Trying to acquire lock on [" + step + "]", Level.FINE, LOGGER, logger);

getContext().get(FlowNode.class).addAction(new PauseAction("Lock"));

List<String> resourceNames = new ArrayList<>();
for (LockStepResource resource : step.getResources()) {
List<String> resources = new ArrayList<>();
if (resource.resource != null) {
Expand All @@ -71,10 +74,15 @@ public boolean start() throws Exception {
logger);
}
resources.add(resource.resource);
resourceNames.addAll(resources);
} else {
resourceNames.add("N/A");
}
resourceHolderList.add(new LockableResourcesStruct(resources, resource.label, resource.quantity));
}

LockedResourcesBuildAction.addLog(run, resourceNames, "try", step.toString());

// determine if there are enough resources available to proceed
available = lrm.getAvailableResources(resourceHolderList, logger, resourceSelectStrategy);
if (available == null || available.isEmpty()) {
Expand All @@ -95,10 +103,10 @@ public boolean start() throws Exception {
// since LockableResource contains transient variables, they cannot be correctly serialized
// hence we use their unique resource names and properties
for (LockableResource resource : available) {
resourceNames.put(resource.getName(), resource.getProperties());
lockedResources.put(resource.getName(), resource.getProperties());
}
LockStepExecution.proceed(lockedResources, getContext(), step.toString(), step.variable);
}
LockStepExecution.proceed(resourceNames, getContext(), step.toString(), step.variable);

return false;
}
Expand Down Expand Up @@ -166,11 +174,12 @@ public static void proceed(
}

try {

LockedResourcesBuildAction.updateAction(build, new ArrayList<>(lockedResources.keySet()));
List<String> resourceNames = new ArrayList<>(lockedResources.keySet());
final String resourceNamesAsString = String.join(",", lockedResources.keySet());
LockedResourcesBuildAction.addLog(build, resourceNames, "acquired", resourceDescription);
PauseAction.endCurrentPause(node);
BodyInvoker bodyInvoker = context.newBodyInvoker()
.withCallback(new Callback(new ArrayList<>(lockedResources.keySet()), resourceDescription));
BodyInvoker bodyInvoker =
context.newBodyInvoker().withCallback(new Callback(resourceNames, resourceDescription));
if (variable != null && !variable.isEmpty()) {
// set the variable for the duration of the block
bodyInvoker.withContext(
Expand All @@ -180,8 +189,7 @@ public static void proceed(
@Override
public void expand(@NonNull EnvVars env) {
final LinkedHashMap<String, String> variables = new LinkedHashMap<>();
final String resourceNames = String.join(",", lockedResources.keySet());
variables.put(variable, resourceNames);
variables.put(variable, resourceNamesAsString);
int index = 0;
for (Entry<String, List<LockableResourceProperty>> lockResourceEntry :
lockedResources.entrySet()) {
Expand Down Expand Up @@ -222,9 +230,11 @@ private static final class Callback extends BodyExecutionCallback.TailCall {

@Override
protected void finished(StepContext context) throws Exception {
LockableResourcesManager.get().unlockNames(this.resourceNames, context.get(Run.class));
Run<?, ?> build = context.get(Run.class);
LockedResourcesBuildAction.addLog(build, this.resourceNames, "released", this.resourceDescription);
LockableResourcesManager.get().unlockNames(this.resourceNames, build);
LockableResourcesManager.printLogs(
"Lock released on resource [" + resourceDescription + "]",
"Lock released on resource [" + this.resourceDescription + "]",
Level.FINE,
LOGGER,
context.get(TaskListener.class).getLogger());
Expand Down
4 changes: 4 additions & 0 deletions src/main/java/org/jenkins/plugins/lockableresources/LockableResource.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -494,14 +494,18 @@ public String getLockCauseDetail() {
return build;
}

// ---------------------------------------------------------------------------
@Exported
public String getBuildName() {
if (getBuild() != null) return getBuild().getFullDisplayName();
else return null;
}

// ---------------------------------------------------------------------------
public void setBuild(@Nullable Run<?, ?> lockedBy) {

this.build = lockedBy;

if (lockedBy != null) {
this.buildExternalizableId = lockedBy.getExternalizableId();
setReservedTimestamp(new Date());
Expand Down
115 changes: 60 additions & 55 deletions src/main/java/org/jenkins/plugins/lockableresources/LockableResourcesManager.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
import jenkins.util.SystemProperties;
import net.sf.json.JSONObject;
import org.apache.commons.lang.StringUtils;
import org.jenkins.plugins.lockableresources.actions.LockedResourcesBuildAction;
import org.jenkins.plugins.lockableresources.queue.LockableResourcesStruct;
import org.jenkins.plugins.lockableresources.queue.QueuedContextStruct;
import org.jenkins.plugins.lockableresources.util.Constants;
Expand Down Expand Up @@ -106,9 +107,10 @@ public List<LockableResource> getReadOnlyResources() {

// ---------------------------------------------------------------------------
/** Get declared resources, means only defined in config file (xml or JCaC yaml). */
@Restricted(NoExternalUse.class)
public List<LockableResource> getDeclaredResources() {
ArrayList<LockableResource> declaredResources = new ArrayList<>();
for (LockableResource r : this.getReadOnlyResources()) {
for (LockableResource r : this.getResources()) {
if (!r.isEphemeral() && !r.isNodeResource()) {
declaredResources.add(r);
}
Expand Down Expand Up @@ -165,7 +167,7 @@ public void setDeclaredResources(List<LockableResource> declaredResources) {
@Restricted(NoExternalUse.class)
public List<LockableResource> getResourcesFromProject(String fullName) {
List<LockableResource> matching = new ArrayList<>();
for (LockableResource r : this.getReadOnlyResources()) {
for (LockableResource r : this.getResources()) {
String rName = r.getQueueItemProject();
if (rName != null && rName.equals(fullName)) {
matching.add(r);
Expand All @@ -174,20 +176,6 @@ public List<LockableResource> getResourcesFromProject(String fullName) {
return matching;
}

// ---------------------------------------------------------------------------
/** Get all resources used by build. */
@Restricted(NoExternalUse.class)
public List<LockableResource> getResourcesFromBuild(Run<?, ?> build) {
List<LockableResource> matching = new ArrayList<>();
for (LockableResource r : this.getReadOnlyResources()) {
Run<?, ?> rBuild = r.getBuild();
if (rBuild != null && rBuild == build) {
matching.add(r);
}
}
return matching;
}

// ---------------------------------------------------------------------------
/**
* Check if the label is valid. Valid in this context means, if is configured on someone resource.
Expand All @@ -198,9 +186,11 @@ public Boolean isValidLabel(@Nullable String label) {
return false;
}

for (LockableResource r : this.getReadOnlyResources()) {
if (r != null && r.isValidLabel(label)) {
return true;
synchronized (this.syncResources) {
for (LockableResource r : this.getResources()) {
if (r != null && r.isValidLabel(label)) {
return true;
}
}
}

Expand Down Expand Up @@ -326,8 +316,10 @@ public LockableResource fromName(@CheckForNull String resourceName) {

if (resourceName != null) {

for (LockableResource r : this.getReadOnlyResources()) {
if (resourceName.equals(r.getName())) return r;
synchronized (this.syncResources) {
for (LockableResource r : this.getResources()) {
if (resourceName.equals(r.getName())) return r;
}
}
} else {
LOGGER.warning("Internal failure, fromName is empty or null:" + getStack());
Expand Down Expand Up @@ -590,47 +582,51 @@ public boolean lock(

// ---------------------------------------------------------------------------
/** Try to lock the resource and return true if locked. */
public boolean lock(List<LockableResource> resources, Run<?, ?> build) {
public boolean lock(List<LockableResource> resourcesToLock, Run<?, ?> build) {

LOGGER.fine("lock it: " + resources + " for build " + build);
LOGGER.fine("lock it: " + resourcesToLock + " for build " + build);

if (build == null) {
LOGGER.warning("lock() will fails, because the build does not exits. " + resources);
LOGGER.warning("lock() will fails, because the build does not exits. " + resourcesToLock);

Check warning on line 590 in src/main/java/org/jenkins/plugins/lockableresources/LockableResourcesManager.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Not covered line 

Line 590 is not covered by tests
return false; // not locked
}

String cause = getCauses(resources);
String cause = getCauses(resourcesToLock);
if (!cause.isEmpty()) {
LOGGER.warning("lock() for build " + build + " will fails, because " + cause);
return false; // not locked
}

for (LockableResource r : resources) {
for (LockableResource r : resourcesToLock) {
r.unqueue();
r.setBuild(build);
}

LockedResourcesBuildAction.findAndInitAction(build).addUsedResources(getResourcesNames(resourcesToLock));

save();

return true;
}

// ---------------------------------------------------------------------------
private void freeResources(List<LockableResource> unlockResources, @Nullable Run<?, ?> build) {
private void freeResources(List<LockableResource> unlockResources, Run<?, ?> build) {

LOGGER.fine("free it: " + unlockResources);

// make sure there is a list of resource names to unlock
if (unlockResources == null || unlockResources.isEmpty()) {
if (unlockResources == null || unlockResources.isEmpty() || build == null) {

Check warning on line 618 in src/main/java/org/jenkins/plugins/lockableresources/LockableResourcesManager.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Partially covered line 

Line 618 is only partially covered, 2 branches are missing
return;
}

List<LockableResource> toBeRemoved = new ArrayList<>();

for (LockableResource resource : unlockResources) {
// No more contexts, unlock resource
if (build != null && build != resource.getBuild()) {
continue; // this happens, when you push the unlock button in LRM page
}

// the resource has been currently unlocked (like by LRM page - button unlock, or by API)
if (!build.equals(resource.getBuild())) continue;

resource.unqueue();
resource.setBuild(null);
uncacheIfFreeing(resource, true, false);
Expand All @@ -640,43 +636,53 @@ private void freeResources(List<LockableResource> unlockResources, @Nullable Run
toBeRemoved.add(resource);
}
}

LockedResourcesBuildAction.findAndInitAction(build).removeUsedResources(getResourcesNames(unlockResources));

// remove all ephemeral resources
removeResources(toBeRemoved);
}

// ---------------------------------------------------------------------------
@Deprecated
@ExcludeFromJacocoGeneratedReport
public void unlock(List<LockableResource> resourcesToUnLock, @Nullable Run<?, ?> build) {
List<String> resourceNamesToUnLock = LockableResourcesManager.getResourcesNames(resourcesToUnLock);
this.unlockNames(resourceNamesToUnLock, build);
}
public void unlockBuild(@Nullable Run<?, ?> build) {

// ---------------------------------------------------------------------------
@Deprecated
@ExcludeFromJacocoGeneratedReport
public void unlock(
@Nullable List<LockableResource> resourcesToUnLock, @Nullable Run<?, ?> build, boolean inversePrecedence) {
unlock(resourcesToUnLock, build);
}
if (build == null) {

Check warning on line 648 in src/main/java/org/jenkins/plugins/lockableresources/LockableResourcesManager.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Partially covered line 

Line 648 is only partially covered, one branch is missing
return;

Check warning on line 649 in src/main/java/org/jenkins/plugins/lockableresources/LockableResourcesManager.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Not covered line 

Line 649 is not covered by tests
}

@Deprecated
@ExcludeFromJacocoGeneratedReport
public void unlockNames(
@Nullable List<String> resourceNamesToUnLock, @Nullable Run<?, ?> build, boolean inversePrecedence) {
this.unlockNames(resourceNamesToUnLock, build);
List<String> resourcesInUse =
LockedResourcesBuildAction.findAndInitAction(build).getCurrentUsedResourceNames();

if (resourcesInUse.size() == 0) {
return;
}
unlockNames(resourcesInUse, build);
}

// ---------------------------------------------------------------------------
@SuppressFBWarnings(value = "REC_CATCH_EXCEPTION", justification = "not sure which exceptions might be catch.")
public void unlockNames(@Nullable List<String> resourceNamesToUnLock, @Nullable Run<?, ?> build) {
public void unlockNames(@Nullable List<String> resourceNamesToUnLock, Run<?, ?> build) {

// make sure there is a list of resource names to unlock
if (resourceNamesToUnLock == null || resourceNamesToUnLock.isEmpty()) {
return;
}
synchronized (this.syncResources) {
unlockResources(this.fromNames(resourceNamesToUnLock), build);
}
}

// ---------------------------------------------------------------------------
public void unlockResources(List<LockableResource> resourcesToUnLock) {
unlockResources(resourcesToUnLock, resourcesToUnLock.get(0).getBuild());
}

// ---------------------------------------------------------------------------
public void unlockResources(List<LockableResource> resourcesToUnLock, Run<?, ?> build) {
if (resourcesToUnLock == null || resourcesToUnLock.isEmpty()) {

Check warning on line 681 in src/main/java/org/jenkins/plugins/lockableresources/LockableResourcesManager.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Partially covered line 

Line 681 is only partially covered, 2 branches are missing
return;

Check warning on line 682 in src/main/java/org/jenkins/plugins/lockableresources/LockableResourcesManager.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Not covered line 

Line 682 is not covered by tests
}
synchronized (this.syncResources) {
this.freeResources(this.fromNames(resourceNamesToUnLock), build);
this.freeResources(resourcesToUnLock, build);

while (proceedNextContext()) {
// process as many contexts as possible
Expand Down Expand Up @@ -913,8 +919,7 @@ public boolean steal(List<LockableResource> resources, String userName) {
r.setReservedBy(userName);
r.setStolen();
}
unlock(resources, null, false);
// unlock() nulls resource.reservedTimestamp via resource.setBuild(null), so set it afterwards
unlockResources(resources);
Date date = new Date();
for (LockableResource r : resources) {
r.setReservedTimestamp(date);
Expand Down Expand Up @@ -999,7 +1004,7 @@ public void recycle(List<LockableResource> resources) {
synchronized (this.syncResources) {
// Not calling reset() because that also un-queues the resource
// and we want to proclaim it is usable (if anyone is waiting)
this.unlock(resources, null);
this.unlockResources(resources);
this.unreserve(resources);
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/org/jenkins/plugins/lockableresources/actions/LockableResourcesRootAction.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -521,7 +521,7 @@ public void doUnlock(StaplerRequest req, StaplerResponse rsp) throws IOException
return;
}

LockableResourcesManager.get().unlock(resources, null);
LockableResourcesManager.get().unlockResources(resources);

rsp.forwardToPreviousPage(req);
}
Expand Down
Loading

0 comments on commit e218743

Please sign in to comment.