builder/vmware: StepShutdown

builder/vmware/common/driver_mock.go

@@ -1,10 +1,14 @@
 package common
 
 import (
+	"sync"
+
 	"github.com/mitchellh/multistep"
 )
 
 type DriverMock struct {
+	sync.Mutex
+
 	CompactDiskCalled bool
 	CompactDiskPath   string
 	CompactDiskErr    error
@@ -65,6 +69,9 @@ func (d *DriverMock) CreateDisk(output string, size string, typeId string) error
 }
 
 func (d *DriverMock) IsRunning(path string) (bool, error) {
+	d.Lock()
+	defer d.Unlock()
+
 	d.IsRunningCalled = true
 	d.IsRunningPath = path
 	return d.IsRunningResult, d.IsRunningErr

builder/vmware/common/step_shutdown.go

@@ -1,11 +1,10 @@
-package iso
+package common
 
 import (
 	"bytes"
 	"errors"
 	"fmt"
 	"github.com/mitchellh/multistep"
-	vmwcommon "github.com/mitchellh/packer/builder/vmware/common"
 	"github.com/mitchellh/packer/packer"
 	"log"
 	"regexp"
@@ -19,30 +18,32 @@ import (
 //
 // Uses:
 //   communicator packer.Communicator
-//   config *config
+//   dir OutputDir
 //   driver Driver
 //   ui     packer.Ui
 //   vmx_path string
 //
 // Produces:
 //   <nothing>
-type stepShutdown struct{}
+type StepShutdown struct {
+	Command string
+	Timeout time.Duration
+}
 
-func (s *stepShutdown) Run(state multistep.StateBag) multistep.StepAction {
+func (s *StepShutdown) Run(state multistep.StateBag) multistep.StepAction {
 	comm := state.Get("communicator").(packer.Communicator)
-	config := state.Get("config").(*config)
-	dir := state.Get("dir").(vmwcommon.OutputDir)
-	driver := state.Get("driver").(vmwcommon.Driver)
+	dir := state.Get("dir").(OutputDir)
+	driver := state.Get("driver").(Driver)
 	ui := state.Get("ui").(packer.Ui)
 	vmxPath := state.Get("vmx_path").(string)
 
-	if config.ShutdownCommand != "" {
+	if s.Command != "" {
 		ui.Say("Gracefully halting virtual machine...")
-		log.Printf("Executing shutdown command: %s", config.ShutdownCommand)
+		log.Printf("Executing shutdown command: %s", s.Command)
 
 		var stdout, stderr bytes.Buffer
 		cmd := &packer.RemoteCmd{
-			Command: config.ShutdownCommand,
+			Command: s.Command,
 			Stdout:  &stdout,
 			Stderr:  &stderr,
 		}
@@ -68,8 +69,8 @@ func (s *stepShutdown) Run(state multistep.StateBag) multistep.StepAction {
 		log.Printf("Shutdown stderr: %s", stderr.String())
 
 		// Wait for the machine to actually shut down
-		log.Printf("Waiting max %s for shutdown to complete", config.shutdownTimeout)
-		shutdownTimer := time.After(config.shutdownTimeout)
+		log.Printf("Waiting max %s for shutdown to complete", s.Timeout)
+		shutdownTimer := time.After(s.Timeout)
 		for {
 			running, _ := driver.IsRunning(vmxPath)
 			if !running {
@@ -83,7 +84,7 @@ func (s *stepShutdown) Run(state multistep.StateBag) multistep.StepAction {
 				ui.Error(err.Error())
 				return multistep.ActionHalt
 			default:
-				time.Sleep(1 * time.Second)
+				time.Sleep(150 * time.Millisecond)
 			}
 		}
 	} else {
@@ -101,7 +102,9 @@ func (s *stepShutdown) Run(state multistep.StateBag) multistep.StepAction {
 LockWaitLoop:
 	for {
 		files, err := dir.ListFiles()
-		if err == nil {
+		if err != nil {
+			log.Printf("Error listing files in outputdir: %s", err)
+		} else {
 			var locks []string
 			for _, file := range files {
 				if lockRegex.MatchString(file) {
@@ -126,7 +129,7 @@ LockWaitLoop:
 		case <-timer:
 			log.Println("Reached timeout on waiting for clean VMware. Assuming clean.")
 			break LockWaitLoop
-		case <-time.After(1 * time.Second):
+		case <-time.After(150 * time.Millisecond):
 		}
 	}
 
@@ -141,4 +144,4 @@ LockWaitLoop:
 	return multistep.ActionContinue
 }
 
-func (s *stepShutdown) Cleanup(state multistep.StateBag) {}
+func (s *StepShutdown) Cleanup(state multistep.StateBag) {}

builder/vmware/common/step_shutdown_test.go

@@ -0,0 +1,174 @@
+package common
+
+import (
+	"io/ioutil"
+	"os"
+	"path/filepath"
+	"testing"
+	"time"
+
+	"github.com/mitchellh/multistep"
+	"github.com/mitchellh/packer/packer"
+)
+
+func testStepShutdownState(t *testing.T) multistep.StateBag {
+	dir := testOutputDir(t)
+	if err := dir.MkdirAll(); err != nil {
+		t.Fatalf("err: %s", err)
+	}
+
+	state := testState(t)
+	state.Put("communicator", new(packer.MockCommunicator))
+	state.Put("dir", dir)
+	state.Put("vmx_path", "foo")
+	return state
+}
+
+func TestStepShutdown_impl(t *testing.T) {
+	var _ multistep.Step = new(StepShutdown)
+}
+
+func TestStepShutdown_command(t *testing.T) {
+	state := testStepShutdownState(t)
+	step := new(StepShutdown)
+	step.Command = "foo"
+	step.Timeout = 10 * time.Second
+
+	comm := state.Get("communicator").(*packer.MockCommunicator)
+	driver := state.Get("driver").(*DriverMock)
+	driver.IsRunningResult = true
+
+	// Set not running after some time
+	go func() {
+		time.Sleep(100 * time.Millisecond)
+		driver.Lock()
+		defer driver.Unlock()
+		driver.IsRunningResult = false
+	}()
+
+	resultCh := make(chan multistep.StepAction, 1)
+	go func() {
+		resultCh <- step.Run(state)
+	}()
+
+	select {
+	case <-resultCh:
+		t.Fatal("should not have returned so quickly")
+	case <-time.After(50 * time.Millisecond):
+	}
+
+	var action multistep.StepAction
+	select {
+	case action = <-resultCh:
+	case <-time.After(300 * time.Millisecond):
+		t.Fatal("should've returned by now")
+	}
+
+	// Test the run
+	if action != multistep.ActionContinue {
+		t.Fatalf("bad action: %#v", action)
+	}
+	if _, ok := state.GetOk("error"); ok {
+		t.Fatal("should NOT have error")
+	}
+
+	// Test the driver
+	if driver.StopCalled {
+		t.Fatal("stop should not be called")
+	}
+
+	if !comm.StartCalled {
+		t.Fatal("start should be called")
+	}
+	if comm.StartCmd.Command != "foo" {
+		t.Fatalf("bad: %#v", comm.StartCmd.Command)
+	}
+}
+
+func TestStepShutdown_noCommand(t *testing.T) {
+	state := testStepShutdownState(t)
+	step := new(StepShutdown)
+
+	comm := state.Get("communicator").(*packer.MockCommunicator)
+	driver := state.Get("driver").(*DriverMock)
+
+	// Test the run
+	if action := step.Run(state); action != multistep.ActionContinue {
+		t.Fatalf("bad action: %#v", action)
+	}
+	if _, ok := state.GetOk("error"); ok {
+		t.Fatal("should NOT have error")
+	}
+
+	// Test the driver
+	if !driver.StopCalled {
+		t.Fatal("stop should be called")
+	}
+	if driver.StopPath != "foo" {
+		t.Fatal("should call with right path")
+	}
+
+	if comm.StartCalled {
+		t.Fatal("start should not be called")
+	}
+}
+
+func TestStepShutdown_locks(t *testing.T) {
+	state := testStepShutdownState(t)
+	step := new(StepShutdown)
+
+	dir := state.Get("dir").(*LocalOutputDir)
+	comm := state.Get("communicator").(*packer.MockCommunicator)
+	driver := state.Get("driver").(*DriverMock)
+
+	// Create some lock files
+	lockPath := filepath.Join(dir.dir, "nope.lck")
+	err := ioutil.WriteFile(lockPath, []byte("foo"), 0644)
+	if err != nil {
+		t.Fatalf("err: %s")
+	}
+
+	// Remove the lock file after a certain time
+	go func() {
+		time.Sleep(100 * time.Millisecond)
+		os.Remove(lockPath)
+	}()
+
+	resultCh := make(chan multistep.StepAction, 1)
+	go func() {
+		resultCh <- step.Run(state)
+	}()
+
+	select {
+	case <-resultCh:
+		t.Fatal("should not have returned so quickly")
+	case <-time.After(50 * time.Millisecond):
+	}
+
+	var action multistep.StepAction
+	select {
+	case action = <-resultCh:
+	case <-time.After(300 * time.Millisecond):
+		t.Fatal("should've returned by now")
+	}
+
+	// Test the run
+	if action != multistep.ActionContinue {
+		t.Fatalf("bad action: %#v", action)
+	}
+	if _, ok := state.GetOk("error"); ok {
+		t.Fatal("should NOT have error")
+	}
+
+	// Test the driver
+	if !driver.StopCalled {
+		t.Fatal("stop should be called")
+	}
+	if driver.StopPath != "foo" {
+		t.Fatal("should call with right path")
+	}
+
+	if comm.StartCalled {
+		t.Fatal("start should not be called")
+	}
+}

builder/vmware/iso/builder.go

@@ -410,7 +410,10 @@ func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packe
 		},
 		&stepUploadTools{},
 		&common.StepProvision{},
-		&stepShutdown{},
+		&vmwcommon.StepShutdown{
+			Command: b.config.ShutdownCommand,
+			Timeout: b.config.shutdownTimeout,
+		},
 		&vmwcommon.StepCleanFiles{},
 		&vmwcommon.StepCleanVMX{},
 		&vmwcommon.StepCompactDisk{