Skip to content

Commit

Permalink
Create volumes in outpost when necessary
Browse files Browse the repository at this point in the history 
With this commit we start passing the outpost arn (if it's present) to EBS.
Before this commit, when the request came from an outpost instance,
we would ask the EBS to create the volume in the parent AZ, which is not
the expected behavior.

In order to determine if we're running in an outpost instance, we use
the ec2 instance metadata "outpost-arn". It returns a '404' for
non-outpost instances or the outpost-arn as string otherwise. Now we
include it in the topology requiment and pass it along to CreateVolume
request if it's present.

Automated e2e tests can be considerd impossible for this case as getting
an outpost rack is not an easy task.
  • Loading branch information
@ayberk
ayberk committed Sep 16, 2020
1 parent 9b33867 commit 77006e8
Show file tree
Hide file tree
Showing 12 changed files with 366 additions and 17 deletions.
34 changes: 26 additions & 8 deletions pkg/cloud/cloud.go
View file
Expand Up @@ -122,6 +122,7 @@ type Disk struct {
CapacityGiB int64
AvailabilityZone string
SnapshotID string
OutpostArn string
}

// DiskOptions represents parameters to create an EBS volume
Expand All @@ -131,6 +132,7 @@ type DiskOptions struct {
VolumeType string
IOPSPerGB int
AvailabilityZone string
OutpostArn string
Encrypted bool
// KmsKeyID represents a fully qualified resource name to the key to use for encryption.
// example: arn:aws:kms:us-east-1:012345678910:key/abcd1234-a123-456a-a12b-a123b4cd56ef
Expand Down Expand Up @@ -269,13 +271,26 @@ func (c *cloud) CreateDisk(ctx context.Context, volumeName string, diskOptions *
return nil, fmt.Errorf("failed to get availability zone %s", err)
}
}

request := &ec2.CreateVolumeInput{
AvailabilityZone: aws.String(zone),
Size: aws.Int64(capacityGiB),
VolumeType: aws.String(createType),
TagSpecifications: []*ec2.TagSpecification{&tagSpec},
Encrypted: aws.Bool(diskOptions.Encrypted),
var request *ec2.CreateVolumeInput

// EBS doesn't handle empty outpost arn, so we have to include it only when it's non-empty
if len(diskOptions.OutpostArn) > 0 {
request = &ec2.CreateVolumeInput{
AvailabilityZone: aws.String(zone),
OutpostArn: aws.String(diskOptions.OutpostArn),
Size: aws.Int64(capacityGiB),
VolumeType: aws.String(createType),
TagSpecifications: []*ec2.TagSpecification{&tagSpec},
Encrypted: aws.Bool(diskOptions.Encrypted),
}
} else {
request = &ec2.CreateVolumeInput{
AvailabilityZone: aws.String(zone),
Size: aws.Int64(capacityGiB),
VolumeType: aws.String(createType),
TagSpecifications: []*ec2.TagSpecification{&tagSpec},
Encrypted: aws.Bool(diskOptions.Encrypted),
}
}
if len(diskOptions.KmsKeyID) > 0 {
request.KmsKeyId = aws.String(diskOptions.KmsKeyID)
Expand Down Expand Up @@ -311,7 +326,9 @@ func (c *cloud) CreateDisk(ctx context.Context, volumeName string, diskOptions *
return nil, fmt.Errorf("failed to get an available volume in EC2: %v", err)
}

return &Disk{CapacityGiB: size, VolumeID: volumeID, AvailabilityZone: zone, SnapshotID: snapshotID}, nil
outpostArn := aws.StringValue(response.OutpostArn)

return &Disk{CapacityGiB: size, VolumeID: volumeID, AvailabilityZone: zone, SnapshotID: snapshotID, OutpostArn: outpostArn}, nil
}

func (c *cloud) DeleteDisk(ctx context.Context, volumeID string) (bool, error) {
Expand Down Expand Up @@ -498,6 +515,7 @@ func (c *cloud) GetDiskByID(ctx context.Context, volumeID string) (*Disk, error)
VolumeID: aws.StringValue(volume.VolumeId),
CapacityGiB: aws.Int64Value(volume.Size),
AvailabilityZone: aws.StringValue(volume.AvailabilityZone),
OutpostArn: aws.StringValue(volume.OutpostArn),
}, nil
}

Expand Down
37 changes: 37 additions & 0 deletions pkg/cloud/cloud_test.go
View file
Expand Up @@ -94,6 +94,39 @@ func TestCreateDisk(t *testing.T) {
},
expErr: nil,
},
{
name: "success: outpost volume",
volumeName: "vol-test-name",
diskOptions: &DiskOptions{
CapacityBytes: util.GiBToBytes(1),
Tags: map[string]string{VolumeNameTagKey: "vol-test"},
AvailabilityZone: expZone,
OutpostArn: "aws:foo:bar",
},
expDisk: &Disk{
VolumeID: "vol-test",
CapacityGiB: 1,
AvailabilityZone: expZone,
OutpostArn: "aws:foo:bar",
},
expErr: nil,
},
{
name: "success: empty outpost arn",
volumeName: "vol-test-name",
diskOptions: &DiskOptions{
CapacityBytes: util.GiBToBytes(1),
Tags: map[string]string{VolumeNameTagKey: "vol-test"},
AvailabilityZone: expZone,
},
expDisk: &Disk{
VolumeID: "vol-test",
CapacityGiB: 1,
AvailabilityZone: expZone,
OutpostArn: "",
},
expErr: nil,
},
{
name: "fail: CreateVolume returned CreateVolume error",
volumeName: "vol-test-name-error",
Expand Down Expand Up @@ -162,6 +195,7 @@ func TestCreateDisk(t *testing.T) {
Size: aws.Int64(util.BytesToGiB(tc.diskOptions.CapacityBytes)),
State: aws.String(volState),
AvailabilityZone: aws.String(tc.diskOptions.AvailabilityZone),
OutpostArn: aws.String(tc.diskOptions.OutpostArn),
}
snapshot := &ec2.Snapshot{
SnapshotId: aws.String(tc.diskOptions.SnapshotID),
Expand Down Expand Up @@ -203,6 +237,9 @@ func TestCreateDisk(t *testing.T) {
if tc.expDisk.AvailabilityZone != disk.AvailabilityZone {
t.Fatalf("CreateDisk() failed: expected availabilityZone %q, got %q", tc.expDisk.AvailabilityZone, disk.AvailabilityZone)
}
if tc.expDisk.OutpostArn != disk.OutpostArn {
t.Fatalf("CreateDisk() failed: expected outpoustArn %q, got %q", tc.expDisk.OutpostArn, disk.OutpostArn)
}
}
}

Expand Down
20 changes: 20 additions & 0 deletions pkg/cloud/metadata.go
View file
Expand Up @@ -18,6 +18,7 @@ package cloud

import (
"fmt"
"strings"

"github.com/aws/aws-sdk-go/aws"
"github.com/aws/aws-sdk-go/aws/ec2metadata"
Expand All @@ -26,6 +27,8 @@ import (

type EC2Metadata interface {
Available() bool
// ec2 instance metadata endpoints: https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/instancedata-data-retrieval.html
GetMetadata(string) (string, error)
GetInstanceIdentityDocument() (ec2metadata.EC2InstanceIdentityDocument, error)
}

Expand All @@ -35,15 +38,19 @@ type MetadataService interface {
GetInstanceType() string
GetRegion() string
GetAvailabilityZone() string
GetOutpostArn() string
}

type Metadata struct {
InstanceID string
InstanceType string
Region string
AvailabilityZone string
OutpostArn string
}

const OutpostArnEndpoint string = "outpost-arn"

var _ MetadataService = &Metadata{}

// GetInstanceID returns the instance identification.
Expand All @@ -66,6 +73,11 @@ func (m *Metadata) GetAvailabilityZone() string {
return m.AvailabilityZone
}

// GetOutpostArn returns outpost arn if instance is running on an outpost. empty otherwise.
func (m *Metadata) GetOutpostArn() string {
return m.OutpostArn
}

func NewMetadata() (MetadataService, error) {
sess := session.Must(session.NewSession(&aws.Config{}))
svc := ec2metadata.New(sess)
Expand Down Expand Up @@ -99,10 +111,18 @@ func NewMetadataService(svc EC2Metadata) (MetadataService, error) {
return nil, fmt.Errorf("could not get valid EC2 availavility zone")
}

outpostArn, err := svc.GetMetadata(OutpostArnEndpoint)
// "outpust-arn" returns 404 for non-outpost instances. note that the request is made to a link-local address.
// There's a case to be made here to ignore the error so a failure here wouldn't affect non-outpost calls.
if err != nil && !strings.Contains(err.Error(), "404") {
return nil, fmt.Errorf("something went wrong while getting EC2 outpost arn")
}

return &Metadata{
InstanceID: doc.InstanceID,
InstanceType: doc.InstanceType,
Region: doc.Region,
AvailabilityZone: doc.AvailabilityZone,
OutpostArn: outpostArn,
}, nil
}
52 changes: 50 additions & 2 deletions pkg/cloud/metadata_test.go
View file
Expand Up @@ -38,7 +38,9 @@ func TestNewMetadataService(t *testing.T) {
isAvailable bool
isPartial bool
identityDocument ec2metadata.EC2InstanceIdentityDocument
outpostArn string
err error
getOutpostArnErr error // We should keep this specific to outpost-arn until we need to use more endpoints
}{
{
name: "success: normal",
Expand All @@ -51,6 +53,31 @@ func TestNewMetadataService(t *testing.T) {
},
err: nil,
},
{
name: "success: outpost-arn is available",
isAvailable: true,
identityDocument: ec2metadata.EC2InstanceIdentityDocument{
InstanceID: stdInstanceID,
InstanceType: stdInstanceType,
Region: stdRegion,
AvailabilityZone: stdAvailabilityZone,
},
outpostArn: "arn:aws:outposts:us-west-2:111111111111:outpost/op-0aaa000a0aaaa00a0",
err: nil,
},
{
name: "success: outpost-arn is not found",
isAvailable: true,
identityDocument: ec2metadata.EC2InstanceIdentityDocument{
InstanceID: stdInstanceID,
InstanceType: stdInstanceType,
Region: stdRegion,
AvailabilityZone: stdAvailabilityZone,
},
outpostArn: "",
err: nil,
getOutpostArnErr: fmt.Errorf("404"),
},
{
name: "fail: metadata not available",
isAvailable: false,
Expand Down Expand Up @@ -109,6 +136,19 @@ func TestNewMetadataService(t *testing.T) {
},
err: nil,
},
{
name: "fail: outpost-arn failed",
isAvailable: true,
identityDocument: ec2metadata.EC2InstanceIdentityDocument{
InstanceID: stdInstanceID,
InstanceType: stdInstanceType,
Region: stdRegion,
AvailabilityZone: stdAvailabilityZone,
},
outpostArn: "",
err: nil,
getOutpostArnErr: fmt.Errorf("405"),
},
}

for _, tc := range testCases {
Expand All @@ -121,8 +161,12 @@ func TestNewMetadataService(t *testing.T) {
mockEC2Metadata.EXPECT().GetInstanceIdentityDocument().Return(tc.identityDocument, tc.err)
}

m, err := NewMetadataService(mockEC2Metadata)
if tc.isAvailable && tc.err == nil && !tc.isPartial {
mockEC2Metadata.EXPECT().GetMetadata(OutpostArnEndpoint).Return(tc.outpostArn, tc.getOutpostArnErr)
}

m, err := NewMetadataService(mockEC2Metadata)
if tc.isAvailable && tc.err == nil && tc.getOutpostArnErr == nil && !tc.isPartial {
if err != nil {
t.Fatalf("NewMetadataService() failed: expected no error, got %v", err)
}
Expand All @@ -142,8 +186,12 @@ func TestNewMetadataService(t *testing.T) {
if m.GetAvailabilityZone() != tc.identityDocument.AvailabilityZone {
t.Fatalf("GetAvailabilityZone() failed: expected %v, got %v", tc.identityDocument.AvailabilityZone, m.GetAvailabilityZone())
}

if m.GetOutpostArn() != tc.outpostArn {
t.Fatalf("GetOutpostArn() failed: expected %v, got %v", tc.outpostArn, m.GetOutpostArn())
}
} else {
if err == nil {
if err == nil && tc.getOutpostArnErr == nil {
t.Fatal("NewMetadataService() failed: expected error when GetInstanceIdentityDocument returns partial data, got nothing")
}
}
Expand Down
15 changes: 15 additions & 0 deletions pkg/cloud/mocks/mock_ec2metadata.go
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

30 changes: 29 additions & 1 deletion pkg/driver/controller.go
View file
Expand Up @@ -186,6 +186,7 @@ func (d *controllerService) CreateVolume(ctx context.Context, req *csi.CreateVol

// create a new volume
zone := pickAvailabilityZone(req.GetAccessibilityRequirements())
outpostArn := getOutpostArn(req.GetAccessibilityRequirements())

// fill volume tags
if d.driverOptions.kubernetesClusterID != "" {
Expand All @@ -203,6 +204,7 @@ func (d *controllerService) CreateVolume(ctx context.Context, req *csi.CreateVol
VolumeType: volumeType,
IOPSPerGB: iopsPerGB,
AvailabilityZone: zone,
OutpostArn: outpostArn,
Encrypted: isEncrypted,
KmsKeyID: kmsKeyID,
SnapshotID: snapshotID,
Expand Down Expand Up @@ -528,6 +530,25 @@ func pickAvailabilityZone(requirement *csi.TopologyRequirement) string {
return ""
}

func getOutpostArn(requirement *csi.TopologyRequirement) string {
if requirement == nil {
return ""
}
for _, topology := range requirement.GetPreferred() {
outpostArn, exists := topology.GetSegments()[OutpostArnKey]
if exists {
return outpostArn
}
}
for _, topology := range requirement.GetRequisite() {
outpostArn, exists := topology.GetSegments()[OutpostArnKey]
if exists {
return outpostArn
}
}
return ""
}

func newCreateVolumeResponse(disk *cloud.Disk) *csi.CreateVolumeResponse {
var src *csi.VolumeContentSource
if disk.SnapshotID != "" {
Expand All @@ -539,14 +560,21 @@ func newCreateVolumeResponse(disk *cloud.Disk) *csi.CreateVolumeResponse {
},
}
}

segments := map[string]string{TopologyKey: disk.AvailabilityZone}

if len(disk.OutpostArn) > 0 {
segments[OutpostArnKey] = disk.OutpostArn
}

return &csi.CreateVolumeResponse{
Volume: &csi.Volume{
VolumeId: disk.VolumeID,
CapacityBytes: util.GiBToBytes(disk.CapacityGiB),
VolumeContext: map[string]string{},
AccessibleTopology: []*csi.Topology{
{
Segments: map[string]string{TopologyKey: disk.AvailabilityZone},
Segments: segments,
},
},
ContentSource: src,
Expand Down

0 comments on commit 77006e8

Please sign in to comment.