github
/
k3s
mirror of https://github.com/k3s-io/k3s
1
0
Fork 0
Code Issues Projects Releases Wiki Activity

Merge pull request #55893 from gnufied/aws-detach-fix-for-stopped-nodes 

Automatic merge from submit-queue (batch tested with PRs 55893, 55906, 55026). 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>.

AWS: Implement fix for detaching volume from stopped instances

We should detach volume from stopped instanes.

Fixes https://github.com/kubernetes/kubernetes/issues/55892

```release-note
AWS: Fix detaching volume from stopped nodes.
```
pull/6/head
Kubernetes Submit Queue 2017-11-28 18:24:49 -08:00 committed by GitHub
parent 8226973ae8 ac2c68ad8f
commit 6503f39bb1
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 97 additions and 114 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

64
pkg/cloudprovider/providers/aws/aws.go
View File

@ -2021,26 +2021,24 @@ func (c *Cloud) AttachDisk(diskName KubernetesVolumeID, nodeName types.NodeName,
// DetachDisk implements Volumes.DetachDisk
func (c *Cloud) DetachDisk(diskName KubernetesVolumeID, nodeName types.NodeName) (string, error) {
disk, err := newAWSDisk(c, diskName)
if err != nil {
diskInfo, attached, err := c.checkIfAttachedToNode(diskName, nodeName)
if diskInfo == nil {
return "", err
}
awsInstance, info, err := c.getFullInstance(nodeName)
if err != nil {
if err == cloudprovider.InstanceNotFound {
// If instance no longer exists, safe to assume volume is not attached.
glog.Warningf(
"Instance %q does not exist. DetachDisk will assume disk %q is not attached to it.",
nodeName,
diskName)
return "", nil
}
return "", err
if !attached && diskInfo.ec2Instance != nil {
glog.Warningf("DetachDisk %s called for node %s but volume is attached to node %s", diskName, nodeName, diskInfo.nodeName)
return "", nil
}
mountDevice, alreadyAttached, err := c.getMountDevice(awsInstance, info, disk.awsID, false)
if !attached {
return "", nil
}
awsInstance := newAWSInstance(c.ec2, diskInfo.ec2Instance)
mountDevice, alreadyAttached, err := c.getMountDevice(awsInstance, diskInfo.ec2Instance, diskInfo.disk.awsID, false)
if err != nil {
return "", err
}
@ -2052,18 +2050,19 @@ func (c *Cloud) DetachDisk(diskName KubernetesVolumeID, nodeName types.NodeName)
request := ec2.DetachVolumeInput{
InstanceId: &awsInstance.awsID,
VolumeId: disk.awsID.awsString(),
VolumeId: diskInfo.disk.awsID.awsString(),
}
response, err := c.ec2.DetachVolume(&request)
if err != nil {
return "", fmt.Errorf("error detaching EBS volume %q from %q: %q", disk.awsID, awsInstance.awsID, err)
return "", fmt.Errorf("error detaching EBS volume %q from %q: %q", diskInfo.disk.awsID, awsInstance.awsID, err)
}
if response == nil {
return "", errors.New("no response from DetachVolume")
}
attachment, err := disk.waitForAttachmentStatus("detached")
attachment, err := diskInfo.disk.waitForAttachmentStatus("detached")
if err != nil {
return "", err
}
@ -2076,7 +2075,7 @@ func (c *Cloud) DetachDisk(diskName KubernetesVolumeID, nodeName types.NodeName)
}
if mountDevice != "" {
c.endAttaching(awsInstance, disk.awsID, mountDevice)
c.endAttaching(awsInstance, diskInfo.disk.awsID, mountDevice)
// We don't check the return value - we don't really expect the attachment to have been
// in progress, though it might have been
}
@ -2320,32 +2319,13 @@ func (c *Cloud) GetDiskPath(volumeName KubernetesVolumeID) (string, error) {
// DiskIsAttached implements Volumes.DiskIsAttached
func (c *Cloud) DiskIsAttached(diskName KubernetesVolumeID, nodeName types.NodeName) (bool, error) {
_, instance, err := c.getFullInstance(nodeName)
if err != nil {
if err == cloudprovider.InstanceNotFound {
// If instance no longer exists, safe to assume volume is not attached.
glog.Warningf(
"Instance %q does not exist. DiskIsAttached will assume disk %q is not attached to it.",
nodeName,
diskName)
return false, nil
}
diskInfo, attached, err := c.checkIfAttachedToNode(diskName, nodeName)
return false, err
if diskInfo == nil {
return true, err
}
diskID, err := diskName.mapToAWSVolumeID()
if err != nil {
return false, fmt.Errorf("error mapping volume spec %q to aws id: %v", diskName, err)
}
for _, blockDevice := range instance.BlockDeviceMappings {
id := awsVolumeID(aws.StringValue(blockDevice.Ebs.VolumeId))
if id == diskID {
return true, nil
}
}
return false, nil
return attached, nil
}
func (c *Cloud) DisksAreAttached(nodeDisks map[types.NodeName][]KubernetesVolumeID) (map[types.NodeName]map[KubernetesVolumeID]bool, error) {

65
pkg/cloudprovider/providers/aws/volumes.go
View File

@ -23,6 +23,9 @@ import (
"strings"
"github.com/aws/aws-sdk-go/aws"
"github.com/aws/aws-sdk-go/service/ec2"
"github.com/golang/glog"
"k8s.io/apimachinery/pkg/types"
)
// awsVolumeRegMatch represents Regex Match for AWS volume.
@ -46,6 +49,16 @@ func (i awsVolumeID) awsString() *string {
// * <awsVolumeId>
type KubernetesVolumeID string
// DiskInfo returns aws disk information in easy to use manner
type diskInfo struct {
ec2Instance *ec2.Instance
nodeName types.NodeName
volumeState string
attachmentState string
hasAttachment bool
disk *awsDisk
}
// mapToAWSVolumeID extracts the awsVolumeID from the KubernetesVolumeID
func (name KubernetesVolumeID) mapToAWSVolumeID() (awsVolumeID, error) {
// name looks like aws://availability-zone/awsVolumeId
@ -85,3 +98,55 @@ func (name KubernetesVolumeID) mapToAWSVolumeID() (awsVolumeID, error) {
return awsVolumeID(awsID), nil
}
func GetAWSVolumeID(kubeVolumeID string) (string, error) {
kid := KubernetesVolumeID(kubeVolumeID)
awsID, err := kid.mapToAWSVolumeID()
return string(awsID), err
}
func (c *Cloud) checkIfAttachedToNode(diskName KubernetesVolumeID, nodeName types.NodeName) (*diskInfo, bool, error) {
disk, err := newAWSDisk(c, diskName)
if err != nil {
return nil, true, err
}
awsDiskInfo := &diskInfo{
disk: disk,
}
info, err := disk.describeVolume()
if err != nil {
describeError := fmt.Errorf("Error describing volume %s with %v", diskName, err)
glog.Warning(describeError)
awsDiskInfo.volumeState = "unknown"
return awsDiskInfo, false, describeError
}
awsDiskInfo.volumeState = aws.StringValue(info.State)
if len(info.Attachments) > 0 {
attachment := info.Attachments[0]
awsDiskInfo.attachmentState = aws.StringValue(attachment.State)
instanceID := aws.StringValue(attachment.InstanceId)
instanceInfo, err := c.getInstanceByID(instanceID)
// This should never happen but if it does it could mean there was a race and instance
// has been deleted
if err != nil {
fetchErr := fmt.Errorf("Error fetching instance %s for volume %s", instanceID, diskName)
glog.Warning(fetchErr)
return awsDiskInfo, false, fetchErr
}
awsDiskInfo.ec2Instance = instanceInfo
awsDiskInfo.nodeName = mapInstanceToNodeName(instanceInfo)
awsDiskInfo.hasAttachment = true
if awsDiskInfo.nodeName == nodeName {
return awsDiskInfo, true, nil
}
}
return awsDiskInfo, false, nil
}

16
pkg/volume/aws_ebs/attacher.go
View File

@ -256,21 +256,7 @@ func (plugin *awsElasticBlockStorePlugin) NewDetacher() (volume.Detacher, error)
func (detacher *awsElasticBlockStoreDetacher) Detach(volumeName string, nodeName types.NodeName) error {
volumeID := aws.KubernetesVolumeID(path.Base(volumeName))
attached, err := detacher.awsVolumes.DiskIsAttached(volumeID, nodeName)
if err != nil {
// Log error and continue with detach
glog.Errorf(
"Error checking if volume (%q) is already attached to current node (%q). Will continue and try detach anyway. err=%v",
volumeID, nodeName, err)
}
if err == nil && !attached {
// Volume is already detached from node.
glog.Infof("detach operation was successful. volume %q is already detached from node %q.", volumeID, nodeName)
return nil
}
if _, err = detacher.awsVolumes.DetachDisk(volumeID, nodeName); err != nil {
if _, err := detacher.awsVolumes.DetachDisk(volumeID, nodeName); err != nil {
glog.Errorf("Error detaching volumeID %q: %v", volumeID, err)
return err
}

66
pkg/volume/aws_ebs/attacher_test.go
View File

@ -62,10 +62,9 @@ func TestGetVolumeName_PersistentVolume(t *testing.T) {
type testcase struct {
name aws.KubernetesVolumeID
// For fake AWS:
attach attachCall
detach detachCall
diskIsAttached diskIsAttachedCall
t *testing.T
attach attachCall
detach detachCall
t *testing.T
// Actual test to run
test func(test *testcase) (string, error)
@ -81,7 +80,6 @@ func TestAttachDetach(t *testing.T) {
spec := createVolSpec(diskName, readOnly)
attachError := errors.New("Fake attach error")
detachError := errors.New("Fake detach error")
diskCheckError := errors.New("Fake DiskIsAttached error")
tests := []testcase{
// Successful Attach call
{
@ -107,44 +105,18 @@ func TestAttachDetach(t *testing.T) {
// Detach succeeds
{
name: "Detach_Positive",
diskIsAttached: diskIsAttachedCall{diskName, nodeName, true, nil},
detach: detachCall{diskName, nodeName, "/dev/sda", nil},
name: "Detach_Positive",
detach: detachCall{diskName, nodeName, "/dev/sda", nil},
test: func(testcase *testcase) (string, error) {
detacher := newDetacher(testcase)
mountPath := "/mnt/" + string(diskName)
return "", detacher.Detach(mountPath, nodeName)
},
},
// Disk is already detached
{
name: "Detach_Positive_AlreadyDetached",
diskIsAttached: diskIsAttachedCall{diskName, nodeName, false, nil},
test: func(testcase *testcase) (string, error) {
detacher := newDetacher(testcase)
mountPath := "/mnt/" + string(diskName)
return "", detacher.Detach(mountPath, nodeName)
},
},
// Detach succeeds when DiskIsAttached fails
{
name: "Detach_Positive_CheckFails",
diskIsAttached: diskIsAttachedCall{diskName, nodeName, false, diskCheckError},
detach: detachCall{diskName, nodeName, "/dev/sda", nil},
test: func(testcase *testcase) (string, error) {
detacher := newDetacher(testcase)
mountPath := "/mnt/" + string(diskName)
return "", detacher.Detach(mountPath, nodeName)
},
},
// Detach fails
{
name: "Detach_Negative",
diskIsAttached: diskIsAttachedCall{diskName, nodeName, false, diskCheckError},
detach: detachCall{diskName, nodeName, "", detachError},
name: "Detach_Negative",
detach: detachCall{diskName, nodeName, "", detachError},
test: func(testcase *testcase) (string, error) {
detacher := newDetacher(testcase)
mountPath := "/mnt/" + string(diskName)
@ -298,28 +270,8 @@ func (testcase *testcase) DetachDisk(diskName aws.KubernetesVolumeID, nodeName t
}
func (testcase *testcase) DiskIsAttached(diskName aws.KubernetesVolumeID, nodeName types.NodeName) (bool, error) {
expected := &testcase.diskIsAttached
if expected.diskName == "" && expected.nodeName == "" {
// testcase.diskIsAttached looks uninitialized, test did not expect to
// call DiskIsAttached
testcase.t.Errorf("Unexpected DiskIsAttached call!")
return false, errors.New("Unexpected DiskIsAttached call!")
}
if expected.diskName != diskName {
testcase.t.Errorf("Unexpected DiskIsAttached call: expected diskName %s, got %s", expected.diskName, diskName)
return false, errors.New("Unexpected DiskIsAttached call: wrong diskName")
}
if expected.nodeName != nodeName {
testcase.t.Errorf("Unexpected DiskIsAttached call: expected nodeName %s, got %s", expected.nodeName, nodeName)
return false, errors.New("Unexpected DiskIsAttached call: wrong nodeName")
}
glog.V(4).Infof("DiskIsAttached call: %s, %s, returning %v, %v", diskName, nodeName, expected.isAttached, expected.ret)
return expected.isAttached, expected.ret
// DetachDisk no longer relies on DiskIsAttached api call
return false, nil
}
func (testcase *testcase) DisksAreAttached(nodeDisks map[types.NodeName][]aws.KubernetesVolumeID) (map[types.NodeName]map[aws.KubernetesVolumeID]bool, error) {