diff --git a/pkg/cmd/roachtest/cluster.go b/pkg/cmd/roachtest/cluster.go index 66f5ef0fc629..f5e43f5775c0 100644 --- a/pkg/cmd/roachtest/cluster.go +++ b/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 diff --git a/pkg/cmd/roachtest/test_impl.go b/pkg/cmd/roachtest/test_impl.go index 5b4bb3f209dc..b1bf821e6ad5 100644 --- a/pkg/cmd/roachtest/test_impl.go +++ b/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) } }