[JENKINS-67099] Make trim labels more selective when we're operating …

…on selected nodes (#5882) Co-authored-by: James Nord <[email protected]>
jenkinsci · Nov 13, 2021 · 4d5a979 · 4d5a979
1 parent 09f0269
commit 4d5a979
Show file tree

Hide file tree

Showing 2 changed files with 34 additions and 9 deletions.
diff --git a/core/src/main/java/jenkins/model/Jenkins.java b/core/src/main/java/jenkins/model/Jenkins.java
@@ -2239,14 +2239,38 @@ public DescribableList<NodeProperty<?>, NodePropertyDescriptor> getGlobalNodePro
      * 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();
+                }
             }
         }
     }

diff --git a/core/src/main/java/jenkins/model/Nodes.java b/core/src/main/java/jenkins/model/Nodes.java
@@ -32,6 +32,7 @@
 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 void addNode(final @NonNull Node node) throws IOException {
             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 void addNode(final @NonNull Node node) throws IOException {
                     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 boolean updateNode(final @NonNull Node node) throws IOException {
                 @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 void run() {
                     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 void run() {
                     }
                     if (node == nodes.remove(node.getNodeName())) {
                         jenkins.updateComputerList();
-                        jenkins.trimLabels();
+                        jenkins.trimLabels(node);
                     }
                 }
             });