Skip to content

Commit

Permalink
roachprod: fix fileExistsOnFirstNode check
Browse files Browse the repository at this point in the history
For some reason, the current form of `fileExistsOnFirstNode`
can return `found=true` when it should return `found=false`.

This can be reproduced by running the `multitenant-upgrade`
roachtest and seeing it hang at:
```
multitenant_upgrade.go:154: test status: checking the pre-upgrade sql server still works after the system tenant binary upgrade
```
because of a `TLS handshake error`.

Note: the test is also broken because of another issue
so with this fix it should now fail with:
```
(assertions.go:333).Fail:
	Error Trace:	github.com/cockroachdb/cockroach/pkg/cmd/roachtest/tests/multitenant_upgrade.go:390
	            				github.com/cockroachdb/cockroach/pkg/cmd/roachtest/tests/multitenant_upgrade.go:189
	            				github.com/cockroachdb/cockroach/pkg/cmd/roachtest/tests/multitenant_upgrade.go:38
	            				main/pkg/cmd/roachtest/test_runner.go:1060
	            				GOROOT/src/runtime/asm_arm64.s:1172
	Error:      	Not equal:
	            	expected: [][]string{[]string{"23.1"}}
	            	actual  : [][]string{[]string{"22.2"}}

	            	Diff:
	            	--- Expected
	            	+++ Actual
	            	@@ -2,3 +2,3 @@
	            	  ([]string) (len=1) {
	            	-  (string) (len=4) "23.1"
	            	+  (string) (len=4) "22.2"
	            	  }
	Test:       	multitenant-upgrade
```

Release note: None
Epic: none
  • Loading branch information
healthy-pod committed Jun 25, 2023
1 parent a2c2c06 commit 305fcae
Showing 1 changed file with 13 additions and 2 deletions.
15 changes: 13 additions & 2 deletions pkg/roachprod/install/cluster_synced.go
Original file line number Diff line number Diff line change
Expand Up @@ -1531,8 +1531,19 @@ func (c *SyncedCluster) fileExistsOnFirstNode(
ctx context.Context, l *logger.Logger, path string,
) (bool, error) {
l.Printf("%s: checking %s", c.Name, path)
result, err := c.runCmdOnSingleNode(ctx, l, c.Nodes[0], `$(test -e `+path+`); echo $?`, false, l.Stdout, l.Stderr)
return result.Stdout == "0", err
testCmd := `$(test -e ` + path + `);`
// Do not log output to stdout/stderr because in some cases this call will be expected to exit 1.
result, err := c.runCmdOnSingleNode(ctx, l, c.Nodes[0], testCmd, true, nil, nil)
if (result.RemoteExitStatus != 0 && result.RemoteExitStatus != 1) || err != nil {
// Unexpected exit status (neither 0 nor 1) or non-nil error. Return combined output along with err returned
// from the call if it's not nil.
if err != nil {
return false, errors.Wrapf(err, "running '%s' failed with exit code=%d: got %s", testCmd, result.RemoteExitStatus, string(result.CombinedOut))
} else {
return false, errors.Newf("running '%s' failed with exit code=%d: got %s", testCmd, result.RemoteExitStatus, string(result.CombinedOut))
}
}
return result.RemoteExitStatus == 0, nil
}

// createNodeCertArguments returns a list of strings appropriate for use as
Expand Down

0 comments on commit 305fcae

Please sign in to comment.