Skip to content

Commit f2379ca

Browse files
author
Marshall Wu
committed
Fix the issue what iso builder does not clean up VMs after failure, and add logging for cleanup process.
1 parent bb42946 commit f2379ca

File tree

2 files changed

+196
-28
lines changed

2 files changed

+196
-28
lines changed

‎builder/xenserver/common/vm_cleanup.go‎

Lines changed: 180 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import (
66

77
"github.com/hashicorp/packer-plugin-sdk/multistep"
88
"github.com/hashicorp/packer-plugin-sdk/packer"
9-
9+
1010
"xenapi"
1111
)
1212

@@ -15,6 +15,7 @@ type VmCleanup struct{}
1515
func (self *VmCleanup) Cleanup(state multistep.StateBag) {
1616
config := state.Get("commonconfig").(CommonConfig)
1717
c := state.Get("client").(*Connection)
18+
ui := state.Get("ui").(packer.Ui)
1819

1920
if config.ShouldKeepVM(state) {
2021
return
@@ -27,44 +28,204 @@ func (self *VmCleanup) Cleanup(state multistep.StateBag) {
2728
return
2829
}
2930

30-
err = xenapi.VM.HardShutdown(c.session, instance)
31+
// Get all VBDs (Virtual Block Devices) attached to the VM
32+
vbds, err := xenapi.VM.GetVBDs(c.session, instance)
3133
if err != nil {
32-
log.Printf(fmt.Sprintf("Unable to force shutdown VM '%s': %s", uuid, err.Error()))
34+
log.Printf(fmt.Sprintf("Unable to get VBDs for VM '%s': %s", uuid, err.Error()))
35+
vbds = []xenapi.VBDRef{}
36+
}
37+
38+
// Collect all VDIs before destroying the VM
39+
var vdis []xenapi.VDIRef
40+
for _, vbd := range vbds {
41+
rec, err := xenapi.VBD.GetRecord(c.session, vbd)
42+
if err != nil {
43+
log.Printf(fmt.Sprintf("Unable to get VBD record: %s", err.Error()))
44+
continue
45+
}
46+
// Skip empty VBDs (like CD drives)
47+
if rec.VDI != "" {
48+
vdis = append(vdis, rec.VDI)
49+
}
3350
}
51+
52+
ui.Say(fmt.Sprintf("Found %d disk(s) attached to VM", len(vdis)))
53+
54+
// Shutdown the VM if it's running
55+
vmstate, err := xenapi.VM.GetPowerState(c.session, instance)
56+
if err != nil {
57+
log.Printf(fmt.Sprintf("Unable to get VM power state '%s': %s", uuid, err.Error()))
58+
}
59+
60+
if vmstate == xenapi.VMPowerStateRunning {
61+
ui.Say(fmt.Sprintf("Shutting down VM on cleanup: %s", uuid))
62+
err = xenapi.VM.Shutdown(c.session, instance)
63+
if err != nil {
64+
log.Printf(fmt.Sprintf("Unable to shutdown VM '%s': %s", uuid, err.Error()))
65+
// Try hard shutdown if normal shutdown fails
66+
err = xenapi.VM.HardShutdown(c.session, instance)
67+
if err != nil {
68+
log.Printf(fmt.Sprintf("Unable to hard shutdown VM '%s': %s", uuid, err.Error()))
69+
}
70+
}
71+
}
72+
73+
// Destroy the VM
74+
ui.Say(fmt.Sprintf("Destroying VM on cleanup: %s", uuid))
75+
err = xenapi.VM.Destroy(c.session, instance)
76+
if err != nil {
77+
log.Printf(fmt.Sprintf("Unable to destroy VM '%s': %s", uuid, err.Error()))
78+
}
79+
80+
// Destroy all VDIs (disks) attached to the VM
81+
// Do this after VM destruction to ensure disks are detached
82+
ui.Say(fmt.Sprintf("Destroying %d disk(s)...", len(vdis)))
83+
for i, vdi := range vdis {
84+
vdiUuid, err := xenapi.VDI.GetUUID(c.session, vdi)
85+
if err != nil {
86+
log.Printf(fmt.Sprintf("Unable to get VDI UUID: %s", err.Error()))
87+
continue
88+
}
89+
90+
// Unplug the VBD first if it still exists
91+
if i < len(vbds) {
92+
_ = xenapi.VBD.Unplug(c.session, vbds[i])
93+
}
94+
95+
ui.Say(fmt.Sprintf("Destroying VDI (disk): %s", vdiUuid))
96+
97+
// Retry destroying the VDI up to 3 times
98+
var lastErr error
99+
for attempt := 1; attempt <= 3; attempt++ {
100+
err = xenapi.VDI.Destroy(c.session, vdi)
101+
if err == nil {
102+
ui.Say(fmt.Sprintf("Successfully destroyed VDI: %s", vdiUuid))
103+
lastErr = nil
104+
break
105+
}
106+
lastErr = err
107+
if attempt < 3 {
108+
log.Printf(fmt.Sprintf("Attempt %d to destroy VDI '%s' failed: %s. Retrying...", attempt, vdiUuid, err.Error()))
109+
}
110+
}
111+
if lastErr != nil {
112+
log.Printf(fmt.Sprintf("Unable to destroy VDI '%s' after 3 attempts: %s", vdiUuid, lastErr.Error()))
113+
}
114+
}
115+
ui.Say("VM and disk cleanup completed")
34116
}
35117

36118
func PreCleanup(state multistep.StateBag, force bool) error {
37119
c := state.Get("client").(*Connection)
38120
ui := state.Get("ui").(packer.Ui)
39121
config := state.Get("commonconfig").(CommonConfig)
40122

123+
ui.Say("Step: PreCleanup - Checking for existing VMs with name '" + config.VMName + "'")
124+
41125
// Let's find existing VMs with the same name
42126
vms, err := xenapi.VM.GetByNameLabel(c.session, config.VMName)
43127
if err != nil {
44128
log.Printf(fmt.Sprintf("Unable to get VM from Name '%s': %s", config.VMName, err.Error()))
45129
return err
46130
}
47131

48-
if force && len(vms) == 1 {
49-
// We found a VM and the force flag is set, so remove it
50-
ui.Say(fmt.Sprintf("The VM / Template %s already exists, but deleting it due to -force flag", config.VMName))
132+
ui.Say(fmt.Sprintf("PreCleanup: Found %d existing VM(s)", len(vms)))
51133

52-
vmstate, err := xenapi.VM.GetPowerState(c.session, vms[0])
53-
if err != nil {
54-
return fmt.Errorf("Error getting powerstate of VM %s: %v", config.VMName, err)
55-
}
56-
if vmstate == xenapi.VMPowerStateHalted || vmstate == xenapi.VMPowerStateRunning {
57-
// Shutdown the VM
58-
err = xenapi.VM.Shutdown(c.session, vms[0])
134+
if force && len(vms) > 0 {
135+
// We found one or more VMs and the force flag is set, so remove them all (but not templates)
136+
ui.Say(fmt.Sprintf("Found %d VM(s) / Template(s) named %s. Deleting due to -force flag", len(vms), config.VMName))
137+
138+
for _, vm := range vms {
139+
// Check if this is a template
140+
isTemplate, err := xenapi.VM.GetIsATemplate(c.session, vm)
59141
if err != nil {
60-
return fmt.Errorf("Error shutting down %s: %v", config.VMName, err)
142+
return fmt.Errorf("Error checking if %s is a template: %v", config.VMName, err)
61143
}
62-
}
63144

64-
// Destroy the VM
65-
err = xenapi.VM.Destroy(c.session, vms[0])
66-
if err != nil {
67-
return fmt.Errorf("Error destroying %s: %v", config.VMName, err)
145+
if isTemplate {
146+
ui.Say(fmt.Sprintf("Skipping template %s (only deleting VMs, not templates)", config.VMName))
147+
continue
148+
}
149+
150+
// Get all VBDs (Virtual Block Devices) attached to the VM
151+
vbds, err := xenapi.VM.GetVBDs(c.session, vm)
152+
if err != nil {
153+
log.Printf(fmt.Sprintf("Unable to get VBDs for VM '%s': %s", config.VMName, err.Error()))
154+
vbds = []xenapi.VBDRef{}
155+
}
156+
157+
// Collect all VDIs before destroying the VM
158+
var vdis []xenapi.VDIRef
159+
for _, vbd := range vbds {
160+
rec, err := xenapi.VBD.GetRecord(c.session, vbd)
161+
if err != nil {
162+
log.Printf(fmt.Sprintf("Unable to get VBD record: %s", err.Error()))
163+
continue
164+
}
165+
// Skip empty VBDs (like CD drives)
166+
if rec.VDI != "" {
167+
vdis = append(vdis, rec.VDI)
168+
}
169+
}
170+
171+
vmstate, err := xenapi.VM.GetPowerState(c.session, vm)
172+
if err != nil {
173+
return fmt.Errorf("Error getting powerstate of VM %s: %v", config.VMName, err)
174+
}
175+
ui.Say(fmt.Sprintf("VM %s is in state: %v", config.VMName, vmstate))
176+
177+
if vmstate == xenapi.VMPowerStateRunning {
178+
// Shutdown the VM only if it's running
179+
ui.Say(fmt.Sprintf("Shutting down running VM: %s", config.VMName))
180+
err = xenapi.VM.Shutdown(c.session, vm)
181+
if err != nil {
182+
return fmt.Errorf("Error shutting down %s: %v", config.VMName, err)
183+
}
184+
ui.Say(fmt.Sprintf("Successfully shut down VM: %s", config.VMName))
185+
}
186+
187+
// Destroy the VM
188+
ui.Say(fmt.Sprintf("Destroying VM: %s", config.VMName))
189+
err = xenapi.VM.Destroy(c.session, vm)
190+
if err != nil {
191+
return fmt.Errorf("Error destroying %s: %v", config.VMName, err)
192+
}
193+
ui.Say(fmt.Sprintf("Successfully destroyed VM: %s", config.VMName))
194+
195+
// Destroy all VDIs (disks) attached to the VM
196+
ui.Say(fmt.Sprintf("Destroying %d disk(s) for VM %s", len(vdis), config.VMName))
197+
for i, vdi := range vdis {
198+
vdiUuid, err := xenapi.VDI.GetUUID(c.session, vdi)
199+
if err != nil {
200+
log.Printf(fmt.Sprintf("Unable to get VDI UUID: %s", err.Error()))
201+
continue
202+
}
203+
204+
// Unplug the VBD first if it still exists
205+
if i < len(vbds) {
206+
_ = xenapi.VBD.Unplug(c.session, vbds[i])
207+
}
208+
209+
ui.Say(fmt.Sprintf("Destroying VDI (disk): %s", vdiUuid))
210+
211+
// Retry destroying the VDI up to 3 times
212+
var lastErr error
213+
for attempt := 1; attempt <= 3; attempt++ {
214+
err = xenapi.VDI.Destroy(c.session, vdi)
215+
if err == nil {
216+
ui.Say(fmt.Sprintf("Successfully destroyed VDI: %s", vdiUuid))
217+
lastErr = nil
218+
break
219+
}
220+
lastErr = err
221+
if attempt < 3 {
222+
log.Printf(fmt.Sprintf("Attempt %d to destroy VDI '%s' failed: %s. Retrying...", attempt, vdiUuid, err.Error()))
223+
}
224+
}
225+
if lastErr != nil {
226+
log.Printf(fmt.Sprintf("Unable to destroy VDI '%s' after 3 attempts: %s", vdiUuid, lastErr.Error()))
227+
}
228+
}
68229
}
69230
}
70231
if !force && len(vms) > 0 {

‎builder/xenserver/iso/step_create_instance.go‎

Lines changed: 16 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -4,14 +4,14 @@ import (
44
"context"
55
"fmt"
66
"log"
7-
"strings"
87
"strconv"
8+
"strings"
99

1010
"github.com/hashicorp/packer-plugin-sdk/multistep"
1111
"github.com/hashicorp/packer-plugin-sdk/packer"
1212

1313
"xenapi"
14-
14+
1515
xscommon "github.com/xenserver/packer-plugin-xenserver/builder/xenserver/common"
1616
)
1717

@@ -28,6 +28,13 @@ func (self *stepCreateInstance) Run(ctx context.Context, state multistep.StateBa
2828

2929
ui.Say("Step: Create Instance")
3030

31+
// Run Pre-Cleanup to check if VM not already exists
32+
err := xscommon.PreCleanup(state, config.PackerForce)
33+
if err != nil {
34+
state.Put("error", err)
35+
return multistep.ActionHalt
36+
}
37+
3138
// Get the template to clone from
3239

3340
vms, err := xenapi.VM.GetByNameLabel(c.GetSession(), config.CloneTemplate)
@@ -236,7 +243,7 @@ func (self *stepCreateInstance) Run(ctx context.Context, state multistep.StateBa
236243
}
237244
// If network Name label starts with "Network ", we assume it is a default network
238245
if len(networks) == 0 && strings.HasPrefix(networkNameLabel, "Network ") {
239-
246+
240247
tmpNetworkNo := strings.TrimPrefix(networkNameLabel, "Network ")
241248
tmpLabel := "Pool-wide network associated with eth" + tmpNetworkNo
242249
ui.Say(fmt.Sprintf("No network found with name-label '%s'. This might be a default built-in network '%s'", networkNameLabel, tmpLabel))
@@ -248,12 +255,12 @@ func (self *stepCreateInstance) Run(ctx context.Context, state multistep.StateBa
248255
}
249256

250257
switch {
251-
case len(networks) == 0:
252-
ui.Error(fmt.Sprintf("Couldn't find a network with the specified name-label '%s'. Aborting.", networkNameLabel))
253-
return multistep.ActionHalt
254-
case len(networks) > 1:
255-
ui.Error(fmt.Sprintf("Found more than one network with the name '%s'. The name must be unique. Aborting.", networkNameLabel))
256-
return multistep.ActionHalt
258+
case len(networks) == 0:
259+
ui.Error(fmt.Sprintf("Couldn't find a network with the specified name-label '%s'. Aborting.", networkNameLabel))
260+
return multistep.ActionHalt
261+
case len(networks) > 1:
262+
ui.Error(fmt.Sprintf("Found more than one network with the name '%s'. The name must be unique. Aborting.", networkNameLabel))
263+
return multistep.ActionHalt
257264
}
258265

259266
//we need the VIF index string

0 commit comments

Comments
 (0)