From 9eb32e5dcdd881e99b52ca1de2a0140874fc40f4 Mon Sep 17 00:00:00 2001 From: Matt Behrens Date: Fri, 23 May 2014 21:14:24 -0400 Subject: [PATCH 1/2] implement `iso_interface` setting Adds a new configuration option, `iso_interface`, that can be set to `sata` to mount the ISO on the SATA controller. Required for OS X. --- .../virtualbox/common/step_remove_devices.go | 15 +++++-- .../common/step_remove_devices_test.go | 27 ++++++++++++ builder/virtualbox/iso/builder.go | 11 +++++ builder/virtualbox/iso/builder_test.go | 41 +++++++++++++++++++ builder/virtualbox/iso/step_attach_iso.go | 35 +++++++++++++--- builder/virtualbox/iso/step_create_disk.go | 13 +++--- 6 files changed, 128 insertions(+), 14 deletions(-) diff --git a/builder/virtualbox/common/step_remove_devices.go b/builder/virtualbox/common/step_remove_devices.go index 2a11dcbba..8f06b1a61 100644 --- a/builder/virtualbox/common/step_remove_devices.go +++ b/builder/virtualbox/common/step_remove_devices.go @@ -41,11 +41,20 @@ func (s *StepRemoveDevices) Run(state multistep.StateBag) multistep.StepAction { } if _, ok := state.GetOk("attachedIso"); ok { + controllerName := "IDE Controller" + port := "0" + device := "1" + if _, ok := state.GetOk("attachedIsoOnSata"); ok { + controllerName = "SATA Controller" + port = "1" + device = "0" + } + command := []string{ "storageattach", vmName, - "--storagectl", "IDE Controller", - "--port", "0", - "--device", "1", + "--storagectl", controllerName, + "--port", port, + "--device", device, "--medium", "none", } diff --git a/builder/virtualbox/common/step_remove_devices_test.go b/builder/virtualbox/common/step_remove_devices_test.go index e74331232..e3df5e6e7 100644 --- a/builder/virtualbox/common/step_remove_devices_test.go +++ b/builder/virtualbox/common/step_remove_devices_test.go @@ -57,6 +57,33 @@ func TestStepRemoveDevices_attachedIso(t *testing.T) { } } +func TestStepRemoveDevices_attachedIsoOnSata(t *testing.T) { + state := testState(t) + step := new(StepRemoveDevices) + + state.Put("attachedIso", true) + state.Put("attachedIsoOnSata", true) + state.Put("vmName", "foo") + + driver := state.Get("driver").(*DriverMock) + + // Test the run + if action := step.Run(state); action != multistep.ActionContinue { + t.Fatalf("bad action: %#v", action) + } + if _, ok := state.GetOk("error"); ok { + t.Fatal("should NOT have error") + } + + // Test that ISO was removed + if len(driver.VBoxManageCalls) != 1 { + t.Fatalf("bad: %#v", driver.VBoxManageCalls) + } + if driver.VBoxManageCalls[0][3] != "SATA Controller" { + t.Fatalf("bad: %#v", driver.VBoxManageCalls) + } +} + func TestStepRemoveDevices_floppyPath(t *testing.T) { state := testState(t) step := new(StepRemoveDevices) diff --git a/builder/virtualbox/iso/builder.go b/builder/virtualbox/iso/builder.go index f41118a45..bbe1ea45c 100644 --- a/builder/virtualbox/iso/builder.go +++ b/builder/virtualbox/iso/builder.go @@ -44,6 +44,7 @@ type config struct { HTTPPortMax uint `mapstructure:"http_port_max"` ISOChecksum string `mapstructure:"iso_checksum"` ISOChecksumType string `mapstructure:"iso_checksum_type"` + ISOInterface string `mapstructure:"iso_interface"` ISOUrls []string `mapstructure:"iso_urls"` VMName string `mapstructure:"vm_name"` @@ -107,6 +108,10 @@ func (b *Builder) Prepare(raws ...interface{}) ([]string, error) { b.config.HTTPPortMax = 9000 } + if b.config.ISOInterface == "" { + b.config.ISOInterface = "ide" + } + if b.config.VMName == "" { b.config.VMName = fmt.Sprintf("packer-%s", b.config.PackerBuildName) } @@ -120,6 +125,7 @@ func (b *Builder) Prepare(raws ...interface{}) ([]string, error) { "http_directory": &b.config.HTTPDir, "iso_checksum": &b.config.ISOChecksum, "iso_checksum_type": &b.config.ISOChecksumType, + "iso_interface": &b.config.ISOInterface, "iso_url": &b.config.RawSingleISOUrl, "vm_name": &b.config.VMName, } @@ -192,6 +198,11 @@ func (b *Builder) Prepare(raws ...interface{}) ([]string, error) { } } + if b.config.ISOInterface != "ide" && b.config.ISOInterface != "sata" { + errs = packer.MultiErrorAppend( + errs, errors.New("iso_interface can only be ide or sata")) + } + if b.config.RawSingleISOUrl == "" && len(b.config.ISOUrls) == 0 { errs = packer.MultiErrorAppend( errs, errors.New("One of iso_url or iso_urls must be specified.")) diff --git a/builder/virtualbox/iso/builder_test.go b/builder/virtualbox/iso/builder_test.go index c9d5414dd..f3a4519af 100644 --- a/builder/virtualbox/iso/builder_test.go +++ b/builder/virtualbox/iso/builder_test.go @@ -401,6 +401,47 @@ func TestBuilderPrepare_ISOChecksumType(t *testing.T) { } } +func TestBuilderPrepare_ISOInterface(t *testing.T) { + var b Builder + config := testConfig() + + // Test a default boot_wait + delete(config, "iso_interface") + warns, err := b.Prepare(config) + if len(warns) > 0 { + t.Fatalf("bad: %#v", warns) + } + if err != nil { + t.Fatalf("err: %s", err) + } + + if b.config.ISOInterface != "ide" { + t.Fatalf("bad: %s", b.config.ISOInterface) + } + + // Test with a bad + config["iso_interface"] = "fake" + b = Builder{} + warns, err = b.Prepare(config) + if len(warns) > 0 { + t.Fatalf("bad: %#v", warns) + } + if err == nil { + t.Fatal("should have error") + } + + // Test with a good + config["iso_interface"] = "sata" + b = Builder{} + warns, err = b.Prepare(config) + if len(warns) > 0 { + t.Fatalf("bad: %#v", warns) + } + if err != nil { + t.Fatalf("should not have error: %s", err) + } +} + func TestBuilderPrepare_ISOUrl(t *testing.T) { var b Builder config := testConfig() diff --git a/builder/virtualbox/iso/step_attach_iso.go b/builder/virtualbox/iso/step_attach_iso.go index f18281434..7fac2ec42 100644 --- a/builder/virtualbox/iso/step_attach_iso.go +++ b/builder/virtualbox/iso/step_attach_iso.go @@ -17,17 +17,27 @@ type stepAttachISO struct { } func (s *stepAttachISO) Run(state multistep.StateBag) multistep.StepAction { + config := state.Get("config").(*config) driver := state.Get("driver").(vboxcommon.Driver) isoPath := state.Get("iso_path").(string) ui := state.Get("ui").(packer.Ui) vmName := state.Get("vmName").(string) + controllerName := "IDE Controller" + port := "0" + device := "1" + if config.ISOInterface == "sata" { + controllerName = "SATA Controller" + port = "1" + device = "0" + } + // Attach the disk to the controller command := []string{ "storageattach", vmName, - "--storagectl", "IDE Controller", - "--port", "0", - "--device", "1", + "--storagectl", controllerName, + "--port", port, + "--device", device, "--type", "dvddrive", "--medium", isoPath, } @@ -43,6 +53,9 @@ func (s *stepAttachISO) Run(state multistep.StateBag) multistep.StepAction { // Set some state so we know to remove state.Put("attachedIso", true) + if controllerName == "SATA Controller" { + state.Put("attachedIsoOnSata", true) + } return multistep.ActionContinue } @@ -52,14 +65,24 @@ func (s *stepAttachISO) Cleanup(state multistep.StateBag) { return } + config := state.Get("config").(*config) driver := state.Get("driver").(vboxcommon.Driver) vmName := state.Get("vmName").(string) + controllerName := "IDE Controller" + port := "0" + device := "1" + if config.ISOInterface == "sata" { + controllerName = "SATA Controller" + port = "1" + device = "0" + } + command := []string{ "storageattach", vmName, - "--storagectl", "IDE Controller", - "--port", "0", - "--device", "1", + "--storagectl", controllerName, + "--port", port, + "--device", device, "--medium", "none", } diff --git a/builder/virtualbox/iso/step_create_disk.go b/builder/virtualbox/iso/step_create_disk.go index 6875d7b33..564a2f4d3 100644 --- a/builder/virtualbox/iso/step_create_disk.go +++ b/builder/virtualbox/iso/step_create_disk.go @@ -43,8 +43,7 @@ func (s *stepCreateDisk) Run(state multistep.StateBag) multistep.StepAction { // Add the IDE controller so we can later attach the disk. // When the hard disk controller is not IDE, this device is still used // by VirtualBox to deliver the guest extensions. - controllerName := "IDE Controller" - err = driver.VBoxManage("storagectl", vmName, "--name", controllerName, "--add", "ide") + err = driver.VBoxManage("storagectl", vmName, "--name", "IDE Controller", "--add", "ide") if err != nil { err := fmt.Errorf("Error creating disk controller: %s", err) state.Put("error", err) @@ -55,9 +54,8 @@ func (s *stepCreateDisk) Run(state multistep.StateBag) multistep.StepAction { // Add a SATA controller if we were asked to use SATA. We still attach // the IDE controller above because some other things (disks) require // that. - if config.HardDriveInterface == "sata" { - controllerName = "SATA Controller" - if err := driver.CreateSATAController(vmName, controllerName); err != nil { + if config.HardDriveInterface == "sata" || config.ISOInterface == "sata" { + if err := driver.CreateSATAController(vmName, "SATA Controller"); err != nil { err := fmt.Errorf("Error creating disk controller: %s", err) state.Put("error", err) ui.Error(err.Error()) @@ -66,6 +64,11 @@ func (s *stepCreateDisk) Run(state multistep.StateBag) multistep.StepAction { } // Attach the disk to the controller + controllerName := "IDE Controller" + if config.HardDriveInterface == "sata" { + controllerName = "SATA Controller" + } + command = []string{ "storageattach", vmName, "--storagectl", controllerName, From 32f6553d04d774b901fbb069ad13b50e6dc8294f Mon Sep 17 00:00:00 2001 From: Matt Behrens Date: Sat, 24 May 2014 07:20:05 -0400 Subject: [PATCH 2/2] add documentation for `iso_interface` --- website/source/docs/builders/virtualbox-iso.html.markdown | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/website/source/docs/builders/virtualbox-iso.html.markdown b/website/source/docs/builders/virtualbox-iso.html.markdown index 2626a4ebe..5ddbad552 100644 --- a/website/source/docs/builders/virtualbox-iso.html.markdown +++ b/website/source/docs/builders/virtualbox-iso.html.markdown @@ -156,6 +156,10 @@ each category, the available options are alphabetized and described. server to be on one port, make this minimum and maximum port the same. By default the values are 8000 and 9000, respectively. +* `iso_interface` (string) - The type of controller that the ISO is attached + to, defaults to "ide". When set to "sata", the drive is attached to an + AHCI SATA controller. + * `iso_urls` (array of strings) - Multiple URLs for the ISO to download. Packer will try these in order. If anything goes wrong attempting to download or while downloading a single URL, it will move on to the next. All URLs