Skip to content

Commit 2c500cd

Browse files
danilrwxdanilrwx
committed
fix(vmop): restore requested virtual disk size from snapshot metadata
Signed-off-by: Daniil Antoshin <daniil.antoshin@flant.com>
1 parent f971d4f commit 2c500cd

5 files changed

Lines changed: 161 additions & 0 deletions

File tree

‎images/virtualization-artifact/pkg/common/annotations/annotations.go‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -198,6 +198,8 @@ const (
198198
AnnVirtualDiskOriginalAnnotations = AnnAPIGroupV + "/vd-original-annotations"
199199
// AnnVirtualDiskOriginalLabels is the annotation for storing original VirtualDisk labels.
200200
AnnVirtualDiskOriginalLabels = AnnAPIGroupV + "/vd-original-labels"
201+
// AnnVirtualDiskRequestedSize is the annotation for storing original VirtualDisk requested PVC size.
202+
AnnVirtualDiskRequestedSize = AnnAPIGroupV + "/vd-requested-size"
201203
// AnnVirtualDiskHadOwnerReference is the annotation on VolumeSnapshot set to "true" when the source VirtualDisk had an owner reference at snapshot time; absent otherwise.
202204
AnnVirtualDiskHadOwnerReference = AnnAPIGroupV + "/vd-had-owner-reference"
203205
// AnnVMOPUID is an annotation on vmop that represents name of VMOP.

‎images/virtualization-artifact/pkg/controller/service/restorer/snapshot_resources.go‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ import (
2525
vsv1 "github.com/kubernetes-csi/external-snapshotter/client/v6/apis/volumesnapshot/v1"
2626
corev1 "k8s.io/api/core/v1"
2727
apierrors "k8s.io/apimachinery/pkg/api/errors"
28+
"k8s.io/apimachinery/pkg/api/resource"
2829
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
2930
"k8s.io/apimachinery/pkg/types"
3031
"sigs.k8s.io/controller-runtime/pkg/client"
@@ -352,6 +353,11 @@ func getVirtualDisks(ctx context.Context, client client.Client, vmSnapshot *v1al
352353
}
353354
}
354355

356+
requestedSize, err := getOriginalRequestedSize(ctx, client, vdSnapshot)
357+
if err != nil {
358+
return nil, err
359+
}
360+
355361
vd := v1alpha2.VirtualDisk{
356362
TypeMeta: metav1.TypeMeta{
357363
Kind: v1alpha2.VirtualDiskKind,
@@ -369,6 +375,9 @@ func getVirtualDisks(ctx context.Context, client client.Client, vmSnapshot *v1al
369375
Name: vdSnapshot.Name,
370376
},
371377
},
378+
PersistentVolumeClaim: v1alpha2.VirtualDiskPersistentVolumeClaim{
379+
Size: requestedSize,
380+
},
372381
},
373382
Status: v1alpha2.VirtualDiskStatus{
374383
AttachedToVirtualMachines: attachedVMs,
@@ -482,6 +491,34 @@ func AddOriginalMetadata(ctx context.Context, vd *v1alpha2.VirtualDisk, vdSnapsh
482491
)
483492
}
484493

494+
func getOriginalRequestedSize(ctx context.Context, client client.Client, vdSnapshot *v1alpha2.VirtualDiskSnapshot) (*resource.Quantity, error) {
495+
vsKey := types.NamespacedName{
496+
Namespace: vdSnapshot.Namespace,
497+
Name: vdSnapshot.Status.VolumeSnapshotName,
498+
}
499+
500+
vs, err := object.FetchObject(ctx, vsKey, client, &vsv1.VolumeSnapshot{})
501+
if err != nil {
502+
return nil, fmt.Errorf("failed to fetch the volume snapshot %q: %w", vsKey.Name, err)
503+
}
504+
505+
if vs == nil {
506+
return nil, fmt.Errorf("the volume snapshot %q is nil, please report a bug", vsKey.Name)
507+
}
508+
509+
requestedSize := vs.Annotations[annotations.AnnVirtualDiskRequestedSize]
510+
if requestedSize == "" {
511+
return nil, nil
512+
}
513+
514+
size, err := resource.ParseQuantity(requestedSize)
515+
if err != nil {
516+
return nil, fmt.Errorf("failed to parse the original requested size %q: %w", requestedSize, err)
517+
}
518+
519+
return &size, nil
520+
}
521+
485522
func setOriginalAnnotations(vd *v1alpha2.VirtualDisk, vs *vsv1.VolumeSnapshot) error {
486523
if vs == nil || vs.Annotations[annotations.AnnVirtualDiskOriginalAnnotations] == "" {
487524
return nil

‎images/virtualization-artifact/pkg/controller/service/restorer/snapshot_resources_test.go‎

Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,11 +20,16 @@ import (
2020
"context"
2121
"encoding/json"
2222

23+
vsv1 "github.com/kubernetes-csi/external-snapshotter/client/v6/apis/volumesnapshot/v1"
2324
. "github.com/onsi/ginkgo/v2"
2425
. "github.com/onsi/gomega"
2526
corev1 "k8s.io/api/core/v1"
27+
"k8s.io/apimachinery/pkg/api/resource"
2628
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
29+
"k8s.io/apimachinery/pkg/runtime"
30+
"sigs.k8s.io/controller-runtime/pkg/client/fake"
2731

32+
"github.com/deckhouse/virtualization-controller/pkg/common/annotations"
2833
"github.com/deckhouse/virtualization-controller/pkg/common/testutil"
2934
"github.com/deckhouse/virtualization/api/core/v1alpha2"
3035
)
@@ -105,4 +110,100 @@ var _ = Describe("SnapshotResources.Prepare", func() {
105110
Expect(restoredVM.Spec.Networks[0].VirtualMachineMACAddressName).To(BeEmpty())
106111
Expect(restoredVM.Spec.Networks[1].VirtualMachineMACAddressName).To(Equal("vm-mac-secondary"))
107112
})
113+
114+
DescribeTable("restores VirtualDisk requested size",
115+
func(originalSize *resource.Quantity, expectedSize *resource.Quantity) {
116+
vm := &v1alpha2.VirtualMachine{
117+
TypeMeta: metav1.TypeMeta{
118+
Kind: v1alpha2.VirtualMachineKind,
119+
APIVersion: v1alpha2.SchemeGroupVersion.String(),
120+
},
121+
ObjectMeta: metav1.ObjectMeta{
122+
Name: "vm",
123+
Namespace: "default",
124+
},
125+
}
126+
127+
vmJSON, err := json.Marshal(vm)
128+
Expect(err).NotTo(HaveOccurred())
129+
130+
restorerSecret := &corev1.Secret{
131+
ObjectMeta: metav1.ObjectMeta{Name: "restorer-secret", Namespace: "default"},
132+
Data: map[string][]byte{
133+
virtualMachineKey: vmJSON,
134+
},
135+
}
136+
137+
annotationsMap := map[string]string{}
138+
if originalSize != nil {
139+
annotationsMap[annotations.AnnVirtualDiskRequestedSize] = originalSize.String()
140+
}
141+
142+
volumeSnapshot := &vsv1.VolumeSnapshot{
143+
ObjectMeta: metav1.ObjectMeta{
144+
Name: "volume-snapshot",
145+
Namespace: "default",
146+
Annotations: annotationsMap,
147+
},
148+
}
149+
150+
vdSnapshot := &v1alpha2.VirtualDiskSnapshot{
151+
ObjectMeta: metav1.ObjectMeta{Name: "vd-snapshot", Namespace: "default"},
152+
Spec: v1alpha2.VirtualDiskSnapshotSpec{
153+
VirtualDiskName: "vd",
154+
},
155+
Status: v1alpha2.VirtualDiskSnapshotStatus{
156+
Phase: v1alpha2.VirtualDiskSnapshotPhaseReady,
157+
VolumeSnapshotName: volumeSnapshot.Name,
158+
},
159+
}
160+
161+
scheme := runtime.NewScheme()
162+
Expect(v1alpha2.AddToScheme(scheme)).To(Succeed())
163+
Expect(corev1.AddToScheme(scheme)).To(Succeed())
164+
Expect(vsv1.AddToScheme(scheme)).To(Succeed())
165+
166+
fakeClient := fake.NewClientBuilder().
167+
WithScheme(scheme).
168+
WithObjects(volumeSnapshot, vdSnapshot).
169+
Build()
170+
171+
vmSnapshot := &v1alpha2.VirtualMachineSnapshot{
172+
ObjectMeta: metav1.ObjectMeta{Name: "snapshot", Namespace: "default"},
173+
Spec: v1alpha2.VirtualMachineSnapshotSpec{VirtualMachineName: "vm"},
174+
Status: v1alpha2.VirtualMachineSnapshotStatus{
175+
VirtualDiskSnapshotNames: []string{vdSnapshot.Name},
176+
},
177+
}
178+
179+
resources := NewSnapshotResources(
180+
fakeClient, v1alpha2.VMOPTypeRestore, v1alpha2.SnapshotOperationModeStrict,
181+
restorerSecret, vmSnapshot, "restore-uid",
182+
)
183+
Expect(resources.Prepare(context.Background())).To(Succeed())
184+
185+
var restoredVD *v1alpha2.VirtualDisk
186+
for _, handler := range resources.GetObjectHandlers() {
187+
if obj, ok := handler.Object().(*v1alpha2.VirtualDisk); ok {
188+
restoredVD = obj
189+
break
190+
}
191+
}
192+
193+
Expect(restoredVD).NotTo(BeNil())
194+
if expectedSize == nil {
195+
Expect(restoredVD.Spec.PersistentVolumeClaim.Size).To(BeNil())
196+
} else {
197+
Expect(restoredVD.Spec.PersistentVolumeClaim.Size).NotTo(BeNil())
198+
Expect(restoredVD.Spec.PersistentVolumeClaim.Size.String()).To(Equal(expectedSize.String()))
199+
}
200+
},
201+
Entry("keeps explicit size from snapshot metadata", ptrToQuantity("20Gi"), ptrToQuantity("20Gi")),
202+
Entry("leaves size empty when it was not set originally", nil, nil),
203+
)
108204
})
205+
206+
func ptrToQuantity(value string) *resource.Quantity {
207+
q := resource.MustParse(value)
208+
return &q
209+
}

‎images/virtualization-artifact/pkg/controller/vdsnapshot/internal/life_cycle.go‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -335,6 +335,10 @@ func (h LifeCycleHandler) Handle(ctx context.Context, vdSnapshot *v1alpha2.Virtu
335335
}
336336
}
337337

