diff --git a/pkg/cloudprovider/aws/aws.go b/pkg/cloudprovider/aws/aws.go index e116cf6bc66..343ce8feadf 100644 --- a/pkg/cloudprovider/aws/aws.go +++ b/pkg/cloudprovider/aws/aws.go @@ -48,9 +48,8 @@ type EC2 interface { // Attach a volume to an instance AttachVolume(volumeID, instanceId, mountDevice string) (resp *ec2.VolumeAttachment, err error) - // Detach a volume from whatever instance it is attached to - // TODO: We should specify the InstanceID and the Device, for safety - DetachVolume(volumeID, instanceId, mountDevice string) (resp *ec2.VolumeAttachment, err error) + // Detach a volume from an instance it is attached to + DetachVolume(request *ec2.DetachVolumeInput) (resp *ec2.VolumeAttachment, err error) // Lists volumes Volumes(volumeIDs []string, filter *ec2.Filter) (resp *ec2.DescribeVolumesOutput, err error) // Create an EBS volume @@ -224,13 +223,8 @@ func (s *awsSdkEC2) AttachVolume(volumeID, instanceId, device string) (resp *ec2 return s.ec2.AttachVolume(&request) } -func (s *awsSdkEC2) DetachVolume(volumeID, instanceId, device string) (resp *ec2.VolumeAttachment, err error) { - request := ec2.DetachVolumeInput{ - Device: &device, - InstanceID: &instanceId, - VolumeID: &volumeID, - } - return s.ec2.DetachVolume(&request) +func (s *awsSdkEC2) DetachVolume(request *ec2.DetachVolumeInput) (*ec2.VolumeAttachment, error) { + return s.ec2.DetachVolume(request) } func (s *awsSdkEC2) Volumes(volumeIDs []string, filter *ec2.Filter) (resp *ec2.DescribeVolumesOutput, err error) { @@ -963,6 +957,27 @@ func (aws *AWSCloud) getSelfAWSInstance() (*awsInstance, error) { return i, nil } +// Gets the awsInstance named instanceName, or the 'self' instance if instanceName == "" +func (aws *AWSCloud) getAwsInstance(instanceName string) (*awsInstance, error) { + var awsInstance *awsInstance + var err error + if instanceName == "" { + awsInstance, err = aws.getSelfAWSInstance() + if err != nil { + return nil, fmt.Errorf("error getting self-instance: %v", err) + } + } else { + instance, err := aws.getInstancesByDnsName(instanceName) + if err != nil { + return nil, fmt.Errorf("error finding instance: %v", err) + } + + awsInstance = newAWSInstance(aws.ec2, *instance.InstanceID) + } + + return awsInstance, nil +} + // Implements Volumes.AttachDisk func (aws *AWSCloud) AttachDisk(instanceName string, diskName string, readOnly bool) (string, error) { disk, err := newAWSDisk(aws.ec2, diskName) @@ -970,19 +985,9 @@ func (aws *AWSCloud) AttachDisk(instanceName string, diskName string, readOnly b return "", err } - var awsInstance *awsInstance - if instanceName == "" { - awsInstance, err = aws.getSelfAWSInstance() - if err != nil { - return "", fmt.Errorf("Error getting self-instance: %v", err) - } - } else { - instance, err := aws.getInstancesByDnsName(instanceName) - if err != nil { - return "", fmt.Errorf("Error finding instance: %v", err) - } - - awsInstance = newAWSInstance(aws.ec2, *instance.InstanceID) + awsInstance, err := aws.getAwsInstance(instanceName) + if err != nil { + return "", err } if readOnly { @@ -1035,8 +1040,17 @@ func (aws *AWSCloud) DetachDisk(instanceName string, diskName string) error { return err } - // TODO: We should specify the InstanceID and the Device, for safety - response, err := aws.ec2.DetachVolume(disk.awsID, instanceName, diskName) + awsInstance, err := aws.getAwsInstance(instanceName) + if err != nil { + return err + } + + request := ec2.DetachVolumeInput{ + InstanceID: &awsInstance.awsID, + VolumeID: &disk.awsID, + } + + response, err := aws.ec2.DetachVolume(&request) if err != nil { return fmt.Errorf("error detaching EBS volume: %v", err) } diff --git a/pkg/cloudprovider/aws/aws_test.go b/pkg/cloudprovider/aws/aws_test.go index e5eeb929ee5..40e62f21edf 100644 --- a/pkg/cloudprovider/aws/aws_test.go +++ b/pkg/cloudprovider/aws/aws_test.go @@ -208,7 +208,7 @@ func (ec2 *FakeEC2) AttachVolume(volumeID, instanceId, mountDevice string) (resp panic("Not implemented") } -func (ec2 *FakeEC2) DetachVolume(volumeID, instanceId, mountDevice string) (resp *ec2.VolumeAttachment, err error) { +func (ec2 *FakeEC2) DetachVolume(request *ec2.DetachVolumeInput) (resp *ec2.VolumeAttachment, err error) { panic("Not implemented") }