Skip to content

Commit 70fdb98

Browse files
committed
roachtest/roachtestutil: move LoggerForCmd to roachtestutil
This helper was previously used exclusively for roachtest run calls, where a quiet logger would be created for remote commands so as to not create noise in the test log. This change extracts that helper to roachtestutil as creating quiet loggers can be useful outside of remote run calls.
1 parent 8656853 commit 70fdb98

File tree

6 files changed

+100
-49
lines changed

6 files changed

+100
-49
lines changed

pkg/cmd/roachtest/cluster.go

Lines changed: 2 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -2501,7 +2501,7 @@ func (c *clusterImpl) RunE(ctx context.Context, options install.RunOptions, args
25012501
return errors.New("No command passed")
25022502
}
25032503
nodes := option.FromInstallNodes(options.Nodes)
2504-
l, logFile, err := c.loggerForCmd(nodes, args...)
2504+
l, logFile, err := roachtestutil.LoggerForCmd(c.l, nodes, args...)
25052505
if err != nil {
25062506
return err
25072507
}
@@ -2569,7 +2569,7 @@ func (c *clusterImpl) RunWithDetails(
25692569
return nil, errors.New("No command passed")
25702570
}
25712571
nodes := option.FromInstallNodes(options.Nodes)
2572-
l, logFile, err := c.loggerForCmd(nodes, args...)
2572+
l, logFile, err := roachtestutil.LoggerForCmd(c.l, nodes, args...)
25732573
if err != nil {
25742574
return nil, err
25752575
}
@@ -2653,30 +2653,6 @@ func (c *clusterImpl) Install(
26532653
return errors.Wrap(roachprod.Install(ctx, l, c.MakeNodes(nodes), software), "cluster.Install")
26542654
}
26552655

2656-
// cmdLogFileName comes up with a log file to use for the given argument string.
2657-
func cmdLogFileName(t time.Time, nodes option.NodeListOption, args ...string) string {
2658-
logFile := fmt.Sprintf(
2659-
"run_%s_n%s_%s",
2660-
t.Format(`150405.000000000`),
2661-
nodes.String()[1:],
2662-
install.GenFilenameFromArgs(20, args...),
2663-
)
2664-
return logFile
2665-
}
2666-
2667-
func (c *clusterImpl) loggerForCmd(
2668-
node option.NodeListOption, args ...string,
2669-
) (*logger.Logger, string, error) {
2670-
logFile := cmdLogFileName(timeutil.Now(), node, args...)
2671-
2672-
// NB: we set no prefix because it's only going to a file anyway.
2673-
l, err := c.l.ChildLogger(logFile, logger.QuietStderr, logger.QuietStdout)
2674-
if err != nil {
2675-
return nil, "", err
2676-
}
2677-
return l, logFile, nil
2678-
}
2679-
26802656
// pgURLErr returns the Postgres endpoint for the specified nodes. It accepts a
26812657
// flag specifying whether the URL should include the node's internal or
26822658
// external IP address. In general, inter-cluster communication and should use

pkg/cmd/roachtest/cluster_test.go

Lines changed: 0 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@ import (
1111
"strconv"
1212
"strings"
1313
"testing"
14-
"time"
1514

1615
"github.com/cockroachdb/cockroach/pkg/cmd/roachtest/option"
1716
"github.com/cockroachdb/cockroach/pkg/cmd/roachtest/roachtestutil/task"
@@ -24,7 +23,6 @@ import (
2423
"github.com/cockroachdb/cockroach/pkg/util/version"
2524
"github.com/cockroachdb/datadriven"
2625
"github.com/cockroachdb/errors"
27-
"github.com/stretchr/testify/assert"
2826
"github.com/stretchr/testify/require"
2927
)
3028

@@ -686,21 +684,6 @@ func TestMachineTypes(t *testing.T) {
686684
})
687685
}
688686