338+
if vd.Spec.PersistentVolumeClaim.Size != nil {
339+
anno[annotations.AnnVirtualDiskRequestedSize] = vd.Spec.PersistentVolumeClaim.Size.String()
340+
}
341+
338342
for _, ownerRef := range vd.OwnerReferences {
339343
if ownerRef.Kind == v1alpha2.VirtualMachineKind {
340344
anno[annotations.AnnVirtualDiskHadOwnerReference] = "true"

‎images/virtualization-artifact/pkg/controller/vdsnapshot/internal/life_cycle_test.go‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,10 +23,12 @@ import (
2323
. "github.com/onsi/ginkgo/v2"
2424
. "github.com/onsi/gomega"
2525
corev1 "k8s.io/api/core/v1"
26+
"k8s.io/apimachinery/pkg/api/resource"
2627
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
2728
"k8s.io/utils/ptr"
2829
virtv1 "kubevirt.io/api/core/v1"
2930

31+
"github.com/deckhouse/virtualization-controller/pkg/common/annotations"
3032
"github.com/deckhouse/virtualization-controller/pkg/controller/conditions"
3133
"github.com/deckhouse/virtualization/api/core/v1alpha2"
3234
"github.com/deckhouse/virtualization/api/core/v1alpha2/vdscondition"
@@ -114,6 +116,21 @@ var _ = Describe("LifeCycle handler", func() {
114116
Expect(ready.Message).ToNot(BeEmpty())
115117
})
116118

119+
It("stores requested size in volume snapshot annotations", func() {
120+
size := resource.MustParse("20Gi")
121+
vd.Spec.PersistentVolumeClaim.Size = &size
122+
123+
snapshotter.CreateVolumeSnapshotFunc = func(_ context.Context, vs *vsv1.VolumeSnapshot) (*vsv1.VolumeSnapshot, error) {
124+
Expect(vs.Annotations[annotations.AnnVirtualDiskRequestedSize]).To(Equal("20Gi"))
125+
return vs, nil
126+
}
127+
128+
h := NewLifeCycleHandler(snapshotter)
129+
130+
_, err := h.Handle(testContext(), vdSnapshot)
131+
Expect(err).To(BeNil())
132+
})
133+
117134
It("The volume snapshot has failed", func() {
118135
snapshotter.GetVolumeSnapshotFunc = func(_ context.Context, _, _ string) (*vsv1.VolumeSnapshot, error) {
119136
vs.Status = &vsv1.VolumeSnapshotStatus{

0 commit comments

Comments
 (0)