Skip to content

Commit 49a72ca

Browse files
committed
fix(vm): fix
Signed-off-by: dmitry.lopatin <[email protected]>
1 parent f60b7f7 commit 49a72ca

File tree

2 files changed

+186
-46
lines changed

2 files changed

+186
-46
lines changed

images/virtualization-artifact/pkg/controller/vm/internal/block_device.go

+18-3
Original file line numberDiff line numberDiff line change
@@ -163,9 +163,20 @@ func (h *BlockDeviceHandler) checkVDToReadyForUse(ctx context.Context, s state.V
163163
if inUseCondition.Status != metav1.ConditionTrue ||
164164
inUseCondition.Reason != vdcondition.AttachedToVirtualMachine.String() ||
165165
inUseCondition.ObservedGeneration != vd.Generation {
166+
var msg string
167+
if len(vds) == 1 {
168+
if inUseCondition.Reason == vdcondition.UsedForImageCreation.String() {
169+
msg = fmt.Sprintf("Virtuak disk %q is in use for image creation.", vd.Name)
170+
} else {
171+
msg = fmt.Sprintf("Waiting for block device %q to be ready to use.", vd.Name)
172+
}
173+
} else {
174+
msg = "Virtual disks cannot be used because they are being used for creating an image."
175+
}
176+
166177
cb.Status(metav1.ConditionFalse).
167178
Reason(vmcondition.ReasonBlockDevicesNotReady).
168-
Message("Virtual disks cannot be used because they are being used for creating an image.")
179+
Message(msg)
169180
conditions.SetCondition(cb, &vm.Status.Conditions)
170181
return ErrBlockDeviceNotReadyForUse
171182
}
@@ -183,8 +194,12 @@ func (h *BlockDeviceHandler) checkVDToReady(s state.VirtualMachineState, bdState
183194

184195
if readyCount, canStartKVVM, warnings := h.countReadyBlockDevices(current, bdState, log); len(current.Spec.BlockDeviceRefs) != readyCount {
185196
var reason vmcondition.Reason
186-
187-
msg := fmt.Sprintf("Waiting for block devices to become ready: %d/%d", readyCount, len(current.Spec.BlockDeviceRefs))
197+
var msg string
198+
if len(current.Spec.BlockDeviceRefs) == 1 {
199+
msg = fmt.Sprintf("Waiting for block device %q to be ready", current.Spec.BlockDeviceRefs[0].Name)
200+
} else {
201+
msg = fmt.Sprintf("Waiting for block devices to be ready: %d/%d", readyCount, len(current.Spec.BlockDeviceRefs))
202+
}
188203
if len(warnings) > 0 {
189204
msg = msg + "; " + strings.Join(warnings, "; ")
190205
}

images/virtualization-artifact/pkg/controller/vm/internal/block_devices_test.go

+168-43
Original file line numberDiff line numberDiff line change
@@ -80,50 +80,60 @@ var _ = Describe("Test BlockDeviceReady condition", func() {
8080
}
8181
}
8282

83-
getNotReadyVD := func() *virtv2.VirtualDisk {
84-
return &virtv2.VirtualDisk{
83+
getVMWithTwoVD := func(phase virtv2.MachinePhase) *virtv2.VirtualMachine {
84+
return &virtv2.VirtualMachine{
8585
ObjectMeta: metav1.ObjectMeta{
86-
Name: "vd1",
86+
Name: namespacedName.Name,
8787
Namespace: namespacedName.Namespace,
8888
},
89-
Status: virtv2.VirtualDiskStatus{
90-
Conditions: []metav1.Condition{},
89+
Spec: virtv2.VirtualMachineSpec{
90+
BlockDeviceRefs: []virtv2.BlockDeviceSpecRef{
91+
{
92+
Kind: virtv2.DiskDevice,
93+
Name: "vd1",
94+
},
95+
{
96+
Kind: virtv2.DiskDevice,
97+
Name: "vd2",
98+
},
99+
},
100+
},
101+
Status: virtv2.VirtualMachineStatus{
102+
Phase: phase,
103+
BlockDeviceRefs: []virtv2.BlockDeviceStatusRef{
104+
{
105+
Kind: virtv2.DiskDevice,
106+
Name: "vd1",
107+
},
108+
{
109+
Kind: virtv2.DiskDevice,
110+
Name: "vd2",
111+
},
112+
},
91113
},
92114
}
93115
}
94116

95-
getNotAllowedReadyVD := func() *virtv2.VirtualDisk {
117+
getNotReadyVD := func(name string, status metav1.ConditionStatus, reason string) *virtv2.VirtualDisk {
96118
return &virtv2.VirtualDisk{
97119
ObjectMeta: metav1.ObjectMeta{
98-
Name: "vd1",
120+
Name: name,
99121
Namespace: namespacedName.Namespace,
100122
},
101123
Status: virtv2.VirtualDiskStatus{
102-
Target: virtv2.DiskTarget{
103-
PersistentVolumeClaim: "testPvc",
104-
},
105-
Phase: virtv2.DiskReady,
106-
Conditions: []metav1.Condition{
107-
{
108-
Type: vdcondition.ReadyType.String(),
109-
Status: metav1.ConditionTrue,
110-
Reason: string(vdcondition.Ready),
111-
Message: "",
112-
},
113-
},
114-
AttachedToVirtualMachines: []virtv2.AttachedVirtualMachine{
115-
{
116-
Name: namespacedName.Name,
117-
},
118-
},
124+
Conditions: []metav1.Condition{{
125+
Type: vdcondition.InUseType.String(),
126+
Status: status,
127+
Reason: reason,
128+
}},
119129
},
120130
}
121131
}
122132

123-
getAllowedReadyVD := func() *virtv2.VirtualDisk {
133+
getReadyVD := func(name string, status metav1.ConditionStatus, reason string) *virtv2.VirtualDisk {
124134
return &virtv2.VirtualDisk{
125135
ObjectMeta: metav1.ObjectMeta{
126-
Name: "vd1",
136+
Name: name,
127137
Namespace: namespacedName.Namespace,
128138
},
129139
Status: virtv2.VirtualDiskStatus{
@@ -140,8 +150,8 @@ var _ = Describe("Test BlockDeviceReady condition", func() {
140150
},
141151
{
142152
Type: vdcondition.InUseType.String(),
143-
Status: metav1.ConditionTrue,
144-
Reason: vdcondition.AttachedToVirtualMachine.String(),
153+
Status: status,
154+
Reason: reason,
145155
},
146156
},
147157
AttachedToVirtualMachines: []virtv2.AttachedVirtualMachine{
@@ -153,66 +163,177 @@ var _ = Describe("Test BlockDeviceReady condition", func() {
153163
}
154164
}
155165

166+
vd1 := "vd1"
167+
vd2 := "vd2"
168+
156169
var testCases = []struct {
157170
vm *virtv2.VirtualMachine
158171
vd1 *virtv2.VirtualDisk
159172
vd2 *virtv2.VirtualDisk
160173
expected string
161174
}{
162175
{
163-
vd1: getNotReadyVD(),
176+
vd1: getNotReadyVD(vd1, metav1.ConditionTrue, vdcondition.AttachedToVirtualMachine.String()),
177+
vd2: nil,
178+
vm: getVMWithOneVD(virtv2.MachinePending),
179+
expected: "Waiting for block device \"vd1\" to be ready.",
180+
},
181+
{
182+
vd1: getNotReadyVD(vd1, metav1.ConditionTrue, vdcondition.AttachedToVirtualMachine.String()),
183+
vd2: nil,
184+
vm: getVMWithOneVD(virtv2.MachineRunning),
185+
expected: "Waiting for block device \"vd1\" to be ready.",
186+
},
187+
{
188+
vd1: getNotReadyVD(vd1, metav1.ConditionTrue, vdcondition.AttachedToVirtualMachine.String()),
189+
vd2: nil,
190+
vm: getVMWithOneVD(virtv2.MachineStopped),
191+
expected: "Waiting for block device \"vd1\" to be ready.",
192+
},
193+
{
194+
vd1: getNotReadyVD(vd1, metav1.ConditionTrue, vdcondition.UsedForImageCreation.String()),
195+
vd2: nil,
196+
vm: getVMWithOneVD(virtv2.MachinePending),
197+
expected: "Waiting for block device \"vd1\" to be ready.",
198+
},
199+
{
200+
vd1: getNotReadyVD(vd1, metav1.ConditionTrue, vdcondition.UsedForImageCreation.String()),
201+
vd2: nil,
202+
vm: getVMWithOneVD(virtv2.MachineRunning),
203+
expected: "Waiting for block device \"vd1\" to be ready.",
204+
},
205+
{
206+
vd1: getNotReadyVD(vd1, metav1.ConditionTrue, vdcondition.UsedForImageCreation.String()),
207+
vd2: nil,
208+
vm: getVMWithOneVD(virtv2.MachineStopped),
209+
expected: "Waiting for block device \"vd1\" to be ready.",
210+
},
211+
{
212+
vd1: getNotReadyVD(vd1, metav1.ConditionFalse, vdcondition.NotInUse.String()),
213+
vd2: nil,
214+
vm: getVMWithOneVD(virtv2.MachinePending),
215+
expected: "Waiting for block device \"vd1\" to be ready.",
216+
},
217+
{
218+
vd1: getNotReadyVD(vd1, metav1.ConditionFalse, vdcondition.NotInUse.String()),
219+
vd2: nil,
220+
vm: getVMWithOneVD(virtv2.MachineRunning),
221+
expected: "Waiting for block device \"vd1\" to be ready.",
222+
},
223+
{
224+
vd1: getNotReadyVD(vd1, metav1.ConditionFalse, vdcondition.NotInUse.String()),
225+
vd2: nil,
226+
vm: getVMWithOneVD(virtv2.MachineStopped),
227+
expected: "Waiting for block device \"vd1\" to be ready.",
228+
},
229+
{
230+
vd1: getReadyVD(vd1, metav1.ConditionFalse, vdcondition.NotInUse.String()),
164231
vd2: nil,
165232
vm: getVMWithOneVD(virtv2.MachinePending),
166-
expected: "Waiting for block devices to become ready: 0/1.",
233+
expected: "Waiting for block device \"vd1\" to be ready to use.",
167234
},
168235
{
169-
vd1: getNotReadyVD(),
236+
vd1: getReadyVD(vd1, metav1.ConditionFalse, vdcondition.NotInUse.String()),
170237
vd2: nil,
171238
vm: getVMWithOneVD(virtv2.MachineRunning),
172-
expected: "Waiting for block devices to become ready: 0/1.",
239+
expected: "Waiting for block device \"vd1\" to be ready to use.",
173240
},
174241
{
175-
vd1: getNotReadyVD(),
242+
vd1: getReadyVD(vd1, metav1.ConditionFalse, vdcondition.NotInUse.String()),
176243
vd2: nil,
177244
vm: getVMWithOneVD(virtv2.MachineStopped),
178-
expected: "Waiting for block devices to become ready: 0/1.",
245+
expected: "",
179246
},
180247
{
181-
vd1: getNotAllowedReadyVD(),
248+
vd1: getReadyVD(vd1, metav1.ConditionTrue, vdcondition.UsedForImageCreation.String()),
182249
vd2: nil,
183250
vm: getVMWithOneVD(virtv2.MachinePending),
184-
expected: "Virtual disks cannot be used because they are being used for creating an image.",
251+
expected: "Virtuak disk \"vd1\" is in use for image creation.",
185252
},
186253
{
187-
vd1: getNotAllowedReadyVD(),
254+
vd1: getReadyVD(vd1, metav1.ConditionTrue, vdcondition.UsedForImageCreation.String()),
188255
vd2: nil,
189256
vm: getVMWithOneVD(virtv2.MachineRunning),
190-
expected: "Virtual disks cannot be used because they are being used for creating an image.",
257+
expected: "Virtuak disk \"vd1\" is in use for image creation.",
191258
},
192259
{
193-
vd1: getNotAllowedReadyVD(),
260+
vd1: getReadyVD(vd1, metav1.ConditionTrue, vdcondition.UsedForImageCreation.String()),
194261
vd2: nil,
195262
vm: getVMWithOneVD(virtv2.MachineStopped),
196-
expected: "Virtual disks cannot be used because they are being used for creating an image.",
263+
expected: "",
197264
},
198265
{
199-
vd1: getAllowedReadyVD(),
266+
vd1: getReadyVD(vd1, metav1.ConditionTrue, vdcondition.AttachedToVirtualMachine.String()),
200267
vd2: nil,
201268
vm: getVMWithOneVD(virtv2.MachinePending),
202269
expected: "",
203270
},
204271
{
205-
vd1: getAllowedReadyVD(),
272+
vd1: getReadyVD(vd1, metav1.ConditionTrue, vdcondition.AttachedToVirtualMachine.String()),
206273
vd2: nil,
207274
vm: getVMWithOneVD(virtv2.MachineRunning),
208275
expected: "",
209276
},
210277
{
211-
vd1: getAllowedReadyVD(),
278+
vd1: getReadyVD(vd1, metav1.ConditionTrue, vdcondition.AttachedToVirtualMachine.String()),
212279
vd2: nil,
213280
vm: getVMWithOneVD(virtv2.MachineStopped),
214281
expected: "",
215282
},
283+
{
284+
vd1: getReadyVD(vd1, metav1.ConditionTrue, vdcondition.AttachedToVirtualMachine.String()),
285+
vd2: getNotReadyVD(vd2, metav1.ConditionTrue, vdcondition.AttachedToVirtualMachine.String()),
286+
vm: getVMWithTwoVD(virtv2.MachinePending),
287+
expected: "Waiting for block devices to be ready: 1/2.",
288+
},
289+
{
290+
vd1: getReadyVD(vd1, metav1.ConditionTrue, vdcondition.AttachedToVirtualMachine.String()),
291+
vd2: getNotReadyVD(vd2, metav1.ConditionTrue, vdcondition.AttachedToVirtualMachine.String()),
292+
vm: getVMWithTwoVD(virtv2.MachineRunning),
293+
expected: "Waiting for block devices to be ready: 1/2.",
294+
},
295+
{
296+
vd1: getReadyVD(vd1, metav1.ConditionTrue, vdcondition.AttachedToVirtualMachine.String()),
297+
vd2: getNotReadyVD(vd2, metav1.ConditionTrue, vdcondition.AttachedToVirtualMachine.String()),
298+
vm: getVMWithTwoVD(virtv2.MachineStopped),
299+
expected: "Waiting for block devices to be ready: 1/2.",
300+
},
301+
{
302+
vd1: getReadyVD(vd1, metav1.ConditionTrue, vdcondition.AttachedToVirtualMachine.String()),
303+
vd2: getNotReadyVD(vd2, metav1.ConditionTrue, vdcondition.UsedForImageCreation.String()),
304+
vm: getVMWithTwoVD(virtv2.MachinePending),
305+
expected: "Waiting for block devices to be ready: 1/2.",
306+
},
307+
{
308+
vd1: getReadyVD(vd1, metav1.ConditionTrue, vdcondition.AttachedToVirtualMachine.String()),
309+
vd2: getNotReadyVD(vd2, metav1.ConditionTrue, vdcondition.UsedForImageCreation.String()),
310+
vm: getVMWithTwoVD(virtv2.MachineRunning),
311+
expected: "Waiting for block devices to be ready: 1/2.",
312+
},
313+
{
314+
vd1: getReadyVD(vd1, metav1.ConditionTrue, vdcondition.AttachedToVirtualMachine.String()),
315+
vd2: getNotReadyVD(vd2, metav1.ConditionTrue, vdcondition.UsedForImageCreation.String()),
316+
vm: getVMWithTwoVD(virtv2.MachineStopped),
317+
expected: "Waiting for block devices to be ready: 1/2.",
318+
},
319+
{
320+
vd1: getReadyVD(vd1, metav1.ConditionTrue, vdcondition.AttachedToVirtualMachine.String()),
321+
vd2: getNotReadyVD(vd2, metav1.ConditionFalse, vdcondition.NotInUse.String()),
322+
vm: getVMWithTwoVD(virtv2.MachinePending),
323+
expected: "Waiting for block devices to be ready: 1/2.",
324+
},
325+
{
326+
vd1: getReadyVD(vd1, metav1.ConditionTrue, vdcondition.AttachedToVirtualMachine.String()),
327+
vd2: getNotReadyVD(vd2, metav1.ConditionFalse, vdcondition.NotInUse.String()),
328+
vm: getVMWithTwoVD(virtv2.MachineRunning),
329+
expected: "Waiting for block devices to be ready: 1/2.",
330+
},
331+
{
332+
vd1: getReadyVD(vd1, metav1.ConditionTrue, vdcondition.AttachedToVirtualMachine.String()),
333+
vd2: getNotReadyVD(vd2, metav1.ConditionFalse, vdcondition.NotInUse.String()),
334+
vm: getVMWithTwoVD(virtv2.MachineStopped),
335+
expected: "Waiting for block devices to be ready: 1/2.",
336+
},
216337
}
217338

218339
var (
@@ -254,10 +375,14 @@ var _ = Describe("Test BlockDeviceReady condition", func() {
254375
var fakeClient client.Client
255376
if tc.vd2 == nil {
256377
fakeClient = fake.NewClientBuilder().WithScheme(scheme).WithObjects(tc.vm, tc.vd1).Build()
378+
} else {
379+
fakeClient = fake.NewClientBuilder().WithScheme(scheme).WithObjects(tc.vm, tc.vd1, tc.vd2).Build()
257380
}
381+
258382
vmResource := service.NewResource(namespacedName, fakeClient, vmFactoryByVm(tc.vm), vmStatusGetter)
259383
err := vmResource.Fetch(ctx)
260384
Expect(err).NotTo(HaveOccurred())
385+
261386
vmState := state.New(fakeClient, vmResource)
262387
handler := NewBlockDeviceHandler(fakeClient, recorderMock, okBlockDeviceServiceMock)
263388
_, err = handler.Handle(ctx, vmState)

0 commit comments

Comments
 (0)