RBD Plugin: Pass monitors addresses in a comma-separated list instead of

trying one by one.

In production, monitors may crash (or have a network problem), if we try
monitors one by one, rbd command will hang a long time (e.g. `rbd map -m
<unconnectable_host_ip>` on linux 4.4 timed out in 6 minutes) when
trying a unconnectable monitor. This is unacceptable.

Actually, we can simply pass a comma-separed list monitor addresses
to `rbd` command utility. Kernel rbd/libceph modules will pick
monitor randomly and try one by one, `rbd` command utility succeed soon
if there is a good one in monitors list.

pkg/volume/rbd/rbd_util.go

@@ -25,7 +25,6 @@ import (
 	"encoding/json"
 	"fmt"
 	"io/ioutil"
-	"math/rand"
 	"os"
 	"os/exec"
 	"path"
@@ -133,6 +132,17 @@ func rbdErrors(runErr, resultErr error) error {
 	return resultErr
 }
 
+// 'rbd' utility simply pass '-m <mon>' parameter to kernel rbd/libceph
+// modules, which takes a comma-seprated list of one or more monitor addresses
+// (e.g.  ip1[:port1][,ip2[:port2]...]) in its first version in linux (see
+// https://github.com/torvalds/linux/blob/602adf400201636e95c3fed9f31fba54a3d7e844/net/ceph/ceph_common.c#L239)
+// Also, libceph choose monitor randomly, so we can simply pass all addresses
+// without randomization (see
+// https://github.com/torvalds/linux/blob/602adf400201636e95c3fed9f31fba54a3d7e844/net/ceph/mon_client.c#L132).
+func (util *RBDUtil) kernelRBDMonitorsOpt(mons []string) string {
+	return strings.Join(mons, ",")
+}
+
 // rbdLock acquires a lock on image if lock is true, otherwise releases if a
 // lock is found on image.
 func (util *RBDUtil) rbdLock(b rbdMounter, lock bool) error {
@@ -156,12 +166,8 @@ func (util *RBDUtil) rbdLock(b rbdMounter, lock bool) error {
 	// construct lock id using host name and a magic prefix
 	lock_id := kubeLockMagic + node.GetHostname("")
 
-	l := len(b.Mon)
+	mon := util.kernelRBDMonitorsOpt(b.Mon)
-	// avoid mount storm, pick a host randomly
+
-	start := rand.Int() % l
-	// iterate all hosts until mount succeeds.
-	for i := start; i < start+l; i++ {
-		mon := b.Mon[i%l]
 	// cmd "rbd lock list" serves two purposes:
 	// for fencing, check if lock already held for this host
 	// this edge case happens if host crashes in the middle of acquiring lock and mounting rbd
@@ -172,7 +178,7 @@ func (util *RBDUtil) rbdLock(b rbdMounter, lock bool) error {
 	output = string(cmd)
 	glog.Infof("lock list output %q", output)
 	if err != nil {
-			continue
+		return err
 	}
 
 	if lock {
@@ -233,11 +239,6 @@ func (util *RBDUtil) rbdLock(b rbdMounter, lock bool) error {
 		}
 	}
 
-		if err == nil {
-			// break if operation succeeds
-			break
-		}
-	}
 	return err
 }
 
@@ -274,13 +275,7 @@ func (util *RBDUtil) AttachDisk(b rbdMounter) (string, error) {
 			return "", fmt.Errorf("rbd image %s/%s is still being used. rbd output: %s", b.Pool, b.Image, rbdOutput)
 		}
 
-		// rbd map
+		mon := util.kernelRBDMonitorsOpt(b.Mon)
-		l := len(b.Mon)
-		// avoid mount storm, pick a host randomly
-		start := rand.Int() % l
-		// iterate all hosts until mount succeeds.
-		for i := start; i < start+l; i++ {
-			mon := b.Mon[i%l]
 		glog.V(1).Infof("rbd: map mon %s", mon)
 		if b.Secret != "" {
 			output, err = b.exec.Run("rbd",
@@ -289,12 +284,8 @@ func (util *RBDUtil) AttachDisk(b rbdMounter) (string, error) {
 			output, err = b.exec.Run("rbd",
 				"map", b.Image, "--pool", b.Pool, "--id", b.Id, "-m", mon, "-k", b.Keyring)
 		}
-			if err == nil {
-				break
-			}
-			glog.V(1).Infof("rbd: map error %v, rbd output: %s", err, string(output))
-		}
 		if err != nil {
+			glog.V(1).Infof("rbd: map error %v, rbd output: %s", err, string(output))
 			return "", fmt.Errorf("rbd: map failed %v, rbd output: %s", err, string(output))
 		}
 		devicePath, found = waitForPath(b.Pool, b.Image, 10)
@@ -378,13 +369,7 @@ func (util *RBDUtil) CreateImage(p *rbdVolumeProvisioner) (r *v1.RBDPersistentVo
 	// convert to MB that rbd defaults on
 	sz := int(volume.RoundUpSize(volSizeBytes, 1024*1024))
 	volSz := fmt.Sprintf("%d", sz)
-	// rbd create
+	mon := util.kernelRBDMonitorsOpt(p.Mon)
-	l := len(p.rbdMounter.Mon)
-	// pick a mon randomly
-	start := rand.Int() % l
-	// iterate all monitors until create succeeds.
-	for i := start; i < start+l; i++ {
-		mon := p.Mon[i%l]
 	if p.rbdMounter.imageFormat == rbdImageFormat2 {
 		glog.V(4).Infof("rbd: create %s size %s format %s (features: %s) using mon %s, pool %s id %s key %s", p.rbdMounter.Image, volSz, p.rbdMounter.imageFormat, p.rbdMounter.imageFeatures, mon, p.rbdMounter.Pool, p.rbdMounter.adminId, p.rbdMounter.adminSecret)
 	} else {
@@ -398,14 +383,9 @@ func (util *RBDUtil) CreateImage(p *rbdVolumeProvisioner) (r *v1.RBDPersistentVo
 		args = append(args, "--image-feature", features)
 	}
 	output, err = p.exec.Run("rbd", args...)
-		if err == nil {
-			break
-		} else {
-			glog.Warningf("failed to create rbd image, output %v", string(output))
-		}
-	}
 
 	if err != nil {
+		glog.Warningf("failed to create rbd image, output %v", string(output))
 		return nil, 0, fmt.Errorf("failed to create rbd image: %v, command output: %s", err, string(output))
 	}
 
@@ -427,21 +407,15 @@ func (util *RBDUtil) DeleteImage(p *rbdVolumeDeleter) error {
 		return fmt.Errorf("rbd image %s/%s is still being used, rbd output: %v", p.rbdMounter.Pool, p.rbdMounter.Image, rbdOutput)
 	}
 	// rbd rm
-	l := len(p.rbdMounter.Mon)
+	mon := util.kernelRBDMonitorsOpt(p.rbdMounter.Mon)
-	// pick a mon randomly
-	start := rand.Int() % l
-	// iterate all monitors until rm succeeds.
-	for i := start; i < start+l; i++ {
-		mon := p.rbdMounter.Mon[i%l]
 	glog.V(4).Infof("rbd: rm %s using mon %s, pool %s id %s key %s", p.rbdMounter.Image, mon, p.rbdMounter.Pool, p.rbdMounter.adminId, p.rbdMounter.adminSecret)
 	output, err = p.exec.Run("rbd",
 		"rm", p.rbdMounter.Image, "--pool", p.rbdMounter.Pool, "--id", p.rbdMounter.adminId, "-m", mon, "--key="+p.rbdMounter.adminSecret)
 	if err == nil {
 		return nil
-		} else {
+	}
+
 	glog.Errorf("failed to delete rbd image: %v, command output: %s", err, string(output))
-		}
-	}
 	return fmt.Errorf("error %v, rbd output: %v", err, string(output))
 }
 
@@ -465,21 +439,15 @@ func (util *RBDUtil) ExpandImage(rbdExpander *rbdVolumeExpander, oldSize resourc
 	}
 
 	// rbd resize
-	l := len(rbdExpander.rbdMounter.Mon)
+	mon := util.kernelRBDMonitorsOpt(rbdExpander.rbdMounter.Mon)
-	// pick a mon randomly
-	start := rand.Int() % l
-	// iterate all monitors until resize succeeds.
-	for i := start; i < start+l; i++ {
-		mon := rbdExpander.rbdMounter.Mon[i%l]
 	glog.V(4).Infof("rbd: resize %s using mon %s, pool %s id %s key %s", rbdExpander.rbdMounter.Image, mon, rbdExpander.rbdMounter.Pool, rbdExpander.rbdMounter.adminId, rbdExpander.rbdMounter.adminSecret)
 	output, err = rbdExpander.exec.Run("rbd",
 		"resize", rbdExpander.rbdMounter.Image, "--size", newVolSz, "--pool", rbdExpander.rbdMounter.Pool, "--id", rbdExpander.rbdMounter.adminId, "-m", mon, "--key="+rbdExpander.rbdMounter.adminSecret)
 	if err == nil {
 		return newSizeQuant, nil
-		} else {
+	}
+
 	glog.Errorf("failed to resize rbd image: %v, command output: %s", err, string(output))
-		}
-	}
 	return oldSize, err
 }
 
@@ -497,11 +465,7 @@ func (util *RBDUtil) rbdInfo(b *rbdMounter) (int, error) {
 		secret = b.Secret
 	}
 
-	l := len(b.Mon)
+	mon := util.kernelRBDMonitorsOpt(b.Mon)
-	start := rand.Int() % l
-	// iterate all hosts until rbd command succeeds.
-	for i := start; i < start+l; i++ {
-		mon := b.Mon[i%l]
 	// cmd "rbd info" get the image info with the following output:
 	//
 	// # image exists (exit=0)
@@ -524,11 +488,6 @@ func (util *RBDUtil) rbdInfo(b *rbdMounter) (int, error) {
 		"info", b.Image, "--pool", b.Pool, "-m", mon, "--id", id, "--key="+secret)
 	output = string(cmd)
 
-		// break if command succeeds
-		if err == nil {
-			break
-		}
-
 	if err, ok := err.(*exec.Error); ok {
 		if err.Err == exec.ErrNotFound {
 			glog.Errorf("rbd cmd not found")
@@ -536,7 +495,6 @@ func (util *RBDUtil) rbdInfo(b *rbdMounter) (int, error) {
 			return 0, err
 		}
 	}
-	}
 
 	// If command never succeed, returns its last error.
 	if err != nil {
@@ -576,11 +534,7 @@ func (util *RBDUtil) rbdStatus(b *rbdMounter) (bool, string, error) {
 		secret = b.Secret
 	}
 
-	l := len(b.Mon)
+	mon := util.kernelRBDMonitorsOpt(b.Mon)
-	start := rand.Int() % l
-	// iterate all hosts until rbd command succeeds.
-	for i := start; i < start+l; i++ {
-		mon := b.Mon[i%l]
 	// cmd "rbd status" list the rbd client watch with the following output:
 	//
 	// # there is a watcher (exit=0)
@@ -600,11 +554,6 @@ func (util *RBDUtil) rbdStatus(b *rbdMounter) (bool, string, error) {
 		"status", b.Image, "--pool", b.Pool, "-m", mon, "--id", id, "--key="+secret)
 	output = string(cmd)
 
-		// break if command succeeds
-		if err == nil {
-			break
-		}
-
 	if err, ok := err.(*exec.Error); ok {
 		if err.Err == exec.ErrNotFound {
 			glog.Errorf("rbd cmd not found")
@@ -612,7 +561,6 @@ func (util *RBDUtil) rbdStatus(b *rbdMounter) (bool, string, error) {
 			return false, output, err
 		}
 	}
-	}
 
 	// If command never succeed, returns its last error.
 	if err != nil {