fix volumeName update multiple times bug

Signed-off-by: stoneshi-yunify <[email protected]>
kubesphere · Nov 22, 2024 · 7b36adb · 7b36adb
1 parent ec4523e
commit 7b36adb
Show file tree

Hide file tree

Showing 2 changed files with 69 additions and 36 deletions.
diff --git a/internal/controller/nfs.go b/internal/controller/nfs.go
@@ -51,6 +51,10 @@ var newAnnotationInvalidError = func(annotation string) error {
 func (p NFSPVC) ParsePV() (*corev1.PersistentVolume, error) {
 	var err error
 
+	if p.Spec.VolumeName == "" {
+		return nil, fmt.Errorf("Spec.VolumeName cannot be empty")
+	}
+
 	server, ok := p.Annotations[AnnotationNFSServer]
 	if !ok || server == "" {
 		return nil, newAnnotationInvalidError(AnnotationNFSServer)
@@ -95,11 +99,6 @@ func (p NFSPVC) ParsePV() (*corev1.PersistentVolume, error) {
 		}
 	}
 
-	name, ok := p.Annotations[AnnotationPVName]
-	if !ok || name == "" {
-		name = fmt.Sprintf("pvc-%s", uuid.NewUUID())
-	}
-
 	var storageClassName string
 	if p.Spec.StorageClassName != nil {
 		storageClassName = *p.Spec.StorageClassName
@@ -111,7 +110,7 @@ func (p NFSPVC) ParsePV() (*corev1.PersistentVolume, error) {
 			APIVersion: "v1",
 		},
 		ObjectMeta: metav1.ObjectMeta{
-			Name: name,
+			Name: p.Spec.VolumeName,
 			Annotations: map[string]string{
 				AnnotationNFSStaticProvision: "true",
 			},
@@ -142,3 +141,16 @@ func (p NFSPVC) ParsePV() (*corev1.PersistentVolume, error) {
 
 	return pv, nil
 }
+
+func GetPVName(pvc *corev1.PersistentVolumeClaim) string {
+	if pvc.Spec.VolumeName != "" {
+		return pvc.Spec.VolumeName
+	}
+	if len(pvc.Annotations) > 0 {
+		name, ok := pvc.Annotations[AnnotationPVName]
+		if ok && name != "" {
+			return name
+		}
+	}
+	return fmt.Sprintf("pvc-%s", uuid.NewUUID())
+}
diff --git a/internal/controller/persistentvolumeclaim_controller.go b/internal/controller/persistentvolumeclaim_controller.go
@@ -19,6 +19,7 @@ package controller
 import (
 	"context"
 	"reflect"
+	"sync"
 
 	corev1 "k8s.io/api/core/v1"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
@@ -37,6 +38,7 @@ type PersistentVolumeClaimReconciler struct {
 	client.Client
 	Scheme   *runtime.Scheme
 	Recorder record.EventRecorder
+	lock     sync.Mutex
 }
 
 // +kubebuilder:rbac:groups=core,resources=events,verbs=get;list;watch;create;update;patch;delete
@@ -79,49 +81,68 @@ func (r *PersistentVolumeClaimReconciler) Reconcile(ctx context.Context, req ctr
 		return ctrl.Result{}, nil
 	}
 
+	if pvc.Spec.VolumeName == "" {
+		r.lock.Lock()
+		defer r.lock.Unlock()
+
+		// re-get the latest data
+		err = r.Get(ctx, req.NamespacedName, pvc)
+		if err != nil {
+			return ctrl.Result{}, err
+		}
+		if pvc.Spec.VolumeName == "" {
+			pvc.Spec.VolumeName = GetPVName(pvc)
+			err = r.Client.Update(ctx, pvc)
+			if err == nil {
+				r.Recorder.Eventf(pvc, corev1.EventTypeNormal, "VolumeNameUpdated", "volumeName updated successfully")
+			} else {
+				r.Recorder.Eventf(pvc, corev1.EventTypeWarning, "UpdateVolumeNameFailed", "failed to update volumeName, error: %s", err.Error())
+			}
+			return ctrl.Result{}, err
+		}
+	}
+
+	// re-get the latest data
+	err = r.Get(ctx, req.NamespacedName, pvc)
+	if err != nil {
+		return ctrl.Result{}, err
+	}
+
+	nfsPVC = NFSPVC(*pvc)
+
 	var pv *corev1.PersistentVolume
 	pv, err = nfsPVC.ParsePV()
 	if err != nil {
 		r.Recorder.Eventf(pvc, corev1.EventTypeWarning, "ParsePVFailed", "failed to parse pv from pvc, error: %s", err.Error())
 		return ctrl.Result{}, err
 	}
 
-	if pvc.Spec.VolumeName == "" {
-		pvc.Spec.VolumeName = pv.Name
-		err = r.Client.Update(ctx, pvc)
-		if err == nil {
-			r.Recorder.Eventf(pvc, corev1.EventTypeNormal, "VolumeNameUpdated", "volumeName updated successfully")
-		} else {
-			r.Recorder.Eventf(pvc, corev1.EventTypeWarning, "UpdateVolumeNameFailed", "failed to update volumeName, error: %s", err.Error())
+	pv2 := &corev1.PersistentVolume{}
+	err = r.Client.Get(ctx, types.NamespacedName{Name: pvc.Spec.VolumeName}, pv2)
+	if err == nil {
+		needUpdate := r.mergePV(pv2, pv)
+		if needUpdate {
+			err = r.Client.Update(ctx, pv2)
+			if err == nil {
+				r.Recorder.Eventf(pvc, corev1.EventTypeNormal, "PVUpdated", "pv %s updated successfully", pv2.Name)
+			} else {
+				r.Recorder.Eventf(pvc, corev1.EventTypeWarning, "UpdatePVFailed", "failed to update pv %s, error: %s", pv2.Name, err.Error())
+			}
 		}
-		return ctrl.Result{}, err
 	} else {
-		pv2 := &corev1.PersistentVolume{}
-		err = r.Client.Get(ctx, types.NamespacedName{Name: pvc.Spec.VolumeName}, pv2)
-		if err == nil {
-			needUpdate := r.mergePV(pv2, pv)
-			if needUpdate {
-				err = r.Client.Update(ctx, pv2)
-				if err == nil {
-					r.Recorder.Eventf(pvc, corev1.EventTypeNormal, "PVUpdated", "pv %s updated successfully", pv2.Name)
-				} else {
-					r.Recorder.Eventf(pvc, corev1.EventTypeWarning, "UpdatePVFailed", "failed to update pv %s, error: %s", pv2.Name, err.Error())
-				}
-			}
-		} else {
-			if apierrors.IsNotFound(err) {
-				err = r.Client.Create(ctx, pv)
-				if err == nil {
-					r.Recorder.Eventf(pvc, corev1.EventTypeNormal, "PVCreated", "pv %s created successfully", pv.Name)
-				} else {
-					r.Recorder.Eventf(pvc, corev1.EventTypeWarning, "CreatePVFailed", "failed to create pv %s, error: %s", pv.Name, err.Error())
-				}
-				return ctrl.Result{}, err
+		if apierrors.IsNotFound(err) {
+			err = r.Client.Create(ctx, pv)
+			if err == nil {
+				r.Recorder.Eventf(pvc, corev1.EventTypeNormal, "PVCreated", "pv %s created successfully", pv.Name)
 			} else {
-				return ctrl.Result{}, err
+				r.Recorder.Eventf(pvc, corev1.EventTypeWarning, "CreatePVFailed", "failed to create pv %s, error: %s", pv.Name, err.Error())
 			}
+			return ctrl.Result{}, err
+		} else {
+			return ctrl.Result{}, err
 		}
 	}
+
 	return ctrl.Result{}, nil
 }