diff --git a/audit/internal/app/copy_path_race_test.go b/audit/internal/app/copy_path_race_test.go new file mode 100644 index 0000000..ba8968c --- /dev/null +++ b/audit/internal/app/copy_path_race_test.go @@ -0,0 +1,100 @@ +package app + +import ( + "os" + "path/filepath" + "testing" +) + +// These simulate the race that left blackbox stuck "degraded" on a real +// crash bundle: bee's own task runner renames a task's artifacts dir +// (e.g. "_pending" -> "_done") concurrently with a copy walking the export +// tree. A source entry that existed when its parent directory was listed but +// is gone by the time it's individually Stat'd/Open'd must be skipped, not +// treated as a fatal error that aborts the whole sync cycle. + +func TestCopyPathSkipsEntryRemovedBetweenListingAndStat(t *testing.T) { + srcDir := t.TempDir() + dstDir := t.TempDir() + + present := filepath.Join(srcDir, "present.txt") + if err := os.WriteFile(present, []byte("data"), 0644); err != nil { + t.Fatalf("WriteFile: %v", err) + } + vanished := filepath.Join(srcDir, "vanished.txt") + + // Never actually create "vanished.txt": copyPath is called directly on + // its path, standing in for "listed by a parent os.ReadDir a moment ago, + // gone by the time we get here" without needing real concurrency. + if err := copyPath(vanished, filepath.Join(dstDir, "vanished.txt")); err != nil { + t.Fatalf("copyPath on a vanished entry should be skipped, not fail: %v", err) + } + if err := copyPath(present, filepath.Join(dstDir, "present.txt")); err != nil { + t.Fatalf("copyPath on a present entry: %v", err) + } + if _, err := os.Stat(filepath.Join(dstDir, "present.txt")); err != nil { + t.Fatalf("present.txt should have been copied: %v", err) + } + if _, err := os.Stat(filepath.Join(dstDir, "vanished.txt")); !os.IsNotExist(err) { + t.Fatalf("vanished.txt should not exist in dst, got err=%v", err) + } +} + +func TestCopyPathSkipsDirRemovedAfterMkdir(t *testing.T) { + // A directory that disappears between copyPath creating dst and reading + // src's entries (e.g. renamed away mid-copy) must not fail the whole + // tree copy either. + srcDir := t.TempDir() + dstDir := t.TempDir() + + renamedAway := filepath.Join(srcDir, "007_pending") + if err := os.MkdirAll(renamedAway, 0755); err != nil { + t.Fatalf("MkdirAll: %v", err) + } + // Remove it before copyPath's own os.Stat, same effect as another + // goroutine renaming it out from under the copy. + if err := os.RemoveAll(renamedAway); err != nil { + t.Fatalf("RemoveAll: %v", err) + } + if err := copyPath(renamedAway, filepath.Join(dstDir, "007_pending")); err != nil { + t.Fatalf("copyPath on a vanished dir should be skipped, not fail: %v", err) + } +} + +func TestCopyPathFilteredSkipsEntryRemovedBetweenListingAndStat(t *testing.T) { + srcDir := t.TempDir() + dstDir := t.TempDir() + vanished := filepath.Join(srcDir, "vanished.txt") + + keepAll := func(string, os.FileInfo) bool { return true } + if err := copyPathFiltered(srcDir, vanished, filepath.Join(dstDir, "vanished.txt"), keepAll); err != nil { + t.Fatalf("copyPathFiltered on a vanished entry should be skipped, not fail: %v", err) + } +} + +func TestCopyDirContentsFilteredToleratesEntryRemovedMidWalk(t *testing.T) { + srcDir := t.TempDir() + dstDir := t.TempDir() + + if err := os.WriteFile(filepath.Join(srcDir, "a.txt"), []byte("a"), 0644); err != nil { + t.Fatalf("WriteFile: %v", err) + } + staleDir := filepath.Join(srcDir, "007_nvbandwidth_pending") + if err := os.MkdirAll(staleDir, 0755); err != nil { + t.Fatalf("MkdirAll: %v", err) + } + // Simulate the rename race by removing the dir right before the copy + // would reach it — copyDirContentsFiltered must finish copying the rest + // of the tree (a.txt) rather than aborting on the missing entry. + if err := os.RemoveAll(staleDir); err != nil { + t.Fatalf("RemoveAll: %v", err) + } + + keepAll := func(string, os.FileInfo) bool { return true } + if err := copyDirContentsFiltered(srcDir, dstDir, keepAll); err != nil { + t.Fatalf("copyDirContentsFiltered should tolerate a vanished entry: %v", err) + } + if _, err := os.Stat(filepath.Join(dstDir, "a.txt")); err != nil { + t.Fatalf("a.txt should have been copied despite the vanished sibling: %v", err) + } +} diff --git a/audit/internal/app/support_bundle.go b/audit/internal/app/support_bundle.go index 6fa49d4..ddecbfc 100644 --- a/audit/internal/app/support_bundle.go +++ b/audit/internal/app/support_bundle.go @@ -752,6 +752,17 @@ func copyDirContentsFiltered(srcDir, dstDir string, keep func(rel string, info o func copyPath(src, dst string) error { info, err := os.Stat(src) if err != nil { + // src is read from a live export dir that bee's own SAT/task runner + // keeps writing to and renaming (e.g. a task dir's "_pending" -> + // "_done" transition) concurrently with this copy — a path present + // in the parent's os.ReadDir listing a moment ago disappearing by + // the time we get here is an expected race, not a real failure. + // Skip it; the next cycle will pick up wherever it landed under its + // new name. Aborting the whole copy over one renamed-away entry is + // what previously left blackbox stuck "degraded" indefinitely. + if os.IsNotExist(err) { + return nil + } return err } if info.IsDir() { @@ -760,6 +771,9 @@ func copyPath(src, dst string) error { } entries, err := os.ReadDir(src) if err != nil { + if os.IsNotExist(err) { + return nil + } return err } for _, entry := range entries { @@ -783,6 +797,9 @@ func copyPath(src, dst string) error { } in, err := os.Open(src) if err != nil { + if os.IsNotExist(err) { + return nil + } return err } defer in.Close() @@ -800,6 +817,12 @@ func copyPath(src, dst string) error { func copyPathFiltered(rootSrc, src, dst string, keep func(rel string, info os.FileInfo) bool) error { info, err := os.Stat(src) if err != nil { + // See the matching comment in copyPath: src disappearing between the + // parent's os.ReadDir and this Stat is an expected race against + // bee's own live task-dir renames, not a real error. + if os.IsNotExist(err) { + return nil + } return err } rel, err := filepath.Rel(rootSrc, src) @@ -815,6 +838,9 @@ func copyPathFiltered(rootSrc, src, dst string, keep func(rel string, info os.Fi } entries, err := os.ReadDir(src) if err != nil { + if os.IsNotExist(err) { + return nil + } return err } for _, entry := range entries {