Skip to content

Commit 6a01cfa

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 d71a929 commit 6a01cfa

File tree

5 files changed

+117
-29
lines changed

5 files changed

+117
-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: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -195,6 +195,31 @@ 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+
usableSpace, err := cc.GetUsableSpace(ctx, p.Client, claim)
210+
211+
if err != nil {
212+
return nil, err
213+
}
214+
if usableSpace.Cmp(realSourcePvcSizeRequest) < 0 {
215+
newUsableSpace, err := cc.InflateSizeWithOverhead(ctx, p.Client, realSourcePvcSizeRequest.Value(), &claim.Spec)
216+
if err != nil {
217+
return nil, err
218+
}
219+
claim.Spec.Resources.Requests[corev1.ResourceStorage] = newUsableSpace
220+
}
221+
}
222+
198223
if err := p.Client.Create(ctx, claim); err != nil {
199224
checkQuotaExceeded(p.Recorder, p.Owner, err)
200225
return nil, err

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

Lines changed: 71 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,45 @@ var _ = Describe("HostClonePhase test", func() {
127151
Expect(pvc.Annotations[cc.AnnPriorityClassName]).To(Equal("priority"))
128152
})
129153

154+
It("should adjust requested size for filesystem volume mode", 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+
// fakeCs := "hostpath-csi"
174+
// pvcSpec.StorageClassName = &fakeCs
175+
},
176+
}, cdiConfig)
177+
178+
result, err := p.Reconcile(context.Background())
179+
Expect(err).ToNot(HaveOccurred())
180+
Expect(result).ToNot(BeNil())
181+
Expect(result.Requeue).To(BeFalse())
182+
Expect(result.RequeueAfter).ToNot(BeZero())
183+
184+
pvc := getDesiredClaim(p)
185+
186+
Expect(*pvc.Spec.VolumeMode).To(Equal(corev1.PersistentVolumeFilesystem))
187+
actualSize := pvc.Spec.Resources.Requests[corev1.ResourceStorage]
188+
originalRequested := resource.MustParse("8Gi")
189+
190+
Expect(actualSize.Cmp(originalRequested)).To(BeNumerically(">", 0), "The actual should be greater than the requested", "actual", actualSize, "requested", originalRequested)
191+
})
192+
130193
Context("with desired claim created", func() {
131194
getCliam := func() *corev1.PersistentVolumeClaim {
132195
return &corev1.PersistentVolumeClaim{
@@ -141,7 +204,7 @@ var _ = Describe("HostClonePhase test", func() {
141204
It("should wait for clone to succeed", func() {
142205
desired := getCliam()
143206
desired.Annotations[cc.AnnPodPhase] = "Running"
144-
p := creatHostClonePhase(desired)
207+
p := creatHostClonePhase(nil, desired)
145208

146209
result, err := p.Reconcile(context.Background())
147210
Expect(err).ToNot(HaveOccurred())
@@ -153,7 +216,7 @@ var _ = Describe("HostClonePhase test", func() {
153216
It("should wait for clone to succeed with preallocation", func() {
154217
desired := getCliam()
155218
desired.Annotations[cc.AnnPodPhase] = "Succeeded"
156-
p := creatHostClonePhase(desired)
219+
p := creatHostClonePhase(nil, desired)
157220
p.Preallocation = true
158221

159222
result, err := p.Reconcile(context.Background())
@@ -166,7 +229,7 @@ var _ = Describe("HostClonePhase test", func() {
166229
It("should succeed", func() {
167230
desired := getCliam()
168231
desired.Annotations[cc.AnnPodPhase] = "Succeeded"
169-
p := creatHostClonePhase(desired)
232+
p := creatHostClonePhase(nil, desired)
170233

171234
result, err := p.Reconcile(context.Background())
172235
Expect(err).ToNot(HaveOccurred())
@@ -177,7 +240,7 @@ var _ = Describe("HostClonePhase test", func() {
177240
desired := getCliam()
178241
desired.Annotations[cc.AnnPodPhase] = "Succeeded"
179242
desired.Annotations[cc.AnnPreallocationApplied] = "true"
180-
p := creatHostClonePhase(desired)
243+
p := creatHostClonePhase(nil, desired)
181244
p.Preallocation = true
182245

183246
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)