From f28a8d7f2b9f8c77f4c1c42e7794cee0e585dad9 Mon Sep 17 00:00:00 2001
From: torubylist <zyp2010sjtu@hotmail.com>
Date: Thu, 23 Sep 2021 21:31:23 +0800
Subject: [PATCH] fix:cached claim is not the newest will cause unexpected
 issue

---
 .../volume/persistentvolume/binder_test.go    | 15 +++++++++++++++
 .../volume/persistentvolume/pv_controller.go  | 19 ++++++++++++++++---
 2 files changed, 31 insertions(+), 3 deletions(-)

diff --git a/pkg/controller/volume/persistentvolume/binder_test.go b/pkg/controller/volume/persistentvolume/binder_test.go
index 5b143cac5b8..26843c6a9f0 100644
--- a/pkg/controller/volume/persistentvolume/binder_test.go
+++ b/pkg/controller/volume/persistentvolume/binder_test.go
@@ -563,6 +563,21 @@ func TestSync(t *testing.T) {
 			noevents, noerrors, testSyncVolume,
 		},
 
+		{
+			// syncVolume with volume bound to claim which exists in etcd but not updated to local cache.
+			"4-12 - volume bound to newest claim but not updated to local cache",
+			newVolumeArray("volume4-12", "10Gi", "uid4-12-new", "claim4-12", v1.VolumeAvailable, v1.PersistentVolumeReclaimDelete, classEmpty),
+			newVolumeArray("volume4-12", "10Gi", "uid4-12-new", "claim4-12", v1.VolumeBound, v1.PersistentVolumeReclaimDelete, classEmpty),
+			func() []*v1.PersistentVolumeClaim {
+				newClaim := newClaimArray("claim4-12", "uid4-12", "10Gi", "volume4-12", v1.ClaimBound, nil, "")
+				// update uid to new-uid and not sync to cache.
+				newClaim = append(newClaim, newClaimArray("claim4-12", "uid4-12-new", "10Gi", "volume4-12", v1.ClaimBound, nil, annSkipLocalStore)...)
+				return newClaim
+			}(),
+			newClaimArray("claim4-12", "uid4-12-new", "10Gi", "volume4-12", v1.ClaimBound, nil, annSkipLocalStore),
+			noevents, noerrors, testSyncVolume,
+		},
+
 		// PVC with class
 		{
 			// syncVolume binds a claim to requested class even if there is a
diff --git a/pkg/controller/volume/persistentvolume/pv_controller.go b/pkg/controller/volume/persistentvolume/pv_controller.go
index 0aae42cf746..8e3bb559bc0 100644
--- a/pkg/controller/volume/persistentvolume/pv_controller.go
+++ b/pkg/controller/volume/persistentvolume/pv_controller.go
@@ -622,9 +622,22 @@ func (ctrl *PersistentVolumeController) syncVolume(volume *v1.PersistentVolume)
 		if claim != nil && claim.UID != volume.Spec.ClaimRef.UID {
 			// The claim that the PV was pointing to was deleted, and another
 			// with the same name created.
-			klog.V(4).Infof("synchronizing PersistentVolume[%s]: claim %s has different UID, the old one must have been deleted", volume.Name, claimrefToClaimKey(volume.Spec.ClaimRef))
-			// Treat the volume as bound to a missing claim.
-			claim = nil
+			// in some cases, the cached claim is not the newest, and the volume.Spec.ClaimRef.UID is newer than cached.
+			// so we should double check by calling apiserver and get the newest claim, then compare them.
+			klog.V(4).Infof("Maybe cached claim: %s is not the newest one, we should fetch it from apiserver", claimrefToClaimKey(volume.Spec.ClaimRef))
+
+			claim, err = ctrl.kubeClient.CoreV1().PersistentVolumeClaims(volume.Spec.ClaimRef.Namespace).Get(context.TODO(), volume.Spec.ClaimRef.Name, metav1.GetOptions{})
+			if err != nil && !apierrors.IsNotFound(err) {
+				return err
+			} else if claim != nil {
+				// Treat the volume as bound to a missing claim.
+				if claim.UID != volume.Spec.ClaimRef.UID {
+					klog.V(4).Infof("synchronizing PersistentVolume[%s]: claim %s has a newer UID than pv.ClaimRef, the old one must have been deleted", volume.Name, claimrefToClaimKey(volume.Spec.ClaimRef))
+					claim = nil
+				} else {
+					klog.V(4).Infof("synchronizing PersistentVolume[%s]: claim %s has a same UID with pv.ClaimRef", volume.Name, claimrefToClaimKey(volume.Spec.ClaimRef))
+				}
+			}
 		}
 
 		if claim == nil {