Merge pull request #52659 from gnufied/fix-resize-plugin-size-check

Automatic merge from submit-queue (batch tested with PRs 52350, 52659). If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>..

Resize plugin should only check for increase in size

We should only check if user is trying to increase the volume.

Fixes https://github.com/kubernetes/kubernetes/issues/52658

Ideally - I would request this to be cherry picked for 1.8, but I understand if this is too late in the process.  I also messed up somewhat by not spotting this. :(

/sig storage 

cc @kubernetes/sig-storage-pr-reviews

plugin/pkg/admission/persistentvolume/resize/admission.go

@@ -98,6 +98,17 @@ func (pvcr *persistentVolumeClaimResize) Admit(a admission.Attributes) error {
 		return nil
 	}
 
+	oldSize := oldPvc.Spec.Resources.Requests[api.ResourceStorage]
+	newSize := pvc.Spec.Resources.Requests[api.ResourceStorage]
+
+	if newSize.Cmp(oldSize) <= 0 {
+		return nil
+	}
+
+	if oldPvc.Status.Phase != api.ClaimBound {
+		return admission.NewForbidden(a, fmt.Errorf("Only bound persistent volume claims can be expanded"))
+	}
+
 	// Growing Persistent volumes is only allowed for PVCs for which their StorageClass
 	// explicitly allows it
 	if !pvcr.allowResize(pvc, oldPvc) {
@@ -108,13 +119,12 @@ func (pvcr *persistentVolumeClaimResize) Admit(a admission.Attributes) error {
 	// volume plugin must support resize
 	pv, err := pvcr.pvLister.Get(pvc.Spec.VolumeName)
 	if err != nil {
-		return nil
+		return admission.NewForbidden(a, fmt.Errorf("Error updating persistent volume claim because fetching associated persistent volume failed"))
 	}
 
 	if !pvcr.checkVolumePlugin(pv) {
 		return admission.NewForbidden(a, fmt.Errorf("volume plugin does not support resize"))
 	}
-
 	return nil
 }
 

plugin/pkg/admission/persistentvolume/resize/admission_test.go

@@ -97,6 +97,7 @@ func TestPVCResizeAdmission(t *testing.T) {
 				},
 				Status: api.PersistentVolumeClaimStatus{
 					Capacity: getResourceList("1Gi"),
+					Phase:    api.ClaimBound,
 				},
 			},
 			newObj: &api.PersistentVolumeClaim{
@@ -109,6 +110,7 @@ func TestPVCResizeAdmission(t *testing.T) {
 				},
 				Status: api.PersistentVolumeClaimStatus{
 					Capacity: getResourceList("2Gi"),
+					Phase:    api.ClaimBound,
 				},
 			},
 			checkError: expectNoError,
@@ -126,6 +128,7 @@ func TestPVCResizeAdmission(t *testing.T) {
 				},
 				Status: api.PersistentVolumeClaimStatus{
 					Capacity: getResourceList("1Gi"),
+					Phase:    api.ClaimBound,
 				},
 			},
 			newObj: &api.PersistentVolumeClaim{
@@ -138,6 +141,7 @@ func TestPVCResizeAdmission(t *testing.T) {
 				},
 				Status: api.PersistentVolumeClaimStatus{
 					Capacity: getResourceList("2Gi"),
+					Phase:    api.ClaimBound,
 				},
 			},
 			checkError: expectVolumePluginError,
@@ -154,6 +158,7 @@ func TestPVCResizeAdmission(t *testing.T) {
 				},
 				Status: api.PersistentVolumeClaimStatus{
 					Capacity: getResourceList("1Gi"),
+					Phase:    api.ClaimBound,
 				},
 			},
 			newObj: &api.PersistentVolumeClaim{
@@ -165,6 +170,7 @@ func TestPVCResizeAdmission(t *testing.T) {
 				},
 				Status: api.PersistentVolumeClaimStatus{
 					Capacity: getResourceList("2Gi"),
+					Phase:    api.ClaimBound,
 				},
 			},
 			checkError: expectDynamicallyProvisionedError,
@@ -182,6 +188,7 @@ func TestPVCResizeAdmission(t *testing.T) {
 				},
 				Status: api.PersistentVolumeClaimStatus{
 					Capacity: getResourceList("1Gi"),
+					Phase:    api.ClaimBound,
 				},
 			},
 			newObj: &api.PersistentVolumeClaim{
@@ -194,10 +201,72 @@ func TestPVCResizeAdmission(t *testing.T) {
 				},
 				Status: api.PersistentVolumeClaimStatus{
 					Capacity: getResourceList("2Gi"),
+					Phase:    api.ClaimBound,
 				},
 			},
 			checkError: expectDynamicallyProvisionedError,
 		},
+		{
+			name:     "PVC update with no change in size",
+			resource: api.SchemeGroupVersion.WithResource("persistentvolumeclaims"),
+			oldObj: &api.PersistentVolumeClaim{
+				Spec: api.PersistentVolumeClaimSpec{
+					Resources: api.ResourceRequirements{
+						Requests: getResourceList("1Gi"),
+					},
+					StorageClassName: &silverClassName,
+				},
+				Status: api.PersistentVolumeClaimStatus{
+					Capacity: getResourceList("0Gi"),
+					Phase:    api.ClaimPending,
+				},
+			},
+			newObj: &api.PersistentVolumeClaim{
+				Spec: api.PersistentVolumeClaimSpec{
+					VolumeName: "volume4",
+					Resources: api.ResourceRequirements{
+						Requests: getResourceList("1Gi"),
+					},
+					StorageClassName: &silverClassName,
+				},
+				Status: api.PersistentVolumeClaimStatus{
+					Capacity: getResourceList("1Gi"),
+					Phase:    api.ClaimBound,
+				},
+			},
+			checkError: expectNoError,
+		},
+		{
+			name:     "expand pvc in pending state",
+			resource: api.SchemeGroupVersion.WithResource("persistentvolumeclaims"),
+			oldObj: &api.PersistentVolumeClaim{
+				Spec: api.PersistentVolumeClaimSpec{
+					Resources: api.ResourceRequirements{
+						Requests: getResourceList("1Gi"),
+					},
+					StorageClassName: &silverClassName,
+				},
+				Status: api.PersistentVolumeClaimStatus{
+					Capacity: getResourceList("0Gi"),
+					Phase:    api.ClaimPending,
+				},
+			},
+			newObj: &api.PersistentVolumeClaim{
+				Spec: api.PersistentVolumeClaimSpec{
+					Resources: api.ResourceRequirements{
+						Requests: getResourceList("2Gi"),
+					},
+					StorageClassName: &silverClassName,
+				},
+				Status: api.PersistentVolumeClaimStatus{
+					Capacity: getResourceList("0Gi"),
+					Phase:    api.ClaimPending,
+				},
+			},
+			checkError: func(err error) bool {
+				return strings.Contains(err.Error(), "Only bound persistent volume claims can be expanded")
+			},
+		},
 	}
 
 	ctrl := newPlugin()