cloud: clean up mutagen sync ssh commands (#555)

Use the `openssh` package where possible when executing the SSH commands
for setting the mutagen sync status. This provides better logging when
commands fail.

Also rename `openssh.Client` to `Cmd` to better reflect that we're
calling an external process.

Author: gcurtis

internal/cloud/cloud.go

@@ -55,15 +55,15 @@ func Shell(w io.Writer, projectDir string, githubUsername string) error {
 	// for github username.
 	telemetryShellStartTime := time.Now()
 
-	sshClient := openssh.Client{
-		Username: username,
-		Addr:     "gateway.devbox.sh",
+	sshCmd := &openssh.Cmd{
+		Username:        username,
+		DestinationAddr: "gateway.devbox.sh",
 	}
 	// When developing we can use this env variable to point
 	// to a different gateway
 	var err error
 	if envGateway := os.Getenv("DEVBOX_GATEWAY"); envGateway != "" {
-		sshClient.Addr = envGateway
+		sshCmd.DestinationAddr = envGateway
 		err = openssh.SetupInsecureDebug(envGateway)
 	} else {
 		err = openssh.SetupDevbox()
@@ -84,7 +84,7 @@ func Shell(w io.Writer, projectDir string, githubUsername string) error {
 			color.New(color.FgGreen).Fprintln(w, "Detected existing virtual machine")
 		} else {
 			var region, vmUser string
-			vmUser, vmHostname, region, err = getVirtualMachine(sshClient)
+			vmUser, vmHostname, region, err = getVirtualMachine(sshCmd)
 			if err != nil {
 				return err
 			}
@@ -191,8 +191,8 @@ func (vm vm) redact() *vm {
 	return &vm
 }
 
-func getVirtualMachine(client openssh.Client) (vmUser, vmHost, region string, err error) {
-	sshOut, err := client.Exec("auth")
+func getVirtualMachine(sshCmd *openssh.Cmd) (vmUser, vmHost, region string, err error) {
+	sshOut, err := sshCmd.ExecRemote("auth")
 	if err != nil {
 		return "", "", "", errors.Wrapf(err, "error requesting VM")
 	}
@@ -280,19 +280,20 @@ func updateSyncStatus(mutagenSessionName, username, hostname, relProjectPathInVM
 	status := "disconnected"
 
 	// Ensure the destination directory exists
-	destServer := fmt.Sprintf("%s@%s", username, hostname)
 	destDir := fmt.Sprintf("/home/%s/.config/devbox/starship/%s", username, hyphenatePath(filepath.Base(relProjectPathInVM)))
-	remoteCmd := fmt.Sprintf("mkdir -p %s", destDir)
-	cmd := exec.Command("ssh", destServer, remoteCmd)
-	err := cmd.Run()
-	debug.Log("mkdir starship mutagen_status command: %s with error: %s", cmd, err)
+	mkdirCmd := openssh.Command(username, hostname)
+	_, err := mkdirCmd.ExecRemote(fmt.Sprintf(`mkdir -p "%s"`, destDir))
+	if err != nil {
+		debug.Log("error setting initial starship mutagen status: %v", err)
+	}
 
 	// Set an initial status
 	displayableStatus := "initial sync"
-	remoteCmd = fmt.Sprintf("echo %s > %s/mutagen_status.txt", displayableStatus, destDir)
-	cmd = exec.Command("ssh", destServer, remoteCmd)
-	err = cmd.Run()
-	debug.Log("scp starship.toml with command: %s and error: %s", cmd, err)
+	statusCmd := openssh.Command(username, hostname)
+	_, err = statusCmd.ExecRemote(fmt.Sprintf(`echo "%s" > "%s/mutagen_status.txt"`, displayableStatus, destDir))
+	if err != nil {
+		debug.Log("error setting initial starship mutagen status: %v", err)
+	}
 	time.Sleep(5 * time.Second)
 
 	debug.Log("Starting check for file sync status")
@@ -309,10 +310,11 @@ func updateSyncStatus(mutagenSessionName, username, hostname, relProjectPathInVM
 			displayableStatus = "\"watching for changes\""
 		}
 
-		remoteCmd = fmt.Sprintf("echo %s > %s/mutagen_status.txt", displayableStatus, destDir)
-		cmd = exec.Command("ssh", destServer, remoteCmd)
-		err = cmd.Run()
-		debug.Log("scp starship.toml with command: %s and error: %s", cmd, err)
+		statusCmd := openssh.Command(username, hostname)
+		_, err = statusCmd.ExecRemote(fmt.Sprintf(`echo "%s" > "%s/mutagen_status.txt"`, displayableStatus, destDir))
+		if err != nil {
+			debug.Log("error setting initial starship mutagen status: %v", err)
+		}
 		time.Sleep(5 * time.Second)
 	}
 }
@@ -335,18 +337,18 @@ func getSyncStatus(mutagenSessionName string) (string, error) {
 func copyConfigFileToVM(hostname, username, projectDir, pathInVM string) error {
 
 	// Ensure the devbox-project's directory exists in the VM
-	destServer := fmt.Sprintf("%s@%s", username, hostname)
-	cmd := exec.Command("ssh", destServer, "--", "mkdir", "-p", pathInVM)
-	err := cmd.Run()
-	debug.Log("ssh mkdir command: %s with error: %s", cmd, err)
+	mkdirCmd := openssh.Command(username, hostname)
+	_, err := mkdirCmd.ExecRemote(fmt.Sprintf(`mkdir -p "%s"`, pathInVM))
 	if err != nil {
+		debug.Log("error copying config file to VM: %v", err)
 		return errors.WithStack(err)
 	}
 
 	// Copy the config file to the devbox-project directory in the VM
+	destServer := fmt.Sprintf("%s@%s", username, hostname)
 	configFilePath := filepath.Join(projectDir, "devbox.json")
 	destPath := fmt.Sprintf("%s:%s", destServer, pathInVM)
-	cmd = exec.Command("scp", configFilePath, destPath)
+	cmd := exec.Command("scp", configFilePath, destPath)
 	err = cmd.Run()
 	debug.Log("scp devbox.json command: %s with error: %s", cmd, err)
 	return errors.WithStack(err)
@@ -358,14 +360,14 @@ func shell(username, hostname, projectDir string, shellStartTime time.Time) erro
 		return err
 	}
 
-	client := &openssh.Client{
-		Addr:           hostname,
-		PathInVM:       absoluteProjectPathInVM(username, projectPath),
-		ShellStartTime: telemetry.UnixTimestampFromTime(shellStartTime),
-		Username:       username,
+	cmd := &openssh.Cmd{
+		DestinationAddr: hostname,
+		PathInVM:        absoluteProjectPathInVM(username, projectPath),
+		ShellStartTime:  telemetry.UnixTimestampFromTime(shellStartTime),
+		Username:        username,
 	}
 	sessionErrors := newSSHSessionErrors()
-	return cloudShellErrorHandler(client.Shell(sessionErrors), sessionErrors)
+	return cloudShellErrorHandler(cmd.Shell(sessionErrors), sessionErrors)
 }
 
 // relativeProjectPathInVM refers to the project path relative to the user's

internal/cloud/openssh/client.go renamed to internal/cloud/openssh/cmd.go

@@ -17,15 +17,23 @@ import (
 	"go.jetpack.io/devbox/internal/debug"
 )
 
-type Client struct {
-	Addr           string
+type Cmd struct {
+	// DestinationAddr is a "hostname[:port]" that specifies the remote host
+	// and port to connect to.
+	DestinationAddr string
+
+	// Username is the remote login name.
+	Username string
+
 	PathInVM       string
 	ShellStartTime string // unix timestamp
-	Username       string
 }
 
-func (c *Client) Shell(w io.Writer) error {
+func Command(user, dest string) *Cmd {
+	return &Cmd{DestinationAddr: dest, Username: user}
+}
 
+func (c *Cmd) Shell(w io.Writer) error {
 	cmd := c.cmd("-t")
 	remoteCmd := fmt.Sprintf(
 		`bash -l -c "start_devbox_shell.sh \"%s\" %s"`,
@@ -39,9 +47,9 @@ func (c *Client) Shell(w io.Writer) error {
 	return logCmdRun(cmd)
 }
 
-func (c *Client) Exec(remoteCmd string) ([]byte, error) {
-	sshCmd := c.cmd()
-	sshCmd.Args = append(sshCmd.Args, remoteCmd)
+func (c *Cmd) ExecRemote(cmd string) ([]byte, error) {
+	sshCmd := c.cmd("-T")
+	sshCmd.Args = append(sshCmd.Args, cmd)
 
 	var stdout, stderr bytes.Buffer
 	sshCmd.Stdout = &stdout
@@ -58,16 +66,14 @@ func (c *Client) Exec(remoteCmd string) ([]byte, error) {
 	return stdout.Bytes(), nil
 }
 
-func (c *Client) cmd(sshArgs ...string) *exec.Cmd {
-	host, port := splitHostPort(c.Addr)
-	cmd := exec.Command("ssh", sshArgs...)
-	cmd.Args = append(cmd.Args, destination(c.Username, host))
-
-	// Add any necessary flags:
+func (c *Cmd) cmd(sshArgs ...string) *exec.Cmd {
+	host, port := splitHostPort(c.DestinationAddr)
+	cmd := exec.Command("ssh", "-l", c.Username)
 	if port != 0 && port != 22 {
 		cmd.Args = append(cmd.Args, "-p", strconv.Itoa(port))
 	}
-
+	cmd.Args = append(cmd.Args, sshArgs...)
+	cmd.Args = append(cmd.Args, host)
 	return cmd
 }
 
@@ -85,15 +91,6 @@ func splitHostPort(addr string) (host string, port int) {
 	return host, port
 }
 
-func destination(username, hostname string) string {
-	result := hostname
-	if username != "" {
-		result = username + "@" + result
-	}
-
-	return result
-}
-
 func logCmdRun(cmd *exec.Cmd) error {
 	// Use cmd.Start so we can log the pid. Don't bother writing errors to
 	// the debug log since those will be printed anyway.