Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Create volumes in outpost for outpost instances #561

Merged
merged 1 commit into from
Sep 30, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 13 additions & 1 deletion pkg/cloud/cloud.go
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 {
ayberk marked this conversation as resolved.
Show resolved Hide resolved
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),
ayberk marked this conversation as resolved.
Show resolved Hide resolved
}, nil
}

Expand Down
62 changes: 62 additions & 0 deletions pkg/cloud/cloud_test.go
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
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