mirror of
https://github.com/k3s-io/kubernetes.git
synced 2025-07-23 19:56:01 +00:00
Wait for detach operation to complete
When volume's status is 'detaching', controllermanager will detach it again and return err. It is necessary to wait for detach operation to complete within the alloted time.
This commit is contained in:
parent
755d368c4a
commit
300f531389
@ -35,6 +35,7 @@ go_library(
|
||||
"//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
|
||||
"//vendor/k8s.io/apimachinery/pkg/types:go_default_library",
|
||||
"//vendor/k8s.io/apimachinery/pkg/util/sets:go_default_library",
|
||||
"//vendor/k8s.io/apimachinery/pkg/util/wait:go_default_library",
|
||||
],
|
||||
)
|
||||
|
||||
|
@ -25,6 +25,7 @@ import (
|
||||
|
||||
"github.com/golang/glog"
|
||||
"k8s.io/apimachinery/pkg/types"
|
||||
"k8s.io/apimachinery/pkg/util/wait"
|
||||
"k8s.io/kubernetes/pkg/util/exec"
|
||||
"k8s.io/kubernetes/pkg/util/mount"
|
||||
"k8s.io/kubernetes/pkg/volume"
|
||||
@ -41,7 +42,13 @@ var _ volume.Attacher = &cinderDiskAttacher{}
|
||||
var _ volume.AttachableVolumePlugin = &cinderPlugin{}
|
||||
|
||||
const (
|
||||
checkSleepDuration = time.Second
|
||||
checkSleepDuration = 1 * time.Second
|
||||
operationFinishInitDealy = 1 * time.Second
|
||||
operationFinishFactor = 1.1
|
||||
operationFinishSteps = 10
|
||||
diskDetachInitDealy = 1 * time.Second
|
||||
diskDetachFactor = 1.2
|
||||
diskDetachSteps = 13
|
||||
)
|
||||
|
||||
func (plugin *cinderPlugin) NewAttacher() (volume.Attacher, error) {
|
||||
@ -267,6 +274,63 @@ func (plugin *cinderPlugin) NewDetacher() (volume.Detacher, error) {
|
||||
}, nil
|
||||
}
|
||||
|
||||
func (detacher *cinderDiskDetacher) waitOperationFinished(volumeID string) error {
|
||||
backoff := wait.Backoff{
|
||||
Duration: operationFinishInitDealy,
|
||||
Factor: operationFinishFactor,
|
||||
Steps: operationFinishSteps,
|
||||
}
|
||||
|
||||
var volumeStatus string
|
||||
err := wait.ExponentialBackoff(backoff, func() (bool, error) {
|
||||
var pending bool
|
||||
var err error
|
||||
pending, volumeStatus, err = detacher.cinderProvider.OperationPending(volumeID)
|
||||
if err == nil {
|
||||
if pending == false {
|
||||
return true, nil
|
||||
} else {
|
||||
return false, nil
|
||||
}
|
||||
} else {
|
||||
return false, err
|
||||
}
|
||||
})
|
||||
|
||||
if err == wait.ErrWaitTimeout {
|
||||
err = fmt.Errorf("Volume %q is %s, can't finish within the alloted time", volumeID, volumeStatus)
|
||||
}
|
||||
|
||||
return err
|
||||
}
|
||||
|
||||
func (detacher *cinderDiskDetacher) waitDiskDetached(instanceID, volumeID string) error {
|
||||
backoff := wait.Backoff{
|
||||
Duration: diskDetachInitDealy,
|
||||
Factor: diskDetachFactor,
|
||||
Steps: diskDetachSteps,
|
||||
}
|
||||
|
||||
err := wait.ExponentialBackoff(backoff, func() (bool, error) {
|
||||
attached, err := detacher.cinderProvider.DiskIsAttached(instanceID, volumeID)
|
||||
if err == nil {
|
||||
if attached == false {
|
||||
return true, nil
|
||||
} else {
|
||||
return false, nil
|
||||
}
|
||||
} else {
|
||||
return false, err
|
||||
}
|
||||
})
|
||||
|
||||
if err == wait.ErrWaitTimeout {
|
||||
err = fmt.Errorf("Volume %q failed to detach within the alloted time", volumeID)
|
||||
}
|
||||
|
||||
return err
|
||||
}
|
||||
|
||||
func (detacher *cinderDiskDetacher) Detach(deviceMountPath string, nodeName types.NodeName) error {
|
||||
volumeID := path.Base(deviceMountPath)
|
||||
instances, res := detacher.cinderProvider.Instances()
|
||||
@ -278,6 +342,10 @@ func (detacher *cinderDiskDetacher) Detach(deviceMountPath string, nodeName type
|
||||
instanceID = instanceID[(ind + 1):]
|
||||
}
|
||||
|
||||
if err := detacher.waitOperationFinished(volumeID); err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
attached, err := detacher.cinderProvider.DiskIsAttached(instanceID, volumeID)
|
||||
if err != nil {
|
||||
// Log error and continue with detach
|
||||
@ -293,10 +361,14 @@ func (detacher *cinderDiskDetacher) Detach(deviceMountPath string, nodeName type
|
||||
}
|
||||
|
||||
if err = detacher.cinderProvider.DetachDisk(instanceID, volumeID); err != nil {
|
||||
glog.Errorf("Error detaching volume %q: %v", volumeID, err)
|
||||
glog.Errorf("Error detaching volume %q from node %q: %v", volumeID, nodeName, err)
|
||||
return err
|
||||
}
|
||||
glog.Infof("detached volume %q from instance %q", volumeID, instanceID)
|
||||
if err = detacher.waitDiskDetached(instanceID, volumeID); err != nil {
|
||||
glog.Errorf("Error waiting for volume %q to detach from node %q: %v", volumeID, nodeName, err)
|
||||
return err
|
||||
}
|
||||
glog.Infof("detached volume %q from node %q", volumeID, nodeName)
|
||||
return nil
|
||||
}
|
||||
|
||||
|
@ -36,6 +36,8 @@ import (
|
||||
const VolumeStatusPending = "pending"
|
||||
const VolumeStatusDone = "done"
|
||||
|
||||
var VolumeIsNotAttached = true
|
||||
|
||||
func TestGetDeviceName_Volume(t *testing.T) {
|
||||
plugin := newPlugin()
|
||||
name := "my-cinder-volume"
|
||||
@ -96,6 +98,7 @@ type testcase struct {
|
||||
disksAreAttached disksAreAttachedCall
|
||||
diskPath diskPathCall
|
||||
t *testing.T
|
||||
notAttached *bool
|
||||
|
||||
instanceID string
|
||||
// Actual test to run
|
||||
@ -118,6 +121,7 @@ func TestAttachDetach(t *testing.T) {
|
||||
diskCheckError := errors.New("Fake DiskIsAttached error")
|
||||
diskPathError := errors.New("Fake GetAttachmentDiskPath error")
|
||||
disksCheckError := errors.New("Fake DisksAreAttached error")
|
||||
operationFinishTimeout := errors.New("Fake waitOperationFinished error")
|
||||
tests := []testcase{
|
||||
// Successful Attach call
|
||||
{
|
||||
@ -247,10 +251,11 @@ func TestAttachDetach(t *testing.T) {
|
||||
|
||||
// Detach succeeds
|
||||
{
|
||||
name: "Detach_Positive",
|
||||
instanceID: instanceID,
|
||||
diskIsAttached: diskIsAttachedCall{instanceID, volumeID, true, nil},
|
||||
detach: detachCall{instanceID, volumeID, nil},
|
||||
name: "Detach_Positive",
|
||||
instanceID: instanceID,
|
||||
operationPending: operationPendingCall{volumeID, false, done, nil},
|
||||
diskIsAttached: diskIsAttachedCall{instanceID, volumeID, true, nil},
|
||||
detach: detachCall{instanceID, volumeID, nil},
|
||||
test: func(testcase *testcase) (string, error) {
|
||||
detacher := newDetacher(testcase)
|
||||
return "", detacher.Detach(volumeID, nodeName)
|
||||
@ -259,9 +264,10 @@ func TestAttachDetach(t *testing.T) {
|
||||
|
||||
// Disk is already detached
|
||||
{
|
||||
name: "Detach_Positive_AlreadyDetached",
|
||||
instanceID: instanceID,
|
||||
diskIsAttached: diskIsAttachedCall{instanceID, volumeID, false, nil},
|
||||
name: "Detach_Positive_AlreadyDetached",
|
||||
instanceID: instanceID,
|
||||
operationPending: operationPendingCall{volumeID, false, done, nil},
|
||||
diskIsAttached: diskIsAttachedCall{instanceID, volumeID, false, nil},
|
||||
test: func(testcase *testcase) (string, error) {
|
||||
detacher := newDetacher(testcase)
|
||||
return "", detacher.Detach(volumeID, nodeName)
|
||||
@ -270,10 +276,11 @@ func TestAttachDetach(t *testing.T) {
|
||||
|
||||
// Detach succeeds when DiskIsAttached fails
|
||||
{
|
||||
name: "Detach_Positive_CheckFails",
|
||||
instanceID: instanceID,
|
||||
diskIsAttached: diskIsAttachedCall{instanceID, volumeID, false, diskCheckError},
|
||||
detach: detachCall{instanceID, volumeID, nil},
|
||||
name: "Detach_Positive_CheckFails",
|
||||
instanceID: instanceID,
|
||||
operationPending: operationPendingCall{volumeID, false, done, nil},
|
||||
diskIsAttached: diskIsAttachedCall{instanceID, volumeID, false, diskCheckError},
|
||||
detach: detachCall{instanceID, volumeID, nil},
|
||||
test: func(testcase *testcase) (string, error) {
|
||||
detacher := newDetacher(testcase)
|
||||
return "", detacher.Detach(volumeID, nodeName)
|
||||
@ -282,20 +289,36 @@ func TestAttachDetach(t *testing.T) {
|
||||
|
||||
// Detach fails
|
||||
{
|
||||
name: "Detach_Negative",
|
||||
instanceID: instanceID,
|
||||
diskIsAttached: diskIsAttachedCall{instanceID, volumeID, false, diskCheckError},
|
||||
detach: detachCall{instanceID, volumeID, detachError},
|
||||
name: "Detach_Negative",
|
||||
instanceID: instanceID,
|
||||
operationPending: operationPendingCall{volumeID, false, done, nil},
|
||||
diskIsAttached: diskIsAttachedCall{instanceID, volumeID, false, diskCheckError},
|
||||
detach: detachCall{instanceID, volumeID, detachError},
|
||||
test: func(testcase *testcase) (string, error) {
|
||||
detacher := newDetacher(testcase)
|
||||
return "", detacher.Detach(volumeID, nodeName)
|
||||
},
|
||||
expectedError: detachError,
|
||||
},
|
||||
|
||||
// // Disk is detaching
|
||||
{
|
||||
name: "Detach_Is_Detaching",
|
||||
instanceID: instanceID,
|
||||
operationPending: operationPendingCall{volumeID, true, pending, operationFinishTimeout},
|
||||
test: func(testcase *testcase) (string, error) {
|
||||
detacher := newDetacher(testcase)
|
||||
return "", detacher.Detach(volumeID, nodeName)
|
||||
},
|
||||
expectedError: operationFinishTimeout,
|
||||
},
|
||||
}
|
||||
|
||||
for _, testcase := range tests {
|
||||
testcase.t = t
|
||||
// set VolumeIsNotAttached to test detach case, attach case ignore it
|
||||
VolumeIsNotAttached = false
|
||||
testcase.notAttached = &VolumeIsNotAttached
|
||||
result, err := testcase.test(&testcase)
|
||||
if err != testcase.expectedError {
|
||||
t.Errorf("%s failed: expected err=%q, got %q", testcase.name, testcase.expectedError, err)
|
||||
@ -457,6 +480,8 @@ func (testcase *testcase) AttachDisk(instanceID, volumeID string) (string, error
|
||||
|
||||
glog.V(4).Infof("AttachDisk call: %s, %s, returning %q, %v", volumeID, instanceID, expected.retDeviceName, expected.ret)
|
||||
|
||||
VolumeIsNotAttached = false
|
||||
testcase.notAttached = &VolumeIsNotAttached
|
||||
return expected.retDeviceName, expected.ret
|
||||
}
|
||||
|
||||
@ -482,6 +507,8 @@ func (testcase *testcase) DetachDisk(instanceID, volumeID string) error {
|
||||
|
||||
glog.V(4).Infof("DetachDisk call: %s, %s, returning %v", volumeID, instanceID, expected.ret)
|
||||
|
||||
VolumeIsNotAttached = true
|
||||
testcase.notAttached = &VolumeIsNotAttached
|
||||
return expected.ret
|
||||
}
|
||||
|
||||
@ -500,6 +527,10 @@ func (testcase *testcase) OperationPending(diskName string) (bool, string, error
|
||||
|
||||
func (testcase *testcase) DiskIsAttached(instanceID, volumeID string) (bool, error) {
|
||||
expected := &testcase.diskIsAttached
|
||||
// If testcase call DetachDisk*, return false
|
||||
if *testcase.notAttached == true {
|
||||
return false, nil
|
||||
}
|
||||
|
||||
if expected.volumeID == "" && expected.instanceID == "" {
|
||||
// testcase.diskIsAttached looks uninitialized, test did not expect to
|
||||
|
Loading…
Reference in New Issue
Block a user