Skip to content

Commit

Permalink
Merge pull request #561 from ayberk/outpost_fix
Browse files Browse the repository at this point in the history 
Create volumes in outpost for outpost instances
  • Loading branch information
@k8s-ci-robot
k8s-ci-robot committed Sep 30, 2020
2 parents f4ecbc6 + be125ff commit 406b203
Show file tree
Hide file tree
Showing 14 changed files with 657 additions and 13 deletions.
14 changes: 13 additions & 1 deletion pkg/cloud/cloud.go
View file
Expand Up @@ -129,6 +129,7 @@ type Disk struct {
CapacityGiB int64
AvailabilityZone string
SnapshotID string
OutpostArn string
}

// DiskOptions represents parameters to create an EBS volume
Expand All @@ -138,6 +139,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 @@ -284,6 +286,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 @@ -318,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 @@ -486,6 +496,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 @@ -505,6 +516,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
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
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
}

0 comments on commit 406b203

Please sign in to comment.