From bbb60b5e6aa0afebce14c223c21de4606d539475 Mon Sep 17 00:00:00 2001 From: Mayank Sachan <34068620+mssachan@users.noreply.github.com> Date: Thu, 23 Apr 2020 12:05:00 +0530 Subject: [PATCH] Add support for Listing VPC Volumes (#202) * Add support for Listing VPC Volumes * Fix formatting issues * Fix typos * Extract next volID only if response is in expected format * Fix UT * Fix UT * Fix review comments * Add support for filtering volumes based on resource group ID * Update list_volumes.go * Update list_volumes.go Co-authored-by: masachan Co-authored-by: Arashad Ahamad <42564459+arahamad@users.noreply.github.com> --- lib/provider/data_types.go | 6 + lib/provider/fake/fake_session.go | 38 +-- lib/provider/volume_manager.go | 5 +- samples/main.go | 45 ++- .../softlayer/block/block_volume_manager.go | 2 +- .../softlayer/file/file_volume_manager.go | 2 +- volume-providers/vpc/messages/messages_en.go | 7 + volume-providers/vpc/provider/list_volumes.go | 58 +++- .../vpc/provider/list_volumes_test.go | 266 +++++++++++++++++- .../vpc/vpcclient/models/volume.go | 21 +- .../vpc/vpcclient/vpcvolume/fakes/volume.go | 26 +- .../vpc/vpcclient/vpcvolume/get_volume.go | 2 +- .../vpc/vpcclient/vpcvolume/list_volumes.go | 6 +- .../vpcclient/vpcvolume/list_volumes_test.go | 16 +- .../vpc/vpcclient/vpcvolume/volume_service.go | 2 +- .../vpcvolumefakes/volume_service.go | 26 +- 16 files changed, 450 insertions(+), 78 deletions(-) diff --git a/lib/provider/data_types.go b/lib/provider/data_types.go index 6570335a..ce7c1b39 100644 --- a/lib/provider/data_types.go +++ b/lib/provider/data_types.go @@ -123,3 +123,9 @@ type VolumeAuthorization struct { // List of HostIPs to authorize HostIPs []string `json:"hostIPs,omitempty"` } + +// VolumeList ... +type VolumeList struct { + Next string `json:"next,omitempty"` + Volumes []*Volume `json:"volumes"` +} diff --git a/lib/provider/fake/fake_session.go b/lib/provider/fake/fake_session.go index e79a1602..70a4ffa9 100644 --- a/lib/provider/fake/fake_session.go +++ b/lib/provider/fake/fake_session.go @@ -227,17 +227,19 @@ type FakeSession struct { result1 []*provider.Snapshot result2 error } - ListVolumesStub func(map[string]string) ([]*provider.Volume, error) + ListVolumesStub func(int, string, map[string]string) (*provider.VolumeList, error) listVolumesMutex sync.RWMutex listVolumesArgsForCall []struct { - arg1 map[string]string + arg1 int + arg2 string + arg3 map[string]string } listVolumesReturns struct { - result1 []*provider.Volume + result1 *provider.VolumeList result2 error } listVolumesReturnsOnCall map[int]struct { - result1 []*provider.Volume + result1 *provider.VolumeList result2 error } OrderSnapshotStub func(provider.Volume) error @@ -1379,16 +1381,18 @@ func (fake *FakeSession) ListSnapshotsReturnsOnCall(i int, result1 []*provider.S }{result1, result2} } -func (fake *FakeSession) ListVolumes(arg1 map[string]string) ([]*provider.Volume, error) { +func (fake *FakeSession) ListVolumes(arg1 int, arg2 string, arg3 map[string]string) (*provider.VolumeList, error) { fake.listVolumesMutex.Lock() ret, specificReturn := fake.listVolumesReturnsOnCall[len(fake.listVolumesArgsForCall)] fake.listVolumesArgsForCall = append(fake.listVolumesArgsForCall, struct { - arg1 map[string]string - }{arg1}) - fake.recordInvocation("ListVolumes", []interface{}{arg1}) + arg1 int + arg2 string + arg3 map[string]string + }{arg1, arg2, arg3}) + fake.recordInvocation("ListVolumes", []interface{}{arg1, arg2, arg3}) fake.listVolumesMutex.Unlock() if fake.ListVolumesStub != nil { - return fake.ListVolumesStub(arg1) + return fake.ListVolumesStub(arg1, arg2, arg3) } if specificReturn { return ret.result1, ret.result2 @@ -1403,41 +1407,41 @@ func (fake *FakeSession) ListVolumesCallCount() int { return len(fake.listVolumesArgsForCall) } -func (fake *FakeSession) ListVolumesCalls(stub func(map[string]string) ([]*provider.Volume, error)) { +func (fake *FakeSession) ListVolumesCalls(stub func(int, string, map[string]string) (*provider.VolumeList, error)) { fake.listVolumesMutex.Lock() defer fake.listVolumesMutex.Unlock() fake.ListVolumesStub = stub } -func (fake *FakeSession) ListVolumesArgsForCall(i int) map[string]string { +func (fake *FakeSession) ListVolumesArgsForCall(i int) (int, string, map[string]string) { fake.listVolumesMutex.RLock() defer fake.listVolumesMutex.RUnlock() argsForCall := fake.listVolumesArgsForCall[i] - return argsForCall.arg1 + return argsForCall.arg1, argsForCall.arg2, argsForCall.arg3 } -func (fake *FakeSession) ListVolumesReturns(result1 []*provider.Volume, result2 error) { +func (fake *FakeSession) ListVolumesReturns(result1 *provider.VolumeList, result2 error) { fake.listVolumesMutex.Lock() defer fake.listVolumesMutex.Unlock() fake.ListVolumesStub = nil fake.listVolumesReturns = struct { - result1 []*provider.Volume + result1 *provider.VolumeList result2 error }{result1, result2} } -func (fake *FakeSession) ListVolumesReturnsOnCall(i int, result1 []*provider.Volume, result2 error) { +func (fake *FakeSession) ListVolumesReturnsOnCall(i int, result1 *provider.VolumeList, result2 error) { fake.listVolumesMutex.Lock() defer fake.listVolumesMutex.Unlock() fake.ListVolumesStub = nil if fake.listVolumesReturnsOnCall == nil { fake.listVolumesReturnsOnCall = make(map[int]struct { - result1 []*provider.Volume + result1 *provider.VolumeList result2 error }) } fake.listVolumesReturnsOnCall[i] = struct { - result1 []*provider.Volume + result1 *provider.VolumeList result2 error }{result1, result2} } diff --git a/lib/provider/volume_manager.go b/lib/provider/volume_manager.go index 0d164d12..5d65fd3c 100644 --- a/lib/provider/volume_manager.go +++ b/lib/provider/volume_manager.go @@ -38,9 +38,8 @@ type VolumeManager interface { // details by usig user provided volume name GetVolumeByName(name string) (*Volume, error) - // Others - // Get volume lists by using snapshot tags - ListVolumes(tags map[string]string) ([]*Volume, error) + // Get volume lists by using filters + ListVolumes(limit int, start string, tags map[string]string) (*VolumeList, error) // GetVolumeByRequestID fetch the volume by request ID. // Request Id is the one that is returned when volume is provsioning request is diff --git a/samples/main.go b/samples/main.go index 81a8c75f..9cf06e61 100644 --- a/samples/main.go +++ b/samples/main.go @@ -103,7 +103,7 @@ func main() { valid := true for valid { - fmt.Println("\n\nSelect your choice\n 1- Get volume details \n 2- Create snapshot \n 3- list snapshot \n 4- Create volume \n 5- Snapshot details \n 6- Snapshot Order \n 7- Create volume from snapshot\n 8- Delete volume \n 9- Delete Snapshot \n 10- List all Snapshot \n 12- Authorize volume \n 13- Create VPC Volume \n 14- Create VPC Snapshot \n 15- Attach VPC volume \n 16- Detach VPC volume \n 17- Get volume by name \n Your choice?:") + fmt.Println("\n\nSelect your choice\n 1- Get volume details \n 2- Create snapshot \n 3- list snapshot \n 4- Create volume \n 5- Snapshot details \n 6- Snapshot Order \n 7- Create volume from snapshot\n 8- Delete volume \n 9- Delete Snapshot \n 10- List all Snapshot \n 12- Authorize volume \n 13- Create VPC Volume \n 14- Create VPC Snapshot \n 15- Attach VPC volume \n 16- Detach VPC volume \n 17- Get volume by name \n 18- List volumes \n Your choice?:") var choiceN int var volumeID string @@ -449,6 +449,49 @@ func main() { } fmt.Printf("\n\n") } else if choiceN == 18 { + fmt.Println("You selected list volumes") + tags := map[string]string{} + volName := "" + zoneName := "" + resourceGroupID := "" + fmt.Printf("Please enter ZONE Name to filter volumes(Optional): ") + _, er11 = fmt.Scanf("%s", &zoneName) + if zoneName != "" { + tags["zone.name"] = zoneName + } + fmt.Printf("Please enter volume Name to filter volumes(Optional): ") + _, er11 = fmt.Scanf("%s", &volName) + if volName != "" { + tags["name"] = volName + } + + fmt.Printf("\nPlease enter resource group ID to filter volumes(Optional): ") + _, er11 = fmt.Scanf("%s", &resourceGroupID) + if resourceGroupID != "" { + tags["resource_group.id"] = resourceGroupID + } + + start := "" + var limit int + fmt.Printf("Please enter max number of volume entries per page to be returned(Optional): ") + _, er11 = fmt.Scanf("%d", &limit) + for true { + volumeobj1, er11 := sess.ListVolumes(limit, start, tags) + if er11 == nil { + ctxLogger.Info("Successfully got volumes list================>", zap.Reflect("VolumesList", *volumeobj1)) + if volumeobj1.Next != "" { + fmt.Printf("\n\nFetching next set of volumes starting from %v...\n\n", volumeobj1.Next) + start = volumeobj1.Next + continue + } + } else { + er11 = updateRequestID(er11, requestID) + ctxLogger.Info("failed to list volumes================>", zap.Reflect("Error", er11)) + } + break + } + fmt.Printf("\n\n") + } else if choiceN == 19 { volumeManager.UpdateVolume() os.Exit(0) } else { diff --git a/volume-providers/softlayer/block/block_volume_manager.go b/volume-providers/softlayer/block/block_volume_manager.go index 23f2cb7a..bbee09ba 100644 --- a/volume-providers/softlayer/block/block_volume_manager.go +++ b/volume-providers/softlayer/block/block_volume_manager.go @@ -368,7 +368,7 @@ func (sls *SLBlockSession) GetVolume(id string) (*provider.Volume, error) { } // Get volume lists by using snapshot tags -func (sls *SLBlockSession) ListVolumes(tags map[string]string) ([]*provider.Volume, error) { +func (sls *SLBlockSession) ListVolumes(limit int, start string, tags map[string]string) (*provider.VolumeList, error) { //! TODO: we may implement return nil, nil } diff --git a/volume-providers/softlayer/file/file_volume_manager.go b/volume-providers/softlayer/file/file_volume_manager.go index 2e88ba73..8422029e 100644 --- a/volume-providers/softlayer/file/file_volume_manager.go +++ b/volume-providers/softlayer/file/file_volume_manager.go @@ -373,7 +373,7 @@ func (sls *SLFileSession) GetVolume(id string) (*provider.Volume, error) { } // Get volume lists by using snapshot tags -func (sls *SLFileSession) ListVolumes(tags map[string]string) ([]*provider.Volume, error) { +func (sls *SLFileSession) ListVolumes(limit int, start string, tags map[string]string) (*provider.VolumeList, error) { //! TODO: we may implement return nil, nil } diff --git a/volume-providers/vpc/messages/messages_en.go b/volume-providers/vpc/messages/messages_en.go index 76548128..961dd150 100644 --- a/volume-providers/vpc/messages/messages_en.go +++ b/volume-providers/vpc/messages/messages_en.go @@ -184,6 +184,13 @@ var messagesEn = map[string]util.Message{ RC: 500, Action: "Wait for volume deletion", }, + "ListVolumesFailed": util.Message{ + Code: "ListVolumesFailed", + Description: "Unable to fetch list of volumes.", + Type: util.RetrivalFailed, + RC: 404, + Action: "Run 'ibmcloud is volumes' to list available volumes in your account.", + }, } // InitMessages ... diff --git a/volume-providers/vpc/provider/list_volumes.go b/volume-providers/vpc/provider/list_volumes.go index e900742b..3df64506 100644 --- a/volume-providers/vpc/provider/list_volumes.go +++ b/volume-providers/vpc/provider/list_volumes.go @@ -11,15 +11,63 @@ package provider import ( + "github.com/IBM/ibmcloud-storage-volume-lib/lib/metrics" "github.com/IBM/ibmcloud-storage-volume-lib/lib/provider" + userError "github.com/IBM/ibmcloud-storage-volume-lib/volume-providers/vpc/messages" + "github.com/IBM/ibmcloud-storage-volume-lib/volume-providers/vpc/vpcclient/models" "go.uber.org/zap" + "strings" + "time" ) // ListVolumes list all volumes -func (vpcs *VPCSession) ListVolumes(tags map[string]string) ([]*provider.Volume, error) { - vpcs.Logger.Info("Entry ListVolumes", zap.Reflect("Tags", tags)) - defer vpcs.Logger.Info("Exit ListVolumes", zap.Reflect("Tags", tags)) +func (vpcs *VPCSession) ListVolumes(limit int, start string, tags map[string]string) (*provider.VolumeList, error) { + vpcs.Logger.Info("Entry ListVolumes", zap.Reflect("start", start), zap.Reflect("filters", tags)) + defer vpcs.Logger.Info("Exit ListVolumes", zap.Reflect("start", start), zap.Reflect("filters", tags)) + defer metrics.UpdateDurationFromStart(vpcs.Logger, "ListVolumes", time.Now()) - //! TODO: we may implement - return nil, nil + filters := &models.ListVolumeFilters{ + // Tag: tags["tag"], + ResourceGroupID: tags["resource_group.id"], + ZoneName: tags["zone.name"], + VolumeName: tags["name"], + } + + vpcs.Logger.Info("Getting volumes list from VPC provider...", zap.Reflect("start", start), zap.Reflect("filters", filters)) + + var volumes *models.VolumeList + var err error + err = retry(vpcs.Logger, func() error { + volumes, err = vpcs.Apiclient.VolumeService().ListVolumes(limit, start, filters, vpcs.Logger) + return err + }) + + if err != nil { + return nil, userError.GetUserError("ListVolumesFailed", err) + } + + vpcs.Logger.Info("Successfully retrieved volumes list from VPC backend", zap.Reflect("VolumesList", volumes)) + + var respVolumesList = &provider.VolumeList{} + if volumes != nil { + if volumes.Next != nil { + var next string + // "Next":{"href":"https://eu-gb.iaas.cloud.ibm.com/v1/volumes?start=3e898aa7-ac71-4323-952d-a8d741c65a68\u0026limit=1\u0026zone.name=eu-gb-1"} + if strings.Contains(volumes.Next.Href, "start=") { + next = strings.Split(strings.Split(volumes.Next.Href, "start=")[1], "\u0026")[0] + } else { + vpcs.Logger.Warn("Volumes.Next.Href is not in expected format", zap.Reflect("volumes.Next.Href", volumes.Next.Href)) + } + respVolumesList.Next = next + } + + volumeslist := volumes.Volumes + if volumeslist != nil && len(volumeslist) > 0 { + for _, volItem := range volumeslist { + volumeResponse := FromProviderToLibVolume(volItem, vpcs.Logger) + respVolumesList.Volumes = append(respVolumesList.Volumes, volumeResponse) + } + } + } + return respVolumesList, err } diff --git a/volume-providers/vpc/provider/list_volumes_test.go b/volume-providers/vpc/provider/list_volumes_test.go index d894f70c..ef417c25 100644 --- a/volume-providers/vpc/provider/list_volumes_test.go +++ b/volume-providers/vpc/provider/list_volumes_test.go @@ -11,6 +11,7 @@ package provider import ( + "errors" "github.com/IBM/ibmcloud-storage-volume-lib/lib/provider" "github.com/IBM/ibmcloud-storage-volume-lib/lib/utils" "github.com/IBM/ibmcloud-storage-volume-lib/lib/utils/reasoncode" @@ -18,6 +19,8 @@ import ( volumeServiceFakes "github.com/IBM/ibmcloud-storage-volume-lib/volume-providers/vpc/vpcclient/vpcvolume/fakes" "github.com/stretchr/testify/assert" "go.uber.org/zap" + "strconv" + "strings" "testing" ) @@ -32,10 +35,11 @@ func TestListVolumes(t *testing.T) { testCases := []struct { testCaseName string - volumeID string - baseVolume *models.Volume + volumeList *models.VolumeList - tags map[string]string + limit int + start string + tags map[string]string setup func() @@ -43,16 +47,230 @@ func TestListVolumes(t *testing.T) { expectedErr string expectedReasonCode string - verify func(t *testing.T, volumes []*provider.Volume, err error) + verify func(t *testing.T, next_token string, volumes *provider.VolumeList, err error) }{ { - testCaseName: "Not supported", - volumeID: "16f293bf-test-4bff-816f-e199c0c65db5", + testCaseName: "Filter by zone", + volumeList: &models.VolumeList{ + First: &models.HReference{Href: "https://eu-gb.iaas.cloud.ibm.com/v1/volumes?start=16f293bf-test-4bff-816f-e199c0c65db5\u0026limit=50\u0026zone.name=test-zone-1"}, + Next: nil, + Limit: 50, + Volumes: []*models.Volume{ + { + ID: "16f293bf-test-4bff-816f-e199c0c65db5", + Name: "test-volume-name1", + Status: models.StatusType("OK"), + Capacity: int64(10), + Iops: int64(1000), + Zone: &models.Zone{Name: "test-zone-1"}, + }, { + ID: "23b154fr-test-4bff-816f-f213s1y34gj8", + Name: "test-volume-name2", + Status: models.StatusType("OK"), + Capacity: int64(10), + Iops: int64(1000), + Zone: &models.Zone{Name: "test-zone-1"}, + }, + }, + }, + tags: map[string]string{ + "zone.name": "test-zone-1", + }, + verify: func(t *testing.T, next_token string, volumes *provider.VolumeList, err error) { + assert.NotNil(t, volumes.Volumes) + assert.Equal(t, next_token, volumes.Next) + assert.Nil(t, err) + }, + }, { + testCaseName: "Filter by zone, 1 entry per page", + volumeList: &models.VolumeList{ + First: &models.HReference{Href: "https://eu-gb.iaas.cloud.ibm.com/v1/volumes?start=16f293bf-test-4bff-816f-e199c0c65db5\u0026limit=1\u0026zone.name=test-zone-1"}, + Next: &models.HReference{Href: "https://eu-gb.iaas.cloud.ibm.com/v1/volumes?start=23b154fr-test-4bff-816f-f213s1y34gj8\u0026limit=1\u0026zone.name=test-zone-1"}, + Limit: 1, + Volumes: []*models.Volume{ + { + ID: "16f293bf-test-4bff-816f-e199c0c65db5", + Name: "test-volume-name1", + Status: models.StatusType("OK"), + Capacity: int64(10), + Iops: int64(1000), + Zone: &models.Zone{Name: "test-zone-1"}, + }, { + ID: "23b154fr-test-4bff-816f-f213s1y34gj8", + Name: "test-volume-name2", + Status: models.StatusType("OK"), + Capacity: int64(10), + Iops: int64(1000), + Zone: &models.Zone{Name: "test-zone-1"}, + }, + }, + }, + tags: map[string]string{ + "zone.name": "test-zone-1", + }, + limit: 1, + verify: func(t *testing.T, next_token string, volumes *provider.VolumeList, err error) { + assert.NotNil(t, volumes.Volumes) + assert.Equal(t, next_token, volumes.Next) + assert.Nil(t, err) + }, + }, { + testCaseName: "Filter by zone: no volume found", // Filter by zone where no volume is present + volumeList: &models.VolumeList{ + First: &models.HReference{Href: "https://eu-gb.iaas.cloud.ibm.com/v1/volumes?limit=50\u0026zone.name=test-zone"}, + Next: nil, + Limit: 50, + Volumes: []*models.Volume{}, + }, + tags: map[string]string{ + "zone.name": "test-zone", + }, + verify: func(t *testing.T, next_token string, volumes *provider.VolumeList, err error) { + assert.Nil(t, volumes.Volumes) + assert.Equal(t, next_token, volumes.Next) + assert.Nil(t, err) + }, + }, { + testCaseName: "Filter by name", + volumeList: &models.VolumeList{ + First: &models.HReference{Href: "https://eu-gb.iaas.cloud.ibm.com/v1/volumes?start=16f293bf-test-4bff-816f-e199c0c65db5\u0026limit=50"}, + Next: nil, + Limit: 50, + Volumes: []*models.Volume{ + { + ID: "16f293bf-test-4bff-816f-e199c0c65db5", + Name: "test-volume-name1", + Status: models.StatusType("OK"), + Capacity: int64(10), + Iops: int64(1000), + Zone: &models.Zone{Name: "test-zone"}, + }, + }, + }, + tags: map[string]string{ + "name": "test-volume-name1", + }, + verify: func(t *testing.T, next_token string, volumes *provider.VolumeList, err error) { + assert.NotNil(t, volumes.Volumes) + assert.Equal(t, next_token, volumes.Next) + assert.Nil(t, err) + }, + }, { + testCaseName: "Filter by name: volume not found", tags: map[string]string{ - "dev": "volume1", + "name": "test-volume-name1", }, - verify: func(t *testing.T, volumes []*provider.Volume, err error) { + expectedErr: "{Code:ErrorUnclassified, Type:RetrivalFailed, Description: Unable to fetch list of volumes. ", + expectedReasonCode: "ErrorUnclassified", + verify: func(t *testing.T, next_token string, volumes *provider.VolumeList, err error) { assert.Nil(t, volumes) + assert.NotNil(t, err) + }, + }, { + testCaseName: "Filter by resource group ID", + volumeList: &models.VolumeList{ + First: &models.HReference{Href: "https://eu-gb.iaas.cloud.ibm.com/v1/volumes?start=16f293bf-test-4bff-816f-e199c0c65db5\u0026limit=50"}, + Next: nil, + Limit: 50, + Volumes: []*models.Volume{ + { + ID: "16f293bf-test-4bff-816f-e199c0c65db5", + Name: "test-volume-name1", + Status: models.StatusType("OK"), + Capacity: int64(10), + Iops: int64(1000), + Zone: &models.Zone{Name: "test-zone-1"}, + }, { + ID: "23b154fr-test-4bff-816f-f213s1y34gj8", + Name: "test-volume-name2", + Status: models.StatusType("OK"), + Capacity: int64(10), + Iops: int64(1000), + Zone: &models.Zone{Name: "test-zone-2"}, + }, + }, + }, + tags: map[string]string{ + "resource_group.id": "12345xy4567z89776", + }, + verify: func(t *testing.T, next_token string, volumes *provider.VolumeList, err error) { + assert.NotNil(t, volumes.Volumes) + assert.Equal(t, next_token, volumes.Next) + assert.Nil(t, err) + }, + }, { + testCaseName: "Filter by resource group ID: no volume found", + volumeList: &models.VolumeList{ + First: &models.HReference{Href: "https://eu-gb.iaas.cloud.ibm.com/v1/volumes?limit=50"}, + Next: nil, + Limit: 50, + Volumes: []*models.Volume{}, + }, + tags: map[string]string{ + "resource_group.id": "12345xy4567z89776", + }, + verify: func(t *testing.T, next_token string, volumes *provider.VolumeList, err error) { + assert.Nil(t, volumes.Volumes) + assert.Equal(t, next_token, volumes.Next) + assert.Nil(t, err) + }, + }, { + testCaseName: "List all volumes", + volumeList: &models.VolumeList{ + First: &models.HReference{Href: "https://eu-gb.iaas.cloud.ibm.com/v1/volumes?start=16f293bf-test-4bff-816f-e199c0c65db5\u0026limit=50"}, + Next: nil, + Limit: 50, + Volumes: []*models.Volume{ + { + ID: "16f293bf-test-4bff-816f-e199c0c65db5", + Name: "test-volume-name1", + Status: models.StatusType("OK"), + Capacity: int64(10), + Iops: int64(1000), + Zone: &models.Zone{Name: "test-zone-1"}, + }, { + ID: "23b154fr-test-4bff-816f-f213s1y34gj8", + Name: "test-volume-name2", + Status: models.StatusType("OK"), + Capacity: int64(10), + Iops: int64(1000), + Zone: &models.Zone{Name: "test-zone-2"}, + }, + }, + }, + verify: func(t *testing.T, next_token string, volumes *provider.VolumeList, err error) { + assert.NotNil(t, volumes.Volumes) + assert.Equal(t, next_token, volumes.Next) + assert.Nil(t, err) + }, + }, { + testCaseName: "Unexpected format of 'Next' parameter in ListVolumes response", + volumeList: &models.VolumeList{ + First: &models.HReference{Href: "https://eu-gb.iaas.cloud.ibm.com/v1/volumes?start=16f293bf-test-4bff-816f-e199c0c65db5\u0026limit=50"}, + Next: &models.HReference{Href: "https://eu-gb.iaas.cloud.ibm.com/v1/volumes?invalid=16f293bf-test-4bff-816f-e199c0c65db5\u0026limit=50"}, + Limit: 1, + Volumes: []*models.Volume{ + { + ID: "16f293bf-test-4bff-816f-e199c0c65db5", + Name: "test-volume-name1", + Status: models.StatusType("OK"), + Capacity: int64(10), + Iops: int64(1000), + Zone: &models.Zone{Name: "test-zone-1"}, + }, { + ID: "23b154fr-test-4bff-816f-f213s1y34gj8", + Name: "test-volume-name2", + Status: models.StatusType("OK"), + Capacity: int64(10), + Iops: int64(1000), + Zone: &models.Zone{Name: "test-zone-2"}, + }, + }, + }, + limit: 1, + verify: func(t *testing.T, next_token string, volumes *provider.VolumeList, err error) { + assert.NotNil(t, volumes.Volumes) + assert.Equal(t, next_token, volumes.Next) assert.Nil(t, err) }, }, @@ -70,10 +288,13 @@ func TestListVolumes(t *testing.T) { assert.NotNil(t, volumeService) uc.VolumeServiceReturns(volumeService) - volumeService.ListVolumesReturns(nil, nil) - - volumes, err := vpcs.ListVolumes(testcase.tags) - logger.Info("Volumes details", zap.Reflect("volumes", volumes)) + if testcase.expectedErr != "" { + volumeService.ListVolumesReturns(testcase.volumeList, errors.New(testcase.expectedReasonCode)) + } else { + volumeService.ListVolumesReturns(testcase.volumeList, nil) + } + volumes, err := vpcs.ListVolumes(testcase.limit, testcase.start, testcase.tags) + logger.Info("VolumesList details", zap.Reflect("VolumesList", volumes)) if testcase.expectedErr != "" { assert.NotNil(t, err) @@ -82,7 +303,26 @@ func TestListVolumes(t *testing.T) { } if testcase.verify != nil { - testcase.verify(t, volumes, err) + var next string + if testcase.volumeList != nil { + if testcase.volumeList.Next != nil { + // "Next":{"href":"https://eu-gb.iaas.cloud.ibm.com/v1/volumes?start=3e898aa7-ac71-4323-952d-a8d741c65a68\u0026limit=1\u0026zone.name=eu-gb-1"} + if strings.Contains(testcase.volumeList.Next.Href, "start=") { + next = strings.Split(strings.Split(testcase.volumeList.Next.Href, "start=")[1], "\u0026")[0] + } + } + } + testcase.verify(t, next, volumes, err) + if volumes != nil && volumes.Volumes != nil { + for index, vol := range volumes.Volumes { + assert.Equal(t, testcase.volumeList.Volumes[index].ID, vol.VolumeID) + assert.Equal(t, testcase.volumeList.Volumes[index].Capacity, int64(*vol.Capacity)) + + iops, _ := strconv.ParseInt(*vol.Iops, 10, 64) + assert.Equal(t, testcase.volumeList.Volumes[index].Iops, iops) + assert.Equal(t, testcase.volumeList.Volumes[index].Zone, &models.Zone{Name: vol.Az}) + } + } } }) diff --git a/volume-providers/vpc/vpcclient/models/volume.go b/volume-providers/vpc/vpcclient/models/volume.go index 0062e4d6..77b526b8 100644 --- a/volume-providers/vpc/vpcclient/models/volume.go +++ b/volume-providers/vpc/vpcclient/models/volume.go @@ -48,17 +48,24 @@ type Volume struct { // ListVolumeFilters ... type ListVolumeFilters struct { - ResourceGroupID string - Tag string - ZoneName string - VolumeName string + ResourceGroupID string `json:"resource_group.id,omitempty"` + Tag string `json:"tag,omitempty"` + ZoneName string `json:"zone.name,omitempty"` + VolumeName string `json:"name,omitempty"` } // VolumeList ... type VolumeList struct { - Volumes []*Volume `json:"volumes,omitempty"` - Limit int `json:"limit,omitempty"` - TotalCount int `json:"total_count,omitempty"` + First *HReference `json:"first,omitempty"` + Next *HReference `json:"next,omitempty"` + Volumes []*Volume `json:"volumes"` + Limit int `json:"limit,omitempty"` + TotalCount int `json:"total_count,omitempty"` +} + +// HReference ... +type HReference struct { + Href string `json:"href,omitempty"` } //NewVolume created model volume from provider volume diff --git a/volume-providers/vpc/vpcclient/vpcvolume/fakes/volume.go b/volume-providers/vpc/vpcclient/vpcvolume/fakes/volume.go index 934bfac0..71e505ed 100644 --- a/volume-providers/vpc/vpcclient/vpcvolume/fakes/volume.go +++ b/volume-providers/vpc/vpcclient/vpcvolume/fakes/volume.go @@ -104,12 +104,13 @@ type VolumeService struct { result1 *[]string result2 error } - ListVolumesStub func(int, *models.ListVolumeFilters, *zap.Logger) (*models.VolumeList, error) + ListVolumesStub func(int, string, *models.ListVolumeFilters, *zap.Logger) (*models.VolumeList, error) listVolumesMutex sync.RWMutex listVolumesArgsForCall []struct { arg1 int - arg2 *models.ListVolumeFilters - arg3 *zap.Logger + arg2 string + arg3 *models.ListVolumeFilters + arg4 *zap.Logger } listVolumesReturns struct { result1 *models.VolumeList @@ -589,18 +590,19 @@ func (fake *VolumeService) ListVolumeTagsReturnsOnCall(i int, result1 *[]string, }{result1, result2} } -func (fake *VolumeService) ListVolumes(arg1 int, arg2 *models.ListVolumeFilters, arg3 *zap.Logger) (*models.VolumeList, error) { +func (fake *VolumeService) ListVolumes(arg1 int, arg2 string, arg3 *models.ListVolumeFilters, arg4 *zap.Logger) (*models.VolumeList, error) { fake.listVolumesMutex.Lock() ret, specificReturn := fake.listVolumesReturnsOnCall[len(fake.listVolumesArgsForCall)] fake.listVolumesArgsForCall = append(fake.listVolumesArgsForCall, struct { arg1 int - arg2 *models.ListVolumeFilters - arg3 *zap.Logger - }{arg1, arg2, arg3}) - fake.recordInvocation("ListVolumes", []interface{}{arg1, arg2, arg3}) + arg2 string + arg3 *models.ListVolumeFilters + arg4 *zap.Logger + }{arg1, arg2, arg3, arg4}) + fake.recordInvocation("ListVolumes", []interface{}{arg1, arg2, arg3, arg4}) fake.listVolumesMutex.Unlock() if fake.ListVolumesStub != nil { - return fake.ListVolumesStub(arg1, arg2, arg3) + return fake.ListVolumesStub(arg1, arg2, arg3, arg4) } if specificReturn { return ret.result1, ret.result2 @@ -615,17 +617,17 @@ func (fake *VolumeService) ListVolumesCallCount() int { return len(fake.listVolumesArgsForCall) } -func (fake *VolumeService) ListVolumesCalls(stub func(int, *models.ListVolumeFilters, *zap.Logger) (*models.VolumeList, error)) { +func (fake *VolumeService) ListVolumesCalls(stub func(int, string, *models.ListVolumeFilters, *zap.Logger) (*models.VolumeList, error)) { fake.listVolumesMutex.Lock() defer fake.listVolumesMutex.Unlock() fake.ListVolumesStub = stub } -func (fake *VolumeService) ListVolumesArgsForCall(i int) (int, *models.ListVolumeFilters, *zap.Logger) { +func (fake *VolumeService) ListVolumesArgsForCall(i int) (int, string, *models.ListVolumeFilters, *zap.Logger) { fake.listVolumesMutex.RLock() defer fake.listVolumesMutex.RUnlock() argsForCall := fake.listVolumesArgsForCall[i] - return argsForCall.arg1, argsForCall.arg2, argsForCall.arg3 + return argsForCall.arg1, argsForCall.arg2, argsForCall.arg3, argsForCall.arg4 } func (fake *VolumeService) ListVolumesReturns(result1 *models.VolumeList, result2 error) { diff --git a/volume-providers/vpc/vpcclient/vpcvolume/get_volume.go b/volume-providers/vpc/vpcclient/vpcvolume/get_volume.go index f4862363..b6222466 100644 --- a/volume-providers/vpc/vpcclient/vpcvolume/get_volume.go +++ b/volume-providers/vpc/vpcclient/vpcvolume/get_volume.go @@ -55,7 +55,7 @@ func (vs *VolumeService) GetVolumeByName(volumeName string, ctxLogger *zap.Logge // Get the volume details for a single volume, ListVolumeFilters will return only 1 volume in list filters := &models.ListVolumeFilters{VolumeName: volumeName} - volumes, err := vs.ListVolumes(1, filters, ctxLogger) + volumes, err := vs.ListVolumes(1, "", filters, ctxLogger) if err != nil { return nil, err } diff --git a/volume-providers/vpc/vpcclient/vpcvolume/list_volumes.go b/volume-providers/vpc/vpcclient/vpcvolume/list_volumes.go index 23994f8e..43974d1a 100644 --- a/volume-providers/vpc/vpcclient/vpcvolume/list_volumes.go +++ b/volume-providers/vpc/vpcclient/vpcvolume/list_volumes.go @@ -20,7 +20,7 @@ import ( ) // ListVolumes GETs /volumes -func (vs *VolumeService) ListVolumes(limit int, filters *models.ListVolumeFilters, ctxLogger *zap.Logger) (*models.VolumeList, error) { +func (vs *VolumeService) ListVolumes(limit int, start string, filters *models.ListVolumeFilters, ctxLogger *zap.Logger) (*models.VolumeList, error) { ctxLogger.Debug("Entry Backend ListVolumes") defer ctxLogger.Debug("Exit Backend ListVolumes") @@ -44,6 +44,10 @@ func (vs *VolumeService) ListVolumes(limit int, filters *models.ListVolumeFilter req.AddQueryValue("limit", strconv.Itoa(limit)) } + if start != "" { + req.AddQueryValue("start", start) + } + if filters != nil { if filters.ResourceGroupID != "" { req.AddQueryValue("resource_group.id", filters.ResourceGroupID) diff --git a/volume-providers/vpc/vpcclient/vpcvolume/list_volumes_test.go b/volume-providers/vpc/vpcclient/vpcvolume/list_volumes_test.go index 03e8d4af..96fcb8c7 100644 --- a/volume-providers/vpc/vpcclient/vpcvolume/list_volumes_test.go +++ b/volume-providers/vpc/vpcclient/vpcvolume/list_volumes_test.go @@ -34,6 +34,7 @@ func TestListVolumes(t *testing.T) { content string limit int + start string filters *models.ListVolumeFilters // Expected return @@ -51,10 +52,19 @@ func TestListVolumes(t *testing.T) { expectErr: "Trace Code:, testerr Please check ", }, { name: "Verify that limit is added to the query", - limit: 21, + limit: 12, status: http.StatusNoContent, muxVerify: func(t *testing.T, r *http.Request) { - expectedValues := url.Values{"limit": []string{"21"}, "version": []string{models.APIVersion}} + expectedValues := url.Values{"limit": []string{"12"}, "version": []string{models.APIVersion}} + actualValues := r.URL.Query() + assert.Equal(t, expectedValues, actualValues) + }, + }, { + name: "Verify that start is added to the query", + start: "x-y-z", + status: http.StatusNoContent, + muxVerify: func(t *testing.T, r *http.Request) { + expectedValues := url.Values{"start": []string{"x-y-z"}, "version": []string{models.APIVersion}} actualValues := r.URL.Query() assert.Equal(t, expectedValues, actualValues) }, @@ -116,7 +126,7 @@ func TestListVolumes(t *testing.T) { volumeService := vpcvolume.New(client) - volumes, err := volumeService.ListVolumes(testcase.limit, testcase.filters, logger) + volumes, err := volumeService.ListVolumes(testcase.limit, testcase.start, testcase.filters, logger) logger.Info("Volumes", zap.Reflect("volumes", volumes)) if testcase.expectErr != "" && assert.Error(t, err) { diff --git a/volume-providers/vpc/vpcclient/vpcvolume/volume_service.go b/volume-providers/vpc/vpcclient/vpcvolume/volume_service.go index c207cf2d..37293e37 100644 --- a/volume-providers/vpc/vpcclient/vpcvolume/volume_service.go +++ b/volume-providers/vpc/vpcclient/vpcvolume/volume_service.go @@ -34,7 +34,7 @@ type VolumeManager interface { // Others // Get volume lists by using snapshot tags - ListVolumes(limit int, filters *models.ListVolumeFilters, ctxLogger *zap.Logger) (*models.VolumeList, error) + ListVolumes(limit int, start string, filters *models.ListVolumeFilters, ctxLogger *zap.Logger) (*models.VolumeList, error) // Set tag for a volume SetVolumeTag(volumeID string, tagName string, ctxLogger *zap.Logger) error diff --git a/volume-providers/vpc/vpcclient/vpcvolume/vpcvolumefakes/volume_service.go b/volume-providers/vpc/vpcclient/vpcvolume/vpcvolumefakes/volume_service.go index 8e5e0735..4b41ace9 100644 --- a/volume-providers/vpc/vpcclient/vpcvolume/vpcvolumefakes/volume_service.go +++ b/volume-providers/vpc/vpcclient/vpcvolume/vpcvolumefakes/volume_service.go @@ -104,12 +104,13 @@ type VolumeService struct { result1 *[]string result2 error } - ListVolumesStub func(int, *models.ListVolumeFilters, *zap.Logger) (*models.VolumeList, error) + ListVolumesStub func(int, string, *models.ListVolumeFilters, *zap.Logger) (*models.VolumeList, error) listVolumesMutex sync.RWMutex listVolumesArgsForCall []struct { arg1 int - arg2 *models.ListVolumeFilters - arg3 *zap.Logger + arg2 string + arg3 *models.ListVolumeFilters + arg4 *zap.Logger } listVolumesReturns struct { result1 *models.VolumeList @@ -589,18 +590,19 @@ func (fake *VolumeService) ListVolumeTagsReturnsOnCall(i int, result1 *[]string, }{result1, result2} } -func (fake *VolumeService) ListVolumes(arg1 int, arg2 *models.ListVolumeFilters, arg3 *zap.Logger) (*models.VolumeList, error) { +func (fake *VolumeService) ListVolumes(arg1 int, arg2 string, arg3 *models.ListVolumeFilters, arg4 *zap.Logger) (*models.VolumeList, error) { fake.listVolumesMutex.Lock() ret, specificReturn := fake.listVolumesReturnsOnCall[len(fake.listVolumesArgsForCall)] fake.listVolumesArgsForCall = append(fake.listVolumesArgsForCall, struct { arg1 int - arg2 *models.ListVolumeFilters - arg3 *zap.Logger - }{arg1, arg2, arg3}) - fake.recordInvocation("ListVolumes", []interface{}{arg1, arg2, arg3}) + arg2 string + arg3 *models.ListVolumeFilters + arg4 *zap.Logger + }{arg1, arg2, arg3, arg4}) + fake.recordInvocation("ListVolumes", []interface{}{arg1, arg2, arg3, arg4}) fake.listVolumesMutex.Unlock() if fake.ListVolumesStub != nil { - return fake.ListVolumesStub(arg1, arg2, arg3) + return fake.ListVolumesStub(arg1, arg2, arg3, arg4) } if specificReturn { return ret.result1, ret.result2 @@ -615,17 +617,17 @@ func (fake *VolumeService) ListVolumesCallCount() int { return len(fake.listVolumesArgsForCall) } -func (fake *VolumeService) ListVolumesCalls(stub func(int, *models.ListVolumeFilters, *zap.Logger) (*models.VolumeList, error)) { +func (fake *VolumeService) ListVolumesCalls(stub func(int, string, *models.ListVolumeFilters, *zap.Logger) (*models.VolumeList, error)) { fake.listVolumesMutex.Lock() defer fake.listVolumesMutex.Unlock() fake.ListVolumesStub = stub } -func (fake *VolumeService) ListVolumesArgsForCall(i int) (int, *models.ListVolumeFilters, *zap.Logger) { +func (fake *VolumeService) ListVolumesArgsForCall(i int) (int, string, *models.ListVolumeFilters, *zap.Logger) { fake.listVolumesMutex.RLock() defer fake.listVolumesMutex.RUnlock() argsForCall := fake.listVolumesArgsForCall[i] - return argsForCall.arg1, argsForCall.arg2, argsForCall.arg3 + return argsForCall.arg1, argsForCall.arg2, argsForCall.arg3, argsForCall.arg4 } func (fake *VolumeService) ListVolumesReturns(result1 *models.VolumeList, result2 error) {