Skip to content

Commit 95f11c4

Browse files
howardgaohowardgao
committed
Host assisted clone should adjust requested storage
when cloning from block to filesystem volume mode (#3900) When cloning from a source, the host cloner should check the requested size for the target tmp pvc if the tmp source pvc is of block volume mode and the target pvc is of filesystem volume mode because it should take filesystem overhead into consideration and enlarge the original request size accordingly before creating the tmp pvc. Because currently the check is missing the cloning will fail on validation. Signed-off-by: Howard Gao <[email protected]>
1 parent 4c3dc65 commit 95f11c4

File tree

5 files changed

+163
-29
lines changed

5 files changed

+163
-29
lines changed

‎pkg/controller/clone-controller.go‎

Lines changed: 2 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@ import (
1414

1515
corev1 "k8s.io/api/core/v1"
1616
k8serrors "k8s.io/apimachinery/pkg/api/errors"
17-
"k8s.io/apimachinery/pkg/api/resource"
1817
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
1918
"k8s.io/apimachinery/pkg/runtime"
2019
"k8s.io/apimachinery/pkg/types"
@@ -779,11 +778,11 @@ func ValidateCanCloneSourceAndTargetSpec(ctx context.Context, c client.Client, s
779778
}
780779

781780
// TODO: use detection pod here, then permissive should not be needed
782-
sourceUsableSpace, err := getUsableSpace(ctx, c, sourcePvc)
781+
sourceUsableSpace, err := cc.GetUsableSpace(ctx, c, sourcePvc)
783782
if err != nil {
784783
return err
785784
}
786-
targetUsableSpace, err := getUsableSpace(ctx, c, targetPvc)
785+
targetUsableSpace, err := cc.GetUsableSpace(ctx, c, targetPvc)
787786
if err != nil {
788787
return err
789788
}
@@ -795,21 +794,3 @@ func ValidateCanCloneSourceAndTargetSpec(ctx context.Context, c client.Client, s
795794
// Can clone.
796795
return nil
797796
}
798-
799-
func getUsableSpace(ctx context.Context, c client.Client, pvc *corev1.PersistentVolumeClaim) (resource.Quantity, error) {
800-
sizeRequest := pvc.Spec.Resources.Requests[corev1.ResourceStorage]
801-
volumeMode := util.ResolveVolumeMode(pvc.Spec.VolumeMode)
802-
803-
if volumeMode == corev1.PersistentVolumeFilesystem {
804-
fsOverhead, err := cc.GetFilesystemOverheadForStorageClass(ctx, c, pvc.Spec.StorageClassName)
805-
if err != nil {
806-
return resource.Quantity{}, err
807-
}
808-
fsOverheadFloat, _ := strconv.ParseFloat(string(fsOverhead), 64)
809-
usableSpaceRaw := util.GetUsableSpace(fsOverheadFloat, sizeRequest.Value())
810-
811-
return *resource.NewScaledQuantity(usableSpaceRaw, 0), nil
812-
}
813-
814-
return sizeRequest, nil
815-
}

‎pkg/controller/clone/BUILD.bazel‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,7 @@ go_test(
5555
],
5656
embed = [":go_default_library"],
5757
deps = [
58+
"//pkg/common:go_default_library",
5859
"//pkg/controller/common:go_default_library",
5960
"//staging/src/kubevirt.io/containerized-data-importer-api/pkg/apis/core/v1beta1:go_default_library",
6061
"//vendor/github.com/kubernetes-csi/external-snapshotter/client/v6/apis/volumesnapshot/v1:go_default_library",

‎pkg/controller/clone/host-clone.go‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -195,6 +195,42 @@ func (p *HostClonePhase) createClaim(ctx context.Context) (*corev1.PersistentVol
195195
}
196196
cc.AddLabel(claim, cc.LabelExcludeFromVeleroBackup, "true")
197197

198+
if myVolumeMode := cc.GetVolumeMode(claim); myVolumeMode == corev1.PersistentVolumeFilesystem {
199+
// It is possible when the source pvc has VolumMode 'block'
200+
// and the claim has 'filesystem' in which case the filesystem overhead need to be considered
201+
sourcePvc := &corev1.PersistentVolumeClaim{}
202+
sourcePvcKey := client.ObjectKey{Namespace: p.Namespace, Name: p.SourceName}
203+
204+
if err := p.Client.Get(ctx, sourcePvcKey, sourcePvc); err != nil {
205+
return nil, err
206+
}
207+
208+
realSourcePvcSizeRequest := sourcePvc.Spec.Resources.Requests[corev1.ResourceStorage]
209+
210+
if claim.Spec.Resources.Requests == nil {
211+
claim.Spec.Resources.Requests = make(corev1.ResourceList)
212+
}
213+
214+
if sourceVolumeMode := cc.GetVolumeMode(sourcePvc); sourceVolumeMode == corev1.PersistentVolumeFilesystem {
215+
// If the source PVC is fs, just copy the size request
216+
claim.Spec.Resources.Requests[corev1.ResourceStorage] = realSourcePvcSizeRequest
217+
} else {
218+
// If the source PVC is block, we need to account for the overhead
219+
usableSpace, err := cc.GetUsableSpace(ctx, p.Client, claim)
220+
221+
if err != nil {
222+
return nil, err
223+
}
224+
if usableSpace.Cmp(realSourcePvcSizeRequest) < 0 {
225+
newUsableSpace, err := cc.InflateSizeWithOverhead(ctx, p.Client, realSourcePvcSizeRequest.Value(), &claim.Spec)
226+
if err != nil {
227+
return nil, err
228+
}
229+
claim.Spec.Resources.Requests[corev1.ResourceStorage] = newUsableSpace
230+
}
231+
}
232+
}
233+
198234
if err := p.Client.Create(ctx, claim); err != nil {
199235
checkQuotaExceeded(p.Recorder, p.Owner, err)
200236
return nil, err

‎pkg/controller/clone/host-clone_test.go‎

Lines changed: 106 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import (
2323
. "github.com/onsi/gomega"
2424

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/apimachinery/pkg/runtime"
2829
"k8s.io/client-go/kubernetes/scheme"
@@ -33,18 +34,37 @@ import (
3334
logf "sigs.k8s.io/controller-runtime/pkg/log"
3435

3536
cdiv1 "kubevirt.io/containerized-data-importer-api/pkg/apis/core/v1beta1"
37+
"kubevirt.io/containerized-data-importer/pkg/common"
3638
cc "kubevirt.io/containerized-data-importer/pkg/controller/common"
3739
)
3840

3941
var _ = Describe("HostClonePhase test", func() {
4042
log := logf.Log.WithName("host-clone-phase-test")
4143

42-
creatHostClonePhase := func(objects ...runtime.Object) *HostClonePhase {
44+
type ResourceModifier struct {
45+
modifySourcePvc func(pvcSpec *corev1.PersistentVolumeClaimSpec)
46+
modifyDesiredPvc func(pvcSpec *corev1.PersistentVolumeClaimSpec)
47+
}
48+
49+
creatHostClonePhase := func(modifier *ResourceModifier, objects ...runtime.Object) *HostClonePhase {
4350
s := scheme.Scheme
4451
_ = cdiv1.AddToScheme(s)
4552

4653
objects = append(objects, cc.MakeEmptyCDICR())
4754

55+
source := &corev1.PersistentVolumeClaim{
56+
ObjectMeta: metav1.ObjectMeta{
57+
Namespace: "ns",
58+
Name: "source",
59+
},
60+
}
61+
62+
if modifier != nil && modifier.modifySourcePvc != nil {
63+
modifier.modifySourcePvc(&source.Spec)
64+
}
65+
66+
objects = append(objects, source)
67+
4868
// Create a fake client to mock API calls.
4969
builder := fake.NewClientBuilder().
5070
WithScheme(s).
@@ -69,11 +89,15 @@ var _ = Describe("HostClonePhase test", func() {
6989
},
7090
}
7191

92+
if modifier != nil && modifier.modifyDesiredPvc != nil {
93+
modifier.modifyDesiredPvc(&desired.Spec)
94+
}
95+
7296
return &HostClonePhase{
7397
Owner: owner,
7498
OwnershipLabel: "label",
7599
Namespace: "ns",
76-
SourceName: "source",
100+
SourceName: source.Name,
77101
DesiredClaim: desired,
78102
ImmediateBind: true,
79103
Preallocation: false,
@@ -91,7 +115,7 @@ var _ = Describe("HostClonePhase test", func() {
91115
}
92116

93117
It("should create pvc", func() {
94-
p := creatHostClonePhase()
118+
p := creatHostClonePhase(nil)
95119

96120
result, err := p.Reconcile(context.Background())
97121
Expect(err).ToNot(HaveOccurred())
@@ -114,7 +138,7 @@ var _ = Describe("HostClonePhase test", func() {
114138
})
115139

116140
It("should create pvc with priorityclass", func() {
117-
p := creatHostClonePhase()
141+
p := creatHostClonePhase(nil)
118142
p.PriorityClassName = "priority"
119143

120144
result, err := p.Reconcile(context.Background())
@@ -127,6 +151,80 @@ var _ = Describe("HostClonePhase test", func() {
127151
Expect(pvc.Annotations[cc.AnnPriorityClassName]).To(Equal("priority"))
128152
})
129153

154+
It("should adjust requested size for block to filesystem volume mode clone", func() {
155+
setPvcAttributes := func(pvcSpec *corev1.PersistentVolumeClaimSpec, volumeMode corev1.PersistentVolumeMode, storage string) {
156+
pvcSpec.VolumeMode = &volumeMode
157+
if pvcSpec.Resources.Requests == nil {
158+
pvcSpec.Resources.Requests = corev1.ResourceList{}
159+
}
160+
pvcSpec.Resources.Requests[corev1.ResourceStorage] = resource.MustParse(storage)
161+
}
162+
cdiConfig := cc.MakeEmptyCDIConfigSpec(common.ConfigName)
163+
cdiConfig.Status.FilesystemOverhead = &cdiv1.FilesystemOverhead{
164+
Global: common.DefaultGlobalOverhead,
165+
}
166+
167+
p := creatHostClonePhase(&ResourceModifier{
168+
modifySourcePvc: func(pvcSpec *corev1.PersistentVolumeClaimSpec) {
169+
setPvcAttributes(pvcSpec, corev1.PersistentVolumeBlock, "8Gi")
170+
},
171+
modifyDesiredPvc: func(pvcSpec *corev1.PersistentVolumeClaimSpec) {
172+
setPvcAttributes(pvcSpec, corev1.PersistentVolumeFilesystem, "8Gi")
173+
},
174+
}, cdiConfig)
175+
176+
result, err := p.Reconcile(context.Background())
177+
Expect(err).ToNot(HaveOccurred())
178+
Expect(result).ToNot(BeNil())
179+
Expect(result.Requeue).To(BeFalse())
180+
Expect(result.RequeueAfter).ToNot(BeZero())
181+
182+
pvc := getDesiredClaim(p)
183+
184+
Expect(*pvc.Spec.VolumeMode).To(Equal(corev1.PersistentVolumeFilesystem))
185+
actualSize := pvc.Spec.Resources.Requests[corev1.ResourceStorage]
186+
originalRequested := resource.MustParse("8Gi")
187+
188+
Expect(actualSize.Cmp(originalRequested)).To(BeNumerically(">", 0), "The actual should be greater than the requested", "actual", actualSize, "requested", originalRequested)
189+
})
190+
191+
It("should use the same requested size for filesystem to filesystem volume mode clone", func() {
192+
setPvcAttributes := func(pvcSpec *corev1.PersistentVolumeClaimSpec, volumeMode corev1.PersistentVolumeMode, storage string) {
193+
pvcSpec.VolumeMode = &volumeMode
194+
if pvcSpec.Resources.Requests == nil {
195+
pvcSpec.Resources.Requests = corev1.ResourceList{}
196+
}
197+
pvcSpec.Resources.Requests[corev1.ResourceStorage] = resource.MustParse(storage)
198+
}
199+
cdiConfig := cc.MakeEmptyCDIConfigSpec(common.ConfigName)
200+
cdiConfig.Status.FilesystemOverhead = &cdiv1.FilesystemOverhead{
201+
Global: common.DefaultGlobalOverhead,
202+
}
203+
204+
p := creatHostClonePhase(&ResourceModifier{
205+
modifySourcePvc: func(pvcSpec *corev1.PersistentVolumeClaimSpec) {
206+
setPvcAttributes(pvcSpec, corev1.PersistentVolumeFilesystem, "8Gi")
207+
},
208+
modifyDesiredPvc: func(pvcSpec *corev1.PersistentVolumeClaimSpec) {
209+
setPvcAttributes(pvcSpec, corev1.PersistentVolumeFilesystem, "8Gi")
210+
},
211+
}, cdiConfig)
212+
213+
result, err := p.Reconcile(context.Background())
214+
Expect(err).ToNot(HaveOccurred())
215+
Expect(result).ToNot(BeNil())
216+
Expect(result.Requeue).To(BeFalse())
217+
Expect(result.RequeueAfter).ToNot(BeZero())
218+
219+
pvc := getDesiredClaim(p)
220+
221+
Expect(*pvc.Spec.VolumeMode).To(Equal(corev1.PersistentVolumeFilesystem))
222+
actualSize := pvc.Spec.Resources.Requests[corev1.ResourceStorage]
223+
originalRequested := resource.MustParse("8Gi")
224+
225+
Expect(actualSize.Cmp(originalRequested)).To(BeNumerically("==", 0), "The actual should be equal to the requested", "actual", actualSize, "requested", originalRequested)
226+
})
227+
130228
Context("with desired claim created", func() {
131229
getCliam := func() *corev1.PersistentVolumeClaim {
132230
return &corev1.PersistentVolumeClaim{
@@ -141,7 +239,7 @@ var _ = Describe("HostClonePhase test", func() {
141239
It("should wait for clone to succeed", func() {
142240
desired := getCliam()
143241
desired.Annotations[cc.AnnPodPhase] = "Running"
144-
p := creatHostClonePhase(desired)
242+
p := creatHostClonePhase(nil, desired)
145243

146244
result, err := p.Reconcile(context.Background())
147245
Expect(err).ToNot(HaveOccurred())
@@ -153,7 +251,7 @@ var _ = Describe("HostClonePhase test", func() {
153251
It("should wait for clone to succeed with preallocation", func() {
154252
desired := getCliam()
155253
desired.Annotations[cc.AnnPodPhase] = "Succeeded"
156-
p := creatHostClonePhase(desired)
254+
p := creatHostClonePhase(nil, desired)
157255
p.Preallocation = true
158256

159257
result, err := p.Reconcile(context.Background())
@@ -166,7 +264,7 @@ var _ = Describe("HostClonePhase test", func() {
166264
It("should succeed", func() {
167265
desired := getCliam()
168266
desired.Annotations[cc.AnnPodPhase] = "Succeeded"
169-
p := creatHostClonePhase(desired)
267+
p := creatHostClonePhase(nil, desired)
170268

171269
result, err := p.Reconcile(context.Background())
172270
Expect(err).ToNot(HaveOccurred())
@@ -177,7 +275,7 @@ var _ = Describe("HostClonePhase test", func() {
177275
desired := getCliam()
178276
desired.Annotations[cc.AnnPodPhase] = "Succeeded"
179277
desired.Annotations[cc.AnnPreallocationApplied] = "true"
180-
p := creatHostClonePhase(desired)
278+
p := creatHostClonePhase(nil, desired)
181279
p.Preallocation = true
182280

183281
result, err := p.Reconcile(context.Background())

‎pkg/controller/common/util.go‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2219,3 +2219,21 @@ func UpdatePVCBoundContionFromEvents(pvc *corev1.PersistentVolumeClaim, c client
22192219

22202220
return nil
22212221
}
2222+
2223+
func GetUsableSpace(ctx context.Context, c client.Client, pvc *corev1.PersistentVolumeClaim) (resource.Quantity, error) {
2224+
sizeRequest := pvc.Spec.Resources.Requests[corev1.ResourceStorage]
2225+
volumeMode := util.ResolveVolumeMode(pvc.Spec.VolumeMode)
2226+
2227+
if volumeMode == corev1.PersistentVolumeFilesystem {
2228+
fsOverhead, err := GetFilesystemOverheadForStorageClass(ctx, c, pvc.Spec.StorageClassName)
2229+
if err != nil {
2230+
return resource.Quantity{}, err
2231+
}
2232+
fsOverheadFloat, _ := strconv.ParseFloat(string(fsOverhead), 64)
2233+
usableSpaceRaw := util.GetUsableSpace(fsOverheadFloat, sizeRequest.Value())
2234+
2235+
return *resource.NewScaledQuantity(usableSpaceRaw, 0), nil
2236+
}
2237+
2238+
return sizeRequest, nil
2239+
}

0 commit comments

Comments
 (0)