Skip to content

Commit 3577b45

Browse files
sameshaisameshai
committed
Review comments
Signed-off-by: Sameer Shaikh <[email protected]>
1 parent ed2a20c commit 3577b45

File tree

3 files changed

+72
-26
lines changed

3 files changed

+72
-26
lines changed

‎common/messages/messages_en.go‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -170,6 +170,34 @@ var messagesEn = map[string]util.Message{
170170
RC: 500,
171171
Action: "Wait for file share deletion",
172172
},
173+
"ListSubnetsFailed": {
174+
Code: "ListSubnetsFailed",
175+
Description: "Unable to fetch list of subnets.",
176+
Type: util.RetrivalFailed,
177+
RC: 404,
178+
Action: "Unable to list subnets. Run 'ibmcloud is subnets' to list available subnets in your account.",
179+
},
180+
"NextSubnetPageParsingError": {
181+
Code: "NextSubnetPageParsingError",
182+
Description: "The next field '%s' specified in the next parameter of the list subnet call could not be parsed.",
183+
Type: util.RetrivalFailed,
184+
RC: 404,
185+
Action: "Please verify that the next field is correct.",
186+
},
187+
"StartSubnetIDEmpty": {
188+
Code: "StartSubnetIDEmpty",
189+
Description: "The start '%s' specified in the next parameter of the list subnet call is empty.",
190+
Type: util.RetrivalFailed,
191+
RC: 404,
192+
Action: "Please verify that the start field is correct.",
193+
},
194+
"SubnetFindFailedWithZoneAndVPC": {
195+
Code: "SubnetFindFailedWithZoneAndVPC",
196+
Description: "A subnet with the specified zone '%s' and vpc '%s' could not be found.",
197+
Type: util.RetrivalFailed,
198+
RC: 404,
199+
Action: "Verify that the subnet exists. Run 'ibmcloud is subnets' to list available subnets in your account.",
200+
},
173201
"ListVolumesFailed": {
174202
Code: "ListVolumesFailed",
175203
Description: "Unable to fetch list of file shares.",

‎file/provider/create_volume_access_point.go‎

Lines changed: 25 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@ func (vpcs *VPCSession) CreateVolumeAccessPoint(volumeAccessPointRequest provide
5454
var varp *provider.VolumeAccessPointResponse
5555

5656
var subnet *models.Subnet
57+
var subnetID string
5758

5859
volumeAccessPoint := models.NewShareTarget(volumeAccessPointRequest)
5960

@@ -78,20 +79,32 @@ func (vpcs *VPCSession) CreateVolumeAccessPoint(volumeAccessPointRequest provide
7879
ResourceGroup: volumeAccessPointRequest.ResourceGroup,
7980
}
8081

82+
// If primaryIP.ID is provided subnet is not mandatory for rest of the cases it is mandatory
8183
if volumeAccessPointRequest.PrimaryIP == nil || len(volumeAccessPointRequest.PrimaryIP.ID) == 0 {
82-
vpcs.Logger.Info("Getting subnet from VPC provider...")
83-
subnet, err = vpcs.getSubnet(volumeAccessPointRequest)
84-
// Keep retry, until we get the proper volumeAccessPointResult object
85-
if err != nil && subnet == nil {
86-
return err, skipRetryForObviousErrors(err)
84+
85+
if len(volumeAccessPointRequest.SubnetID) == 0 {
86+
vpcs.Logger.Info("Getting subnet from VPC provider...")
87+
subnet, err = vpcs.getSubnet(volumeAccessPointRequest)
88+
// Keep retry, until we get the proper volumeAccessPointResult object
89+
if err != nil && subnet == nil {
90+
return err, skipRetryForObviousErrors(err)
91+
}
92+
subnetID = subnet.ID
93+
94+
} else {
95+
vpcs.Logger.Info("Using subnet provided by user...", zap.Reflect("subnetID", volumeAccessPointRequest.SubnetID))
96+
subnetID = volumeAccessPointRequest.SubnetID
8797
}
8898

8999
volumeAccessPoint.VirtualNetworkInterface.Subnet = &models.SubnetRef{
90-
ID: subnet.ID,
100+
ID: subnetID,
91101
}
92102
}
93-
vpcs.Logger.Info("Primary IP ID provided using it for virtual network interface...")
94-
volumeAccessPoint.VirtualNetworkInterface.PrimaryIP = (*models.PrimaryIP)(volumeAccessPointRequest.PrimaryIP)
103+
104+
if volumeAccessPointRequest.PrimaryIP != nil {
105+
vpcs.Logger.Info("Primary IP ID provided using it for virtual network interface...")
106+
volumeAccessPoint.VirtualNetworkInterface.PrimaryIP = (*models.PrimaryIP)(volumeAccessPointRequest.PrimaryIP)
107+
}
95108
}
96109

97110
//Try creating volume accessPoint if it's not already created or there is error in getting current volume accessPoint
@@ -162,7 +175,7 @@ func (vpcs *VPCSession) getSubnetByVPCIDAndZone(volumeAccessPointRequest provide
162175

163176
if err != nil {
164177
// API call is failed
165-
userErr := userError.GetUserError(string(userError.AccessPointWithVPCIDFindFailed), err, volumeAccessPointRequest.Zone, volumeAccessPointRequest.VPCID)
178+
userErr := userError.GetUserError("ListSubnetsFailed", err)
166179
return nil, userErr
167180
}
168181

@@ -185,23 +198,23 @@ func (vpcs *VPCSession) getSubnetByVPCIDAndZone(volumeAccessPointRequest provide
185198
startUrl, err := url.Parse(subnets.Next.Href)
186199
if err != nil {
187200
// API call is failed
188-
userErr := userError.GetUserError(string(userError.AccessPointWithVPCIDFindFailed), err, volumeAccessPointRequest.Zone, volumeAccessPointRequest.VPCID)
201+
userErr := userError.GetUserError("NextSubnetPageParsingError", err, subnets.Next.Href)
189202
return nil, userErr
190203
}
191204

192205
vpcs.Logger.Info("startUrl", zap.Reflect("startUrl", startUrl))
193206
start = startUrl.Query().Get("start") //parse query param into map
194207
if start == "" {
195208
// API call is failed
196-
userErr := userError.GetUserError(string(userError.AccessPointWithVPCIDFindFailed), err, volumeAccessPointRequest.Zone, volumeAccessPointRequest.VPCID)
209+
userErr := userError.GetUserError("StartSubnetIDEmpty", err, startUrl)
197210
return nil, userErr
198211
}
199212

200213
}
201214
}
202215

203216
// No volume Subnet found in the list. So return error
204-
userErr := userError.GetUserError(string(userError.AccessPointWithVPCIDFindFailed), errors.New("no subnet found"), volumeAccessPointRequest.Zone, volumeAccessPointRequest.VPCID)
217+
userErr := userError.GetUserError(string("SubnetFindFailedWithZoneAndVPC"), errors.New("no subnet found"), volumeAccessPointRequest.Zone, volumeAccessPointRequest.VPCID)
205218
vpcs.Logger.Error("Subnet not found", zap.Error(err))
206219
return nil, userErr
207220
}

‎file/provider/util.go‎

Lines changed: 19 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -47,20 +47,25 @@ var volumeIDPartsCount = 5
4747

4848
// TODO need to introduce file share related to error codes
4949
var skipErrorCodes = map[string]bool{
50-
"shares_profile_iops_not_allowed": true,
51-
"shares_profile_capacity_invalid": true,
52-
"shares_zone_not_found": true,
53-
"shares_bad_request": true,
54-
"shares_resource_group_bad_request": true,
55-
"shares_vpc_not_found": true,
56-
"shares_not_found": true,
57-
"shares_target_not_found": true,
58-
"bad_field": true,
59-
"shares_name_duplicate": true,
60-
"shares_status_pending": false,
61-
"internal_error": false,
62-
"invalid_route": false,
63-
"service_error": false,
50+
"shares_profile_iops_not_allowed": true,
51+
"shares_profile_capacity_invalid": true,
52+
"shares_zone_not_found": true,
53+
"shares_bad_request": true,
54+
"shares_resource_group_bad_request": true,
55+
"shares_vpc_not_found": true,
56+
"shares_not_found": true,
57+
"shares_target_not_found": true,
58+
"shares_target_one_per_vpc": true,
59+
"bad_field": true,
60+
"shares_name_duplicate": true,
61+
"shares_subnet_zone_mismatch": true,
62+
"targets_primary_ip_id_required": true,
63+
"targets_primary_ip_not_related_to_subnet": true,
64+
"InvalidArgument": true,
65+
"shares_status_pending": false,
66+
"internal_error": false,
67+
"invalid_route": false,
68+
"service_error": false,
6469
}
6570

6671
// retry ...

0 commit comments

Comments
 (0)