[JENKINS-72009, JENKINS-72200, JENKINS-24947] various fixes and improvements around disk space monitoring

core/src/main/java/hudson/Functions.java

@@ -2299,13 +2299,17 @@ public static String humanReadableByteSize(long size) {
         double number = size;
         if (number >= 1024) {
             number = number / 1024;
-            measure = "KB";
+            measure = "KiB";
             if (number >= 1024) {
                 number = number / 1024;
-                measure = "MB";
+                measure = "MiB";
                 if (number >= 1024) {
                     number = number / 1024;
-                    measure = "GB";
+                    measure = "GiB";
+                    if (number >= 1024) {
+                        number = number / 1024;
+                        measure = "TiB";
+                    }
                 }
             }
         }

core/src/main/java/hudson/model/Computer.java

@@ -44,6 +44,8 @@
 import hudson.model.Queue.FlyweightTask;
 import hudson.model.labels.LabelAtom;
 import hudson.model.queue.WorkUnit;
+import hudson.node_monitors.AbstractDiskSpaceMonitor;
+import hudson.node_monitors.DiskSpaceMonitorNodeProperty;
 import hudson.node_monitors.NodeMonitor;
 import hudson.remoting.Channel;
 import hudson.remoting.VirtualChannel;
@@ -1515,6 +1517,14 @@
         Node result = node.reconfigure(req, req.getSubmittedForm());
         Jenkins.get().getNodesObject().replaceNode(this.getNode(), result);
 
+        if (result.getNodeProperty(DiskSpaceMonitorNodeProperty.class) != null) {
+            for (NodeMonitor monitor : NodeMonitor.getAll()) {
+                if (monitor instanceof AbstractDiskSpaceMonitor) {
+                    monitor.data(this);
+                }
+            }
+        }
+
         // take the user back to the agent top page.
         rsp.sendRedirect2("../" + result.getNodeName() + '/');
     }

core/src/main/java/hudson/node_monitors/AbstractAsyncNodeMonitorDescriptor.java

