Skip to content

Commit 2bf5667

Browse files
sameshaisameshai
committed
Review comments
Signed-off-by: Sameer Shaikh <[email protected]>
1 parent 17d0c98 commit 2bf5667

File tree

6 files changed

+63
-37
lines changed

6 files changed

+63
-37
lines changed

‎common/vpcclient/models/subnet.go‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -42,9 +42,11 @@ type SubnetRef struct {
4242

4343
// SubnetList ...
4444
type SubnetList struct {
45-
Next string `json:"next,omitempty"`
46-
Subnets []*Subnet `json:"subnets,omitempty"`
47-
Limit int `json:"limit,omitempty"`
45+
First *HReference `json:"first,omitempty"`
46+
Next *HReference `json:"next,omitempty"`
47+
Subnets []*Subnet `json:"subnets,omitempty"`
48+
Limit int `json:"limit,omitempty"`
49+
TotalCount int `json:"total_count,omitempty"`
4850
}
4951

5052
// ListSubnetFilters ...

‎file/provider/create_volume_access_point.go‎

Lines changed: 50 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,8 @@ package provider
1919

2020
import (
2121
"errors"
22+
"net/url"
23+
"strings"
2224
"time"
2325

2426
userError "github.com/IBM/ibmcloud-volume-file-vpc/common/messages"
@@ -78,7 +80,7 @@ func (vpcs *VPCSession) CreateVolumeAccessPoint(volumeAccessPointRequest provide
7880

7981
if volumeAccessPointRequest.PrimaryIP == nil || len(volumeAccessPointRequest.PrimaryIP.ID) == 0 {
8082
vpcs.Logger.Info("Getting subnet from VPC provider...")
81-
subnet, err = vpcs.getSubnet(volumeAccessPointRequest.Zone, volumeAccessPointRequest.VPCID, volumeAccessPointRequest.ResourceGroup.ID)
83+
subnet, err = vpcs.getSubnet(volumeAccessPointRequest)
8284
// Keep retry, until we get the proper volumeAccessPointResult object
8385
if err != nil && subnet == nil {
8486
return err, skipRetryForObviousErrors(err)
@@ -122,6 +124,7 @@ func (vpcs *VPCSession) validateVolumeAccessPointRequest(volumeAccessPointReques
122124
vpcs.Logger.Error("volumeAccessPointRequest.VolumeID is required", zap.Error(err))
123125
return err
124126
}
127+
125128
// Check for VPC ID - required validation
126129
if len(volumeAccessPointRequest.VPCID) == 0 && len(volumeAccessPointRequest.SubnetID) == 0 && len(volumeAccessPointRequest.AccessPointID) == 0 {
127130
err = userError.GetUserError(string(reasoncode.ErrorRequiredFieldMissing), nil, "VPCID")
@@ -132,52 +135,73 @@ func (vpcs *VPCSession) validateVolumeAccessPointRequest(volumeAccessPointReques
132135
}
133136

134137
// GetSubnet get the subnet based on the request
135-
func (vpcs *VPCSession) getSubnet(zoneName string, vpcID string, resourceGroupID string) (*models.Subnet, error) {
136-
vpcs.Logger.Debug("Entry of GetSubnet method...", zap.Reflect("zoneName", zoneName), zap.Reflect("vpcID", vpcID), zap.Reflect("resourceGroupID", resourceGroupID))
138+
func (vpcs *VPCSession) getSubnet(volumeAccessPointRequest provider.VolumeAccessPointRequest) (*models.Subnet, error) {
139+
vpcs.Logger.Debug("Entry of GetSubnet method...", zap.Reflect("volumeAccessPointRequest", volumeAccessPointRequest))
137140
defer vpcs.Logger.Debug("Exit from GetSubnet method...")
138141
var err error
139142

140-
/* err = vpcs.validateVolumeAccessPointRequest(volumeAccessPointRequest)
141-
if err != nil {
142-
return nil, err
143-
} */
144-
145143
// Get Subnet by VPC ID and zone. This is inefficient operation which requires iteration over subnet list
146-
subnet, err := vpcs.getSubnetByVPCIDAndZone(zoneName, vpcID, resourceGroupID)
144+
subnet, err := vpcs.getSubnetByVPCIDAndZone(volumeAccessPointRequest)
147145
vpcs.Logger.Info("getSubnetByVPCIDAndZone response", zap.Reflect("subnet", subnet), zap.Error(err))
148146
return subnet, err
149147
}
150148

151-
func (vpcs *VPCSession) getSubnetByVPCIDAndZone(zoneName string, vpcID string, resourceGroupID string) (*models.Subnet, error) {
149+
func (vpcs *VPCSession) getSubnetByVPCIDAndZone(volumeAccessPointRequest provider.VolumeAccessPointRequest) (*models.Subnet, error) {
152150
vpcs.Logger.Debug("Entry of getSubnetByVPCIDAndZone()")
153151
defer vpcs.Logger.Debug("Exit from getSubnetByVPCIDAndZone()")
154152
vpcs.Logger.Info("Getting getSubnetByVPCIDAndZone from VPC provider...")
155153
var err error
156154
var subnets *models.SubnetList
155+
var start = ""
157156

158-
filters := &models.ListSubnetFilters{ResourceGroupID: resourceGroupID}
157+
filters := &models.ListSubnetFilters{ResourceGroupID: volumeAccessPointRequest.ResourceGroup.ID}
159158

160-
subnets, err = vpcs.Apiclient.FileShareService().ListSubnets(10, "", filters, vpcs.Logger)
159+
for {
161160

162-
if err != nil {
163-
// API call is failed
164-
userErr := userError.GetUserError(string(userError.AccessPointWithVPCIDFindFailed), err, zoneName, vpcID)
165-
return nil, userErr
166-
}
161+
subnets, err = vpcs.Apiclient.FileShareService().ListSubnets(10, start, filters, vpcs.Logger)
162+
163+
if err != nil {
164+
// API call is failed
165+
userErr := userError.GetUserError(string(userError.AccessPointWithVPCIDFindFailed), err, volumeAccessPointRequest.Zone, volumeAccessPointRequest.VPCID)
166+
return nil, userErr
167+
}
167168

168-
// Iterate over the subnet list for given volume
169-
if subnets != nil {
170-
subnetList := subnets.Subnets
171-
for _, subnetItem := range subnetList {
172-
// Check if VPC ID and zone name is matching with requested input
173-
if subnetItem.VPC != nil && subnetItem.VPC.ID == vpcID && subnetItem.Zone != nil && subnetItem.Zone.Name == zoneName {
174-
vpcs.Logger.Info("Successfully found subnet", zap.Reflect("subnetItem", subnetItem))
175-
return subnetItem, nil
169+
// Iterate over the subnet list for given volume
170+
if subnets != nil {
171+
subnetList := subnets.Subnets
172+
for _, subnetItem := range subnetList {
173+
// Check if VPC ID and zone name is matching with requested input
174+
if subnetItem.VPC != nil && subnetItem.VPC.ID == volumeAccessPointRequest.VPCID && subnetItem.Zone != nil && subnetItem.Zone.Name == volumeAccessPointRequest.Zone && strings.Contains(volumeAccessPointRequest.SubnetIDList, subnetItem.ID) {
175+
vpcs.Logger.Info("Successfully found subnet", zap.Reflect("subnetItem", subnetItem))
176+
return subnetItem, nil
177+
}
178+
}
179+
180+
if subnets.Next == nil {
181+
break // No more pages, exit the loop
182+
}
183+
184+
// Fetch the start of next page
185+
startUrl, err := url.Parse(subnets.Next.Href)
186+
if err != nil {
187+
// API call is failed
188+
userErr := userError.GetUserError(string(userError.AccessPointWithVPCIDFindFailed), err, volumeAccessPointRequest.Zone, volumeAccessPointRequest.VPCID)
189+
return nil, userErr
176190
}
191+
192+
vpcs.Logger.Info("startUrl", zap.Reflect("startUrl", startUrl))
193+
start = startUrl.Query().Get("start") //parse query param into map
194+
if start == "" {
195+
// API call is failed
196+
userErr := userError.GetUserError(string(userError.AccessPointWithVPCIDFindFailed), err, volumeAccessPointRequest.Zone, volumeAccessPointRequest.VPCID)
197+
return nil, userErr
198+
}
199+
177200
}
178201
}
202+
179203
// No volume Subnet found in the list. So return error
180-
userErr := userError.GetUserError(string(userError.AccessPointWithVPCIDFindFailed), errors.New("no subnet found"), zoneName, vpcID)
204+
userErr := userError.GetUserError(string(userError.AccessPointWithVPCIDFindFailed), errors.New("no subnet found"), volumeAccessPointRequest.Zone, volumeAccessPointRequest.VPCID)
181205
vpcs.Logger.Error("Subnet not found", zap.Error(err))
182206
return nil, userErr
183207
}

‎file/provider/util.go‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -28,17 +28,17 @@ import (
2828
)
2929

3030
// maxRetryAttempt ...
31-
var maxRetryAttempt = 0
31+
var maxRetryAttempt = 10
3232

3333
// maxRetryGap ...
34-
var maxRetryGap = 0
34+
var maxRetryGap = 60
3535

3636
// retryGap ...
37-
var retryGap = 0
37+
var retryGap = 10
3838

3939
// ConstantRetryGap ...
4040
const (
41-
ConstantRetryGap = 0 // seconds
41+
ConstantRetryGap = 10 // seconds
4242
SecurityGroupMode = "security_group"
4343
VPCMode = "vpc"
4444
)

‎go.mod‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ go 1.19
55
require (
66
github.com/IBM-Cloud/ibm-cloud-cli-sdk v0.6.7
77
github.com/IBM/ibm-csi-common v1.1.6
8-
github.com/IBM/ibmcloud-volume-interface v1.1.5-0.20230517070752-7b6e024472c2
8+
github.com/IBM/ibmcloud-volume-interface v1.1.5-0.20230518103801-9f6369a64a54
99
github.com/IBM/secret-common-lib v1.1.4
1010
github.com/IBM/secret-utils-lib v1.1.4
1111
github.com/fatih/structs v1.1.0

‎go.sum‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -41,8 +41,8 @@ github.com/IBM/go-sdk-core/v5 v5.9.1 h1:06pXbD9Rgmqqe2HA5YAeQbB4eYRRFgIoOT+Kh3cp
4141
github.com/IBM/go-sdk-core/v5 v5.9.1/go.mod h1:axE2JrRq79gIJTjKPBwV6gWHswvVptBjbcvvCPIxARM=
4242
github.com/IBM/ibm-csi-common v1.1.6 h1:fduMnvAGftkt21k4eXfwxg8ELj0FeaycDDSskqvUOQM=
4343
github.com/IBM/ibm-csi-common v1.1.6/go.mod h1:qZ2vJUM8/X5rFIBwpjPxV12y7ThUDObAcjjBHYwfFf4=
44-
github.com/IBM/ibmcloud-volume-interface v1.1.5-0.20230517070752-7b6e024472c2 h1:JK0IS1dyKbnIdE/4v0ZkKNOMX9L81TSbXIFt1k5xnmY=
45-
github.com/IBM/ibmcloud-volume-interface v1.1.5-0.20230517070752-7b6e024472c2/go.mod h1:646HOeq8dAKbgpr7jRehGKckhgduJyII2uN5T6RDLww=
44+
github.com/IBM/ibmcloud-volume-interface v1.1.5-0.20230518103801-9f6369a64a54 h1:5vYb0mLIR/cn5D4DG4SrDyEFL5L1bfiAfJs/I8C7klA=
45+
github.com/IBM/ibmcloud-volume-interface v1.1.5-0.20230518103801-9f6369a64a54/go.mod h1:646HOeq8dAKbgpr7jRehGKckhgduJyII2uN5T6RDLww=
4646
github.com/IBM/secret-common-lib v1.1.4 h1:gKpKnaP45Y6u7VpSlFfXjjTAHpu4bz9Ofy+aR0t2RcI=
4747
github.com/IBM/secret-common-lib v1.1.4/go.mod h1:0L/lLfwi5jwTTmNYE2246HzBIdGz0m6wu/5tXoRp/Lc=
4848
github.com/IBM/secret-utils-lib v1.1.4 h1:8WPG9KBrLLRhGbQn34NWzrFKlyfIIaUfLeDg+iRJkes=

‎pkg/ibmcloudprovider/volume_provider.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ func NewIBMCloudStorageProvider(clusterVolumeLabel string, k8sClient *k8s_utils.
5858
conf.VPC.APIVersion = fmt.Sprintf("%d-%02d-%02d", dateTime.Year(), dateTime.Month(), dateTime.Day())
5959
} else {
6060
logger.Warn("Failed to parse VPC_API_VERSION, setting default value")
61-
conf.VPC.APIVersion = "2023-01-15" // setting default values
61+
conf.VPC.APIVersion = "2023-05-15" // setting default values
6262
}
6363

6464
var clusterInfo utilsConfig.ClusterConfig

0 commit comments

Comments
 (0)