fix step_shutdown when a null communicator is used (#10178)

builder/qemu/builder.go

@@ -130,7 +130,11 @@ func (b *Builder) Run(ctx context.Context, ui packer.Ui, hook packer.Hook) (pack
 		&common.StepCleanupTempKeys{
 			Comm: &b.config.CommConfig.Comm,
 		},
-		new(stepShutdown),
+		&stepShutdown{
+			ShutdownTimeout: b.config.ShutdownTimeout,
+			ShutdownCommand: b.config.ShutdownCommand,
+			Comm:            &b.config.CommConfig.Comm,
+		},
 		&stepConvertDisk{
 			DiskCompression: b.config.DiskCompression,
 			Format:          b.config.Format,

builder/qemu/step_shutdown.go

@@ -7,6 +7,7 @@ import (
 	"log"
 	"time"
 
+	"github.com/hashicorp/packer/helper/communicator"
 	"github.com/hashicorp/packer/helper/multistep"
 	"github.com/hashicorp/packer/packer"
 )
@@ -22,18 +23,21 @@ import (
 //
 // Produces:
 //   <nothing>
-type stepShutdown struct{}
+type stepShutdown struct {
+	ShutdownCommand string
+	ShutdownTimeout time.Duration
+	Comm            *communicator.Config
+}
 
 func (s *stepShutdown) Run(ctx context.Context, state multistep.StateBag) multistep.StepAction {
-	config := state.Get("config").(*Config)
 	driver := state.Get("driver").(Driver)
 	ui := state.Get("ui").(packer.Ui)
 
-	if state.Get("communicator") == nil {
+	if s.Comm.Type == "none" {
 		cancelCh := make(chan struct{}, 1)
 		go func() {
 			defer close(cancelCh)
-			<-time.After(config.ShutdownTimeout)
+			<-time.After(s.ShutdownTimeout)
 		}()
 		ui.Say("Waiting for shutdown...")
 		if ok := driver.WaitForShutdown(cancelCh); ok {
@@ -47,11 +51,11 @@ func (s *stepShutdown) Run(ctx context.Context, state multistep.StateBag) multis
 		}
 	}
 
-	comm := state.Get("communicator").(packer.Communicator)
-	if config.ShutdownCommand != "" {
+	if s.ShutdownCommand != "" {
+		comm := state.Get("communicator").(packer.Communicator)
 		ui.Say("Gracefully halting virtual machine...")
-		log.Printf("Executing shutdown command: %s", config.ShutdownCommand)
-		cmd := &packer.RemoteCmd{Command: config.ShutdownCommand}
+		log.Printf("Executing shutdown command: %s", s.ShutdownCommand)
+		cmd := &packer.RemoteCmd{Command: s.ShutdownCommand}
 		if err := cmd.RunWithUi(ctx, comm, ui); err != nil {
 			err := fmt.Errorf("Failed to send shutdown command: %s", err)
 			state.Put("error", err)
@@ -63,10 +67,10 @@ func (s *stepShutdown) Run(ctx context.Context, state multistep.StateBag) multis
 		cancelCh := make(chan struct{}, 1)
 		go func() {
 			defer close(cancelCh)
-			<-time.After(config.ShutdownTimeout)
+			<-time.After(s.ShutdownTimeout)
 		}()
 
-		log.Printf("Waiting max %s for shutdown to complete", config.ShutdownTimeout)
+		log.Printf("Waiting max %s for shutdown to complete", s.ShutdownTimeout)
 		if ok := driver.WaitForShutdown(cancelCh); !ok {
 			err := errors.New("Timeout while waiting for machine to shut down.")
 			state.Put("error", err)

builder/qemu/step_shutdown_test.go

@@ -0,0 +1,88 @@
+package qemu
+
+import (
+	"context"
+	"testing"
+	"time"
+
+	"github.com/hashicorp/packer/helper/communicator"
+	"github.com/hashicorp/packer/helper/multistep"
+	"github.com/hashicorp/packer/packer"
+)
+
+func Test_Shutdown_Null_success(t *testing.T) {
+	state := new(multistep.BasicStateBag)
+	state.Put("ui", packer.TestUi(t))
+	driverMock := new(DriverMock)
+	driverMock.WaitForShutdownState = true
+	state.Put("driver", driverMock)
+
+	step := &stepShutdown{
+		ShutdownCommand: "",
+		ShutdownTimeout: 5 * time.Minute,
+		Comm: &communicator.Config{
+			Type: "none",
+		},
+	}
+	action := step.Run(context.TODO(), state)
+	if action != multistep.ActionContinue {
+		t.Fatalf("Should have successfully shut down.")
+	}
+	err := state.Get("error")
+	if err != nil {
+		err = err.(error)
+		t.Fatalf("Shutdown shouldn't have errored; err: %v", err)
+	}
+}
+
+func Test_Shutdown_Null_failure(t *testing.T) {
+	state := new(multistep.BasicStateBag)
+	state.Put("ui", packer.TestUi(t))
+	driverMock := new(DriverMock)
+	driverMock.WaitForShutdownState = false
+	state.Put("driver", driverMock)
+
+	step := &stepShutdown{
+		ShutdownCommand: "",
+		ShutdownTimeout: 5 * time.Minute,
+		Comm: &communicator.Config{
+			Type: "none",
+		},
+	}
+	action := step.Run(context.TODO(), state)
+	if action != multistep.ActionHalt {
+		t.Fatalf("Shouldn't have successfully shut down.")
+	}
+	err := state.Get("error")
+	if err == nil {
+		t.Fatalf("Shutdown should have errored")
+	}
+}
+
+func Test_Shutdown_NoShutdownCommand(t *testing.T) {
+	state := new(multistep.BasicStateBag)
+	state.Put("ui", packer.TestUi(t))
+	driverMock := new(DriverMock)
+	state.Put("driver", driverMock)
+
+	step := &stepShutdown{
+		ShutdownCommand: "",
+		ShutdownTimeout: 5 * time.Minute,
+		Comm: &communicator.Config{
+			Type: "ssh",
+		},
+	}
+	action := step.Run(context.TODO(), state)
+	if action != multistep.ActionContinue {
+		t.Fatalf("Should have successfully shut down.")
+	}
+
+	if !driverMock.StopCalled {
+		t.Fatalf("should have called Stop through the driver.")
+	}
+	err := state.Get("error")
+	if err != nil {
+		err = err.(error)
+		t.Fatalf("Shutdown shouldn't have errored; err: %v", err)
+	}
+}