From 68912f322489824a151f9bf3d5710e21c7f44719 Mon Sep 17 00:00:00 2001 From: Peter Mattis Date: Sat, 12 Oct 2019 11:06:01 -0400 Subject: [PATCH 1/3] roachtest: remove unnecessary tmp dir creation Release note: None --- pkg/cmd/roachtest/disk_stall.go | 9 --------- 1 file changed, 9 deletions(-) diff --git a/pkg/cmd/roachtest/disk_stall.go b/pkg/cmd/roachtest/disk_stall.go index 30c92498bafb..70562cfba236 100644 --- a/pkg/cmd/roachtest/disk_stall.go +++ b/pkg/cmd/roachtest/disk_stall.go @@ -13,9 +13,7 @@ package main import ( "context" "fmt" - "io/ioutil" "math/rand" - "os" "strings" "time" @@ -48,13 +46,6 @@ func runDiskStalledDetection( ctx context.Context, t *test, c *cluster, affectsLogDir bool, affectsDataDir bool, ) { n := c.Node(1) - tmpDir, err := ioutil.TempDir("", "stalled") - if err != nil { - t.Fatal(err) - } - defer func() { - _ = os.RemoveAll(tmpDir) - }() c.Put(ctx, cockroach, "./cockroach") c.Run(ctx, n, "sudo umount -f {store-dir}/faulty || true") From 16e779439b90e76fa1c3d766fc71efd8254e83c6 Mon Sep 17 00:00:00 2001 From: Peter Mattis Date: Sat, 12 Oct 2019 13:18:41 -0400 Subject: [PATCH 2/3] storage/engine: force mode 0755 for the cockroach-temp* directory Noticed while debugging the `disk-stall` roachtest. Forcing 0755 permissions on the `cockroach-temp*` directory is done for consistency with every other directory created by cockroach. Release note: None --- pkg/storage/engine/temp_dir.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/pkg/storage/engine/temp_dir.go b/pkg/storage/engine/temp_dir.go index 1ce504f9ad78..7af75d7b29cc 100644 --- a/pkg/storage/engine/temp_dir.go +++ b/pkg/storage/engine/temp_dir.go @@ -35,6 +35,12 @@ func CreateTempDir(parentDir, prefix string, stopper *stop.Stopper) (string, err return "", err } + // TempDir creates a directory with permissions 0700. Manually change the + // permissions to be 0755 like every other directory created by cockroach. + if err := os.Chmod(tempPath, 0755); err != nil { + return "", err + } + absPath, err := filepath.Abs(tempPath) if err != nil { return "", err From ceda2dfff8080463329206ebf4c63a590927d733 Mon Sep 17 00:00:00 2001 From: Peter Mattis Date: Sat, 12 Oct 2019 13:20:06 -0400 Subject: [PATCH 3/3] cmd/roachprod: ensure wipe really wipes Previously, `roachprod wipe` was being run by the default user (now `ubuntu`) which meant it couldn't remove files created by root. It was also accidentally swallowing errors. Now `roachprod wipe` runs as root when wiping remote nodes, ensuring it really cleans up any files. The failure to remove root owned files was causing the `disk-stall` tests to be flaky because files created through `charybdefs` are created as root. These root owned files could be removed, but cockroach also creates a root owned directory and the files in that root owned directory could not be removed. If two of the `disk-stall` tests ran on the same cluster, the first could end up leaving around files that the second encountered and complained about. Fixes #41530 Release note: None --- pkg/cmd/roachprod/install/cluster_synced.go | 8 ++++---- pkg/cmd/roachtest/disk_stall.go | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/pkg/cmd/roachprod/install/cluster_synced.go b/pkg/cmd/roachprod/install/cluster_synced.go index f1b6f42fd57a..fe3eec73efd7 100644 --- a/pkg/cmd/roachprod/install/cluster_synced.go +++ b/pkg/cmd/roachprod/install/cluster_synced.go @@ -209,12 +209,12 @@ func (c *SyncedCluster) Wipe(preserveCerts bool) { cmd += fmt.Sprintf(`rm -fr ${HOME}/local/%d/%s ;`, c.Nodes[i], dir) } } else { - cmd = `find /mnt/data* -maxdepth 1 -type f -exec rm -f {} \; ; -rm -fr /mnt/data*/{auxiliary,local,tmp,cassandra,cockroach,cockroach-temp*,mongo-data} \; ; -rm -fr logs ; + cmd = `sudo find /mnt/data* -maxdepth 1 -type f -exec rm -f {} \; && +sudo rm -fr /mnt/data*/{auxiliary,local,tmp,cassandra,cockroach,cockroach-temp*,mongo-data} && +sudo rm -fr logs && ` if !preserveCerts { - cmd += "rm -fr certs* ;\n" + cmd += "sudo rm -fr certs* ;\n" } } return sess.CombinedOutput(cmd) diff --git a/pkg/cmd/roachtest/disk_stall.go b/pkg/cmd/roachtest/disk_stall.go index 70562cfba236..bbe8696592f0 100644 --- a/pkg/cmd/roachtest/disk_stall.go +++ b/pkg/cmd/roachtest/disk_stall.go @@ -51,7 +51,7 @@ func runDiskStalledDetection( c.Run(ctx, n, "sudo umount -f {store-dir}/faulty || true") c.Run(ctx, n, "mkdir -p {store-dir}/{real,faulty} || true") // Make sure the actual logs are downloaded as artifacts. - c.Run(ctx, n, "rm -f logs/real && ln -s {store-dir}/real/logs logs/real || true") + c.Run(ctx, n, "rm -f logs && ln -s {store-dir}/real/logs logs || true") t.Status("setting up charybdefs")