Merge #41552

41552: cmd/roachprod: ensure wipe really wipes r=tbg a=petermattis

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

Co-authored-by: Peter Mattis <[email protected]>

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)

pkg/cmd/roachtest/disk_stall.go

@@ -13,9 +13,7 @@ package main
 import (
 	"context"
 	"fmt"
-	"io/ioutil"
 	"math/rand"
-	"os"
 	"strings"
 	"time"
 
@@ -48,19 +46,12 @@ 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")
 	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")
 

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