failure-injection: fix cgroup disk stall for logs

The previous implementation of stalling logs had a comment
about needing to temporarily move logs while a symlink was
created. This comment changes the disk stall failure mode
to actually do so.

Author: DarrylWong

pkg/cmd/roachtest/tests/failure_injection.go

@@ -557,6 +557,71 @@ var cgroupsDiskStallTests = func(c cluster.Cluster) []failureSmokeTest {
 	return tests
 }
 
+var cgroupStallLogsTest = func(c cluster.Cluster) failureSmokeTest {
+	nodes := c.CRDBNodes()
+	rand.Shuffle(len(nodes), func(i, j int) {
+		nodes[i], nodes[j] = nodes[j], nodes[i]
+	})
+	// We only want to stall one node for this test. If we stall writes on a quorum
+	// of nodes, then auth-session login will fail even if logs are not stalled.
+	stalledNode := c.Node(nodes[0])
+	unaffectedNode := c.Node(nodes[1])
+
+	getSessionCookie := func(
+		ctx context.Context,
+		l *logger.Logger,
+		c cluster.Cluster,
+		node option.NodeListOption,
+	) bool {
+		loginCmd := fmt.Sprintf(
+			"%s auth-session login root --url={pgurl%s} --certs-dir ./certs --only-cookie",
+			test.DefaultCockroachPath, node,
+		)
+		err := c.RunE(ctx, option.WithNodes(node), loginCmd)
+		return err == nil
+	}
+
+	return failureSmokeTest{
+		testName:    fmt.Sprintf("%s/WritesStalled=true/LogsStalled=true", failures.CgroupsDiskStallName),
+		failureName: failures.CgroupsDiskStallName,
+		args: failures.DiskStallArgs{
+			StallWrites:  true,
+			RestartNodes: true,
+			Nodes:        stalledNode.InstallNodes(),
+			StallLogs:    true,
+		},
+		validateFailure: func(ctx context.Context, l *logger.Logger, c cluster.Cluster, f *failures.Failer) error {
+			// Confirm symlink exists
+			if err := c.RunE(ctx, option.WithNodes(stalledNode), "test -L logs"); err != nil {
+				return errors.Wrapf(err, "`logs` is not a symlink on node %d", stalledNode)
+			}
+
+			// The cockroach-sql-auth.log file is appended to each time an authenticated session event
+			// occurs, e.g. a client logging in. If we attempt to fetch a cookie from the stalled node,
+			// we should expect to see our request time out, as it will be unable to write to the log.
+			if getSessionCookie(ctx, l, c, stalledNode) {
+				return errors.Errorf("was able to successfully get session cookie from stalled node %d", stalledNode)
+			}
+
+			// The unaffected node should be able to write to the log, so we should be able to
+			// get the session cookie with no issues.
+			if !getSessionCookie(ctx, l, c, unaffectedNode) {
+				return errors.Errorf("was unable to get session cookie from unaffected node %d", unaffectedNode)
+			}
+			return nil
+		},
+		validateRecover: func(ctx context.Context, l *logger.Logger, c cluster.Cluster, f *failures.Failer) error {
+			if !getSessionCookie(ctx, l, c, stalledNode) {
+				return errors.Errorf("was unable to get session cookie from stalled node %d", stalledNode)
+			}
+			return nil
+		},
+		workload: func(ctx context.Context, c cluster.Cluster, args ...string) error {
+			return nil
+		},
+	}
+}
+
 var dmsetupDiskStallTest = func(c cluster.Cluster) failureSmokeTest {
 	rng, _ := randutil.NewPseudoRand()
 	// SeededRandGroups only returns an error if the requested size is larger than the
@@ -776,9 +841,7 @@ func defaultFailureSmokeTestWorkload(ctx context.Context, c cluster.Cluster, arg
 	return c.RunE(ctx, option.WithNodes(c.WorkloadNode()), cmd)
 }
 
-func setupFailureSmokeTests(
-	ctx context.Context, t test.Test, c cluster.Cluster, fr *failures.FailureRegistry,
-) error {
+func setupFailureSmokeTests(ctx context.Context, t test.Test, c cluster.Cluster) error {
 	// Download any dependencies needed.
 	if err := c.Install(ctx, t.L(), c.CRDBNodes(), "nmap"); err != nil {
 		return err
@@ -804,7 +867,7 @@ func setupFailureSmokeTests(
 func runFailureSmokeTest(ctx context.Context, t test.Test, c cluster.Cluster, noopFailer bool) {
 	fr := failures.NewFailureRegistry()
 	fr.Register()
-	if err := setupFailureSmokeTests(ctx, t, c, fr); err != nil {
+	if err := setupFailureSmokeTests(ctx, t, c); err != nil {
 		t.Error(err)
 	}
 
@@ -815,6 +878,7 @@ func runFailureSmokeTest(ctx context.Context, t test.Test, c cluster.Cluster, no
 		latencyTest(c),
 		dmsetupDiskStallTest(c),
 		resetVMTests(c),
+		cgroupStallLogsTest(c),
 	}
 	failureSmokeTests = append(failureSmokeTests, cgroupsDiskStallTests(c)...)
 	failureSmokeTests = append(failureSmokeTests, processKillTests(c)...)

pkg/roachprod/failureinjection/failures/disk_stall.go

@@ -17,6 +17,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/roachprod"
 	"github.com/cockroachdb/cockroach/pkg/roachprod/install"
 	"github.com/cockroachdb/cockroach/pkg/roachprod/logger"
+	"github.com/cockroachdb/cockroach/pkg/util/timeutil"
 	"github.com/cockroachdb/errors"
 )
 
@@ -63,27 +64,58 @@ func (s *CGroupDiskStaller) Description() string {
 func (s *CGroupDiskStaller) Setup(ctx context.Context, l *logger.Logger, args FailureArgs) error {
 	diskStallArgs := args.(DiskStallArgs)
 
-	// To stall logs we need to create a symlink that points to our stalled
-	// store directory. In order to do that we need to temporarily move the
-	// existing logs directory and copy the contents over after. If a symlink
-	// already exists, don't attempt to recreate it.
+	// Cgroup throttles a specific disk device, however our logs directory
+	// is usually mounted on a different device than our cockroach data. To
+	// stall both logs and the cockroach process, they must both be mounted
+	// on the same device. To do so, we create a new logs directory in our
+	// stalled device, e.g. {store-dir}/logs, and create a symlink from logs
+	// to that directory.
+	//
+	// If the cluster is already running, we want to make sure we don't lose
+	// any existing logs. We first move our existing logs to a temporary
+	// directory, before copying them into the new symlinked directory.
 	if diskStallArgs.StallLogs {
-		createSymlinkCmd := `
+		// N.B. Because multiple FS operations aren't atomic, we must temporarily
+		// stop the cluster before moving the logs directory.
+		if diskStallArgs.RestartNodes {
+			if err := s.StopCluster(ctx, l, roachprod.DefaultStopOpts()); err != nil {
+				return err
+			}
+		}
+
+		tmpLogsDir := fmt.Sprintf("tmp-disk-stall-%d", timeutil.Now().Unix())
+		createSymlinkCmd := fmt.Sprintf(`
 if [ ! -L logs ]; then
-	echo "creating symlink";
-	mkdir -p {store-dir}/logs;
-	ln -s {store-dir}/logs logs;
+    if [ -e logs ]; then
+				echo "moving existing logs to tmp directory %[1]s"
+				mv logs %[1]s
+    fi
+    mkdir -p {store-dir}/logs
+		echo "creating symlink logs -> {store-dir}/logs";
+    ln -s {store-dir}/logs logs
+		if [ -e %[1]s ]; then
+				echo "copying tmp directory %[1]s to logs";
+				cp -va %[1]s/* logs/
+		fi
+else
+		echo "symlink already exists, not creating";
 fi
-`
+`, tmpLogsDir)
 		if err := s.Run(ctx, l, diskStallArgs.Nodes, createSymlinkCmd); err != nil {
 			return err
 		}
+		if diskStallArgs.RestartNodes {
+			if err := s.StartCluster(ctx, l); err != nil {
+				return err
+			}
+		}
 	}
 	return nil
 }
 func (s *CGroupDiskStaller) Cleanup(ctx context.Context, l *logger.Logger, args FailureArgs) error {
+	diskStallArgs := args.(DiskStallArgs)
 	stallType := []bandwidthType{readBandwidth, writeBandwidth}
-	nodes := args.(DiskStallArgs).Nodes
+	nodes := diskStallArgs.Nodes
 
 	// Setting cgroup limits is idempotent so attempt to unlimit reads/writes in case
 	// something went wrong in Recover.
@@ -92,9 +124,22 @@ func (s *CGroupDiskStaller) Cleanup(ctx context.Context, l *logger.Logger, args
 		l.PrintfCtx(ctx, "error unstalling the disk; stumbling on: %v", err)
 	}
 	if args.(DiskStallArgs).StallLogs {
-		if err = s.Run(ctx, l, nodes, "unlink logs/logs"); err != nil {
+		// Cleanup our symlinked logs. Similar to Setup(), we must first stop the cluster
+		// to stop the cockroach process from concurrently writing to the logs directory.
+		if err = s.Run(ctx, l, nodes, "unlink logs"); err != nil {
 			return err
 		}
+		if diskStallArgs.RestartNodes {
+			if err = s.StopCluster(ctx, l, roachprod.DefaultStopOpts()); err != nil {
+				return err
+			}
+		}
+		if err = s.Run(ctx, l, nodes, "cp -r {store-dir}/logs logs"); err != nil {
+			return err
+		}
+		if diskStallArgs.RestartNodes {
+			return s.StartCluster(ctx, l)
+		}
 	}
 	return nil
 }