Skip to content

Commit 0a14f02

Browse files
sunpa93sunpa93
committed
fix: add annotation to pv after resize if filesystem resize is
necessary. delete when pvc.status.capacity >= pv.spec.capacity
1 parent 132dd95 commit 0a14f02

File tree

3 files changed

+121
-25
lines changed

3 files changed

+121
-25
lines changed

‎pkg/controller/controller.go

Lines changed: 53 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -172,8 +172,11 @@ func (ctrl *resizeController) updatePVC(oldObj, newObj interface{}) {
172172
return
173173
}
174174

175-
newSize := newPVC.Spec.Resources.Requests[v1.ResourceStorage]
176-
oldSize := oldPVC.Spec.Resources.Requests[v1.ResourceStorage]
175+
newReq := newPVC.Spec.Resources.Requests[v1.ResourceStorage]
176+
oldReq := oldPVC.Spec.Resources.Requests[v1.ResourceStorage]
177+
178+
newCap := newPVC.Status.Capacity[v1.ResourceStorage]
179+
oldCap := oldPVC.Status.Capacity[v1.ResourceStorage]
177180

178181
newResizerName := newPVC.Annotations[util.VolumeResizerKey]
179182
oldResizerName := oldPVC.Annotations[util.VolumeResizerKey]
@@ -192,23 +195,27 @@ func (ctrl *resizeController) updatePVC(oldObj, newObj interface{}) {
192195
// know how to support resizing of a "un-annotated" in-tree PVC. When in-tree resizer does add the annotation, a second
193196
// update even will be received and we add the pvc to workqueue. If annotation matches the registered driver name in
194197
// csi_resizer object, we proceeds with expansion internally or we discard the PVC.
195-
// 2. An already expanded in-tree PVC:
198+
// 3. An already expanded in-tree PVC:
196199
// An in-tree PVC is resized with in-tree resizer. And later, CSI migration is turned on and resizer name is updated from
197200
// in-tree resizer name to CSI driver name.
198-
if newSize.Cmp(oldSize) > 0 || newResizerName != oldResizerName {
201+
if newReq.Cmp(oldReq) > 0 || newResizerName != oldResizerName {
199202
ctrl.addPVC(newObj)
200203
} else {
201204
// PVC's size not changed, so this Update event maybe caused by:
202205
//
203206
// 1. Administrators or users introduce other changes(such as add labels, modify annotations, etc.)
204207
// unrelated to volume resize.
205208
// 2. Informer resynced the PVC and send this Update event without any changes.
209+
// 3. PV's filesystem has recently been resized and requires removal of its resize annotation
206210
//
207-
// If it is case 1, we can just discard this event. If case 2, we need to put it into the queue to
211+
// If it is case 1, we can just discard this event. If case 2 or 3, we need to put it into the queue to
208212
// perform a resync operation.
209213
if newPVC.ResourceVersion == oldPVC.ResourceVersion {
210214
// This is case 2.
211215
ctrl.addPVC(newObj)
216+
} else if newCap.Cmp(oldCap) > 0 {
217+
// This is case 3
218+
ctrl.addPVC(newObj)
212219
}
213220
}
214221
}
@@ -300,16 +307,10 @@ func (ctrl *resizeController) syncPVC(key string) error {
300307
return fmt.Errorf("expected PVC got: %v", pvcObject)
301308
}
302309

303-
if !ctrl.pvcNeedResize(pvc) {
304-
klog.V(4).Infof("No need to resize PVC %q", util.PVCKey(pvc))
305-
return nil
306-
}
307-
308310
volumeObj, exists, err := ctrl.volumes.GetByKey(pvc.Spec.VolumeName)
309311
if err != nil {
310312
return fmt.Errorf("Get PV %q of pvc %q failed: %v", pvc.Spec.VolumeName, util.PVCKey(pvc), err)
311313
}
312-
313314
if !exists {
314315
klog.Warningf("PV %q bound to PVC %s not found", pvc.Spec.VolumeName, util.PVCKey(pvc))
315316
return nil
@@ -320,6 +321,16 @@ func (ctrl *resizeController) syncPVC(key string) error {
320321
return fmt.Errorf("expected volume but got %+v", volumeObj)
321322
}
322323

324+
if ctrl.isNodeExpandComplete(pvc, pv) && metav1.HasAnnotation(pv.ObjectMeta, util.AnnPreResizeCapacity) {
325+
if err := ctrl.deletePreResizeCapAnnotation(pv); err != nil {
326+
return fmt.Errorf("failed removing annotation %s from pv %q: %v", util.AnnPreResizeCapacity, pv.Name, err)
327+
}
328+
}
329+
330+
if !ctrl.pvcNeedResize(pvc) {
331+
klog.V(4).Infof("No need to resize PVC %q", util.PVCKey(pvc))
332+
return nil
333+
}
323334
if !ctrl.pvNeedResize(pvc, pv) {
324335
klog.V(4).Infof("No need to resize PV %q", pv.Name)
325336
return nil
@@ -376,6 +387,13 @@ func (ctrl *resizeController) pvNeedResize(pvc *v1.PersistentVolumeClaim, pv *v1
376387
return true
377388
}
378389

390+
// isNodeExpandComplete returns true if pvc.Status.Capacity >= pv.Spec.Capacity
391+
func (ctrl *resizeController) isNodeExpandComplete(pvc *v1.PersistentVolumeClaim, pv *v1.PersistentVolume) bool {
392+
klog.V(4).Infof("pv %q capacity = %v, pvc %s capacity = %v", pv.Name, pv.Spec.Capacity[v1.ResourceStorage], util.PVCKey(pvc), pvc.Status.Capacity[v1.ResourceStorage])
393+
pvcCap, pvCap := pvc.Status.Capacity[v1.ResourceStorage], pv.Spec.Capacity[v1.ResourceStorage]
394+
return pvcCap.Cmp(pvCap) >= 0
395+
}
396+
379397
// resizePVC will:
380398
// 1. Mark pvc as resizing.
381399
// 2. Resize the volume and the pv object.
@@ -446,7 +464,7 @@ func (ctrl *resizeController) resizeVolume(
446464
}
447465
klog.V(4).Infof("Resize volume succeeded for volume %q, start to update PV's capacity", pv.Name)
448466

449-
err = ctrl.updatePVCapacity(pv, newSize)
467+
err = ctrl.updatePVCapacity(pv, pvc.Status.Capacity[v1.ResourceStorage], newSize, fsResizeRequired)
450468
if err != nil {
451469
return newSize, fsResizeRequired, err
452470
}
@@ -533,11 +551,33 @@ func (ctrl *resizeController) patchClaim(oldPVC, newPVC *v1.PersistentVolumeClai
533551
return updatedClaim, nil
534552
}
535553

536-
func (ctrl *resizeController) updatePVCapacity(pv *v1.PersistentVolume, newCapacity resource.Quantity) error {
554+
func (ctrl *resizeController) deletePreResizeCapAnnotation(pv *v1.PersistentVolume) error {
555+
// if the pv does not have a resize annotation skip the entire process
556+
if !metav1.HasAnnotation(pv.ObjectMeta, util.AnnPreResizeCapacity) {
557+
return nil
558+
}
559+
pvClone := pv.DeepCopy()
560+
delete(pvClone.ObjectMeta.Annotations, util.AnnPreResizeCapacity)
561+
562+
_, err := ctrl.patchPersistentVolume(pv, pvClone)
563+
return err
564+
}
565+
566+
func (ctrl *resizeController) updatePVCapacity(pv *v1.PersistentVolume, oldCapacity, newCapacity resource.Quantity, fsResizeRequired bool) error {
537567
klog.V(4).Infof("Resize volume succeeded for volume %q, start to update PV's capacity", pv.Name)
538568
newPV := pv.DeepCopy()
539569
newPV.Spec.Capacity[v1.ResourceStorage] = newCapacity
540570

571+
if fsResizeRequired {
572+
// only update annotation if there already isn't one
573+
if !metav1.HasAnnotation(pv.ObjectMeta, util.AnnPreResizeCapacity) {
574+
if newPV.ObjectMeta.Annotations == nil {
575+
newPV.ObjectMeta.Annotations = make(map[string]string)
576+
}
577+
newPV.ObjectMeta.Annotations[util.AnnPreResizeCapacity] = oldCapacity.String()
578+
}
579+
}
580+
541581
_, err := ctrl.patchPersistentVolume(pv, newPV)
542582
if err != nil {
543583
return fmt.Errorf("updating capacity of PV %q to %s failed: %v", pv.Name, newCapacity.String(), err)

‎pkg/controller/controller_test.go

Lines changed: 61 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99

1010
"github.com/kubernetes-csi/external-resizer/pkg/csi"
1111
"github.com/kubernetes-csi/external-resizer/pkg/resizer"
12+
"github.com/kubernetes-csi/external-resizer/pkg/util"
1213

1314
v1 "k8s.io/api/core/v1"
1415
"k8s.io/apimachinery/pkg/api/resource"
@@ -29,10 +30,11 @@ func TestController(t *testing.T) {
2930
PVC *v1.PersistentVolumeClaim
3031
PV *v1.PersistentVolume
3132

32-
CreateObjects bool
33-
NodeResize bool
34-
CallCSIExpand bool
35-
expectBlockVolume bool
33+
CreateObjects bool
34+
NodeResize bool
35+
CallCSIExpand bool
36+
expectBlockVolume bool
37+
expectDeleteAnnotation bool
3638

3739
// is PVC being expanded in-use
3840
pvcInUse bool
@@ -95,6 +97,15 @@ func TestController(t *testing.T) {
9597
NodeResize: true,
9698
CallCSIExpand: true,
9799
},
100+
{
101+
Name: "PV nodeExpand Complete",
102+
PVC: createPVC(2, 1),
103+
PV: createPV(1, "testPVC", defaultNS, "foobar", &fsVolumeMode),
104+
CreateObjects: true,
105+
NodeResize: true,
106+
CallCSIExpand: true,
107+
expectDeleteAnnotation: true,
108+
},
98109
{
99110
Name: "Block Resize PVC with FS resize",
100111
PVC: createPVC(2, 1),
@@ -193,10 +204,12 @@ func TestController(t *testing.T) {
193204
client := csi.NewMockClient("mock", test.NodeResize, true, true)
194205
driverName, _ := client.GetDriverName(context.TODO())
195206

207+
var expectedCap resource.Quantity
196208
initialObjects := []runtime.Object{}
197209
if test.CreateObjects {
198210
if test.PVC != nil {
199211
initialObjects = append(initialObjects, test.PVC)
212+
expectedCap = test.PVC.Status.Capacity[v1.ResourceStorage]
200213
}
201214
if test.PV != nil {
202215
test.PV.Spec.PersistentVolumeSource.CSI.Driver = driverName
@@ -270,6 +283,29 @@ func TestController(t *testing.T) {
270283
if test.CallCSIExpand && !test.expectBlockVolume && usedCapability.GetMount() == nil {
271284
t.Errorf("For %s: expected mount accesstype got: %v", test.Name, usedCapability)
272285
}
286+
287+
// check if pre resize capacity annotation get properly populated after volume resize if node expand is required
288+
if test.PV != nil && test.CreateObjects {
289+
volObj, _, _ := ctrlInstance.volumes.GetByKey("testPV")
290+
pv := volObj.(*v1.PersistentVolume)
291+
requestCap, statusCap := test.PVC.Spec.Resources.Requests[v1.ResourceStorage], test.PVC.Status.Capacity[v1.ResourceStorage]
292+
if requestCap.Cmp(statusCap) > 0 && test.NodeResize && !expectedCap.IsZero() {
293+
checkPreResizeCap(t, test.Name, pv, expectedCap.String())
294+
}
295+
}
296+
297+
// check if pre resize capacity annotation gets properly deleted after node expand
298+
if test.PVC != nil && test.CreateObjects && test.expectDeleteAnnotation {
299+
ctrlInstance.markPVCResizeFinished(test.PVC, test.PVC.Spec.Resources.Requests[v1.ResourceStorage])
300+
time.Sleep(time.Second * 2)
301+
volObj, _, _ := ctrlInstance.volumes.GetByKey("testPV")
302+
pv := volObj.(*v1.PersistentVolume)
303+
if pv.ObjectMeta.Annotations != nil {
304+
if _, exists := pv.ObjectMeta.Annotations[util.AnnPreResizeCapacity]; exists {
305+
t.Errorf("For %s: expected annotation %s to be empty, but received %s", test.Name, util.AnnPreResizeCapacity, pv.ObjectMeta.Annotations[util.AnnPreResizeCapacity])
306+
}
307+
}
308+
}
273309
}
274310
}
275311

@@ -306,9 +342,11 @@ func TestResizePVC(t *testing.T) {
306342
}
307343
driverName, _ := client.GetDriverName(context.TODO())
308344

345+
var expectedCap resource.Quantity
309346
initialObjects := []runtime.Object{}
310347
if test.PVC != nil {
311348
initialObjects = append(initialObjects, test.PVC)
349+
expectedCap = test.PVC.Status.Capacity[v1.ResourceStorage]
312350
}
313351
if test.PV != nil {
314352
test.PV.Spec.PersistentVolumeSource.CSI.Driver = driverName
@@ -350,10 +388,24 @@ func TestResizePVC(t *testing.T) {
350388
t.Errorf("for %s expected error got nothing", test.Name)
351389
continue
352390
}
353-
if !test.expectFailure && err != nil {
354-
t.Errorf("for %s, unexpected error: %v", test.Name, err)
391+
if !test.expectFailure {
392+
if err != nil {
393+
t.Errorf("for %s, unexpected error: %v", test.Name, err)
394+
// check if pre resize capacity annotation gets properly populated after resize
395+
} else if test.PV != nil && test.NodeResize && !expectedCap.IsZero() {
396+
volObj, _, _ := ctrlInstance.volumes.GetByKey("testPV")
397+
pv := volObj.(*v1.PersistentVolume)
398+
checkPreResizeCap(t, test.Name, pv, expectedCap.String())
399+
}
355400
}
401+
}
402+
}
356403

404+
func checkPreResizeCap(t *testing.T, testName string, pv *v1.PersistentVolume, expectedCap string) {
405+
if pv.ObjectMeta.Annotations == nil {
406+
t.Errorf("for %s, AnnpreResizeCapacity was not successfully updated, expected: %s, received: nil Annotations", testName, expectedCap)
407+
} else if pv.ObjectMeta.Annotations[util.AnnPreResizeCapacity] != expectedCap {
408+
t.Errorf("for %s, AnnpreResizeCapacity was not successfully updated, expected: %s, received: %s", testName, expectedCap, pv.ObjectMeta.Annotations[util.AnnPreResizeCapacity])
357409
}
358410
}
359411

@@ -366,7 +418,7 @@ func invalidPVC() *v1.PersistentVolumeClaim {
366418
}
367419

368420
func quantityGB(i int) resource.Quantity {
369-
q := resource.NewQuantity(int64(i*1024*1024), resource.BinarySI)
421+
q := resource.NewQuantity(int64(i*1024*1024*1024), resource.BinarySI)
370422
return *q
371423
}
372424

@@ -402,7 +454,8 @@ func createPV(capacityGB int, pvcName, pvcNamespace string, pvcUID types.UID, vo
402454

403455
pv := &v1.PersistentVolume{
404456
ObjectMeta: metav1.ObjectMeta{
405-
Name: "testPV",
457+
Name: "testPV",
458+
Annotations: make(map[string]string),
406459
},
407460
Spec: v1.PersistentVolumeSpec{
408461
AccessModes: []v1.PersistentVolumeAccessMode{v1.ReadWriteOnce},

‎pkg/util/util.go

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -27,10 +27,13 @@ import (
2727
"k8s.io/apimachinery/pkg/util/strategicpatch"
2828
)
2929

30-
var knownResizeConditions = map[v1.PersistentVolumeClaimConditionType]bool{
31-
v1.PersistentVolumeClaimResizing: true,
32-
v1.PersistentVolumeClaimFileSystemResizePending: true,
33-
}
30+
var (
31+
knownResizeConditions = map[v1.PersistentVolumeClaimConditionType]bool{
32+
v1.PersistentVolumeClaimResizing: true,
33+
v1.PersistentVolumeClaimFileSystemResizePending: true,
34+
}
35+
AnnPreResizeCapacity = "volume.kubernetes.io/pre-resize-capacity"
36+
)
3437

3538
// PVCKey returns an unique key of a PVC object,
3639
func PVCKey(pvc *v1.PersistentVolumeClaim) string {

0 commit comments

Comments
 (0)