Changes requested in PR hashicorp#6243

- Logging error if vmconnect.exe fails. - Using StepRun struct rather than StateBag for command Cancel function - Better handling in Disconnect when headless is true or vmconnect failed in Start
enrico-usai · May 14, 2018 · 29c4b44 · 29c4b44
1 parent fc734b6
commit 29c4b44
Show file tree

Hide file tree

Showing 5 changed files with 22 additions and 14 deletions.
diff --git a/builder/hyperv/common/driver.go b/builder/hyperv/common/driver.go
@@ -115,7 +115,7 @@ type Driver interface {
 	UnmountFloppyDrive(string) error
 
 	// Connect connects to a VM specified by the name given.
-	Connect(string) context.CancelFunc
+	Connect(string) (context.CancelFunc, error)
 
 	// Disconnect disconnects to a VM specified by the context cancel function.
 	Disconnect(context.CancelFunc)

diff --git a/builder/hyperv/common/driver_mock.go b/builder/hyperv/common/driver_mock.go
@@ -248,6 +248,7 @@ type DriverMock struct {
 	Connect_Called bool
 	Connect_VmName string
 	Connect_Cancel context.CancelFunc
+	Connect_Err    error
 
 	Disconnect_Called bool
 	Disconnect_Cancel context.CancelFunc
@@ -565,10 +566,10 @@ func (d *DriverMock) UnmountFloppyDrive(vmName string) error {
 	return d.UnmountFloppyDrive_Err
 }
 
-func (d *DriverMock) Connect(vmName string) context.CancelFunc {
+func (d *DriverMock) Connect(vmName string) (context.CancelFunc, error) {
 	d.Connect_Called = true
 	d.Connect_VmName = vmName
-	return d.Connect_Cancel
+	return d.Connect_Cancel, d.Connect_Err
 }
 
 func (d *DriverMock) Disconnect(cancel context.CancelFunc) {

diff --git a/builder/hyperv/common/driver_ps_4.go b/builder/hyperv/common/driver_ps_4.go
@@ -350,7 +350,7 @@ func (d *HypervPS4Driver) verifyHypervPermissions() error {
 }
 
 // Connect connects to a VM specified by the name given.
-func (d *HypervPS4Driver) Connect(vmName string) context.CancelFunc {
+func (d *HypervPS4Driver) Connect(vmName string) (context.CancelFunc, error) {
 	return hyperv.ConnectVirtualMachine(vmName)
 }
 

diff --git a/builder/hyperv/common/step_run.go b/builder/hyperv/common/step_run.go
@@ -3,14 +3,16 @@ package common
 import (
 	"context"
 	"fmt"
+	"log"
 
 	"github.com/hashicorp/packer/helper/multistep"
 	"github.com/hashicorp/packer/packer"
 )
 
 type StepRun struct {
-	Headless bool
-	vmName   string
+	GuiCancelFunc context.CancelFunc
+	Headless      bool
+	vmName        string
 }
 
 func (s *StepRun) Run(_ context.Context, state multistep.StateBag) multistep.StepAction {
@@ -32,8 +34,10 @@ func (s *StepRun) Run(_ context.Context, state multistep.StateBag) multistep.Ste
 
 	if !s.Headless {
 		ui.Say("Connecting to vmconnect...")
-		cancel := driver.Connect(vmName)
-		state.Put("guiCancelFunc", cancel)
+		s.GuiCancelFunc, err = driver.Connect(vmName)
+		if err != nil {
+			log.Printf(fmt.Sprintf("Non-fatal error starting vmconnect: %s. continuing...", err))
+		}
 	}
 	return multistep.ActionContinue
 }
@@ -45,11 +49,10 @@ func (s *StepRun) Cleanup(state multistep.StateBag) {
 
 	driver := state.Get("driver").(Driver)
 	ui := state.Get("ui").(packer.Ui)
-	guiCancelFunc := state.Get("guiCancelFunc").(context.CancelFunc)
 
-	if guiCancelFunc != nil {
+	if !s.Headless && s.GuiCancelFunc != nil {
 		ui.Say("Disconnecting from vmconnect...")
-		guiCancelFunc()
+		s.GuiCancelFunc()
 	}
 
 	if running, _ := driver.IsRunning(s.vmName); running {

diff --git a/common/powershell/hyperv/hyperv.go b/common/powershell/hyperv/hyperv.go
@@ -1247,11 +1247,15 @@ param([string]$vmName, [string]$scanCodes)
 	return err
 }
 
-func ConnectVirtualMachine(vmName string) context.CancelFunc {
+func ConnectVirtualMachine(vmName string) (context.CancelFunc, error) {
 	ctx, cancel := context.WithCancel(context.Background())
 	cmd := exec.CommandContext(ctx, "vmconnect.exe", "localhost", vmName)
-	cmd.Start()
-	return cancel
+	err := cmd.Start()
+	if err != nil {
+		// Failed to start so cancel function not required
+		cancel = nil
+	}
+	return cancel, err
 }
 
 func DisconnectVirtualMachine(cancel context.CancelFunc) {