Skip to content

Commit

Permalink
roachtest: use first transient error when checking for flakes
Browse files Browse the repository at this point in the history
Previously, roachtest would only look at the outermost error in a
chain that matched a `TransientError` (or `ErrorWithOwnership`) when
checking for flakes. However, that is in most cases *not* what we
want: if a transient error wraps another transient error, the actual
reason for the failure is the original (wrapped) error.

Informs: cockroachdb#123887

Release note: None
  • Loading branch information
renatolabs committed May 23, 2024
1 parent 59ec42d commit 2fef722
Show file tree
Hide file tree
Showing 4 changed files with 57 additions and 2 deletions.
15 changes: 15 additions & 0 deletions pkg/cmd/roachtest/github_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -323,6 +323,21 @@ func TestCreatePostRequest(t *testing.T) {
expectedMessagePrefix: testName + " failed",
expectedLabels: []string{"T-testeng", "X-infra-flake"},
},
// 11. When a transient error happens as a result of *another*
// transient error, the corresponding issue uses the first
// transient error in the chain.
{
failures: []failure{
createFailure(rperrors.TransientFailure(
rperrors.NewSSHError(errors.New("oops")), "some_problem",
)),
},
expectedPost: true,
expectedTeam: "@cockroachdb/test-eng",
expectedName: "ssh_problem",
expectedMessagePrefix: testName + " failed",
expectedLabels: []string{"T-testeng", "X-infra-flake"},
},
}

reg := makeTestRegistry()
Expand Down
23 changes: 21 additions & 2 deletions pkg/cmd/roachtest/test_impl.go
Original file line number Diff line number Diff line change
Expand Up @@ -492,14 +492,33 @@ func (t *testImpl) failureMsg() string {
// target. If it does, `refError` is set to that target error value
// and returns true. Otherwise, it returns false.
func failuresMatchingError(failures []failure, refError any) bool {
// unwrap unwraps the error passed to find the innermost error in the
// chain that satisfies the `refError` provided.
unwrap := func(err error) bool {
var matched bool
for {
if isRef := errors.As(err, refError); !isRef {
break
}

matched = true
err = errors.Unwrap(err)
if err == nil {
break
}
}

return matched
}

for _, f := range failures {
for _, err := range f.errors {
if errors.As(err, refError) {
if unwrap(err) {
return true
}
}

if errors.As(f.squashedErr, refError) {
if unwrap(f.squashedErr) {
return true
}
}
Expand Down
17 changes: 17 additions & 0 deletions pkg/cmd/roachtest/testdata/help_command_createpost_11.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
echo
----
----


See: [roachtest README](https://github.com/cockroachdb/cockroach/blob/master/pkg/cmd/roachtest/README.md)



See: [How To Investigate \(internal\)](https://cockroachlabs.atlassian.net/l/c/SSSBr8c7)



See: [Grafana](https://go.crdb.dev/roachtest-grafana//github-test/1689957243000/1689957853000)

----
----
4 changes: 4 additions & 0 deletions pkg/roachprod/errors/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,10 @@ func (te TransientError) Is(other error) bool {
return errors.Is(te.Err, other)
}

func (te TransientError) Unwrap() error {
return te.Err
}

func (te TransientError) ExitCode() int {
return transientExitCode
}
Expand Down

0 comments on commit 2fef722

Please sign in to comment.