@@ -139,7 +139,7 @@ private void error(Computer c, Throwable x) {
     /**
      * Result object for {@link AbstractAsyncNodeMonitorDescriptor#monitorDetailed()} to facilitate extending information
      * returned in the future.
-     *
+     * <p>
      * The {@link #getMonitoringData()} provides the results of the monitoring as {@link #monitor()} does. Note the value
      * in the map can be {@code null} for several reasons:
      * <ul>
@@ -149,8 +149,8 @@ private void error(Computer c, Throwable x) {
      *     <li>The {@link AbstractAsyncNodeMonitorDescriptor#createCallable} has returned null.</li>
      * </ul>
      *
-     * Clients can distinguishing among these states based on the additional data attached to this object. {@link #getSkipped()}
-     * returns computers that was not monitored as they ware either offline or monitor produced {@code null} {@link Callable}.
+     * Clients can distinguish among these states based on the additional data attached to this object. {@link #getSkipped()}
+     * returns computers that were not monitored as they were either offline or monitor produced {@code null} {@link Callable}.
      */
     protected static final class Result<T> {
         private static final long serialVersionUID = -7671448355804481216L;
@@ -163,15 +163,15 @@ private Result(@NonNull Map<Computer, T> data, @NonNull Collection<Computer> ski
             this.skipped = new ArrayList<>(skipped);
         }
 
-        protected @NonNull Map<Computer, T> getMonitoringData() {
+        public @NonNull Map<Computer, T> getMonitoringData() {
             return data;
         }
 
         /**
-         * Computers that ware skipped during monitoring as they either do not have a a channel (offline) or the monitor
-         * have not produced the Callable. Computers that caused monitor to throw exception are not returned here.
+         * Computers that were skipped during monitoring as they either do not have a channel (offline) or the monitor
+         * has not produced the Callable. Computers that caused monitor to throw exception are not returned here.
          */
-        protected @NonNull List<Computer> getSkipped() {
+        public @NonNull List<Computer> getSkipped() {
             return skipped;
         }
     }

core/src/main/java/hudson/node_monitors/AbstractDiskSpaceMonitor.java

@@ -4,8 +4,9 @@
 import hudson.node_monitors.DiskSpaceMonitorDescriptor.DiskSpace;
 import java.text.ParseException;
 import java.util.logging.Logger;
-import org.kohsuke.accmod.Restricted;
-import org.kohsuke.accmod.restrictions.NoExternalUse;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+import org.kohsuke.stapler.DataBoundSetter;
 
 /**
  * @author Kohsuke Kawaguchi
@@ -17,54 +18,80 @@
      * This is a human readable string representation as entered by the user, so that we can retain the original notation.
      */
     public final String freeSpaceThreshold;
+    private String freeSpaceWarningThreshold;
 
     protected AbstractDiskSpaceMonitor(String threshold) throws ParseException {
         this.freeSpaceThreshold = threshold;
         DiskSpace.parse(threshold); // make sure it parses
     }
 
     protected AbstractDiskSpaceMonitor() {
-        this.freeSpaceThreshold = "1GB";
+        this.freeSpaceThreshold = "1GiB";
+        this.freeSpaceWarningThreshold = "2GiB";
+    }
+
+    public Object readResolve() {
+        if (freeSpaceWarningThreshold == null) {
+            if (freeSpaceThreshold != null) {
+                Pattern p = Pattern.compile("(\\d+)(.*)");
+                Matcher m = p.matcher(freeSpaceThreshold);
+                if (m.matches()) {
+                    String digits = m.group(1);
+                    String unit = m.group(2);
+                    try {
+                        int wt = Integer.parseInt(digits) * 2;
+                        freeSpaceWarningThreshold = wt + unit;
+                    } catch (NumberFormatException nfe) {
+                        // unreachable
+                        freeSpaceWarningThreshold = "2GiB";
+                    }
+                }
+            } else {
+                freeSpaceWarningThreshold = "2GiB";
+            }
+        }
+        return this;
+    }
+
+    @DataBoundSetter
+    public void setFreeSpaceWarningThreshold(String freeSpaceWarningThreshold) {
+        this.freeSpaceWarningThreshold = freeSpaceWarningThreshold;
+    }
+
+    public String getFreeSpaceWarningThreshold() {
+        return freeSpaceWarningThreshold;
     }
 
     public long getThresholdBytes() {
-        if (freeSpaceThreshold == null)
-            return DEFAULT_THRESHOLD; // backward compatibility with the data format that didn't have 'freeSpaceThreshold'
         try {
             return DiskSpace.parse(freeSpaceThreshold).size;
         } catch (ParseException e) {
             return DEFAULT_THRESHOLD;
         }
     }
 
-    @Override
-    public Object data(Computer c) {
-        DiskSpace size = markNodeOfflineIfDiskspaceIsTooLow(c);
+    protected long getThresholdBytes(Computer c) {
+        return getThresholdBytes();
+    }
 
-        // mark online (again), if free space is over threshold
-        if (size != null && size.size > getThresholdBytes() && c.isOffline() && c.getOfflineCause() instanceof DiskSpace)
-            if (this.getClass().equals(((DiskSpace) c.getOfflineCause()).getTrigger()))
-                if (getDescriptor().markOnline(c)) {
-                    LOGGER.info(Messages.DiskSpaceMonitor_MarkedOnline(c.getDisplayName()));
-                }
-        return size;
+    protected long getWarningThresholdBytes() {
+        if (freeSpaceWarningThreshold == null)
+            return DEFAULT_THRESHOLD * 2;
+        try {
+            return DiskSpace.parse(freeSpaceWarningThreshold).size;
+        } catch (ParseException e) {
+            return DEFAULT_THRESHOLD * 2;
+        }
     }
 
-    /**
-     * Marks the given node as offline if free disk space is below the configured threshold.
-     * @param c the node
-     * @return the free space
-     * @since 1.521
-     */
-    @Restricted(NoExternalUse.class)
-    public DiskSpace markNodeOfflineIfDiskspaceIsTooLow(Computer c) {
+    protected long getWarningThresholdBytes(Computer c) {
+        return getWarningThresholdBytes();
+    }
+
+    @Override
+    public Object data(Computer c) {
         DiskSpace size = (DiskSpace) super.data(c);
-        if (size != null && size.size < getThresholdBytes()) {
-            size.setTriggered(this.getClass(), true);
-            if (getDescriptor().markOffline(c, size))  {
-                LOGGER.warning(Messages.DiskSpaceMonitor_MarkedOffline(c.getDisplayName()));
-            }
-        }
+        ((DiskSpaceMonitorDescriptor) getDescriptor()).markNodeOfflineOrOnline(c, size, this);
         return size;
     }
 

core/src/main/java/hudson/node_monitors/DiskSpaceMonitor.java

@@ -51,6 +51,38 @@
 
     public DiskSpaceMonitor() {}
 
+    @Override
+    public long getThresholdBytes(Computer c) {
+        Node node = c.getNode();
+        if (node != null) {
+            DiskSpaceMonitorNodeProperty nodeProperty = node.getNodeProperty(DiskSpaceMonitorNodeProperty.class);
+            if (nodeProperty != null) {
+                try {
+                    return DiskSpace.parse(nodeProperty.getFreeDiskSpaceThreshold()).size;
+                } catch (ParseException e) {
+                    return getThresholdBytes();
+                }
+            }
+        }
+        return getThresholdBytes();
+    }
+
+    @Override
+    protected long getWarningThresholdBytes(Computer c) {
+        Node node = c.getNode();
+        if (node != null) {
+            DiskSpaceMonitorNodeProperty nodeProperty = node.getNodeProperty(DiskSpaceMonitorNodeProperty.class);
+            if (nodeProperty != null) {
+                try {
+                    return DiskSpace.parse(nodeProperty.getFreeDiskSpaceWarningThreshold()).size;
+                } catch (ParseException e) {
+                    return getWarningThresholdBytes();
+                }
+            }
+        }
+        return getWarningThresholdBytes();
+    }
+
     public DiskSpace getFreeSpace(Computer c) {
         return DESCRIPTOR.get(c);
     }