util: modify GetMonsAndClusterID() to take clusterID instead of options

This commit:
- modifies GetMonsAndClusterID() to take clusterID instead of options.
- moves out validation of clusterID is set or not out of GetMonsAndClusterID().
- defines ErrClusterIDNotSet new error for reusability.
- add GetClusterID() to obtain clusterID from options.

Signed-off-by: Rakshith R <rar@redhat.com>

internal/cephfs/volumeoptions.go

@@ -595,7 +595,11 @@ func genSnapFromOptions(ctx context.Context, req *csi.CreateSnapshotRequest) (sn
 	cephfsSnap.RequestName = req.GetName()
 	snapOptions := req.GetParameters()
 
-	cephfsSnap.Monitors, cephfsSnap.ClusterID, err = util.GetMonsAndClusterID(ctx, snapOptions, false)
+	clusterID, err := util.GetClusterID(snapOptions)
+	if err != nil {
+		return nil, err
+	}
+	cephfsSnap.Monitors, cephfsSnap.ClusterID, err = util.GetMonsAndClusterID(ctx, clusterID, false)
 	if err != nil {
 		log.ErrorLog(ctx, "failed getting mons (%s)", err)
 

internal/rbd/rbd_util.go

@@ -848,11 +848,7 @@ func genSnapFromSnapID(
 	snapshotID string,
 	cr *util.Credentials,
 	secrets map[string]string) error {
-	var (
+	var vi util.CSIIdentifier
-		options map[string]string
-		vi      util.CSIIdentifier
-	)
-	options = make(map[string]string)
 
 	rbdSnap.VolID = snapshotID
 
@@ -864,9 +860,8 @@ func genSnapFromSnapID(
 	}
 
 	rbdSnap.ClusterID = vi.ClusterID
-	options["clusterID"] = rbdSnap.ClusterID
 
-	rbdSnap.Monitors, _, err = util.GetMonsAndClusterID(ctx, options, false)
+	rbdSnap.Monitors, _, err = util.GetMonsAndClusterID(ctx, rbdSnap.ClusterID, false)
 	if err != nil {
 		log.ErrorLog(ctx, "failed getting mons (%s)", err)
 
@@ -940,11 +935,9 @@ func generateVolumeFromVolumeID(
 	cr *util.Credentials,
 	secrets map[string]string) (*rbdVolume, error) {
 	var (
-		options map[string]string
+		rbdVol *rbdVolume
-		rbdVol  *rbdVolume
+		err    error
-		err     error
 	)
-	options = make(map[string]string)
 
 	// rbdVolume fields that are not filled up in this function are:
 	//              Mounter, MultiNodeWritable
@@ -952,9 +945,8 @@ func generateVolumeFromVolumeID(
 	rbdVol.VolID = volumeID
 
 	rbdVol.ClusterID = vi.ClusterID
-	options["clusterID"] = rbdVol.ClusterID
 
-	rbdVol.Monitors, _, err = util.GetMonsAndClusterID(ctx, options, false)
+	rbdVol.Monitors, _, err = util.GetMonsAndClusterID(ctx, rbdVol.ClusterID, false)
 	if err != nil {
 		log.ErrorLog(ctx, "failed getting mons (%s)", err)
 
@@ -1170,7 +1162,11 @@ func genVolFromVolumeOptions(
 		rbdVol.NamePrefix = namePrefix
 	}
 
-	rbdVol.Monitors, rbdVol.ClusterID, err = util.GetMonsAndClusterID(ctx, volOptions, checkClusterIDMapping)
+	clusterID, err := util.GetClusterID(volOptions)
+	if err != nil {
+		return nil, err
+	}
+	rbdVol.Monitors, rbdVol.ClusterID, err = util.GetMonsAndClusterID(ctx, clusterID, checkClusterIDMapping)
 	if err != nil {
 		log.ErrorLog(ctx, "failed getting mons (%s)", err)
 
@@ -1247,7 +1243,11 @@ func genSnapFromOptions(ctx context.Context, rbdVol *rbdVolume, snapOptions map[
 	rbdSnap.JournalPool = rbdVol.JournalPool
 	rbdSnap.RadosNamespace = rbdVol.RadosNamespace
 
-	rbdSnap.Monitors, rbdSnap.ClusterID, err = util.GetMonsAndClusterID(ctx, snapOptions, false)
+	clusterID, err := util.GetClusterID(snapOptions)
+	if err != nil {
+		return nil, err
+	}
+	rbdSnap.Monitors, rbdSnap.ClusterID, err = util.GetMonsAndClusterID(ctx, clusterID, false)
 	if err != nil {
 		log.ErrorLog(ctx, "failed getting mons (%s)", err)
 

internal/util/csiconfig.go

@@ -19,7 +19,6 @@ package util
 import (
 	"context"
 	"encoding/json"
-	"errors"
 	"fmt"
 	"io/ioutil"
 	"strings"
@@ -32,6 +31,9 @@ const (
 
 	// CsiConfigFile is the location of the CSI config file.
 	CsiConfigFile = "/etc/ceph-csi-config/config.json"
+
+	// ClusterIDKey is the name of the key containing clusterID.
+	clusterIDKey = "clusterID"
 )
 
 // ClusterInfo strongly typed JSON spec for the below JSON structure.
@@ -132,11 +134,7 @@ func CephFSSubvolumeGroup(pathToConfig, clusterID string) (string, error) {
 
 // GetMonsAndClusterID returns monitors and clusterID information read from
 // configfile.
-func GetMonsAndClusterID(ctx context.Context, options map[string]string, checkClusterIDMapping bool) (string, string, error) {
+func GetMonsAndClusterID(ctx context.Context, clusterID string, checkClusterIDMapping bool) (string, string, error) {
-	clusterID, ok := options["clusterID"]
-	if !ok {
-		return "", "", errors.New("clusterID must be set")
-	}
 	if checkClusterIDMapping {
 		monitors, mappedClusterID, err := FetchMappedClusterIDAndMons(ctx, clusterID)
 		if err != nil {
@@ -153,3 +151,13 @@ func GetMonsAndClusterID(ctx context.Context, options map[string]string, checkCl
 
 	return monitors, clusterID, nil
 }
+
+// GetClusterID fetches clusterID from given options map.
+func GetClusterID(options map[string]string) (string, error) {
+	clusterID, ok := options[clusterIDKey]
+	if !ok {
+		return "", ErrClusterIDNotSet
+	}
+
+	return clusterID, nil
+}

internal/util/errors.go

@@ -33,6 +33,8 @@ var (
 	ErrSnapNameConflict = errors.New("snapshot name conflict")
 	// ErrPoolNotFound is returned when pool is not found.
 	ErrPoolNotFound = errors.New("pool not found")
+	// ErrClusterIDNotSet is returned when cluster id is not set.
+	ErrClusterIDNotSet = errors.New("clusterID must be set")
 )
 
 type errorPair struct {