[JENKINS-67099] Make trim labels more selective when we're operating on selected nodes (#5882)

Co-authored-by: James Nord <jtnord@users.noreply.github.com>
This commit is contained in:
Vincent LatombeandJames Nord authored and GitHub committed 2021-11-13 16:59:26 +00:00
1 parent 09f0269e87
commit 4d5a979995
2 files changed
+34 -9

No files matched your search

+28 -4
View File
@@ -2239,14 +2239,38 @@ public class Jenkins extends AbstractCIBase implements DirectlyModifiableTopLeve
* but we also call this periodically to self-heal any data out-of-sync issue.
*/
/*package*/ void trimLabels() {
trimLabels((Set) null);
}
/**
* Reset labels and remove invalid ones for the given nodes.
* @param nodes the nodes taken as reference to update labels
*/
void trimLabels(Node... nodes) {
Set<LabelAtom> includedLabels = new HashSet<>();
Arrays.asList(nodes).stream().filter(Objects::nonNull).forEach(n -> includedLabels.addAll(n.getAssignedLabels()));
trimLabels(includedLabels);
}
/**
* Reset labels and remove invalid ones for the given nodes.
* @param includedLabels the labels taken as reference to update labels. If {@code null}, all labels are considered.
*/
private void trimLabels(@CheckForNull Set<LabelAtom> includedLabels) {
Set<Label> nodeLabels = new HashSet<>(this.getAssignedLabels());
this.getNodes().forEach(n -> nodeLabels.addAll(n.getAssignedLabels()));
for (Iterator<Label> itr = labels.values().iterator(); itr.hasNext();) {
Label l = itr.next();
if (nodeLabels.contains(l) || this.clouds.stream().anyMatch(c -> c.canProvision(l))) {
resetLabel(l);
} else {
itr.remove();
if (includedLabels == null || includedLabels.contains(l)) {
if (nodeLabels.contains(l) || !l.getClouds().isEmpty()) {
// there is at least one static agent or one cloud that currently claims it can handle the label.
// if the cloud has been removed, or its labels updated such that it can not handle this, this is handle in later calls
// resetLabel will remove the agents, and clouds from the label, and they will be repopulated later.
// not checking `cloud.canProvision()` here prevents a potential call that will only be repeated later
resetLabel(l);
} else {
itr.remove();
}
}
}
}
+6 -5
View File
@@ -32,6 +32,7 @@ import hudson.model.Computer;
import hudson.model.Node;
import hudson.model.Queue;
import hudson.model.Saveable;
import hudson.model.labels.LabelAtom;
import hudson.model.listeners.SaveableListener;
import hudson.slaves.EphemeralNode;
import hudson.slaves.OfflineCause;
@@ -141,7 +142,7 @@ public class Nodes implements Saveable {
AtomicReference<Node> old = new AtomicReference<>();
old.set(nodes.put(node.getNodeName(), node));
jenkins.updateNewComputer(node);
jenkins.trimLabels();
jenkins.trimLabels(node, oldNode);
// TODO there is a theoretical race whereby the node instance is updated/removed after lock release
try {
persistNode(node);
@@ -153,7 +154,7 @@ public class Nodes implements Saveable {
public void run() {
nodes.compute(node.getNodeName(), (ignoredNodeName, ignoredNode) -> oldNode);
jenkins.updateComputerList();
jenkins.trimLabels();
jenkins.trimLabels(node, oldNode);
}
});
throw e;
@@ -201,7 +202,7 @@ public class Nodes implements Saveable {
@Override
public Boolean call() throws Exception {
if (node == nodes.get(node.getNodeName())) {
jenkins.trimLabels();
jenkins.trimLabels(node);
return true;
}
return false;
@@ -242,7 +243,7 @@ public class Nodes implements Saveable {
Nodes.this.nodes.remove(oldOne.getNodeName());
Nodes.this.nodes.put(newOne.getNodeName(), newOne);
jenkins.updateComputerList();
jenkins.trimLabels();
jenkins.trimLabels(oldOne, newOne);
}
});
updateNode(newOne);
@@ -276,7 +277,7 @@ public class Nodes implements Saveable {
}
if (node == nodes.remove(node.getNodeName())) {
jenkins.updateComputerList();
jenkins.trimLabels();
jenkins.trimLabels(node);
}
}
});