From ddd41d358f4b049a18ae6c26d9bbe65944842cb6 Mon Sep 17 00:00:00 2001
From: Niels de Vos <ndevos@ibm.com>
Date: Fri, 1 Nov 2024 13:02:20 +0100
Subject: [PATCH] cleanup: drop rbdVol.storeImageID() in favor of
 rbdVol.repairImageID()

Both `rbdVolume.repairImageID()` and `rbdVolume.storeImageID()` do the
same thing. `repairImageID()` includes optional extra checking for an
already set ImageID, which is not part of `storeImageID()`.

Remove some unneeded `ImageID == ""` checking because `repairImageID()`
does that, and remove the `storeImageID()` function.

Signed-off-by: Niels de Vos <ndevos@ibm.com>
---
 internal/rbd/controllerserver.go |  2 +-
 internal/rbd/rbd_journal.go      | 39 ++++++++------------------------
 internal/rbd/rbd_util.go         |  9 ++++----
 3 files changed, 15 insertions(+), 35 deletions(-)

diff --git a/internal/rbd/controllerserver.go b/internal/rbd/controllerserver.go
index 1745dbc37..08bd4734f 100644
--- a/internal/rbd/controllerserver.go
+++ b/internal/rbd/controllerserver.go
@@ -790,7 +790,7 @@ func (cs *ControllerServer) createBackingImage(
 			}
 		}
 	}()
-	err = rbdVol.storeImageID(ctx, j)
+	err = rbdVol.repairImageID(ctx, j, true)
 	if err != nil {
 		return status.Error(codes.Internal, err.Error())
 	}
diff --git a/internal/rbd/rbd_journal.go b/internal/rbd/rbd_journal.go
index 38f4c8c70..3af6d7a82 100644
--- a/internal/rbd/rbd_journal.go
+++ b/internal/rbd/rbd_journal.go
@@ -541,7 +541,7 @@ func undoVolReservation(ctx context.Context, rbdVol *rbdVolume, cr *util.Credent
 // The volume handler won't remain same as its contains poolID,clusterID etc
 // which are not same across clusters.
 //
-//nolint:gocyclo,cyclop,nestif // TODO: reduce complexity
+//nolint:gocyclo,cyclop // TODO: reduce complexity
 func RegenerateJournal(
 	volumeAttributes map[string]string,
 	claimName,
@@ -623,12 +623,12 @@ func RegenerateJournal(
 		rbdVol.ImageID = imageData.ImageAttributes.ImageID
 		rbdVol.Owner = imageData.ImageAttributes.Owner
 		rbdVol.RbdImageName = imageData.ImageAttributes.ImageName
-		if rbdVol.ImageID == "" {
-			err = rbdVol.storeImageID(ctx, j)
-			if err != nil {
-				return "", err
-			}
+
+		err = rbdVol.repairImageID(ctx, j, false)
+		if err != nil {
+			return "", err
 		}
+
 		if rbdVol.Owner != owner {
 			err = j.ResetVolumeOwner(ctx, rbdVol.JournalPool, rbdVol.ReservedID, owner)
 			if err != nil {
@@ -675,30 +675,11 @@ func RegenerateJournal(
 
 	log.DebugLog(ctx, "re-generated Volume ID (%s) and image name (%s) for request name (%s)",
 		rbdVol.VolID, rbdVol.RbdImageName, rbdVol.RequestName)
-	if rbdVol.ImageID == "" {
-		err = rbdVol.storeImageID(ctx, j)
-		if err != nil {
-			return "", err
-		}
+
+	err = rbdVol.repairImageID(ctx, j, false)
+	if err != nil {
+		return "", err
 	}
 
 	return rbdVol.VolID, nil
 }
-
-// storeImageID retrieves the image ID and stores it in OMAP.
-func (rv *rbdVolume) storeImageID(ctx context.Context, j *journal.Connection) error {
-	err := rv.getImageID()
-	if err != nil {
-		log.ErrorLog(ctx, "failed to get image id %s: %v", rv, err)
-
-		return err
-	}
-	err = j.StoreImageID(ctx, rv.JournalPool, rv.ReservedID, rv.ImageID)
-	if err != nil {
-		log.ErrorLog(ctx, "failed to store volume id %s: %v", rv, err)
-
-		return err
-	}
-
-	return nil
-}
diff --git a/internal/rbd/rbd_util.go b/internal/rbd/rbd_util.go
index 6a146d743..748309b88 100644
--- a/internal/rbd/rbd_util.go
+++ b/internal/rbd/rbd_util.go
@@ -1169,12 +1169,11 @@ func generateVolumeFromVolumeID(
 		}
 	}
 
-	if rbdVol.ImageID == "" {
-		err = rbdVol.storeImageID(ctx, j)
-		if err != nil {
-			return rbdVol, err
-		}
+	err = rbdVol.repairImageID(ctx, j, false)
+	if err != nil {
+		return rbdVol, err
 	}
+
 	err = rbdVol.getImageInfo()
 
 	return rbdVol, err