Merge pull request #453 from AndrewSirenko/modifyInfeasibleSlowSet

Ensure infeasible PVC modifications are retried at slower pace

Author: k8s-ci-robot

cmd/csi-resizer/main.go

@@ -222,7 +222,7 @@ func main() {
 	var mc modifycontroller.ModifyController
 	// Add modify controller only if the feature gate is enabled
 	if utilfeature.DefaultFeatureGate.Enabled(features.VolumeAttributesClass) {
-		mc = modifycontroller.NewModifyController(modifierName, csiModifier, kubeClient, *resyncPeriod, *extraModifyMetadata, informerFactory,
+		mc = modifycontroller.NewModifyController(modifierName, csiModifier, kubeClient, *resyncPeriod, *retryIntervalMax, *extraModifyMetadata, informerFactory,
 			workqueue.NewTypedItemExponentialFailureRateLimiter[string](*retryIntervalStart, *retryIntervalMax))
 	}
 

pkg/csi/mock_client.go

@@ -39,7 +39,7 @@ type MockClient struct {
 	expandCalled                            atomic.Int32
 	modifyCalled                            atomic.Int32
 	expansionError                          error
-	modifyFailed                            bool
+	modifyError                             error
 	checkMigratedLabel                      bool
 	usedSecrets                             atomic.Pointer[map[string]string]
 	usedCapability                          atomic.Pointer[csi.VolumeCapability]
@@ -74,8 +74,8 @@ func (c *MockClient) SetExpansionError(err error) {
 	c.expansionError = err
 }
 
-func (c *MockClient) SetModifyFailed() {
-	c.modifyFailed = true
+func (c *MockClient) SetModifyError(err error) {
+	c.modifyError = err
 }
 
 func (c *MockClient) SetCheckMigratedLabel() {
@@ -135,8 +135,8 @@ func (c *MockClient) Modify(
 	secrets map[string]string,
 	mutableParameters map[string]string) error {
 	c.modifyCalled.Add(1)
-	if c.modifyFailed {
-		return fmt.Errorf("modify failed")
+	if c.modifyError != nil {
+		return c.modifyError
 	}
 	return nil
 }

pkg/modifycontroller/controller.go

@@ -61,6 +61,8 @@ type modifyController struct {
 	extraModifyMetadata bool
 	// the key of the map is {PVC_NAMESPACE}/{PVC_NAME}
 	uncertainPVCs map[string]v1.PersistentVolumeClaim
+	// slowSet tracks PVCs for which modification failed with infeasible error and should be retried at slower rate.
+	slowSet *util.SlowSet
 }
 
 // NewModifyController returns a ModifyController.
@@ -69,6 +71,7 @@ func NewModifyController(
 	modifier modifier.Modifier,
 	kubeClient kubernetes.Interface,
 	resyncPeriod time.Duration,
+	maxRetryInterval time.Duration,
 	extraModifyMetadata bool,
 	informerFactory informers.SharedInformerFactory,
 	pvcRateLimiter workqueue.TypedRateLimiter[string]) ModifyController {
@@ -99,8 +102,9 @@ func NewModifyController(
 		claimQueue:          claimQueue,
 		eventRecorder:       eventRecorder,
 		extraModifyMetadata: extraModifyMetadata,
+		slowSet:             util.NewSlowSet(maxRetryInterval),
 	}
-	// Add a resync period as the PVC's request modify can be modified again when we handling
+	// Add a resync period as the PVC's request modify can be modified again when we are handling
 	// a previous modify request of the same PVC.
 	pvcInformer.Informer().AddEventHandlerWithResyncPeriod(cache.ResourceEventHandlerFuncs{
 		AddFunc:    ctrl.addPVC,
@@ -211,6 +215,10 @@ func (ctrl *modifyController) Run(
 	}
 
 	stopCh := ctx.Done()
+
+	// Starts go-routine that deletes expired slowSet entries.
+	go ctrl.slowSet.Run(stopCh)
+
 	for i := 0; i < workers; i++ {
 		go wait.Until(ctrl.sync, 0, stopCh)
 	}
@@ -235,7 +243,7 @@ func (ctrl *modifyController) sync() {
 	}
 }
 
-// syncPVC checks if a pvc requests resizing, and execute the resize operation if requested.
+// syncPVC checks if a pvc requests modification, and execute the ModifyVolume operation if requested.
 func (ctrl *modifyController) syncPVC(key string) error {
 	klog.V(4).InfoS("Started PVC processing for modify controller", "key", key)
 
@@ -260,7 +268,7 @@ func (ctrl *modifyController) syncPVC(key string) error {
 	}
 
 	// Only trigger modify volume if the following conditions are met
-	// 1. Non empty vac name
+	// 1. Non-empty vac name
 	// 2. PVC is in Bound state
 	// 3. PV CSI driver name matches local driver
 	vacName := pvc.Spec.VolumeAttributesClassName