From d378625656a04ce2292d162391c77cd93785f37f Mon Sep 17 00:00:00 2001 From: Renato Costa Date: Mon, 6 Feb 2023 16:38:06 -0500 Subject: [PATCH] roachtest: return error in StopCockroachGracefullyOnNode That function misleadingly returned an (always nil) error, calling `t.Fatal()` functions in it. The calls to `Stop` have been replaced with calls to `StopE`. Epic: none Release note: None --- pkg/cmd/roachtest/cluster.go | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/pkg/cmd/roachtest/cluster.go b/pkg/cmd/roachtest/cluster.go index b6629203a625..2912588e41e6 100644 --- a/pkg/cmd/roachtest/cluster.go +++ b/pkg/cmd/roachtest/cluster.go @@ -1047,14 +1047,14 @@ func (c *clusterImpl) StopCockroachGracefullyOnNode( gracefulOpts.RoachprodOpts.Sig = 15 // SIGTERM gracefulOpts.RoachprodOpts.Wait = true gracefulOpts.RoachprodOpts.MaxWait = 60 - c.Stop(ctx, l, gracefulOpts, c.Node(node)) - // NB: we still call Stop to make sure the process is dead when we try - // to restart it (or we'll catch an error from the RocksDB dir being - // locked). This won't happen unless run with --local due to timing. - c.Stop(ctx, l, option.DefaultStopOpts(), c.Node(node)) - // TODO(tschottdorf): should return an error. I doubt that we want to - // call these *testing.T-style methods on goroutines. - return nil + if err := c.StopE(ctx, l, gracefulOpts, c.Node(node)); err != nil { + return err + } + + // NB: we still call Stop to make sure the process is dead when we + // try to restart it (in case it takes longer than `MaxWait` for it + // to finish). + return c.StopE(ctx, l, option.DefaultStopOpts(), c.Node(node)) } // Save marks the cluster as "saved" so that it doesn't get destroyed.