Skip to content

K
k3s
Commit 4a6e1f2a authored by FengyunPan's avatar FengyunPan
Browse files
Options

Don't return err when volume's status is 'attaching'

When volume's status is 'attaching', its attachments will be None, controllermanager can't get device path and make some failed event. But it is normal, let's fix it.
parent b58a1b56
Hide whitespace changes
Inline Side-by-side
Showing with 175 additions and 35 deletions
pkg/cloudprovider/providers/openstack/openstack_volumes.go
View file @ 4a6e1f2a
...@@ -74,8 +74,14 @@ type VolumeCreateOpts struct { ...@@ -74,8 +74,14 @@ type VolumeCreateOpts struct {
Metadata map[string]string Metadata map[string]string
} }
func (volumes *VolumesV1) createVolume(opts VolumeCreateOpts) (string, string, error) { const (
VolumeAvailableStatus = "available"
VolumeInUseStatus = "in-use"
VolumeDeletedStatus = "deleted"
VolumeErrorStatus = "error"
)
func (volumes *VolumesV1) createVolume(opts VolumeCreateOpts) (string, string, error) {
create_opts := volumes_v1.CreateOpts{ create_opts := volumes_v1.CreateOpts{
Name: opts.Name, Name: opts.Name,
Size: opts.Size, Size: opts.Size,
...@@ -92,7 +98,6 @@ func (volumes *VolumesV1) createVolume(opts VolumeCreateOpts) (string, string, e ...@@ -92,7 +98,6 @@ func (volumes *VolumesV1) createVolume(opts VolumeCreateOpts) (string, string, e
} }
func (volumes *VolumesV2) createVolume(opts VolumeCreateOpts) (string, string, error) { func (volumes *VolumesV2) createVolume(opts VolumeCreateOpts) (string, string, error) {
create_opts := volumes_v2.CreateOpts{ create_opts := volumes_v2.CreateOpts{
Name: opts.Name, Name: opts.Name,
Size: opts.Size, Size: opts.Size,
...@@ -201,12 +206,33 @@ func (volumes *VolumesV2) deleteVolume(volumeName string) error { ...@@ -201,12 +206,33 @@ func (volumes *VolumesV2) deleteVolume(volumeName string) error {
return err return err
} }
func (os *OpenStack) OperationPending(diskName string) (bool, string, error) {
volume, err := os.getVolume(diskName)
if err != nil {
return false, "", err
}
volumeStatus := volume.Status
if volumeStatus == VolumeErrorStatus {
glog.Errorf("status of volume %s is %s", diskName, volumeStatus)
return false, volumeStatus, nil
}
if volumeStatus == VolumeAvailableStatus || volumeStatus == VolumeInUseStatus || volumeStatus == VolumeDeletedStatus {
return false, volume.Status, nil
}
return true, volumeStatus, nil
}
// Attaches given cinder volume to the compute running kubelet // Attaches given cinder volume to the compute running kubelet
func (os *OpenStack) AttachDisk(instanceID string, diskName string) (string, error) { func (os *OpenStack) AttachDisk(instanceID string, diskName string) (string, error) {
volume, err := os.getVolume(diskName) volume, err := os.getVolume(diskName)
if err != nil { if err != nil {
return "", err return "", err
} }
if volume.Status != VolumeAvailableStatus {
errmsg := fmt.Sprintf("volume %s status is %s, not %s, can not be attached to instance %s.", volume.Name, volume.Status, VolumeAvailableStatus, instanceID)
glog.Errorf(errmsg)
return "", errors.New(errmsg)
}
cClient, err := openstack.NewComputeV2(os.provider, gophercloud.EndpointOpts{ cClient, err := openstack.NewComputeV2(os.provider, gophercloud.EndpointOpts{
Region: os.region, Region: os.region,
}) })
...@@ -245,6 +271,11 @@ func (os *OpenStack) DetachDisk(instanceID string, partialDiskId string) error { ...@@ -245,6 +271,11 @@ func (os *OpenStack) DetachDisk(instanceID string, partialDiskId string) error {
if err != nil { if err != nil {
return err return err
} }
if volume.Status != VolumeInUseStatus {
errmsg := fmt.Sprintf("can not detach volume %s, its status is %s.", volume.Name, volume.Status)
glog.Errorf(errmsg)
return errors.New(errmsg)
}
cClient, err := openstack.NewComputeV2(os.provider, gophercloud.EndpointOpts{ cClient, err := openstack.NewComputeV2(os.provider, gophercloud.EndpointOpts{
Region: os.region, Region: os.region,
}) })
...@@ -369,6 +400,11 @@ func (os *OpenStack) GetAttachmentDiskPath(instanceID string, diskName string) ( ...@@ -369,6 +400,11 @@ func (os *OpenStack) GetAttachmentDiskPath(instanceID string, diskName string) (
if err != nil { if err != nil {
return "", err return "", err
} }
if volume.Status != VolumeInUseStatus {
errmsg := fmt.Sprintf("can not get device path of volume %s, its status is %s.", volume.Name, volume.Status)
glog.Errorf(errmsg)
return "", errors.New(errmsg)
}
if volume.AttachedServerId != "" { if volume.AttachedServerId != "" {
if instanceID == volume.AttachedServerId { if instanceID == volume.AttachedServerId {
// Attachment[0]["device"] points to the device path // Attachment[0]["device"] points to the device path
...@@ -380,7 +416,7 @@ func (os *OpenStack) GetAttachmentDiskPath(instanceID string, diskName string) ( ...@@ -380,7 +416,7 @@ func (os *OpenStack) GetAttachmentDiskPath(instanceID string, diskName string) (
return "", errors.New(errMsg) return "", errors.New(errMsg)
} }
} }
return "", fmt.Errorf("volume %s is not attached to %s", diskName, instanceID) return "", fmt.Errorf("volume %s has not ServerId.", diskName)
} }
// query if a volume is attached to a compute instance // query if a volume is attached to a compute instance
......
pkg/cloudprovider/providers/rackspace/rackspace.go
View file @ 4a6e1f2a
...@@ -45,8 +45,13 @@ import ( ...@@ -45,8 +45,13 @@ import (
"k8s.io/kubernetes/pkg/cloudprovider" "k8s.io/kubernetes/pkg/cloudprovider"
) )
const ProviderName = "rackspace" const (
const metaDataPath = "/media/configdrive/openstack/latest/meta_data.json" ProviderName = "rackspace"
MetaDataPath = "/media/configdrive/openstack/latest/meta_data.json"
VolumeAvailableStatus = "available"
VolumeInUseStatus = "in-use"
VolumeErrorStatus = "error"
)
var ErrNotFound = errors.New("Failed to find object") var ErrNotFound = errors.New("Failed to find object")
var ErrMultipleResults = errors.New("Multiple results where only one expected") var ErrMultipleResults = errors.New("Multiple results where only one expected")
...@@ -146,16 +151,16 @@ func parseMetaData(file io.Reader) (string, error) { ...@@ -146,16 +151,16 @@ func parseMetaData(file io.Reader) (string, error) {
metaData := MetaData{} metaData := MetaData{}
err = json.Unmarshal(metaDataBytes, &metaData) err = json.Unmarshal(metaDataBytes, &metaData)
if err != nil { if err != nil {
return "", fmt.Errorf("Cannot parse %s: %v", metaDataPath, err) return "", fmt.Errorf("Cannot parse %s: %v", MetaDataPath, err)
} }
return metaData.UUID, nil return metaData.UUID, nil
} }
func readInstanceID() (string, error) { func readInstanceID() (string, error) {
file, err := os.Open(metaDataPath) file, err := os.Open(MetaDataPath)
if err != nil { if err != nil {
return "", fmt.Errorf("Cannot open %s: %v", metaDataPath, err) return "", fmt.Errorf("Cannot open %s: %v", MetaDataPath, err)
} }
defer file.Close() defer file.Close()
...@@ -487,6 +492,22 @@ func (rs *Rackspace) DeleteVolume(volumeName string) error { ...@@ -487,6 +492,22 @@ func (rs *Rackspace) DeleteVolume(volumeName string) error {
return errors.New("unimplemented") return errors.New("unimplemented")
} }
func (rs *Rackspace) OperationPending(diskName string) (bool, string, error) {
disk, err := rs.getVolume(diskName)
if err != nil {
return false, "", err
}
volumeStatus := disk.Status
if volumeStatus == VolumeErrorStatus {
glog.Errorf("status of volume %s is %s", diskName, volumeStatus)
return false, volumeStatus, nil
}
if volumeStatus == VolumeAvailableStatus || volumeStatus == VolumeInUseStatus {
return false, disk.Status, nil
}
return true, volumeStatus, nil
}
// Attaches given cinder volume to the compute running kubelet // Attaches given cinder volume to the compute running kubelet
func (rs *Rackspace) AttachDisk(instanceID string, diskName string) (string, error) { func (rs *Rackspace) AttachDisk(instanceID string, diskName string) (string, error) {
disk, err := rs.getVolume(diskName) disk, err := rs.getVolume(diskName)
...@@ -494,6 +515,12 @@ func (rs *Rackspace) AttachDisk(instanceID string, diskName string) (string, err ...@@ -494,6 +515,12 @@ func (rs *Rackspace) AttachDisk(instanceID string, diskName string) (string, err
return "", err return "", err
} }
if disk.Status != VolumeAvailableStatus {
errmsg := fmt.Sprintf("volume %s status is %s, not %s, can not be attached to instance %s.", disk.Name, disk.Status, VolumeAvailableStatus, instanceID)
glog.Errorf(errmsg)
return "", errors.New(errmsg)
}
compute, err := rs.getComputeClient() compute, err := rs.getComputeClient()
if err != nil { if err != nil {
return "", err return "", err
...@@ -589,6 +616,12 @@ func (rs *Rackspace) DetachDisk(instanceID string, partialDiskId string) error { ...@@ -589,6 +616,12 @@ func (rs *Rackspace) DetachDisk(instanceID string, partialDiskId string) error {
return err return err
} }
if disk.Status != VolumeInUseStatus {
errmsg := fmt.Sprintf("can not detach volume %s, its status is %s.", disk.Name, disk.Status)
glog.Errorf(errmsg)
return errors.New(errmsg)
}
compute, err := rs.getComputeClient() compute, err := rs.getComputeClient()
if err != nil { if err != nil {
return err return err
...@@ -626,6 +659,11 @@ func (rs *Rackspace) GetAttachmentDiskPath(instanceID string, diskName string) ( ...@@ -626,6 +659,11 @@ func (rs *Rackspace) GetAttachmentDiskPath(instanceID string, diskName string) (
if err != nil { if err != nil {
return "", err return "", err
} }
if disk.Status != VolumeInUseStatus {
errmsg := fmt.Sprintf("can not get device path of volume %s, its status is %s.", disk.Name, disk.Status)
glog.Errorf(errmsg)
return "", errors.New(errmsg)
}
if len(disk.Attachments) > 0 && disk.Attachments[0]["server_id"] != nil { if len(disk.Attachments) > 0 && disk.Attachments[0]["server_id"] != nil {
if instanceID == disk.Attachments[0]["server_id"] { if instanceID == disk.Attachments[0]["server_id"] {
// Attachment[0]["device"] points to the device path // Attachment[0]["device"] points to the device path
......
pkg/volume/cinder/attacher.go
View file @ 4a6e1f2a
...@@ -73,6 +73,15 @@ func (attacher *cinderDiskAttacher) Attach(spec *volume.Spec, nodeName types.Nod ...@@ -73,6 +73,15 @@ func (attacher *cinderDiskAttacher) Attach(spec *volume.Spec, nodeName types.Nod
return "", err return "", err
} }
pending, volumeStatus, err := attacher.cinderProvider.OperationPending(volumeID)
if err != nil {
return "", err
}
if pending {
glog.Infof("status of volume %q is %s, wait it to be finished", volumeID, volumeStatus)
return "", nil
}
attached, err := attacher.cinderProvider.DiskIsAttached(volumeID, instanceid) attached, err := attacher.cinderProvider.DiskIsAttached(volumeID, instanceid)
if err != nil { if err != nil {
// Log error and continue with attach // Log error and continue with attach
...@@ -89,18 +98,27 @@ func (attacher *cinderDiskAttacher) Attach(spec *volume.Spec, nodeName types.Nod ...@@ -89,18 +98,27 @@ func (attacher *cinderDiskAttacher) Attach(spec *volume.Spec, nodeName types.Nod
if err == nil { if err == nil {
glog.Infof("Attach operation successful: volume %q attached to instance %q.", volumeID, instanceid) glog.Infof("Attach operation successful: volume %q attached to instance %q.", volumeID, instanceid)
} else { } else {
glog.Infof("Attach volume %q to instance %q failed with %v", volumeID, instanceid, err) glog.Infof("Attach volume %q to instance %q failed with: %v", volumeID, instanceid, err)
return "", err return "", err
} }
} }
devicePath, err := attacher.cinderProvider.GetAttachmentDiskPath(instanceid, volumeID) // When volume's status is 'attaching', DiskIsAttached() return <false, nil>, and can't get device path.
// We should get device path next time and do not return err.
devicePath := ""
pending, _, err = attacher.cinderProvider.OperationPending(volumeID)
if err != nil { if err != nil {
glog.Infof("Attach volume %q to instance %q failed with %v", volumeID, instanceid, err)
return "", err return "", err
} }
if !pending {
devicePath, err = attacher.cinderProvider.GetAttachmentDiskPath(instanceid, volumeID)
if err != nil {
glog.Infof("Can not get device path of volume %q which be attached to instance %q, failed with: %v", volumeID, instanceid, err)
return "", err
}
}
return devicePath, err return devicePath, nil
} }
func (attacher *cinderDiskAttacher) VolumesAreAttached(specs []*volume.Spec, nodeName types.NodeName) (map[*volume.Spec]bool, error) { func (attacher *cinderDiskAttacher) VolumesAreAttached(specs []*volume.Spec, nodeName types.NodeName) (map[*volume.Spec]bool, error) {
......
pkg/volume/cinder/attacher_test.go
View file @ 4a6e1f2a
...@@ -33,6 +33,9 @@ import ( ...@@ -33,6 +33,9 @@ import (
"k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/types"
) )
const VolumeStatusPending = "pending"
const VolumeStatusDone = "done"
func TestGetDeviceName_Volume(t *testing.T) { func TestGetDeviceName_Volume(t *testing.T) {
plugin := newPlugin() plugin := newPlugin()
name := "my-cinder-volume" name := "my-cinder-volume"
...@@ -88,6 +91,7 @@ type testcase struct { ...@@ -88,6 +91,7 @@ type testcase struct {
// For fake GCE: // For fake GCE:
attach attachCall attach attachCall
detach detachCall detach detachCall
operationPending operationPendingCall
diskIsAttached diskIsAttachedCall diskIsAttached diskIsAttachedCall
disksAreAttached disksAreAttachedCall disksAreAttached disksAreAttachedCall
diskPath diskPathCall diskPath diskPathCall
...@@ -104,6 +108,8 @@ type testcase struct { ...@@ -104,6 +108,8 @@ type testcase struct {
func TestAttachDetach(t *testing.T) { func TestAttachDetach(t *testing.T) {
diskName := "disk" diskName := "disk"
instanceID := "instance" instanceID := "instance"
pending := VolumeStatusPending
done := VolumeStatusDone
nodeName := types.NodeName("nodeName") nodeName := types.NodeName("nodeName")
readOnly := false readOnly := false
spec := createVolSpec(diskName, readOnly) spec := createVolSpec(diskName, readOnly)
...@@ -115,11 +121,12 @@ func TestAttachDetach(t *testing.T) { ...@@ -115,11 +121,12 @@ func TestAttachDetach(t *testing.T) {
tests := []testcase{ tests := []testcase{
// Successful Attach call // Successful Attach call
{ {
name: "Attach_Positive", name: "Attach_Positive",
instanceID: instanceID, instanceID: instanceID,
diskIsAttached: diskIsAttachedCall{diskName, instanceID, false, nil}, operationPending: operationPendingCall{diskName, false, done, nil},
attach: attachCall{diskName, instanceID, "", nil}, diskIsAttached: diskIsAttachedCall{diskName, instanceID, false, nil},
diskPath: diskPathCall{diskName, instanceID, "/dev/sda", nil}, attach: attachCall{diskName, instanceID, "", nil},
diskPath: diskPathCall{diskName, instanceID, "/dev/sda", nil},
test: func(testcase *testcase) (string, error) { test: func(testcase *testcase) (string, error) {
attacher := newAttacher(testcase) attacher := newAttacher(testcase)
return attacher.Attach(spec, nodeName) return attacher.Attach(spec, nodeName)
...@@ -129,10 +136,11 @@ func TestAttachDetach(t *testing.T) { ...@@ -129,10 +136,11 @@ func TestAttachDetach(t *testing.T) {
// Disk is already attached // Disk is already attached
{ {
name: "Attach_Positive_AlreadyAttached", name: "Attach_Positive_AlreadyAttached",
instanceID: instanceID, instanceID: instanceID,
diskIsAttached: diskIsAttachedCall{diskName, instanceID, true, nil}, operationPending: operationPendingCall{diskName, false, done, nil},
diskPath: diskPathCall{diskName, instanceID, "/dev/sda", nil}, diskIsAttached: diskIsAttachedCall{diskName, instanceID, true, nil},
diskPath: diskPathCall{diskName, instanceID, "/dev/sda", nil},
test: func(testcase *testcase) (string, error) { test: func(testcase *testcase) (string, error) {
attacher := newAttacher(testcase) attacher := newAttacher(testcase)
return attacher.Attach(spec, nodeName) return attacher.Attach(spec, nodeName)
...@@ -140,13 +148,27 @@ func TestAttachDetach(t *testing.T) { ...@@ -140,13 +148,27 @@ func TestAttachDetach(t *testing.T) {
expectedResult: "/dev/sda", expectedResult: "/dev/sda",
}, },
// Disk is attaching
{
name: "Attach_is_attaching",
instanceID: instanceID,
operationPending: operationPendingCall{diskName, true, pending, nil},
diskIsAttached: diskIsAttachedCall{diskName, instanceID, false, diskCheckError},
test: func(testcase *testcase) (string, error) {
attacher := newAttacher(testcase)
return attacher.Attach(spec, nodeName)
},
expectedResult: "",
},
// DiskIsAttached fails and Attach succeeds // DiskIsAttached fails and Attach succeeds
{ {
name: "Attach_Positive_CheckFails", name: "Attach_Positive_CheckFails",
instanceID: instanceID, instanceID: instanceID,
diskIsAttached: diskIsAttachedCall{diskName, instanceID, false, diskCheckError}, operationPending: operationPendingCall{diskName, false, done, nil},
attach: attachCall{diskName, instanceID, "", nil}, diskIsAttached: diskIsAttachedCall{diskName, instanceID, false, diskCheckError},
diskPath: diskPathCall{diskName, instanceID, "/dev/sda", nil}, attach: attachCall{diskName, instanceID, "", nil},
diskPath: diskPathCall{diskName, instanceID, "/dev/sda", nil},
test: func(testcase *testcase) (string, error) { test: func(testcase *testcase) (string, error) {
attacher := newAttacher(testcase) attacher := newAttacher(testcase)
return attacher.Attach(spec, nodeName) return attacher.Attach(spec, nodeName)
...@@ -156,10 +178,11 @@ func TestAttachDetach(t *testing.T) { ...@@ -156,10 +178,11 @@ func TestAttachDetach(t *testing.T) {
// Attach call fails // Attach call fails
{ {
name: "Attach_Negative", name: "Attach_Negative",
instanceID: instanceID, instanceID: instanceID,
diskIsAttached: diskIsAttachedCall{diskName, instanceID, false, diskCheckError}, operationPending: operationPendingCall{diskName, false, done, nil},
attach: attachCall{diskName, instanceID, "/dev/sda", attachError}, diskIsAttached: diskIsAttachedCall{diskName, instanceID, false, diskCheckError},
attach: attachCall{diskName, instanceID, "/dev/sda", attachError},
test: func(testcase *testcase) (string, error) { test: func(testcase *testcase) (string, error) {
attacher := newAttacher(testcase) attacher := newAttacher(testcase)
return attacher.Attach(spec, nodeName) return attacher.Attach(spec, nodeName)
...@@ -169,11 +192,12 @@ func TestAttachDetach(t *testing.T) { ...@@ -169,11 +192,12 @@ func TestAttachDetach(t *testing.T) {
// GetAttachmentDiskPath call fails // GetAttachmentDiskPath call fails
{ {
name: "Attach_Negative_DiskPatchFails", name: "Attach_Negative_DiskPatchFails",
instanceID: instanceID, instanceID: instanceID,
diskIsAttached: diskIsAttachedCall{diskName, instanceID, false, diskCheckError}, operationPending: operationPendingCall{diskName, false, done, nil},
attach: attachCall{diskName, instanceID, "", nil}, diskIsAttached: diskIsAttachedCall{diskName, instanceID, false, diskCheckError},
diskPath: diskPathCall{diskName, instanceID, "", diskPathError}, attach: attachCall{diskName, instanceID, "", nil},
diskPath: diskPathCall{diskName, instanceID, "", diskPathError},
test: func(testcase *testcase) (string, error) { test: func(testcase *testcase) (string, error) {
attacher := newAttacher(testcase) attacher := newAttacher(testcase)
return attacher.Attach(spec, nodeName) return attacher.Attach(spec, nodeName)
...@@ -384,6 +408,13 @@ type detachCall struct { ...@@ -384,6 +408,13 @@ type detachCall struct {
ret error ret error
} }
type operationPendingCall struct {
diskName string
pending bool
volumeStatus string
ret error
}
type diskIsAttachedCall struct { type diskIsAttachedCall struct {
diskName, instanceID string diskName, instanceID string
isAttached bool isAttached bool
...@@ -453,6 +484,19 @@ func (testcase *testcase) DetachDisk(instanceID string, partialDiskId string) er ...@@ -453,6 +484,19 @@ func (testcase *testcase) DetachDisk(instanceID string, partialDiskId string) er
return expected.ret return expected.ret
} }
func (testcase *testcase) OperationPending(diskName string) (bool, string, error) {
expected := &testcase.operationPending
if expected.volumeStatus == VolumeStatusPending {
glog.V(4).Infof("OperationPending call: %s, returning %v, %v, %v", diskName, expected.pending, expected.volumeStatus, expected.ret)
return true, expected.volumeStatus, expected.ret
}
glog.V(4).Infof("OperationPending call: %s, returning %v, %v, %v", diskName, expected.pending, expected.volumeStatus, expected.ret)
return false, expected.volumeStatus, expected.ret
}
func (testcase *testcase) DiskIsAttached(diskName, instanceID string) (bool, error) { func (testcase *testcase) DiskIsAttached(diskName, instanceID string) (bool, error) {
expected := &testcase.diskIsAttached expected := &testcase.diskIsAttached
......
pkg/volume/cinder/cinder.go
View file @ 4a6e1f2a
...@@ -51,6 +51,7 @@ type CinderProvider interface { ...@@ -51,6 +51,7 @@ type CinderProvider interface {
GetDevicePath(diskId string) string GetDevicePath(diskId string) string
InstanceID() (string, error) InstanceID() (string, error)
GetAttachmentDiskPath(instanceID string, diskName string) (string, error) GetAttachmentDiskPath(instanceID string, diskName string) (string, error)
OperationPending(diskName string) (bool, string, error)
DiskIsAttached(diskName, instanceID string) (bool, error) DiskIsAttached(diskName, instanceID string) (bool, error)
DisksAreAttached(diskNames []string, instanceID string) (map[string]bool, error) DisksAreAttached(diskNames []string, instanceID string) (map[string]bool, error)
ShouldTrustDevicePath() bool ShouldTrustDevicePath() bool
......
pkg/volume/util/operationexecutor/operation_generator.go
View file @ 4a6e1f2a
...@@ -263,6 +263,9 @@ func (og *operationGenerator) GenerateAttachVolumeFunc( ...@@ -263,6 +263,9 @@ func (og *operationGenerator) GenerateAttachVolumeFunc(
og.recorder.Eventf(pod, v1.EventTypeWarning, kevents.FailedMountVolume, eventErr.Error()) og.recorder.Eventf(pod, v1.EventTypeWarning, kevents.FailedMountVolume, eventErr.Error())
} }
return detailedErr return detailedErr
} else if devicePath == "" {
// do nothing and get device path next time.
return nil
} }
glog.Infof(volumeToAttach.GenerateMsgDetailed("AttachVolume.Attach succeeded", "")) glog.Infof(volumeToAttach.GenerateMsgDetailed("AttachVolume.Attach succeeded", ""))
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment