Skip to content

Commit 9a49376

Browse files
mPokornyETMmPokornyETM
authored
Add 'reason' parameter to lock() step (#982)
* Add updateLock pipeline step for resource management This step allows pipelines to dynamically manage lockable resources: - Create new resources (createResource: true) - Delete existing resources (deleteResource: true) - Modify labels (setLabels, addLabels, removeLabels) - Set notes (setNote) Based on the original design from PR #305 by @gaspardpetit. Fixes #305 * Remove @SInCE TODO - not applicable for plugins * Add updateLock step documentation to README * Add 'reason' parameter to lock() step Allows users to specify a reason when locking a resource, displayed in the lockable resources UI while the resource is locked. Usage: lock(resource: 'my-resource', reason: 'Running integration tests') { // ... } Features: - New 'reason' parameter on lock() step - Reason displayed in UI status column while locked - Reason cleared automatically when resource is unlocked - Works with label-based and resource-based locking - Reason preserved when lock request is queued - Environment variable 'resourceLockReason' available in groovy scripts Fixes #520 * feat: Add reason parameter to reserve button When a user reserves a resource via the Reserve button, they are now prompted to provide a reason. The reason is: - Stored in the lockReason field (shared with lock() step) - Displayed in the resources table - Cleared when the resource is unreserved Changes: - LockableResource: Add reserve(userName, reason) overload - LockableResourcesManager: Add reserve(resources, userName, reason) - doReserve: Read reason parameter from request - table.jelly: Show reason for reserved resources, add i18n template - lockable-resources.js: Prompt for reason on reserve action - table.properties: Add dialog messages * fix: consolidate i18n templates to avoid duplicate IDs * feat: Enhance reservation logging and add authentication check * refactor: improve logging format for resource reservation and user authentication (#1009) * fix: include reason in toString(), steal action, and fix spotless formatting
1 parent 8ce5140 commit 9a49376

24 files changed

Lines changed: 530 additions & 36 deletions

File tree

‎README.md‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,19 @@ echo 'Finish'
8484
8585
```
8686

87+
#### Lock with a reason
88+
89+
You can specify a reason why the resource is being locked. This is displayed
90+
in the lockable resources UI while the resource is locked:
91+
92+
```groovy
93+
lock(resource: 'staging-server', reason: 'Running integration tests') {
94+
echo 'Deploying to staging'
95+
}
96+
```
97+
98+
The reason helps other users understand why a resource is unavailable.
99+
87100
Example for declarative pipeline:
88101

89102
```groovy

‎src/main/java/org/jenkins/plugins/lockableresources/LockStep.java‎

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,11 @@ public class LockStep extends Step implements Serializable {
4242
@SuppressFBWarnings(value = "PA_PUBLIC_PRIMITIVE_ATTRIBUTE", justification = "Preserve API compatibility.")
4343
public String label = null;
4444

45+
/** The reason why this resource is being locked, displayed in the UI while locked. */
46+
@CheckForNull
47+
@SuppressFBWarnings(value = "PA_PUBLIC_PRIMITIVE_ATTRIBUTE", justification = "Preserve API compatibility.")
48+
public String reason = null;
49+
4550
@SuppressFBWarnings(value = "PA_PUBLIC_PRIMITIVE_ATTRIBUTE", justification = "Preserve API compatibility.")
4651
public int quantity = 0;
4752

@@ -117,6 +122,13 @@ public void setLabel(String label) {
117122
}
118123
}
119124

125+
@DataBoundSetter
126+
public void setReason(String reason) {
127+
if (reason != null && !reason.trim().isEmpty()) {
128+
this.reason = reason.trim();
129+
}
130+
}
131+
120132
@DataBoundSetter
121133
public void setVariable(String variable) {
122134
if (variable != null && !variable.trim().isEmpty()) {
@@ -230,7 +242,7 @@ public String toString() {
230242
.map(res -> "{" + res.toString() + "}")
231243
.collect(Collectors.joining(","));
232244
} else if (resource != null || label != null) {
233-
String ret = LockStepResource.toString(resource, label, quantity);
245+
String ret = LockStepResource.toString(resource, label, quantity, reason);
234246
if (this.priority != 0) {
235247
ret += ", Priority: " + this.priority;
236248
}
@@ -251,7 +263,7 @@ public void validate(boolean allowEmptyOrNullValues) {
251263
public List<LockStepResource> getResources() {
252264
List<LockStepResource> resources = new ArrayList<>();
253265
if (resource != null || label != null) {
254-
resources.add(new LockStepResource(resource, label, quantity));
266+
resources.add(new LockStepResource(resource, label, quantity, reason));
255267
}
256268

257269
if (extra != null) {

‎src/main/java/org/jenkins/plugins/lockableresources/LockStepExecution.java‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,7 @@ public boolean start() throws Exception {
9090
return false;
9191
}
9292

93-
if (!lrm.lock(available, run)) {
93+
if (!lrm.lock(available, run, step.reason)) {
9494
// this here is very defensive code, and you will probably never hit it. (hopefully)
9595
LOGGER.warning("Internal program error: Can not lock resources: " + available);
9696
onLockFailed(logger, resourceHolderList);
@@ -156,7 +156,8 @@ private void onLockFailed(PrintStream logger, List<LockableResourcesStruct> reso
156156
step.toString(),
157157
step.variable,
158158
step.inversePrecedence,
159-
step.priority);
159+
step.priority,
160+
step.reason);
160161
}
161162
}
162163

‎src/main/java/org/jenkins/plugins/lockableresources/LockStepResource.java‎

Lines changed: 30 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -35,10 +35,20 @@ public class LockStepResource extends AbstractDescribableImpl<LockStepResource>
3535
@SuppressFBWarnings(value = "PA_PUBLIC_PRIMITIVE_ATTRIBUTE", justification = "Preserve API compatibility.")
3636
public int quantity = 0;
3737

38+
/** The reason why this resource is being locked, displayed in the UI while locked. */
39+
@CheckForNull
40+
@SuppressFBWarnings(value = "PA_PUBLIC_PRIMITIVE_ATTRIBUTE", justification = "Preserve API compatibility.")
41+
public String reason = null;
42+
3843
LockStepResource(@Nullable String resource, @Nullable String label, int quantity) {
44+
this(resource, label, quantity, null);
45+
}
46+
47+
LockStepResource(@Nullable String resource, @Nullable String label, int quantity, @Nullable String reason) {
3948
this.resource = Util.fixEmptyAndTrim(resource);
4049
this.label = Util.fixEmptyAndTrim(label);
4150
this.quantity = quantity;
51+
this.reason = Util.fixEmptyAndTrim(reason);
4252
}
4353

4454
@DataBoundConstructor
@@ -56,24 +66,37 @@ public void setQuantity(int quantity) {
5666
this.quantity = quantity;
5767
}
5868

69+
@DataBoundSetter
70+
public void setReason(String reason) {
71+
this.reason = Util.fixEmptyAndTrim(reason);
72+
}
73+
5974
@Override
6075
public String toString() {
61-
return toString(resource, label, quantity);
76+
return toString(resource, label, quantity, reason);
6277
}
6378

6479
public static String toString(String resource, String label, int quantity) {
80+
return toString(resource, label, quantity, null);
81+
}
82+
83+
public static String toString(String resource, String label, int quantity, String reason) {
6584
// a label takes always priority
85+
StringBuilder sb = new StringBuilder();
6686
if (label != null) {
87+
sb.append("Label: ").append(label);
6788
if (quantity > 0) {
68-
return "Label: " + label + ", Quantity: " + quantity;
89+
sb.append(", Quantity: ").append(quantity);
6990
}
70-
return "Label: " + label;
91+
} else if (resource != null) {
92+
sb.append("Resource: ").append(resource);
93+
} else {
94+
return "[no resource/label specified - probably a bug]";
7195
}
72-
// make sure there is an actual resource specified
73-
if (resource != null) {
74-
return "Resource: " + resource;
96+
if (reason != null && !reason.isEmpty()) {
97+
sb.append(", Reason: ").append(reason);
7598
}
76-
return "[no resource/label specified - probably a bug]";
99+
return sb.toString();
77100
}
78101

79102
// -------------------------------------------------------------------------

‎src/main/java/org/jenkins/plugins/lockableresources/LockableResource.java‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,12 @@ public class LockableResource extends AbstractDescribableImpl<LockableResource>
7272
private Date reservedTimestamp = null;
7373
private String note = "";
7474

75+
/**
76+
* The reason why this resource is currently locked. Set via the lock() step's reason parameter.
77+
* Cleared when the resource is unlocked.
78+
*/
79+
private String lockReason = "";
80+
7581
/**
7682
* Track that a currently reserved resource was originally reserved for someone else, or locked
7783
* for some other job, and explicitly taken away - e.g. for SUT post-mortem while a test job runs.
@@ -255,6 +261,26 @@ public void setNote(@Nullable String note) {
255261
this.note = Util.fixNull(note);
256262
}
257263

264+
/**
265+
* Returns the reason why this resource is currently locked.
266+
*
267+
* @return The lock reason, or empty string if not set.
268+
*/
269+
@Exported
270+
public String getLockReason() {
271+
return this.lockReason;
272+
}
273+
274+
/**
275+
* Sets the reason why this resource is being locked. This is typically set via the lock() step's
276+
* reason parameter and cleared when the resource is unlocked.
277+
*
278+
* @param lockReason The reason for locking, or null to clear.
279+
*/
280+
public void setLockReason(@Nullable String lockReason) {
281+
this.lockReason = Util.fixNull(lockReason);
282+
}
283+
258284
@DataBoundSetter
259285
public void setEphemeral(boolean ephemeral) {
260286
this.ephemeral = ephemeral;
@@ -422,6 +448,7 @@ private boolean evaluateScript(@NonNull SecureGroovyScript script, @CheckForNull
422448
binding.setVariable("resourceDescription", description);
423449
binding.setVariable("resourceLabels", this.getLabelsAsList());
424450
binding.setVariable("resourceNote", note);
451+
binding.setVariable("resourceLockReason", lockReason);
425452
try {
426453
Object result = script.evaluate(Jenkins.get().getPluginManager().uberClassLoader, binding, null);
427454
if (LOGGER.isLoggable(Level.FINE)) {
@@ -650,20 +677,27 @@ public boolean isStolen() {
650677
}
651678

652679
public void reserve(String userName) {
680+
reserve(userName, null);
681+
}
682+
683+
public void reserve(String userName, String reason) {
653684
setReservedBy(userName);
654685
setReservedTimestamp(new Date());
686+
setLockReason(reason);
655687
}
656688

657689
public void unReserve() {
658690
this.setReservedBy(null);
659691
this.setReservedTimestamp(null);
692+
this.setLockReason(null);
660693
this.stolen = false;
661694
}
662695

663696
public void reset() {
664697
this.unReserve();
665698
this.unqueue();
666699
this.setBuild(null);
700+
this.setLockReason(null);
667701
invalidateCaches();
668702
}
669703

@@ -678,6 +712,7 @@ public void copyUnconfigurableProperties(final LockableResource sourceResource)
678712
setReservedTimestamp(sourceResource.getReservedTimestamp());
679713
setNote(sourceResource.getNote());
680714
setReservedBy(sourceResource.getReservedBy());
715+
setLockReason(sourceResource.getLockReason());
681716
}
682717
}
683718

@@ -689,6 +724,7 @@ public void resetUnconfigurableProperties() {
689724
setReservedBy(null);
690725
setReservedTimestamp(null);
691726
setNote("");
727+
setLockReason("");
692728
}
693729

694730
/**

‎src/main/java/org/jenkins/plugins/lockableresources/LockableResourcesManager.java‎

Lines changed: 72 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -664,8 +664,21 @@ public boolean lock(
664664
// ---------------------------------------------------------------------------
665665
/** Try to lock the resource and return true if locked. */
666666
public boolean lock(List<LockableResource> resourcesToLock, Run<?, ?> build) {
667+
return lock(resourcesToLock, build, (String) null);
668+
}
669+
670+
// ---------------------------------------------------------------------------
671+
/**
672+
* Try to lock the resource and return true if locked.
673+
*
674+
* @param resourcesToLock The resources to lock.
675+
* @param build The build that is locking the resources.
676+
* @param reason The reason why the resources are being locked (displayed in UI).
677+
* @return true if locked successfully.
678+
*/
679+
public boolean lock(List<LockableResource> resourcesToLock, Run<?, ?> build, @Nullable String reason) {
667680

668-
LOGGER.fine("lock it: " + resourcesToLock + " for build " + build);
681+
LOGGER.fine("lock it: " + resourcesToLock + " for build " + build + " with reason: " + reason);
669682

670683
if (build == null) {
671684
LOGGER.warning("lock() will fails, because the build does not exits. " + resourcesToLock);
@@ -681,6 +694,9 @@ public boolean lock(List<LockableResource> resourcesToLock, Run<?, ?> build) {
681694
for (LockableResource r : resourcesToLock) {
682695
r.unqueue();
683696
r.setBuild(build);
697+
if (reason != null && !reason.isEmpty()) {
698+
r.setLockReason(reason);
699+
}
684700
}
685701

686702
LockedResourcesBuildAction.findAndInitAction(build).addUsedResources(getResourcesNames(resourcesToLock));
@@ -710,6 +726,7 @@ private void freeResources(List<LockableResource> unlockResources, Run<?, ?> bui
710726

711727
resource.unqueue();
712728
resource.setBuild(null);
729+
resource.setLockReason(null);
713730
uncacheIfFreeing(resource, true, false);
714731

715732
if (resource.isEphemeral()) {
@@ -795,7 +812,7 @@ private boolean proceedNextContext() {
795812
LOGGER.warning("Skip this context, as the build cannot be retrieved");
796813
return true;
797814
}
798-
boolean locked = this.lock(requiredResourceForNextContext, build);
815+
boolean locked = this.lock(requiredResourceForNextContext, build, nextContext.getReason());
799816
if (!locked) {
800817
// defensive line, shall never happen
801818
LOGGER.warning("Can not lock resources: " + requiredResourceForNextContext);
@@ -994,17 +1011,35 @@ public boolean addResource(@Nullable final LockableResource resource, final bool
9941011
* explicit scripted action, decides to release the resource).
9951012
*/
9961013
public boolean reserve(List<LockableResource> resources, String userName) {
1014+
return reserve(resources, userName, null);
1015+
}
1016+
1017+
// ---------------------------------------------------------------------------
1018+
/**
1019+
* Reserves an available resource for the userName indefinitely (until that person, or some
1020+
* explicit scripted action, decides to release the resource).
1021+
*
1022+
* @param resources list of resources to reserve
1023+
* @param userName the user reserving the resources
1024+
* @param reason the reason for reserving (optional)
1025+
* @return true if all resources were successfully reserved, false if any was not free
1026+
*/
1027+
public boolean reserve(List<LockableResource> resources, String userName, String reason) {
1028+
LOGGER.info("reserve() called user='" + userName + "' resources=" + getResourcesNames(resources) + " reason='"
1029+
+ reason + "'");
9971030
synchronized (syncResources) {
9981031
for (LockableResource r : resources) {
9991032
if (!r.isFree()) {
1033+
LOGGER.fine("reserve() failed because resource not free: " + r.getName());
10001034
return false;
10011035
}
10021036
}
10031037
for (LockableResource r : resources) {
1004-
r.reserve(userName);
1038+
r.reserve(userName, reason);
10051039
}
10061040
save();
10071041
}
1042+
LOGGER.info("reserve() succeeded user='" + userName + "' resources=" + getResourcesNames(resources));
10081043
return true;
10091044
}
10101045

@@ -1015,6 +1050,21 @@ public boolean reserve(List<LockableResource> resources, String userName) {
10151050
* scripted action, later decides to release the resource).
10161051
*/
10171052
public boolean steal(List<LockableResource> resources, String userName) {
1053+
return steal(resources, userName, null);
1054+
}
1055+
1056+
// ---------------------------------------------------------------------------
1057+
/**
1058+
* Reserves a resource that may be or not be locked by some job (or reserved by some user)
1059+
* already, giving it away to the userName indefinitely (until that person, or some explicit
1060+
* scripted action, later decides to release the resource).
1061+
*
1062+
* @param resources list of resources to steal
1063+
* @param userName the user stealing the resources
1064+
* @param reason the reason for stealing (optional)
1065+
* @return true if stolen successfully
1066+
*/
1067+
public boolean steal(List<LockableResource> resources, String userName, String reason) {
10181068
synchronized (syncResources) {
10191069
for (LockableResource r : resources) {
10201070
r.setReservedBy(userName);
@@ -1024,6 +1074,7 @@ public boolean steal(List<LockableResource> resources, String userName) {
10241074
Date date = new Date();
10251075
for (LockableResource r : resources) {
10261076
r.setReservedTimestamp(date);
1077+
r.setLockReason(reason);
10271078
}
10281079
save();
10291080
}
@@ -1407,6 +1458,22 @@ public void queueContext(
14071458
String variableName,
14081459
boolean inversePrecedence,
14091460
int priority) {
1461+
queueContext(context, requiredResources, resourceDescription, variableName, inversePrecedence, priority, null);
1462+
}
1463+
1464+
/*
1465+
* Adds the given context and the required resources to the queue if
1466+
* this context is not yet queued.
1467+
*/
1468+
@Restricted(NoExternalUse.class)
1469+
public void queueContext(
1470+
StepContext context,
1471+
List<LockableResourcesStruct> requiredResources,
1472+
String resourceDescription,
1473+
String variableName,
1474+
boolean inversePrecedence,
1475+
int priority,
1476+
String reason) {
14101477
synchronized (syncResources) {
14111478
for (QueuedContextStruct entry : this.queuedContexts) {
14121479
if (entry.getContext() == context) {
@@ -1416,8 +1483,8 @@ public void queueContext(
14161483
}
14171484

14181485
int queueIndex = 0;
1419-
QueuedContextStruct newQueueItem =
1420-
new QueuedContextStruct(context, requiredResources, resourceDescription, variableName, priority);
1486+
QueuedContextStruct newQueueItem = new QueuedContextStruct(
1487+
context, requiredResources, resourceDescription, variableName, priority, reason);
14211488

14221489
if (!inversePrecedence || priority != 0) {
14231490
queueIndex = this.queuedContexts.size() - 1;

0 commit comments

Comments
 (0)