Merge pull request cockroachdb#98214 from smg260/backport22.1-81779-9…

…2845-93124

release-22.1: roachtest: logger nil dereference guards

pkg/cmd/roachtest/cluster.go

@@ -1960,9 +1960,16 @@ func (c *clusterImpl) RunE(ctx context.Context, node option.NodeListOption, args
 	if err := ctx.Err(); err != nil {
 		l.Printf("(note: incoming context was canceled: %s", err)
 	}
-	physicalFileName := l.File.Name()
+	// We need to protect ourselves from a race where cluster logger is
+	// concurrently closed before child logger is created. In that case child
+	// logger will have no log file but would write to stderr instead and we can't
+	// create a meaningful ".failed" file for it.
+	physicalFileName := ""
+	if l.File != nil {
+		physicalFileName = l.File.Name()
+	}
 	l.Close()
-	if err != nil {
+	if err != nil && len(physicalFileName) > 0 {
 		failedPhysicalFileName := strings.TrimSuffix(physicalFileName, ".log") + ".failed"
 		if failedFile, err2 := os.Create(failedPhysicalFileName); err2 != nil {
 			failedFile.Close()
@@ -2003,7 +2010,10 @@ func (c *clusterImpl) RunWithDetails(
 	if err != nil {
 		return nil, err
 	}
-	physicalFileName := l.File.Name()
+	physicalFileName := ""
+	if l.File != nil {
+		physicalFileName = l.File.Name()
+	}
 
 	if err := ctx.Err(); err != nil {
 		l.Printf("(note: incoming context was canceled: %s", err)
@@ -2016,7 +2026,7 @@ func (c *clusterImpl) RunWithDetails(
 	}
 
 	results, err := roachprod.RunWithDetails(ctx, l, c.MakeNodes(nodes), "" /* SSHOptions */, "" /* processTag */, false /* secure */, args)
-	if err != nil {
+	if err != nil && len(physicalFileName) > 0 {
 		l.Printf("> result: %+v", err)
 		createFailedFile(physicalFileName)
 		return results, err

pkg/cmd/roachtest/test_impl.go

@@ -335,9 +335,13 @@ func (t *testImpl) addFailure(depth int, format string, args ...interface{}) {
 			// We don't actually log through this logger since it adds an unrelated
 			// file:line caller (namely ours). The error already has stack traces
 			// so it's better to write only it to the file to avoid confusion.
-			path := cl.File.Name()
+			if cl.File != nil {
+				path := cl.File.Name()
+				if len(path) > 0 {
+					_ = os.WriteFile(path, []byte(fmt.Sprintf("%+v", reportFailure.squashedErr)), 0644)
+				}
+			}
 			cl.Close() // we just wanted the filename
-			_ = os.WriteFile(path, []byte(fmt.Sprintf("%+v", reportFailure.squashedErr)), 0644)
 		}
 	}