From d56a5e3563ff8bc408ef147f9b15184474a343ad Mon Sep 17 00:00:00 2001 From: Ryan Johnson Date: Mon, 15 Jul 2024 22:05:28 -0400 Subject: [PATCH] fix: remote hypervisor snapshot creation Moves the snapshot creation step after the upload of the `.vmx` file to the remote hypervisor. When not set in this order, the virtual machine will not be set to run on the created snapshot. Signed-off-by: Ryan Johnson --- builder/vmware/common/step_create_snapshot.go | 33 ++++++++++--------- .../common/step_create_snapshot_test.go | 15 ++++----- builder/vmware/iso/builder.go | 6 ++-- builder/vmware/vmx/builder.go | 6 ++-- 4 files changed, 31 insertions(+), 29 deletions(-) diff --git a/builder/vmware/common/step_create_snapshot.go b/builder/vmware/common/step_create_snapshot.go index 546c6edf..9275fc03 100644 --- a/builder/vmware/common/step_create_snapshot.go +++ b/builder/vmware/common/step_create_snapshot.go @@ -11,28 +11,31 @@ import ( packersdk "github.com/hashicorp/packer-plugin-sdk/packer" ) -// StepCreateSnapshot step creates the intial snapshot for the VM after clean-up. +// StepCreateSnapshot step creates a snapshot for the virtual machine after the +// build has been completed. type StepCreateSnapshot struct { SnapshotName *string } func (s *StepCreateSnapshot) Run(ctx context.Context, state multistep.StateBag) multistep.StepAction { - if *s.SnapshotName != "" { // if snapshot name is set create one, if not don't - driver := state.Get("driver").(Driver) - ui := state.Get("ui").(packersdk.Ui) - - ui.Say("Creating inital snapshot") - vmFullPath := state.Get("vmx_path").(string) - if err := driver.CreateSnapshot(vmFullPath, *s.SnapshotName); err != nil { - err := fmt.Errorf("error creating snapshot: %s", err) - state.Put("error", err) - ui.Error(err.Error()) - return multistep.ActionHalt - } - state.Put("snapshot_created", true) - } else { + // If snapshot name is not set, skip snapshot creation. + if *s.SnapshotName == "" { state.Put("snapshot_skipped", true) + return multistep.ActionContinue } + + driver := state.Get("driver").(Driver) + ui := state.Get("ui").(packersdk.Ui) + + ui.Say("Creating snapshot of virtual machine...") + vmFullPath := state.Get("vmx_path").(string) + if err := driver.CreateSnapshot(vmFullPath, *s.SnapshotName); err != nil { + err := fmt.Errorf("error creating snapshot: %s", err) + state.Put("error", err) + ui.Error(err.Error()) + return multistep.ActionHalt + } + state.Put("snapshot_created", true) return multistep.ActionContinue } diff --git a/builder/vmware/common/step_create_snapshot_test.go b/builder/vmware/common/step_create_snapshot_test.go index 70e21e31..8831f051 100644 --- a/builder/vmware/common/step_create_snapshot_test.go +++ b/builder/vmware/common/step_create_snapshot_test.go @@ -29,24 +29,24 @@ func TestStepCreateSnapshot(t *testing.T) { if action := step.Run(context.Background(), state); action != multistep.ActionContinue { t.Fatalf("bad action: %#v", action) } + if _, ok := state.GetOk("error"); ok { - t.Fatal("should NOT have error") + t.Fatal("should not error") } driver := state.Get("driver").(*DriverMock) if !driver.CreateSnapshotCalled { - t.Fatalf("Should have called create snapshot.") + t.Fatalf("should call create snapshot") } - if _, ok := state.GetOk("snapshot_skiped"); ok { - t.Fatalf("Should NOT skip snapshot creation") + if _, ok := state.GetOk("snapshot_skipped"); ok { + t.Fatalf("should not skip snapshot") } if _, ok := state.GetOk("snapshot_created"); !ok { t.Fatalf("Should create snapshot") } - // Cleanup step.Cleanup(state) } @@ -61,14 +61,13 @@ func TestStepCreateSnapshot_skip(t *testing.T) { t.Fatalf("bad action: %#v", action) } if _, ok := state.GetOk("error"); ok { - t.Fatal("should NOT have error") + t.Fatal("should not error") } driver := state.Get("driver").(*DriverMock) if driver.CreateSnapshotCalled { - t.Fatalf("Should NOT have called create snapshot.") + t.Fatalf("should not call create snapshot") } - // Cleanup step.Cleanup(state) } diff --git a/builder/vmware/iso/builder.go b/builder/vmware/iso/builder.go index 8c8268af..fd859818 100644 --- a/builder/vmware/iso/builder.go +++ b/builder/vmware/iso/builder.go @@ -188,12 +188,12 @@ func (b *Builder) Run(ctx context.Context, ui packersdk.Ui, hook packersdk.Hook) RemoveEthernetInterfaces: b.config.VMXConfig.VMXRemoveEthernet, VNCEnabled: !b.config.DisableVNC, }, - &vmwcommon.StepCreateSnapshot{ - SnapshotName: &b.config.SnapshotName, - }, &vmwcommon.StepUploadVMX{ RemoteType: b.config.RemoteType, }, + &vmwcommon.StepCreateSnapshot{ + SnapshotName: &b.config.SnapshotName, + }, &vmwcommon.StepExport{ Format: b.config.Format, SkipExport: b.config.SkipExport, diff --git a/builder/vmware/vmx/builder.go b/builder/vmware/vmx/builder.go index f1988ff5..05a1b7f7 100644 --- a/builder/vmware/vmx/builder.go +++ b/builder/vmware/vmx/builder.go @@ -182,12 +182,12 @@ func (b *Builder) Run(ctx context.Context, ui packersdk.Ui, hook packersdk.Hook) RemoveEthernetInterfaces: b.config.VMXConfig.VMXRemoveEthernet, VNCEnabled: !b.config.DisableVNC, }, - &vmwcommon.StepCreateSnapshot{ - SnapshotName: &b.config.SnapshotName, - }, &vmwcommon.StepUploadVMX{ RemoteType: b.config.RemoteType, }, + &vmwcommon.StepCreateSnapshot{ + SnapshotName: &b.config.SnapshotName, + }, &vmwcommon.StepExport{ Format: b.config.Format, SkipExport: b.config.SkipExport,