Skip to content

Commit

Permalink
NextDevices Fix
Browse files Browse the repository at this point in the history 
Courtesy of akutz's latest libstorage commit (PR thecodeteam#268) the device
path of the next available device is now retrieved from the executor
and not the storage driver.

The device names for EBS are now /dev/xvd[f-p] as opposed to
/dev/xvd[a-z], since EBS has suddenly begun to rebel against device
names that are not of the pattern [f-p].

Also added some comments to the executor
  • Loading branch information
@proudh
proudh committed Sep 14, 2016
1 parent 096060b commit 6c5f821
Show file tree
Hide file tree
Showing 3 changed files with 41 additions and 165 deletions.
20 changes: 16 additions & 4 deletions drivers/storage/ebs/executor/ec2_executor.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,12 @@ func newDriver() types.StorageExecutor {

func (d *driver) Init(ctx types.Context, config gofig.Config) error {
d.config = config
// EBS suggests to use /dev/sd[f-p] for Linux EC2 instances.
// Also on Linux EC2 instances, although the device path may show up
// as /dev/sd* on the EC2 side, it will appear locally as /dev/xvd*
d.nextDeviceInfo = &types.NextDeviceInfo{
Prefix: "xvd",
Pattern: "[a-z]",
Pattern: "[f-p]",
Ignore: false,
}

Expand All @@ -52,10 +55,11 @@ func InstanceID() (*types.InstanceID, error) {
return newDriver().InstanceID(nil, nil)
}

// InstanceID returns the aws instance configuration
// InstanceID returns the instance ID from the current instance from metadata
func (d *driver) InstanceID(
ctx types.Context,
opts types.Store) (*types.InstanceID, error) {
// Retrieve instance ID from metadata
res, err := http.Get("http://169.254.169.254/latest/meta-data/instance-id/")
if err != nil {
return nil, goof.WithError("ec2 instance id lookup failed", err)
Expand All @@ -78,10 +82,11 @@ func (d *driver) InstanceID(
func (d *driver) NextDevice(
ctx types.Context,
opts types.Store) (string, error) {
// All possible device paths on Linux EC2 instances are /dev/xvd[f-p]
letters := []string{
"a", "b", "c", "d", "e", "f", "g", "h",
"i", "j", "k", "l", "m", "n", "o", "p"}
"f", "g", "h", "i", "j", "k", "l", "m", "n", "o", "p"}

// Find which letters are used for local devices
localDeviceNames := make(map[string]bool)

localDevices, err := d.LocalDevices(
Expand All @@ -101,6 +106,7 @@ func (d *driver) NextDevice(
}
}

// Find which letters are used for ephemeral devices
ephemeralDevices, err := d.getEphemeralDevices()
if err != nil {
return "", goof.WithError("error getting ephemeral devices", err)
Expand All @@ -116,6 +122,7 @@ func (d *driver) NextDevice(
}
}

// Find next available letter for device path
for _, letter := range letters {
if !localDeviceNames[letter] {
nextDeviceName := "/dev/" +
Expand All @@ -126,6 +133,7 @@ func (d *driver) NextDevice(
return "", goof.New("No available device")
}

// Retrieve device paths currently attached and/or mounted
func (d *driver) LocalDevices(
ctx types.Context,
opts *types.LocalDevicesOpts) (*types.LocalDevices, error) {
Expand All @@ -147,6 +155,9 @@ func (d *driver) LocalDevices(
fields := strings.Fields(line)
if len(fields) == 4 {
deviceName = "/dev/" + fields[3]
// Device ID is also device path for EBS, since it
// can be obtained both locally and remotely
// (remotely being from the AWS API side)
localDevices[deviceName] = deviceName
}
}
Expand All @@ -158,6 +169,7 @@ func (d *driver) LocalDevices(

}

// Find ephemeral devices from metadata
func (d *driver) getEphemeralDevices() (deviceNames []string, err error) {
// Get list of all block devices
res, err := http.Get("http://169.254.169.254/latest/meta-data/block-device-mapping/")
Expand Down
171 changes: 12 additions & 159 deletions drivers/storage/ebs/storage/ec2_storage.go
View file
Original file line number Diff line number Diff line change
@@ -1,13 +1,11 @@
package storage

import (
"bufio"
"encoding/json"
"fmt"
"io/ioutil"
"net"
"net/http"
"regexp"
"strings"
"time"

Expand Down Expand Up @@ -102,8 +100,11 @@ func (d *driver) Init(context types.Context, config gofig.Config) error {
}

d.nextDeviceInfo = &types.NextDeviceInfo{
Prefix: "xvd",
Pattern: "[a-z]",
Prefix: "xvd",
// EBS suggests to use /dev/sd[f-p] for Linux EC2 instances.
// Also on Linux EC2 instances, although the device path may show up
// as /dev/sd* on the EC2 side, it will appear locally as /dev/xvd*
Pattern: "[f-p]",
Ignore: false,
}

Expand Down Expand Up @@ -519,12 +520,6 @@ func (d *driver) VolumeAttach(
ctx types.Context,
volumeID string,
opts *types.VolumeAttachOpts) (*types.Volume, string, error) {
nextDeviceName, err := d.GetNextAvailableDeviceName(ctx)
if err != nil {
return nil, "", goof.WithError(
"Error getting next available device name", err)
}

// review volume with attachments to any host
ec2vols, err := d.getVolume(ctx, volumeID, "")
if err != nil {
Expand All @@ -550,6 +545,11 @@ func (d *driver) VolumeAttach(
return nil, "", goof.WithError("Error detaching volume", err)
}
}
// Retrieve next device name
nextDeviceName := ""
if opts.NextDevice != nil {
nextDeviceName = *opts.NextDevice
}

// Attach volume via helper function which uses EC2 API call
err = d.attachVolume(ctx, volumeID, volumes[0].Name, nextDeviceName)
Expand Down Expand Up @@ -578,7 +578,8 @@ func (d *driver) VolumeAttach(
return nil, "", goof.WithError("error getting volume", err)
}

// Token is the attachment's device name
// Token is the attachment's device name, which will be matched
// to the executor's device ID
return attachedVol, nextDeviceName, nil
}

Expand Down Expand Up @@ -1026,154 +1027,6 @@ func getInstanceIdentityDocument() (*instanceIdentityDocument, error) {
return &document, nil
}

// Reviews current device names and determines the next for volume attachment
func (d *driver) GetNextAvailableDeviceName(ctx types.Context) (string, error) {
// Possible suffixes for device names
letters := []string{
"a", "b", "c", "d", "e", "f", "g", "h",
"i", "j", "k", "l", "m", "n", "o", "p"}

blockDeviceNames := make(map[string]bool)

// Get block devices' names
blockDeviceMapping, err := d.GetVolumeMapping()
if err != nil {
return "", err
}

for _, blockDevice := range blockDeviceMapping {
// Compensate for kernel volume mapping i.e. check for both "/dev/sda" and "/dev/xvda"
re, _ := regexp.Compile(`^/dev/(sd|` +
d.nextDeviceInfo.Prefix + `)(` +
d.nextDeviceInfo.Pattern + `)`)
res := re.FindStringSubmatch(blockDevice.Name)
if len(res) > 0 {
blockDeviceNames[res[2]] = true
}
}

// Get local devices' names
localDevices, ldOK := context.LocalDevices(ctx)
if !ldOK {
return "", goof.New("Error getting local devices from context")
}

for _, localDevice := range localDevices.DeviceMap {
re, _ := regexp.Compile(`^/dev/` +
d.nextDeviceInfo.Prefix +
`(` + d.nextDeviceInfo.Pattern + `)`)
res := re.FindStringSubmatch(localDevice)
if len(res) > 0 {
blockDeviceNames[res[1]] = true
}
}

// Get ephemeral devices' names
ephemeralDevices, err := d.getEphemeralDevices()
if err != nil {
return "", goof.WithError("error getting ephemeral devices", err)
}

for _, ephemeralDevice := range ephemeralDevices {
re, _ := regexp.Compile(`^` +
d.nextDeviceInfo.Prefix +
`(` + d.nextDeviceInfo.Pattern + `)`)
res := re.FindStringSubmatch(ephemeralDevice)
if len(res) > 0 {
blockDeviceNames[res[1]] = true
}
}

// Check which device names have been used
for _, letter := range letters {
if !blockDeviceNames[letter] {
nextDeviceName := "/dev/" +
d.nextDeviceInfo.Prefix + letter
log.WithFields(log.Fields{
"driverName": d.Name(),
"nextDeviceName": nextDeviceName}).Info("got next device name")
return nextDeviceName, nil
}
}
return "", goof.New("No available device")
}

// Retrieves ephemeral device names
func (d *driver) getEphemeralDevices() (deviceNames []string, err error) {
// Get list of all block devices
res, err := http.Get("http://169.254.169.254/latest/meta-data/block-device-mapping/")
if err != nil {
return nil, goof.WithError("ec2 block device mapping lookup failed", err)
}
blockDeviceMappings, err := ioutil.ReadAll(res.Body)
res.Body.Close()
if err != nil {
return nil, goof.WithError("error reading ec2 block device mappings", err)
}

// Filter list of all block devices for ephemeral devices
re, _ := regexp.Compile(`ephemeral([0-9]|1[0-9]|2[0-3])$`)

scanner := bufio.NewScanner(strings.NewReader(string(blockDeviceMappings)))
scanner.Split(bufio.ScanWords)

var input string
for scanner.Scan() {
input = scanner.Text()
if re.MatchString(input) {
// Find device name for ephemeral device
res, err := http.Get("http://169.254.169.254/latest/meta-data/block-device-mapping/" + input)
if err != nil {
return nil, goof.WithError("ec2 block device mapping lookup failed", err)
}
deviceName, err := ioutil.ReadAll(res.Body)
res.Body.Close()
if err != nil {
return nil, goof.WithError("error reading ec2 block device mappings", err)
}

deviceNames = append(deviceNames, string(deviceName))
}
}

return deviceNames, nil
}

// Retrieves EC2 API block devices as libStorage types.VolumeDevice
func (d *driver) GetVolumeMapping() ([]*types.VolumeDevice, error) {
blockDevices, err := d.getBlockDevices(d.instanceDocument.InstanceID)
if err != nil {
return nil, goof.WithError("Error getting block devices", err)
}

var BlockDevices []*types.VolumeDevice
for _, blockDevice := range blockDevices {
sdBlockDevice := &types.VolumeDevice{
ProviderName: d.Name(),
InstanceID: &types.InstanceID{ID: d.instanceDocument.InstanceID, Driver: d.Name()},
Region: d.instanceDocument.Region,
Name: *blockDevice.DeviceName,
VolumeID: *((*blockDevice.Ebs).VolumeId),
Status: *((*blockDevice.Ebs).Status),
}
BlockDevices = append(BlockDevices, sdBlockDevice)
}

return BlockDevices, nil
}

// Retrieves EC2 API BlockDeviceMappings from instance
func (d *driver) getBlockDevices(
instanceID string) ([]*awsec2.InstanceBlockDeviceMapping, error) {

instance, err := d.getInstance()
if err != nil {
return nil, goof.WithError("Error getting instance", err)
}

return instance.BlockDeviceMappings, nil
}

// Used in VolumeCreate
func (d *driver) createVolume(ctx types.Context, volumeName, snapshotID string,
vol *types.Volume) (*awsec2.Volume, error) {
Expand Down
15 changes: 13 additions & 2 deletions drivers/storage/ebs/tests/ec2_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ func TestMain(m *testing.M) {
///////////////////////////////////////////////////////////////////////
///////// PUBLIC TESTS /////////
///////////////////////////////////////////////////////////////////////

// Test if backwards compatibility for "ec2" and "ebs" work in config
func TestConfig(t *testing.T) {
if skipTests() {
t.SkipNow()
Expand Down Expand Up @@ -407,8 +407,19 @@ func volumeRemove(t *testing.T, client types.Client, volumeID string) {
func volumeAttach(
t *testing.T, client types.Client, volumeID string) *types.Volume {
log.WithField("volumeID", volumeID).Info("attaching volume")
// Get next device name from executor
nextDevice, err := client.Executor().NextDevice(context.Background().WithValue(context.ServiceKey, ebs.Name),
utils.NewStore())
assert.NoError(t, err)
if err != nil {
t.Error("error getting next device name from executor")
t.FailNow()
}

reply, token, err := client.API().VolumeAttach(
nil, ebs.Name, volumeID, &types.VolumeAttachRequest{})
nil, ebs.Name, volumeID, &types.VolumeAttachRequest{
NextDeviceName: &nextDevice,
})

assert.NoError(t, err)
if err != nil {
Expand Down

0 comments on commit 6c5f821

Please sign in to comment.