From ad8642e2c294822e4ce37bdf931a9b7078b466da Mon Sep 17 00:00:00 2001
From: James Bardin <j.bardin@gmail.com>
Date: Fri, 23 Mar 2018 11:36:57 -0400
Subject: [PATCH] have remote.ExitError format errors and status

Since all use cases of ExitStatus are just putting it into fmt.Errorf,
usually with the command string, have ExitStatus do that for the caller.
---
 .../provisioners/chef/resource_provisioner.go |  3 --
 .../habitat/resource_provisioner.go           |  3 --
 .../remote-exec/resource_provisioner.go       |  5 +---
 .../salt-masterless/resource_provisioner.go   | 18 ------------
 communicator/remote/command.go                | 29 ++++++++++++-------
 5 files changed, 20 insertions(+), 38 deletions(-)

diff --git a/builtin/provisioners/chef/resource_provisioner.go b/builtin/provisioners/chef/resource_provisioner.go
index 19f226eca..ac47d2fdd 100644
--- a/builtin/provisioners/chef/resource_provisioner.go
+++ b/builtin/provisioners/chef/resource_provisioner.go
@@ -697,9 +697,6 @@ func (p *provisioner) runCommand(o terraform.UIOutput, comm communicator.Communi
 	}
 
 	if err := cmd.Wait(); err != nil {
-		if rc, ok := err.(remote.ExitError); ok {
-			return fmt.Errorf("Command %q exited with non-zero exit status: %d", cmd.Command, rc)
-		}
 		return err
 	}
 
diff --git a/builtin/provisioners/habitat/resource_provisioner.go b/builtin/provisioners/habitat/resource_provisioner.go
index d4067bd86..a5ade0a97 100644
--- a/builtin/provisioners/habitat/resource_provisioner.go
+++ b/builtin/provisioners/habitat/resource_provisioner.go
@@ -756,9 +756,6 @@ func (p *provisioner) runCommand(o terraform.UIOutput, comm communicator.Communi
 	}
 
 	if err := cmd.Wait(); err != nil {
-		if rc, ok := err.(remote.ExitError); ok {
-			return fmt.Errorf("Command %q exited with non-zero exit status: %d", cmd.Command, rc)
-		}
 		return err
 	}
 
diff --git a/builtin/provisioners/remote-exec/resource_provisioner.go b/builtin/provisioners/remote-exec/resource_provisioner.go
index a1662bc78..0f1f90117 100644
--- a/builtin/provisioners/remote-exec/resource_provisioner.go
+++ b/builtin/provisioners/remote-exec/resource_provisioner.go
@@ -198,10 +198,7 @@ func runScripts(
 		}
 
 		if err := cmd.Wait(); err != nil {
-			if rc, ok := err.(remote.ExitError); ok {
-				return fmt.Errorf("Script exited with non-zero exit status: %d", rc)
-			}
-			return fmt.Errorf("Remote command exited with error: %s", err)
+			return err
 		}
 
 		// Upload a blank follow up file in the same path to prevent residual
diff --git a/builtin/provisioners/salt-masterless/resource_provisioner.go b/builtin/provisioners/salt-masterless/resource_provisioner.go
index 2dc59a912..3f2f9b403 100644
--- a/builtin/provisioners/salt-masterless/resource_provisioner.go
+++ b/builtin/provisioners/salt-masterless/resource_provisioner.go
@@ -161,9 +161,6 @@ func applyFn(ctx context.Context) error {
 		}
 
 		if err := cmd.Wait(); err != nil {
-			if rc, ok := err.(remote.ExitError); ok {
-				return fmt.Errorf("Curl exited with non-zero exit status: %d", rc)
-			}
 			return err
 		}
 
@@ -186,9 +183,6 @@ func applyFn(ctx context.Context) error {
 		}
 
 		if err := cmd.Wait(); err != nil {
-			if rc, ok := err.(remote.ExitError); ok {
-				return fmt.Errorf("install_salt.sh exited with non-zero exit status: %d", rc)
-			}
 			return err
 		}
 	}
@@ -273,9 +267,6 @@ func applyFn(ctx context.Context) error {
 	}
 
 	if err := cmd.Wait(); err != nil {
-		if rc, ok := err.(remote.ExitError); ok {
-			return fmt.Errorf("Script exited with non-zero exit status: %d", rc)
-		}
 		return err
 	}
 	return nil
@@ -340,9 +331,6 @@ func (p *provisioner) moveFile(o terraform.UIOutput, comm communicator.Communica
 		return fmt.Errorf("Unable to move %s to %s: %s", src, dst, err)
 	}
 	if err := cmd.Wait(); err != nil {
-		if rc, ok := err.(remote.ExitError); ok {
-			return fmt.Errorf("Unable to move %s to %s: exit status: %d", src, dst, rc)
-		}
 		return err
 	}
 	return nil
@@ -358,9 +346,6 @@ func (p *provisioner) createDir(o terraform.UIOutput, comm communicator.Communic
 	}
 
 	if err := cmd.Wait(); err != nil {
-		if _, ok := err.(remote.ExitError); ok {
-			return fmt.Errorf("Non-zero exit status.")
-		}
 		return err
 	}
 	return nil
@@ -375,9 +360,6 @@ func (p *provisioner) removeDir(o terraform.UIOutput, comm communicator.Communic
 		return err
 	}
 	if err := cmd.Wait(); err != nil {
-		if _, ok := err.(remote.ExitError); ok {
-			return fmt.Errorf("Non-zero exit status.")
-		}
 		return err
 	}
 	return nil
diff --git a/communicator/remote/command.go b/communicator/remote/command.go
index ad69fdfa4..4c90368ec 100644
--- a/communicator/remote/command.go
+++ b/communicator/remote/command.go
@@ -69,19 +69,28 @@ func (c *Cmd) Wait() error {
 	c.Lock()
 	defer c.Unlock()
 
-	if c.err != nil {
-		return c.err
-	}
-
-	if c.exitStatus != 0 {
-		return ExitError(c.exitStatus)
+	if c.err != nil || c.exitStatus != 0 {
+		return &ExitError{
+			Command:    c.Command,
+			ExitStatus: c.exitStatus,
+			Err:        c.err,
+		}
 	}
 
 	return nil
 }
 
-type ExitError int
-
-func (e ExitError) Error() string {
-	return fmt.Sprintf("exit status: %d", e)
+// ExitError is returned by Wait to indicate and error executing the remote
+// command, or a non-zero exit status.
+type ExitError struct {
+	Command    string
+	ExitStatus int
+	Err        error
+}
+
+func (e *ExitError) Error() string {
+	if e.Err != nil {
+		return fmt.Sprintf("error executing %q: %v", e.Command, e.Err)
+	}
+	return fmt.Sprintf("%q exit status: %d", e.Command, e.ExitStatus)
 }