Update comments about allocatedResourceStatus

Update API types with more comments

pkg/apis/core/types.go

@@ -524,16 +524,17 @@ const (
 	// State set when resize controller starts expanding the volume in control-plane
 	PersistentVolumeClaimControllerResizeInProgress ClaimResourceStatus = "ControllerResizeInProgress"
 
-	// State set when expansion has failed in resize controller with a terminal error.
+	// State set when resize has failed in resize controller with a terminal error.
 	// Transient errors such as timeout should not set this status and should leave allocatedResourceStatus
 	// unmodified, so as resize controller can resume the volume expansion.
 	PersistentVolumeClaimControllerResizeFailed ClaimResourceStatus = "ControllerResizeFailed"
 
-	// State set when resize controller has finished expanding the volume but further expansion is needed on the node.
+	// State set when resize controller has finished resizing the volume but further resizing of volume
+	// is needed on the node.
 	PersistentVolumeClaimNodeResizePending ClaimResourceStatus = "NodeResizePending"
-	// State set when kubelet starts expanding the volume.
+	// State set when kubelet starts resizing the volume.
 	PersistentVolumeClaimNodeResizeInProgress ClaimResourceStatus = "NodeResizeInProgress"
-	// State set when expansion has failed in kubelet with a terminal error. Transient errors don't set NodeExpansionFailed.
+	// State set when resizing has failed in kubelet with a terminal error. Transient errors don't set NodeResizeFailed
 	PersistentVolumeClaimNodeResizeFailed ClaimResourceStatus = "NodeResizeFailed"
 )
 
@@ -564,8 +565,14 @@ type PersistentVolumeClaimStatus struct {
 	Capacity ResourceList
 	// +optional
 	Conditions []PersistentVolumeClaimCondition
-	// The storage resource within AllocatedResources tracks the capacity allocated to a PVC. It may
+	// AllocatedResources tracks the resources allocated to a PVC including its capacity.
-	// be larger than the actual capacity when a volume expansion operation is requested.
+	// Following are valid key names for allocatedResources:
+	// 	- storage
+	//	- example.com/foobar
+	// 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.
 	// If allocatedResources is not set, PVC.spec.resources alone is used for quota calculation.
 	// If a volume expansion capacity request is lowered, allocatedResources is only
@@ -575,14 +582,36 @@ type PersistentVolumeClaimStatus struct {
 	// +featureGate=RecoverVolumeExpansionFailure
 	// +optional
 	AllocatedResources ResourceList
-	// allocatedResourceStatuses stores status of resource being resized for the given PVC.
+	// AllocatedResourceStatuses stores status of resource being resized for the given PVC.
-	// If Expanding a PVC for more capacity - this field can be one of the following states:
+	// Following are valid key names for allocatedResourceStatuses:
+	// 	- storage
+	//	- example.com/foobar
+	// 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.
+	// 	- ControllerResizeFailed:
+	//		State set when resize has failed in resize controller with a terminal error.
+	//	- NodeResizePending:
+	//		State set when resize controller has finished resizing the volume but further resizing of
+	//		volume is needed on the node.
+	//	- NodeResizeInProgress:
+	//		State set when kubelet starts resizing the volume.
+	//	- NodeResizeFailed:
+	//		State set when resizing has failed in kubelet with a terminal error. Transient errors don't set
+	//		NodeResizeFailed.
+	// 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
+	// resources associated with PVC.
 	// This is an alpha field and requires enabling RecoverVolumeExpansionFailure feature.
 	// +featureGate=RecoverVolumeExpansionFailure
 	// +mapType=granular

pkg/apis/core/validation/validation_test.go

@@ -18391,6 +18391,21 @@ func TestValidatePersistentVolumeClaimStatusUpdate(t *testing.T) {
 		},
 	})
 
