fingerprint: update storage fingerprint freebytes calculation

.changelog/27019.txt

@@ -0,0 +1,3 @@
+```release-note:bug
+fingerprint: simplify storage fingerprint calculation to just (total disk space - reserved disk)
+```

api/allocations_test.go

@@ -213,7 +213,7 @@ func TestAllocations_Info(t *testing.T) {
 	// Fetch alloc info.
 	qo.WaitIndex = qm.LastIndex
 	alloc, _, err := a.Info(allocs[0].ID, qo)
-
+	must.NoError(t, err)
 	must.NotNil(t, alloc.NetworkStatus)
 }
 

client/config/config.go

@@ -143,6 +143,7 @@ type Config struct {
 	// determined dynamically.
 	DiskTotalMB int
 
+	// DEPRECATED: Remove in Nomad 1.13.0. Use Reserved.Disk instead.
 	// DiskFreeMB is the default node free disk space in megabytes if it cannot be
 	// determined dynamically.
 	DiskFreeMB int

client/fingerprint/storage.go

@@ -39,30 +39,22 @@ func (f *StorageFingerprint) Fingerprint(req *FingerprintRequest, resp *Fingerpr
 		}
 	}
 
-	volume, total, free, err := f.diskFree(storageDir)
+	volume, total, err := f.diskInfo(storageDir)
 	if err != nil {
 		return fmt.Errorf("failed to determine disk space for %s: %v", storageDir, err)
 	}
 
 	if cfg.DiskTotalMB > 0 {
 		total = uint64(cfg.DiskTotalMB) * bytesPerMegabyte
 	}
-	if cfg.DiskFreeMB > 0 {
-		free = uint64(cfg.DiskFreeMB) * bytesPerMegabyte
-	}
-
-	if total < free {
-		return fmt.Errorf("detected more free disk space (%d) than total disk space (%d), use disk_total_mb and disk_free_mb to correct", free, total)
-	}
 
 	resp.AddAttribute("unique.storage.volume", volume)
 	resp.AddAttribute("unique.storage.bytestotal", strconv.FormatUint(total, 10))
-	resp.AddAttribute("unique.storage.bytesfree", strconv.FormatUint(free, 10))
 
 	// set the disk size for the response
 	resp.NodeResources = &structs.NodeResources{
 		Disk: structs.NodeDiskResources{
-			DiskMB: int64(free / bytesPerMegabyte),
+			DiskMB: int64(total / bytesPerMegabyte),
 		},
 	}
 	resp.Detected = true

client/fingerprint/storage_test.go

