Skip to content

Commit

Permalink
fix(volume): fix panic bug when enable ModifyVolume feature
Browse files Browse the repository at this point in the history
  • Loading branch information
liubog2008 committed Jun 9, 2023
1 parent 0cd7c85 commit 17ca051
Show file tree
Hide file tree
Showing 4 changed files with 140 additions and 120 deletions.
55 changes: 30 additions & 25 deletions pkg/manager/volumes/phase.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,9 +71,7 @@ func (p VolumePhase) String() string {

func (p *podVolModifier) getVolumePhase(vol *ActualVolume) VolumePhase {
if err := p.validate(vol); err != nil {
if !errors.Is(err, ErrChangeDefaultStorageClass) {
klog.Warningf("volume %s/%s modification is not allowed: %v", vol.PVC.Namespace, vol.PVC.Name, err)
}
klog.Warningf("volume %s/%s modification is not allowed: %v", vol.PVC.Namespace, vol.PVC.Name, err)
return VolumePhaseCannotModify
}
if isPVCRevisionChanged(vol.PVC) {
Expand All @@ -91,30 +89,34 @@ func (p *podVolModifier) getVolumePhase(vol *ActualVolume) VolumePhase {
return VolumePhasePreparing
}

func isVolumeExpansionSupported(sc *storagev1.StorageClass) bool {
func isVolumeExpansionSupported(sc *storagev1.StorageClass) (bool, error) {
if sc == nil {
// always assume expansion is supported
return true, fmt.Errorf("expansion cap of volume is unknown")
}
if sc.AllowVolumeExpansion == nil {
return false
return false, nil
}
return *sc.AllowVolumeExpansion
return *sc.AllowVolumeExpansion, nil
}

func (p *podVolModifier) validate(vol *ActualVolume) error {
if vol.Desired == nil {
return fmt.Errorf("can't match desired volume")
}
if vol.Desired.StorageClass == nil {
// TODO: support default storage class
return ErrChangeDefaultStorageClass
}
desired := vol.Desired.Size
actual := getStorageSize(vol.PVC.Spec.Resources.Requests)
desired := vol.Desired.GetStorageSize()
actual := vol.GetStorageSize()
result := desired.Cmp(actual)
switch {
case result == 0:
case result < 0:
return fmt.Errorf("can't shrunk size from %s to %s", &actual, &desired)
case result > 0:
if !isVolumeExpansionSupported(vol.StorageClass) {
supported, err := isVolumeExpansionSupported(vol.StorageClass)
if err != nil {
klog.Warningf("volume expansion of storage class %s may be not supported, but it will be tried", vol.GetStorageClassName())
}
if !supported {
return fmt.Errorf("volume expansion is not supported by storageclass %s", vol.StorageClass.Name)
}
}
Expand All @@ -125,6 +127,11 @@ func (p *podVolModifier) validate(vol *ActualVolume) error {
desiredPVC := vol.PVC.DeepCopy()
desiredPVC.Spec.Resources.Requests[corev1.ResourceStorage] = desired

// if no pv permission but have sc permission: cannot change sc
if isStorageClassChanged(vol.GetStorageClassName(), vol.Desired.GetStorageClassName()) && vol.PV == nil {
return fmt.Errorf("cannot change storage class (%s to %s), because there is no permission to get persistent volume", vol.GetStorageClassName(), vol.Desired.GetStorageClassName())
}

return m.Validate(vol.PVC, desiredPVC, vol.StorageClass, vol.Desired.StorageClass)
}

Expand Down Expand Up @@ -163,23 +170,14 @@ func (p *podVolModifier) waitForNextTime(pvc *corev1.PersistentVolumeClaim, sc *

func needModify(pvc *corev1.PersistentVolumeClaim, desired *DesiredVolume) bool {
size := desired.Size
scName := ""
if desired.StorageClass != nil {
scName = desired.StorageClass.Name
}
scName := desired.GetStorageClassName()

return isPVCStatusMatched(pvc, scName, size)
}

func isPVCStatusMatched(pvc *corev1.PersistentVolumeClaim, scName string, size resource.Quantity) bool {
isChanged := false
oldSc, ok := pvc.Annotations[annoKeyPVCStatusStorageClass]
if !ok {
oldSc = ignoreNil(pvc.Spec.StorageClassName)
}
if oldSc != scName {
isChanged = true
}
oldSc := getStorageClassNameFromPVC(pvc)
isChanged := isStorageClassChanged(oldSc, scName)

oldSize, ok := pvc.Annotations[annoKeyPVCStatusStorageSize]
if !ok {
Expand All @@ -195,3 +193,10 @@ func isPVCStatusMatched(pvc *corev1.PersistentVolumeClaim, scName string, size r

return isChanged
}

func isStorageClassChanged(pre, cur string) bool {
if cur != "" && pre != cur {
return true
}
return false
}
Loading

0 comments on commit 17ca051

Please sign in to comment.