+	multipleResourceStatusPVC := testVolumeClaimWithStatus("foo", "ns", core.PersistentVolumeClaimSpec{
+		AccessModes: []core.PersistentVolumeAccessMode{
+			core.ReadWriteOnce,
+		},
+	}, core.PersistentVolumeClaimStatus{
+		AllocatedResources: core.ResourceList{
+			core.ResourceStorage: resource.MustParse("5Gi"),
+			validResizeKeyCustom: resource.MustParse("10Gi"),
+		},
+		AllocatedResourceStatuses: map[core.ResourceName]core.ClaimResourceStatus{
+			core.ResourceStorage: core.PersistentVolumeClaimControllerResizeFailed,
+			validResizeKeyCustom: core.PersistentVolumeClaimControllerResizeInProgress,
+		},
+	})
+
 	invalidNativeResourceAllocatedKey := testVolumeClaimWithStatus("foo", "ns", core.PersistentVolumeClaimSpec{
 		AccessModes: []core.PersistentVolumeAccessMode{
 			core.ReadWriteOnce,
@@ -18522,6 +18537,13 @@ func TestValidatePersistentVolumeClaimStatusUpdate(t *testing.T) {
 			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,

pkg/volume/util/resize_util_test.go

@@ -22,9 +22,14 @@ import (
 	"testing"
 	"time"
 
-	"k8s.io/api/core/v1"
+	v1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/resource"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	utilfeature "k8s.io/apiserver/pkg/util/feature"
+	clientset "k8s.io/client-go/kubernetes"
+	"k8s.io/client-go/kubernetes/fake"
+	featuregatetesting "k8s.io/component-base/featuregate/testing"
+	"k8s.io/kubernetes/pkg/features"
 )
 
 type conditionMergeTestCase struct {
@@ -37,15 +42,15 @@ type conditionMergeTestCase struct {
 func TestMergeResizeCondition(t *testing.T) {
 	currentTime := metav1.Now()
 
-	pvc := getPVC([]v1.PersistentVolumeClaimCondition{
+	pvc := makePVC([]v1.PersistentVolumeClaimCondition{
 		{
 			Type:               v1.PersistentVolumeClaimResizing,
 			Status:             v1.ConditionTrue,
 			LastTransitionTime: currentTime,
 		},
-	})
+	}).get()
 
-	noConditionPVC := getPVC([]v1.PersistentVolumeClaimCondition{})
+	noConditionPVC := makePVC([]v1.PersistentVolumeClaimCondition{}).get()
 
 	conditionFalseTime := metav1.Now()
 	newTime := metav1.NewTime(time.Now().Add(1 * time.Hour))
@@ -142,14 +147,83 @@ func TestMergeResizeCondition(t *testing.T) {
 
 }
 
+func TestResizeFunctions(t *testing.T) {
+	basePVC := makePVC([]v1.PersistentVolumeClaimCondition{})
+
+	tests := []struct {
+		name        string
+		pvc         *v1.PersistentVolumeClaim
+		expectedPVC *v1.PersistentVolumeClaim
+		testFunc    func(*v1.PersistentVolumeClaim, clientset.Interface, resource.Quantity) (*v1.PersistentVolumeClaim, error)
+	}{
+		{
+			name:        "mark fs resize, with no other conditions",
+			pvc:         basePVC.get(),
+			expectedPVC: basePVC.modifyStorageResourceStatus(v1.PersistentVolumeClaimNodeResizePending).get(),
+			testFunc: func(pvc *v1.PersistentVolumeClaim, c clientset.Interface, _ resource.Quantity) (*v1.PersistentVolumeClaim, error) {
+				return MarkForFSResize(pvc, c)
+			},
+		},
+		{
+			name: "mark fs resize, when other resource statuses are present",
+			pvc:  basePVC.modifyResourceStatus(v1.ResourceCPU, v1.PersistentVolumeClaimControllerResizeFailed).get(),
+			expectedPVC: basePVC.modifyResourceStatus(v1.ResourceCPU, v1.PersistentVolumeClaimControllerResizeFailed).
+				modifyStorageResourceStatus(v1.PersistentVolumeClaimNodeResizePending).get(),
+			testFunc: func(pvc *v1.PersistentVolumeClaim, c clientset.Interface, _ resource.Quantity) (*v1.PersistentVolumeClaim, error) {
+				return MarkForFSResize(pvc, c)
+			},
+		},
+		{
+			name:        "mark controller resize in-progress",
+			pvc:         basePVC.get(),
+			expectedPVC: basePVC.modifyStorageResourceStatus(v1.PersistentVolumeClaimControllerResizeInProgress).get(),
+			testFunc: func(pvc *v1.PersistentVolumeClaim, i clientset.Interface, q resource.Quantity) (*v1.PersistentVolumeClaim, error) {
+				return MarkControllerReisizeInProgress(pvc, "foobar", q, i)
+			},
+		},
+		{
+			name: "mark resize finished",
+			pvc: basePVC.modifyResourceStatus(v1.ResourceCPU, v1.PersistentVolumeClaimControllerResizeFailed).
+				modifyStorageResourceStatus(v1.PersistentVolumeClaimNodeResizePending).get(),
+			expectedPVC: basePVC.modifyResourceStatus(v1.ResourceCPU, v1.PersistentVolumeClaimControllerResizeFailed).
+				modifyStorageResourceStatus("").get(),
+			testFunc: func(pvc *v1.PersistentVolumeClaim, i clientset.Interface, q resource.Quantity) (*v1.PersistentVolumeClaim, error) {
+				return MarkFSResizeFinished(pvc, q, i)
+			},
+		},
+	}
+
+	for _, test := range tests {
+		tc := test
+		t.Run(tc.name, func(t *testing.T) {
+			defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.RecoverVolumeExpansionFailure, true)()
+			pvc := tc.pvc
+			kubeClient := fake.NewSimpleClientset(pvc)
+
+			var err error
+
+			pvc, err = tc.testFunc(pvc, kubeClient, resource.MustParse("10Gi"))
+			if err != nil {
+				t.Errorf("Expected no error but got %v", err)
+			}
+			realStatus := pvc.Status.AllocatedResourceStatuses
+			expectedStatus := tc.expectedPVC.Status.AllocatedResourceStatuses
+			if !reflect.DeepEqual(realStatus, expectedStatus) {
+				t.Errorf("expected %+v got %+v", expectedStatus, realStatus)
+			}
+		})
+	}
+
+}
+
 func TestCreatePVCPatch(t *testing.T) {
-	pvc1 := getPVC([]v1.PersistentVolumeClaimCondition{
+	pvc1 := makePVC([]v1.PersistentVolumeClaimCondition{
 		{
 			Type:               v1.PersistentVolumeClaimFileSystemResizePending,
 			Status:             v1.ConditionTrue,
 			LastTransitionTime: metav1.Now(),
 		},
-	})
+	}).get()
 	pvc1.SetResourceVersion("10")
 	pvc2 := pvc1.DeepCopy()
 	pvc2.Status.Capacity = v1.ResourceList{
@@ -174,7 +248,15 @@ func TestCreatePVCPatch(t *testing.T) {
 	}
 }
 
-func getPVC(conditions []v1.PersistentVolumeClaimCondition) *v1.PersistentVolumeClaim {
+type pvcModifier struct {
+	pvc *v1.PersistentVolumeClaim
+}
+
+func (m pvcModifier) get() *v1.PersistentVolumeClaim {
+	return m.pvc.DeepCopy()
+}
+
+func makePVC(conditions []v1.PersistentVolumeClaimCondition) pvcModifier {
 	pvc := &v1.PersistentVolumeClaim{
 		ObjectMeta: metav1.ObjectMeta{Name: "foo", Namespace: "resize"},
 		Spec: v1.PersistentVolumeClaimSpec{
@@ -196,5 +278,24 @@ func getPVC(conditions []v1.PersistentVolumeClaimCondition) *v1.PersistentVolume
 			},
 		},
 	}
-	return pvc
+	return pvcModifier{pvc}
+}
+
+func (m pvcModifier) modifyStorageResourceStatus(status v1.ClaimResourceStatus) pvcModifier {
+	return m.modifyResourceStatus(v1.ResourceStorage, status)
+}
+
+func (m pvcModifier) modifyResourceStatus(resource v1.ResourceName, status v1.ClaimResourceStatus) pvcModifier {
+	if m.pvc.Status.AllocatedResourceStatuses != nil && status == "" {
+		delete(m.pvc.Status.AllocatedResourceStatuses, resource)
+		return m
+	}
+	if m.pvc.Status.AllocatedResourceStatuses != nil {
+		m.pvc.Status.AllocatedResourceStatuses[resource] = status
+	} else {
+		m.pvc.Status.AllocatedResourceStatuses = map[v1.ResourceName]v1.ClaimResourceStatus{
+			resource: status,
+		}
+	}
+	return m
 }

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

@@ -558,22 +558,26 @@ const (
 )
 
 // +enum
+// When a controller receives persistentvolume claim update with ClaimResourceStatus for a resource
+// that it does not recognizes, then it should ignore that update and let other controllers
+// handle it.
 type ClaimResourceStatus string
 
 const (
 	// State set when resize controller starts expanding the volume in control-plane
 	PersistentVolumeClaimControllerResizeInProgress ClaimResourceStatus = "ControllerResizeInProgress"
 
-	// State set when expansion has failed in resize controller with a terminal error.
+	// State set when resize has failed in resize controller with a terminal error.
 	// Transient errors such as timeout should not set this status and should leave allocatedResourceStatus
 	// unmodified, so as resize controller can resume the volume expansion.
 	PersistentVolumeClaimControllerResizeFailed ClaimResourceStatus = "ControllerResizeFailed"
 
-	// State set when resize controller has finished expanding the volume but further expansion is needed on the node.
+	// State set when resize controller has finished resizing the volume but further resizing of volume
+	// is needed on the node.
 	PersistentVolumeClaimNodeResizePending ClaimResourceStatus = "NodeResizePending"
-	// State set when kubelet starts expanding the volume.
+	// State set when kubelet starts resizing the volume.
 	PersistentVolumeClaimNodeResizeInProgress ClaimResourceStatus = "NodeResizeInProgress"
-	// State set when expansion has failed in kubelet with a terminal error. Transient errors don't set NodeExpansionFailed.
+	// State set when resizing has failed in kubelet with a terminal error. Transient errors don't set NodeResizeFailed
 	PersistentVolumeClaimNodeResizeFailed ClaimResourceStatus = "NodeResizeFailed"
 )
 
@@ -615,8 +619,14 @@ type PersistentVolumeClaimStatus struct {
 	// +patchMergeKey=type
 	// +patchStrategy=merge
 	Conditions []PersistentVolumeClaimCondition `json:"conditions,omitempty" patchStrategy:"merge" patchMergeKey:"type" protobuf:"bytes,4,rep,name=conditions"`
-	// allocatedResources is the storage resource within AllocatedResources tracks the capacity allocated to a PVC. It may
+	// allocatedResources tracks the resources allocated to a PVC including its capacity.
-	// be larger than the actual capacity when a volume expansion operation is requested.
+	// Following are valid key names for allocatedResources:
+	// 	- storage
+	//	- example.com/foobar
+	// 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.
 	// If allocatedResources is not set, PVC.spec.resources alone is used for quota calculation.
 	// If a volume expansion capacity request is lowered, allocatedResources is only
@@ -631,13 +641,35 @@ 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.
-	// If Expanding a PVC for more capacity - this field can be one of the following states:
+	// Following are valid key names for allocatedResourceStatuses:
+	// 	- storage
+	//	- example.com/foobar
+	// 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.
+	// 	- ControllerResizeFailed:
+	//		State set when resize has failed in resize controller with a terminal error.
+	//	- NodeResizePending:
+	//		State set when resize controller has finished resizing the volume but further resizing of
+	//		volume is needed on the node.
+	//	- NodeResizeInProgress:
+	//		State set when kubelet starts resizing the volume.
+	//	- 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:
 	// 	- 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
+	// resources associated with PVC.
 	// This is an alpha field and requires enabling RecoverVolumeExpansionFailure feature.
 	// +featureGate=RecoverVolumeExpansionFailure
 	// +mapType=granular