Merge pull request #6186 from DanHam/simplify-vmware-disks

[WIP] Simplify handling of VMware disk operations by collating disk requirements and unifying treatment
This commit is contained in:
M. Marsh 2018-04-27 10:54:39 -07:00 committed by GitHub
commit 6fdab18e6e
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 100 additions and 65 deletions

View File

@ -4,6 +4,8 @@ import (
"context"
"fmt"
"log"
"math"
"os"
"github.com/hashicorp/packer/helper/multistep"
"github.com/hashicorp/packer/packer"
@ -14,7 +16,7 @@ import (
//
// Uses:
// driver Driver
// full_disk_path string
// disk_full_paths ([]string) - The full paths to all created disks
// ui packer.Ui
//
// Produces:
@ -26,27 +28,47 @@ type StepCompactDisk struct {
func (s StepCompactDisk) Run(_ context.Context, state multistep.StateBag) multistep.StepAction {
driver := state.Get("driver").(Driver)
ui := state.Get("ui").(packer.Ui)
full_disk_path := state.Get("full_disk_path").(string)
diskPaths := state.Get("disk_full_paths").([]string)
if s.Skip {
log.Println("Skipping disk compaction step...")
return multistep.ActionContinue
}
ui.Say("Compacting the disk image")
if err := driver.CompactDisk(full_disk_path); err != nil {
state.Put("error", fmt.Errorf("Error compacting disk: %s", err))
return multistep.ActionHalt
}
if state.Get("additional_disk_paths") != nil {
if moreDisks := state.Get("additional_disk_paths").([]string); len(moreDisks) > 0 {
for i, path := range moreDisks {
ui.Say(fmt.Sprintf("Compacting additional disk image %d", i+1))
if err := driver.CompactDisk(path); err != nil {
state.Put("error", fmt.Errorf("Error compacting additional disk %d: %s", i+1, err))
return multistep.ActionHalt
}
ui.Say("Compacting all attached virtual disks...")
for i, diskPath := range diskPaths {
ui.Message(fmt.Sprintf("Compacting virtual disk %d", i+1))
// Get the file size of the virtual disk prior to compaction
fi, err := os.Stat(diskPath)
if err != nil {
state.Put("error", fmt.Errorf("Error getting virtual disk file info pre compaction: %s", err))
return multistep.ActionHalt
}
diskFileSizeStart := fi.Size()
// Defragment and compact the disk
if err := driver.CompactDisk(diskPath); err != nil {
state.Put("error", fmt.Errorf("Error compacting disk: %s", err))
return multistep.ActionHalt
}
// Get the file size of the virtual disk post compaction
fi, err = os.Stat(diskPath)
if err != nil {
state.Put("error", fmt.Errorf("Error getting virtual disk file info post compaction: %s", err))
return multistep.ActionHalt
}
diskFileSizeEnd := fi.Size()
// Report compaction results
log.Printf("Before compaction the disk file size was: %d", diskFileSizeStart)
log.Printf("After compaction the disk file size was: %d", diskFileSizeEnd)
if diskFileSizeStart > 0 {
percentChange := ((float64(diskFileSizeEnd) / float64(diskFileSizeStart)) * 100.0) - 100.0
switch {
case percentChange < 0:
ui.Message(fmt.Sprintf("Compacting reduced the disk file size by %.2f%%", math.Abs(percentChange)))
case percentChange == 0:
ui.Message(fmt.Sprintf("The compacting operation left the disk file size unchanged"))
case percentChange > 0:
ui.Message(fmt.Sprintf("WARNING: Compacting increased the disk file size by %.2f%%", percentChange))
}
}
}

View File

@ -2,6 +2,8 @@ package common
import (
"context"
"io/ioutil"
"os"
"testing"
"github.com/hashicorp/packer/helper/multistep"
@ -15,7 +17,25 @@ func TestStepCompactDisk(t *testing.T) {
state := testState(t)
step := new(StepCompactDisk)
state.Put("full_disk_path", "foo")
// Create a fake vmdk file for disk file size operations
diskFile, err := ioutil.TempFile("", "disk.vmdk")
if err != nil {
t.Fatalf("Error creating fake vmdk file: %s", err)
}
diskPath := diskFile.Name()
defer os.Remove(diskPath)
content := []byte("I am the fake vmdk's contents")
if _, err := diskFile.Write(content); err != nil {
t.Fatalf("Error writing to fake vmdk file: %s", err)
}
if err := diskFile.Close(); err != nil {
t.Fatalf("Error closing fake vmdk file: %s", err)
}
// Set up required state
state.Put("disk_full_paths", []string{diskPath})
driver := state.Get("driver").(*DriverMock)
@ -31,7 +51,7 @@ func TestStepCompactDisk(t *testing.T) {
if !driver.CompactDiskCalled {
t.Fatal("should've called")
}
if driver.CompactDiskPath != "foo" {
if driver.CompactDiskPath != diskPath {
t.Fatal("should call with right path")
}
}
@ -41,7 +61,8 @@ func TestStepCompactDisk_skip(t *testing.T) {
step := new(StepCompactDisk)
step.Skip = true
state.Put("full_disk_path", "foo")
diskPaths := []string{"foo"}
state.Put("disk_full_paths", diskPaths)
driver := state.Get("driver").(*DriverMock)

View File

@ -3,6 +3,7 @@ package iso
import (
"context"
"fmt"
"log"
"path/filepath"
vmwcommon "github.com/hashicorp/packer/builder/vmware/common"
@ -18,7 +19,7 @@ import (
// ui packer.Ui
//
// Produces:
// full_disk_path (string) - The full path to the created disk.
// disk_full_paths ([]string) - The full paths to all created disks
type stepCreateDisk struct{}
func (stepCreateDisk) Run(_ context.Context, state multistep.StateBag) multistep.StepAction {
@ -26,39 +27,39 @@ func (stepCreateDisk) Run(_ context.Context, state multistep.StateBag) multistep
driver := state.Get("driver").(vmwcommon.Driver)
ui := state.Get("ui").(packer.Ui)
ui.Say("Creating virtual machine disk")
full_disk_path := filepath.Join(config.OutputDir, config.DiskName+".vmdk")
if err := driver.CreateDisk(full_disk_path, fmt.Sprintf("%dM", config.DiskSize), config.DiskAdapterType, config.DiskTypeId); err != nil {
err := fmt.Errorf("Error creating disk: %s", err)
state.Put("error", err)
ui.Error(err.Error())
return multistep.ActionHalt
}
state.Put("full_disk_path", full_disk_path)
ui.Say("Creating required virtual machine disks")
// Users can configure disks at several locations in the template so
// first collate all the disk requirements
var diskPaths, diskSizes []string
// The 'main' or 'default' disk
diskPaths = append(diskPaths, filepath.Join(config.OutputDir, config.DiskName+".vmdk"))
diskSizes = append(diskSizes, fmt.Sprintf("%dM", uint64(config.DiskSize)))
// Additional disks
if len(config.AdditionalDiskSize) > 0 {
// stash the disk paths we create
additional_paths := make([]string, len(config.AdditionalDiskSize))
ui.Say("Creating additional hard drives...")
for i, additionalsize := range config.AdditionalDiskSize {
additionalpath := filepath.Join(config.OutputDir, fmt.Sprintf("%s-%d.vmdk", config.DiskName, i+1))
size := fmt.Sprintf("%dM", uint64(additionalsize))
if err := driver.CreateDisk(additionalpath, size, config.DiskAdapterType, config.DiskTypeId); err != nil {
err := fmt.Errorf("Error creating additional disk: %s", err)
state.Put("error", err)
ui.Error(err.Error())
return multistep.ActionHalt
}
additional_paths[i] = additionalpath
for i, diskSize := range config.AdditionalDiskSize {
path := filepath.Join(config.OutputDir, fmt.Sprintf("%s-%d.vmdk", config.DiskName, i+1))
diskPaths = append(diskPaths, path)
size := fmt.Sprintf("%dM", uint64(diskSize))
diskSizes = append(diskSizes, size)
}
state.Put("additional_disk_paths", additional_paths)
}
// Create all required disks
for i, diskPath := range diskPaths {
log.Printf("[INFO] Creating disk with Path: %s and Size: %s", diskPath, diskSizes[i])
// Additional disks currently use the same adapter type and disk
// type as specified for the main disk
if err := driver.CreateDisk(diskPath, diskSizes[i], config.DiskAdapterType, config.DiskTypeId); err != nil {
err := fmt.Errorf("Error creating disk: %s", err)
state.Put("error", err)
ui.Error(err.Error())
return multistep.ActionHalt
}
}
// Stash the disk paths so we can retrieve later e.g. when compacting
state.Put("disk_full_paths", diskPaths)
return multistep.ActionContinue
}

View File

@ -115,13 +115,13 @@ func (s *StepCloneVMX) Run(_ context.Context, state multistep.StateBag) multiste
}
// Write out the relative, host filesystem paths to the disks
var diskFullPaths []string
var diskPaths []string
for _, diskFilename := range diskFilenames {
log.Printf("Found attached disk with filename: %s", diskFilename)
diskFullPaths = append(diskFullPaths, filepath.Join(s.OutputDir, diskFilename))
diskPaths = append(diskPaths, filepath.Join(s.OutputDir, diskFilename))
}
if len(diskFullPaths) == 0 {
if len(diskPaths) == 0 {
state.Put("error", fmt.Errorf("Could not enumerate disk info from the vmx file"))
return multistep.ActionHalt
}
@ -139,10 +139,7 @@ func (s *StepCloneVMX) Run(_ context.Context, state multistep.StateBag) multiste
// Stash all required information in our state bag
state.Put("vmx_path", vmxPath)
// 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("disk_full_paths", diskPaths)
state.Put("vmnetwork", networkType)
return multistep.ActionContinue

View File

@ -88,17 +88,11 @@ func TestStepCloneVMX(t *testing.T) {
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 disk path: %#v", diskPath)
}
if stateDiskPaths, ok := state.GetOk("additional_disk_paths"); !ok {
t.Fatal("should set additional_disk_paths")
if stateDiskPaths, ok := state.GetOk("disk_full_paths"); !ok {
t.Fatal("should set disk_full_paths")
} else {
assert.ElementsMatchf(t, stateDiskPaths.([]string), diskPaths[1:],
"%s\nshould contain the same elements as:\n%s", stateDiskPaths.([]string), diskPaths[1:])
assert.ElementsMatchf(t, stateDiskPaths.([]string), diskPaths,
"%s\nshould contain the same elements as:\n%s", stateDiskPaths.([]string), diskPaths)
}
// Test we got the network type