diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md index c937a5df64..3071a40497 100644 --- a/.github/PULL_REQUEST_TEMPLATE.md +++ b/.github/PULL_REQUEST_TEMPLATE.md @@ -80,8 +80,8 @@ Leave the proposed upgrade guidelines in the pull request with the "N/A" value i The changelog generator relies on the presence of the upgrade guidelines section as part of its data extraction process. --> -```[tasklist] ### Submitter checklist + - [ ] The Jira issue, if it exists, is well-described. - [ ] The changelog entries and upgrade guidelines are appropriate for the audience affected by the change (users or developers, depending on the change) and are in the imperative mood (see [examples](https://github.com/jenkins-infra/jenkins.io/blob/master/content/_data/changelogs/weekly.yml)). Fill in the **Proposed upgrade guidelines** section only if there are breaking changes or changes that may require extra steps from users during upgrade. - [ ] There is automated testing or an explanation as to why this change has no tests. @@ -90,7 +90,6 @@ The changelog generator relies on the presence of the upgrade guidelines section - [ ] New or substantially changed JavaScript is not defined inline and does not call `eval` to ease future introduction of Content Security Policy (CSP) directives (see [documentation](https://www.jenkins.io/doc/developer/security/csp/)). - [ ] For dependency updates, there are links to external changelogs and, if possible, full differentials. - [ ] For new APIs and extension points, there is a link to at least one consumer. -``` ### Desired reviewers @@ -102,12 +101,11 @@ If you need an accelerated review process by the community (e.g., for critical b Before the changes are marked as `ready-for-merge`: -```[tasklist] ### Maintainer checklist + - [ ] There are at least two (2) approvals for the pull request and no outstanding requests for change. - [ ] Conversations in the pull request are over, or it is explicit that a reviewer is not blocking the change. - [ ] Changelog entries in the pull request title and/or **Proposed changelog entries** are accurate, human-readable, and in the imperative mood. - [ ] Proper changelog labels are set so that the changelog can be generated automatically. - [ ] If the change needs additional upgrade steps from users, the `upgrade-guide-needed` label is set and there is a **Proposed upgrade guidelines** section in the pull request title (see [example](https://github.com/jenkinsci/jenkins/pull/4387)). - [ ] If it would make sense to backport the change to LTS, a Jira issue must exist, be a _Bug_ or _Improvement_, and be labeled as `lts-candidate` to be considered (see [query](https://issues.jenkins.io/issues/?filter=12146)). -``` diff --git a/core/src/main/java/hudson/model/Queue.java b/core/src/main/java/hudson/model/Queue.java index 797cba56f3..28b5acb696 100644 --- a/core/src/main/java/hudson/model/Queue.java +++ b/core/src/main/java/hudson/model/Queue.java @@ -1615,6 +1615,9 @@ public class Queue extends ResourceController implements Saveable { // Ensure that identification of blocked tasks is using the live state: JENKINS-27708 & JENKINS-27871 updateSnapshot(); + // JENKINS-75152. Save the label and blockage reasons of items that have no candidates into this map. + // It is used to speed up the judgement of other buildable items with the same label. + Map> noCandidateLabelsMap = new HashMap<>(); // allocate buildable jobs to executors for (BuildableItem p : new ArrayList<>( buildables)) { // copy as we'll mutate the list in the loop @@ -1641,6 +1644,13 @@ public class Queue extends ResourceController implements Saveable { updateSnapshot(); } } else { + // JENKINS-75152. Skip the remaining steps if the label of this item has been determined + // to have no candidate executors in the previous allocation process. + Label itemLabel = p.getAssignedLabel(); + if (noCandidateLabelsMap.containsKey(itemLabel)) { + p.transientCausesOfBlockage = noCandidateLabelsMap.get(itemLabel); + continue; + } List candidates = new ArrayList<>(parked.size()); Map reasonMap = new HashMap<>(); @@ -1673,6 +1683,11 @@ public class Queue extends ResourceController implements Saveable { new Object[]{p, candidates, parked.values()}); List reasons = reasonMap.values().stream().filter(Objects::nonNull).collect(Collectors.toList()); p.transientCausesOfBlockage = reasons.isEmpty() ? null : reasons; + // If no candidates, save the label and blockage reasons. + if (candidates.isEmpty()) { + noCandidateLabelsMap.put(itemLabel, p.transientCausesOfBlockage); + LOGGER.log(Level.FINEST, "{0} changes to the state of no candidate executor", itemLabel); + } continue; } diff --git a/test/src/test/java/hudson/cli/DisconnectNodeCommandTest.java b/test/src/test/java/hudson/cli/DisconnectNodeCommandTest.java index 1b358bdde4..c3be6e4f7a 100644 --- a/test/src/test/java/hudson/cli/DisconnectNodeCommandTest.java +++ b/test/src/test/java/hudson/cli/DisconnectNodeCommandTest.java @@ -36,6 +36,7 @@ import static org.hamcrest.Matchers.not; import hudson.model.Computer; import hudson.slaves.DumbSlave; import hudson.slaves.OfflineCause; +import java.util.ArrayList; import jenkins.model.Jenkins; import org.junit.Before; import org.junit.Rule; @@ -187,32 +188,38 @@ public class DisconnectNodeCommandTest { @Test public void disconnectNodeManyShouldSucceedWithCause() throws Exception { - DumbSlave slave1 = j.createSlave("aNode1", "", null); - DumbSlave slave2 = j.createSlave("aNode2", "", null); - DumbSlave slave3 = j.createSlave("aNode3", "", null); - slave1.toComputer().waitUntilOnline(); - assertThat(slave1.toComputer().isOnline(), equalTo(true)); - assertThat(slave1.toComputer().getOfflineCause(), equalTo(null)); - slave2.toComputer().waitUntilOnline(); - assertThat(slave2.toComputer().isOnline(), equalTo(true)); - assertThat(slave2.toComputer().getOfflineCause(), equalTo(null)); - slave3.toComputer().waitUntilOnline(); - assertThat(slave3.toComputer().isOnline(), equalTo(true)); - assertThat(slave3.toComputer().getOfflineCause(), equalTo(null)); + int n = 3; + var agents = new ArrayList(); + for (int i = 1; i <= n; i++) { + agents.add(j.createSlave("aNode" + i, "", null)); + } + for (var agent : agents) { + var computer = agent.toComputer(); + computer.waitUntilOnline(); + assertThat(computer.isOnline(), equalTo(true)); + assertThat(computer.getOfflineCause(), equalTo(null)); + } + var args = new ArrayList(); + for (var agent : agents) { + args.add(agent.getNodeName()); + } + args.add("-m"); + args.add("aCause"); final CLICommandInvoker.Result result = command .authorizedTo(Computer.DISCONNECT, Jenkins.READ) - .invokeWithArgs("aNode1", "aNode2", "aNode3", "-m", "aCause"); + .invokeWithArgs(args.toArray(String[]::new)); assertThat(result, succeededSilently()); - assertThat(slave1.toComputer().isOffline(), equalTo(true)); - assertThat(slave1.toComputer().getOfflineCause(), instanceOf(OfflineCause.ByCLI.class)); - assertThat(((OfflineCause.ByCLI) slave1.toComputer().getOfflineCause()).message, equalTo("aCause")); - assertThat(slave2.toComputer().isOffline(), equalTo(true)); - assertThat(slave2.toComputer().getOfflineCause(), instanceOf(OfflineCause.ByCLI.class)); - assertThat(((OfflineCause.ByCLI) slave2.toComputer().getOfflineCause()).message, equalTo("aCause")); - assertThat(slave3.toComputer().isOffline(), equalTo(true)); - assertThat(slave3.toComputer().getOfflineCause(), instanceOf(OfflineCause.ByCLI.class)); - assertThat(((OfflineCause.ByCLI) slave3.toComputer().getOfflineCause()).message, equalTo("aCause")); + for (var agent : agents) { + var computer = agent.toComputer(); + assertThat(computer.isOffline(), equalTo(true)); + var cause = computer.getOfflineCause(); + if (cause instanceof OfflineCause.ByCLI cliCause) { + assertThat(cliCause.message, equalTo("aCause")); + } else { + assertThat("seen occasionally in CI", cause, instanceOf(OfflineCause.ChannelTermination.class)); + } + } } @Test