From e01118711494b4834cd5cc67567c832a5d56d774 Mon Sep 17 00:00:00 2001 From: Hemant Kumar Date: Mon, 10 Jul 2023 12:13:25 -0400 Subject: [PATCH] Update code to use new generic allocatedResourceStatus field --- pkg/api/persistentvolumeclaim/util.go | 4 +- pkg/api/persistentvolumeclaim/util_test.go | 24 +-- pkg/apis/core/types.go | 35 ++-- pkg/apis/core/validation/validation.go | 50 +++-- pkg/apis/core/validation/validation_test.go | 172 +++++++++++++++++- .../cache/desired_state_of_world.go | 2 +- .../reconciler/reconciler_common.go | 2 +- .../util/operationexecutor/node_expander.go | 34 ++-- .../operationexecutor/node_expander_test.go | 31 ++-- .../operationexecutor/operation_executor.go | 4 +- .../operationexecutor/operation_generator.go | 41 +++-- .../operation_generator_test.go | 63 ++++--- pkg/volume/util/resize_util.go | 52 ++++-- .../admission/noderestriction/admission.go | 4 +- .../noderestriction/admission_test.go | 36 ++-- staging/src/k8s.io/api/core/v1/types.go | 36 ++-- .../storage/csi_mock/csi_volume_expansion.go | 33 ++-- 17 files changed, 444 insertions(+), 179 deletions(-) diff --git a/pkg/api/persistentvolumeclaim/util.go b/pkg/api/persistentvolumeclaim/util.go index 0fa4c7e7f62..5f818c2e1e3 100644 --- a/pkg/api/persistentvolumeclaim/util.go +++ b/pkg/api/persistentvolumeclaim/util.go @@ -99,7 +99,7 @@ func DropDisabledFieldsFromStatus(pvc, oldPVC *core.PersistentVolumeClaim) { pvc.Status.AllocatedResources = nil } if !resizeStatusInUse(oldPVC) { - pvc.Status.ResizeStatus = nil + pvc.Status.AllocatedResourceStatuses = nil } } } @@ -179,7 +179,7 @@ func resizeStatusInUse(oldPVC *core.PersistentVolumeClaim) bool { if oldPVC == nil { return false } - if oldPVC.Status.ResizeStatus != nil { + if oldPVC.Status.AllocatedResourceStatuses != nil { return true } return false diff --git a/pkg/api/persistentvolumeclaim/util_test.go b/pkg/api/persistentvolumeclaim/util_test.go index 38253b1fb4b..4ceec00b9b3 100644 --- a/pkg/api/persistentvolumeclaim/util_test.go +++ b/pkg/api/persistentvolumeclaim/util_test.go @@ -436,30 +436,30 @@ func TestDropDisabledFieldsFromStatus(t *testing.T) { { name: "for:newPVC=hasResizeStatus,oldPVC=nil, featuregate=false should drop field", feature: false, - pvc: withResizeStatus(core.PersistentVolumeClaimNodeExpansionFailed), + pvc: withResizeStatus(core.PersistentVolumeClaimNodeResizeFailed), oldPVC: nil, expected: getPVC(), }, { name: "for:newPVC=hasResizeStatus,oldPVC=doesnot,featuregate=true; should keep field", feature: true, - pvc: withResizeStatus(core.PersistentVolumeClaimNodeExpansionFailed), + pvc: withResizeStatus(core.PersistentVolumeClaimNodeResizeFailed), oldPVC: getPVC(), - expected: withResizeStatus(core.PersistentVolumeClaimNodeExpansionFailed), + expected: withResizeStatus(core.PersistentVolumeClaimNodeResizeFailed), }, { name: "for:newPVC=hasResizeStatus,oldPVC=hasResizeStatus,featuregate=true; should keep field", feature: true, - pvc: withResizeStatus(core.PersistentVolumeClaimNodeExpansionFailed), - oldPVC: withResizeStatus(core.PersistentVolumeClaimNodeExpansionFailed), - expected: withResizeStatus(core.PersistentVolumeClaimNodeExpansionFailed), + pvc: withResizeStatus(core.PersistentVolumeClaimNodeResizeFailed), + oldPVC: withResizeStatus(core.PersistentVolumeClaimNodeResizeFailed), + expected: withResizeStatus(core.PersistentVolumeClaimNodeResizeFailed), }, { name: "for:newPVC=hasResizeStatus,oldPVC=hasResizeStatus,featuregate=false; should keep field", feature: false, - pvc: withResizeStatus(core.PersistentVolumeClaimNodeExpansionFailed), - oldPVC: withResizeStatus(core.PersistentVolumeClaimNodeExpansionFailed), - expected: withResizeStatus(core.PersistentVolumeClaimNodeExpansionFailed), + pvc: withResizeStatus(core.PersistentVolumeClaimNodeResizeFailed), + oldPVC: withResizeStatus(core.PersistentVolumeClaimNodeResizeFailed), + expected: withResizeStatus(core.PersistentVolumeClaimNodeResizeFailed), }, } @@ -490,10 +490,12 @@ func withAllocatedResource(q string) *core.PersistentVolumeClaim { } } -func withResizeStatus(status core.PersistentVolumeClaimResizeStatus) *core.PersistentVolumeClaim { +func withResizeStatus(status core.ClaimResourceStatus) *core.PersistentVolumeClaim { return &core.PersistentVolumeClaim{ Status: core.PersistentVolumeClaimStatus{ - ResizeStatus: &status, + AllocatedResourceStatuses: map[core.ResourceName]core.ClaimResourceStatus{ + core.ResourceStorage: status, + }, }, } } diff --git a/pkg/apis/core/types.go b/pkg/apis/core/types.go index 22fee46ce22..b4ca684bdd1 100644 --- a/pkg/apis/core/types.go +++ b/pkg/apis/core/types.go @@ -515,23 +515,26 @@ const ( ) // +enum -type PersistentVolumeClaimResizeStatus string +// 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 ( - // When expansion is complete, the empty string is set by resize controller or kubelet. - PersistentVolumeClaimNoExpansionInProgress PersistentVolumeClaimResizeStatus = "" // State set when resize controller starts expanding the volume in control-plane - PersistentVolumeClaimControllerExpansionInProgress PersistentVolumeClaimResizeStatus = "ControllerExpansionInProgress" + PersistentVolumeClaimControllerResizeInProgress ClaimResourceStatus = "ControllerResizeInProgress" + // State set when expansion has failed in resize controller with a terminal error. - // Transient errors such as timeout should not set this status and should leave ResizeStatus + // Transient errors such as timeout should not set this status and should leave allocatedResourceStatus // unmodified, so as resize controller can resume the volume expansion. - PersistentVolumeClaimControllerExpansionFailed PersistentVolumeClaimResizeStatus = "ControllerExpansionFailed" + PersistentVolumeClaimControllerResizeFailed ClaimResourceStatus = "ControllerResizeFailed" + // State set when resize controller has finished expanding the volume but further expansion is needed on the node. - PersistentVolumeClaimNodeExpansionPending PersistentVolumeClaimResizeStatus = "NodeExpansionPending" + PersistentVolumeClaimNodeResizePending ClaimResourceStatus = "NodeResizePending" // State set when kubelet starts expanding the volume. - PersistentVolumeClaimNodeExpansionInProgress PersistentVolumeClaimResizeStatus = "NodeExpansionInProgress" + PersistentVolumeClaimNodeResizeInProgress ClaimResourceStatus = "NodeResizeInProgress" // State set when expansion has failed in kubelet with a terminal error. Transient errors don't set NodeExpansionFailed. - PersistentVolumeClaimNodeExpansionFailed PersistentVolumeClaimResizeStatus = "NodeExpansionFailed" + PersistentVolumeClaimNodeResizeFailed ClaimResourceStatus = "NodeResizeFailed" ) // PersistentVolumeClaimCondition represents the current condition of PV claim @@ -572,13 +575,19 @@ type PersistentVolumeClaimStatus struct { // +featureGate=RecoverVolumeExpansionFailure // +optional AllocatedResources ResourceList - // ResizeStatus stores status of resize operation. - // ResizeStatus is not set by default but when expansion is complete resizeStatus is set to empty - // string by resize controller or kubelet. + // 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: + // - 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. // This is an alpha field and requires enabling RecoverVolumeExpansionFailure feature. // +featureGate=RecoverVolumeExpansionFailure + // +mapType=granular // +optional - ResizeStatus *PersistentVolumeClaimResizeStatus + AllocatedResourceStatuses map[ResourceName]ClaimResourceStatus } // PersistentVolumeAccessMode defines various access modes for PV. diff --git a/pkg/apis/core/validation/validation.go b/pkg/apis/core/validation/validation.go index 5c7b20adf6e..2a4d97b9d17 100644 --- a/pkg/apis/core/validation/validation.go +++ b/pkg/apis/core/validation/validation.go @@ -2294,12 +2294,29 @@ func validateStorageClassUpgradeFromNil(oldAnnotations map[string]string, oldScN (!oldAnnotationExist || *newScName == oldAnnotation) /* condition 3 */ } -var resizeStatusSet = sets.NewString(string(core.PersistentVolumeClaimNoExpansionInProgress), - string(core.PersistentVolumeClaimControllerExpansionInProgress), - string(core.PersistentVolumeClaimControllerExpansionFailed), - string(core.PersistentVolumeClaimNodeExpansionPending), - string(core.PersistentVolumeClaimNodeExpansionInProgress), - string(core.PersistentVolumeClaimNodeExpansionFailed)) +func validatePersistentVolumeClaimResourceKey(value string, fldPath *field.Path) field.ErrorList { + allErrs := field.ErrorList{} + for _, msg := range validation.IsQualifiedName(value) { + allErrs = append(allErrs, field.Invalid(fldPath, value, msg)) + } + if len(allErrs) != 0 { + return allErrs + } + // For native resource names such as - either unprefixed names or with kubernetes.io prefix, + // only allowed value is storage + if helper.IsNativeResource(core.ResourceName(value)) { + if core.ResourceName(value) != core.ResourceStorage { + return append(allErrs, field.NotSupported(fldPath, value, []string{string(core.ResourceStorage)})) + } + } + return allErrs +} + +var resizeStatusSet = sets.NewString(string(core.PersistentVolumeClaimControllerResizeInProgress), + string(core.PersistentVolumeClaimControllerResizeFailed), + string(core.PersistentVolumeClaimNodeResizePending), + string(core.PersistentVolumeClaimNodeResizeInProgress), + string(core.PersistentVolumeClaimNodeResizeFailed)) // ValidatePersistentVolumeClaimStatusUpdate validates an update to status of a PersistentVolumeClaim func ValidatePersistentVolumeClaimStatusUpdate(newPvc, oldPvc *core.PersistentVolumeClaim, validationOpts PersistentVolumeClaimSpecValidationOptions) field.ErrorList { @@ -2316,19 +2333,26 @@ func ValidatePersistentVolumeClaimStatusUpdate(newPvc, oldPvc *core.PersistentVo allErrs = append(allErrs, validateBasicResource(qty, capPath.Key(string(r)))...) } if validationOpts.EnableRecoverFromExpansionFailure { - resizeStatusPath := field.NewPath("status", "resizeStatus") - if newPvc.Status.ResizeStatus != nil { - resizeStatus := *newPvc.Status.ResizeStatus - if !resizeStatusSet.Has(string(resizeStatus)) { - allErrs = append(allErrs, field.NotSupported(resizeStatusPath, resizeStatus, resizeStatusSet.List())) + resizeStatusPath := field.NewPath("status", "allocatedResourceStatus") + if newPvc.Status.AllocatedResourceStatuses != nil { + resizeStatus := newPvc.Status.AllocatedResourceStatuses + for k, v := range resizeStatus { + if errs := validatePersistentVolumeClaimResourceKey(k.String(), resizeStatusPath); len(errs) > 0 { + allErrs = append(allErrs, errs...) + } + if !resizeStatusSet.Has(string(v)) { + allErrs = append(allErrs, field.NotSupported(resizeStatusPath, k, resizeStatusSet.List())) + continue + } } } allocPath := field.NewPath("status", "allocatedResources") for r, qty := range newPvc.Status.AllocatedResources { - if r != core.ResourceStorage { - allErrs = append(allErrs, field.NotSupported(allocPath, r, []string{string(core.ResourceStorage)})) + if errs := validatePersistentVolumeClaimResourceKey(r.String(), allocPath); len(errs) > 0 { + allErrs = append(allErrs, errs...) continue } + if errs := validateBasicResource(qty, allocPath.Key(string(r))); len(errs) > 0 { allErrs = append(allErrs, errs...) } else { diff --git a/pkg/apis/core/validation/validation_test.go b/pkg/apis/core/validation/validation_test.go index 9d9029d03fe..8a96fa9f7ac 100644 --- a/pkg/apis/core/validation/validation_test.go +++ b/pkg/apis/core/validation/validation_test.go @@ -18305,15 +18305,60 @@ func TestValidatePersistentVolumeClaimStatusUpdate(t *testing.T) { core.ResourceName(core.ResourceCPU): resource.MustParse("10G"), }, }) - progressResizeStatus := core.PersistentVolumeClaimControllerExpansionInProgress - invalidResizeStatus := core.PersistentVolumeClaimResizeStatus("foo") + progressResizeStatus := core.PersistentVolumeClaimControllerResizeInProgress + + invalidResizeStatus := core.ClaimResourceStatus("foo") + validResizeKeyCustom := core.ResourceName("example.com/foo") + invalidNativeResizeKey := core.ResourceName("kubernetes.io/foo") validResizeStatusPVC := testVolumeClaimWithStatus("foo", "ns", core.PersistentVolumeClaimSpec{ AccessModes: []core.PersistentVolumeAccessMode{ core.ReadWriteOnce, }, }, core.PersistentVolumeClaimStatus{ - ResizeStatus: &progressResizeStatus, + AllocatedResourceStatuses: map[core.ResourceName]core.ClaimResourceStatus{ + core.ResourceStorage: progressResizeStatus, + }, + }) + + validResizeStatusControllerResizeFailed := testVolumeClaimWithStatus("foo", "ns", core.PersistentVolumeClaimSpec{ + AccessModes: []core.PersistentVolumeAccessMode{ + core.ReadWriteOnce, + }, + }, core.PersistentVolumeClaimStatus{ + AllocatedResourceStatuses: map[core.ResourceName]core.ClaimResourceStatus{ + core.ResourceStorage: core.PersistentVolumeClaimControllerResizeFailed, + }, + }) + + validNodeResizePending := testVolumeClaimWithStatus("foo", "ns", core.PersistentVolumeClaimSpec{ + AccessModes: []core.PersistentVolumeAccessMode{ + core.ReadWriteOnce, + }, + }, core.PersistentVolumeClaimStatus{ + AllocatedResourceStatuses: map[core.ResourceName]core.ClaimResourceStatus{ + core.ResourceStorage: core.PersistentVolumeClaimNodeResizePending, + }, + }) + + validNodeResizeInProgress := testVolumeClaimWithStatus("foo", "ns", core.PersistentVolumeClaimSpec{ + AccessModes: []core.PersistentVolumeAccessMode{ + core.ReadWriteOnce, + }, + }, core.PersistentVolumeClaimStatus{ + AllocatedResourceStatuses: map[core.ResourceName]core.ClaimResourceStatus{ + core.ResourceStorage: core.PersistentVolumeClaimNodeResizeInProgress, + }, + }) + + validNodeResizeFailed := testVolumeClaimWithStatus("foo", "ns", core.PersistentVolumeClaimSpec{ + AccessModes: []core.PersistentVolumeAccessMode{ + core.ReadWriteOnce, + }, + }, core.PersistentVolumeClaimStatus{ + AllocatedResourceStatuses: map[core.ResourceName]core.ClaimResourceStatus{ + core.ResourceStorage: core.PersistentVolumeClaimNodeResizeFailed, + }, }) invalidResizeStatusPVC := testVolumeClaimWithStatus("foo", "ns", core.PersistentVolumeClaimSpec{ @@ -18321,7 +18366,69 @@ func TestValidatePersistentVolumeClaimStatusUpdate(t *testing.T) { core.ReadWriteOnce, }, }, core.PersistentVolumeClaimStatus{ - ResizeStatus: &invalidResizeStatus, + AllocatedResourceStatuses: map[core.ResourceName]core.ClaimResourceStatus{ + core.ResourceStorage: invalidResizeStatus, + }, + }) + + invalidNativeResizeStatusPVC := testVolumeClaimWithStatus("foo", "ns", core.PersistentVolumeClaimSpec{ + AccessModes: []core.PersistentVolumeAccessMode{ + core.ReadWriteOnce, + }, + }, core.PersistentVolumeClaimStatus{ + AllocatedResourceStatuses: map[core.ResourceName]core.ClaimResourceStatus{ + invalidNativeResizeKey: core.PersistentVolumeClaimNodeResizePending, + }, + }) + + validExternalResizeStatusPVC := testVolumeClaimWithStatus("foo", "ns", core.PersistentVolumeClaimSpec{ + AccessModes: []core.PersistentVolumeAccessMode{ + core.ReadWriteOnce, + }, + }, core.PersistentVolumeClaimStatus{ + AllocatedResourceStatuses: map[core.ResourceName]core.ClaimResourceStatus{ + validResizeKeyCustom: core.PersistentVolumeClaimNodeResizePending, + }, + }) + + invalidNativeResourceAllocatedKey := testVolumeClaimWithStatus("foo", "ns", core.PersistentVolumeClaimSpec{ + AccessModes: []core.PersistentVolumeAccessMode{ + core.ReadWriteOnce, + core.ReadOnlyMany, + }, + Resources: core.ResourceRequirements{ + Requests: core.ResourceList{ + core.ResourceName(core.ResourceStorage): resource.MustParse("10G"), + }, + }, + }, core.PersistentVolumeClaimStatus{ + Phase: core.ClaimPending, + Conditions: []core.PersistentVolumeClaimCondition{ + {Type: core.PersistentVolumeClaimResizing, Status: core.ConditionTrue}, + }, + AllocatedResources: core.ResourceList{ + invalidNativeResizeKey: resource.MustParse("14G"), + }, + }) + + validExternalAllocatedResource := testVolumeClaimWithStatus("foo", "ns", core.PersistentVolumeClaimSpec{ + AccessModes: []core.PersistentVolumeAccessMode{ + core.ReadWriteOnce, + core.ReadOnlyMany, + }, + Resources: core.ResourceRequirements{ + Requests: core.ResourceList{ + core.ResourceName(core.ResourceStorage): resource.MustParse("10G"), + }, + }, + }, core.PersistentVolumeClaimStatus{ + Phase: core.ClaimPending, + Conditions: []core.PersistentVolumeClaimCondition{ + {Type: core.PersistentVolumeClaimResizing, Status: core.ConditionTrue}, + }, + AllocatedResources: core.ResourceList{ + validResizeKeyCustom: resource.MustParse("14G"), + }, }) scenarios := map[string]struct { @@ -18344,6 +18451,21 @@ func TestValidatePersistentVolumeClaimStatusUpdate(t *testing.T) { 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, + }, + "status-update-with-invalid-allocatedResources-feature-enabled": { isExpectedFailure: true, oldClaim: validClaim, @@ -18358,6 +18480,48 @@ func TestValidatePersistentVolumeClaimStatusUpdate(t *testing.T) { 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-valid-pvc-resize-status": { isExpectedFailure: false, oldClaim: validClaim, diff --git a/pkg/kubelet/volumemanager/cache/desired_state_of_world.go b/pkg/kubelet/volumemanager/cache/desired_state_of_world.go index c8cab650ca0..2a7abe23c94 100644 --- a/pkg/kubelet/volumemanager/cache/desired_state_of_world.go +++ b/pkg/kubelet/volumemanager/cache/desired_state_of_world.go @@ -587,7 +587,7 @@ func (dsw *desiredStateOfWorld) GetVolumesToMount() []VolumeToMount { }, } if volumeObj.persistentVolumeSize != nil { - vmt.PersistentVolumeSize = volumeObj.persistentVolumeSize.DeepCopy() + vmt.DesiredPersistentVolumeSize = volumeObj.persistentVolumeSize.DeepCopy() } volumesToMount = append(volumesToMount, vmt) } diff --git a/pkg/kubelet/volumemanager/reconciler/reconciler_common.go b/pkg/kubelet/volumemanager/reconciler/reconciler_common.go index 512b3a30e6d..b895f943fd8 100644 --- a/pkg/kubelet/volumemanager/reconciler/reconciler_common.go +++ b/pkg/kubelet/volumemanager/reconciler/reconciler_common.go @@ -178,7 +178,7 @@ func (rc *reconciler) unmountVolumes() { func (rc *reconciler) mountOrAttachVolumes() { // Ensure volumes that should be attached/mounted are attached/mounted. for _, volumeToMount := range rc.desiredStateOfWorld.GetVolumesToMount() { - volMounted, devicePath, err := rc.actualStateOfWorld.PodExistsInVolume(volumeToMount.PodName, volumeToMount.VolumeName, volumeToMount.PersistentVolumeSize, volumeToMount.SELinuxLabel) + volMounted, devicePath, err := rc.actualStateOfWorld.PodExistsInVolume(volumeToMount.PodName, volumeToMount.VolumeName, volumeToMount.DesiredPersistentVolumeSize, volumeToMount.SELinuxLabel) volumeToMount.DevicePath = devicePath if cache.IsSELinuxMountMismatchError(err) { // The volume is mounted, but with an unexpected SELinux context. diff --git a/pkg/volume/util/operationexecutor/node_expander.go b/pkg/volume/util/operationexecutor/node_expander.go index a1beedec65e..cf9c57504e8 100644 --- a/pkg/volume/util/operationexecutor/node_expander.go +++ b/pkg/volume/util/operationexecutor/node_expander.go @@ -37,7 +37,7 @@ type NodeExpander struct { // computed via precheck pvcStatusCap resource.Quantity pvCap resource.Quantity - resizeStatus *v1.PersistentVolumeClaimResizeStatus + resizeStatus v1.ClaimResourceStatus // pvcAlreadyUpdated if true indicates that although we are calling NodeExpandVolume on the kubelet // PVC has already been updated - possibly because expansion already succeeded on different node. @@ -68,29 +68,37 @@ type testResponseData struct { } // runPreCheck performs some sanity checks before expansion can be performed on the PVC. +// This function returns true only if node expansion is allowed to proceed otherwise +// it returns false. func (ne *NodeExpander) runPreCheck() bool { ne.pvcStatusCap = ne.pvc.Status.Capacity[v1.ResourceStorage] ne.pvCap = ne.pv.Spec.Capacity[v1.ResourceStorage] - ne.resizeStatus = ne.pvc.Status.ResizeStatus + allocatedResourceStatus := ne.pvc.Status.AllocatedResourceStatuses + if currentStatus, ok := allocatedResourceStatus[v1.ResourceStorage]; ok { + ne.resizeStatus = currentStatus + } // PVC is already expanded but we are still trying to expand the volume because // last recorded size in ASOW is older. This can happen for RWX volume types. - if ne.pvcStatusCap.Cmp(ne.pluginResizeOpts.NewSize) >= 0 && (ne.resizeStatus == nil || *ne.resizeStatus == v1.PersistentVolumeClaimNoExpansionInProgress) { + if ne.pvcStatusCap.Cmp(ne.pluginResizeOpts.NewSize) >= 0 && ne.resizeStatus == "" { ne.pvcAlreadyUpdated = true - } - - // if resizestatus is nil or NodeExpansionInProgress or NodeExpansionPending then we - // should allow volume expansion on the node to proceed. We are making an exception for - // resizeStatus being nil because it will support use cases where - // resizeStatus may not be set (old control-plane expansion controller etc). - if ne.resizeStatus == nil || - ne.pvcAlreadyUpdated || - *ne.resizeStatus == v1.PersistentVolumeClaimNodeExpansionPending || - *ne.resizeStatus == v1.PersistentVolumeClaimNodeExpansionInProgress { return true } + // recovery features will only work for newer version of resize controller + if ne.resizeStatus == "" { + return false + } + + resizeStatusVal := ne.resizeStatus + + // if resizestatus is nil or NodeExpansionInProgress or NodeExpansionPending then we + // should allow volume expansion on the node to proceed. + if resizeStatusVal == v1.PersistentVolumeClaimNodeResizePending || + resizeStatusVal == v1.PersistentVolumeClaimNodeResizeInProgress { + return true + } return false } diff --git a/pkg/volume/util/operationexecutor/node_expander_test.go b/pkg/volume/util/operationexecutor/node_expander_test.go index bf2be4b45b5..d0733b0ba02 100644 --- a/pkg/volume/util/operationexecutor/node_expander_test.go +++ b/pkg/volume/util/operationexecutor/node_expander_test.go @@ -17,6 +17,8 @@ limitations under the License. package operationexecutor import ( + "testing" + v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/resource" utilfeature "k8s.io/apiserver/pkg/util/feature" @@ -24,10 +26,12 @@ import ( "k8s.io/kubernetes/pkg/features" "k8s.io/kubernetes/pkg/volume" volumetesting "k8s.io/kubernetes/pkg/volume/testing" - "testing" ) func TestNodeExpander(t *testing.T) { + nodeResizeFailed := v1.PersistentVolumeClaimNodeResizeFailed + + nodeResizePending := v1.PersistentVolumeClaimNodeResizePending var tests = []struct { name string pvc *v1.PersistentVolumeClaim @@ -39,7 +43,7 @@ func TestNodeExpander(t *testing.T) { actualSize *resource.Quantity // expectations of test - expectedResizeStatus v1.PersistentVolumeClaimResizeStatus + expectedResizeStatus v1.ClaimResourceStatus expectedStatusSize resource.Quantity expectResizeCall bool assumeResizeOpAsFinished bool @@ -47,39 +51,39 @@ func TestNodeExpander(t *testing.T) { }{ { name: "pv.spec.cap > pvc.status.cap, resizeStatus=node_expansion_failed", - pvc: getTestPVC("test-vol0", "2G", "1G", "", v1.PersistentVolumeClaimNodeExpansionFailed), + pvc: getTestPVC("test-vol0", "2G", "1G", "", &nodeResizeFailed), pv: getTestPV("test-vol0", "2G"), - expectedResizeStatus: v1.PersistentVolumeClaimNodeExpansionFailed, + expectedResizeStatus: nodeResizeFailed, expectResizeCall: false, assumeResizeOpAsFinished: true, expectedStatusSize: resource.MustParse("1G"), }, { name: "pv.spec.cap > pvc.status.cap, resizeStatus=node_expansion_pending", - pvc: getTestPVC("test-vol0", "2G", "1G", "2G", v1.PersistentVolumeClaimNodeExpansionPending), + pvc: getTestPVC("test-vol0", "2G", "1G", "2G", &nodeResizePending), pv: getTestPV("test-vol0", "2G"), - expectedResizeStatus: v1.PersistentVolumeClaimNoExpansionInProgress, + expectedResizeStatus: "", expectResizeCall: true, assumeResizeOpAsFinished: true, expectedStatusSize: resource.MustParse("2G"), }, { name: "pv.spec.cap > pvc.status.cap, resizeStatus=node_expansion_pending, reize_op=failing", - pvc: getTestPVC(volumetesting.AlwaysFailNodeExpansion, "2G", "1G", "2G", v1.PersistentVolumeClaimNodeExpansionPending), + pvc: getTestPVC(volumetesting.AlwaysFailNodeExpansion, "2G", "1G", "2G", &nodeResizePending), pv: getTestPV(volumetesting.AlwaysFailNodeExpansion, "2G"), expectError: true, - expectedResizeStatus: v1.PersistentVolumeClaimNodeExpansionFailed, + expectedResizeStatus: nodeResizeFailed, expectResizeCall: true, assumeResizeOpAsFinished: true, expectedStatusSize: resource.MustParse("1G"), }, { name: "pv.spec.cap = pvc.status.cap, resizeStatus='', desiredSize > actualSize", - pvc: getTestPVC("test-vol0", "2G", "2G", "2G", v1.PersistentVolumeClaimNoExpansionInProgress), + pvc: getTestPVC("test-vol0", "2G", "2G", "2G", nil), pv: getTestPV("test-vol0", "2G"), - expectedResizeStatus: v1.PersistentVolumeClaimNoExpansionInProgress, + expectedResizeStatus: "", expectResizeCall: true, assumeResizeOpAsFinished: true, expectedStatusSize: resource.MustParse("2G"), @@ -143,8 +147,11 @@ func TestNodeExpander(t *testing.T) { if test.assumeResizeOpAsFinished != expansionResponse.assumeResizeFinished { t.Errorf("For test %s, expected assumeResizeOpAsFinished %t, got %t", test.name, test.assumeResizeOpAsFinished, expansionResponse.assumeResizeFinished) } - if test.expectedResizeStatus != *pvc.Status.ResizeStatus { - t.Errorf("For test %s, expected resizeStatus %v, got %v", test.name, test.expectedResizeStatus, *pvc.Status.ResizeStatus) + allocatedResourceStatus := pvc.Status.AllocatedResourceStatuses + resizeStatus := allocatedResourceStatus[v1.ResourceStorage] + + if test.expectedResizeStatus != resizeStatus { + t.Errorf("For test %s, expected resizeStatus %v, got %v", test.name, test.expectedResizeStatus, resizeStatus) } if pvcStatusCap.Cmp(test.expectedStatusSize) != 0 { t.Errorf("For test %s, expected status size %s, got %s", test.name, test.expectedStatusSize.String(), pvcStatusCap.String()) diff --git a/pkg/volume/util/operationexecutor/operation_executor.go b/pkg/volume/util/operationexecutor/operation_executor.go index f9b5e8160b9..f4d257b005a 100644 --- a/pkg/volume/util/operationexecutor/operation_executor.go +++ b/pkg/volume/util/operationexecutor/operation_executor.go @@ -440,9 +440,9 @@ type VolumeToMount struct { // time at which volume was requested to be mounted MountRequestTime time.Time - // PersistentVolumeSize stores desired size of the volume. + // DesiredPersistentVolumeSize stores desired size of the volume. // usually this is the size if pv.Spec.Capacity - PersistentVolumeSize resource.Quantity + DesiredPersistentVolumeSize resource.Quantity // SELinux label that should be used to mount. SELinuxLabel string diff --git a/pkg/volume/util/operationexecutor/operation_generator.go b/pkg/volume/util/operationexecutor/operation_generator.go index af6633ed590..bd59e865cbe 100644 --- a/pkg/volume/util/operationexecutor/operation_generator.go +++ b/pkg/volume/util/operationexecutor/operation_generator.go @@ -1781,12 +1781,14 @@ func (og *operationGenerator) expandAndRecoverFunction(resizeOpts inTreeResizeOp resizeCalled: false, } - // by default we are expanding to full-fill size requested in pvc.Spec.Resources + // by default we are expanding to fulfill size requested in pvc.Spec.Resources newSize := pvcSpecSize - resizeStatus := v1.PersistentVolumeClaimNoExpansionInProgress - if pvc.Status.ResizeStatus != nil { - resizeStatus = *pvc.Status.ResizeStatus + + var resizeStatus v1.ClaimResourceStatus + if status, ok := pvc.Status.AllocatedResourceStatuses[v1.ResourceStorage]; ok { + resizeStatus = status } + var allocatedSize *resource.Quantity t, ok := pvc.Status.AllocatedResources[v1.ResourceStorage] if ok { @@ -1798,10 +1800,10 @@ func (og *operationGenerator) expandAndRecoverFunction(resizeOpts inTreeResizeOp // pv is not of requested size yet and hence will require expanding switch resizeStatus { - case v1.PersistentVolumeClaimControllerExpansionInProgress: - case v1.PersistentVolumeClaimNodeExpansionPending: - case v1.PersistentVolumeClaimNodeExpansionInProgress: - case v1.PersistentVolumeClaimNodeExpansionFailed: + case v1.PersistentVolumeClaimControllerResizeInProgress, + v1.PersistentVolumeClaimNodeResizePending, + v1.PersistentVolumeClaimNodeResizeInProgress, + v1.PersistentVolumeClaimNodeResizeFailed: if allocatedSize != nil { newSize = *allocatedSize } @@ -1820,30 +1822,29 @@ func (og *operationGenerator) expandAndRecoverFunction(resizeOpts inTreeResizeOp // safe to do so. // 4. While expansion was still pending on the node, user reduced the pvc size. switch resizeStatus { - case v1.PersistentVolumeClaimNodeExpansionInProgress: - case v1.PersistentVolumeClaimNodeExpansionPending: + case v1.PersistentVolumeClaimNodeResizeInProgress, + v1.PersistentVolumeClaimNodeResizePending: // we don't need to do any work. We could be here because of a spurious update event. // This is case #1 return resizeResponse - case v1.PersistentVolumeClaimNodeExpansionFailed: + case v1.PersistentVolumeClaimNodeResizeFailed: // This is case#3 pvc, err = og.markForPendingNodeExpansion(pvc, pv) resizeResponse.pvc = pvc resizeResponse.err = err return resizeResponse - case v1.PersistentVolumeClaimControllerExpansionInProgress: - case v1.PersistentVolumeClaimControllerExpansionFailed: - case v1.PersistentVolumeClaimNoExpansionInProgress: + case v1.PersistentVolumeClaimControllerResizeInProgress, + v1.PersistentVolumeClaimControllerResizeFailed: // This is case#2 or it could also be case#4 when user manually shrunk the PVC // after expanding it. if allocatedSize != nil { newSize = *allocatedSize } default: - // It is impossible for ResizeStatus to be nil and allocatedSize to be not nil but somehow + // It is impossible for ResizeStatus to be "" and allocatedSize to be not nil but somehow // if we do end up in this state, it is safest to resume expansion to last recorded size in // allocatedSize variable. - if pvc.Status.ResizeStatus == nil && allocatedSize != nil { + if resizeStatus == "" && allocatedSize != nil { newSize = *allocatedSize } else { newSize = pvcSpecSize @@ -1938,7 +1939,7 @@ func (og *operationGenerator) GenerateExpandInUseVolumeFunc( var eventErr, detailedErr error migrated := false - if currentSize.IsZero() || volumeToMount.PersistentVolumeSize.IsZero() { + if currentSize.IsZero() || volumeToMount.DesiredPersistentVolumeSize.IsZero() { err := fmt.Errorf("current or new size of the volume is not set") eventErr, detailedErr = volumeToMount.GenerateError("NodeExpandvolume.expansion failed", err) return volumetypes.NewOperationContext(eventErr, detailedErr, migrated) @@ -1948,7 +1949,7 @@ func (og *operationGenerator) GenerateExpandInUseVolumeFunc( VolumeSpec: volumeToMount.VolumeSpec, DevicePath: volumeToMount.DevicePath, OldSize: currentSize, - NewSize: volumeToMount.PersistentVolumeSize, + NewSize: volumeToMount.DesiredPersistentVolumeSize, } fsVolume, err := util.CheckVolumeModeFilesystem(volumeToMount.VolumeSpec) if err != nil { @@ -2168,7 +2169,7 @@ func (og *operationGenerator) nodeExpandVolume( } func (og *operationGenerator) checkForRecoveryFromExpansion(pvc *v1.PersistentVolumeClaim, volumeToMount VolumeToMount) bool { - resizeStatus := pvc.Status.ResizeStatus + resizeStatus := pvc.Status.AllocatedResourceStatuses[v1.ResourceStorage] allocatedResource := pvc.Status.AllocatedResources featureGateStatus := utilfeature.DefaultFeatureGate.Enabled(features.RecoverVolumeExpansionFailure) @@ -2179,7 +2180,7 @@ func (og *operationGenerator) checkForRecoveryFromExpansion(pvc *v1.PersistentVo // Even though RecoverVolumeExpansionFailure feature gate is enabled, it appears that we are running with older version // of resize controller, which will not populate allocatedResource and resizeStatus. This can happen because of version skew // and hence we are going to keep expanding using older logic. - if resizeStatus == nil && allocatedResource == nil { + if resizeStatus == "" && allocatedResource == nil { _, detailedMsg := volumeToMount.GenerateMsg("MountVolume.NodeExpandVolume running with", "older external resize controller") klog.Warningf(detailedMsg) return false diff --git a/pkg/volume/util/operationexecutor/operation_generator_test.go b/pkg/volume/util/operationexecutor/operation_generator_test.go index 3fba64e78a4..281e1db3059 100644 --- a/pkg/volume/util/operationexecutor/operation_generator_test.go +++ b/pkg/volume/util/operationexecutor/operation_generator_test.go @@ -93,6 +93,8 @@ func TestOperationGenerator_GenerateUnmapVolumeFunc_PluginName(t *testing.T) { } func TestOperationGenerator_GenerateExpandAndRecoverVolumeFunc(t *testing.T) { + nodeResizePending := v1.PersistentVolumeClaimNodeResizePending + nodeResizeFailed := v1.PersistentVolumeClaimNodeResizeFailed var tests = []struct { name string pvc *v1.PersistentVolumeClaim @@ -100,53 +102,53 @@ func TestOperationGenerator_GenerateExpandAndRecoverVolumeFunc(t *testing.T) { recoverFeatureGate bool disableNodeExpansion bool // expectations of test - expectedResizeStatus v1.PersistentVolumeClaimResizeStatus + expectedResizeStatus v1.ClaimResourceStatus expectedAllocatedSize resource.Quantity expectResizeCall bool }{ { name: "pvc.spec.size > pv.spec.size, recover_expansion=on", - pvc: getTestPVC("test-vol0", "2G", "1G", "", v1.PersistentVolumeClaimNoExpansionInProgress), + pvc: getTestPVC("test-vol0", "2G", "1G", "", nil), pv: getTestPV("test-vol0", "1G"), recoverFeatureGate: true, - expectedResizeStatus: v1.PersistentVolumeClaimNodeExpansionPending, + expectedResizeStatus: v1.PersistentVolumeClaimNodeResizePending, expectedAllocatedSize: resource.MustParse("2G"), expectResizeCall: true, }, { name: "pvc.spec.size = pv.spec.size, recover_expansion=on", - pvc: getTestPVC("test-vol0", "1G", "1G", "", v1.PersistentVolumeClaimNoExpansionInProgress), + pvc: getTestPVC("test-vol0", "1G", "1G", "", nil), pv: getTestPV("test-vol0", "1G"), recoverFeatureGate: true, - expectedResizeStatus: v1.PersistentVolumeClaimNodeExpansionPending, + expectedResizeStatus: v1.PersistentVolumeClaimNodeResizePending, expectedAllocatedSize: resource.MustParse("1G"), expectResizeCall: true, }, { name: "pvc.spec.size = pv.spec.size, recover_expansion=on", - pvc: getTestPVC("test-vol0", "1G", "1G", "1G", v1.PersistentVolumeClaimNodeExpansionPending), + pvc: getTestPVC("test-vol0", "1G", "1G", "1G", &nodeResizePending), pv: getTestPV("test-vol0", "1G"), recoverFeatureGate: true, - expectedResizeStatus: v1.PersistentVolumeClaimNodeExpansionPending, + expectedResizeStatus: v1.PersistentVolumeClaimNodeResizePending, expectedAllocatedSize: resource.MustParse("1G"), expectResizeCall: false, }, { name: "pvc.spec.size > pv.spec.size, recover_expansion=on, disable_node_expansion=true", - pvc: getTestPVC("test-vol0", "2G", "1G", "", v1.PersistentVolumeClaimNoExpansionInProgress), + pvc: getTestPVC("test-vol0", "2G", "1G", "", nil), pv: getTestPV("test-vol0", "1G"), disableNodeExpansion: true, recoverFeatureGate: true, - expectedResizeStatus: v1.PersistentVolumeClaimNoExpansionInProgress, + expectedResizeStatus: "", expectedAllocatedSize: resource.MustParse("2G"), expectResizeCall: true, }, { name: "pv.spec.size >= pvc.spec.size, recover_expansion=on, resize_status=node_expansion_failed", - pvc: getTestPVC("test-vol0", "2G", "1G", "2G", v1.PersistentVolumeClaimNodeExpansionFailed), + pvc: getTestPVC("test-vol0", "2G", "1G", "2G", &nodeResizeFailed), pv: getTestPV("test-vol0", "2G"), recoverFeatureGate: true, - expectedResizeStatus: v1.PersistentVolumeClaimNodeExpansionPending, + expectedResizeStatus: v1.PersistentVolumeClaimNodeResizePending, expectedAllocatedSize: resource.MustParse("2G"), expectResizeCall: false, }, @@ -173,7 +175,8 @@ func TestOperationGenerator_GenerateExpandAndRecoverVolumeFunc(t *testing.T) { t.Fatalf("GenerateExpandAndRecoverVolumeFunc failed: %v", expansionResponse.err) } updatedPVC := expansionResponse.pvc - assert.Equal(t, *updatedPVC.Status.ResizeStatus, test.expectedResizeStatus) + actualResizeStatus := updatedPVC.Status.AllocatedResourceStatuses[v1.ResourceStorage] + assert.Equal(t, actualResizeStatus, test.expectedResizeStatus) actualAllocatedSize := updatedPVC.Status.AllocatedResources.Storage() if test.expectedAllocatedSize.Cmp(*actualAllocatedSize) != 0 { t.Fatalf("GenerateExpandAndRecoverVolumeFunc failed: expected allocated size %s, got %s", test.expectedAllocatedSize.String(), actualAllocatedSize.String()) @@ -191,6 +194,8 @@ func TestOperationGenerator_nodeExpandVolume(t *testing.T) { return &x } + nodeResizeFailed := v1.PersistentVolumeClaimNodeResizeFailed + nodeResizePending := v1.PersistentVolumeClaimNodeResizePending var tests = []struct { name string pvc *v1.PersistentVolumeClaim @@ -202,65 +207,65 @@ func TestOperationGenerator_nodeExpandVolume(t *testing.T) { actualSize *resource.Quantity // expectations of test - expectedResizeStatus v1.PersistentVolumeClaimResizeStatus + expectedResizeStatus v1.ClaimResourceStatus expectedStatusSize resource.Quantity resizeCallCount int expectError bool }{ { name: "pv.spec.cap > pvc.status.cap, resizeStatus=node_expansion_failed", - pvc: getTestPVC("test-vol0", "2G", "1G", "", v1.PersistentVolumeClaimNodeExpansionFailed), + pvc: getTestPVC("test-vol0", "2G", "1G", "", &nodeResizeFailed), pv: getTestPV("test-vol0", "2G"), - expectedResizeStatus: v1.PersistentVolumeClaimNodeExpansionFailed, + expectedResizeStatus: v1.PersistentVolumeClaimNodeResizeFailed, resizeCallCount: 0, expectedStatusSize: resource.MustParse("1G"), }, { name: "pv.spec.cap > pvc.status.cap, resizeStatus=node_expansion_pending", - pvc: getTestPVC("test-vol0", "2G", "1G", "2G", v1.PersistentVolumeClaimNodeExpansionPending), + pvc: getTestPVC("test-vol0", "2G", "1G", "2G", &nodeResizePending), pv: getTestPV("test-vol0", "2G"), - expectedResizeStatus: v1.PersistentVolumeClaimNoExpansionInProgress, + expectedResizeStatus: "", resizeCallCount: 1, expectedStatusSize: resource.MustParse("2G"), }, { name: "pv.spec.cap > pvc.status.cap, resizeStatus=node_expansion_pending, reize_op=failing", - pvc: getTestPVC(volumetesting.AlwaysFailNodeExpansion, "2G", "1G", "2G", v1.PersistentVolumeClaimNodeExpansionPending), + pvc: getTestPVC(volumetesting.AlwaysFailNodeExpansion, "2G", "1G", "2G", &nodeResizePending), pv: getTestPV(volumetesting.AlwaysFailNodeExpansion, "2G"), expectError: true, - expectedResizeStatus: v1.PersistentVolumeClaimNodeExpansionFailed, + expectedResizeStatus: v1.PersistentVolumeClaimNodeResizeFailed, resizeCallCount: 1, expectedStatusSize: resource.MustParse("1G"), }, { name: "pv.spec.cap = pvc.status.cap, resizeStatus='', desiredSize = actualSize", - pvc: getTestPVC("test-vol0", "2G", "2G", "2G", v1.PersistentVolumeClaimNoExpansionInProgress), + pvc: getTestPVC("test-vol0", "2G", "2G", "2G", nil), pv: getTestPV("test-vol0", "2G"), - expectedResizeStatus: v1.PersistentVolumeClaimNoExpansionInProgress, + expectedResizeStatus: "", resizeCallCount: 0, expectedStatusSize: resource.MustParse("2G"), }, { name: "pv.spec.cap = pvc.status.cap, resizeStatus='', desiredSize > actualSize", - pvc: getTestPVC("test-vol0", "2G", "2G", "2G", v1.PersistentVolumeClaimNoExpansionInProgress), + pvc: getTestPVC("test-vol0", "2G", "2G", "2G", nil), pv: getTestPV("test-vol0", "2G"), desiredSize: getSizeFunc("2G"), actualSize: getSizeFunc("1G"), - expectedResizeStatus: v1.PersistentVolumeClaimNoExpansionInProgress, + expectedResizeStatus: "", resizeCallCount: 1, expectedStatusSize: resource.MustParse("2G"), }, { name: "pv.spec.cap = pvc.status.cap, resizeStatus=node-expansion-failed, desiredSize > actualSize", - pvc: getTestPVC("test-vol0", "2G", "2G", "2G", v1.PersistentVolumeClaimNodeExpansionFailed), + pvc: getTestPVC("test-vol0", "2G", "2G", "2G", &nodeResizeFailed), pv: getTestPV("test-vol0", "2G"), desiredSize: getSizeFunc("2G"), actualSize: getSizeFunc("1G"), - expectedResizeStatus: v1.PersistentVolumeClaimNodeExpansionFailed, + expectedResizeStatus: v1.PersistentVolumeClaimNodeResizeFailed, resizeCallCount: 0, expectedStatusSize: resource.MustParse("2G"), }, @@ -336,7 +341,7 @@ func getTestPod(podName, pvcName string) *v1.Pod { } } -func getTestPVC(volumeName string, specSize, statusSize, allocatedSize string, resizeStatus v1.PersistentVolumeClaimResizeStatus) *v1.PersistentVolumeClaim { +func getTestPVC(volumeName string, specSize, statusSize, allocatedSize string, resizeStatus *v1.ClaimResourceStatus) *v1.PersistentVolumeClaim { pvc := &v1.PersistentVolumeClaim{ ObjectMeta: metav1.ObjectMeta{ Name: "claim01", @@ -358,7 +363,11 @@ func getTestPVC(volumeName string, specSize, statusSize, allocatedSize string, r if len(allocatedSize) > 0 { pvc.Status.AllocatedResources = v1.ResourceList{v1.ResourceStorage: resource.MustParse(allocatedSize)} } - pvc.Status.ResizeStatus = &resizeStatus + if resizeStatus != nil { + pvc.Status.AllocatedResourceStatuses = map[v1.ResourceName]v1.ClaimResourceStatus{ + v1.ResourceStorage: *resizeStatus, + } + } return pvc } diff --git a/pkg/volume/util/resize_util.go b/pkg/volume/util/resize_util.go index d6d028b0d92..0f1495f7b34 100644 --- a/pkg/volume/util/resize_util.go +++ b/pkg/volume/util/resize_util.go @@ -152,12 +152,11 @@ func MarkControllerReisizeInProgress(pvc *v1.PersistentVolumeClaim, resizerName Status: v1.ConditionTrue, LastTransitionTime: metav1.Now(), } - controllerExpansionInProgress := v1.PersistentVolumeClaimControllerExpansionInProgress conditions := []v1.PersistentVolumeClaimCondition{progressCondition} newPVC := pvc.DeepCopy() newPVC = MergeResizeConditionOnPVC(newPVC, conditions) - newPVC.Status.ResizeStatus = &controllerExpansionInProgress - newPVC.Status.AllocatedResources = v1.ResourceList{v1.ResourceStorage: newSize} + newPVC = mergeStorageResourceStatus(newPVC, v1.PersistentVolumeClaimControllerResizeInProgress) + newPVC = mergeStorageAllocatedResources(newPVC, newSize) newPVC = setResizer(newPVC, resizerName) return PatchPVCStatus(pvc /*oldPVC*/, newPVC, kubeClient) } @@ -192,10 +191,11 @@ func MarkForFSResize( } conditions := []v1.PersistentVolumeClaimCondition{pvcCondition} newPVC := pvc.DeepCopy() + if utilfeature.DefaultFeatureGate.Enabled(features.RecoverVolumeExpansionFailure) { - expansionPendingOnNode := v1.PersistentVolumeClaimNodeExpansionPending - newPVC.Status.ResizeStatus = &expansionPendingOnNode + newPVC = mergeStorageResourceStatus(newPVC, v1.PersistentVolumeClaimNodeResizePending) } + newPVC = MergeResizeConditionOnPVC(newPVC, conditions) updatedPVC, err := PatchPVCStatus(pvc /*oldPVC*/, newPVC, kubeClient) return updatedPVC, err @@ -220,8 +220,13 @@ func MarkFSResizeFinished( // if RecoverVolumeExpansionFailure is enabled, we need to reset ResizeStatus back to nil if utilfeature.DefaultFeatureGate.Enabled(features.RecoverVolumeExpansionFailure) { - expansionFinished := v1.PersistentVolumeClaimNoExpansionInProgress - newPVC.Status.ResizeStatus = &expansionFinished + allocatedResourceStatusMap := newPVC.Status.AllocatedResourceStatuses + delete(allocatedResourceStatusMap, v1.ResourceStorage) + if len(allocatedResourceStatusMap) == 0 { + newPVC.Status.AllocatedResourceStatuses = nil + } else { + newPVC.Status.AllocatedResourceStatuses = allocatedResourceStatusMap + } } newPVC = MergeResizeConditionOnPVC(newPVC, []v1.PersistentVolumeClaimCondition{}) @@ -232,9 +237,9 @@ func MarkFSResizeFinished( // MarkNodeExpansionFailed marks a PVC for node expansion as failed. Kubelet should not retry expansion // of volumes which are in failed state. func MarkNodeExpansionFailed(pvc *v1.PersistentVolumeClaim, kubeClient clientset.Interface) (*v1.PersistentVolumeClaim, error) { - expansionFailedOnNode := v1.PersistentVolumeClaimNodeExpansionFailed newPVC := pvc.DeepCopy() - newPVC.Status.ResizeStatus = &expansionFailedOnNode + newPVC = mergeStorageResourceStatus(newPVC, v1.PersistentVolumeClaimNodeResizeFailed) + patchBytes, err := createPVCPatch(pvc, newPVC, false /* addResourceVersionCheck */) if err != nil { return pvc, fmt.Errorf("patchPVCStatus failed to patch PVC %q: %v", pvc.Name, err) @@ -250,9 +255,8 @@ func MarkNodeExpansionFailed(pvc *v1.PersistentVolumeClaim, kubeClient clientset // MarkNodeExpansionInProgress marks pvc expansion in progress on node func MarkNodeExpansionInProgress(pvc *v1.PersistentVolumeClaim, kubeClient clientset.Interface) (*v1.PersistentVolumeClaim, error) { - nodeExpansionInProgress := v1.PersistentVolumeClaimNodeExpansionInProgress newPVC := pvc.DeepCopy() - newPVC.Status.ResizeStatus = &nodeExpansionInProgress + newPVC = mergeStorageResourceStatus(newPVC, v1.PersistentVolumeClaimNodeResizeInProgress) updatedPVC, err := PatchPVCStatus(pvc /* oldPVC */, newPVC, kubeClient) return updatedPVC, err } @@ -365,6 +369,32 @@ func MergeResizeConditionOnPVC( return pvc } +func mergeStorageResourceStatus(pvc *v1.PersistentVolumeClaim, status v1.ClaimResourceStatus) *v1.PersistentVolumeClaim { + allocatedResourceStatusMap := pvc.Status.AllocatedResourceStatuses + if allocatedResourceStatusMap == nil { + pvc.Status.AllocatedResourceStatuses = map[v1.ResourceName]v1.ClaimResourceStatus{ + v1.ResourceStorage: status, + } + return pvc + } + allocatedResourceStatusMap[v1.ResourceStorage] = status + pvc.Status.AllocatedResourceStatuses = allocatedResourceStatusMap + return pvc +} + +func mergeStorageAllocatedResources(pvc *v1.PersistentVolumeClaim, size resource.Quantity) *v1.PersistentVolumeClaim { + allocatedResourcesMap := pvc.Status.AllocatedResources + if allocatedResourcesMap == nil { + pvc.Status.AllocatedResources = map[v1.ResourceName]resource.Quantity{ + v1.ResourceStorage: size, + } + return pvc + } + allocatedResourcesMap[v1.ResourceStorage] = size + pvc.Status.AllocatedResources = allocatedResourcesMap + return pvc +} + // GenericResizeFS : call generic filesystem resizer for plugins that don't have any special filesystem resize requirements func GenericResizeFS(host volume.VolumeHost, pluginName, devicePath, deviceMountPath string) (bool, error) { resizer := mount.NewResizeFs(host.GetExec(pluginName)) diff --git a/plugin/pkg/admission/noderestriction/admission.go b/plugin/pkg/admission/noderestriction/admission.go index cb21f2360f4..64353380859 100644 --- a/plugin/pkg/admission/noderestriction/admission.go +++ b/plugin/pkg/admission/noderestriction/admission.go @@ -360,8 +360,8 @@ func (p *Plugin) admitPVCStatus(nodeName string, a admission.Attributes) error { newPVC.Status.Conditions = nil if p.expansionRecoveryEnabled { - oldPVC.Status.ResizeStatus = nil - newPVC.Status.ResizeStatus = nil + oldPVC.Status.AllocatedResourceStatuses = nil + newPVC.Status.AllocatedResourceStatuses = nil oldPVC.Status.AllocatedResources = nil newPVC.Status.AllocatedResources = nil diff --git a/plugin/pkg/admission/noderestriction/admission_test.go b/plugin/pkg/admission/noderestriction/admission_test.go index dbfec0e6ed8..412634d685b 100644 --- a/plugin/pkg/admission/noderestriction/admission_test.go +++ b/plugin/pkg/admission/noderestriction/admission_test.go @@ -1438,6 +1438,8 @@ func TestAdmitPVCStatus(t *testing.T) { noExistingPods := corev1lister.NewPodLister(noExistingPodsIndex) mynode := &user.DefaultInfo{Name: "system:node:mynode", Groups: []string{"system:nodes"}} + nodeExpansionFailed := api.PersistentVolumeClaimNodeResizeFailed + tests := []struct { name string resource schema.GroupVersionResource @@ -1452,11 +1454,11 @@ func TestAdmitPVCStatus(t *testing.T) { name: "should not allow full pvc update from nodes", oldObj: makeTestPVC( api.PersistentVolumeClaimResizing, - api.PersistentVolumeClaimNoExpansionInProgress, "10G", + "10G", nil, ), subresource: "", newObj: makeTestPVC( - "", api.PersistentVolumeClaimNoExpansionInProgress, "10G", + "", "10G", nil, ), expectError: "is forbidden: may only update PVC status", }, @@ -1464,13 +1466,13 @@ func TestAdmitPVCStatus(t *testing.T) { name: "should allow capacity and condition updates, if expansion is enabled", oldObj: makeTestPVC( api.PersistentVolumeClaimResizing, - api.PersistentVolumeClaimNoExpansionInProgress, "10G", + "10G", nil, ), expansionFeatureEnabled: true, subresource: "status", newObj: makeTestPVC( api.PersistentVolumeClaimFileSystemResizePending, - api.PersistentVolumeClaimNoExpansionInProgress, "10G", + "10G", nil, ), expectError: "", }, @@ -1478,13 +1480,13 @@ func TestAdmitPVCStatus(t *testing.T) { name: "should not allow updates to allocatedResources with just expansion enabled", oldObj: makeTestPVC( api.PersistentVolumeClaimResizing, - api.PersistentVolumeClaimNoExpansionInProgress, "10G", + "10G", nil, ), subresource: "status", expansionFeatureEnabled: true, newObj: makeTestPVC( api.PersistentVolumeClaimFileSystemResizePending, - api.PersistentVolumeClaimNoExpansionInProgress, "15G", + "15G", nil, ), expectError: "is not allowed to update fields other than", }, @@ -1492,14 +1494,14 @@ func TestAdmitPVCStatus(t *testing.T) { name: "should allow updates to allocatedResources with expansion and recovery enabled", oldObj: makeTestPVC( api.PersistentVolumeClaimResizing, - api.PersistentVolumeClaimNoExpansionInProgress, "10G", + "10G", nil, ), subresource: "status", expansionFeatureEnabled: true, recoveryFeatureEnabled: true, newObj: makeTestPVC( api.PersistentVolumeClaimFileSystemResizePending, - api.PersistentVolumeClaimNoExpansionInProgress, "15G", + "15G", nil, ), expectError: "", }, @@ -1507,14 +1509,14 @@ func TestAdmitPVCStatus(t *testing.T) { name: "should allow updates to resizeStatus with expansion and recovery enabled", oldObj: makeTestPVC( api.PersistentVolumeClaimResizing, - api.PersistentVolumeClaimNoExpansionInProgress, "10G", + "10G", nil, ), subresource: "status", expansionFeatureEnabled: true, recoveryFeatureEnabled: true, newObj: makeTestPVC( api.PersistentVolumeClaimResizing, - api.PersistentVolumeClaimNodeExpansionFailed, "10G", + "10G", &nodeExpansionFailed, ), expectError: "", }, @@ -1545,8 +1547,8 @@ func TestAdmitPVCStatus(t *testing.T) { func makeTestPVC( condition api.PersistentVolumeClaimConditionType, - resizeStatus api.PersistentVolumeClaimResizeStatus, - allocatedResources string) *api.PersistentVolumeClaim { + allocatedResources string, + resizeStatus *api.ClaimResourceStatus) *api.PersistentVolumeClaim { pvc := &api.PersistentVolumeClaim{ Spec: api.PersistentVolumeClaimSpec{ VolumeName: "volume1", @@ -1560,13 +1562,19 @@ func makeTestPVC( Capacity: api.ResourceList{ api.ResourceStorage: resource.MustParse(allocatedResources), }, - Phase: api.ClaimBound, - ResizeStatus: &resizeStatus, + Phase: api.ClaimBound, AllocatedResources: api.ResourceList{ api.ResourceStorage: resource.MustParse(allocatedResources), }, }, } + if resizeStatus != nil { + claimStatusMap := map[api.ResourceName]api.ClaimResourceStatus{ + api.ResourceStorage: *resizeStatus, + } + pvc.Status.AllocatedResourceStatuses = claimStatusMap + } + if len(condition) > 0 { pvc.Status.Conditions = []api.PersistentVolumeClaimCondition{ { diff --git a/staging/src/k8s.io/api/core/v1/types.go b/staging/src/k8s.io/api/core/v1/types.go index 0c9248307ca..bdac6164cb6 100644 --- a/staging/src/k8s.io/api/core/v1/types.go +++ b/staging/src/k8s.io/api/core/v1/types.go @@ -558,23 +558,23 @@ const ( ) // +enum -type PersistentVolumeClaimResizeStatus string +type ClaimResourceStatus string const ( - // When expansion is complete, the empty string is set by resize controller or kubelet. - PersistentVolumeClaimNoExpansionInProgress PersistentVolumeClaimResizeStatus = "" // State set when resize controller starts expanding the volume in control-plane - PersistentVolumeClaimControllerExpansionInProgress PersistentVolumeClaimResizeStatus = "ControllerExpansionInProgress" + PersistentVolumeClaimControllerResizeInProgress ClaimResourceStatus = "ControllerResizeInProgress" + // State set when expansion has failed in resize controller with a terminal error. - // Transient errors such as timeout should not set this status and should leave ResizeStatus + // Transient errors such as timeout should not set this status and should leave allocatedResourceStatus // unmodified, so as resize controller can resume the volume expansion. - PersistentVolumeClaimControllerExpansionFailed PersistentVolumeClaimResizeStatus = "ControllerExpansionFailed" + PersistentVolumeClaimControllerResizeFailed ClaimResourceStatus = "ControllerResizeFailed" + // State set when resize controller has finished expanding the volume but further expansion is needed on the node. - PersistentVolumeClaimNodeExpansionPending PersistentVolumeClaimResizeStatus = "NodeExpansionPending" + PersistentVolumeClaimNodeResizePending ClaimResourceStatus = "NodeResizePending" // State set when kubelet starts expanding the volume. - PersistentVolumeClaimNodeExpansionInProgress PersistentVolumeClaimResizeStatus = "NodeExpansionInProgress" + PersistentVolumeClaimNodeResizeInProgress ClaimResourceStatus = "NodeResizeInProgress" // State set when expansion has failed in kubelet with a terminal error. Transient errors don't set NodeExpansionFailed. - PersistentVolumeClaimNodeExpansionFailed PersistentVolumeClaimResizeStatus = "NodeExpansionFailed" + PersistentVolumeClaimNodeResizeFailed ClaimResourceStatus = "NodeResizeFailed" ) // PersistentVolumeClaimCondition contains details about state of pvc @@ -626,13 +626,23 @@ type PersistentVolumeClaimStatus struct { // +featureGate=RecoverVolumeExpansionFailure // +optional AllocatedResources ResourceList `json:"allocatedResources,omitempty" protobuf:"bytes,5,rep,name=allocatedResources,casttype=ResourceList,castkey=ResourceName"` - // resizeStatus stores status of resize operation. - // ResizeStatus is not set by default but when expansion is complete resizeStatus is set to empty - // string by resize controller or kubelet. + + // resizestatus is tombstoned since the field was replaced by allocatedResourceStatus + // 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: + // - 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. // This is an alpha field and requires enabling RecoverVolumeExpansionFailure feature. // +featureGate=RecoverVolumeExpansionFailure + // +mapType=granular // +optional - ResizeStatus *PersistentVolumeClaimResizeStatus `json:"resizeStatus,omitempty" protobuf:"bytes,6,opt,name=resizeStatus,casttype=PersistentVolumeClaimResizeStatus"` + AllocatedResourceStatuses map[ResourceName]ClaimResourceStatus `json:"allocatedResourceStatuses,omitempty" protobuf:"bytes,7,rep,name=allocatedResourceStatuses"` } // +enum diff --git a/test/e2e/storage/csi_mock/csi_volume_expansion.go b/test/e2e/storage/csi_mock/csi_volume_expansion.go index 2159b2d5f9d..a0a54a95799 100644 --- a/test/e2e/storage/csi_mock/csi_volume_expansion.go +++ b/test/e2e/storage/csi_mock/csi_volume_expansion.go @@ -63,7 +63,7 @@ type recoveryTest struct { pvcRequestSize string allocatedResource string simulatedCSIDriverError expansionStatus - expectedResizeStatus v1.PersistentVolumeClaimResizeStatus + expectedResizeStatus v1.ClaimResourceStatus recoverySize resource.Quantity } @@ -403,14 +403,14 @@ var _ = utils.SIGDescribe("CSI Mock volume expansion", func() { pvcRequestSize: "4Gi", allocatedResource: "4Gi", simulatedCSIDriverError: expansionSuccess, - expectedResizeStatus: v1.PersistentVolumeClaimNoExpansionInProgress, + expectedResizeStatus: "", }, { name: "should allow recovery if controller expansion fails with final error", pvcRequestSize: "11Gi", // expansion to 11Gi will cause expansion to fail on controller allocatedResource: "11Gi", simulatedCSIDriverError: expansionFailedOnController, - expectedResizeStatus: v1.PersistentVolumeClaimControllerExpansionFailed, + expectedResizeStatus: v1.PersistentVolumeClaimControllerResizeFailed, recoverySize: resource.MustParse("4Gi"), }, { @@ -418,7 +418,7 @@ var _ = utils.SIGDescribe("CSI Mock volume expansion", func() { pvcRequestSize: "9Gi", // expansion to 9Gi will cause expansion to fail on node allocatedResource: "9Gi", simulatedCSIDriverError: expansionFailedOnNode, - expectedResizeStatus: v1.PersistentVolumeClaimNodeExpansionFailed, + expectedResizeStatus: v1.PersistentVolumeClaimNodeResizeFailed, recoverySize: resource.MustParse("5Gi"), }, } @@ -499,7 +499,7 @@ func validateRecoveryBehaviour(ctx context.Context, pvc *v1.PersistentVolumeClai // if expansion succeeded on controller but failed on the node if test.simulatedCSIDriverError == expansionFailedOnNode { ginkgo.By("Wait for expansion to fail on node again") - err = waitForResizeStatus(pvc, m.cs, v1.PersistentVolumeClaimNodeExpansionFailed) + err = waitForResizeStatus(pvc, m.cs, v1.PersistentVolumeClaimNodeResizeFailed) framework.ExpectNoError(err, "While waiting for resize status to be set to expansion-failed-on-node") ginkgo.By("verify allocated resources after recovery") @@ -536,13 +536,13 @@ func validateExpansionSuccess(ctx context.Context, pvc *v1.PersistentVolumeClaim framework.Failf("expected allocated Resources to be %s got %s", expectedAllocatedResource.String(), allocatedResource.String()) } - resizeStatus := pvc.Status.ResizeStatus - gomega.Expect(resizeStatus).NotTo(gomega.BeNil(), "resize status should not be nil") - framework.ExpectEqual(*resizeStatus, v1.PersistentVolumeClaimNoExpansionInProgress, "resize status should be empty") + resizeStatus := pvc.Status.AllocatedResourceStatuses[v1.ResourceStorage] + framework.ExpectEqual(resizeStatus, "", "resize status should be empty") } -func waitForResizeStatus(pvc *v1.PersistentVolumeClaim, c clientset.Interface, expectedStates ...v1.PersistentVolumeClaimResizeStatus) error { - var actualResizeStatus *v1.PersistentVolumeClaimResizeStatus +func waitForResizeStatus(pvc *v1.PersistentVolumeClaim, c clientset.Interface, expectedState v1.ClaimResourceStatus) error { + var actualResizeStatus *v1.ClaimResourceStatus + waitErr := wait.PollImmediate(resizePollInterval, csiResizeWaitPeriod, func() (bool, error) { var err error updatedPVC, err := c.CoreV1().PersistentVolumeClaims(pvc.Namespace).Get(context.TODO(), pvc.Name, metav1.GetOptions{}) @@ -551,18 +551,11 @@ func waitForResizeStatus(pvc *v1.PersistentVolumeClaim, c clientset.Interface, e return false, fmt.Errorf("error fetching pvc %q for checking for resize status: %w", pvc.Name, err) } - actualResizeStatus = updatedPVC.Status.ResizeStatus - if actualResizeStatus != nil { - for _, s := range expectedStates { - if s == *actualResizeStatus { - return true, nil - } - } - } - return false, nil + actualResizeStatus := updatedPVC.Status.AllocatedResourceStatuses[v1.ResourceStorage] + return (actualResizeStatus == expectedState), nil }) if waitErr != nil { - return fmt.Errorf("error while waiting for resize status to sync to %+v, actualStatus %s: %v", expectedStates, *actualResizeStatus, waitErr) + return fmt.Errorf("error while waiting for resize status to sync to %v, actualStatus %s: %v", expectedState, *actualResizeStatus, waitErr) } return nil }