@@ -28,20 +28,11 @@ func TestStorageFingerprint(t *testing.T) {
 
 	assertNodeAttributeContains(t, response.Attributes, "unique.storage.volume")
 	assertNodeAttributeContains(t, response.Attributes, "unique.storage.bytestotal")
-	assertNodeAttributeContains(t, response.Attributes, "unique.storage.bytesfree")
 
-	total, err := strconv.ParseInt(response.Attributes["unique.storage.bytestotal"], 10, 64)
+	_, err := strconv.ParseInt(response.Attributes["unique.storage.bytestotal"], 10, 64)
 	if err != nil {
 		t.Fatalf("Failed to parse unique.storage.bytestotal: %s", err)
 	}
-	free, err := strconv.ParseInt(response.Attributes["unique.storage.bytesfree"], 10, 64)
-	if err != nil {
-		t.Fatalf("Failed to parse unique.storage.bytesfree: %s", err)
-	}
-
-	if free > total {
-		t.Fatalf("unique.storage.bytesfree %d is larger than unique.storage.bytestotal %d", free, total)
-	}
 
 	if response.NodeResources == nil || response.NodeResources.Disk.DiskMB == 0 {
 		t.Errorf("Expected node.NodeResources.DiskMB.DiskMB to be non-zero")

client/fingerprint/storage_unix.go

@@ -15,12 +15,12 @@ import (
 	"strings"
 )
 
-// diskFree inspects the filesystem for path and returns the volume name and
-// the total and free bytes available on the file system.
-func (f *StorageFingerprint) diskFree(path string) (volume string, total, free uint64, err error) {
+// diskInfo inspects the filesystem for path and returns the volume name and
+// the total bytes available on the file system.
+func (f *StorageFingerprint) diskInfo(path string) (volume string, total uint64, err error) {
 	absPath, err := filepath.Abs(path)
 	if err != nil {
-		return "", 0, 0, fmt.Errorf("failed to determine absolute path for %s", path)
+		return "", 0, fmt.Errorf("failed to determine absolute path for %s", path)
 	}
 
 	// Use -k to standardize the output values between darwin and linux
@@ -34,35 +34,28 @@ func (f *StorageFingerprint) diskFree(path string) (volume string, total, free u
 
 	mountOutput, err := exec.Command("df", dfArgs, absPath).Output()
 	if err != nil {
-		return "", 0, 0, fmt.Errorf("failed to determine mount point for %s", absPath)
+		return "", 0, fmt.Errorf("failed to determine mount point for %s", absPath)
 	}
 	// Output looks something like:
 	//	Filesystem 1024-blocks      Used Available Capacity   iused    ifree %iused  Mounted on
 	//	/dev/disk1   487385240 423722532  63406708    87% 105994631 15851677   87%   /
 	//	[0] volume [1] capacity [2] SKIP  [3] free
 	lines := strings.Split(string(mountOutput), "\n")
 	if len(lines) < 2 {
-		return "", 0, 0, fmt.Errorf("failed to parse `df` output; expected at least 2 lines")
+		return "", 0, fmt.Errorf("failed to parse `df` output; expected at least 2 lines")
 	}
 	fields := strings.Fields(lines[1])
 	if len(fields) < 4 {
-		return "", 0, 0, fmt.Errorf("failed to parse `df` output; expected at least 4 columns")
+		return "", 0, fmt.Errorf("failed to parse `df` output; expected at least 4 columns")
 	}
 	volume = fields[0]
 
 	total, err = strconv.ParseUint(fields[1], 10, 64)
 	if err != nil {
-		return "", 0, 0, fmt.Errorf("failed to parse storage.bytestotal size in kilobytes")
+		return "", 0, fmt.Errorf("failed to parse storage.bytestotal size in kilobytes")
 	}
 	// convert to bytes
 	total *= 1024
 
-	free, err = strconv.ParseUint(fields[3], 10, 64)
-	if err != nil {
-		return "", 0, 0, fmt.Errorf("failed to parse storage.bytesfree size in kilobytes")
-	}
-	// convert to bytes
-	free *= 1024
-
-	return volume, total, free, nil
+	return volume, total, nil
 }

client/fingerprint/storage_windows.go

@@ -9,28 +9,28 @@ import (
 	"syscall"
 )
 
-//go:generate go run $GOROOT/src/syscall/mksyscall_windows.go -output zstorage_windows.go storage_windows.go
+//go:generate go run golang.org/x/sys/windows/mkwinsyscall -output zstorage_windows.go storage_windows.go
 
-//sys	getDiskFreeSpaceEx(dirName *uint16, availableFreeBytes *uint64, totalBytes *uint64, totalFreeBytes *uint64) (err error) = kernel32.GetDiskFreeSpaceExW
+//sys	getDiskSpaceEx(dirName *uint16, availableFreeBytes *uint64, totalBytes *uint64, totalFreeBytes *uint64) (err error) = kernel32.GetDiskFreeSpaceExW
 
-// diskFree inspects the filesystem for path and returns the volume name and
-// the total and free bytes available on the file system.
-func (f *StorageFingerprint) diskFree(path string) (volume string, total, free uint64, err error) {
+// diskInfo inspects the filesystem for path and returns the volume name and
+// the total bytes available on the file system.
+func (f *StorageFingerprint) diskInfo(path string) (volume string, total uint64, err error) {
 	absPath, err := filepath.Abs(path)
 	if err != nil {
-		return "", 0, 0, fmt.Errorf("failed to determine absolute path for %s", path)
+		return "", 0, fmt.Errorf("failed to determine absolute path for %s", path)
 	}
 
 	volume = filepath.VolumeName(absPath)
 
 	absPathp, err := syscall.UTF16PtrFromString(absPath)
 	if err != nil {
-		return "", 0, 0, fmt.Errorf("failed to convert \"%s\" to UTF16: %v", absPath, err)
+		return "", 0, fmt.Errorf("failed to convert \"%s\" to UTF16: %v", absPath, err)
 	}
 
-	if err := getDiskFreeSpaceEx(absPathp, nil, &total, &free); err != nil {
-		return "", 0, 0, fmt.Errorf("failed to get free disk space for %s: %v", absPath, err)
+	if err := getDiskSpaceEx(absPathp, nil, &total, nil); err != nil {
+		return "", 0, fmt.Errorf("failed to get free disk space for %s: %v", absPath, err)
 	}
 
-	return volume, total, free, nil
+	return volume, total, nil
 }

command/agent/agent.go

@@ -771,6 +771,12 @@ func (a *Agent) finalizeClientConfig(c *clientconfig.Config) error {
 		to configure Nomad to work with Consul.`)
 	}
 
+	// Log deprecation message about setting disk_free_mb
+	if c.DiskFreeMB != 0 {
+		a.logger.Warn(`disk_free_mb is deprecated and ignored by Nomad.
+		Please use client.reserved.disk to configure reservable disk for scheduling.`)
+	}
+
 	// If the operator has not set an intro token via the CLI or an environment
 	// variable, attempt to read the intro token from the file system. This
 	// cannot be used as a CLI override.

command/agent/config.go

@@ -307,6 +307,7 @@ type ClientConfig struct {
 	// DiskTotalMB is used to override any detected or default total disk space.
 	DiskTotalMB int `hcl:"disk_total_mb"`
 
+	// DEPRECATED: Remove in Nomad 1.13.0. Use Reserved.Disk instead.
 	// DiskFreeMB is used to override any detected or default free disk space.
 	DiskFreeMB int `hcl:"disk_free_mb"`
 

website/content/docs/configuration/client.mdx

@@ -95,9 +95,10 @@ client {
   constraints set on the attribute `unique.storage.bytestotal`. The actual total
   disk space can be determined via the [Read Stats API](/nomad/api-docs/client#read-stats)
 
-- `disk_free_mb` `(int:0)` - Specifies the disk space free for scheduling
-  allocations. If set, this value overrides any detected free disk space. This
-  value can be seen in `nomad node status` under Allocated Resources.
+- `disk_free_mb` `(int:0)` - Previously specified the disk space free for scheduling
+  allocations. If set, it would override any detected free disk space. This value
+  has been deprecated and is now ignored by Nomad clients.
+  Use [`client.reserved.disk`](#reserved-parameters) instead.
 
 - `min_dynamic_port` `(int:20000)` - Specifies the minimum dynamic port to be
   assigned. Individual ports and ranges of ports may be excluded from dynamic

website/content/docs/upgrade/upgrade-specific.mdx

@@ -14,6 +14,15 @@ used to document those details separately from the standard upgrade flow.
 
 ## Nomad 1.11.0
 
+#### Storage fingerprinting calculation changed
+
+Nomad now calculates the storage available for scheduling using only
+`totalBytes - client.reserved.disk`. The previous strategy using free disk
+space could lead to incorrect values when clients with running allocations
+restarted. The `unique.storage.bytesfree` attribute has also been removed.
+We recommend that you reserve at least the amount of disk that is used
+by the host OS.
+
 #### Sysbatch jobs will no longer accept `reschedule` blocks
 
 In Nomad 1.11.0, submitting a sysbatch job with a `reschedule` block returns