Address remenant subject reference and code style reviews

Signed-off-by: ShyamsundarR <srangana@redhat.com>

cmd/rbd/main.go

@@ -58,7 +58,7 @@ func main() {
 	}
 
 	driver := rbd.NewDriver()
-	driver.Run(*driverName, *nodeID, *endpoint, *containerized, *configRoot, cp)
+	driver.Run(*driverName, *nodeID, *endpoint, *configRoot, *containerized, cp)
 
 	os.Exit(0)
 }

docs/deploy-rbd.md

@@ -63,7 +63,7 @@ Parameter | Required | Description
 
 NOTE: If `clusterID` parameter is used, then an accompanying Ceph cluster
 configuration secret or config files needs to be provided to the running pods.
-Refer to `examples/README.md` section titled "Cluster ID based configuration"
+Refer to [Cluster ID based configuration](../examples/README.md#cluster-id-based-configuration)
 for more information. A suggested way to populate the clusterID is to use the
 output of `ceph fsid` of the Ceph cluster to be used for provisioning.
 

examples/README.md

@@ -239,7 +239,9 @@ Get the following information from the Ceph cluster,
   * Output of `ceph fsid`
   * Used to substitute `<cluster-id>` references in the files below
 
-Update the template `rbd/template-ceph-cluster-ID-secret.yaml` with values from
+Update the template
+[template-ceph-cluster-ID-secret.yaml](./rbd/template-ceph-cluster-ID-secret.yaml)
+with values from
 a Ceph cluster and replace `<cluster-id>` with the chosen clusterID to create
 the following secret,
 

examples/rbd/snapshotclass.yaml

@@ -11,7 +11,7 @@ parameters:
   monitors: mon1:port,mon2:port,...
   # OR,
   # String representing a Ceph cluster to provision storage from.
-  # Should be unique unique across all Ceph clusters in use for provisioning,
+  # Should be unique across all Ceph clusters in use for provisioning,
   # cannot be greater than 36 bytes in length, and should remain immutable for
   # the lifetime of the StorageClass in use.
   # If using clusterID, ensure to create a secret, as in

examples/rbd/storageclass.yaml

@@ -10,7 +10,7 @@ parameters:
    monitors: mon1:port,mon2:port,...
    # OR,
    # String representing a Ceph cluster to provision storage from.
-   # Should be unique unique across all Ceph clusters in use for provisioning,
+   # Should be unique across all Ceph clusters in use for provisioning,
    # cannot be greater than 36 bytes in length, and should remain immutable for
    # the lifetime of the StorageClass in use.
    # If using clusterID, ensure to create a secret, as in

pkg/rbd/rbd.go

@@ -89,12 +89,12 @@ func NewNodeServer(d *csicommon.CSIDriver, containerized bool) (*NodeServer, err
 
 // Run start a non-blocking grpc controller,node and identityserver for
 // rbd CSI driver which can serve multiple parallel requests
-func (r *Driver) Run(driverName, nodeID, endpoint string, containerized bool, configroot string, cachePersister util.CachePersister) {
+func (r *Driver) Run(driverName, nodeID, endpoint, configRoot string, containerized bool, cachePersister util.CachePersister) {
 	var err error
 	klog.Infof("Driver: %v version: %v", driverName, version)
 
 	// Initialize config store
-	confStore, err = util.NewConfigStore(configroot)
+	confStore, err = util.NewConfigStore(configRoot)
 	if err != nil {
 		klog.Fatalln("Failed to initialize config store.")
 	}

pkg/rbd/rbd_util.go

@@ -87,10 +87,12 @@ var (
 	supportedFeatures = sets.NewString("layering")
 )
 
-func getRBDKey(clusterid string, id string, credentials map[string]string) (string, error) {
-	var ok bool
-	var err error
-	var key string
+func getRBDKey(clusterid, id string, credentials map[string]string) (string, error) {
+	var (
+		ok  bool
+		err error
+		key string
+	)
 
 	if key, ok = credentials[id]; !ok {
 		if clusterid != "" {
@@ -272,8 +274,8 @@ func getIDs(options map[string]string, clusterID string) (adminID, userID string
 	case ok:
 	case clusterID != "":
 		if adminID, err = confStore.AdminID(clusterID); err != nil {
-			klog.Errorf("failed getting subject (%s)", err)
-			return "", "", fmt.Errorf("failed to fetch admin ID for clusterID (%s)", clusterID)
+			klog.Errorf("failed getting adminID (%s)", err)
+			return "", "", fmt.Errorf("failed to fetch adminID for clusterID (%s)", clusterID)
 		}
 	default:
 		adminID = rbdDefaultAdminID
@@ -284,8 +286,8 @@ func getIDs(options map[string]string, clusterID string) (adminID, userID string
 	case ok:
 	case clusterID != "":
 		if userID, err = confStore.UserID(clusterID); err != nil {
-			klog.Errorf("failed getting subject (%s)", err)
-			return "", "", fmt.Errorf("failed to fetch user ID using clusterID (%s)", clusterID)
+			klog.Errorf("failed getting userID (%s)", err)
+			return "", "", fmt.Errorf("failed to fetch userID using clusterID (%s)", clusterID)
 		}
 	default:
 		userID = rbdDefaultUserID
@@ -295,8 +297,10 @@ func getIDs(options map[string]string, clusterID string) (adminID, userID string
 }
 
 func getRBDVolumeOptions(volOptions map[string]string, disableInUseChecks bool) (*rbdVolume, error) {
-	var ok bool
-	var err error
+	var (
+		ok  bool
+		err error
+	)
 
 	rbdVol := &rbdVolume{}
 	rbdVol.Pool, ok = volOptions["pool"]
@@ -342,8 +346,10 @@ func getRBDVolumeOptions(volOptions map[string]string, disableInUseChecks bool)
 }
 
 func getCredsFromVol(rbdVol *rbdVolume, volOptions map[string]string) error {
-	var ok bool
-	var err error
+	var (
+		ok  bool
+		err error
+	)
 
 	rbdVol.AdminID, rbdVol.UserID, err = getIDs(volOptions, rbdVol.ClusterID)
 	if err != nil {
@@ -355,12 +361,14 @@ func getCredsFromVol(rbdVol *rbdVolume, volOptions map[string]string) error {
 		rbdVol.Mounter = rbdDefaultMounter
 	}
 
-	return nil
+	return err
 }
 
 func getRBDSnapshotOptions(snapOptions map[string]string) (*rbdSnapshot, error) {
-	var ok bool
-	var err error
+	var (
+		ok  bool
+		err error
+	)
 
 	rbdSnap := &rbdSnapshot{}
 	rbdSnap.Pool, ok = snapOptions["pool"]

pkg/util/configstore.go

@@ -1,5 +1,5 @@
 /*
-Copyright 2018 The Ceph-CSI Authors.
+Copyright 2019 The Ceph-CSI Authors.
 
 Licensed under the Apache License, Version 2.0 (the "License");
 you may not use this file except in compliance with the License.
@@ -34,7 +34,7 @@ type StoreReader interface {
 - csMonitors: MON list, comma separated
 - csAdminID: adminID, used for provisioning
 - csUserID: userID, used for publishing
-- csAdminKey: key, for userID in csProvisionerUser
+- csAdminKey: key, for adminID in csProvisionerUser
 - csUserKey: key, for userID in csPublisherUser
 - csPools: Pool list, comma separated
 */
@@ -55,13 +55,12 @@ type ConfigStore struct {
 }
 
 // dataForKey returns data from the config store for the provided key
-func (dc *ConfigStore) dataForKey(clusterID string, key string) (string, error) {
+func (dc *ConfigStore) dataForKey(clusterID, key string) (string, error) {
 	if dc.StoreReader != nil {
 		return dc.StoreReader.DataForKey(clusterID, key)
 	}
 
-	err := errors.New("config store location uninitialized")
-	return "", err
+	return "", errors.New("config store location uninitialized")
 }
 
 // Mons returns a comma separated MON list from the cluster config represented by clusterID

pkg/util/fileconfig.go

@@ -43,7 +43,7 @@ type FileConfig struct {
 
 // DataForKey reads the appropriate config file, named using key, and returns
 // the contents of the file to the caller
-func (fc *FileConfig) DataForKey(clusterid string, key string) (data string, err error) {
+func (fc *FileConfig) DataForKey(clusterid, key string) (data string, err error) {
 	pathToKey := path.Join(fc.BasePath, "ceph-cluster-"+clusterid, key)
 	// #nosec
 	content, err := ioutil.ReadFile(pathToKey)

pkg/util/k8sconfig.go

@@ -40,7 +40,7 @@ type K8sConfig struct {
 
 // DataForKey reads the appropriate k8s secret, named using clusterid, and
 // returns the contents of key within the secret
-func (kc *K8sConfig) DataForKey(clusterid string, key string) (data string, err error) {
+func (kc *K8sConfig) DataForKey(clusterid, key string) (data string, err error) {
 	secret, err := kc.Client.CoreV1().Secrets(kc.Namespace).Get("ceph-cluster-"+clusterid, metav1.GetOptions{})
 	if err != nil {
 		err = fmt.Errorf("error fetching configuration for cluster ID (%s). (%s)", clusterid, err)