Skip to content

Commit

Permalink
Add unit test
Browse files Browse the repository at this point in the history
  • Loading branch information
Cheng Pan committed Oct 24, 2018
1 parent c274b03 commit 7416d2f
Show file tree
Hide file tree
Showing 2 changed files with 69 additions and 4 deletions.
68 changes: 64 additions & 4 deletions pkg/cloud/cloud.go
Original file line number Diff line number Diff line change
Expand Up @@ -244,16 +244,23 @@ func (c *cloud) CreateDisk(ctx context.Context, volumeName string, diskOptions *
return nil, fmt.Errorf("disk size was not returned by CreateVolume")
}

if len(diskOptions.KmsKeyID) > 0 {
err := c.waitForCreate(ctx, volumeID)
if err != nil {
if isAWSErrorVolumeNotFound(err) {
return nil, fmt.Errorf("failed to create encrypted volume: the volume disappeared after creation, most likely due to inaccessible KMS encryption key")
}
}
}

return &Disk{CapacityGiB: size, VolumeID: volumeID, AvailabilityZone: zone}, nil
}

func (c *cloud) DeleteDisk(ctx context.Context, volumeID string) (bool, error) {
request := &ec2.DeleteVolumeInput{VolumeId: &volumeID}
if _, err := c.ec2.DeleteVolumeWithContext(ctx, request); err != nil {
if awsErr, ok := err.(awserr.Error); ok {
if awsErr.Code() == "InvalidVolume.NotFound" {
return false, ErrNotFound
}
if isAWSErrorVolumeNotFound(err) {
return false, ErrNotFound
}
return false, fmt.Errorf("DeleteDisk could not delete volume: %v", err)
}
Expand Down Expand Up @@ -496,3 +503,56 @@ func (c *cloud) waitForAttachmentState(ctx context.Context, volumeID, state stri

return wait.ExponentialBackoff(backoff, verifyVolumeFunc)
}

// waitForCreate waits for volume to be created for encrypted volume only
// it polls for created volume to check it has not been silently removed by AWS.
// On a random AWS account (shared among several developers) it took 4s on average.
func (c *cloud) waitForCreate(ctx context.Context, volumeID string) error {
var (
checkInterval = 1 * time.Second
checkTimeout = 30 * time.Second
)

request := &ec2.DescribeVolumesInput{
VolumeIds: []*string{
aws.String(volumeID),
},
}

err := wait.Poll(checkInterval, checkTimeout, func() (done bool, err error) {
vol, err := c.getVolume(ctx, request)
if err != nil {
return true, err
}
if vol.State != nil {
switch *vol.State {
case "available":
// The volume is Available, it won't be deleted now.
return true, nil
case "creating":
return false, nil
default:
return true, fmt.Errorf("unexpected State of newly created AWS EBS volume %s: %q", volumeID, *vol.State)
}
}
return false, nil
})

return err
}

// Helper function for describeVolume callers. Tries to retype given error to AWS error
// and returns true in case the AWS error is "InvalidVolume.NotFound", false otherwise
func isAWSErrorVolumeNotFound(err error) bool {
if err == nil {
return false
}
if awsError, ok := err.(awserr.Error); ok {
// https://docs.aws.amazon.com/AWSEC2/latest/APIReference/errors-overview.html
if awsError.Code() == "InvalidVolume.NotFound" {
return true
}
}

return false
}
5 changes: 5 additions & 0 deletions pkg/cloud/cloud_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -106,12 +106,17 @@ func TestCreateDisk(t *testing.T) {
vol = &ec2.Volume{
VolumeId: aws.String(tc.diskOptions.Tags[VolumeNameTagKey]),
Size: aws.Int64(util.BytesToGiB(tc.diskOptions.CapacityBytes)),
State: aws.String("available"),
}
}

ctx := context.Background()
mockEC2.EXPECT().CreateVolumeWithContext(gomock.Eq(ctx), gomock.Any()).Return(vol, tc.expErr)

if tc.diskOptions.Encrypted {
mockEC2.EXPECT().DescribeVolumesWithContext(gomock.Eq(ctx), gomock.Any()).Return(&ec2.DescribeVolumesOutput{Volumes: []*ec2.Volume{vol}}, nil)
}

disk, err := c.CreateDisk(ctx, tc.volumeName, tc.diskOptions)
if err != nil {
if tc.expErr == nil {
Expand Down

0 comments on commit 7416d2f

Please sign in to comment.