689-
func TestCmdLogFileName(t *testing.T) {
690-
ts := time.Date(2000, 1, 1, 15, 4, 12, 0, time.Local)
691-
692-
const exp = `run_150412.000000000_n1,3-4,9_cockroach-bla-foo-ba`
693-
nodes := option.NodeListOption{1, 3, 4, 9}
694-
assert.Equal(t,
695-
exp,
696-
cmdLogFileName(ts, nodes, "./cockroach", "bla", "--foo", "bar"),
697-
)
698-
assert.Equal(t,
699-
exp,
700-
cmdLogFileName(ts, nodes, "./cockroach bla --foo bar"),
701-
)
702-
}
703-
704687
func TestVerifyLibraries(t *testing.T) {
705688
originalLibraryPaths := libraryFilePaths
706689
defer func() { libraryFilePaths = originalLibraryPaths }()

pkg/cmd/roachtest/roachtestutil/BUILD.bazel

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,10 +51,12 @@ go_test(
5151
srcs = [
5252
"commandbuilder_test.go",
5353
"load_group_test.go",
54+
"utils_test.go",
5455
],
5556
embed = [":roachtestutil"],
5657
deps = [
5758
"//pkg/cmd/roachtest/option",
59+
"@com_github_stretchr_testify//assert",
5860
"@com_github_stretchr_testify//require",
5961
],
6062
)

pkg/cmd/roachtest/roachtestutil/utils.go

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -219,6 +219,31 @@ func IfLocal(c cluster.Cluster, trueVal, falseVal string) string {
219219
return falseVal
220220
}
221221

222+
// LoggerForCmd creates a logger to a file with quiet stdout and stderr.
223+
func LoggerForCmd(
224+
l *logger.Logger, node option.NodeListOption, args ...string,
225+
) (*logger.Logger, string, error) {
226+
logFile := cmdLogFileName(timeutil.Now(), node, args...)
227+
228+
// NB: we set no prefix because it's only going to a file anyway.
229+
l, err := l.ChildLogger(logFile, logger.QuietStderr, logger.QuietStdout)
230+
if err != nil {
231+
return nil, "", err
232+
}
233+
return l, logFile, nil
234+
}
235+
236+
// cmdLogFileName comes up with a log file to use for the given argument string.
237+
func cmdLogFileName(t time.Time, nodes option.NodeListOption, args ...string) string {
238+
logFile := fmt.Sprintf(
239+
"run_%s_n%s_%s",
240+
t.Format(`150405.000000000`),
241+
nodes.String()[1:],
242+
install.GenFilenameFromArgs(20, args...),
243+
)
244+
return logFile
245+
}
246+
222247
// CheckPortBlocked returns true if a connection from a node to a port on another node
223248
// can be established. Requires nmap to be installed.
224249
func CheckPortBlocked(
Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
// Copyright 2025 The Cockroach Authors.
2+
//
3+
// Use of this software is governed by the CockroachDB Software License
4+
// included in the /LICENSE file.
5+
6+
package roachtestutil
7+
8+
import (
9+
"testing"
10+
"time"
11+
12+
"github.com/cockroachdb/cockroach/pkg/cmd/roachtest/option"
13+
"github.com/stretchr/testify/assert"
14+
)
15+
16+
func TestCmdLogFileName(t *testing.T) {
17+
ts := time.Date(2000, 1, 1, 15, 4, 12, 0, time.Local)
18+
19+
const exp = `run_150412.000000000_n1,3-4,9_cockroach-bla-foo-ba`
20+
nodes := option.NodeListOption{1, 3, 4, 9}
21+
assert.Equal(t,
22+
exp,
23+
cmdLogFileName(ts, nodes, "./cockroach", "bla", "--foo", "bar"),
24+
)
25+
assert.Equal(t,
26+
exp,
27+
cmdLogFileName(ts, nodes, "./cockroach bla --foo bar"),
28+
)
29+
}

pkg/cmd/roachtest/tests/failure_injection.go

Lines changed: 42 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -46,32 +46,68 @@ func (t *failureSmokeTest) run(
4646
}
4747
// Make sure to cleanup the failure mode even if the test fails.
4848
defer func() {
49-
err = errors.CombineErrors(err, failureMode.Cleanup(ctx, l, t.args))
49+
quietLogger, file, logErr := roachtestutil.LoggerForCmd(l, c.CRDBNodes(), t.testName, "cleanup")
50+
if logErr != nil {
51+
l.Printf("failed to create logger for cleanup: %v", logErr)
52+
quietLogger = l
53+
}
54+
l.Printf("%s: Running Cleanup(); details in %s.log", t.failureName, file)
55+
err = errors.CombineErrors(err, failureMode.Cleanup(ctx, quietLogger, t.args))
5056
}()
51-
if err = failureMode.Setup(ctx, l, t.args); err != nil {
57+
58+
quietLogger, file, err := roachtestutil.LoggerForCmd(l, c.CRDBNodes(), t.testName, "setup")
59+
if err != nil {
60+
return err
61+
}
62+
l.Printf("%s: Running Setup(); details in %s.log", t.failureName, file)
63+
if err = failureMode.Setup(ctx, quietLogger, t.args); err != nil {
5264
return err
5365
}
54-
if err = failureMode.Inject(ctx, l, t.args); err != nil {
66+
67+
quietLogger, file, err = roachtestutil.LoggerForCmd(l, c.CRDBNodes(), t.testName, "inject")
68+
if err != nil {
69+
return err
70+
}
71+
l.Printf("%s: Running Inject(); details in %s.log", t.failureName, file)
72+
if err = failureMode.Inject(ctx, quietLogger, t.args); err != nil {
5573
return err
5674
}
5775

5876
// Allow the failure to take effect.
59-
if err = failureMode.WaitForFailureToPropagate(ctx, l, t.args); err != nil {
77+
quietLogger, file, err = roachtestutil.LoggerForCmd(l, c.CRDBNodes(), t.testName, "wait for propagate")
78+
if err != nil {
79+
return err
80+
}
81+
l.Printf("%s: Running WaitForFailureToPropagate(); details in %s.log", t.failureName, file)
82+
if err = failureMode.WaitForFailureToPropagate(ctx, quietLogger, t.args); err != nil {
6083
return err
6184
}
6285

86+
l.Printf("validating failure was properly injected")
6387
if err = t.validateFailure(ctx, l, c); err != nil {
6488
return err
6589
}
66-
if err = failureMode.Restore(ctx, l, t.args); err != nil {
90+
91+
quietLogger, file, err = roachtestutil.LoggerForCmd(l, c.CRDBNodes(), t.testName, "restore")
92+
if err != nil {
93+
return err
94+
}
95+
l.Printf("%s: Running Restore(); details in %s.log", t.failureName, file)
96+
if err = failureMode.Restore(ctx, quietLogger, t.args); err != nil {
6797
return err
6898
}
6999

70100
// Allow the cluster to return to normal.
71-
if err = failureMode.WaitForFailureToRestore(ctx, l, t.args); err != nil {
101+
quietLogger, file, err = roachtestutil.LoggerForCmd(l, c.CRDBNodes(), t.testName, "wait for restore")
102+
if err != nil {
103+
return err
104+
}
105+
l.Printf("%s: Running WaitForFailureToRestore(); details in %s.log", t.failureName, file)
106+
if err = failureMode.WaitForFailureToRestore(ctx, quietLogger, t.args); err != nil {
72107
return err
73108
}
74109

110+
l.Printf("validating failure was properly restored")
75111
return t.validateRestore(ctx, l, c)
76112
}
77113

0 commit comments

Comments
 (0)