From f5693a4008d28a3879cbabeb85898ebe62494908 Mon Sep 17 00:00:00 2001 From: Roman Bednar Date: Mon, 17 Oct 2022 14:05:34 +0200 Subject: [PATCH] restart claim sync when PVC is updated We should not rely on syncUnboundClaim() to do nothing after it updates PVC with a default storage class until next re-sync but instead restart the sync explicitly to make sure we hit isDelayBindingMode() and findBestMatchForClaim() immediately right after the PVC update. --- .../volume/persistentvolume/pv_controller.go | 22 ++++++++++++------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/pkg/controller/volume/persistentvolume/pv_controller.go b/pkg/controller/volume/persistentvolume/pv_controller.go index 20c61c038b1..ca98f91caaf 100644 --- a/pkg/controller/volume/persistentvolume/pv_controller.go +++ b/pkg/controller/volume/persistentvolume/pv_controller.go @@ -353,9 +353,14 @@ func (ctrl *PersistentVolumeController) syncUnboundClaim(ctx context.Context, cl if utilfeature.DefaultFeatureGate.Enabled(features.RetroactiveDefaultStorageClass) { klog.V(4).Infof("FeatureGate[%s] is enabled, attempting to assign storage class to unbound PersistentVolumeClaim[%s]", features.RetroactiveDefaultStorageClass, claimToClaimKey(claim)) - if claim, err = ctrl.assignDefaultStorageClass(claim); err != nil { + updated, err := ctrl.assignDefaultStorageClass(claim) + if err != nil { return fmt.Errorf("can't update PersistentVolumeClaim[%q]: %w", claimToClaimKey(claim), err) } + if updated { + klog.V(4).Infof("PersistentVolumeClaim[%q] update successful, restarting claim sync", claimToClaimKey(claim)) + return nil + } } switch { @@ -929,9 +934,9 @@ func (ctrl *PersistentVolumeController) updateVolumePhaseWithEvent(volume *v1.Pe // assignDefaultStorageClass updates the claim storage class if there is any, the claim is updated to the API server. // Ignores claims that already have a storage class. // TODO: if resync is ever changed to a larger period, we might need to change how we set the default class on existing unbound claims -func (ctrl *PersistentVolumeController) assignDefaultStorageClass(claim *v1.PersistentVolumeClaim) (*v1.PersistentVolumeClaim, error) { +func (ctrl *PersistentVolumeController) assignDefaultStorageClass(claim *v1.PersistentVolumeClaim) (bool, error) { if claim.Spec.StorageClassName != nil { - return claim, nil + return false, nil } class, err := util.GetDefaultClass(ctrl.classLister) @@ -939,20 +944,21 @@ func (ctrl *PersistentVolumeController) assignDefaultStorageClass(claim *v1.Pers // It is safe to ignore errors here because it means we either could not list SCs or there is more than one default. // TODO: do not ignore errors after this PR is merged: https://github.com/kubernetes/kubernetes/pull/110559 klog.V(4).Infof("failed to get default storage class: %v", err) - return claim, nil + return false, nil } else if class == nil { klog.V(4).Infof("can not assign storage class to PersistentVolumeClaim[%s]: default storage class not found", claimToClaimKey(claim)) - return claim, nil + return false, nil } klog.V(4).Infof("assigning StorageClass[%s] to PersistentVolumeClaim[%s]", class.Name, claimToClaimKey(claim)) claim.Spec.StorageClassName = &class.Name - newClaim, err := ctrl.kubeClient.CoreV1().PersistentVolumeClaims(claim.GetNamespace()).Update(context.TODO(), claim, metav1.UpdateOptions{}) + _, err = ctrl.kubeClient.CoreV1().PersistentVolumeClaims(claim.GetNamespace()).Update(context.TODO(), claim, metav1.UpdateOptions{}) if err != nil { - return claim, err + return false, err } - return newClaim, nil + klog.V(4).Infof("successfully assigned StorageClass[%s] to PersistentVolumeClaim[%s]", claimToClaimKey(claim), class.Name) + return true, nil } // bindVolumeToClaim modifies given volume to be bound to a claim and saves it to