Merge pull request #54454 from cofyc/rbd_status

Automatic merge from submit-queue (batch tested with PRs 54229, 54380, 54302, 54454). If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

RBD Plugin: rbdStatus only check output of successful `rbd status` run

**What this PR does / why we need it**:

Current RBDUtil.rbdStatus implementation never return error in any cases, even if command not found or `rbd status` never succeed. This PR change it to only check output of successful `rbd status` run, and return error on other cases.

Because there are maybe network problem or ceph cluster unresponsive conditions which will cause `rbd status` command to fail. We cannot assume there is no watchers on given image. It's better to return error, and let the caller to decide what to do.

**Which issue this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close that issue when PR gets merged)*: fixes #


**Special notes for your reviewer**:

**Release note**:

```release-note
NONE
```
pull/6/head
Kubernetes Submit Queue 2017-10-24 08:35:14 -07:00 committed by GitHub
commit 5f030f4568
1 changed files with 33 additions and 16 deletions

View File

@ -436,7 +436,7 @@ func (util *RBDUtil) DeleteImage(p *rbdVolumeDeleter) error {
return err return err
} }
// run rbd status command to check if there is watcher on the image // rbdStatus runs `rbd status` command to check if there is watcher on the image.
func (util *RBDUtil) rbdStatus(b *rbdMounter) (bool, error) { func (util *RBDUtil) rbdStatus(b *rbdMounter) (bool, error) {
var err error var err error
var output string var output string
@ -444,33 +444,50 @@ func (util *RBDUtil) rbdStatus(b *rbdMounter) (bool, error) {
l := len(b.Mon) l := len(b.Mon)
start := rand.Int() % l start := rand.Int() % l
// iterate all hosts until mount succeeds. // iterate all hosts until rbd command succeeds.
for i := start; i < start+l; i++ { for i := start; i < start+l; i++ {
mon := b.Mon[i%l] mon := b.Mon[i%l]
// cmd "rbd status" list the rbd client watch with the following output: // cmd "rbd status" list the rbd client watch with the following output:
//
// # there is a watcher (exit=0)
// Watchers: // Watchers:
// watcher=10.16.153.105:0/710245699 client.14163 cookie=1 // watcher=10.16.153.105:0/710245699 client.14163 cookie=1
//
// # there is no watcher (exit=0)
// Watchers: none
//
// Otherwise, exit is non-zero, for example:
//
// # image does not exist (exit=2)
// rbd: error opening image kubernetes-dynamic-pvc-<UUID>: (2) No such file or directory
//
glog.V(4).Infof("rbd: status %s using mon %s, pool %s id %s key %s", b.Image, mon, b.Pool, b.adminId, b.adminSecret) glog.V(4).Infof("rbd: status %s using mon %s, pool %s id %s key %s", b.Image, mon, b.Pool, b.adminId, b.adminSecret)
cmd, err = b.exec.Run("rbd", cmd, err = b.exec.Run("rbd",
"status", b.Image, "--pool", b.Pool, "-m", mon, "--id", b.adminId, "--key="+b.adminSecret) "status", b.Image, "--pool", b.Pool, "-m", mon, "--id", b.adminId, "--key="+b.adminSecret)
output = string(cmd) output = string(cmd)
if err != nil { // break if command succeeds
if err.Error() == rbdCmdErr { if err == nil {
glog.Errorf("rbd cmd not found") break
} else {
// ignore error code, just checkout output for watcher string
glog.Warningf("failed to execute rbd status on mon %s", mon)
}
} }
if strings.Contains(output, imageWatcherStr) { if err.Error() == rbdCmdErr {
glog.V(4).Infof("rbd: watchers on %s: %s", b.Image, output) glog.Errorf("rbd cmd not found")
return true, nil // fail fast if command not found
} else { return false, err
glog.Warningf("rbd: no watchers on %s", b.Image)
return false, nil
} }
} }
return false, nil
// If command never succeed, returns its last error.
if err != nil {
return false, err
}
if strings.Contains(output, imageWatcherStr) {
glog.V(4).Infof("rbd: watchers on %s: %s", b.Image, output)
return true, nil
} else {
glog.Warningf("rbd: no watchers on %s", b.Image)
return false, nil
}
} }