Merge pull request #6411 from jmartin-r7/esxi_compact

add support for disk compaction to esx5 builder

builder/vmware/iso/builder.go

@@ -139,6 +139,14 @@ func (b *Builder) Prepare(raws ...interface{}) ([]string, error) {
 		b.config.DiskAdapterType = "lsilogic"
 	}
 
+	if !b.config.SkipCompaction {
+		if b.config.RemoteType == "esx5" {
+			if b.config.DiskTypeId == "" {
+				b.config.SkipCompaction = true
+			}
+		}
+	}
+
 	if b.config.DiskTypeId == "" {
 		// Default is growable virtual disk split in 2GB files.
 		b.config.DiskTypeId = "1"
@@ -148,6 +156,13 @@ func (b *Builder) Prepare(raws ...interface{}) ([]string, error) {
 		}
 	}
 
+	if b.config.RemoteType == "esx5" {
+		if b.config.DiskTypeId != "thin" && !b.config.SkipCompaction {
+			errs = packer.MultiErrorAppend(
+				errs, fmt.Errorf("skip_compaction must be 'true' for disk_type_id: %s", b.config.DiskTypeId))
+		}
+	}
+
 	if b.config.GuestOSType == "" {
 		b.config.GuestOSType = "other"
 	}

builder/vmware/iso/driver_esx5.go

