From 6c9d4efd9fdaffa73ef18b82ad06a22055f10e55 Mon Sep 17 00:00:00 2001 From: DanHam Date: Fri, 30 Mar 2018 21:59:38 +0100 Subject: [PATCH 1/4] Fix error on compaction step of vmx build. Support compacting multi-disk vm --- builder/vmware/vmx/step_clone_vmx.go | 124 ++++++++++++++++++++++----- 1 file changed, 103 insertions(+), 21 deletions(-) diff --git a/builder/vmware/vmx/step_clone_vmx.go b/builder/vmware/vmx/step_clone_vmx.go index f0bdd586d..edd81ea99 100644 --- a/builder/vmware/vmx/step_clone_vmx.go +++ b/builder/vmware/vmx/step_clone_vmx.go @@ -18,14 +18,70 @@ type StepCloneVMX struct { VMName string } +type vmxAdapter struct { + // The string portion of the address used in the vmx file + strAddr string + // Max address for adapter, controller, or controller channel + aAddrMax int + // Max address for device or channel supported by adapter + dAddrMax int +} + +const ( + // VMware Configuration Maximums - Virtual Hardware Versions 13/14 + // + // Specifying the max numbers for the adapter/controller:bus/channel + // *address* as opposed to specifying the maximums as per the VMware + // documentation allows consistent (inclusive) treatment when looping + // over each adapter/controller type + // + // SCSI - Address range: scsi0:0 to scsi3:15 + scsiAddrName = "scsi" // String part of address used in the vmx file + maxSCSIAdapterAddr = 3 // Max 4 adapters + maxSCSIDeviceAddr = 15 // Max 15 devices per adapter; ID 7 is the HBA + // SATA - Address range: sata0:0 to scsi3:29 + sataAddrName = "sata" // String part of address used in the vmx file + maxSATAAdapterAddr = 3 // Max 4 controllers + maxSATADeviceAddr = 29 // Max 30 devices per controller + // NVMe - Address range: nvme0:0 to nvme3:14 + nvmeAddrName = "nvme" // String part of address used in the vmx file + maxNVMeAdapterAddr = 3 // Max 4 adapters + maxNVMeDeviceAddr = 14 // Max 15 devices per adapter + // IDE - Address range: ide0:0 to ide1:1 + ideAddrName = "ide" // String part of address used in the vmx file + maxIDEAdapterAddr = 1 // One controller with primary/secondary channels + maxIDEDeviceAddr = 1 // Each channel supports master and slave +) + +var ( + scsiAdapter = vmxAdapter{ + strAddr: scsiAddrName, + aAddrMax: maxSCSIAdapterAddr, + dAddrMax: maxSCSIDeviceAddr, + } + sataAdapter = vmxAdapter{ + strAddr: sataAddrName, + aAddrMax: maxSATAAdapterAddr, + dAddrMax: maxSATADeviceAddr, + } + nvmeAdapter = vmxAdapter{ + strAddr: nvmeAddrName, + aAddrMax: maxNVMeAdapterAddr, + dAddrMax: maxNVMeDeviceAddr, + } + ideAdapter = vmxAdapter{ + strAddr: ideAddrName, + aAddrMax: maxIDEAdapterAddr, + dAddrMax: maxIDEDeviceAddr, + } +) + func (s *StepCloneVMX) Run(_ context.Context, state multistep.StateBag) multistep.StepAction { driver := state.Get("driver").(vmwcommon.Driver) ui := state.Get("ui").(packer.Ui) - // initially we need to stash the path to the original .vmx file + // Set the path we want for the new .vmx file and clone vmxPath := filepath.Join(s.OutputDir, s.VMName+".vmx") - - // so first, let's clone the source path to the vmxPath ui.Say("Cloning source VM...") log.Printf("Cloning from: %s", s.Path) log.Printf("Cloning to: %s", vmxPath) @@ -33,34 +89,44 @@ func (s *StepCloneVMX) Run(_ context.Context, state multistep.StateBag) multiste state.Put("error", err) return multistep.ActionHalt } - ui.Say(fmt.Sprintf("Successfully cloned source VM to: %s", vmxPath)) - // now we read the .vmx so we can determine what else to stash + // Read in the machine configuration from the cloned VMX file + // + // * The main driver needs the path to the vmx (set above) and the + // network type so that it can work out things like IP's and MAC + // addresses + // * The disk compaction step needs the paths to all attached disks vmxData, err := vmwcommon.ReadVMX(vmxPath) if err != nil { state.Put("error", err) return multistep.ActionHalt } - // figure out the disk filename by walking through all device types - var diskName string - if _, ok := vmxData["scsi0:0.filename"]; ok { - diskName = vmxData["scsi0:0.filename"] + // Search across all adapter types to get the filenames of attached disks + allDiskAdapters := []vmxAdapter{ + scsiAdapter, + sataAdapter, + nvmeAdapter, + ideAdapter, } - if _, ok := vmxData["sata0:0.filename"]; ok { - diskName = vmxData["sata0:0.filename"] + var diskFilenames []string + for _, adapter := range allDiskAdapters { + diskFilenames = append(diskFilenames, getAttachedDisks(adapter, vmxData)...) } - if _, ok := vmxData["ide0:0.filename"]; ok { - diskName = vmxData["ide0:0.filename"] + + // Write out the relative, host filesystem paths to the disks + var diskFullPaths []string + for _, diskFilename := range diskFilenames { + log.Printf("Found attached disk with filename: %s", diskFilename) + diskFullPaths = append(diskFullPaths, filepath.Join(s.OutputDir, diskFilename)) } - if diskName == "" { - err := fmt.Errorf("Root disk filename could not be found!") - state.Put("error", err) + + if len(diskFullPaths) == 0 { + state.Put("error", fmt.Errorf("Could not enumerate disk info from the vmx file")) return multistep.ActionHalt } - log.Printf("Found root disk filename: %s", diskName) - // determine the network type by reading out of the .vmx + // Determine the network type by reading out of the .vmx var networkType string if _, ok := vmxData["ethernet0.connectiontype"]; ok { networkType = vmxData["ethernet0.connectiontype"] @@ -70,11 +136,13 @@ func (s *StepCloneVMX) Run(_ context.Context, state multistep.StateBag) multiste networkType = "nat" log.Printf("Defaulting to network type: %s", networkType) } - ui.Say(fmt.Sprintf("Using network type: %s", networkType)) - // we were able to find everything, so stash it in our state. + // Stash all required information in our state bag state.Put("vmx_path", vmxPath) - state.Put("full_disk_path", filepath.Join(s.OutputDir, diskName)) + // What disks get assigned to what key doesn't actually matter here + // since it's unimportant to the way the disk compaction step works + state.Put("full_disk_path", diskFullPaths[0]) + state.Put("additional_disk_paths", diskFullPaths[1:]) state.Put("vmnetwork", networkType) return multistep.ActionContinue @@ -82,3 +150,17 @@ func (s *StepCloneVMX) Run(_ context.Context, state multistep.StateBag) multiste func (s *StepCloneVMX) Cleanup(state multistep.StateBag) { } + +func getAttachedDisks(a vmxAdapter, data map[string]string) (attachedDisks []string) { + // Loop over possible adapter, controller or controller channel + for x := 0; x <= a.aAddrMax; x++ { + // Loop over possible addresses for attached devices + for y := 0; y <= a.dAddrMax; y++ { + address := fmt.Sprintf("%s%d:%d.filename", a.strAddr, x, y) + if device, _ := data[address]; filepath.Ext(device) == ".vmdk" { + attachedDisks = append(attachedDisks, device) + } + } + } + return +} From 1aee759f0673d424d321bdbb50b49abb00a0609c Mon Sep 17 00:00:00 2001 From: DanHam Date: Fri, 30 Mar 2018 22:00:21 +0100 Subject: [PATCH 2/4] Fix tests and reconfigure for support of multi-disk vm --- builder/vmware/vmx/step_clone_vmx_test.go | 37 ++++++++++++++++++++--- 1 file changed, 32 insertions(+), 5 deletions(-) diff --git a/builder/vmware/vmx/step_clone_vmx_test.go b/builder/vmware/vmx/step_clone_vmx_test.go index e086f0ce5..990a2d20e 100644 --- a/builder/vmware/vmx/step_clone_vmx_test.go +++ b/builder/vmware/vmx/step_clone_vmx_test.go @@ -2,6 +2,7 @@ package vmx import ( "context" + "fmt" "io/ioutil" "os" "path/filepath" @@ -9,6 +10,14 @@ import ( vmwcommon "github.com/hashicorp/packer/builder/vmware/common" "github.com/hashicorp/packer/helper/multistep" + "github.com/stretchr/testify/assert" +) + +const ( + scsiFilename = "scsiDisk.vmdk" + sataFilename = "sataDisk.vmdk" + nvmeFilename = "nvmeDisk.vmdk" + ideFilename = "ideDisk.vmdk" ) func TestStepCloneVMX_impl(t *testing.T) { @@ -23,6 +32,21 @@ func TestStepCloneVMX(t *testing.T) { } defer os.RemoveAll(td) + // Set up mock vmx file contents + var testCloneVMX = fmt.Sprintf("scsi0:0.filename = \"%s\"\n"+ + "sata0:0.filename = \"%s\"\n"+ + "nvme0:0.filename = \"%s\"\n"+ + "ide1:0.filename = \"%s\"\n"+ + "ide0:0.filename = \"auto detect\"\n", scsiFilename, + sataFilename, nvmeFilename, ideFilename) + + // Set up expected mock disk file paths + diskFilenames := []string{scsiFilename, sataFilename, ideFilename, nvmeFilename} + var diskPaths []string + for _, diskFilename := range diskFilenames { + diskPaths = append(diskPaths, filepath.Join(td, diskFilename)) + } + // Create the source sourcePath := filepath.Join(td, "source.vmx") if err := ioutil.WriteFile(sourcePath, []byte(testCloneVMX), 0644); err != nil { @@ -65,11 +89,14 @@ func TestStepCloneVMX(t *testing.T) { if diskPath, ok := state.GetOk("full_disk_path"); !ok { t.Fatal("should set full_disk_path") - } else if diskPath != filepath.Join(td, "foo") { + } else if diskPath != diskPaths[0] { t.Fatalf("bad: %#v", diskPath) } -} -const testCloneVMX = ` -scsi0:0.fileName = "foo" -` + if stateDiskPaths, ok := state.GetOk("additional_disk_paths"); !ok { + t.Fatal("should set additional_disk_paths") + } else { + assert.ElementsMatchf(t, stateDiskPaths.([]string), diskPaths[1:], + "%s\nshould contain the same elements as:\n%s", stateDiskPaths.([]string), diskPaths[1:]) + } +} From 94d5a7f2e2c5cd40ece1faf67a09fd816361820d Mon Sep 17 00:00:00 2001 From: DanHam Date: Thu, 29 Mar 2018 01:19:15 +0100 Subject: [PATCH 3/4] Fix copy/paste error referencing Virtualbox --- builder/vmware/vmx/builder.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/builder/vmware/vmx/builder.go b/builder/vmware/vmx/builder.go index bfe6b06c9..130c533d7 100644 --- a/builder/vmware/vmx/builder.go +++ b/builder/vmware/vmx/builder.go @@ -32,7 +32,7 @@ func (b *Builder) Prepare(raws ...interface{}) ([]string, error) { } // Run executes a Packer build and returns a packer.Artifact representing -// a VirtualBox appliance. +// a VMware image. func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packer.Artifact, error) { driver, err := vmwcommon.NewDriver(&b.config.DriverConfig, &b.config.SSHConfig) if err != nil { From bd9e585cb993222e14281061d140c91c2df3e24b Mon Sep 17 00:00:00 2001 From: DanHam Date: Fri, 30 Mar 2018 22:44:14 +0100 Subject: [PATCH 4/4] Add test for enumeration of vmx network type --- builder/vmware/vmx/step_clone_vmx_test.go | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/builder/vmware/vmx/step_clone_vmx_test.go b/builder/vmware/vmx/step_clone_vmx_test.go index 990a2d20e..e7ef9ff95 100644 --- a/builder/vmware/vmx/step_clone_vmx_test.go +++ b/builder/vmware/vmx/step_clone_vmx_test.go @@ -37,7 +37,8 @@ func TestStepCloneVMX(t *testing.T) { "sata0:0.filename = \"%s\"\n"+ "nvme0:0.filename = \"%s\"\n"+ "ide1:0.filename = \"%s\"\n"+ - "ide0:0.filename = \"auto detect\"\n", scsiFilename, + "ide0:0.filename = \"auto detect\"\n"+ + "ethernet0.connectiontype = \"nat\"\n", scsiFilename, sataFilename, nvmeFilename, ideFilename) // Set up expected mock disk file paths @@ -84,13 +85,13 @@ func TestStepCloneVMX(t *testing.T) { if vmxPath, ok := state.GetOk("vmx_path"); !ok { t.Fatal("should set vmx_path") } else if vmxPath != destPath { - t.Fatalf("bad: %#v", vmxPath) + t.Fatalf("bad path to vmx: %#v", vmxPath) } if diskPath, ok := state.GetOk("full_disk_path"); !ok { t.Fatal("should set full_disk_path") } else if diskPath != diskPaths[0] { - t.Fatalf("bad: %#v", diskPath) + t.Fatalf("bad disk path: %#v", diskPath) } if stateDiskPaths, ok := state.GetOk("additional_disk_paths"); !ok { @@ -99,4 +100,11 @@ func TestStepCloneVMX(t *testing.T) { assert.ElementsMatchf(t, stateDiskPaths.([]string), diskPaths[1:], "%s\nshould contain the same elements as:\n%s", stateDiskPaths.([]string), diskPaths[1:]) } + + // Test we got the network type + if networkType, ok := state.GetOk("vmnetwork"); !ok { + t.Fatal("should set vmnetwork") + } else if networkType != "nat" { + t.Fatalf("bad network type: %#v", networkType) + } }