From 3577c4a2831e44f6335390eb65b27301bdc09b6f Mon Sep 17 00:00:00 2001
From: Megan Marsh <megan@hashicorp.com>
Date: Tue, 15 Sep 2020 16:36:51 -0700
Subject: [PATCH] refactor multistep array construction in qemu builder to push
 majority of conditionals into their steps rather than the multistep
 constructor code.

---
 builder/qemu/builder.go                 | 62 +++++++------------------
 builder/qemu/step_port_forward.go       | 12 +++++
 builder/qemu/step_wait_guest_address.go | 14 +++++-
 3 files changed, 43 insertions(+), 45 deletions(-)

diff --git a/builder/qemu/builder.go b/builder/qemu/builder.go
index 6e1354ae3..2b68e1035 100644
--- a/builder/qemu/builder.go
+++ b/builder/qemu/builder.go
@@ -53,14 +53,12 @@ func (b *Builder) Run(ctx context.Context, ui packer.Ui, hook packer.Hook) (pack
 			ResultKey:   "iso_path",
 			TargetPath:  b.config.TargetPath,
 			Url:         b.config.ISOUrls,
-		},
-		)
+		})
 	} else {
 		steps = append(steps, &stepSetISO{
 			ResultKey: "iso_path",
 			Url:       b.config.ISOUrls,
-		},
-		)
+		})
 	}
 
 	steps = append(steps, new(stepPrepareOutputDir),
@@ -83,15 +81,10 @@ func (b *Builder) Run(ctx context.Context, ui packer.Ui, hook packer.Hook) (pack
 			HTTPPortMax: b.config.HTTPPortMax,
 			HTTPAddress: b.config.HTTPAddress,
 		},
-	)
-
-	if b.config.CommConfig.Comm.Type != "none" && b.config.NetBridge == "" {
-		steps = append(steps,
-			new(stepPortForward),
-		)
-	}
-
-	steps = append(steps,
+		&stepPortForward{
+			CommunicatorType: b.config.CommConfig.Comm.Type,
+			NetBridge:        b.config.NetBridge,
+		},
 		new(stepConfigureVNC),
 		&stepRun{
 			DiskImage: b.config.DiskImage,
@@ -100,42 +93,23 @@ func (b *Builder) Run(ctx context.Context, ui packer.Ui, hook packer.Hook) (pack
 			QMPSocketPath: b.config.QMPSocketPath,
 		},
 		&stepTypeBootCommand{},
-	)
-
-	if b.config.CommConfig.Comm.Type != "none" && b.config.NetBridge != "" {
-		steps = append(steps,
-			&stepWaitGuestAddress{
-				timeout: b.config.CommConfig.Comm.SSHTimeout,
-			},
-		)
-	}
-
-	if b.config.CommConfig.Comm.Type != "none" {
-		steps = append(steps,
-			&communicator.StepConnect{
-				Config:    &b.config.CommConfig.Comm,
-				Host:      commHost(b.config.CommConfig.Comm.Host()),
-				SSHConfig: b.config.CommConfig.Comm.SSHConfigFunc(),
-				SSHPort:   commPort,
-				WinRMPort: commPort,
-			},
-		)
-	}
-
-	steps = append(steps,
+		&stepWaitGuestAddress{
+			CommunicatorType: b.config.CommConfig.Comm.Type,
+			NetBridge:        b.config.NetBridge,
+			timeout:          b.config.CommConfig.Comm.SSHTimeout,
+		},
+		&communicator.StepConnect{
+			Config:    &b.config.CommConfig.Comm,
+			Host:      commHost(b.config.CommConfig.Comm.Host()),
+			SSHConfig: b.config.CommConfig.Comm.SSHConfigFunc(),
+			SSHPort:   commPort,
+			WinRMPort: commPort,
+		},
 		new(common.StepProvision),
-	)
-
-	steps = append(steps,
 		&common.StepCleanupTempKeys{
 			Comm: &b.config.CommConfig.Comm,
 		},
-	)
-	steps = append(steps,
 		new(stepShutdown),
-	)
-
-	steps = append(steps,
 		new(stepConvertDisk),
 	)
 
diff --git a/builder/qemu/step_port_forward.go b/builder/qemu/step_port_forward.go
index e4f24b691..ab5871fa2 100644
--- a/builder/qemu/step_port_forward.go
+++ b/builder/qemu/step_port_forward.go
@@ -13,6 +13,9 @@ import (
 // This step adds a NAT port forwarding definition so that SSH or WinRM is available
 // on the guest machine.
 type stepPortForward struct {
+	CommunicatorType string
+	NetBridge        string
+
 	l *net.Listener
 }
 
@@ -20,6 +23,15 @@ func (s *stepPortForward) Run(ctx context.Context, state multistep.StateBag) mul
 	config := state.Get("config").(*Config)
 	ui := state.Get("ui").(packer.Ui)
 
+	if s.CommunicatorType == "none" {
+		ui.Message("No communicator is set; skipping port forwarding setup.")
+		return multistep.ActionContinue
+	}
+	if s.NetBridge != "" {
+		ui.Message("net_bridge is set; skipping port forwarding setup.")
+		return multistep.ActionContinue
+	}
+
 	commHostPort := config.CommConfig.Comm.Port()
 
 	if config.CommConfig.SkipNatMapping {
diff --git a/builder/qemu/step_wait_guest_address.go b/builder/qemu/step_wait_guest_address.go
index b1541d9ec..0fa946a4c 100644
--- a/builder/qemu/step_wait_guest_address.go
+++ b/builder/qemu/step_wait_guest_address.go
@@ -19,12 +19,24 @@ import (
 // This step waits for the guest address to become available in the network
 // bridge, then it sets the guestAddress state property.
 type stepWaitGuestAddress struct {
-	timeout time.Duration
+	CommunicatorType string
+	NetBridge        string
+	timeout          time.Duration
 }
 
 func (s *stepWaitGuestAddress) Run(ctx context.Context, state multistep.StateBag) multistep.StepAction {
 	config := state.Get("config").(*Config)
 	ui := state.Get("ui").(packer.Ui)
+
+	if s.CommunicatorType == "none" {
+		ui.Message("Not using a communicator -- skipping StepWaitGuestAddress")
+		return multistep.ActionContinue
+	}
+	if s.NetBridge == "" {
+		ui.Message("Not using a NetBridge -- skipping StepWaitGuestAddress")
+		return multistep.ActionContinue
+	}
+
 	qmpMonitor := state.Get("qmp_monitor").(*qmp.SocketMonitor)
 	ctx, cancel := context.WithTimeout(ctx, s.timeout)
 	defer cancel()