From 823275939767d2ec32a50206430ca506334389d6 Mon Sep 17 00:00:00 2001 From: Taliesin Sisson Date: Mon, 29 May 2017 02:36:01 +0100 Subject: [PATCH] If vhd or vhdx extension is specified for ISOUrls, we want to use an existing hard drive which means that we don't need to specify hard drive size Filepath.ext includes the dot --- builder/hyperv/common/step_clone_vm.go | 2 +- builder/hyperv/common/step_create_vm.go | 2 +- builder/hyperv/iso/builder.go | 11 +-- builder/hyperv/iso/builder_test.go | 92 +++++++++++++++++++++++++ 4 files changed, 96 insertions(+), 11 deletions(-) diff --git a/builder/hyperv/common/step_clone_vm.go b/builder/hyperv/common/step_clone_vm.go index 9b005422d..f867bd331 100644 --- a/builder/hyperv/common/step_clone_vm.go +++ b/builder/hyperv/common/step_clone_vm.go @@ -40,7 +40,7 @@ func (s *StepCloneVM) Run(state multistep.StateBag) multistep.StepAction { harddrivePath := "" if harddrivePathRaw, ok := state.GetOk("iso_path"); ok { extension := strings.ToLower(filepath.Ext(harddrivePathRaw.(string))) - if extension == "vhd" || extension == "vhdx" { + if extension == ".vhd" || extension == ".vhdx" { harddrivePath = harddrivePathRaw.(string) } else { log.Println("No existing virtual harddrive, not attaching.") diff --git a/builder/hyperv/common/step_create_vm.go b/builder/hyperv/common/step_create_vm.go index 46c4964cb..4b9601842 100644 --- a/builder/hyperv/common/step_create_vm.go +++ b/builder/hyperv/common/step_create_vm.go @@ -39,7 +39,7 @@ func (s *StepCreateVM) Run(state multistep.StateBag) multistep.StepAction { harddrivePath := "" if harddrivePathRaw, ok := state.GetOk("iso_path"); ok { extension := strings.ToLower(filepath.Ext(harddrivePathRaw.(string))) - if extension == "vhd" || extension == "vhdx" { + if extension == ".vhd" || extension == ".vhdx" { harddrivePath = harddrivePathRaw.(string) } else { log.Println("No existing virtual harddrive, not attaching.") diff --git a/builder/hyperv/iso/builder.go b/builder/hyperv/iso/builder.go index 382c35394..4d5583239 100644 --- a/builder/hyperv/iso/builder.go +++ b/builder/hyperv/iso/builder.go @@ -117,22 +117,15 @@ func (b *Builder) Prepare(raws ...interface{}) ([]string, error) { isoWarnings, isoErrs := b.config.ISOConfig.Prepare(&b.config.ctx) warnings = append(warnings, isoWarnings...) errs = packer.MultiErrorAppend(errs, isoErrs...) - - if len(b.config.ISOConfig.ISOUrls) > 0 { - extension := strings.ToLower(filepath.Ext(b.config.ISOConfig.ISOUrls[0])) - if extension == "vhd" || extension == "vhdx" { - b.config.ISOConfig.TargetExtension = extension - } - } errs = packer.MultiErrorAppend(errs, b.config.FloppyConfig.Prepare(&b.config.ctx)...) errs = packer.MultiErrorAppend(errs, b.config.HTTPConfig.Prepare(&b.config.ctx)...) errs = packer.MultiErrorAppend(errs, b.config.RunConfig.Prepare(&b.config.ctx)...) errs = packer.MultiErrorAppend(errs, b.config.OutputConfig.Prepare(&b.config.ctx, &b.config.PackerConfig)...) errs = packer.MultiErrorAppend(errs, b.config.SSHConfig.Prepare(&b.config.ctx)...) - errs = packer.MultiErrorAppend(errs, b.config.ShutdownConfig.Prepare(&b.config.ctx)...) + errs = packer.MultiErrorAppend(errs, b.config.ShutdownConfig.Prepare(&b.config.ctx)...) - if b.config.ISOConfig.TargetExtension != "vhd" && b.config.ISOConfig.TargetExtension != "vhdx" { + if len(b.config.ISOConfig.ISOUrls) < 1 || (strings.ToLower(filepath.Ext(b.config.ISOConfig.ISOUrls[0])) != ".vhd" && strings.ToLower(filepath.Ext(b.config.ISOConfig.ISOUrls[0])) != ".vhdx") { //We only create a new hard drive if an existing one to copy from does not exist err = b.checkDiskSize() if err != nil { diff --git a/builder/hyperv/iso/builder_test.go b/builder/hyperv/iso/builder_test.go index d32ba4f41..0fea9b8e0 100644 --- a/builder/hyperv/iso/builder_test.go +++ b/builder/hyperv/iso/builder_test.go @@ -300,6 +300,98 @@ func TestBuilderPrepare_ISOUrl(t *testing.T) { } } +func TestBuilderPrepare_SizeNotRequiredWhenUsingExistingHarddrive(t *testing.T) { + var b Builder + config := testConfig() + delete(config, "iso_url") + delete(config, "iso_urls") + delete(config, "disk_size") + + config["disk_size"] = 1 + + // Test just iso_urls set but with vhdx + delete(config, "iso_url") + config["iso_urls"] = []string{ + "http://www.packer.io/hdd.vhdx", + "http://www.hashicorp.com/dvd.iso", + } + + b = Builder{} + warns, err := b.Prepare(config) + if len(warns) > 0 { + t.Fatalf("bad: %#v", warns) + } + if err != nil { + t.Errorf("should not have error: %s", err) + } + + expected := []string{ + "http://www.packer.io/hdd.vhdx", + "http://www.hashicorp.com/dvd.iso", + } + if !reflect.DeepEqual(b.config.ISOUrls, expected) { + t.Fatalf("bad: %#v", b.config.ISOUrls) + } + + // Test just iso_urls set but with vhd + delete(config, "iso_url") + config["iso_urls"] = []string{ + "http://www.packer.io/hdd.vhd", + "http://www.hashicorp.com/dvd.iso", + } + + b = Builder{} + warns, err = b.Prepare(config) + if len(warns) > 0 { + t.Fatalf("bad: %#v", warns) + } + if err != nil { + t.Errorf("should not have error: %s", err) + } + + expected = []string{ + "http://www.packer.io/hdd.vhd", + "http://www.hashicorp.com/dvd.iso", + } + if !reflect.DeepEqual(b.config.ISOUrls, expected) { + t.Fatalf("bad: %#v", b.config.ISOUrls) + } +} + +func TestBuilderPrepare_SizeIsRequiredWhenNotUsingExistingHarddrive(t *testing.T) { + var b Builder + config := testConfig() + delete(config, "iso_url") + delete(config, "iso_urls") + delete(config, "disk_size") + + config["disk_size"] = 1 + + // Test just iso_urls set but with vhdx + delete(config, "iso_url") + config["iso_urls"] = []string{ + "http://www.packer.io/os.iso", + "http://www.hashicorp.com/dvd.iso", + } + + b = Builder{} + warns, err := b.Prepare(config) + if len(warns) > 0 { + t.Fatalf("bad: %#v", warns) + } + if err == nil { + t.Errorf("should have error") + } + + expected := []string{ + "http://www.packer.io/os.iso", + "http://www.hashicorp.com/dvd.iso", + } + if !reflect.DeepEqual(b.config.ISOUrls, expected) { + t.Fatalf("bad: %#v", b.config.ISOUrls) + } +} + func TestBuilderPrepare_FloppyFiles(t *testing.T) { var b Builder config := testConfig()