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 30, 2020
1 parent 427263a commit be125ff
Show file tree
Hide file tree
Showing 15 changed files with 658 additions and 14 deletions.
14 changes: 13 additions & 1 deletion pkg/cloud/cloud.go
View file
Original file line number Diff line number Diff line change
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 @@ -277,6 +279,12 @@ func (c *cloud) CreateDisk(ctx context.Context, volumeName string, diskOptions *
TagSpecifications: []*ec2.TagSpecification{&tagSpec},
Encrypted: aws.Bool(diskOptions.Encrypted),
}

// 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.OutpostArn = aws.String(diskOptions.OutpostArn)
}

if len(diskOptions.KmsKeyID) > 0 {
request.KmsKeyId = aws.String(diskOptions.KmsKeyID)
request.Encrypted = aws.Bool(true)
Expand Down Expand Up @@ -311,7 +319,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 @@ -479,6 +489,7 @@ func (c *cloud) GetDiskByName(ctx context.Context, name string, capacityBytes in
CapacityGiB: volSizeBytes,
AvailabilityZone: aws.StringValue(volume.AvailabilityZone),
SnapshotID: aws.StringValue(volume.SnapshotId),
OutpostArn: aws.StringValue(volume.OutpostArn),
}, nil
}

Expand All @@ -498,6 +509,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
62 changes: 62 additions & 0 deletions pkg/cloud/cloud_test.go
View file
Original file line number Diff line number Diff line change
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: "arn:aws:outposts:us-west-2:111111111111:outpost/op-0aaa000a0aaaa00a0",
},
expDisk: &Disk{
VolumeID: "vol-test",
CapacityGiB: 1,
AvailabilityZone: expZone,
OutpostArn: "arn:aws:outposts:us-west-2:111111111111:outpost/op-0aaa000a0aaaa00a0",
},
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 Expand Up @@ -380,6 +417,7 @@ func TestGetDiskByName(t *testing.T) {
volumeName string
volumeCapacity int64
availabilityZone string
outpostArn string
expErr error
}{
{
Expand All @@ -389,6 +427,14 @@ func TestGetDiskByName(t *testing.T) {
availabilityZone: expZone,
expErr: nil,
},
{
name: "success: outpost volume",
volumeName: "vol-test-1234",
volumeCapacity: util.GiBToBytes(1),
availabilityZone: expZone,
outpostArn: "arn:aws:outposts:us-west-2:111111111111:outpost/op-0aaa000a0aaaa00a0",
expErr: nil,
},
{
name: "fail: DescribeVolumes returned generic error",
volumeName: "vol-test-1234",
Expand All @@ -407,6 +453,7 @@ func TestGetDiskByName(t *testing.T) {
VolumeId: aws.String(tc.volumeName),
Size: aws.Int64(util.BytesToGiB(tc.volumeCapacity)),
AvailabilityZone: aws.String(tc.availabilityZone),
OutpostArn: aws.String(tc.outpostArn),
}

ctx := context.Background()
Expand All @@ -427,6 +474,9 @@ func TestGetDiskByName(t *testing.T) {
if tc.availabilityZone != disk.AvailabilityZone {
t.Fatalf("GetDiskByName() failed: expected availabilityZone %q, got %q", tc.availabilityZone, disk.AvailabilityZone)
}
if tc.outpostArn != disk.OutpostArn {
t.Fatalf("GetDiskByName() failed: expected outpostArn %q, got %q", tc.outpostArn, disk.OutpostArn)
}
}

mockCtrl.Finish()
Expand All @@ -439,6 +489,7 @@ func TestGetDiskByID(t *testing.T) {
name string
volumeID string
availabilityZone string
outpostArn string
expErr error
}{
{
Expand All @@ -447,6 +498,13 @@ func TestGetDiskByID(t *testing.T) {
availabilityZone: expZone,
expErr: nil,
},
{
name: "success: outpost volume",
volumeID: "vol-test-1234",
availabilityZone: expZone,
outpostArn: "arn:aws:outposts:us-west-2:111111111111:outpost/op-0aaa000a0aaaa00a0",
expErr: nil,
},
{
name: "fail: DescribeVolumes returned generic error",
volumeID: "vol-test-1234",
Expand All @@ -467,6 +525,7 @@ func TestGetDiskByID(t *testing.T) {
{
VolumeId: aws.String(tc.volumeID),
AvailabilityZone: aws.String(tc.availabilityZone),
OutpostArn: aws.String(tc.outpostArn),
},
},
},
Expand All @@ -488,6 +547,9 @@ func TestGetDiskByID(t *testing.T) {
if tc.availabilityZone != disk.AvailabilityZone {
t.Fatalf("GetDiskByName() failed: expected availabilityZone %q, got %q", tc.availabilityZone, disk.AvailabilityZone)
}
if disk.OutpostArn != tc.outpostArn {
t.Fatalf("GetDisk() failed: expected outpostArn %q, got %q", tc.outpostArn, disk.OutpostArn)
}
}

mockCtrl.Finish()
Expand Down
40 changes: 37 additions & 3 deletions pkg/cloud/metadata.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,14 +18,20 @@ package cloud

import (
"fmt"
"strings"

"github.com/aws/aws-sdk-go/aws"
"github.com/aws/aws-sdk-go/aws/arn"
"github.com/aws/aws-sdk-go/aws/ec2metadata"
"github.com/aws/aws-sdk-go/aws/session"

"k8s.io/klog"
)

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,23 +41,28 @@ type MetadataService interface {
GetInstanceType() string
GetRegion() string
GetAvailabilityZone() string
GetOutpostArn() arn.ARN
}

type Metadata struct {
InstanceID string
InstanceType string
Region string
AvailabilityZone string
OutpostArn arn.ARN
}

// OutpostArnEndpoint is the ec2 instance metadata endpoint to query to get the outpost arn
const OutpostArnEndpoint string = "outpost-arn"

var _ MetadataService = &Metadata{}

// GetInstanceID returns the instance identification.
func (m *Metadata) GetInstanceID() string {
return m.InstanceID
}

// GetInstanceID returns the instance type.
// GetInstanceType returns the instance type.
func (m *Metadata) GetInstanceType() string {
return m.InstanceType
}
Expand All @@ -66,6 +77,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() arn.ARN {
return m.OutpostArn
}

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

return &Metadata{
outpostArn, err := svc.GetMetadata(OutpostArnEndpoint)
// "outpust-arn" returns 404 for non-outpost instances. note that the request is made to a link-local address.
// it's guaranteed to be in the form `arn:<partition>:outposts:<region>:<account>:outpost/<outpost-id>`
// 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")
}

metadata := Metadata{
InstanceID: doc.InstanceID,
InstanceType: doc.InstanceType,
Region: doc.Region,
AvailabilityZone: doc.AvailabilityZone,
}, nil
}

outpostArn = strings.ReplaceAll(outpostArn, "outpost/", "")
parsedArn, err := arn.Parse(outpostArn)
if err != nil {
klog.Warningf("Failed to parse the outpost arn: %s", outpostArn)
} else {
metadata.OutpostArn = parsedArn
}

return &metadata, nil
}
Loading

0 comments on commit be125ff

Please sign in to comment.