Remove duplicate calls to describeInstance from aws

This change removes all duplicate calls to describeInstance
from aws volume code path.
pull/6/head
Hemant Kumar 2017-01-10 10:25:49 -05:00
parent a310171afd
commit aaa56e2c56
1 changed files with 27 additions and 22 deletions

View File

@ -1175,16 +1175,16 @@ func (i *awsInstance) describeInstance() (*ec2.Instance, error) {
// Gets the mountDevice already assigned to the volume, or assigns an unused mountDevice. // Gets the mountDevice already assigned to the volume, or assigns an unused mountDevice.
// If the volume is already assigned, this will return the existing mountDevice with alreadyAttached=true. // If the volume is already assigned, this will return the existing mountDevice with alreadyAttached=true.
// Otherwise the mountDevice is assigned by finding the first available mountDevice, and it is returned with alreadyAttached=false. // Otherwise the mountDevice is assigned by finding the first available mountDevice, and it is returned with alreadyAttached=false.
func (c *Cloud) getMountDevice(i *awsInstance, volumeID awsVolumeID, assign bool) (assigned mountDevice, alreadyAttached bool, err error) { func (c *Cloud) getMountDevice(
i *awsInstance,
info *ec2.Instance,
volumeID awsVolumeID,
assign bool) (assigned mountDevice, alreadyAttached bool, err error) {
instanceType := i.getInstanceType() instanceType := i.getInstanceType()
if instanceType == nil { if instanceType == nil {
return "", false, fmt.Errorf("could not get instance type for instance: %s", i.awsID) return "", false, fmt.Errorf("could not get instance type for instance: %s", i.awsID)
} }
info, err := i.describeInstance()
if err != nil {
return "", false, err
}
deviceMappings := map[mountDevice]awsVolumeID{} deviceMappings := map[mountDevice]awsVolumeID{}
for _, blockDevice := range info.BlockDeviceMappings { for _, blockDevice := range info.BlockDeviceMappings {
name := aws.StringValue(blockDevice.DeviceName) name := aws.StringValue(blockDevice.DeviceName)
@ -1441,7 +1441,7 @@ func (c *Cloud) AttachDisk(diskName KubernetesVolumeID, nodeName types.NodeName,
return "", err return "", err
} }
awsInstance, err := c.getAwsInstance(nodeName) awsInstance, info, err := c.getFullInstance(nodeName)
if err != nil { if err != nil {
return "", fmt.Errorf("error finding instance %s: %v", nodeName, err) return "", fmt.Errorf("error finding instance %s: %v", nodeName, err)
} }
@ -1468,7 +1468,7 @@ func (c *Cloud) AttachDisk(diskName KubernetesVolumeID, nodeName types.NodeName,
} }
}() }()
mountDevice, alreadyAttached, err = c.getMountDevice(awsInstance, disk.awsID, true) mountDevice, alreadyAttached, err = c.getMountDevice(awsInstance, info, disk.awsID, true)
if err != nil { if err != nil {
return "", err return "", err
} }
@ -1528,7 +1528,7 @@ func (c *Cloud) DetachDisk(diskName KubernetesVolumeID, nodeName types.NodeName)
return "", err return "", err
} }
awsInstance, err := c.getAwsInstance(nodeName) awsInstance, info, err := c.getFullInstance(nodeName)
if err != nil { if err != nil {
if err == cloudprovider.InstanceNotFound { if err == cloudprovider.InstanceNotFound {
// If instance no longer exists, safe to assume volume is not attached. // If instance no longer exists, safe to assume volume is not attached.
@ -1542,7 +1542,7 @@ func (c *Cloud) DetachDisk(diskName KubernetesVolumeID, nodeName types.NodeName)
return "", err return "", err
} }
mountDevice, alreadyAttached, err := c.getMountDevice(awsInstance, disk.awsID, false) mountDevice, alreadyAttached, err := c.getMountDevice(awsInstance, info, disk.awsID, false)
if err != nil { if err != nil {
return "", err return "", err
} }
@ -1726,7 +1726,7 @@ func (c *Cloud) GetDiskPath(volumeName KubernetesVolumeID) (string, error) {
// DiskIsAttached implements Volumes.DiskIsAttached // DiskIsAttached implements Volumes.DiskIsAttached
func (c *Cloud) DiskIsAttached(diskName KubernetesVolumeID, nodeName types.NodeName) (bool, error) { func (c *Cloud) DiskIsAttached(diskName KubernetesVolumeID, nodeName types.NodeName) (bool, error) {
awsInstance, err := c.getAwsInstance(nodeName) _, instance, err := c.getFullInstance(nodeName)
if err != nil { if err != nil {
if err == cloudprovider.InstanceNotFound { if err == cloudprovider.InstanceNotFound {
// If instance no longer exists, safe to assume volume is not attached. // If instance no longer exists, safe to assume volume is not attached.
@ -1745,11 +1745,7 @@ func (c *Cloud) DiskIsAttached(diskName KubernetesVolumeID, nodeName types.NodeN
return false, fmt.Errorf("error mapping volume spec %q to aws id: %v", diskName, err) return false, fmt.Errorf("error mapping volume spec %q to aws id: %v", diskName, err)
} }
info, err := awsInstance.describeInstance() for _, blockDevice := range instance.BlockDeviceMappings {
if err != nil {
return false, err
}
for _, blockDevice := range info.BlockDeviceMappings {
id := awsVolumeID(aws.StringValue(blockDevice.Ebs.VolumeId)) id := awsVolumeID(aws.StringValue(blockDevice.Ebs.VolumeId))
if id == diskID { if id == diskID {
return true, nil return true, nil
@ -1769,7 +1765,7 @@ func (c *Cloud) DisksAreAttached(diskNames []KubernetesVolumeID, nodeName types.
idToDiskName[volumeID] = diskName idToDiskName[volumeID] = diskName
attached[diskName] = false attached[diskName] = false
} }
awsInstance, err := c.getAwsInstance(nodeName) _, instance, err := c.getFullInstance(nodeName)
if err != nil { if err != nil {
if err == cloudprovider.InstanceNotFound { if err == cloudprovider.InstanceNotFound {
// If instance no longer exists, safe to assume volume is not attached. // If instance no longer exists, safe to assume volume is not attached.
@ -1782,11 +1778,7 @@ func (c *Cloud) DisksAreAttached(diskNames []KubernetesVolumeID, nodeName types.
return attached, err return attached, err
} }
info, err := awsInstance.describeInstance() for _, blockDevice := range instance.BlockDeviceMappings {
if err != nil {
return attached, err
}
for _, blockDevice := range info.BlockDeviceMappings {
volumeID := awsVolumeID(aws.StringValue(blockDevice.Ebs.VolumeId)) volumeID := awsVolumeID(aws.StringValue(blockDevice.Ebs.VolumeId))
diskName, found := idToDiskName[volumeID] diskName, found := idToDiskName[volumeID]
if found { if found {
@ -3136,7 +3128,7 @@ func (c *Cloud) getInstanceByID(instanceID string) (*ec2.Instance, error) {
} }
if len(instances) == 0 { if len(instances) == 0 {
return nil, fmt.Errorf("no instances found for instance: %s", instanceID) return nil, cloudprovider.InstanceNotFound
} }
if len(instances) > 1 { if len(instances) > 1 {
return nil, fmt.Errorf("multiple instances found for instance: %s", instanceID) return nil, fmt.Errorf("multiple instances found for instance: %s", instanceID)
@ -3267,6 +3259,19 @@ func (c *Cloud) getInstanceByNodeName(nodeName types.NodeName) (*ec2.Instance, e
return instance, err return instance, err
} }
func (c *Cloud) getFullInstance(nodeName types.NodeName) (*awsInstance, *ec2.Instance, error) {
if nodeName == "" {
instance, err := c.getInstanceByID(c.selfAWSInstance.awsID)
return c.selfAWSInstance, instance, err
}
instance, err := c.getInstanceByNodeName(nodeName)
if err != nil {
return nil, nil, err
}
awsInstance := newAWSInstance(c.ec2, instance)
return awsInstance, instance, err
}
// Add additional filters, to match on our tags // Add additional filters, to match on our tags
// This lets us run multiple k8s clusters in a single EC2 AZ // This lets us run multiple k8s clusters in a single EC2 AZ
func (c *Cloud) addFilters(filters []*ec2.Filter) []*ec2.Filter { func (c *Cloud) addFilters(filters []*ec2.Filter) []*ec2.Filter {