From 52433841b479f5841606db37b15d151de3ce7fee Mon Sep 17 00:00:00 2001
From: Niels de Vos <ndevos@redhat.com>
Date: Thu, 8 Apr 2021 15:04:32 +0200
Subject: [PATCH] cleanup: move copyEncryptionConfig() from CreateVolume to
 Exists()

The rbdVolume that needs its encryption configured is constructed in the
Exists() method. It is suitable to move the copyEncryptionConfig() call
there as well, so that the object is completely constructed in a single
place.

Golang-ci:gocyclo complained about the increased complexity of the
Exists() function. Moving the repairing of the ImageID into its own
helper function makes the code a little easier to understand.

Signed-off-by: Niels de Vos <ndevos@redhat.com>
---
 internal/rbd/controllerserver.go |  8 ------
 internal/rbd/rbd_journal.go      | 44 ++++++++++++++++++++++++--------
 2 files changed, 33 insertions(+), 19 deletions(-)

diff --git a/internal/rbd/controllerserver.go b/internal/rbd/controllerserver.go
index e28634e06..87d3f04ca 100644
--- a/internal/rbd/controllerserver.go
+++ b/internal/rbd/controllerserver.go
@@ -269,14 +269,6 @@ func (cs *ControllerServer) CreateVolume(ctx context.Context, req *csi.CreateVol
 			}
 		}
 
-		if parentVol != nil && parentVol.isEncrypted() {
-			err = parentVol.copyEncryptionConfig(&rbdVol.rbdImage)
-			if err != nil {
-				util.ErrorLog(ctx, err.Error())
-				return nil, status.Error(codes.Internal, err.Error())
-			}
-		}
-
 		return buildCreateVolumeResponse(req, rbdVol), nil
 	}
 
diff --git a/internal/rbd/rbd_journal.go b/internal/rbd/rbd_journal.go
index 72e28c7b3..c534afe2f 100644
--- a/internal/rbd/rbd_journal.go
+++ b/internal/rbd/rbd_journal.go
@@ -300,17 +300,9 @@ func (rv *rbdVolume) Exists(ctx context.Context, parentVol *rbdVolume) (bool, er
 		return false, err
 	}
 
-	if rv.ImageID == "" {
-		err = rv.getImageID()
-		if err != nil {
-			util.ErrorLog(ctx, "failed to get image id %s: %v", rv, err)
-			return false, err
-		}
-		err = j.StoreImageID(ctx, rv.JournalPool, rv.ReservedID, rv.ImageID)
-		if err != nil {
-			util.ErrorLog(ctx, "failed to store volume id %s: %v", rv, err)
-			return false, err
-		}
+	err = rv.repairImageID(ctx, j)
+	if err != nil {
+		return false, err
 	}
 
 	// size checks
@@ -327,12 +319,42 @@ func (rv *rbdVolume) Exists(ctx context.Context, parentVol *rbdVolume) (bool, er
 		return false, err
 	}
 
+	if parentVol != nil && parentVol.isEncrypted() {
+		err = parentVol.copyEncryptionConfig(&rv.rbdImage)
+		if err != nil {
+			util.ErrorLog(ctx, err.Error())
+			return false, err
+		}
+	}
+
 	util.DebugLog(ctx, "found existing volume (%s) with image name (%s) for request (%s)",
 		rv.VolID, rv.RbdImageName, rv.RequestName)
 
 	return true, nil
 }
 
+// repairImageID checks if rv.ImageID is already available (if so, it was
+// fetched from the journal), in case it is missing, the imageID is obtained
+// and stored in the journal.
+func (rv *rbdVolume) repairImageID(ctx context.Context, j *journal.Connection) error {
+	if rv.ImageID != "" {
+		return nil
+	}
+
+	err := rv.getImageID()
+	if err != nil {
+		util.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 {
+		util.ErrorLog(ctx, "failed to store volume id %s: %v", rv, err)
+		return err
+	}
+
+	return nil
+}
+
 // reserveSnap is a helper routine to request a rbdSnapshot name reservation and generate the
 // volume ID for the generated name.
 func reserveSnap(ctx context.Context, rbdSnap *rbdSnapshot, rbdVol *rbdVolume, cr *util.Credentials) error {