From 6c823dbdabc43abd1d2efcd938630584381cca3e Mon Sep 17 00:00:00 2001 From: Justin Santa Barbara Date: Thu, 2 Apr 2015 11:56:11 -0700 Subject: [PATCH] Small clean-ups --- pkg/cloudprovider/aws/aws.go | 27 ++++++++++++++++----------- pkg/volume/aws_pd/aws_pd.go | 5 ++++- pkg/volume/aws_pd/aws_util.go | 9 ++++----- 3 files changed, 24 insertions(+), 17 deletions(-) diff --git a/pkg/cloudprovider/aws/aws.go b/pkg/cloudprovider/aws/aws.go index bd22b747f2..8d9ed7a848 100644 --- a/pkg/cloudprovider/aws/aws.go +++ b/pkg/cloudprovider/aws/aws.go @@ -70,7 +70,8 @@ type VolumeOptions struct { type Volumes interface { // Attach the disk to the specified instance // instanceName can be empty to mean "the instance on which we are running" - AttachDisk(instanceName string, volumeName string, readOnly bool) error + // Returns the device (e.g. /dev/xvdf) where we attached the volume + AttachDisk(instanceName string, volumeName string, readOnly bool) (string, error) // Detach the disk from the specified instance // instanceName can be empty to mean "the instance on which we are running" DetachDisk(instanceName string, volumeName string) error @@ -819,10 +820,10 @@ func (aws *AWSCloud) getSelfAwsInstance() *awsInstance { } // Implements Volumes.AttachDisk -func (aws *AWSCloud) AttachDisk(instanceName string, diskName string, readOnly bool) error { +func (aws *AWSCloud) AttachDisk(instanceName string, diskName string, readOnly bool) (string, error) { disk, err := newAwsDisk(aws.ec2, diskName) if err != nil { - return err + return "", err } var awsInstance *awsInstance @@ -831,7 +832,7 @@ func (aws *AWSCloud) AttachDisk(instanceName string, diskName string, readOnly b } else { instance, err := aws.getInstancesByDnsName(instanceName) if err != nil { - return fmt.Errorf("Error finding instance: %v", err) + return "", fmt.Errorf("Error finding instance: %v", err) } awsInstance = newAwsInstance(aws.ec2, instance.InstanceId) @@ -840,12 +841,12 @@ func (aws *AWSCloud) AttachDisk(instanceName string, diskName string, readOnly b if readOnly { // TODO: We could enforce this when we mount the volume (?) // TODO: We could also snapshot the volume and attach copies of it - return errors.New("AWS volumes cannot be mounted read-only") + return "", errors.New("AWS volumes cannot be mounted read-only") } mountDevice, alreadyAttached, err := awsInstance.assignMountDevice(disk.awsId) if err != nil { - return err + return "", err } attached := false @@ -858,8 +859,8 @@ func (aws *AWSCloud) AttachDisk(instanceName string, diskName string, readOnly b if !alreadyAttached { attachResponse, err := aws.ec2.AttachVolume(disk.awsId, awsInstance.awsId, mountDevice) if err != nil { - // TODO: Check if concurrently attached? - return fmt.Errorf("Error attaching EBS volume: %v", err) + // TODO: Check if the volume was concurrently attached? + return "", fmt.Errorf("Error attaching EBS volume: %v", err) } glog.V(2).Info("AttachVolume request returned %v", attachResponse) @@ -867,13 +868,17 @@ func (aws *AWSCloud) AttachDisk(instanceName string, diskName string, readOnly b err = disk.waitForAttachmentStatus("attached") if err != nil { - return err + return "", err } attached = true - // TODO: Return device name (and note that it might look like /dev/xvdf, not /dev/sdf) - return nil + hostDevice := mountDevice + if strings.HasPrefix(hostDevice, "/dev/sd") { + // Inside the instance, the mountpoint /dev/sdf looks like /dev/xvdf + hostDevice = "/dev/xvd" + hostDevice[7:] + } + return hostDevice, nil } // Implements Volumes.DetachDisk diff --git a/pkg/volume/aws_pd/aws_pd.go b/pkg/volume/aws_pd/aws_pd.go index cb48b07cd8..49645c3205 100644 --- a/pkg/volume/aws_pd/aws_pd.go +++ b/pkg/volume/aws_pd/aws_pd.go @@ -231,7 +231,10 @@ func (pd *awsPersistentDisk) SetUpAt(dir string) error { } func makeGlobalPDName(host volume.VolumeHost, devName string) string { - return path.Join(host.GetPluginDir(awsPersistentDiskPluginName), "mounts", devName) + // Clean up the URI to be more fs-friendly + name := devName + name = strings.Replace(name, "://", "/", -1) + return path.Join(host.GetPluginDir(awsPersistentDiskPluginName), "mounts", name) } func (pd *awsPersistentDisk) GetPath() string { diff --git a/pkg/volume/aws_pd/aws_util.go b/pkg/volume/aws_pd/aws_util.go index f265aebdcb..c69157ff9b 100644 --- a/pkg/volume/aws_pd/aws_util.go +++ b/pkg/volume/aws_pd/aws_util.go @@ -20,7 +20,6 @@ import ( "errors" "fmt" "os" - "path" "time" "github.com/GoogleCloudPlatform/kubernetes/pkg/util/exec" @@ -41,12 +40,12 @@ func (util *AWSDiskUtil) AttachAndMountDisk(pd *awsPersistentDisk, globalPDPath if pd.readOnly { flags = mount.FlagReadOnly } - if err := volumes.AttachDisk("", pd.pdName, pd.readOnly); err != nil { + devicePath, err := volumes.AttachDisk("", pd.pdName, pd.readOnly) + if err != nil { return err } - devicePath := path.Join("/dev/disk/by-id/", "aws-"+pd.pdName) if pd.partition != "" { - devicePath = devicePath + "-part" + pd.partition + devicePath = devicePath + pd.partition } //TODO(jonesdl) There should probably be better method than busy-waiting here. numTries := 0 @@ -60,7 +59,7 @@ func (util *AWSDiskUtil) AttachAndMountDisk(pd *awsPersistentDisk, globalPDPath } numTries++ if numTries == 10 { - return errors.New("Could not attach disk: Timeout after 10s") + return errors.New("Could not attach disk: Timeout after 10s (" + devicePath + ")") } time.Sleep(time.Second) }