Skip to content

Commit a1d970c

Browse files
mpywellmpywell
committed
common: skip_convert_to_template updates
- updated VM stop/start logic for hardware changes to a VM build - reverted getExistingTemplate signature changes
1 parent f2a0326 commit a1d970c

File tree

4 files changed

+46
-29
lines changed

4 files changed

+46
-29
lines changed

‎builder/proxmox/common/step_convert_to_template.go‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -32,19 +32,19 @@ func (s *stepConvertToTemplate) Run(ctx context.Context, state multistep.StateBa
3232
vmRef := state.Get("vmRef").(*proxmox.VmRef)
3333
c := state.Get("config").(*Config)
3434

35-
ui.Say("Stopping VM")
36-
_, err := client.ShutdownVm(vmRef)
37-
if err != nil {
38-
err := fmt.Errorf("Error converting VM to template, could not stop: %s", err)
39-
state.Put("error", err)
40-
ui.Error(err.Error())
41-
return multistep.ActionHalt
42-
}
43-
4435
if c.SkipConvertToTemplate {
4536
ui.Say("skip_convert_to_template set, skipping conversion to template")
4637
state.Put("artifact_type", "VM")
4738
} else {
39+
ui.Say("Stopping VM")
40+
_, err := client.ShutdownVm(vmRef)
41+
if err != nil {
42+
err := fmt.Errorf("Error converting VM to template, could not stop: %s", err)
43+
state.Put("error", err)
44+
ui.Error(err.Error())
45+
return multistep.ActionHalt
46+
}
47+
4848
ui.Say("Converting VM to template")
4949
err = client.CreateTemplate(vmRef)
5050
if err != nil {

‎builder/proxmox/common/step_finalize_config.go‎

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ type finalizer interface {
2323
GetVmConfig(*proxmox.VmRef) (map[string]interface{}, error)
2424
SetVmConfig(*proxmox.VmRef, map[string]interface{}) (interface{}, error)
2525
StartVm(*proxmox.VmRef) (string, error)
26+
ShutdownVm(*proxmox.VmRef) (string, error)
2627
}
2728

2829
var _ finalizer = &proxmox.Client{}
@@ -46,7 +47,7 @@ func (s *stepFinalizeConfig) Run(ctx context.Context, state multistep.StateBag)
4647

4748
vmParams, err := client.GetVmConfig(vmRef)
4849
if err != nil {
49-
err := fmt.Errorf("error fetching template config: %s", err)
50+
err := fmt.Errorf("error fetching config: %s", err)
5051
state.Put("error", err)
5152
ui.Error(err.Error())
5253
return multistep.ActionHalt
@@ -140,6 +141,17 @@ func (s *stepFinalizeConfig) Run(ctx context.Context, state multistep.StateBag)
140141
changes["delete"] = strings.Join(deleteItems, ",")
141142

142143
if len(changes) > 0 {
144+
// Adding a Cloud-Init drive or removing CD-ROM devices won't take effect without a power off and on of the QEMU VM
145+
if c.SkipConvertToTemplate {
146+
ui.Say("Hardware changes pending for VM, stopping VM")
147+
_, err := client.ShutdownVm(vmRef)
148+
if err != nil {
149+
err := fmt.Errorf("Error converting VM to template, could not stop: %s", err)
150+
state.Put("error", err)
151+
ui.Error(err.Error())
152+
return multistep.ActionHalt
153+
}
154+
}
143155
_, err := client.SetVmConfig(vmRef, changes)
144156
if err != nil {
145157
err := fmt.Errorf("Error updating template: %s", err)
@@ -151,7 +163,7 @@ func (s *stepFinalizeConfig) Run(ctx context.Context, state multistep.StateBag)
151163

152164
// When build artifact is to be a VM, return a running VM
153165
if c.SkipConvertToTemplate {
154-
ui.Say("skip_convert_to_template set, resuming VM")
166+
ui.Say("Resuming VM")
155167
_, err := client.StartVm(vmRef)
156168
if err != nil {
157169
err := fmt.Errorf("Error starting VM: %s", err)

‎builder/proxmox/common/step_finalize_config_test.go‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,9 +16,10 @@ import (
1616
)
1717

1818
type finalizerMock struct {
19-
getConfig func() (map[string]interface{}, error)
20-
setConfig func(map[string]interface{}) (string, error)
21-
startVm func() (string, error)
19+
getConfig func() (map[string]interface{}, error)
20+
setConfig func(map[string]interface{}) (string, error)
21+
startVm func() (string, error)
22+
shutdownVm func() (string, error)
2223
}
2324

2425
func (m finalizerMock) GetVmConfig(*proxmox.VmRef) (map[string]interface{}, error) {
@@ -32,6 +33,10 @@ func (m finalizerMock) StartVm(*proxmox.VmRef) (string, error) {
3233
return m.startVm()
3334
}
3435

36+
func (m finalizerMock) ShutdownVm(*proxmox.VmRef) (string, error) {
37+
return m.shutdownVm()
38+
}
39+
3540
var _ finalizer = finalizerMock{}
3641

3742
func TestTemplateFinalize(t *testing.T) {

‎builder/proxmox/common/step_start_vm.go‎

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ var (
4747

4848
// Check if the given builder configuration maps to an existing VM template on the Proxmox cluster.
4949
// Returns an empty *proxmox.VmRef when no matching ID or name is found.
50-
func getExistingTemplate(c *Config, client vmStarter) (*proxmox.VmRef, string, error) {
50+
func getExistingTemplate(c *Config, client vmStarter) (*proxmox.VmRef, error) {
5151
vmRef := &proxmox.VmRef{}
5252
if c.VMID > 0 {
5353
log.Printf("looking up VM with ID %d", c.VMID)
@@ -59,9 +59,9 @@ func getExistingTemplate(c *Config, client vmStarter) (*proxmox.VmRef, string, e
5959
notFoundError := fmt.Sprintf("vm '%d' not found", c.VMID)
6060
if err.Error() == notFoundError {
6161
log.Println(err.Error())
62-
return &proxmox.VmRef{}, "", nil
62+
return &proxmox.VmRef{}, nil
6363
}
64-
return &proxmox.VmRef{}, "", err
64+
return &proxmox.VmRef{}, err
6565
}
6666
log.Printf("found VM with ID %d", vmRef.VmId())
6767
} else {
@@ -73,33 +73,33 @@ func getExistingTemplate(c *Config, client vmStarter) (*proxmox.VmRef, string, e
7373
notFoundError := fmt.Sprintf("vm '%s' not found", c.TemplateName)
7474
if err.Error() == notFoundError {
7575
log.Println(err.Error())
76-
return &proxmox.VmRef{}, "", nil
76+
return &proxmox.VmRef{}, nil
7777
}
78-
return &proxmox.VmRef{}, "", err
78+
return &proxmox.VmRef{}, err
7979
}
8080
if len(vmRefs) > 1 {
8181
vmIDs := []int{}
8282
for _, vmr := range vmRefs {
8383
vmIDs = append(vmIDs, vmr.VmId())
8484
}
85-
return &proxmox.VmRef{}, "", fmt.Errorf("found multiple VMs with name '%s', IDs: %v", c.TemplateName, vmIDs)
85+
return &proxmox.VmRef{}, fmt.Errorf("found multiple VMs with name '%s', IDs: %v", c.TemplateName, vmIDs)
8686
}
8787
vmRef = vmRefs[0]
8888
log.Printf("found VM with name '%s' (ID: %d)", c.TemplateName, vmRef.VmId())
8989
}
9090
if c.SkipConvertToTemplate {
91-
return vmRef, "VM", nil
91+
return vmRef, nil
9292
}
9393
log.Printf("check if VM %d is a template", vmRef.VmId())
9494
vmConfig, err := client.GetVmConfig(vmRef)
9595
if err != nil {
96-
return &proxmox.VmRef{}, "", err
96+
return &proxmox.VmRef{}, err
9797
}
9898
log.Printf("VM %d template: %d", vmRef.VmId(), vmConfig["template"])
9999
if vmConfig["template"] == nil {
100-
return &proxmox.VmRef{}, "", fmt.Errorf("found matching VM (ID: %d, name: %s), but it is not a template", vmRef.VmId(), vmConfig["name"])
100+
return &proxmox.VmRef{}, fmt.Errorf("found matching VM (ID: %d, name: %s), but it is not a template", vmRef.VmId(), vmConfig["name"])
101101
}
102-
return vmRef, "VM template", nil
102+
return vmRef, nil
103103
}
104104

105105
func (s *stepStartVM) Run(ctx context.Context, state multistep.StateBag) multistep.StepAction {
@@ -167,14 +167,14 @@ func (s *stepStartVM) Run(ctx context.Context, state multistep.StateBag) multist
167167

168168
if c.PackerForce {
169169
ui.Say("Force set, checking for existing artifact on PVE cluster")
170-
vmRef, vmType, err := getExistingTemplate(c, client)
170+
vmRef, err := getExistingTemplate(c, client)
171171
if err != nil {
172172
state.Put("error", err)
173173
ui.Error(err.Error())
174174
return multistep.ActionHalt
175175
}
176176
if vmRef.VmId() != 0 {
177-
ui.Say(fmt.Sprintf("found existing %s with ID %d on PVE node %s, deleting it", vmType, vmRef.VmId(), vmRef.Node()))
177+
ui.Say(fmt.Sprintf("found existing resource with ID %d on PVE node %s, deleting it", vmRef.VmId(), vmRef.Node()))
178178
// If building a VM artifact and c.PackerForce is true,
179179
// running VMs can't be deleted. Stop before deleting.
180180
vmState, err := client.GetVmState(vmRef)
@@ -188,17 +188,17 @@ func (s *stepStartVM) Run(ctx context.Context, state multistep.StateBag) multist
188188
_, err = client.StopVm(vmRef)
189189
if err != nil {
190190
state.Put("error", err)
191-
ui.Error(fmt.Sprintf("error stopping %s: %s", vmType, err.Error()))
191+
ui.Error(fmt.Sprintf("error stopping resource: %s", err.Error()))
192192
return multistep.ActionHalt
193193
}
194194
}
195195
_, err = client.DeleteVm(vmRef)
196196
if err != nil {
197197
state.Put("error", err)
198-
ui.Error(fmt.Sprintf("error deleting %s: %s", vmType, err.Error()))
198+
ui.Error(fmt.Sprintf("error deleting resource: %s", err.Error()))
199199
return multistep.ActionHalt
200200
}
201-
ui.Say(fmt.Sprintf("Successfully deleted %s %d", vmType, vmRef.VmId()))
201+
ui.Say(fmt.Sprintf("Successfully deleted %d", vmRef.VmId()))
202202
} else {
203203
ui.Say("No existing artifact found")
204204
}

0 commit comments

Comments
 (0)