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.

Sign up for GitHub

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

Implementing ListSnapshots #286

Merged
merged 3 commits into from
May 11, 2019
Merged

Implementing ListSnapshots #286

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
44bf7e1
Implementing ListSnapshots
zacharya Apr 25, 2019
fdca564
Sanity tests passing
zacharya May 8, 2019
6c0eb0d
Adding mock to TestListSnapshots
zacharya May 8, 2019
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
79 changes: 78 additions & 1 deletion pkg/cloud/cloud.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,10 @@ var (

// ErrAlreadyExists is returned when a resource is already existent.
ErrAlreadyExists = errors.New("Resource already exists")

// ErrMultiSnapshots is returned when multiple snapshots are found
// with the same ID
ErrMultiSnapshots = errors.New("Multiple snapshots with the same name found")
)

// Disk represents a EBS volume
Expand Down Expand Up @@ -124,11 +128,23 @@ type Snapshot struct {
ReadyToUse bool
}

// ListSnapshotsResponse is the container for our snapshots along with a pagination token to pass back to the caller
type ListSnapshotsResponse struct {
Snapshots []*Snapshot
NextToken string
}

// SnapshotOptions represents parameters to create an EBS volume
type SnapshotOptions struct {
Tags map[string]string
}

// ec2ListSnapshotsResponse is a helper struct returned from the AWS API calling function to the main ListSnapshots function
type ec2ListSnapshotsResponse struct {
Snapshots []*ec2.Snapshot
NextToken string
zacharya marked this conversation as resolved.
Show resolved Hide resolved
}

