Address review comments and return nicer errors

This commit is contained in:
Hemant Kumar 2024-07-22 10:43:38 -04:00
parent 68df11f4f8
commit f7f1a6c81a
3 changed files with 10 additions and 8 deletions

View File

@ -121,7 +121,7 @@ func (c *csiPlugin) nodeExpandWithClient(
} }
if isInfeasibleError(err) { if isInfeasibleError(err) {
infeasibleError := fmt.Errorf("Expander.NodeExpand failed to expand the volume: %w", volumetypes.NewInfeasibleError(err.Error())) infeasibleError := volumetypes.NewInfeasibleError(fmt.Sprintf("Expander.NodeExpand failed to expand the volume %s", err.Error()))
return false, infeasibleError return false, infeasibleError
} }
return false, fmt.Errorf("Expander.NodeExpand failed to expand the volume : %w", err) return false, fmt.Errorf("Expander.NodeExpand failed to expand the volume : %w", err)

View File

@ -39,9 +39,9 @@ type NodeExpander struct {
pvCap resource.Quantity pvCap resource.Quantity
resizeStatus v1.ClaimResourceStatus resizeStatus v1.ClaimResourceStatus
// indicates that pvc spec size has actually changed since last we observed size // indicates that if volume expansion failed on the node, then current expansion should be marked
// on the kubelet // as infeasible so as controller can reconcile the resizing operation by using new user requested size.
markExpansionInfeasible bool markExpansionInfeasibleOnFailure bool
// pvcAlreadyUpdated if true indicates that although we are calling NodeExpandVolume on the kubelet // 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. // PVC has already been updated - possibly because expansion already succeeded on different node.
@ -84,12 +84,14 @@ func (ne *NodeExpander) runPreCheck() bool {
} }
pvcSpecCap := ne.pvc.Spec.Resources.Requests[v1.ResourceStorage] pvcSpecCap := ne.pvc.Spec.Resources.Requests[v1.ResourceStorage]
// usually when are performing node expansion, we expect pv size and pvc spec size // usually when are performing node expansion, we expect pv size and pvc spec size
// to be the same, but if user has edited pvc since then and volume expansion failed // to be the same, but if user has edited pvc since then and volume expansion failed
// with final error, then we should let controller reconcile this state. // with final error, then we should let controller reconcile this state, by marking entire
// node expansion as infeasible.
if pvcSpecCap.Cmp(ne.pluginResizeOpts.NewSize) != 0 && if pvcSpecCap.Cmp(ne.pluginResizeOpts.NewSize) != 0 &&
ne.actualStateOfWorld.CheckVolumeInFailedExpansionWithFinalErrors(ne.vmt.VolumeName) { ne.actualStateOfWorld.CheckVolumeInFailedExpansionWithFinalErrors(ne.vmt.VolumeName) {
ne.markExpansionInfeasible = true ne.markExpansionInfeasibleOnFailure = true
} }
// PVC is already expanded but we are still trying to expand the volume because // PVC is already expanded but we are still trying to expand the volume because
@ -138,7 +140,7 @@ func (ne *NodeExpander) expandOnPlugin() (bool, error, testResponseData) {
if volumetypes.IsOperationFinishedError(resizeErr) { if volumetypes.IsOperationFinishedError(resizeErr) {
var markFailedError error var markFailedError error
ne.actualStateOfWorld.MarkVolumeExpansionFailedWithFinalError(ne.vmt.VolumeName) ne.actualStateOfWorld.MarkVolumeExpansionFailedWithFinalError(ne.vmt.VolumeName)
if volumetypes.IsInfeasibleError(resizeErr) || ne.markExpansionInfeasible { if volumetypes.IsInfeasibleError(resizeErr) || ne.markExpansionInfeasibleOnFailure {
ne.pvc, markFailedError = util.MarkNodeExpansionInfeasible(ne.pvc, ne.kubeClient, resizeErr) ne.pvc, markFailedError = util.MarkNodeExpansionInfeasible(ne.pvc, ne.kubeClient, resizeErr)
if markFailedError != nil { if markFailedError != nil {
klog.Errorf(ne.vmt.GenerateErrorDetailed("MountMount.NodeExpandVolume failed to mark node expansion as failed: %v", err).Error()) klog.Errorf(ne.vmt.GenerateErrorDetailed("MountMount.NodeExpandVolume failed to mark node expansion as failed: %v", err).Error())

View File

@ -556,7 +556,7 @@ func waitForResizeStatus(ctx context.Context, pvc *v1.PersistentVolumeClaim, c c
return (actualResizeStatus == expectedState), nil return (actualResizeStatus == expectedState), nil
}) })
if waitErr != nil { if waitErr != nil {
return fmt.Errorf("error while waiting for resize status to sync to %v, actualStatus %s: %v", expectedState, actualResizeStatus, waitErr) return fmt.Errorf("error while waiting for resize status to sync to %v, actualStatus %s: %w", expectedState, actualResizeStatus, waitErr)
} }
return nil return nil
} }