diff --git a/builder/qemu/builder.go b/builder/qemu/builder.go index d52a74222..4c6330aa1 100644 --- a/builder/qemu/builder.go +++ b/builder/qemu/builder.go @@ -192,6 +192,8 @@ func (b *Builder) Run(ctx context.Context, ui packer.Ui, hook packer.Hook) (pack artifact.state["generated_data"] = state.Get("generated_data") artifact.state["diskName"] = b.config.VMName + + // placed in state in steap_create_disk.go diskpaths, ok := state.Get("qemu_disk_paths").([]string) if ok { artifact.state["diskPaths"] = diskpaths diff --git a/builder/qemu/step_run.go b/builder/qemu/step_run.go index d7f60d37f..37a5daf5a 100644 --- a/builder/qemu/step_run.go +++ b/builder/qemu/step_run.go @@ -16,41 +16,45 @@ import ( // stepRun runs the virtual machine type stepRun struct { DiskImage bool -} -type qemuArgsTemplateData struct { - HTTPIP string - HTTPPort int - HTTPDir string - OutputDir string - Name string - SSHHostPort int + atLeastVersion2 bool + ui packer.Ui } func (s *stepRun) Run(ctx context.Context, state multistep.StateBag) multistep.StepAction { + config := state.Get("config").(*Config) driver := state.Get("driver").(Driver) - ui := state.Get("ui").(packer.Ui) + s.ui = state.Get("ui").(packer.Ui) - // Run command is different depending whether we're booting from an - // installation CD or a pre-baked image - bootDrive := "once=d" - message := "Starting VM, booting from CD-ROM" - if s.DiskImage { - bootDrive = "c" - message = "Starting VM, booting disk image" + // Figure out version of qemu; store on step for later use + rawVersion, err := driver.Version() + if err != nil { + err := fmt.Errorf("Error determining qemu version: %s", err) + s.ui.Error(err.Error()) + return multistep.ActionHalt } - ui.Say(message) + qemuVersion, err := version.NewVersion(rawVersion) + if err != nil { + err := fmt.Errorf("Error parsing qemu version: %s", err) + s.ui.Error(err.Error()) + return multistep.ActionHalt + } + v2 := version.Must(version.NewVersion("2.0")) - command, err := getCommandArgs(bootDrive, state) + s.atLeastVersion2 = qemuVersion.GreaterThanOrEqual(v2) + + // Generate the qemu command + command, err := s.getCommandArgs(config, state) if err != nil { err := fmt.Errorf("Error processing QemuArgs: %s", err) - ui.Error(err.Error()) + s.ui.Error(err.Error()) return multistep.ActionHalt } + // run the qemu command if err := driver.Qemu(command...); err != nil { err := fmt.Errorf("Error launching VM: %s", err) - ui.Error(err.Error()) + s.ui.Error(err.Error()) return multistep.ActionHalt } @@ -66,140 +70,187 @@ func (s *stepRun) Cleanup(state multistep.StateBag) { } } -func getCommandArgs(bootDrive string, state multistep.StateBag) ([]string, error) { - config := state.Get("config").(*Config) - isoPath := state.Get("iso_path").(string) - vncIP := config.VNCBindAddress - vncPort := state.Get("vnc_port").(int) - ui := state.Get("ui").(packer.Ui) - driver := state.Get("driver").(Driver) - vmName := config.VMName - imgPath := filepath.Join(config.OutputDir, vmName) +func (s *stepRun) getDefaultArgs(config *Config, state multistep.StateBag) map[string]interface{} { defaultArgs := make(map[string]interface{}) - var deviceArgs []string - var driveArgs []string - var commHostPort int + + // Configure "boot" arguement + // Run command is different depending whether we're booting from an + // installation CD or a pre-baked image + bootDrive := "once=d" + message := "Starting VM, booting from CD-ROM" + if s.DiskImage { + bootDrive = "c" + message = "Starting VM, booting disk image" + } + s.ui.Say(message) + defaultArgs["-boot"] = bootDrive + + // configure "-qmp" arguments + if config.QMPEnable { + defaultArgs["-qmp"] = fmt.Sprintf("unix:%s,server,nowait", config.QMPSocketPath) + } + + // configure "-name" arguments + defaultArgs["-name"] = config.VMName + + // Configure "-machine" arguments + if config.Accelerator == "none" { + defaultArgs["-machine"] = fmt.Sprintf("type=%s", config.MachineType) + s.ui.Message("WARNING: The VM will be started with no hardware acceleration.\n" + + "The installation may take considerably longer to finish.\n") + } else { + defaultArgs["-machine"] = fmt.Sprintf("type=%s,accel=%s", + config.MachineType, config.Accelerator) + } + + // Configure "-netdev" arguments + defaultArgs["-netdev"] = fmt.Sprintf("bridge,id=user.0,br=%s", config.NetBridge) + if config.NetBridge == "" { + defaultArgs["-netdev"] = fmt.Sprintf("user,id=user.0") + if config.CommConfig.Comm.Type != "none" { + commHostPort := state.Get("commHostPort").(int) + defaultArgs["-netdev"] = fmt.Sprintf("user,id=user.0,hostfwd=tcp::%v-:%d", commHostPort, config.CommConfig.Comm.Port()) + } + } + + // Configure "-vnc" arguments + // vncPort is always set in stepConfigureVNC, so we don't need to + // defensively assert + vncPort := state.Get("vnc_port").(int) + vncIP := config.VNCBindAddress vncPort = vncPort - config.VNCPortMin vnc := fmt.Sprintf("%s:%d", vncIP, vncPort) if config.VNCUsePassword { vnc = fmt.Sprintf("%s:%d,password", vncIP, vncPort) } + defaultArgs["-vnc"] = vnc - if config.QMPEnable { - defaultArgs["-qmp"] = fmt.Sprintf("unix:%s,server,nowait", config.QMPSocketPath) + // Track the connection for the user + vncPass, _ := state.Get("vnc_password").(string) + + message = getVncConnectionMessage(config.Headless, vnc, vncPass) + if message != "" { + s.ui.Message(message) } - defaultArgs["-name"] = vmName - defaultArgs["-machine"] = fmt.Sprintf("type=%s", config.MachineType) + // Configure "-m" memory argument + defaultArgs["-m"] = fmt.Sprintf("%dM", config.MemorySize) - if config.NetBridge == "" { - if config.CommConfig.Comm.Type != "none" { - commHostPort = state.Get("commHostPort").(int) - defaultArgs["-netdev"] = fmt.Sprintf("user,id=user.0,hostfwd=tcp::%v-:%d", commHostPort, config.CommConfig.Comm.Port()) - } else { - defaultArgs["-netdev"] = fmt.Sprintf("user,id=user.0") - } + // Configure "-smp" processor hardware arguments + if config.CpuCount > 1 { + defaultArgs["-smp"] = fmt.Sprintf("cpus=%d,sockets=%d", config.CpuCount, config.CpuCount) + } + + // Configure "-fda" floppy disk attachment + if floppyPathRaw, ok := state.GetOk("floppy_path"); ok { + defaultArgs["-fda"] = floppyPathRaw.(string) } else { - defaultArgs["-netdev"] = fmt.Sprintf("bridge,id=user.0,br=%s", config.NetBridge) + log.Println("Qemu Builder has no floppy files, not attaching a floppy.") } - rawVersion, err := driver.Version() - if err != nil { - return nil, err - } - qemuVersion, err := version.NewVersion(rawVersion) - if err != nil { - return nil, err - } - v2 := version.Must(version.NewVersion("2.0")) - - if qemuVersion.GreaterThanOrEqual(v2) { - if config.DiskInterface == "virtio-scsi" { - if config.DiskImage { - deviceArgs = append(deviceArgs, "virtio-scsi-pci,id=scsi0", "scsi-hd,bus=scsi0.0,drive=drive0") - driveArgumentString := fmt.Sprintf("if=none,file=%s,id=drive0,cache=%s,discard=%s,format=%s", imgPath, config.DiskCache, config.DiskDiscard, config.Format) - if config.DetectZeroes != "off" { - driveArgumentString = fmt.Sprintf("%s,detect-zeroes=%s", driveArgumentString, config.DetectZeroes) - } - driveArgs = append(driveArgs, driveArgumentString) - } else { - deviceArgs = append(deviceArgs, "virtio-scsi-pci,id=scsi0") - diskFullPaths := state.Get("qemu_disk_paths").([]string) - for i, diskFullPath := range diskFullPaths { - deviceArgs = append(deviceArgs, fmt.Sprintf("scsi-hd,bus=scsi0.0,drive=drive%d", i)) - driveArgumentString := fmt.Sprintf("if=none,file=%s,id=drive%d,cache=%s,discard=%s,format=%s", diskFullPath, i, config.DiskCache, config.DiskDiscard, config.Format) - if config.DetectZeroes != "off" { - driveArgumentString = fmt.Sprintf("%s,detect-zeroes=%s", driveArgumentString, config.DetectZeroes) - } - driveArgs = append(driveArgs, driveArgumentString) - } - } - } else { - if config.DiskImage { - driveArgumentString := fmt.Sprintf("file=%s,if=%s,cache=%s,discard=%s,format=%s", imgPath, config.DiskInterface, config.DiskCache, config.DiskDiscard, config.Format) - if config.DetectZeroes != "off" { - driveArgumentString = fmt.Sprintf("%s,detect-zeroes=%s", driveArgumentString, config.DetectZeroes) - } - driveArgs = append(driveArgs, driveArgumentString) - } else { - diskFullPaths := state.Get("qemu_disk_paths").([]string) - for _, diskFullPath := range diskFullPaths { - driveArgumentString := fmt.Sprintf("file=%s,if=%s,cache=%s,discard=%s,format=%s", diskFullPath, config.DiskInterface, config.DiskCache, config.DiskDiscard, config.Format) - if config.DetectZeroes != "off" { - driveArgumentString = fmt.Sprintf("%s,detect-zeroes=%s", driveArgumentString, config.DetectZeroes) - } - driveArgs = append(driveArgs, driveArgumentString) - } - } - } - } else { - driveArgs = append(driveArgs, fmt.Sprintf("file=%s,if=%s,cache=%s,format=%s", imgPath, config.DiskInterface, config.DiskCache, config.Format)) - } - deviceArgs = append(deviceArgs, fmt.Sprintf("%s,netdev=user.0", config.NetDevice)) - - if config.Headless == true { - vncPortRaw, vncPortOk := state.GetOk("vnc_port") - vncPass := state.Get("vnc_password") - - if vncPortOk && vncPass != nil && len(vncPass.(string)) > 0 { - vncPort := vncPortRaw.(int) - - ui.Message(fmt.Sprintf( - "The VM will be run headless, without a GUI. If you want to\n"+ - "view the screen of the VM, connect via VNC to vnc://%s:%d\n"+ - "with the password: %s", vncIP, vncPort, vncPass)) - } else if vncPortOk { - vncPort := vncPortRaw.(int) - - ui.Message(fmt.Sprintf( - "The VM will be run headless, without a GUI. If you want to\n"+ - "view the screen of the VM, connect via VNC without a password to\n"+ - "vnc://%s:%d", vncIP, vncPort)) - } else { - ui.Message("The VM will be run headless, without a GUI, as configured.\n" + - "If the run isn't succeeding as you expect, please enable the GUI\n" + - "to inspect the progress of the build.") - } - } else { - if qemuVersion.GreaterThanOrEqual(v2) { - if len(config.Display) > 0 { - if config.Display != "none" { - defaultArgs["-display"] = config.Display - } + // Configure GUI display + if !config.Headless { + if s.atLeastVersion2 { + // FIXME: "none" is a valid display option in qemu but we have + // departed from the qemu usage here to instaed mean "let qemu + // set a reasonable default". We need to deprecate this behavior + // and let users just set "UseDefaultDisplay" if they want to let + // qemu do its thing. + if len(config.Display) > 0 && config.Display != "none" { + defaultArgs["-display"] = config.Display } else if !config.UseDefaultDisplay { defaultArgs["-display"] = "gtk" } } else { - ui.Message("WARNING: The version of qemu on your host doesn't support display mode.\n" + + s.ui.Message("WARNING: The version of qemu on your host doesn't support display mode.\n" + "The display parameter will be ignored.") } } + deviceArgs, driveArgs := s.getDeviceAndDriveArgs(config, state) + defaultArgs["-device"] = deviceArgs + defaultArgs["-drive"] = driveArgs + + return defaultArgs +} + +func getVncConnectionMessage(headless bool, vnc string, vncPass string) string { + // Configure GUI display + if headless { + if vnc == "" { + return "The VM will be run headless, without a GUI, as configured.\n" + + "If the run isn't succeeding as you expect, please enable the GUI\n" + + "to inspect the progress of the build." + } + + if vncPass != "" { + return fmt.Sprintf( + "The VM will be run headless, without a GUI. If you want to\n"+ + "view the screen of the VM, connect via VNC to vnc://%s\n"+ + "with the password: %s", vnc, vncPass) + } + + return fmt.Sprintf( + "The VM will be run headless, without a GUI. If you want to\n"+ + "view the screen of the VM, connect via VNC without a password to\n"+ + "vnc://%s", vnc) + } + return "" +} + +func (s *stepRun) getDeviceAndDriveArgs(config *Config, state multistep.StateBag) ([]string, []string) { + var deviceArgs []string + var driveArgs []string + + vmName := config.VMName + imgPath := filepath.Join(config.OutputDir, vmName) + + // Configure virtual hard drives + if s.atLeastVersion2 { + // We have different things to attach based on whether we are booting + // from an iso or a boot image. + drivesToAttach := []string{} + if config.DiskImage { + drivesToAttach = append(drivesToAttach, imgPath) + } + + diskFullPaths := state.Get("qemu_disk_paths").([]string) + drivesToAttach = append(drivesToAttach, diskFullPaths...) + + for i, drivePath := range drivesToAttach { + driveArgumentString := fmt.Sprintf("file=%s,if=%s,cache=%s,discard=%s,format=%s", drivePath, config.DiskInterface, config.DiskCache, config.DiskDiscard, config.Format) + if config.DiskInterface == "virtio-scsi" { + // TODO: Megan: Remove this conditional. This, and the code + // under the TODO below, reproduce the old behavior. While it + // may be broken, the goal of this commit is to refactor in a way + // that creates a result that is testably the same as the old + // code. A pr will follow fixing this broken behavior. + if i == 0 { + deviceArgs = append(deviceArgs, fmt.Sprintf("virtio-scsi-pci,id=scsi%d", i)) + } + // TODO: Megan: When you remove above conditional, + // set deviceArgs = append(deviceArgs, fmt.Sprintf("scsi-hd,bus=scsi%d.0,drive=drive%d", i, i)) + deviceArgs = append(deviceArgs, fmt.Sprintf("scsi-hd,bus=scsi0.0,drive=drive%d", i)) + driveArgumentString = fmt.Sprintf("if=none,file=%s,id=drive%d,cache=%s,discard=%s,format=%s", drivePath, i, config.DiskCache, config.DiskDiscard, config.Format) + } + if config.DetectZeroes != "off" { + driveArgumentString = fmt.Sprintf("%s,detect-zeroes=%s", driveArgumentString, config.DetectZeroes) + } + driveArgs = append(driveArgs, driveArgumentString) + } + } else { + driveArgs = append(driveArgs, fmt.Sprintf("file=%s,if=%s,cache=%s,format=%s", imgPath, config.DiskInterface, config.DiskCache, config.Format)) + } + + deviceArgs = append(deviceArgs, fmt.Sprintf("%s,netdev=user.0", config.NetDevice)) + + // Configure virtual CDs cdPaths := []string{} // Add the installation CD to the run command if !config.DiskImage { + isoPath := state.Get("iso_path").(string) cdPaths = append(cdPaths, isoPath) } // Add our custom CD created from cd_files, if it exists @@ -220,64 +271,48 @@ func getCommandArgs(bootDrive string, state multistep.StateBag) ([]string, error } } - defaultArgs["-device"] = deviceArgs - defaultArgs["-drive"] = driveArgs + return deviceArgs, driveArgs +} - defaultArgs["-boot"] = bootDrive - defaultArgs["-m"] = fmt.Sprintf("%dM", config.MemorySize) - if config.CpuCount > 1 { - defaultArgs["-smp"] = fmt.Sprintf("cpus=%d,sockets=%d", config.CpuCount, config.CpuCount) - } - defaultArgs["-vnc"] = vnc - - // Append the accelerator to the machine type if it is specified - if config.Accelerator != "none" { - defaultArgs["-machine"] = fmt.Sprintf("%s,accel=%s", defaultArgs["-machine"], config.Accelerator) - } else { - ui.Message("WARNING: The VM will be started with no hardware acceleration.\n" + - "The installation may take considerably longer to finish.\n") - } - - // Determine if we have a floppy disk to attach - if floppyPathRaw, ok := state.GetOk("floppy_path"); ok { - defaultArgs["-fda"] = floppyPathRaw.(string) - } else { - log.Println("Qemu Builder has no floppy files, not attaching a floppy.") - } +func (s *stepRun) applyUserOverrides(defaultArgs map[string]interface{}, config *Config, state multistep.StateBag) ([]string, error) { + // Done setting up defaults; time to process user args and defaults together + // and generate output args inArgs := make(map[string][]string) if len(config.QemuArgs) > 0 { - ui.Say("Overriding default Qemu arguments with QemuArgs...") + s.ui.Say("Overriding default Qemu arguments with qemuargs template option...") + commHostPort := state.Get("commHostPort").(int) httpIp := state.Get("http_ip").(string) httpPort := state.Get("http_port").(int) - ictx := config.ctx - if config.CommConfig.Comm.Type != "none" { - ictx.Data = qemuArgsTemplateData{ - httpIp, - httpPort, - config.HTTPDir, - config.OutputDir, - config.VMName, - commHostPort, - } - } else { - ictx.Data = qemuArgsTemplateData{ - HTTPIP: httpIp, - HTTPPort: httpPort, - HTTPDir: config.HTTPDir, - OutputDir: config.OutputDir, - Name: config.VMName, - } + + type qemuArgsTemplateData struct { + HTTPIP string + HTTPPort int + HTTPDir string + OutputDir string + Name string + SSHHostPort int } + + ictx := config.ctx + ictx.Data = qemuArgsTemplateData{ + HTTPIP: httpIp, + HTTPPort: httpPort, + HTTPDir: config.HTTPDir, + OutputDir: config.OutputDir, + Name: config.VMName, + SSHHostPort: commHostPort, + } + + // Interpolate each string in qemuargs newQemuArgs, err := processArgs(config.QemuArgs, &ictx) if err != nil { return nil, err } - // because qemu supports multiple appearances of the same - // switch, just different values, each key in the args hash - // will have an array of string values + // Qemu supports multiple appearances of the same switch. This means + // each key in the args hash will have an array of string values for _, qemuArgs := range newQemuArgs { key := qemuArgs[0] val := strings.Join(qemuArgs[1:], "") @@ -326,6 +361,12 @@ func getCommandArgs(bootDrive string, state multistep.StateBag) ([]string, error return outArgs, nil } +func (s *stepRun) getCommandArgs(config *Config, state multistep.StateBag) ([]string, error) { + defaultArgs := s.getDefaultArgs(config, state) + + return s.applyUserOverrides(defaultArgs, config, state) +} + func processArgs(args [][]string, ctx *interpolate.Context) ([][]string, error) { var err error diff --git a/builder/qemu/step_run_test.go b/builder/qemu/step_run_test.go index 1facfce12..0fe3687dd 100644 --- a/builder/qemu/step_run_test.go +++ b/builder/qemu/step_run_test.go @@ -4,6 +4,8 @@ import ( "fmt" "testing" + "github.com/hashicorp/packer/common" + "github.com/hashicorp/packer/helper/communicator" "github.com/hashicorp/packer/helper/multistep" "github.com/hashicorp/packer/packer" "github.com/stretchr/testify/assert" @@ -11,8 +13,6 @@ import ( func runTestState(t *testing.T, config *Config) multistep.StateBag { state := new(multistep.BasicStateBag) - - state.Put("ui", packer.TestUi(t)) state.Put("config", config) d := new(DriverMock) @@ -31,86 +31,396 @@ func runTestState(t *testing.T, config *Config) multistep.StateBag { return state } -func Test_getCommandArgs(t *testing.T) { - state := runTestState(t, &Config{}) - - args, err := getCommandArgs("", state) - if err != nil { - t.Fatalf("should not have an error getting args. Error: %s", err) +func Test_UserOverrides(t *testing.T) { + type testCase struct { + Config *Config + Expected []string + Reason string + } + testcases := []testCase{ + { + &Config{ + HTTPConfig: common.HTTPConfig{ + HTTPDir: "http/directory", + }, + OutputDir: "output/directory", + VMName: "myvm", + QemuArgs: [][]string{ + {"-randomflag1", "{{.HTTPIP}}-{{.HTTPPort}}-{{.HTTPDir}}"}, + {"-randomflag2", "{{.OutputDir}}-{{.Name}}"}, + }, + }, + []string{ + "-display", "gtk", + "-drive", "file=/path/to/test.iso,index=0,media=cdrom", + "-randomflag1", "127.0.0.1-1234-http/directory", + "-randomflag2", "output/directory-myvm", + "-device", ",netdev=user.0", + }, + "Test that interpolation overrides work.", + }, + { + &Config{ + VMName: "myvm", + QemuArgs: [][]string{{"-display", "partydisplay"}}, + }, + []string{ + "-display", "partydisplay", + "-drive", "file=/path/to/test.iso,index=0,media=cdrom", + "-device", ",netdev=user.0", + }, + "User input overrides default, rest is populated as normal", + }, + { + &Config{ + VMName: "myvm", + NetDevice: "mynetdevice", + QemuArgs: [][]string{{"-device", "somerandomdevice"}}, + }, + []string{ + "-display", "gtk", + "-device", "somerandomdevice", + "-device", "mynetdevice,netdev=user.0", + "-drive", "file=/path/to/test.iso,index=0,media=cdrom", + }, + "Net device gets added", + }, } - expected := []string{ - "-display", "gtk", - "-m", "0M", - "-boot", "", - "-fda", "fake_floppy_path", - "-name", "", - "-netdev", "user,id=user.0,hostfwd=tcp::5000-:0", - "-vnc", ":5905", - "-machine", "type=,accel=", - "-device", ",netdev=user.0", - "-drive", "file=/path/to/test.iso,index=0,media=cdrom", + for _, tc := range testcases { + state := runTestState(t, tc.Config) + + step := &stepRun{ + atLeastVersion2: true, + ui: packer.TestUi(t), + } + args, err := step.getCommandArgs(tc.Config, state) + if err != nil { + t.Fatalf("should not have an error getting args. Error: %s", err) + } + + expected := append([]string{ + "-m", "0M", + "-boot", "once=d", + "-fda", "fake_floppy_path", + "-name", "myvm", + "-netdev", "user,id=user.0,hostfwd=tcp::5000-:0", + "-vnc", ":5905", + "-machine", "type=,accel="}, + tc.Expected...) + + assert.ElementsMatch(t, args, expected, + fmt.Sprintf("%s, \nRecieved: %#v", tc.Reason, args)) } - assert.ElementsMatch(t, args, expected, "unexpected generated args") } -func Test_CDFilesPath(t *testing.T) { - // cd_path is set and DiskImage is false - state := runTestState(t, &Config{}) - state.Put("cd_path", "fake_cd_path.iso") - - args, err := getCommandArgs("", state) - if err != nil { - t.Fatalf("should not have an error getting args. Error: %s", err) +func Test_DriveAndDeviceArgs(t *testing.T) { + type testCase struct { + Config *Config + ExtraState map[string]interface{} + Step *stepRun + Expected []string + Reason string } - expected := []string{ - "-display", "gtk", - "-m", "0M", - "-boot", "", - "-fda", "fake_floppy_path", - "-name", "", - "-netdev", "user,id=user.0,hostfwd=tcp::5000-:0", - "-vnc", ":5905", - "-machine", "type=,accel=", - "-device", ",netdev=user.0", - "-drive", "file=/path/to/test.iso,index=0,media=cdrom", - "-drive", "file=fake_cd_path.iso,index=1,media=cdrom", + testcases := []testCase{ + { + &Config{}, + map[string]interface{}{}, + &stepRun{ + atLeastVersion2: true, + ui: packer.TestUi(t), + }, + []string{ + "-display", "gtk", + "-boot", "once=d", + "-drive", "file=/path/to/test.iso,index=0,media=cdrom", + }, + "Boot value should default to once=d when diskImage isn't set", + }, + { + &Config{ + DiskImage: true, + DiskInterface: "virtio-scsi", + + OutputDir: "/path/to/output", + DiskCache: "writeback", + Format: "qcow2", + DetectZeroes: "off", + }, + map[string]interface{}{ + "cd_path": "fake_cd_path.iso", + }, + &stepRun{ + DiskImage: true, + atLeastVersion2: true, + ui: packer.TestUi(t), + }, + []string{ + "-display", "gtk", + "-boot", "c", + "-device", "virtio-scsi-pci,id=scsi0", + "-device", "scsi-hd,bus=scsi0.0,drive=drive0", + "-drive", "if=none,file=/path/to/output,id=drive0,cache=writeback,discard=,format=qcow2", + "-drive", "file=fake_cd_path.iso,index=0,media=cdrom", + }, + "virtio-scsi interface, DiskImage true, extra cdrom, detectZeroes off", + }, + { + &Config{ + DiskImage: true, + DiskInterface: "virtio-scsi", + + OutputDir: "/path/to/output", + DiskCache: "writeback", + Format: "qcow2", + DetectZeroes: "on", + }, + map[string]interface{}{ + "cd_path": "fake_cd_path.iso", + }, + &stepRun{ + DiskImage: true, + atLeastVersion2: true, + ui: packer.TestUi(t), + }, + []string{ + "-display", "gtk", + "-boot", "c", + "-device", "virtio-scsi-pci,id=scsi0", + "-device", "scsi-hd,bus=scsi0.0,drive=drive0", + "-drive", "if=none,file=/path/to/output,id=drive0,cache=writeback,discard=,format=qcow2,detect-zeroes=on", + "-drive", "file=fake_cd_path.iso,index=0,media=cdrom", + }, + "virtio-scsi interface, DiskImage true, extra cdrom, detectZeroes on", + }, + { + &Config{ + DiskInterface: "virtio-scsi", + + OutputDir: "/path/to/output", + DiskCache: "writeback", + Format: "qcow2", + DetectZeroes: "off", + }, + map[string]interface{}{ + "cd_path": "fake_cd_path.iso", + // when disk image is false, we will always have at least one + // disk path: the one we create to be the main disk. + "qemu_disk_paths": []string{"qemupath1", "qemupath2"}, + }, + &stepRun{ + atLeastVersion2: true, + ui: packer.TestUi(t), + }, + []string{ + "-display", "gtk", + "-boot", "once=d", + "-device", "virtio-scsi-pci,id=scsi0", + "-device", "scsi-hd,bus=scsi0.0,drive=drive0", + "-device", "scsi-hd,bus=scsi0.0,drive=drive1", + "-drive", "if=none,file=qemupath1,id=drive0,cache=writeback,discard=,format=qcow2", + "-drive", "if=none,file=qemupath2,id=drive1,cache=writeback,discard=,format=qcow2", + "-drive", "file=/path/to/test.iso,index=0,media=cdrom", + "-drive", "file=fake_cd_path.iso,index=1,media=cdrom", + }, + "virtio-scsi interface, bootable iso, cdrom", + }, + { + &Config{ + DiskInterface: "virtio-scsi", + + OutputDir: "/path/to/output", + DiskCache: "writeback", + Format: "qcow2", + DetectZeroes: "on", + }, + map[string]interface{}{ + "cd_path": "fake_cd_path.iso", + // when disk image is false, we will always have at least one + // disk path: the one we create to be the main disk. + "qemu_disk_paths": []string{"qemupath1", "qemupath2"}, + }, + &stepRun{ + atLeastVersion2: true, + ui: packer.TestUi(t), + }, + []string{ + "-display", "gtk", + "-boot", "once=d", + "-device", "virtio-scsi-pci,id=scsi0", + "-device", "scsi-hd,bus=scsi0.0,drive=drive0", + "-device", "scsi-hd,bus=scsi0.0,drive=drive1", + "-drive", "if=none,file=qemupath1,id=drive0,cache=writeback,discard=,format=qcow2,detect-zeroes=on", + "-drive", "if=none,file=qemupath2,id=drive1,cache=writeback,discard=,format=qcow2,detect-zeroes=on", + "-drive", "file=/path/to/test.iso,index=0,media=cdrom", + "-drive", "file=fake_cd_path.iso,index=1,media=cdrom", + }, + "virtio-scsi interface, DiskImage false, extra cdrom, detect zeroes on", + }, + { + &Config{ + DiskInterface: "virtio-scsi", + + OutputDir: "/path/to/output", + DiskCache: "writeback", + Format: "qcow2", + }, + map[string]interface{}{ + // when disk image is false, we will always have at least one + // disk path: the one we create to be the main disk. + "qemu_disk_paths": []string{"output/dir/path/mydisk.qcow2"}, + }, + &stepRun{ + atLeastVersion2: true, + ui: packer.TestUi(t), + }, + []string{ + "-display", "gtk", + "-boot", "once=d", + "-device", "virtio-scsi-pci,id=scsi0", + "-device", "scsi-hd,bus=scsi0.0,drive=drive0", + "-drive", "if=none,file=output/dir/path/mydisk.qcow2,id=drive0,cache=writeback,discard=,format=qcow2,detect-zeroes=", + "-drive", "file=/path/to/test.iso,index=0,media=cdrom", + }, + "virtio-scsi interface, DiskImage false, no extra disks or cds", + }, + { + &Config{}, + map[string]interface{}{ + "cd_path": "fake_cd_path.iso", + "qemu_disk_paths": []string{"output/dir/path/mydisk.qcow2"}, + }, + &stepRun{ + atLeastVersion2: true, + ui: packer.TestUi(t), + }, + []string{ + "-display", "gtk", + "-boot", "once=d", + "-drive", "file=output/dir/path/mydisk.qcow2,if=,cache=,discard=,format=,detect-zeroes=", + "-drive", "file=/path/to/test.iso,index=0,media=cdrom", + "-drive", "file=fake_cd_path.iso,index=1,media=cdrom", + }, + "cd_path is set and DiskImage is false", + }, + { + &Config{}, + map[string]interface{}{ + // when disk image is false, we will always have at least one + // disk path: the one we create to be the main disk. + "qemu_disk_paths": []string{"output/dir/path/mydisk.qcow2"}, + }, + &stepRun{ + atLeastVersion2: true, + ui: packer.TestUi(t), + }, + []string{ + "-display", "gtk", + "-boot", "once=d", + "-drive", "file=output/dir/path/mydisk.qcow2,if=,cache=,discard=,format=,detect-zeroes=", + "-drive", "file=/path/to/test.iso,index=0,media=cdrom", + }, + "empty config", + }, + { + &Config{ + OutputDir: "/path/to/output", + DiskInterface: "virtio", + DiskCache: "writeback", + Format: "qcow2", + }, + map[string]interface{}{ + // when disk image is false, we will always have at least one + // disk path: the one we create to be the main disk. + "qemu_disk_paths": []string{"output/dir/path/mydisk.qcow2"}, + }, + &stepRun{ + atLeastVersion2: false, + ui: packer.TestUi(t), + }, + []string{ + "-boot", "once=d", + "-drive", "file=/path/to/output,if=virtio,cache=writeback,format=qcow2", + "-drive", "file=/path/to/test.iso,index=0,media=cdrom", + }, + "version less than 2", + }, + { + &Config{ + OutputDir: "/path/to/output", + DiskInterface: "virtio", + DiskCache: "writeback", + Format: "qcow2", + }, + map[string]interface{}{ + "cd_path": "fake_cd_path.iso", + "qemu_disk_paths": []string{"qemupath1", "qemupath2"}, + }, + &stepRun{ + atLeastVersion2: true, + ui: packer.TestUi(t), + }, + []string{ + "-display", "gtk", + "-boot", "once=d", + "-drive", "file=qemupath1,if=virtio,cache=writeback,discard=,format=qcow2,detect-zeroes=", + "-drive", "file=qemupath2,if=virtio,cache=writeback,discard=,format=qcow2,detect-zeroes=", + "-drive", "file=fake_cd_path.iso,index=1,media=cdrom", + "-drive", "file=/path/to/test.iso,index=0,media=cdrom", + }, + "virtio interface with extra disks", + }, + { + &Config{ + DiskImage: true, + OutputDir: "/path/to/output", + DiskInterface: "virtio", + DiskCache: "writeback", + Format: "qcow2", + }, + map[string]interface{}{ + "cd_path": "fake_cd_path.iso", + }, + &stepRun{ + DiskImage: true, + atLeastVersion2: true, + ui: packer.TestUi(t), + }, + []string{ + "-display", "gtk", + "-boot", "c", + "-drive", "file=/path/to/output,if=virtio,cache=writeback,discard=,format=qcow2,detect-zeroes=", + "-drive", "file=fake_cd_path.iso,index=0,media=cdrom", + }, + "virtio interface with disk image", + }, } + for _, tc := range testcases { + state := runTestState(t, &Config{}) + for k, v := range tc.ExtraState { + state.Put(k, v) + } - assert.ElementsMatch(t, args, expected, fmt.Sprintf("unexpected generated args: %#v", args)) + args, err := tc.Step.getCommandArgs(tc.Config, state) + if err != nil { + t.Fatalf("should not have an error getting args. Error: %s", err) + } - // cd_path is set and DiskImage is true - config := &Config{ - DiskImage: true, - DiskInterface: "virtio-scsi", + expected := append([]string{ + "-m", "0M", + "-fda", "fake_floppy_path", + "-name", "", + "-netdev", "user,id=user.0,hostfwd=tcp::5000-:0", + "-vnc", ":5905", + "-machine", "type=,accel=", + "-device", ",netdev=user.0"}, + tc.Expected...) + + assert.ElementsMatch(t, args, expected, + fmt.Sprintf("%s, \nRecieved: %#v", tc.Reason, args)) } - state = runTestState(t, config) - state.Put("cd_path", "fake_cd_path.iso") - - args, err = getCommandArgs("c", state) - if err != nil { - t.Fatalf("should not have an error getting args. Error: %s", err) - } - - expected = []string{ - "-display", "gtk", - "-m", "0M", - "-boot", "c", - "-fda", "fake_floppy_path", - "-name", "", - "-netdev", "user,id=user.0,hostfwd=tcp::5000-:0", - "-vnc", ":5905", - "-machine", "type=,accel=", - "-device", ",netdev=user.0", - "-device", "virtio-scsi-pci,id=scsi0", - "-device", "scsi-hd,bus=scsi0.0,drive=drive0", - "-drive", "if=none,file=,id=drive0,cache=,discard=,format=,detect-zeroes=", - "-drive", "file=fake_cd_path.iso,index=0,media=cdrom", - } - - assert.ElementsMatch(t, args, expected, fmt.Sprintf("unexpected generated args: %#v", args)) } func Test_OptionalConfigOptionsGetSet(t *testing.T) { @@ -124,8 +434,11 @@ func Test_OptionalConfigOptionsGetSet(t *testing.T) { } state := runTestState(t, c) - - args, err := getCommandArgs("once=d", state) + step := &stepRun{ + atLeastVersion2: true, + ui: packer.TestUi(t), + } + args, err := step.getCommandArgs(c, state) if err != nil { t.Fatalf("should not have an error getting args. Error: %s", err) } @@ -144,5 +457,251 @@ func Test_OptionalConfigOptionsGetSet(t *testing.T) { "-qmp", "unix:qmp_path,server,nowait", } - assert.ElementsMatch(t, args, expected, "password flag should be set, and d drive should be set.") + assert.ElementsMatch(t, args, expected, "password flag should be set, and d drive should be set: %s", args) +} + +// Tests for presence of Packer-generated arguments. Doesn't test that +// arguments which shouldn't be there are absent. +func Test_Defaults(t *testing.T) { + type testCase struct { + Config *Config + ExtraState map[string]interface{} + Step *stepRun + Expected []string + Reason string + } + + testcases := []testCase{ + { + &Config{}, + map[string]interface{}{}, + &stepRun{ui: packer.TestUi(t)}, + []string{"-boot", "once=d"}, + "Boot value should default to once=d", + }, + { + &Config{}, + map[string]interface{}{}, + &stepRun{ + DiskImage: true, + ui: packer.TestUi(t), + }, + []string{"-boot", "c"}, + "Boot value should be set to c when DiskImage is set on step", + }, + { + &Config{ + QMPEnable: true, + QMPSocketPath: "/path/to/socket", + }, + map[string]interface{}{}, + &stepRun{ui: packer.TestUi(t)}, + []string{"-qmp", "unix:/path/to/socket,server,nowait"}, + "Args should contain -qmp when qmp_enable is set", + }, + { + &Config{ + QMPEnable: true, + }, + map[string]interface{}{}, + &stepRun{ui: packer.TestUi(t)}, + []string{"-qmp", "unix:,server,nowait"}, + "Args contain -qmp even when socket path isn't set, if qmp enabled", + }, + { + &Config{ + VMName: "partyname", + }, + map[string]interface{}{}, + &stepRun{ui: packer.TestUi(t)}, + []string{"-name", "partyname"}, + "Name is set from config", + }, + { + &Config{}, + map[string]interface{}{}, + &stepRun{ui: packer.TestUi(t)}, + []string{"-name", ""}, + "Name is set from config, even when name is blank (which won't " + + "happen for real thanks to defaulting in build prepare)", + }, + { + &Config{ + Accelerator: "none", + MachineType: "fancymachine", + }, + map[string]interface{}{}, + &stepRun{ui: packer.TestUi(t)}, + []string{"-machine", "type=fancymachine"}, + "Don't add accelerator tag when no accelerator is set.", + }, + { + &Config{ + Accelerator: "kvm", + MachineType: "fancymachine", + }, + map[string]interface{}{}, + &stepRun{ui: packer.TestUi(t)}, + []string{"-machine", "type=fancymachine,accel=kvm"}, + "Add accelerator tag when accelerator is set.", + }, + { + &Config{ + NetBridge: "fakebridge", + }, + map[string]interface{}{}, + &stepRun{ui: packer.TestUi(t)}, + []string{"-netdev", "bridge,id=user.0,br=fakebridge"}, + "Add netbridge tag when netbridge is set.", + }, + { + &Config{ + CommConfig: CommConfig{ + Comm: communicator.Config{ + Type: "none", + }, + }, + }, + map[string]interface{}{}, + &stepRun{ui: packer.TestUi(t)}, + []string{"-netdev", "user,id=user.0"}, + "No host forwarding when no net bridge and no communicator", + }, + { + &Config{ + CommConfig: CommConfig{ + Comm: communicator.Config{ + Type: "ssh", + SSH: communicator.SSH{ + SSHPort: 4567, + }, + }, + }, + }, + map[string]interface{}{ + "commHostPort": 1111, + }, + &stepRun{ui: packer.TestUi(t)}, + []string{"-netdev", "user,id=user.0,hostfwd=tcp::1111-:4567"}, + "Host forwarding when a communicator is configured", + }, + { + &Config{ + VNCBindAddress: "1.1.1.1", + }, + map[string]interface{}{ + "vnc_port": 5959, + }, + &stepRun{ui: packer.TestUi(t)}, + []string{"-vnc", "1.1.1.1:5959"}, + "no VNC password should be set", + }, + { + &Config{ + VNCBindAddress: "1.1.1.1", + VNCUsePassword: true, + }, + map[string]interface{}{ + "vnc_port": 5959, + }, + &stepRun{ui: packer.TestUi(t)}, + []string{"-vnc", "1.1.1.1:5959,password"}, + "VNC password should be set", + }, + { + &Config{ + MemorySize: 2345, + }, + map[string]interface{}{}, + &stepRun{ui: packer.TestUi(t)}, + []string{"-m", "2345M"}, + "Memory is set, with unit M", + }, + { + &Config{ + CpuCount: 2, + }, + map[string]interface{}{}, + &stepRun{ui: packer.TestUi(t)}, + []string{"-smp", "cpus=2,sockets=2"}, + "both cpus and sockets are set to config's CpuCount", + }, + { + &Config{ + CpuCount: 2, + }, + map[string]interface{}{}, + &stepRun{ui: packer.TestUi(t)}, + []string{"-smp", "cpus=2,sockets=2"}, + "both cpus and sockets are set to config's CpuCount", + }, + { + &Config{ + CpuCount: 2, + }, + map[string]interface{}{ + "floppy_path": "/path/to/floppy", + }, + &stepRun{ui: packer.TestUi(t)}, + []string{"-fda", "/path/to/floppy"}, + "floppy path should be set under fda flag, when it exists", + }, + { + &Config{ + Headless: false, + Display: "fakedisplay", + UseDefaultDisplay: false, + }, + map[string]interface{}{}, + &stepRun{ + atLeastVersion2: true, + ui: packer.TestUi(t), + }, + []string{"-display", "fakedisplay"}, + "Display option should value config display", + }, + { + &Config{ + Headless: false, + }, + map[string]interface{}{}, + &stepRun{ + atLeastVersion2: true, + ui: packer.TestUi(t), + }, + []string{"-display", "gtk"}, + "Display option should default to gtk", + }, + } + + for _, tc := range testcases { + state := runTestState(t, &Config{}) + for k, v := range tc.ExtraState { + state.Put(k, v) + } + + args, err := tc.Step.getCommandArgs(tc.Config, state) + if err != nil { + t.Fatalf("should not have an error getting args. Error: %s", err) + } + if !matchArgument(args, tc.Expected) { + t.Fatalf("Couldn't find %#v in result. Got: %#v, Reason: %s", + tc.Expected, args, tc.Reason) + } + } +} + +// This test makes sure that arguments don't end up in the final boot command +// if they aren't configured in the config. +// func TestDefaultsAbsentValues(t *testing.T) {} +func matchArgument(actual []string, expected []string) bool { + key := expected[0] + for i, k := range actual { + if key == k { + if expected[1] == actual[i+1] { + return true + } + } + } + return false }