// EC2 abstracts aws.EC2 to facilitate its mocking.
// See https://docs.aws.amazon.com/sdk-for-go/api/service/ec2/ for details
type EC2 interface {
Expand Down Expand Up @@ -156,6 +172,7 @@ type Cloud interface {
CreateSnapshot(ctx context.Context, volumeID string, snapshotOptions *SnapshotOptions) (snapshot *Snapshot, err error)
DeleteSnapshot(ctx context.Context, snapshotID string) (success bool, err error)
GetSnapshotByName(ctx context.Context, name string) (snapshot *Snapshot, err error)
ListSnapshots(ctx context.Context, volumeID string, maxResults int64, nextToken string) (listSnapshotsResponse *ListSnapshotsResponse, err error)
}

type cloud struct {
Expand Down Expand Up @@ -542,6 +559,44 @@ func (c *cloud) GetSnapshotByName(ctx context.Context, name string) (snapshot *S
return c.ec2SnapshotResponseToStruct(ec2snapshot), nil
}

func (c *cloud) ListSnapshots(ctx context.Context, volumeID string, maxResults int64, nextToken string) (listSnapshotsResponse *ListSnapshotsResponse, err error) {
zacharya marked this conversation as resolved.
Show resolved Hide resolved
describeSnapshotsInput := &ec2.DescribeSnapshotsInput{}

if maxResults >= 5 {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since controller.ListSnapshots already has logic to guarantee maxResults >= 5, as cloud is an internal struct, I feel we can just set describeSnapshotsInput.MaxResults = aws.Int64(maxResults) without checking if maxResults >= 5 {.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just pushed another commit that enabled ListSnapshot sanity testing. In order to satisfy the unit tests from the sanity testing packages, I had to push the logic down into the cloud package so controller was able to handle 0 < MaxEntries < 5. Let me know how you want to proceed with this. It's mostly cosmetic as we're just using fakeCloudProvider anyways.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If that's what the sanity test checks, im okay with the change.

describeSnapshotsInput.MaxResults = aws.Int64(maxResults)
}

if len(nextToken) != 0 {
describeSnapshotsInput.NextToken = aws.String(nextToken)
}
if len(volumeID) != 0 {
describeSnapshotsInput.Filters = []*ec2.Filter{
{
Name: aws.String("volume-id"),
Values: []*string{aws.String(volumeID)},
},
}
}

ec2SnapshotsResponse, err := c.listSnapshots(ctx, describeSnapshotsInput)
if err != nil {
return nil, err
}
var snapshots []*Snapshot
for _, ec2Snapshot := range ec2SnapshotsResponse.Snapshots {
snapshots = append(snapshots, c.ec2SnapshotResponseToStruct(ec2Snapshot))
}

if len(snapshots) == 0 {
return nil, ErrNotFound
}

return &ListSnapshotsResponse{
Snapshots: snapshots,
NextToken: ec2SnapshotsResponse.NextToken,
}, nil
}

// Helper method converting EC2 snapshot type to the internal struct
func (c *cloud) ec2SnapshotResponseToStruct(ec2Snapshot *ec2.Snapshot) *Snapshot {
if ec2Snapshot == nil {
Expand Down Expand Up @@ -640,14 +695,36 @@ func (c *cloud) getSnapshot(ctx context.Context, request *ec2.DescribeSnapshotsI
}

if l := len(snapshots); l > 1 {
return nil, errors.New("Multiple snapshots with the same name found")
return nil, ErrMultiSnapshots
} else if l < 1 {
return nil, ErrNotFound
}

return snapshots[0], nil
}

// listSnapshots returns all snapshots based from a request
func (c *cloud) listSnapshots(ctx context.Context, request *ec2.DescribeSnapshotsInput) (*ec2ListSnapshotsResponse, error) {
var snapshots []*ec2.Snapshot
var nextToken string

response, err := c.ec2.DescribeSnapshotsWithContext(ctx, request)
if err != nil {
return nil, err
}

snapshots = append(snapshots, response.Snapshots...)

if response.NextToken != nil {
nextToken = *response.NextToken
}

return &ec2ListSnapshotsResponse{
Snapshots: snapshots,
NextToken: nextToken,
}, nil
}

// waitForVolume waits for volume to be in the "available" state.
// On a random AWS account (shared among several developers) it took 4s on average.
func (c *cloud) waitForVolume(ctx context.Context, volumeID string) error {
Expand Down
160 changes: 160 additions & 0 deletions pkg/cloud/cloud_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package cloud

import (
"context"
"errors"
"fmt"
"strings"
"testing"
Expand Down Expand Up @@ -647,6 +648,165 @@ func TestGetSnapshotByName(t *testing.T) {
}
}

func TestListSnapshots(t *testing.T) {
testCases := []struct {
name string
testFunc func(t *testing.T)
}{
{
name: "success: normal",
testFunc: func(t *testing.T) {
expSnapshots := []*Snapshot{
{
SourceVolumeID: "snap-test-volume1",
SnapshotID: "snap-test-name1",
},
{
SourceVolumeID: "snap-test-volume2",
SnapshotID: "snap-test-name2",
},
}
ec2Snapshots := []*ec2.Snapshot{
{
SnapshotId: aws.String(expSnapshots[0].SnapshotID),
VolumeId: aws.String("snap-test-volume1"),
State: aws.String("completed"),
},
{
SnapshotId: aws.String(expSnapshots[1].SnapshotID),
VolumeId: aws.String("snap-test-volume2"),
State: aws.String("completed"),
},
}

mockCtl := gomock.NewController(t)
defer mockCtl.Finish()
mockEC2 := mocks.NewMockEC2(mockCtl)
c := newCloud(mockEC2)

ctx := context.Background()

mockEC2.EXPECT().DescribeSnapshotsWithContext(gomock.Eq(ctx), gomock.Any()).Return(&ec2.DescribeSnapshotsOutput{Snapshots: ec2Snapshots}, nil)

_, err := c.ListSnapshots(ctx, "", 0, "")
if err != nil {
t.Fatalf("ListSnapshots() failed: expected no error, got: %v", err)
}
},
},
{
name: "success: max results, next token",
testFunc: func(t *testing.T) {
maxResults := 5
nextTokenValue := "nextTokenValue"
var expSnapshots []*Snapshot
for i := 0; i < maxResults*2; i++ {
expSnapshots = append(expSnapshots, &Snapshot{
SourceVolumeID: "snap-test-volume1",
SnapshotID: fmt.Sprintf("snap-test-name%d", i),
})
}

var ec2Snapshots []*ec2.Snapshot
for i := 0; i < maxResults*2; i++ {
ec2Snapshots = append(ec2Snapshots, &ec2.Snapshot{
SnapshotId: aws.String(expSnapshots[i].SnapshotID),
VolumeId: aws.String(fmt.Sprintf("snap-test-volume%d", i)),
State: aws.String("completed"),
})
}

mockCtl := gomock.NewController(t)
defer mockCtl.Finish()
mockEC2 := mocks.NewMockEC2(mockCtl)
c := newCloud(mockEC2)

ctx := context.Background()

firstCall := mockEC2.EXPECT().DescribeSnapshotsWithContext(gomock.Eq(ctx), gomock.Any()).Return(&ec2.DescribeSnapshotsOutput{
Snapshots: ec2Snapshots[:maxResults],
NextToken: aws.String(nextTokenValue),
}, nil)
secondCall := mockEC2.EXPECT().DescribeSnapshotsWithContext(gomock.Eq(ctx), gomock.Any()).Return(&ec2.DescribeSnapshotsOutput{
Snapshots: ec2Snapshots[maxResults:],
}, nil)
gomock.InOrder(
firstCall,
secondCall,
)

firstSnapshotsResponse, err := c.ListSnapshots(ctx, "", 5, "")
if err != nil {
t.Fatalf("ListSnapshots() failed: expected no error, got: %v", err)
}

if len(firstSnapshotsResponse.Snapshots) != maxResults {
t.Fatalf("Expected %d snapshots, got %d", maxResults, len(firstSnapshotsResponse.Snapshots))
}

if firstSnapshotsResponse.NextToken != nextTokenValue {
t.Fatalf("Expected next token value '%s' got '%s'", nextTokenValue, firstSnapshotsResponse.NextToken)
}

secondSnapshotsResponse, err := c.ListSnapshots(ctx, "", 0, firstSnapshotsResponse.NextToken)
if err != nil {
t.Fatalf("CreateSnapshot() failed: expected no error, got: %v", err)
}

if len(secondSnapshotsResponse.Snapshots) != maxResults {
t.Fatalf("Expected %d snapshots, got %d", maxResults, len(secondSnapshotsResponse.Snapshots))
}

if secondSnapshotsResponse.NextToken != "" {
t.Fatalf("Expected next token value to be empty got %s", secondSnapshotsResponse.NextToken)
}
},
},
{
name: "fail: AWS DescribeSnapshotsWithContext error",
testFunc: func(t *testing.T) {
mockCtl := gomock.NewController(t)
defer mockCtl.Finish()
mockEC2 := mocks.NewMockEC2(mockCtl)
c := newCloud(mockEC2)

ctx := context.Background()

mockEC2.EXPECT().DescribeSnapshotsWithContext(gomock.Eq(ctx), gomock.Any()).Return(nil, errors.New("test error"))

if _, err := c.ListSnapshots(ctx, "", 0, ""); err == nil {
t.Fatalf("ListSnapshots() failed: expected an error, got none")
}
},
},
{
name: "fail: no snapshots ErrNotFound",
testFunc: func(t *testing.T) {
mockCtl := gomock.NewController(t)
defer mockCtl.Finish()
mockEC2 := mocks.NewMockEC2(mockCtl)
c := newCloud(mockEC2)

ctx := context.Background()

mockEC2.EXPECT().DescribeSnapshotsWithContext(gomock.Eq(ctx), gomock.Any()).Return(&ec2.DescribeSnapshotsOutput{}, nil)

if _, err := c.ListSnapshots(ctx, "", 0, ""); err != nil {
if err != ErrNotFound {
t.Fatalf("Expected error %v, got %v", ErrNotFound, err)
}
} else {
t.Fatalf("Expected error, got none")
}
},
},
zacharya marked this conversation as resolved.
Show resolved Hide resolved
}

for _, tc := range testCases {
t.Run(tc.name, tc.testFunc)
}
}

func newCloud(mockEC2 EC2) Cloud {
return &cloud{
metadata: &Metadata{
Expand Down
78 changes: 77 additions & 1 deletion pkg/driver/controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -369,7 +369,51 @@ func (d *controllerService) DeleteSnapshot(ctx context.Context, req *csi.DeleteS
}

func (d *controllerService) ListSnapshots(ctx context.Context, req *csi.ListSnapshotsRequest) (*csi.ListSnapshotsResponse, error) {
return nil, status.Error(codes.Unimplemented, "")
klog.V(4).Infof("ListSnapshots: called with args %+v", req)
var snapshots []*cloud.Snapshot

snapshotID := req.GetSnapshotId()
if len(snapshotID) != 0 {
snapshot, err := d.cloud.GetSnapshotByName(ctx, snapshotID)
if err != nil {
if err == cloud.ErrNotFound {
klog.V(4).Info("ListSnapshots: snapshot not found, returning with success")
return &csi.ListSnapshotsResponse{}, nil
}
return nil, status.Errorf(codes.Internal, "Could not get snapshot ID %q: %v", snapshotID, err)
}
snapshots = append(snapshots, snapshot)
if response, err := newListSnapshotsResponse(&cloud.ListSnapshotsResponse{
Snapshots: snapshots,
}); err != nil {
return nil, status.Errorf(codes.Internal, "Could not build ListSnapshotsResponse: %v", err)
} else {
return response, nil
}
}

volumeID := req.GetSourceVolumeId()
nextToken := req.GetStartingToken()
maxEntries := int64(req.GetMaxEntries())

if maxEntries > 0 && maxEntries < 5 {
return nil, status.Errorf(codes.InvalidArgument, "MaxEntries must be greater than or equal to 5")
}

cloudSnapshots, err := d.cloud.ListSnapshots(ctx, volumeID, maxEntries, nextToken)
if err != nil {
if err == cloud.ErrNotFound {
klog.V(4).Info("ListSnapshots: snapshot not found, returning with success")
return &csi.ListSnapshotsResponse{}, nil
}
return nil, status.Errorf(codes.Internal, "Could not list snapshots: %v", err)
}

response, err := newListSnapshotsResponse(cloudSnapshots)
if err != nil {
return nil, status.Errorf(codes.Internal, "Could not build ListSnapshotsResponse: %v", err)
}
return response, nil
}

func (d *Driver) ControllerExpandVolume(ctx context.Context, req *csi.ControllerExpandVolumeRequest) (*csi.ControllerExpandVolumeResponse, error) {
Expand Down Expand Up @@ -430,6 +474,38 @@ func newCreateSnapshotResponse(snapshot *cloud.Snapshot) (*csi.CreateSnapshotRes
}, nil
}

func newListSnapshotsResponse(cloudResponse *cloud.ListSnapshotsResponse) (*csi.ListSnapshotsResponse, error) {

var entries []*csi.ListSnapshotsResponse_Entry
for _, snapshot := range cloudResponse.Snapshots {
snapshotResponseEntry, err := newListSnapshotsResponseEntry(snapshot)
if err != nil {
return nil, err
}
entries = append(entries, snapshotResponseEntry)
}
return &csi.ListSnapshotsResponse{
Entries: entries,
NextToken: cloudResponse.NextToken,
}, nil
}

func newListSnapshotsResponseEntry(snapshot *cloud.Snapshot) (*csi.ListSnapshotsResponse_Entry, error) {
ts, err := ptypes.TimestampProto(snapshot.CreationTime)
if err != nil {
return nil, err
}
return &csi.ListSnapshotsResponse_Entry{
Snapshot: &csi.Snapshot{
SnapshotId: snapshot.SnapshotID,
SourceVolumeId: snapshot.SourceVolumeID,
SizeBytes: snapshot.Size,
CreationTime: ts,
ReadyToUse: snapshot.ReadyToUse,
},
}, nil
}

func getVolSizeBytes(req *csi.CreateVolumeRequest) (int64, error) {
var volSizeBytes int64
capRange := req.GetCapacityRange()
Expand Down
Loading