Skip to content

Commit

Permalink
Refactor NewCloud by pass in region
Browse files Browse the repository at this point in the history
  • Loading branch information
Cheng Pan committed Nov 1, 2019
1 parent 94107a3 commit d4ee938
Show file tree
Hide file tree
Showing 12 changed files with 104 additions and 160 deletions.
66 changes: 33 additions & 33 deletions pkg/cloud/cloud.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ import (

"github.com/aws/aws-sdk-go/aws"
"github.com/aws/aws-sdk-go/aws/awserr"
"github.com/aws/aws-sdk-go/aws/ec2metadata"
"github.com/aws/aws-sdk-go/aws/request"
"github.com/aws/aws-sdk-go/aws/session"
"github.com/aws/aws-sdk-go/service/ec2"
Expand Down Expand Up @@ -180,10 +179,10 @@ type EC2 interface {
DescribeSnapshotsWithContext(ctx aws.Context, input *ec2.DescribeSnapshotsInput, opts ...request.Option) (*ec2.DescribeSnapshotsOutput, error)
ModifyVolumeWithContext(ctx aws.Context, input *ec2.ModifyVolumeInput, opts ...request.Option) (*ec2.ModifyVolumeOutput, error)
DescribeVolumesModificationsWithContext(ctx aws.Context, input *ec2.DescribeVolumesModificationsInput, opts ...request.Option) (*ec2.DescribeVolumesModificationsOutput, error)
DescribeAvailabilityZonesWithContext(ctx aws.Context, input *ec2.DescribeAvailabilityZonesInput, opts ...request.Option) (*ec2.DescribeAvailabilityZonesOutput, error)
}

type Cloud interface {
GetMetadata() MetadataService
CreateDisk(ctx context.Context, volumeName string, diskOptions *DiskOptions) (disk *Disk, err error)
DeleteDisk(ctx context.Context, volumeID string) (success bool, err error)
AttachDisk(ctx context.Context, volumeID string, nodeID string) (devicePath string, err error)
Expand All @@ -201,38 +200,22 @@ type Cloud interface {
}

type cloud struct {
metadata MetadataService
ec2 EC2
dm dm.DeviceManager
region string
ec2 EC2
dm dm.DeviceManager
}

var _ Cloud = &cloud{}

// NewCloud returns a new instance of AWS cloud
// It panics if session is invalid
func NewCloud() (Cloud, error) {
svc := newEC2MetadataSvc()

metadata, err := NewMetadataService(svc)
if err != nil {
return nil, fmt.Errorf("could not get metadata from AWS: %v", err)
}

return newEC2Cloud(metadata, svc)
}

func NewCloudWithMetadata(metadata MetadataService) (Cloud, error) {
return newEC2Cloud(metadata, newEC2MetadataSvc())
}

func newEC2MetadataSvc() *ec2metadata.EC2Metadata {
sess := session.Must(session.NewSession(&aws.Config{}))
return ec2metadata.New(sess)
func NewCloud(region string) (Cloud, error) {
return newEC2Cloud(region)
}

func newEC2Cloud(metadata MetadataService, svc *ec2metadata.EC2Metadata) (Cloud, error) {
func newEC2Cloud(region string) (Cloud, error) {
awsConfig := &aws.Config{
Region: aws.String(metadata.GetRegion()),
Region: aws.String(region),
CredentialsChainVerboseErrors: aws.Bool(true),
}

Expand All @@ -242,16 +225,12 @@ func newEC2Cloud(metadata MetadataService, svc *ec2metadata.EC2Metadata) (Cloud,
}

return &cloud{
metadata: metadata,
dm: dm.NewDeviceManager(),
ec2: ec2.New(session.Must(session.NewSession(awsConfig))),
region: region,
dm: dm.NewDeviceManager(),
ec2: ec2.New(session.Must(session.NewSession(awsConfig))),
}, nil
}

func (c *cloud) GetMetadata() MetadataService {
return c.metadata
}

func (c *cloud) CreateDisk(ctx context.Context, volumeName string, diskOptions *DiskOptions) (*Disk, error) {
var (
createType string
Expand Down Expand Up @@ -290,8 +269,12 @@ func (c *cloud) CreateDisk(ctx context.Context, volumeName string, diskOptions *

zone := diskOptions.AvailabilityZone
if zone == "" {
zone = c.metadata.GetAvailabilityZone()
klog.V(5).Infof("AZ is not provided. Using node AZ [%s]", zone)
var err error
zone, err = c.randomAvailabilityZone(ctx, c.region)
if err != nil {
return nil, fmt.Errorf("failed to get availability zone %s", err)
}
}

request := &ec2.CreateVolumeInput{
Expand Down Expand Up @@ -938,3 +921,20 @@ func (c *cloud) getLatestVolumeModification(ctx context.Context, volumeID string

return volumeMods[len(volumeMods)-1], nil
}

// randomAvailabilityZone returns a random zone from the given region
// the randomness relies on the response of DescribeAvailabilityZones
func (c *cloud) randomAvailabilityZone(ctx context.Context, region string) (string, error) {
request := &ec2.DescribeAvailabilityZonesInput{}
response, err := c.ec2.DescribeAvailabilityZonesWithContext(ctx, request)
if err != nil {
return "", err
}

zones := []string{}
for _, zone := range response.AvailabilityZones {
zones = append(zones, *zone.ZoneName)
}

return zones[0], nil
}
18 changes: 11 additions & 7 deletions pkg/cloud/cloud_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,14 @@ func TestCreateDisk(t *testing.T) {
mockEC2.EXPECT().DescribeSnapshotsWithContext(gomock.Eq(ctx), gomock.Any()).Return(&ec2.DescribeSnapshotsOutput{Snapshots: []*ec2.Snapshot{snapshot}}, nil).AnyTimes()
}

if len(tc.diskOptions.AvailabilityZone) == 0 {
mockEC2.EXPECT().DescribeAvailabilityZonesWithContext(gomock.Eq(ctx), gomock.Any()).Return(&ec2.DescribeAvailabilityZonesOutput{
AvailabilityZones: []*ec2.AvailabilityZone{
{ZoneName: aws.String(defaultZone)},
},
}, nil)
}

disk, err := c.CreateDisk(ctx, tc.volumeName, tc.diskOptions)
if err != nil {
if tc.expErr == nil {
Expand Down Expand Up @@ -1041,13 +1049,9 @@ func TestListSnapshots(t *testing.T) {

func newCloud(mockEC2 EC2) Cloud {
return &cloud{
metadata: &Metadata{
InstanceID: "test-instance",
Region: "test-region",
AvailabilityZone: defaultZone,
},
dm: dm.NewDeviceManager(),
ec2: mockEC2,
region: "test-region",
dm: dm.NewDeviceManager(),
ec2: mockEC2,
}
}

Expand Down
8 changes: 8 additions & 0 deletions pkg/cloud/metadata.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,9 @@ package cloud
import (
"fmt"

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

type EC2Metadata interface {
Expand Down Expand Up @@ -64,6 +66,12 @@ func (m *Metadata) GetAvailabilityZone() string {
return m.AvailabilityZone
}

func NewMetadata() (MetadataService, error) {
sess := session.Must(session.NewSession(&aws.Config{}))
svc := ec2metadata.New(sess)
return NewMetadataService(svc)
}

// NewMetadataService returns a new MetadataServiceImplementation.
func NewMetadataService(svc EC2Metadata) (MetadataService, error) {
if !svc.Available() {
Expand Down
42 changes: 30 additions & 12 deletions pkg/cloud/mocks/mock_ec2.go

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

7 changes: 6 additions & 1 deletion pkg/driver/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,12 @@ type controllerService struct {
// newControllerService creates a new controller service
// it panics if failed to create the service
func newControllerService(driverOptions *DriverOptions) controllerService {
cloud, err := cloud.NewCloud()
metadata, err := cloud.NewMetadata()
if err != nil {
panic(err)
}
region := metadata.GetRegion()
cloud, err := cloud.NewCloud(region)
if err != nil {
panic(err)
}
Expand Down
6 changes: 5 additions & 1 deletion pkg/driver/fakes.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,11 @@ func NewFakeDriver(endpoint string, fakeCloud cloud.Cloud, fakeMounter *mount.Fa
driverOptions: driverOptions,
},
nodeService: nodeService{
metadata: fakeCloud.GetMetadata(),
metadata: &cloud.Metadata{
InstanceID: "instanceID",
Region: "region",
AvailabilityZone: "az",
},
mounter: &NodeMounter{mount.SafeFormatAndMount{Interface: fakeMounter, Exec: mount.NewFakeExec(nil)}},
inFlight: internal.NewInFlight(),
},
Expand Down
12 changes: 0 additions & 12 deletions pkg/driver/mocks/mock_cloud.go

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

Loading

0 comments on commit d4ee938

Please sign in to comment.