Skip to content

Commit 40e5147

Browse files
mPokornyETMmPokornyETM
authored
fix: Correct NodesMirror package and add label quantity regression tests (#1028)
* fix: Correct NodesMirror package declaration to match directory structure NodesMirror.java resides in the nodes/ subdirectory but declared package org.jenkins.plugins.lockableresources instead of org.jenkins.plugins.lockableresources.nodes. Fix the package declaration and update fully-qualified references in ConcurrentModificationExceptionTest and NodesMirrorTest. * test: Add regression tests for label quantity allocation (#939) Add LockStepLabelQuantityOneTest with three test scenarios for JENKINS-67083: 3 resources with the same label and 3 concurrent builds each requesting quantity 1. All three tests pass, confirming the issue is no longer reproducible. * Fix LockableResourcesManager get() lookup * test: Apply Spotless formatting
1 parent 2c727bb commit 40e5147

5 files changed

Lines changed: 219 additions & 4 deletions

File tree

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

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1592,7 +1592,11 @@ public boolean unqueueContext(StepContext context) {
15921592

15931593
// ---------------------------------------------------------------------------
15941594
public static LockableResourcesManager get() {
1595-
return (LockableResourcesManager) Jenkins.get().getDescriptorOrDie(LockableResourcesManager.class);
1595+
LockableResourcesManager mgr = Jenkins.get().getDescriptorByType(LockableResourcesManager.class);
1596+
if (mgr == null) {
1597+
throw new IllegalStateException("LockableResourcesManager is not registered");
1598+
}
1599+
return mgr;
15961600
}
15971601

15981602
// ---------------------------------------------------------------------------

‎src/main/java/org/jenkins/plugins/lockableresources/nodes/NodesMirror.java‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
package org.jenkins.plugins.lockableresources;
1+
package org.jenkins.plugins.lockableresources.nodes;
22

33
import hudson.Extension;
44
import hudson.init.InitMilestone;
@@ -10,6 +10,8 @@
1010
import java.util.stream.Collectors;
1111
import jenkins.model.Jenkins;
1212
import jenkins.util.SystemProperties;
13+
import org.jenkins.plugins.lockableresources.LockableResource;
14+
import org.jenkins.plugins.lockableresources.LockableResourcesManager;
1315
import org.jenkins.plugins.lockableresources.util.Constants;
1416

1517
// -----------------------------------------------------------------------------

‎src/test/java/org/jenkins/plugins/lockableresources/ConcurrentModificationExceptionTest.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,7 @@ public void run() {
9191
public void run() {
9292
System.setProperty(Constants.SYSTEM_PROPERTY_ENABLE_NODE_MIRROR, "true");
9393
LOGGER.info("run NodesMirror");
94-
org.jenkins.plugins.lockableresources.NodesMirror.createNodeResources();
94+
org.jenkins.plugins.lockableresources.nodes.NodesMirror.createNodeResources();
9595
LOGGER.info("NodesMirror done");
9696
}
9797
};

‎src/test/java/org/jenkins/plugins/lockableresources/LockStepLabelQuantityOneTest.java‎

Lines changed: 209 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,209 @@
1+
package org.jenkins.plugins.lockableresources;
2+
3+
import static org.junit.jupiter.api.Assertions.assertNotNull;
4+
5+
import org.jenkins.plugins.lockableresources.util.Constants;
6+
import org.jenkinsci.plugins.workflow.cps.CpsFlowDefinition;
7+
import org.jenkinsci.plugins.workflow.job.WorkflowJob;
8+
import org.jenkinsci.plugins.workflow.job.WorkflowRun;
9+
import org.jenkinsci.plugins.workflow.test.steps.SemaphoreStep;
10+
import org.junit.jupiter.api.BeforeEach;
11+
import org.junit.jupiter.api.Test;
12+
import org.jvnet.hudson.test.Issue;
13+
import org.jvnet.hudson.test.JenkinsRule;
14+
import org.jvnet.hudson.test.junit.jupiter.WithJenkins;
15+
16+
/**
17+
* Regression tests for JENKINS-67083 / GitHub issue #939.
18+
*
19+
* <p>3 resources with the same label, 3 concurrent builds each requesting
20+
* {@code lock(label: '...', quantity: 1)} — the third build must obtain a
21+
* resource instead of waiting indefinitely.
22+
*/
23+
@WithJenkins
24+
class LockStepLabelQuantityOneTest extends LockStepTestBase {
25+
26+
@BeforeEach
27+
void setUp() {
28+
System.setProperty(Constants.SYSTEM_PROPERTY_DISABLE_SAVE, "true");
29+
}
30+
31+
/**
32+
* Three resources with the same label, three concurrent builds each requesting
33+
* quantity 1 — all three must acquire a resource concurrently.
34+
*/
35+
@Test
36+
@Issue("JENKINS-67083")
37+
void threeBuildsEachLockOneOfThreeResources(JenkinsRule j) throws Exception {
38+
LockableResourcesManager lrm = LockableResourcesManager.get();
39+
lrm.createResourceWithLabel("Board_1", "imx8");
40+
lrm.createResourceWithLabel("Board_2", "imx8");
41+
lrm.createResourceWithLabel("Board_3", "imx8");
42+
43+
// Build 1: lock one resource
44+
WorkflowJob p1 = j.jenkins.createProject(WorkflowJob.class, "p1");
45+
p1.setDefinition(new CpsFlowDefinition(
46+
"lock(label: 'imx8', quantity: 1) {\n" + " semaphore 'wait-b1'\n" + "}\n" + "echo 'b1 done'", true));
47+
WorkflowRun b1 = p1.scheduleBuild2(0).waitForStart();
48+
SemaphoreStep.waitForStart("wait-b1/1", b1);
49+
50+
// Build 2: lock a second resource
51+
WorkflowJob p2 = j.jenkins.createProject(WorkflowJob.class, "p2");
52+
p2.setDefinition(new CpsFlowDefinition(
53+
"lock(label: 'imx8', quantity: 1) {\n" + " semaphore 'wait-b2'\n" + "}\n" + "echo 'b2 done'", true));
54+
WorkflowRun b2 = p2.scheduleBuild2(0).waitForStart();
55+
SemaphoreStep.waitForStart("wait-b2/1", b2);
56+
57+
// Build 3: must also lock the remaining resource without waiting
58+
WorkflowJob p3 = j.jenkins.createProject(WorkflowJob.class, "p3");
59+
p3.setDefinition(new CpsFlowDefinition(
60+
"lock(label: 'imx8', quantity: 1) {\n" + " semaphore 'wait-b3'\n" + "}\n" + "echo 'b3 done'", true));
61+
WorkflowRun b3 = p3.scheduleBuild2(0).waitForStart();
62+
// If #939 is present, b3 would report "Found 0 available resource(s)" and hang here
63+
SemaphoreStep.waitForStart("wait-b3/1", b3);
64+
65+
// All three are running concurrently — release them
66+
SemaphoreStep.success("wait-b1/1", null);
67+
SemaphoreStep.success("wait-b2/1", null);
68+
SemaphoreStep.success("wait-b3/1", null);
69+
70+
j.assertBuildStatusSuccess(j.waitForCompletion(b1));
71+
j.assertBuildStatusSuccess(j.waitForCompletion(b2));
72+
j.assertBuildStatusSuccess(j.waitForCompletion(b3));
73+
74+
j.assertLogContains("b1 done", b1);
75+
j.assertLogContains("b2 done", b2);
76+
j.assertLogContains("b3 done", b3);
77+
}
78+
79+
/**
80+
* Four builds compete for three resources (label, quantity 1). The first three
81+
* must proceed; the fourth waits and proceeds after one is released.
82+
*/
83+
@Test
84+
@Issue("JENKINS-67083")
85+
void fourthBuildWaitsThenProceedsWhenResourceFreed(JenkinsRule j) throws Exception {
86+
LockableResourcesManager lrm = LockableResourcesManager.get();
87+
lrm.createResourceWithLabel("Board_1", "imx8");
88+
lrm.createResourceWithLabel("Board_2", "imx8");
89+
lrm.createResourceWithLabel("Board_3", "imx8");
90+
91+
// Builds 1-3 lock all three resources
92+
WorkflowJob p1 = j.jenkins.createProject(WorkflowJob.class, "p1");
93+
p1.setDefinition(new CpsFlowDefinition(
94+
"lock(label: 'imx8', quantity: 1) {\n" + " semaphore 'wait-b1'\n" + "}\n" + "echo 'b1 done'", true));
95+
WorkflowRun b1 = p1.scheduleBuild2(0).waitForStart();
96+
SemaphoreStep.waitForStart("wait-b1/1", b1);
97+
98+
WorkflowJob p2 = j.jenkins.createProject(WorkflowJob.class, "p2");
99+
p2.setDefinition(new CpsFlowDefinition(
100+
"lock(label: 'imx8', quantity: 1) {\n" + " semaphore 'wait-b2'\n" + "}\n" + "echo 'b2 done'", true));
101+
WorkflowRun b2 = p2.scheduleBuild2(0).waitForStart();
102+
SemaphoreStep.waitForStart("wait-b2/1", b2);
103+
104+
WorkflowJob p3 = j.jenkins.createProject(WorkflowJob.class, "p3");
105+
p3.setDefinition(new CpsFlowDefinition(
106+
"lock(label: 'imx8', quantity: 1) {\n" + " semaphore 'wait-b3'\n" + "}\n" + "echo 'b3 done'", true));
107+
WorkflowRun b3 = p3.scheduleBuild2(0).waitForStart();
108+
SemaphoreStep.waitForStart("wait-b3/1", b3);
109+
110+
// Build 4: all resources taken — must wait
111+
WorkflowJob p4 = j.jenkins.createProject(WorkflowJob.class, "p4");
112+
p4.setDefinition(new CpsFlowDefinition(
113+
"lock(label: 'imx8', quantity: 1) {\n" + " semaphore 'wait-b4'\n" + "}\n" + "echo 'b4 done'", true));
114+
WorkflowRun b4 = p4.scheduleBuild2(0).waitForStart();
115+
j.waitForMessage("[Label: imx8, Quantity: 1] is not free, waiting for execution ...", b4);
116+
j.waitForMessage("Found 0 available resource(s). Waiting for correct amount: 1.", b4);
117+
118+
// Release b1 → b4 should proceed
119+
SemaphoreStep.success("wait-b1/1", null);
120+
j.assertBuildStatusSuccess(j.waitForCompletion(b1));
121+
122+
SemaphoreStep.waitForStart("wait-b4/1", b4);
123+
j.waitForMessage("Lock acquired on [Label: imx8, Quantity: 1]", b4);
124+
125+
// Cleanup
126+
SemaphoreStep.success("wait-b2/1", null);
127+
SemaphoreStep.success("wait-b3/1", null);
128+
SemaphoreStep.success("wait-b4/1", null);
129+
130+
j.assertBuildStatusSuccess(j.waitForCompletion(b2));
131+
j.assertBuildStatusSuccess(j.waitForCompletion(b3));
132+
j.assertBuildStatusSuccess(j.waitForCompletion(b4));
133+
134+
j.assertLogContains("b4 done", b4);
135+
}
136+
137+
/**
138+
* Verifies resources are reported via the {@code LOCK_NAME} variable and that
139+
* each build gets a different resource.
140+
*/
141+
@Test
142+
@Issue("JENKINS-67083")
143+
void threeBuildsGetDistinctResources(JenkinsRule j) throws Exception {
144+
LockableResourcesManager lrm = LockableResourcesManager.get();
145+
lrm.createResourceWithLabel("Board_1", "imx8");
146+
lrm.createResourceWithLabel("Board_2", "imx8");
147+
lrm.createResourceWithLabel("Board_3", "imx8");
148+
149+
WorkflowJob p1 = j.jenkins.createProject(WorkflowJob.class, "p1");
150+
p1.setDefinition(new CpsFlowDefinition(
151+
"lock(label: 'imx8', quantity: 1, variable: 'LOCK_NAME') {\n"
152+
+ " echo \"Locked: ${env.LOCK_NAME}\"\n"
153+
+ " semaphore 'wait-b1'\n"
154+
+ "}\n"
155+
+ "echo 'b1 done'",
156+
true));
157+
WorkflowRun b1 = p1.scheduleBuild2(0).waitForStart();
158+
SemaphoreStep.waitForStart("wait-b1/1", b1);
159+
160+
WorkflowJob p2 = j.jenkins.createProject(WorkflowJob.class, "p2");
161+
p2.setDefinition(new CpsFlowDefinition(
162+
"lock(label: 'imx8', quantity: 1, variable: 'LOCK_NAME') {\n"
163+
+ " echo \"Locked: ${env.LOCK_NAME}\"\n"
164+
+ " semaphore 'wait-b2'\n"
165+
+ "}\n"
166+
+ "echo 'b2 done'",
167+
true));
168+
WorkflowRun b2 = p2.scheduleBuild2(0).waitForStart();
169+
SemaphoreStep.waitForStart("wait-b2/1", b2);
170+
171+
WorkflowJob p3 = j.jenkins.createProject(WorkflowJob.class, "p3");
172+
p3.setDefinition(new CpsFlowDefinition(
173+
"lock(label: 'imx8', quantity: 1, variable: 'LOCK_NAME') {\n"
174+
+ " echo \"Locked: ${env.LOCK_NAME}\"\n"
175+
+ " semaphore 'wait-b3'\n"
176+
+ "}\n"
177+
+ "echo 'b3 done'",
178+
true));
179+
WorkflowRun b3 = p3.scheduleBuild2(0).waitForStart();
180+
SemaphoreStep.waitForStart("wait-b3/1", b3);
181+
182+
SemaphoreStep.success("wait-b1/1", null);
183+
SemaphoreStep.success("wait-b2/1", null);
184+
SemaphoreStep.success("wait-b3/1", null);
185+
186+
j.assertBuildStatusSuccess(j.waitForCompletion(b1));
187+
j.assertBuildStatusSuccess(j.waitForCompletion(b2));
188+
j.assertBuildStatusSuccess(j.waitForCompletion(b3));
189+
190+
// Each build must have locked exactly one resource
191+
String log1 = j.getLog(b1);
192+
String log2 = j.getLog(b2);
193+
String log3 = j.getLog(b3);
194+
195+
// Verify each build actually got a Board_* resource
196+
assertNotNull(extractLockedResource(log1, "Board_"), "b1 should lock a Board resource");
197+
assertNotNull(extractLockedResource(log2, "Board_"), "b2 should lock a Board resource");
198+
assertNotNull(extractLockedResource(log3, "Board_"), "b3 should lock a Board resource");
199+
}
200+
201+
private static String extractLockedResource(String log, String prefix) {
202+
for (String line : log.split("\n")) {
203+
if (line.contains("Locked: ") && line.contains(prefix)) {
204+
return line.trim();
205+
}
206+
}
207+
return null;
208+
}
209+
}

‎src/test/java/org/jenkins/plugins/lockableresources/NodesMirrorTest.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@
1818
class NodesMirrorTest {
1919

2020
private static final Logger LOGGER =
21-
Logger.getLogger(org.jenkins.plugins.lockableresources.NodesMirror.class.getName());
21+
Logger.getLogger(org.jenkins.plugins.lockableresources.nodes.NodesMirror.class.getName());
2222

2323
@Test
2424
void mirror_few_nodes(JenkinsRule j) throws Exception {

0 commit comments

Comments
 (0)