Fix validation options for old pvc

Also update comments on allocatedresourcestatuses fields

pkg/apis/core/helper/helpers.go

@@ -360,6 +360,13 @@ func ContainsAccessMode(modes []core.PersistentVolumeAccessMode, mode core.Persi
 	return false
 }
 
+func ClaimContainsAllocatedResources(pvc *core.PersistentVolumeClaim) bool {
+	if pvc.Status.AllocatedResourceStatuses != nil || pvc.Status.AllocatedResources != nil {
+		return true
+	}
+	return false
+}
+
 // GetTolerationsFromPodAnnotations gets the json serialized tolerations data from Pod.Annotations
 // and converts it to the []Toleration type in core.
 func GetTolerationsFromPodAnnotations(annotations map[string]string) ([]core.Toleration, error) {

pkg/apis/core/types.go

@@ -521,7 +521,7 @@ const (
 type ClaimResourceStatus string
 
 const (
-	// State set when resize controller starts expanding the volume in control-plane
+	// State set when resize controller starts resizing the volume in control-plane
 	PersistentVolumeClaimControllerResizeInProgress ClaimResourceStatus = "ControllerResizeInProgress"
 
 	// State set when resize has failed in resize controller with a terminal error.
@@ -566,11 +566,13 @@ type PersistentVolumeClaimStatus struct {
 	// +optional
 	Conditions []PersistentVolumeClaimCondition
 	// AllocatedResources tracks the resources allocated to a PVC including its capacity.
-	// Following are valid key names for allocatedResources:
+	// Key names follow standard Kubernetes label syntax. Valid values are either:
-	// 	- storage
+	// 	* Un-prefixed keys:
-	//	- example.com/foobar
+	//		- storage - the capacity of the volume.
+	//	* Custom resources must use implementation-defined prefixed names such as "example.com/my-custom-resource"
 	// Apart from above values - keys that are unprefixed or have kubernetes.io prefix are considered
 	// reserved and hence may not be used.
+	//
 	// Capacity reported here may be larger than the actual capacity when a volume expansion operation
 	// is requested.
 	// For storage quota, the larger value from allocatedResources and PVC.spec.resources is used.
@@ -578,19 +580,27 @@ type PersistentVolumeClaimStatus struct {
 	// If a volume expansion capacity request is lowered, allocatedResources is only
 	// lowered if there are no expansion operations in progress and if the actual volume capacity
 	// is equal or lower than the requested capacity.
+	//
+	// A controller that receives PVC update with previously unknown resourceName
+	// should ignore the update for the purpose it was designed. For example - a controller that
+	// only is responsible for resizing capacity of the volume, should ignore PVC updates that change other valid
+	// resources associated with PVC.
+	//
 	// This is an alpha field and requires enabling RecoverVolumeExpansionFailure feature.
 	// +featureGate=RecoverVolumeExpansionFailure
 	// +optional
 	AllocatedResources ResourceList
 	// AllocatedResourceStatuses stores status of resource being resized for the given PVC.
-	// Following are valid key names for allocatedResourceStatuses:
+	// Key names follow standard Kubernetes label syntax. Valid values are either:
-	// 	- storage
+	// 	* Un-prefixed keys:
-	//	- example.com/foobar
+	//		- storage - the capacity of the volume.
+	//	* Custom resources must use implementation-defined prefixed names such as "example.com/my-custom-resource"
 	// Apart from above values - keys that are unprefixed or have kubernetes.io prefix are considered
 	// reserved and hence may not be used.
+	//
 	// ClaimResourceStatus can be in any of following states:
 	//	- ControllerResizeInProgress:
-	//		State set when resize controller starts expanding the volume in control-plane.
+	//		State set when resize controller starts resizing the volume in control-plane.
 	// 	- ControllerResizeFailed:
 	//		State set when resize has failed in resize controller with a terminal error.
 	//	- NodeResizePending:
@@ -608,10 +618,12 @@ type PersistentVolumeClaimStatus struct {
 	//      - pvc.status.allocatedResourceStatus['storage'] = "NodeResizeInProgress"
 	//      - pvc.status.allocatedResourceStatus['storage'] = "NodeResizeFailed"
 	// When this field is not set, it means that no resize operation is in progress for the given PVC.
+	//
 	// A controller that receives PVC update with previously unknown resourceName or ClaimResourceStatus
 	// should ignore the update for the purpose it was designed. For example - a controller that
-	// only is responsible for resizing capacity of the volume, should ignore pvc updates that change other valid
+	// only is responsible for resizing capacity of the volume, should ignore PVC updates that change other valid
 	// resources associated with PVC.
+	//
 	// This is an alpha field and requires enabling RecoverVolumeExpansionFailure feature.
 	// +featureGate=RecoverVolumeExpansionFailure
 	// +mapType=granular

pkg/apis/core/validation/validation.go

@@ -2046,6 +2046,10 @@ func ValidationOptionsForPersistentVolumeClaim(pvc, oldPvc *core.PersistentVolum
 		// If the old object allowed "ReadWriteOncePod", continue to allow it in the new object
 		opts.AllowReadWriteOncePod = true
 	}
+
+	if helper.ClaimContainsAllocatedResources(oldPvc) {
+		opts.EnableRecoverFromExpansionFailure = true
+	}
 	return opts
 }
 

pkg/apis/core/validation/validation_test.go

@@ -18450,34 +18450,29 @@ func TestValidatePersistentVolumeClaimStatusUpdate(t *testing.T) {
 		isExpectedFailure          bool
 		oldClaim                   *core.PersistentVolumeClaim
 		newClaim                   *core.PersistentVolumeClaim
-		enableResize               bool
 		enableRecoverFromExpansion bool
 	}{
 		"condition-update-with-enabled-feature-gate": {
 			isExpectedFailure: false,
 			oldClaim:          validClaim,
 			newClaim:          validConditionUpdate,
-			enableResize:      true,
 		},
 		"status-update-with-valid-allocatedResources-feature-enabled": {
 			isExpectedFailure:          false,
 			oldClaim:                   validClaim,
 			newClaim:                   validAllocatedResources,
-			enableResize:               true,
 			enableRecoverFromExpansion: true,
 		},
 		"status-update-with-invalid-allocatedResources-native-key-feature-enabled": {
 			isExpectedFailure:          true,
 			oldClaim:                   validClaim,
 			newClaim:                   invalidNativeResourceAllocatedKey,
-			enableResize:               true,
 			enableRecoverFromExpansion: true,
 		},
 		"status-update-with-valid-allocatedResources-external-key-feature-enabled": {
 			isExpectedFailure:          false,
 			oldClaim:                   validClaim,
 			newClaim:                   validExternalAllocatedResource,
-			enableResize:               true,
 			enableRecoverFromExpansion: true,
 		},
 
@@ -18485,85 +18480,87 @@ func TestValidatePersistentVolumeClaimStatusUpdate(t *testing.T) {
 			isExpectedFailure:          true,
 			oldClaim:                   validClaim,
 			newClaim:                   invalidAllocatedResources,
-			enableResize:               true,
 			enableRecoverFromExpansion: true,
 		},
 		"status-update-with-no-storage-update": {
 			isExpectedFailure:          true,
 			oldClaim:                   validClaim,
 			newClaim:                   noStoraegeClaimStatus,
-			enableResize:               true,
 			enableRecoverFromExpansion: true,
 		},
 		"staus-update-with-controller-resize-failed": {
 			isExpectedFailure:          false,
 			oldClaim:                   validClaim,
 			newClaim:                   validResizeStatusControllerResizeFailed,
-			enableResize:               true,
 			enableRecoverFromExpansion: true,
 		},
 		"staus-update-with-node-resize-pending": {
 			isExpectedFailure:          false,
 			oldClaim:                   validClaim,
 			newClaim:                   validNodeResizePending,
-			enableResize:               true,
 			enableRecoverFromExpansion: true,
 		},
 		"staus-update-with-node-resize-inprogress": {
 			isExpectedFailure:          false,
 			oldClaim:                   validClaim,
 			newClaim:                   validNodeResizeInProgress,
-			enableResize:               true,
 			enableRecoverFromExpansion: true,
 		},
 		"staus-update-with-node-resize-failed": {
 			isExpectedFailure:          false,
 			oldClaim:                   validClaim,
 			newClaim:                   validNodeResizeFailed,
-			enableResize:               true,
 			enableRecoverFromExpansion: true,
 		},
 		"staus-update-with-invalid-native-resource-status-key": {
 			isExpectedFailure:          true,
 			oldClaim:                   validClaim,
 			newClaim:                   invalidNativeResizeStatusPVC,
-			enableResize:               true,
 			enableRecoverFromExpansion: true,
 		},
 		"staus-update-with-valid-external-resource-status-key": {
 			isExpectedFailure:          false,
 			oldClaim:                   validClaim,
 			newClaim:                   validExternalResizeStatusPVC,
-			enableResize:               true,
 			enableRecoverFromExpansion: true,
 		},
 		"status-update-with-multiple-resources-key": {
 			isExpectedFailure:          false,
 			oldClaim:                   validClaim,
 			newClaim:                   multipleResourceStatusPVC,
-			enableResize:               true,
 			enableRecoverFromExpansion: true,
 		},
 		"status-update-with-valid-pvc-resize-status": {
 			isExpectedFailure:          false,
 			oldClaim:                   validClaim,
 			newClaim:                   validResizeStatusPVC,
-			enableResize:               true,
 			enableRecoverFromExpansion: true,
 		},
 		"status-update-with-invalid-pvc-resize-status": {
 			isExpectedFailure:          true,
 			oldClaim:                   validClaim,
 			newClaim:                   invalidResizeStatusPVC,
-			enableResize:               true,
 			enableRecoverFromExpansion: true,
 		},
+		"status-update-with-old-pvc-valid-resourcestatus-newpvc-invalid-recovery-disabled": {
+			isExpectedFailure:          true,
+			oldClaim:                   validResizeStatusPVC,
+			newClaim:                   invalidResizeStatusPVC,
+			enableRecoverFromExpansion: false,
+		},
+		"status-update-with-old-pvc-valid-allocatedResource-newpvc-invalid-recovery-disabled": {
+			isExpectedFailure:          true,
+			oldClaim:                   validExternalAllocatedResource,
+			newClaim:                   invalidNativeResourceAllocatedKey,
+			enableRecoverFromExpansion: false,
+		},
 	}
 	for name, scenario := range scenarios {
 		t.Run(name, func(t *testing.T) {
-			validateOpts := PersistentVolumeClaimSpecValidationOptions{
+			defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.RecoverVolumeExpansionFailure, scenario.enableRecoverFromExpansion)()
-				EnableRecoverFromExpansionFailure: scenario.enableRecoverFromExpansion,
+
-			}
+			validateOpts := ValidationOptionsForPersistentVolumeClaim(scenario.newClaim, scenario.oldClaim)
+
 			// ensure we have a resource version specified for updates
 			scenario.oldClaim.ResourceVersion = "1"
 			scenario.newClaim.ResourceVersion = "1"

staging/src/k8s.io/api/core/v1/types.go

@@ -564,7 +564,7 @@ const (
 type ClaimResourceStatus string
 
 const (
-	// State set when resize controller starts expanding the volume in control-plane
+	// State set when resize controller starts resizing the volume in control-plane.
 	PersistentVolumeClaimControllerResizeInProgress ClaimResourceStatus = "ControllerResizeInProgress"
 
 	// State set when resize has failed in resize controller with a terminal error.
@@ -620,11 +620,13 @@ type PersistentVolumeClaimStatus struct {
 	// +patchStrategy=merge
 	Conditions []PersistentVolumeClaimCondition `json:"conditions,omitempty" patchStrategy:"merge" patchMergeKey:"type" protobuf:"bytes,4,rep,name=conditions"`
 	// allocatedResources tracks the resources allocated to a PVC including its capacity.
-	// Following are valid key names for allocatedResources:
+	// Key names follow standard Kubernetes label syntax. Valid values are either:
-	// 	- storage
+	// 	* Un-prefixed keys:
-	//	- example.com/foobar
+	//		- storage - the capacity of the volume.
+	//	* Custom resources must use implementation-defined prefixed names such as "example.com/my-custom-resource"
 	// Apart from above values - keys that are unprefixed or have kubernetes.io prefix are considered
 	// reserved and hence may not be used.
+	//
 	// Capacity reported here may be larger than the actual capacity when a volume expansion operation
 	// is requested.
 	// For storage quota, the larger value from allocatedResources and PVC.spec.resources is used.
@@ -632,6 +634,12 @@ type PersistentVolumeClaimStatus struct {
 	// If a volume expansion capacity request is lowered, allocatedResources is only
 	// lowered if there are no expansion operations in progress and if the actual volume capacity
 	// is equal or lower than the requested capacity.
+	//
+	// A controller that receives PVC update with previously unknown resourceName
+	// should ignore the update for the purpose it was designed. For example - a controller that
+	// only is responsible for resizing capacity of the volume, should ignore PVC updates that change other valid
+	// resources associated with PVC.
+	//
 	// This is an alpha field and requires enabling RecoverVolumeExpansionFailure feature.
 	// +featureGate=RecoverVolumeExpansionFailure
 	// +optional
@@ -641,14 +649,16 @@ type PersistentVolumeClaimStatus struct {
 	// ResizeStatus *PersistentVolumeClaimResizeStatus `json:"resizeStatus,omitempty" protobuf:"bytes,6,opt,name=resizeStatus,casttype=PersistentVolumeClaimResizeStatus"`
 
 	// allocatedResourceStatuses stores status of resource being resized for the given PVC.
-	// Following are valid key names for allocatedResourceStatuses:
+	// Key names follow standard Kubernetes label syntax. Valid values are either:
-	// 	- storage
+	// 	* Un-prefixed keys:
-	//	- example.com/foobar
+	//		- storage - the capacity of the volume.
+	//	* Custom resources must use implementation-defined prefixed names such as "example.com/my-custom-resource"
 	// Apart from above values - keys that are unprefixed or have kubernetes.io prefix are considered
 	// reserved and hence may not be used.
+	//
 	// ClaimResourceStatus can be in any of following states:
 	//	- ControllerResizeInProgress:
-	//		State set when resize controller starts expanding the volume in control-plane.
+	//		State set when resize controller starts resizing the volume in control-plane.
 	// 	- ControllerResizeFailed:
 	//		State set when resize has failed in resize controller with a terminal error.
 	//	- NodeResizePending:
@@ -659,17 +669,19 @@ type PersistentVolumeClaimStatus struct {
 	//	- NodeResizeFailed:
 	//		State set when resizing has failed in kubelet with a terminal error. Transient errors don't set
 	//		NodeResizeFailed.
-	// For example expanding a PVC for more capacity - this field can be one of the following states:
+	// For example: if expanding a PVC for more capacity - this field can be one of the following states:
 	// 	- pvc.status.allocatedResourceStatus['storage'] = "ControllerResizeInProgress"
 	//      - pvc.status.allocatedResourceStatus['storage'] = "ControllerResizeFailed"
 	//      - pvc.status.allocatedResourceStatus['storage'] = "NodeResizePending"
 	//      - pvc.status.allocatedResourceStatus['storage'] = "NodeResizeInProgress"
 	//      - pvc.status.allocatedResourceStatus['storage'] = "NodeResizeFailed"
 	// When this field is not set, it means that no resize operation is in progress for the given PVC.
+	//
 	// A controller that receives PVC update with previously unknown resourceName or ClaimResourceStatus
 	// should ignore the update for the purpose it was designed. For example - a controller that
-	// only is responsible for resizing capacity of the volume, should ignore pvc updates that change other valid
+	// only is responsible for resizing capacity of the volume, should ignore PVC updates that change other valid
 	// resources associated with PVC.
+	//
 	// This is an alpha field and requires enabling RecoverVolumeExpansionFailure feature.
 	// +featureGate=RecoverVolumeExpansionFailure
 	// +mapType=granular