diff --git a/builder/proxmox/step_finalize_template_config.go b/builder/proxmox/step_finalize_template_config.go index 2e442654a..c6de188ef 100644 --- a/builder/proxmox/step_finalize_template_config.go +++ b/builder/proxmox/step_finalize_template_config.go @@ -45,7 +45,7 @@ func (s *stepFinalizeTemplateConfig) Run(ctx context.Context, state multistep.St return multistep.ActionHalt } - if !strings.HasSuffix(vmParams["ide2"].(string), "media=cdrom") { + if vmParams["ide2"] == nil || !strings.HasSuffix(vmParams["ide2"].(string), "media=cdrom") { err := fmt.Errorf("Cannot eject ISO from cdrom drive, ide2 is not present, or not a cdrom media") state.Put("error", err) ui.Error(err.Error()) diff --git a/builder/proxmox/step_finalize_template_config_test.go b/builder/proxmox/step_finalize_template_config_test.go index 1f0a82541..58de6c966 100644 --- a/builder/proxmox/step_finalize_template_config_test.go +++ b/builder/proxmox/step_finalize_template_config_test.go @@ -2,6 +2,7 @@ package proxmox import ( "context" + "fmt" "testing" "github.com/Telmate/proxmox-api-go/proxmox" @@ -22,42 +23,127 @@ func (m finalizerMock) SetVmConfig(vmref *proxmox.VmRef, c map[string]interface{ } func TestTemplateFinalize(t *testing.T) { - finalizer := finalizerMock{ - getConfig: func() (map[string]interface{}, error) { - return map[string]interface{}{ + cs := []struct { + name string + builderConfig *Config + initialVMConfig map[string]interface{} + getConfigErr error + expectCallSetConfig bool + expectedVMConfig map[string]interface{} + setConfigErr error + expectedAction multistep.StepAction + }{ + { + name: "empty config changes only description", + builderConfig: &Config{}, + initialVMConfig: map[string]interface{}{ "name": "dummy", "description": "Packer ephemeral build VM", "ide2": "local:iso/Fedora-Server-dvd-x86_64-29-1.2.iso,media=cdrom", - }, nil + }, + expectCallSetConfig: true, + expectedVMConfig: map[string]interface{}{ + "name": nil, + "description": "", + "ide2": nil, + }, + expectedAction: multistep.ActionContinue, }, - setConfig: func(c map[string]interface{}) (string, error) { - if c["name"] != "my-template" { - t.Errorf("Expected name to be my-template, got %q", c["name"]) - } - if c["description"] != "foo" { - t.Errorf("Expected description to be foo, got %q", c["description"]) - } - if c["ide2"] != "none,media=cdrom" { - t.Errorf("Expected ide2 to be none,media=cdrom, got %q", c["ide2"]) - } - - return "", nil + { + name: "all options", + builderConfig: &Config{ + TemplateName: "my-template", + TemplateDescription: "some-description", + UnmountISO: true, + }, + initialVMConfig: map[string]interface{}{ + "name": "dummy", + "description": "Packer ephemeral build VM", + "ide2": "local:iso/Fedora-Server-dvd-x86_64-29-1.2.iso,media=cdrom", + }, + expectCallSetConfig: true, + expectedVMConfig: map[string]interface{}{ + "name": "my-template", + "description": "some-description", + "ide2": "none,media=cdrom", + }, + expectedAction: multistep.ActionContinue, + }, + { + name: "no cd-drive with unmount=true should returns halt", + builderConfig: &Config{ + TemplateName: "my-template", + TemplateDescription: "some-description", + UnmountISO: true, + }, + initialVMConfig: map[string]interface{}{ + "name": "dummy", + "description": "Packer ephemeral build VM", + "ide1": "local:iso/Fedora-Server-dvd-x86_64-29-1.2.iso,media=cdrom", + }, + expectCallSetConfig: false, + expectedAction: multistep.ActionHalt, + }, + { + name: "GetVmConfig error should return halt", + builderConfig: &Config{ + TemplateName: "my-template", + TemplateDescription: "some-description", + UnmountISO: true, + }, + getConfigErr: fmt.Errorf("some error"), + expectCallSetConfig: false, + expectedAction: multistep.ActionHalt, + }, + { + name: "SetVmConfig error should return halt", + builderConfig: &Config{ + TemplateName: "my-template", + TemplateDescription: "some-description", + UnmountISO: true, + }, + initialVMConfig: map[string]interface{}{ + "name": "dummy", + "description": "Packer ephemeral build VM", + "ide2": "local:iso/Fedora-Server-dvd-x86_64-29-1.2.iso,media=cdrom", + }, + expectCallSetConfig: true, + setConfigErr: fmt.Errorf("some error"), + expectedAction: multistep.ActionHalt, }, } - state := new(multistep.BasicStateBag) - state.Put("ui", packer.TestUi(t)) - state.Put("config", &Config{ - TemplateName: "my-template", - TemplateDescription: "foo", - UnmountISO: true, - }) - state.Put("vmRef", proxmox.NewVmRef(1)) - state.Put("proxmoxClient", finalizer) - - step := stepFinalizeTemplateConfig{} - action := step.Run(context.TODO(), state) - if action != multistep.ActionContinue { - t.Error("Expected action to be Continue, got Halt") + for _, c := range cs { + t.Run(c.name, func(t *testing.T) { + finalizer := finalizerMock{ + getConfig: func() (map[string]interface{}, error) { + return c.initialVMConfig, c.getConfigErr + }, + setConfig: func(cfg map[string]interface{}) (string, error) { + if !c.expectCallSetConfig { + t.Error("Did not expect SetVmConfig to be called") + } + for key, val := range c.expectedVMConfig { + if cfg[key] != val { + t.Errorf("Expected %q to be %q, got %q", key, val, cfg[key]) + } + } + + return "", c.setConfigErr + }, + } + + state := new(multistep.BasicStateBag) + state.Put("ui", packer.TestUi(t)) + state.Put("config", c.builderConfig) + state.Put("vmRef", proxmox.NewVmRef(1)) + state.Put("proxmoxClient", finalizer) + + step := stepFinalizeTemplateConfig{} + action := step.Run(context.TODO(), state) + if action != c.expectedAction { + t.Errorf("Expected action to be %v, got %v", c.expectedAction, action) + } + }) } }