Skip to content

Commit

Permalink
Address review comments
Browse files Browse the repository at this point in the history
Addressed review comments across the code. Changes are
non-functional, hence retained as a separate commit instead
of squashing it into the prior commit.

Signed-off-by: ShyamsundarR <srangana@redhat.com>
  • Loading branch information
ShyamsundarR committed May 2, 2019
1 parent ad31b5c commit b72caa6
Show file tree
Hide file tree
Showing 6 changed files with 22 additions and 23 deletions.
8 changes: 4 additions & 4 deletions docs/deploy-rbd.md
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,8 @@ Parameter | Required | Description
`mounter`| no | if set to `rbd-nbd`, use `rbd-nbd` on nodes that have `rbd-nbd` and `nbd` kernel modules to map rbd images

**NOTE:** An accompanying CSI configuration file, needs to be provided to the
running pods. Refer to [Creating CSI configuration for RBD based provisioning]
(../examples/README.md#creating-csi-configuration-for-rbd-based-provisioning)
running pods. Refer to [Creating CSI configuration for RBD based
provisioning](../examples/README.md#creating-csi-configuration-for-rbd-based-provisioning)
for more information.

**NOTE:** A suggested way to populate and retain uniquness of the clusterID is
Expand Down Expand Up @@ -96,8 +96,8 @@ kubectl create -f csi-config-map.yaml

The config map deploys an empty CSI configuration that is mounted as a volume
within the Ceph CSI plugin pods. To add a specific Ceph clusters configuration
details, refer to [Creating CSI configuration for RBD based provisioning]
(../examples/README.md#creating-csi-configuration-for-rbd-based-provisioning)
details, refer to [Creating CSI configuration for RBD based
provisioning](../examples/README.md#creating-csi-configuration-for-rbd-based-provisioning)
for more information.

**Deploy CSI sidecar containers:**
Expand Down
10 changes: 6 additions & 4 deletions examples/rbd/csi-config-map-sample.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,15 @@ kind: ConfigMap
# The <cluster-id> is used by the CSI plugin to uniquely identify and use a
# Ceph cluster, the value MUST match the value provided as `clusterID` in the
# StorageClass
# The <MONValuei> fields are the various monitor addresses for the Ceph cluster
# The <MONValue#> fields are the various monitor addresses for the Ceph cluster
# identified by the <cluster-id>
# If a CSI plugin is using more than one Ceph cluster, repeat the "<cluster-id>"
# section for each such cluster in use.
# NOTE: If MON addresses change, this config map can be updated and replaced
# in the Kubernetes cluster, for the plugins to pick up the changes
# automatically.
# To add more clusters or edit MON addresses in an existing config map, use
# the `kubectl replace` command.
# NOTE: Changes to the config map is automatically updated in the running pods,
# thus restarting existing pods using the config map is NOT required on edits
# to the config map.
data:
config.json: |-
{
Expand Down
3 changes: 0 additions & 3 deletions examples/rbd/secret.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,3 @@ data:
admin: BASE64-ENCODED-PASSWORD
# Key value corresponds to a user name defined in ceph cluster
kubernetes: BASE64-ENCODED-PASSWORD
# if monValueFromSecret is set to "monitors", uncomment the
# following and set the mon there
# monitors: BASE64-ENCODED-Comma-Delimited-Mons
4 changes: 2 additions & 2 deletions examples/rbd/snapshotclass.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@ parameters:
# 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.
# Ensure to create a secret named ceph-cluster-<cluster-id>, based on
# template-ceph-cluster-ID-secret.yaml, to accompany the string chosen to
# Ensure to create an entry in the config map named ceph-csi-config, based on
# csi-config-map-sample.yaml, to accompany the string chosen to
# represent the Ceph cluster in clusterID below
clusterID: <cluster-id>

Expand Down
4 changes: 2 additions & 2 deletions examples/rbd/storageclass.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@ parameters:
# 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.
# Ensure to create a secret named ceph-cluster-<cluster-id>, based on
# template-ceph-cluster-ID-secret.yaml, to accompany the string chosen to
# Ensure to create an entry in the config map named ceph-csi-config, based on
# csi-config-map-sample.yaml, to accompany the string chosen to
# represent the Ceph cluster in clusterID below
clusterID: <cluster-id>

Expand Down
16 changes: 8 additions & 8 deletions pkg/rbd/rbd_util.go
Original file line number Diff line number Diff line change
Expand Up @@ -332,7 +332,7 @@ Check comment on checkSnapExists, to understand how this function behaves
**NOTE:** These functions manipulate the rados omaps that hold information regarding
volume names as requested by the CSI drivers. Hence, these need to be invoked only when the
respective CSI snapshot or volume name based locks are held, as otherwise racy access to these
omaps may end up leaving the ompas in an inconsistent state.
omaps may end up leaving the omaps in an inconsistent state.
*/
func checkVolExists(rbdVol *rbdVolume, credentials map[string]string) (found bool, err error) {
var vi util.CsiIdentifier
Expand Down Expand Up @@ -484,10 +484,10 @@ func unreserveVol(rbdVol *rbdVolume, credentials map[string]string) error {
return nil
}

// reserveOMapName creates an omap with passed in oMapNameSuffix and a generated <uuid>.
// reserveOMapName creates an omap with passed in oMapNamePrefix and a generated <uuid>.
// It ensures generated omap name does not already exist and if conflicts are detected, a set
// number of retires with newer uuids are attempted before returning an error
func reserveOMapName(monitors, adminID, key, poolName, oMapNameSuffix string) (string, error) {
func reserveOMapName(monitors, adminID, key, poolName, oMapNamePrefix string) (string, error) {
var iterUUID string

maxAttempts := 5
Expand All @@ -496,7 +496,7 @@ func reserveOMapName(monitors, adminID, key, poolName, oMapNameSuffix string) (s
// generate a uuid for the image name
iterUUID = uuid.NewUUID().String()

err := util.CreateObject(monitors, adminID, key, poolName, oMapNameSuffix+iterUUID)
err := util.CreateObject(monitors, adminID, key, poolName, oMapNamePrefix+iterUUID)
if err != nil {
if _, ok := err.(util.ErrObjectExists); ok {
attempt++
Expand Down Expand Up @@ -718,7 +718,7 @@ func updateSnapWithImageInfo(rbdSnap *rbdSnapshot, credentials map[string]string

rbdSnap.SizeBytes = snapInfo.Size

tm, err := time.Parse(time.ANSIC, snapInfo.TimeStamp)
tm, err := time.Parse(time.ANSIC, snapInfo.Timestamp)
if err != nil {
return err
}
Expand Down Expand Up @@ -1149,7 +1149,7 @@ func getSnapshotMetadata(pSnapOpts *rbdSnapshot, adminID string, credentials map

pSnapOpts.SizeBytes = snapInfo.Size

tm, err := time.Parse(time.ANSIC, snapInfo.TimeStamp)
tm, err := time.Parse(time.ANSIC, snapInfo.Timestamp)
if err != nil {
return err
}
Expand Down Expand Up @@ -1221,7 +1221,7 @@ type snapInfo struct {
ID int64 `json:"id"`
Name string `json:"name"`
Size int64 `json:"size"`
TimeStamp string `json:"timestamp"`
Timestamp string `json:"timestamp"`
}

// ErrSnapNotFound is returned when snap name passed is not found in the list of snapshots for the
Expand All @@ -1237,7 +1237,7 @@ func (e ErrSnapNotFound) Error() string {

/*
getSnapInfo queries rbd about the snapshots of the given image and returns its metadata, and
returns ErrImageNotFound if provided image is not found, and ErrSnapNotFound if povided snap
returns ErrImageNotFound if provided image is not found, and ErrSnapNotFound if provided snap
is not found in the images snapshot list
*/
func getSnapInfo(monitors, adminID, key, poolName, imageName, snapName string) (snapInfo, error) {
Expand Down

0 comments on commit b72caa6

Please sign in to comment.