commit ead27a89ac001b1acf154c309ab2cd8b23ffc820
author Leandro Lovisolo <lovisolo@google.com> Mon Aug 07 16:01:49 2023 +0000
committer SkCQ <skcq-be@skia-corp.google.com.iam.gserviceaccount.com> Mon Aug 07 16:23:48 2023 +0000
tree 7ea6fc8f3f7dd429c414d5e858aaa07f3d0cf967
parent 89658e42bb911122c8c0056ae67e2bb80d7ddf34

ephemeral_storage.go: Fix test_machine_monitor panic on Windows.

Currently the ephemeral storage monitoring routine is making test_machine_monitor panic and crash on Windows because /tmp does not exist on said OS. Specifically, the filepath.Walk() callback ignores the err argument and calls info.IsDir(), which causes a nil pointer dereference because info is nil.

This CL tries to prevent the aforementioned panic by checking the err argument in the filepath.Walk() callback, and before that, it falls back to os.TempDir() if /tmp does not exist.

Bug: b/40045300
Change-Id: I62fcf87325d30071817f3f9319f4dfbf7d5cdb12
Reviewed-on: https://skia-review.googlesource.com/c/buildbot/+/736745
Commit-Queue: Leandro Lovisolo <lovisolo@google.com>
Reviewed-by: Eric Boren <borenet@google.com>
Auto-Submit: Leandro Lovisolo <lovisolo@google.com>
Commit-Queue: Eric Boren <borenet@google.com>

go/ephemeral_storage/ephemeral_storage.go

diff --git a/go/ephemeral_storage/ephemeral_storage.go b/go/ephemeral_storage/ephemeral_storage.go
index 058c284..9f9139e 100644
--- a/go/ephemeral_storage/ephemeral_storage.go
+++ b/go/ephemeral_storage/ephemeral_storage.go
@@ -6,6 +6,7 @@
 	"encoding/json"
 	"fmt"
 	"io/fs"
+	"os"
 	"path/filepath"
 	"time"
 
@@ -27,6 +28,7 @@
 type report struct {
 	// Type is a const to make it easier to filter these structured log entries.
 	Type       string
+	TempDir    string // Usually "/tmp", or os.TempDir() if "/tmp" does not exist (e.g. on Windows).
 	TotalBytes int64
 	TotalFiles int64
 	Files      []file
@@ -36,9 +38,24 @@
 //
 // The JSON emitted on a single line will be picked up by StackDriver as a structured log.
 func UsageViaStructuredLogging(ctx context.Context) error {
+	// Note we use "/tmp" and not $TMPDIR, because if $TMPDIR is set then it's
+	// probably not using ephemeral storage.
+	//
+	// Other potential directories to walk are listed here:
+	//
+	// https://cloud.google.com/container-optimized-os/docs/concepts/disks-and-filesystem#working_with_the_file_system
+	//
+	// If /tmp does not exist, we fall back to os.TempDir(). This has the benefit of being compatible
+	// with Windows.
+	tempDir := "/tmp"
+	if fileInfo, err := os.Stat(tempDir); err != nil || !fileInfo.IsDir() {
+		tempDir = os.TempDir()
+	}
+
 	report := report{
-		Type:  typeTag,
-		Files: []file{}, // Serialize to at least [] in JSON.
+		Type:    typeTag,
+		TempDir: tempDir,
+		Files:   []file{}, // Serialize to at least [] in JSON.
 	}
 
 	var totalSize int64
@@ -48,13 +65,10 @@
 		return skerr.Wrap(err)
 	}
 
-	// Note we use "/tmp" and not $TMPDIR, because if $TMPDIR is set then it's
-	// probably not using ephemeral storage.
-	//
-	// Other potential directories to walk are listed here:
-	//
-	// https://cloud.google.com/container-optimized-os/docs/concepts/disks-and-filesystem#working_with_the_file_system
-	err := filepath.Walk("/tmp", func(path string, info fs.FileInfo, err error) error {
+	err := filepath.Walk(tempDir, func(path string, info fs.FileInfo, err error) error {
+		if err != nil {
+			return skerr.Wrap(err)
+		}
 		if info.IsDir() {
 			return nil
 		}