Some cleanups and improvements in code

Signed-off-by: Elena Gershkovich <[email protected]>
red-hat-storage · Oct 29, 2024 · b83b03a · b83b03a
1 parent e781669
commit b83b03a
Show file tree

Hide file tree

Showing 3 changed files with 23 additions and 31 deletions.
diff --git a/internal/controller/vrg_status_pvcs.go b/internal/controller/vrg_status_pvcs.go
@@ -19,6 +19,14 @@ func FindProtectedPVC(vrg *ramen.VolumeReplicationGroup, pvcNamespaceName, pvcNa
 	return protectedPvc
 }
 
+func (v *VRGInstance) addProtectedPVC(pvcNamespace, pvcName string) *ramen.ProtectedPVC {
+	protectedPVC := &ramen.ProtectedPVC{Namespace: pvcNamespace, Name: pvcName}
+
+	v.instance.Status.ProtectedPVCs = append(v.instance.Status.ProtectedPVCs, *protectedPVC)
+
+	return &v.instance.Status.ProtectedPVCs[len(v.instance.Status.ProtectedPVCs)-1]
+}
+
 func (v *VRGInstance) pvcStatusDeleteIfPresent(pvcNamespaceName, pvcName string, log logr.Logger) {
 	pvcStatus, i := FindProtectedPvcAndIndex(v.instance, pvcNamespaceName, pvcName)
 	if pvcStatus == nil {

diff --git a/internal/controller/vrg_volrep.go b/internal/controller/vrg_volrep.go
@@ -239,8 +239,7 @@ func (v *VRGInstance) updateProtectedPVCs(pvc *corev1.PersistentVolumeClaim) err
 
 	protectedPVC := v.findProtectedPVC(pvc.GetNamespace(), pvc.GetName())
 	if protectedPVC == nil {
-		protectedPVC = &ramendrv1alpha1.ProtectedPVC{Namespace: pvc.GetNamespace(), Name: pvc.GetName()}
-		v.instance.Status.ProtectedPVCs = append(v.instance.Status.ProtectedPVCs, *protectedPVC)
+		protectedPVC = v.addProtectedPVC(pvc.GetNamespace(), pvc.GetName())
 	}
 
 	protectedPVC.ProtectedByVolSync = false
@@ -1644,17 +1643,12 @@ func (v *VRGInstance) updatePVCDataReadyConditionHelper(
 }
 
 func (v *VRGInstance) updatePVCDataReadyCondition(pvcNamespace, pvcName, reason, message string) {
-	if protectedPVC := v.findProtectedPVC(pvcNamespace, pvcName); protectedPVC != nil {
-		setPVCDataReadyCondition(protectedPVC, reason, message, v.instance.Generation)
-		// No need to append it as an already existing entry from the list is being modified.
-		return
+	protectedPVC := v.findProtectedPVC(pvcNamespace, pvcName)
+	if protectedPVC == nil {
+		protectedPVC = v.addProtectedPVC(pvcNamespace, pvcName)
 	}
 
-	protectedPVC := &ramendrv1alpha1.ProtectedPVC{Namespace: pvcNamespace, Name: pvcName}
 	setPVCDataReadyCondition(protectedPVC, reason, message, v.instance.Generation)
-
-	// created a new instance. Add it to the list
-	v.instance.Status.ProtectedPVCs = append(v.instance.Status.ProtectedPVCs, *protectedPVC)
 }
 
 // Disabling unparam linter as currently every invokation of this
@@ -1677,17 +1671,12 @@ func (v *VRGInstance) updatePVCDataProtectedConditionHelper(
 }
 
 func (v *VRGInstance) updatePVCDataProtectedCondition(pvcNamespace, pvcName, reason, message string) {
-	if protectedPVC := v.findProtectedPVC(pvcNamespace, pvcName); protectedPVC != nil {
-		setPVCDataProtectedCondition(protectedPVC, reason, message, v.instance.Generation)
-		// No need to append it as an already existing entry from the list is being modified.
-		return
+	protectedPVC := v.findProtectedPVC(pvcNamespace, pvcName)
+	if protectedPVC == nil {
+		protectedPVC = v.addProtectedPVC(pvcNamespace, pvcName)
 	}
 
-	protectedPVC := &ramendrv1alpha1.ProtectedPVC{Namespace: pvcNamespace, Name: pvcName}
 	setPVCDataProtectedCondition(protectedPVC, reason, message, v.instance.Generation)
-
-	// created a new instance. Add it to the list
-	v.instance.Status.ProtectedPVCs = append(v.instance.Status.ProtectedPVCs, *protectedPVC)
 }
 
 func setPVCDataReadyCondition(protectedPVC *ramendrv1alpha1.ProtectedPVC, reason, message string,
@@ -1743,15 +1732,12 @@ func setPVCDataProtectedCondition(protectedPVC *ramendrv1alpha1.ProtectedPVC, re
 }
 
 func (v *VRGInstance) updatePVCClusterDataProtectedCondition(pvcNamespace, pvcName, reason, message string) {
-	if protectedPVC := v.findProtectedPVC(pvcNamespace, pvcName); protectedPVC != nil {
-		setPVCClusterDataProtectedCondition(protectedPVC, reason, message, v.instance.Generation)
-		// No need to append it as an already existing entry from the list is being modified.
-		return
+	protectedPVC := v.findProtectedPVC(pvcNamespace, pvcName)
+	if protectedPVC == nil {
+		protectedPVC = v.addProtectedPVC(pvcNamespace, pvcName)
 	}
 
-	protectedPVC := &ramendrv1alpha1.ProtectedPVC{Namespace: pvcNamespace, Name: pvcName}
 	setPVCClusterDataProtectedCondition(protectedPVC, reason, message, v.instance.Generation)
-	v.instance.Status.ProtectedPVCs = append(v.instance.Status.ProtectedPVCs, *protectedPVC)
 }
 
 func setPVCClusterDataProtectedCondition(protectedPVC *ramendrv1alpha1.ProtectedPVC, reason, message string,

diff --git a/internal/controller/vrg_volsync.go b/internal/controller/vrg_volsync.go
@@ -51,11 +51,10 @@ func (v *VRGInstance) restorePVsAndPVCsForVolSync() (int, error) {
 		if err != nil {
 			v.log.Info(fmt.Sprintf("Unable to ensure PVC %v -- err: %v", rdSpec, err))
 
-			protectedPVC := FindProtectedPVC(v.instance, rdSpec.ProtectedPVC.Namespace, rdSpec.ProtectedPVC.Name)
+			protectedPVC := v.findProtectedPVC(rdSpec.ProtectedPVC.Namespace, rdSpec.ProtectedPVC.Name)
 			if protectedPVC == nil {
-				protectedPVC = &ramendrv1alpha1.ProtectedPVC{}
+				protectedPVC = v.addProtectedPVC(rdSpec.ProtectedPVC.Namespace, rdSpec.ProtectedPVC.Name)
 				rdSpec.ProtectedPVC.DeepCopyInto(protectedPVC)
-				v.instance.Status.ProtectedPVCs = append(v.instance.Status.ProtectedPVCs, *protectedPVC)
 			}
 
 			setVRGConditionTypeVolSyncPVRestoreError(&protectedPVC.Conditions, v.instance.Generation,
@@ -66,11 +65,10 @@ func (v *VRGInstance) restorePVsAndPVCsForVolSync() (int, error) {
 
 		numPVsRestored++
 
-		protectedPVC := FindProtectedPVC(v.instance, rdSpec.ProtectedPVC.Namespace, rdSpec.ProtectedPVC.Name)
+		protectedPVC := v.findProtectedPVC(rdSpec.ProtectedPVC.Namespace, rdSpec.ProtectedPVC.Name)
 		if protectedPVC == nil {
-			protectedPVC = &ramendrv1alpha1.ProtectedPVC{}
+			protectedPVC = v.addProtectedPVC(rdSpec.ProtectedPVC.Namespace, rdSpec.ProtectedPVC.Name)
 			rdSpec.ProtectedPVC.DeepCopyInto(protectedPVC)
-			v.instance.Status.ProtectedPVCs = append(v.instance.Status.ProtectedPVCs, *protectedPVC)
 		}
 
 		setVRGConditionTypeVolSyncPVRestoreComplete(&protectedPVC.Conditions, v.instance.Generation, "PVC restored")
@@ -142,7 +140,7 @@ func (v *VRGInstance) reconcilePVCAsVolSyncPrimary(pvc corev1.PersistentVolumeCl
 		Resources:          pvc.Spec.Resources,
 	}
 
-	protectedPVC := FindProtectedPVC(v.instance, pvc.Namespace, pvc.Name)
+	protectedPVC := v.findProtectedPVC(pvc.Namespace, pvc.Name)
 	if protectedPVC == nil {
 		protectedPVC = newProtectedPVC
 		v.instance.Status.ProtectedPVCs = append(v.instance.Status.ProtectedPVCs, *protectedPVC)