@@ -46,7 +46,8 @@ func (d *ESX5Driver) Clone(dst, src string, linked bool) error {
 }
 
 func (d *ESX5Driver) CompactDisk(diskPathLocal string) error {
-	return nil
+	diskPath := d.datastorePath(diskPathLocal)
+	return d.sh("vmkfstools", "--punchzero", diskPath)
 }
 
 func (d *ESX5Driver) CreateDisk(diskPathLocal string, size string, adapter_type string, typeId string) error {

command/fix.go

@@ -157,6 +157,8 @@ Fixes that are run:
                              Hyper-V ISO builder templates
   hyperv-vmxc-typo           Corrects a typo in the "clone_from_vmxc_path"
                              setting. Replaces with "clone_from_vmcx_path".
+  vmware-compaction          Adds "skip_compaction = true" to "vmware-iso"
+                             builders with incompatible disk_type_id
 
 Options:
 

fix/fixer.go

@@ -38,6 +38,7 @@ func init() {
 		"powershell-escapes":         new(FixerPowerShellEscapes),
 		"hyperv-deprecations":        new(FixerHypervDeprecations),
 		"hyperv-vmxc-typo":           new(FixerHypervVmxcTypo),
+		"vmware-compaction":          new(FixerVMwareCompaction),
 	}
 
 	FixerOrder = []string{
@@ -57,7 +58,6 @@ func init() {
 		"amazon-private-ip",
 		"docker-email",
 		"powershell-escapes",
-		"hyperv-deprecations",
+		"vmware-compaction",
-		"hyperv-vmxc-typo",
 	}
 }

fix/fixer_vmware_compaction.go

@@ -0,0 +1,91 @@
+package fix
+
+import (
+	"github.com/mitchellh/mapstructure"
+)
+
+// FixerVMwareCompaction adds "skip_compaction = true" to "vmware-iso" builders with incompatible disk_type_id
+type FixerVMwareCompaction struct{}
+
+func (FixerVMwareCompaction) Fix(input map[string]interface{}) (map[string]interface{}, error) {
+	// The type we'll decode into; we only care about builders
+	type template struct {
+		Builders []map[string]interface{}
+	}
+
+	// Decode the input into our structure, if we can
+	var tpl template
+	if err := mapstructure.Decode(input, &tpl); err != nil {
+		return nil, err
+	}
+
+	for _, builder := range tpl.Builders {
+		builderTypeRaw, ok := builder["type"]
+		if !ok {
+			continue
+		}
+
+		builderType, ok := builderTypeRaw.(string)
+		if !ok {
+			continue
+		}
+
+		if builderType != "vmware-iso" {
+			continue
+		}
+
+		builderRemoteTypeRaw, ok := builder["remote_type"]
+		if !ok {
+			continue
+		}
+
+		builderRemoteType, ok := builderRemoteTypeRaw.(string)
+		if !ok {
+			continue
+		}
+
+		if builderRemoteType != "esx5" {
+			continue
+		}
+
+		builderDiskTypeIdRaw, ok := builder["disk_type_id"]
+		// set to default when this fixer was added due to incompatibility of defaults
+		if !ok {
+			builderDiskTypeId := "zeroedthick"
+			builder["disk_type_id"] = builderDiskTypeId
+		}
+
+		if ok {
+			builderDiskTypeId, ok := builderDiskTypeIdRaw.(string)
+			if !ok {
+				continue
+			}
+			if builderDiskTypeId == "thin" {
+				continue
+			}
+		}
+
+		builderSkipCompactionRaw, ok := builder["skip_compaction"]
+		// already verified this is not creating a "thin" disk, will need to skip_compaction
+		if ok {
+			builderSkipCompaction, ok := builderSkipCompactionRaw.(bool)
+			if !ok {
+				continue
+			}
+			if !builderSkipCompaction {
+				builder["skip_compaction"] = !builderSkipCompaction
+			}
+			continue
+		}
+
+		builderSkipCompaction := true
+		builder["skip_compaction"] = builderSkipCompaction
+	}
+
+	input["builders"] = tpl.Builders
+	return input, nil
+}
+
+func (FixerVMwareCompaction) Synopsis() string {
+	return `Adds "skip_compaction = true" to "vmware-iso" builders with incompatible disk_type_id`
+}

fix/fixer_vmware_compaction_test.go

@@ -0,0 +1,112 @@
+package fix
+
+import (
+	"reflect"
+	"testing"
+)
+
+func TestFixerVMwareCompaction_impl(t *testing.T) {
+	var _ Fixer = new(FixerVMwareCompaction)
+}
+
+func TestFixerVMwareCompaction_Fix(t *testing.T) {
+	cases := []struct {
+		Input    map[string]interface{}
+		Expected map[string]interface{}
+	}{
+		{
+			Input: map[string]interface{}{
+				"type": "virtualbox-iso",
+			},
+
+			Expected: map[string]interface{}{
+				"type": "virtualbox-iso",
+			},
+		},
+		{
+			Input: map[string]interface{}{
+				"type": "vmware-iso",
+			},
+
+			Expected: map[string]interface{}{
+				"type": "vmware-iso",
+			},
+		},
+		{
+			Input: map[string]interface{}{
+				"type":        "vmware-iso",
+				"remote_type": "esx5",
+			},
+
+			Expected: map[string]interface{}{
+				"type":            "vmware-iso",
+				"remote_type":     "esx5",
+				"disk_type_id":    "zeroedthick",
+				"skip_compaction": true,
+			},
+		},
+		{
+			Input: map[string]interface{}{
+				"type":         "vmware-iso",
+				"remote_type":  "esx5",
+				"disk_type_id": "zeroedthick",
+			},
+
+			Expected: map[string]interface{}{
+				"type":            "vmware-iso",
+				"remote_type":     "esx5",
+				"disk_type_id":    "zeroedthick",
+				"skip_compaction": true,
+			},
+		},
+		{
+			Input: map[string]interface{}{
+				"type":            "vmware-iso",
+				"remote_type":     "esx5",
+				"disk_type_id":    "zeroedthick",
+				"skip_compaction": false,
+			},
+
+			Expected: map[string]interface{}{
+				"type":            "vmware-iso",
+				"remote_type":     "esx5",
+				"disk_type_id":    "zeroedthick",
+				"skip_compaction": true,
+			},
+		},
+		{
+			Input: map[string]interface{}{
+				"type":         "vmware-iso",
+				"remote_type":  "esx5",
+				"disk_type_id": "thin",
+			},
+
+			Expected: map[string]interface{}{
+				"type":         "vmware-iso",
+				"remote_type":  "esx5",
+				"disk_type_id": "thin",
+			},
+		},
+	}
+
+	for _, tc := range cases {
+		var f FixerVMwareCompaction
+
+		input := map[string]interface{}{
+			"builders": []map[string]interface{}{tc.Input},
+		}
+
+		expected := map[string]interface{}{
+			"builders": []map[string]interface{}{tc.Expected},
+		}
+
+		output, err := f.Fix(input)
+		if err != nil {
+			t.Fatalf("err: %s", err)
+		}
+
+		if !reflect.DeepEqual(output, expected) {
+			t.Fatalf("unexpected: %#v\nexpected: %#v\n", output, expected)
+		}
+	}
+}

website/source/docs/builders/vmware-iso.html.md.erb

@@ -146,8 +146,9 @@ builder.
     The default is `1`.
 
     For ESXi, this defaults to `zeroedthick`. The available options for ESXi
-    are: `zeroedthick`, `eagerzeroedthick`, `thin`, `rdm:dev`, `rdmp:dev`,
+    are: `zeroedthick`, `eagerzeroedthick`, `thin`. `rdm:dev`, `rdmp:dev`,
-    `2gbsparse`.
+    `2gbsparse` are not supported. Due to default disk compaction, when using
+    `zeroedthick` or `eagerzeroedthick` set `skip_compaction` to `true`.
 
     For more information, please consult the [Virtual Disk Manager User's
     Guide](https://www.vmware.com/pdf/VirtualDiskManager.pdf) for desktop
@@ -326,10 +327,12 @@ builder.
     `5m` or five minutes.
 
 -   `skip_compaction` (boolean) - VMware-created disks are defragmented and
-    compacted at the end of the build process using `vmware-vdiskmanager`. In
+    compacted at the end of the build process using `vmware-vdiskmanager` or
-    certain rare cases, this might actually end up making the resulting disks
+    `vmkfstools` in ESXi. In certain rare cases, this might actually end up
-    slightly larger. If you find this to be the case, you can disable compaction
+    making the resulting disks slightly larger. If you find this to be the case,
-    using this configuration value. Defaults to `false`.
+    you can disable compaction using this configuration value. Defaults to
+    `false`. Default to `true` for ESXi when `disk_type_id` is not explicitly
+    defined and `false` otherwise.
 
 -   `skip_export` (boolean) - Defaults to `false`. When enabled, Packer will
     not export the VM. Useful if the build output is not the resultant