Skip to content

Commit

Permalink
roachprod: don't pollute logs with verbose ssh logs
Browse files Browse the repository at this point in the history
I added verbose logging in cockroachdb#37483 but made the poor decision to spew it
to stderr when the command exited nonzero. As a consequence, many test
failures are now obfuscated by high-verbosity ssh logs.

Change the behavior so that the error message is wrapped with a path to
the logs. We grab the contents of `~/.roachprod` in CI, so we'll be able
to access the logs in the artifacts (`roachprod_state`).

```
$ ./bin/roachprod ssh tobias-test -- echo hi
hi
$ ls ~/.roachprod/debug/
$ ./bin/roachprod ssh tobias-test -- false
Error:  ssh verbose log retained in /Users/tschottdorf/.roachprod/debug/ssh_35.231.83.24_2019-05-20T12:53:49Z: exit status 1
$ ls ~/.roachprod/debug/
ssh_35.231.83.24_2019-05-20T12:53:49Z
```

Touches cockroachdb#36963.

Release note: None
  • Loading branch information
tbg committed May 20, 2019
1 parent 7b26514 commit 2295bd0
Show file tree
Hide file tree
Showing 4 changed files with 20 additions and 11 deletions.
7 changes: 4 additions & 3 deletions pkg/cmd/roachprod/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,8 @@ func init() {
// take place on the local machine. Later in the refactoring,
// this ought to be replaced by a LocalCloudProvider or somesuch.
const (
DefaultHostDir = "${HOME}/.roachprod/hosts"
EmailDomain = "@cockroachlabs.com"
Local = "local"
DefaultDebugDir = "${HOME}/.roachprod/debug"
DefaultHostDir = "${HOME}/.roachprod/hosts"
EmailDomain = "@cockroachlabs.com"
Local = "local"
)
6 changes: 5 additions & 1 deletion pkg/cmd/roachprod/hosts.go
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,9 @@ func loadClusters() error {
return err
}

debugDir := os.ExpandEnv(config.DefaultDebugDir)
_ = os.MkdirAll(debugDir, 0755)

for _, file := range files {
if !file.Mode().IsRegular() {
continue
Expand All @@ -132,7 +135,8 @@ func loadClusters() error {
lines := strings.Split(string(contents), "\n")

c := &install.SyncedCluster{
Name: file.Name(),
Name: file.Name(),
DebugDir: debugDir,
}

for _, l := range lines {
Expand Down
5 changes: 4 additions & 1 deletion pkg/cmd/roachprod/install/cluster_synced.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,9 @@ type SyncedCluster struct {
Quiet bool
// AuthorizedKeys is used by SetupSSH to add additional authorized keys.
AuthorizedKeys []byte

// Used to stash debug information.
DebugDir string
}

func (c *SyncedCluster) host(index int) string {
Expand Down Expand Up @@ -144,7 +147,7 @@ func (c *SyncedCluster) newSession(i int) (session, error) {
if c.IsLocal() {
return newLocalSession(), nil
}
return newRemoteSession(c.user(i), c.host(i))
return newRemoteSession(c.user(i), c.host(i), c.DebugDir)
}

// Stop TODO(peter): document
Expand Down
13 changes: 7 additions & 6 deletions pkg/cmd/roachprod/install/session.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ import (
"context"
"fmt"
"io"
"io/ioutil"
"os"
"os/exec"
"path/filepath"
Expand Down Expand Up @@ -52,9 +51,9 @@ type remoteSession struct {
logfile string // captures ssh -vvv
}

func newRemoteSession(user, host string) (*remoteSession, error) {
func newRemoteSession(user, host string, logdir string) (*remoteSession, error) {
logfile := filepath.Join(
os.TempDir(),
logdir,
fmt.Sprintf("ssh_%s_%s", host, timeutil.Now().Format(time.RFC3339)),
)
args := []string{
Expand All @@ -80,8 +79,8 @@ func newRemoteSession(user, host string) (*remoteSession, error) {

func (s *remoteSession) errWithDebug(err error) error {
if err != nil {
debug, _ := ioutil.ReadFile(s.logfile)
err = errors.Wrapf(err, "ssh verbose log:\n%s\n%s", s.Cmd.Args, debug)
err = errors.Wrapf(err, "ssh verbose log retained in %s", s.logfile)
s.logfile = "" // prevent removal on close
}
return err
}
Expand Down Expand Up @@ -138,8 +137,10 @@ func (s *remoteSession) Wait() error {
}

func (s *remoteSession) Close() {
_ = os.Remove(s.logfile)
s.cancel()
if s.logfile != "" {
_ = os.Remove(s.logfile)
}
}

type localSession struct {
Expand Down

0 comments on commit 2295bd0

Please sign in to comment.