Merge pull request #298 from hashicorp/winrm_error_handling

fix(winrm): catch cmd err from winrm

Author: anurag5sh

packer/communicator.go

@@ -123,11 +123,6 @@ func (r *RemoteCmd) RunWithUi(ctx context.Context, c Communicator, ui Ui) error
 		r.Stderr = io.MultiWriter(r.Stderr, stderr_w)
 	}
 
-	// Start the command
-	if err := c.Start(ctx, r); err != nil {
-		return err
-	}
-
 	// Create the channels we'll use for data
 	stdoutCh := iochan.DelimReader(stdout_r, '\n')
 	stderrCh := iochan.DelimReader(stderr_r, '\n')
@@ -140,32 +135,49 @@ func (r *RemoteCmd) RunWithUi(ctx context.Context, c Communicator, ui Ui) error
 	}()
 
 	// Loop and get all our output
-OutputLoop:
-	for {
-		select {
-		case output := <-stderrCh:
-			if output != "" {
-				ui.Error(r.cleanOutputLine(output))
-			}
-		case output := <-stdoutCh:
-			if output != "" {
-				ui.Message(r.cleanOutputLine(output))
+	var wg sync.WaitGroup
+	var ctxErr error
+	wg.Add(1)
+
+	go func() {
+		defer wg.Done()
+		exiting := false
+
+		for !exiting {
+			select {
+			case output, ok := <-stderrCh:
+				if ok && output != "" {
+					ui.Error(r.cleanOutputLine(output))
+				}
+			case output, ok := <-stdoutCh:
+				if ok && output != "" {
+					ui.Say(r.cleanOutputLine(output))
+				}
+			case <-r.exitCh:
+				exiting = true
+			case <-ctx.Done():
+				ctxErr = ctx.Err()
+				return
 			}
-		case <-r.exitCh:
-			break OutputLoop
-		case <-ctx.Done():
-			return ctx.Err()
 		}
-	}
 
-	// Make sure we finish off stdout/stderr because we may have gotten
-	// a message from the exit channel before finishing these first.
-	for output := range stdoutCh {
-		ui.Message(r.cleanOutputLine(output))
+		// Drain remaining messages from stdout and stderr if channels are still open.
+		for output := range stdoutCh {
+			ui.Say(r.cleanOutputLine(output))
+		}
+		for output := range stderrCh {
+			ui.Error(r.cleanOutputLine(output))
+		}
+	}()
+
+	// Start the command
+	if err := c.Start(ctx, r); err != nil {
+		return err
 	}
 
-	for output := range stderrCh {
-		ui.Error(r.cleanOutputLine(output))
+	wg.Wait()
+	if ctxErr != nil {
+		return ctxErr
 	}
 
 	return nil

sdk-internals/communicator/winrm/communicator.go

@@ -16,7 +16,6 @@ import (
 	"os"
 	"path/filepath"
 	"strings"
-	"sync"
 
 	packersdk "github.com/hashicorp/packer-plugin-sdk/packer"
 	"github.com/masterzen/winrm"
@@ -82,50 +81,12 @@ func New(config *Config) (*Communicator, error) {
 
 // Start implementation of communicator.Communicator interface
 func (c *Communicator) Start(ctx context.Context, rc *packersdk.RemoteCmd) error {
-	shell, err := c.client.CreateShell()
-	if err != nil {
-		return err
-	}
-
 	log.Printf("[INFO] starting remote command: %s", rc.Command)
-	cmd, err := shell.Execute(rc.Command)
-	if err != nil {
-		return err
-	}
-
-	go runCommand(shell, cmd, rc)
-	return nil
-}
+	exitCode, err := c.client.Run(rc.Command, rc.Stdout, rc.Stderr)
 
-func runCommand(shell *winrm.Shell, cmd *winrm.Command, rc *packersdk.RemoteCmd) {
-	defer shell.Close()
-	var wg sync.WaitGroup
-
-	copyFunc := func(w io.Writer, r io.Reader) {
-		defer wg.Done()
-		io.Copy(w, r)
-	}
-
-	if rc.Stdout != nil && cmd.Stdout != nil {
-		wg.Add(1)
-		go copyFunc(rc.Stdout, cmd.Stdout)
-	} else {
-		log.Printf("[WARN] Failed to read stdout for command '%s'", rc.Command)
-	}
-
-	if rc.Stderr != nil && cmd.Stderr != nil {
-		wg.Add(1)
-		go copyFunc(rc.Stderr, cmd.Stderr)
-	} else {
-		log.Printf("[WARN] Failed to read stderr for command '%s'", rc.Command)
-	}
-
-	cmd.Wait()
-	wg.Wait()
-
-	code := cmd.ExitCode()
-	log.Printf("[INFO] command '%s' exited with code: %d", rc.Command, code)
-	rc.SetExited(code)
+	rc.SetExited(exitCode)
+	log.Printf("[INFO] command '%s' exited with code: %d", rc.Command, exitCode)
+	return err
 }
 
 // Upload implementation of communicator.Communicator interface