Merge pull request #58739 from jsafrane/fix-prebound-pvc-access
Automatic merge from submit-queue (batch tested with PRs 58661, 58764, 58368, 58739, 58773). 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>. Don't bind PVs and PVCs with different access modes. PVC pre-bound to a PV can bind to the PV only if it has correct access mode. Report an event if it does not and keep the PVC Pending. ++ minor refactoring of `syncClaim`, `isMisMatch` was declared too far away from place when it's used. /sig storage /assign @gnufied @rootfs ```release-note None ```
This commit is contained in:
		| @@ -198,6 +198,32 @@ func TestSync(t *testing.T) { | ||||
| 			newClaimArray("claim1-1", "uid1-1", "1Gi", "volume1-1", v1.ClaimBound, &classWait, annBoundByController, annBindCompleted), | ||||
| 			noevents, noerrors, testSyncClaim, | ||||
| 		}, | ||||
| 		{ | ||||
| 			// syncClaim binds pre-bound PVC only to the volume it points to, | ||||
| 			// even if there is smaller volume available | ||||
| 			"1-15 - successful prebound PVC", | ||||
| 			[]*v1.PersistentVolume{ | ||||
| 				newVolume("volume1-15_1", "10Gi", "", "", v1.VolumePending, v1.PersistentVolumeReclaimRetain, classEmpty), | ||||
| 				newVolume("volume1-15_2", "1Gi", "", "", v1.VolumePending, v1.PersistentVolumeReclaimRetain, classEmpty), | ||||
| 			}, | ||||
| 			[]*v1.PersistentVolume{ | ||||
| 				newVolume("volume1-15_1", "10Gi", "uid1-15", "claim1-15", v1.VolumeBound, v1.PersistentVolumeReclaimRetain, classEmpty, annBoundByController), | ||||
| 				newVolume("volume1-15_2", "1Gi", "", "", v1.VolumePending, v1.PersistentVolumeReclaimRetain, classEmpty), | ||||
| 			}, | ||||
| 			newClaimArray("claim1-15", "uid1-15", "1Gi", "volume1-15_1", v1.ClaimPending, nil), | ||||
| 			withExpectedCapacity("10Gi", newClaimArray("claim1-15", "uid1-15", "1Gi", "volume1-15_1", v1.ClaimBound, nil, annBindCompleted)), | ||||
| 			noevents, noerrors, testSyncClaim, | ||||
| 		}, | ||||
| 		{ | ||||
| 			// syncClaim does not bind pre-bound PVC to PV with different AccessMode | ||||
| 			"1-16 - successful prebound PVC", | ||||
| 			// PV has ReadWriteOnce | ||||
| 			newVolumeArray("volume1-16", "1Gi", "", "", v1.VolumePending, v1.PersistentVolumeReclaimRetain, classEmpty), | ||||
| 			newVolumeArray("volume1-16", "1Gi", "", "", v1.VolumePending, v1.PersistentVolumeReclaimRetain, classEmpty), | ||||
| 			claimWithAccessMode([]v1.PersistentVolumeAccessMode{v1.ReadWriteMany}, newClaimArray("claim1-16", "uid1-16", "1Gi", "volume1-16", v1.ClaimPending, nil)), | ||||
| 			claimWithAccessMode([]v1.PersistentVolumeAccessMode{v1.ReadWriteMany}, newClaimArray("claim1-16", "uid1-16", "1Gi", "volume1-16", v1.ClaimPending, nil)), | ||||
| 			noevents, noerrors, testSyncClaim, | ||||
| 		}, | ||||
|  | ||||
| 		// [Unit test set 2] User asked for a specific PV. | ||||
| 		// Test the binding when pv.ClaimRef is already set by controller or | ||||
|   | ||||
| @@ -802,6 +802,13 @@ func claimWithAnnotation(name, value string, claims []*v1.PersistentVolumeClaim) | ||||
| 	return claims | ||||
| } | ||||
|  | ||||
| // claimWithAccessMode saves given access into given claims. | ||||
| // Meant to be used to compose claims specified inline in a test. | ||||
| func claimWithAccessMode(modes []v1.PersistentVolumeAccessMode, claims []*v1.PersistentVolumeClaim) []*v1.PersistentVolumeClaim { | ||||
| 	claims[0].Spec.AccessModes = modes | ||||
| 	return claims | ||||
| } | ||||
|  | ||||
| func testSyncClaim(ctrl *PersistentVolumeController, reactor *volumeReactor, test controllerTest) error { | ||||
| 	return ctrl.syncClaim(test.initialClaims[0]) | ||||
| } | ||||
|   | ||||
| @@ -233,11 +233,6 @@ func (ctrl *PersistentVolumeController) syncClaim(claim *v1.PersistentVolumeClai | ||||
| func checkVolumeSatisfyClaim(volume *v1.PersistentVolume, claim *v1.PersistentVolumeClaim) error { | ||||
| 	requestedQty := claim.Spec.Resources.Requests[v1.ResourceName(v1.ResourceStorage)] | ||||
| 	requestedSize := requestedQty.Value() | ||||
| 	isMisMatch, err := checkVolumeModeMisMatches(&claim.Spec, &volume.Spec) | ||||
| 	if err != nil { | ||||
| 		return fmt.Errorf("error checking volumeMode: %v", err) | ||||
| 	} | ||||
|  | ||||
| 	volumeQty := volume.Spec.Capacity[v1.ResourceStorage] | ||||
| 	volumeSize := volumeQty.Value() | ||||
| 	if volumeSize < requestedSize { | ||||
| @@ -249,10 +244,18 @@ func checkVolumeSatisfyClaim(volume *v1.PersistentVolume, claim *v1.PersistentVo | ||||
| 		return fmt.Errorf("storageClasseName does not match") | ||||
| 	} | ||||
|  | ||||
| 	isMisMatch, err := checkVolumeModeMisMatches(&claim.Spec, &volume.Spec) | ||||
| 	if err != nil { | ||||
| 		return fmt.Errorf("error checking volumeMode: %v", err) | ||||
| 	} | ||||
| 	if isMisMatch { | ||||
| 		return fmt.Errorf("incompatible volumeMode") | ||||
| 	} | ||||
|  | ||||
| 	if !checkAccessModes(claim, volume) { | ||||
| 		return fmt.Errorf("incompatible accessMode") | ||||
| 	} | ||||
|  | ||||
| 	return nil | ||||
| } | ||||
|  | ||||
|   | ||||
		Reference in New Issue
	
	Block a user
	 Kubernetes Submit Queue
					Kubernetes Submit Queue