Skip to content

Commit 37e90ef

Browse files
LopatinDmitrLopatinDmitr
committed
fix(vm): refactor blockDeviceReady handler
Signed-off-by: dmitry.lopatin <[email protected]>
1 parent 26b0d68 commit 37e90ef

File tree

10 files changed

+1395
-327
lines changed

10 files changed

+1395
-327
lines changed

‎api/core/v1alpha2/virtual_disk.go

+3
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,10 @@ type VirtualDiskStatsCreationDuration struct {
102102

103103
// List of VirtualMachines that use the disk.
104104
type AttachedVirtualMachine struct {
105+
// Name of attached VirtualMachine.
105106
Name string `json:"name,omitempty"`
107+
// Flag indicating that VirtualDisk is currently being used by this attached VirtualMachine.
108+
Mounted bool `json:"mounted,omitempty"`
106109
}
107110

108111
// +kubebuilder:validation:XValidation:rule="self.type == 'HTTP' ? has(self.http) && !has(self.containerImage) && !has(self.objectRef) : true",message="HTTP requires http and cannot have ContainerImage or ObjectRef."

‎crds/doc-ru-virtualdisks.yaml

+8
Original file line numberDiff line numberDiff line change
@@ -144,6 +144,14 @@ spec:
144144
attachedToVirtualMachines:
145145
description: |
146146
Список виртуальных машин, использующих данный диск.
147+
properties:
148+
mounted:
149+
description:
150+
Флаг, указывающий, что этот VirtualDisk в настоящее время используется
151+
этой присоединенной VirtualMachine.
152+
type: boolean
153+
name:
154+
description: Имя присоединённой VirtualMachine.
147155
stats:
148156
description: |
149157
Статистика по виртуальному диску.

‎crds/virtualdisks.yaml

+6
Original file line numberDiff line numberDiff line change
@@ -257,7 +257,13 @@ spec:
257257
items:
258258
description: List of VirtualMachines that use the disk.
259259
properties:
260+
mounted:
261+
description:
262+
Flag indicating that VirtualDisk is currently being
263+
used by this attached VirtualMachine.
264+
type: boolean
260265
name:
266+
description: Name of attached VirtualMachine.
261267
type: string
262268
type: object
263269
type: array

‎images/virtualization-artifact/pkg/controller/vd/internal/attachee.go

-103
This file was deleted.

‎images/virtualization-artifact/pkg/controller/vd/internal/inuse.go

+38-21
Original file line numberDiff line numberDiff line change
@@ -164,20 +164,22 @@ func (h InUseHandler) checkUsageByVM(ctx context.Context, vd *virtv2.VirtualDisk
164164
return false, fmt.Errorf("error getting virtual machines: %w", err)
165165
}
166166

167+
var attachedVMs []virtv2.AttachedVirtualMachine
168+
vmFound := false
169+
167170
for _, vm := range vms.Items {
168171
if !h.isVDAttachedToVM(vd.GetName(), vm) {
169172
continue
170173
}
171174

175+
usedByVM := false
176+
172177
switch vm.Status.Phase {
173178
case "":
174-
return false, nil
175-
179+
usedByVM = false
176180
case virtv2.MachinePending:
177-
usedByVM := canStartVM(vm.Status.Conditions)
178-
179-
if usedByVM {
180-
return true, nil
181+
if canStartVM(vm.Status.Conditions) {
182+
usedByVM = true
181183
}
182184
case virtv2.MachineStopped:
183185
kvvm, err := object.FetchObject(ctx, types.NamespacedName{Name: vm.Name, Namespace: vm.Namespace}, h.client, &virtv1.VirtualMachine{})
@@ -186,29 +188,44 @@ func (h InUseHandler) checkUsageByVM(ctx context.Context, vd *virtv2.VirtualDisk
186188
}
187189

188190
if kvvm != nil && kvvm.Status.StateChangeRequests != nil {
189-
return true, nil
191+
usedByVM = true
192+
} else {
193+
podList := corev1.PodList{}
194+
err = h.client.List(ctx, &podList, &client.ListOptions{
195+
Namespace: vm.GetNamespace(),
196+
LabelSelector: labels.SelectorFromSet(map[string]string{virtv1.VirtualMachineNameLabel: vm.GetName()}),
197+
})
198+
if err != nil {
199+
return false, fmt.Errorf("unable to list virt-launcher Pod for VM %q: %w", vm.GetName(), err)
200+
}
201+
202+
for _, pod := range podList.Items {
203+
if pod.Status.Phase == corev1.PodRunning {
204+
usedByVM = true
205+
break
206+
}
207+
}
190208
}
209+
default:
210+
usedByVM = true
211+
}
191212

192-
podList := corev1.PodList{}
193-
err = h.client.List(ctx, &podList, &client.ListOptions{
194-
Namespace: vm.GetNamespace(),
195-
LabelSelector: labels.SelectorFromSet(map[string]string{virtv1.VirtualMachineNameLabel: vm.GetName()}),
196-
})
197-
if err != nil {
198-
return false, fmt.Errorf("unable to list virt-launcher Pod for VM %q: %w", vm.GetName(), err)
213+
if usedByVM {
214+
attachedVM := virtv2.AttachedVirtualMachine{
215+
Name: vm.GetName(),
199216
}
200217

201-
for _, pod := range podList.Items {
202-
if pod.Status.Phase == corev1.PodRunning {
203-
return true, nil
204-
}
218+
if !vmFound {
219+
attachedVM.Mounted = true
220+
vmFound = true
205221
}
206-
default:
207-
return true, nil
222+
223+
attachedVMs = append(attachedVMs, attachedVM)
208224
}
209225
}
210226

211-
return false, nil
227+
vd.Status.AttachedToVirtualMachines = attachedVMs
228+
return vmFound, nil
212229
}
213230

214231
func (h InUseHandler) checkUsageByVI(ctx context.Context, vd *virtv2.VirtualDisk) (bool, error) {

‎images/virtualization-artifact/pkg/controller/vd/internal/inuse_test.go

+109
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,115 @@ var _ = Describe("InUseHandler", func() {
5151
ctx = context.TODO()
5252
})
5353

54+
Context("when handling VirtualDisk usage", func() {
55+
It("should correctly update status for a disk used by a running VM", func() {
56+
vd := &virtv2.VirtualDisk{
57+
ObjectMeta: metav1.ObjectMeta{
58+
Name: "test-vd",
59+
Namespace: "default",
60+
},
61+
Status: virtv2.VirtualDiskStatus{
62+
Conditions: []metav1.Condition{},
63+
},
64+
}
65+
66+
vm := &virtv2.VirtualMachine{
67+
ObjectMeta: metav1.ObjectMeta{
68+
Name: "test-vm",
69+
Namespace: "default",
70+
},
71+
Spec: virtv2.VirtualMachineSpec{
72+
BlockDeviceRefs: []virtv2.BlockDeviceSpecRef{
73+
{
74+
Kind: virtv2.DiskDevice,
75+
Name: "test-vd",
76+
},
77+
},
78+
},
79+
Status: virtv2.VirtualMachineStatus{
80+
Phase: virtv2.MachineRunning,
81+
BlockDeviceRefs: []virtv2.BlockDeviceStatusRef{
82+
{
83+
Kind: virtv2.DiskDevice,
84+
Name: "test-vd",
85+
},
86+
},
87+
},
88+
}
89+
90+
k8sClient := fake.NewClientBuilder().WithScheme(scheme).WithObjects(vd, vm).Build()
91+
handler = &InUseHandler{client: k8sClient}
92+
93+
result, err := handler.Handle(ctx, vd)
94+
Expect(err).ToNot(HaveOccurred())
95+
Expect(result).To(Equal(ctrl.Result{}))
96+
97+
cond, _ := conditions.GetCondition(vdcondition.InUseType, vd.Status.Conditions)
98+
Expect(cond).ToNot(BeNil())
99+
Expect(cond.Status).To(Equal(metav1.ConditionTrue))
100+
Expect(cond.Reason).To(Equal(vdcondition.AttachedToVirtualMachine.String()))
101+
102+
Expect(len(vd.Status.AttachedToVirtualMachines)).To(Equal(1))
103+
Expect(vd.Status.AttachedToVirtualMachines[0].Name).To(Equal("test-vm"))
104+
Expect(vd.Status.AttachedToVirtualMachines[0].Mounted).To(BeTrue())
105+
})
106+
107+
It("should update the status to NotInUse if no VM uses the disk", func() {
108+
vd := &virtv2.VirtualDisk{
109+
ObjectMeta: metav1.ObjectMeta{
110+
Name: "test-vd",
111+
Namespace: "default",
112+
},
113+
Status: virtv2.VirtualDiskStatus{
114+
Conditions: []metav1.Condition{},
115+
},
116+
}
117+
118+
k8sClient := fake.NewClientBuilder().WithScheme(scheme).WithObjects(vd).Build()
119+
handler = &InUseHandler{client: k8sClient}
120+
121+
result, err := handler.Handle(ctx, vd)
122+
Expect(err).ToNot(HaveOccurred())
123+
Expect(result).To(Equal(ctrl.Result{}))
124+
125+
cond, _ := conditions.GetCondition(vdcondition.InUseType, vd.Status.Conditions)
126+
Expect(cond).ToNot(BeNil())
127+
Expect(cond.Status).To(Equal(metav1.ConditionFalse))
128+
Expect(cond.Reason).To(Equal(vdcondition.NotInUse.String()))
129+
130+
Expect(len(vd.Status.AttachedToVirtualMachines)).To(Equal(0))
131+
})
132+
133+
It("should handle VM disappearance and update status accordingly", func() {
134+
vd := &virtv2.VirtualDisk{
135+
ObjectMeta: metav1.ObjectMeta{
136+
Name: "test-vd",
137+
Namespace: "default",
138+
},
139+
Status: virtv2.VirtualDiskStatus{
140+
Conditions: []metav1.Condition{},
141+
AttachedToVirtualMachines: []virtv2.AttachedVirtualMachine{
142+
{Name: "missing-vm"},
143+
},
144+
},
145+
}
146+
147+
k8sClient := fake.NewClientBuilder().WithScheme(scheme).WithObjects(vd).Build()
148+
handler = &InUseHandler{client: k8sClient}
149+
150+
result, err := handler.Handle(ctx, vd)
151+
Expect(err).ToNot(HaveOccurred())
152+
Expect(result).To(Equal(ctrl.Result{}))
153+
154+
cond, _ := conditions.GetCondition(vdcondition.InUseType, vd.Status.Conditions)
155+
Expect(cond).ToNot(BeNil())
156+
Expect(cond.Status).To(Equal(metav1.ConditionFalse))
157+
Expect(cond.Reason).To(Equal(vdcondition.NotInUse.String()))
158+
159+
Expect(len(vd.Status.AttachedToVirtualMachines)).To(Equal(0))
160+
})
161+
})
162+
54163
Context("when VirtualDisk is not in use", func() {
55164
It("must set status Unknown and reason Unknown", func() {
56165
vd := &virtv2.VirtualDisk{

0 commit comments

Comments
 (0)