Skip to content

Commit 0151a07

Browse files
sameshaisameshai
committed
Review Comments
Signed-off-by: Sameer Shaikh <[email protected]>
1 parent 020525f commit 0151a07

File tree

7 files changed

+20
-785
lines changed

7 files changed

+20
-785
lines changed

‎common/messages/messages_en.go‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -191,12 +191,12 @@ var messagesEn = map[string]util.Message{
191191
RC: 404,
192192
Action: "Please verify that the start field is correct.",
193193
},
194-
"SubnetFindFailedWithZoneAndVPC": {
195-
Code: "SubnetFindFailedWithZoneAndVPC",
196-
Description: "A subnet with the specified zone '%s' and vpc '%s' could not be found.",
194+
"SubnetFindFailedWithZoneAndSubnetID": {
195+
Code: "SubnetFindFailedWithZoneAndSubnetID",
196+
Description: "A subnet with the specified zone '%s' and availble cluster subnet list '%s' could not be found.",
197197
Type: util.RetrivalFailed,
198198
RC: 404,
199-
Action: "Verify that the subnet exists. Run 'ibmcloud is subnets' to list available subnets in your account.",
199+
Action: "Verify that the subnet exists. Run 'ibmcloud is subnets' to list available subnets in your account. If there exist subnet in the respective zone, then please verify if the subnet exists in the configmap using `kubectl get cm ibm-cloud-provider-data -n kube-system -o yaml`.",
200200
},
201201
"ListVolumesFailed": {
202202
Code: "ListVolumesFailed",

‎cover.out‎

Lines changed: 0 additions & 669 deletions
This file was deleted.

‎file/provider/create_volume_access_point.go‎

Lines changed: 10 additions & 103 deletions
Original file line numberDiff line numberDiff line change
@@ -18,9 +18,6 @@
1818
package provider
1919

2020
import (
21-
"errors"
22-
"net/url"
23-
"strings"
2421
"time"
2522

2623
userError "github.com/IBM/ibmcloud-volume-file-vpc/common/messages"
@@ -53,9 +50,6 @@ func (vpcs *VPCSession) CreateVolumeAccessPoint(volumeAccessPointRequest provide
5350
var volumeAccessPointResult *models.ShareTarget
5451
var varp *provider.VolumeAccessPointResponse
5552

56-
var subnet *models.Subnet
57-
var subnetID string
58-
5953
volumeAccessPoint := models.NewShareTarget(volumeAccessPointRequest)
6054

6155
err = vpcs.APIRetry.FlexyRetry(vpcs.Logger, func() (error, bool) {
@@ -70,34 +64,18 @@ func (vpcs *VPCSession) CreateVolumeAccessPoint(volumeAccessPointRequest provide
7064
return nil, true // stop retry volume accessPoint already created
7165
}
7266

73-
// If ENI is enabled
74-
if volumeAccessPointRequest.AccessControlMode == SecurityGroupMode {
75-
volumeAccessPoint.VPC = nil
76-
volumeAccessPoint.EncryptionInTransit = volumeAccessPointRequest.EncryptionInTransit
67+
// If ENI/VNI is enabled
68+
if volumeAccessPointRequest.AccessControlMode == SecurityGroup {
69+
volumeAccessPoint.VPC = nil // We can either pass VPC or VNI
7770
volumeAccessPoint.VirtualNetworkInterface = &models.VirtualNetworkInterface{
7871
SecurityGroups: volumeAccessPointRequest.SecurityGroups,
7972
ResourceGroup: volumeAccessPointRequest.ResourceGroup,
8073
}
8174

82-
// If primaryIP.ID is provided subnet is not mandatory for rest of the cases it is mandatory
83-
if volumeAccessPointRequest.PrimaryIP == nil || len(volumeAccessPointRequest.PrimaryIP.ID) == 0 {
84-
85-
if len(volumeAccessPointRequest.SubnetID) == 0 {
86-
vpcs.Logger.Info("Getting subnet from VPC provider...")
87-
subnet, err = vpcs.getSubnet(volumeAccessPointRequest)
88-
// Return error if we dont find subnet
89-
if err != nil && subnet == nil {
90-
return err, true // stop retry
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
97-
}
98-
75+
if len(volumeAccessPointRequest.SubnetID) != 0 {
76+
vpcs.Logger.Info("Using subnet...", zap.Reflect("subnetID", volumeAccessPointRequest.SubnetID))
9977
volumeAccessPoint.VirtualNetworkInterface.Subnet = &models.SubnetRef{
100-
ID: subnetID,
78+
ID: volumeAccessPointRequest.SubnetID,
10179
}
10280
}
10381

@@ -138,83 +116,12 @@ func (vpcs *VPCSession) validateVolumeAccessPointRequest(volumeAccessPointReques
138116
return err
139117
}
140118

141-
// Check for VPC ID - required validation
119+
// Check for VPC ID, SubnetID or AccessPointID - required validation
142120
if len(volumeAccessPointRequest.VPCID) == 0 && len(volumeAccessPointRequest.SubnetID) == 0 && len(volumeAccessPointRequest.AccessPointID) == 0 {
143-
err = userError.GetUserError(string(reasoncode.ErrorRequiredFieldMissing), nil, "VPCID")
121+
err = userError.GetUserError(string(reasoncode.ErrorRequiredFieldMissing), nil, "VPCID or SubnetID or AccessPointID")
144122
vpcs.Logger.Error("One of volumeAccessPointRequest.VPCID, volumeAccessPointRequest.SubnetID and volumeAccessPointRequest.AccessPoint is required", zap.Error(err))
145123
return err
146124
}
147-
return nil
148-
}
149-
150-
// GetSubnet get the subnet based on the request
151-
func (vpcs *VPCSession) getSubnet(volumeAccessPointRequest provider.VolumeAccessPointRequest) (*models.Subnet, error) {
152-
vpcs.Logger.Debug("Entry of GetSubnet method...", zap.Reflect("volumeAccessPointRequest", volumeAccessPointRequest))
153-
defer vpcs.Logger.Debug("Exit from GetSubnet method...")
154-
var err error
155-
156-
// Get Subnet by VPC ID and zone. This is inefficient operation which requires iteration over subnet list
157-
subnet, err := vpcs.getSubnetByVPCIDAndZone(volumeAccessPointRequest)
158-
vpcs.Logger.Info("getSubnetByVPCIDAndZone response", zap.Reflect("subnet", subnet), zap.Error(err))
159-
return subnet, err
160-
}
161-
162-
func (vpcs *VPCSession) getSubnetByVPCIDAndZone(volumeAccessPointRequest provider.VolumeAccessPointRequest) (*models.Subnet, error) {
163-
vpcs.Logger.Debug("Entry of getSubnetByVPCIDAndZone()")
164-
defer vpcs.Logger.Debug("Exit from getSubnetByVPCIDAndZone()")
165-
vpcs.Logger.Info("Getting getSubnetByVPCIDAndZone from VPC provider...")
166-
var err error
167-
var subnets *models.SubnetList
168-
var start = ""
169-
170-
filters := &models.ListSubnetFilters{ResourceGroupID: volumeAccessPointRequest.ResourceGroup.ID}
171-
172-
for {
173-
174-
subnets, err = vpcs.Apiclient.FileShareService().ListSubnets(10, start, filters, vpcs.Logger)
175-
176-
if err != nil {
177-
// API call is failed
178-
userErr := userError.GetUserError("ListSubnetsFailed", err)
179-
return nil, userErr
180-
}
181-
182-
// Iterate over the subnet list for given volume
183-
if subnets != nil {
184-
subnetList := subnets.Subnets
185-
for _, subnetItem := range subnetList {
186-
// Check if VPC ID and zone name is matching with requested input
187-
if subnetItem.VPC != nil && subnetItem.VPC.ID == volumeAccessPointRequest.VPCID && subnetItem.Zone != nil && subnetItem.Zone.Name == volumeAccessPointRequest.Zone && strings.Contains(volumeAccessPointRequest.SubnetIDList, subnetItem.ID) {
188-
vpcs.Logger.Info("Successfully found subnet", zap.Reflect("subnetItem", subnetItem))
189-
return subnetItem, nil
190-
}
191-
}
192-
193-
if subnets.Next == nil {
194-
break // No more pages, exit the loop
195-
}
196-
197-
// Fetch the start of next page
198-
startUrl, err := url.Parse(subnets.Next.Href)
199-
if err != nil {
200-
// API call is failed
201-
userErr := userError.GetUserError("NextSubnetPageParsingError", err, subnets.Next.Href)
202-
return nil, userErr
203-
}
204-
205-
vpcs.Logger.Info("startUrl", zap.Reflect("startUrl", startUrl))
206-
start = startUrl.Query().Get("start") //parse query param into map
207-
if start == "" {
208-
// API call is failed
209-
userErr := userError.GetUserError("StartSubnetIDEmpty", err, startUrl)
210-
return nil, userErr
211-
}
212-
213-
}
214-
}
215125

216-
// No volume Subnet found in the list. So return error
217-
userErr := userError.GetUserError(string("SubnetFindFailedWithZoneAndVPC"), errors.New("no subnet found"), volumeAccessPointRequest.Zone, volumeAccessPointRequest.VPCID)
218-
vpcs.Logger.Error("Subnet not found", zap.Error(err))
219-
return nil, userErr
220-
}
126+
return nil
127+
}

‎file/provider/create_volume_access_point_test.go‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,6 @@ func TestCreateVolumeAccessPoint(t *testing.T) {
8080
providerVolumeAccessPointRequest: provider.VolumeAccessPointRequest{
8181
VolumeID: "volume-id1",
8282
VPCID: "VPC-id1",
83-
Zone: "test-zone",
8483
ResourceGroup: &provider.ResourceGroup{ID: "default resource group id", Name: "default resource group"},
8584
},
8685

@@ -121,7 +120,6 @@ func TestCreateVolumeAccessPoint(t *testing.T) {
121120
providerVolumeAccessPointRequest: provider.VolumeAccessPointRequest{
122121
VolumeID: "volume-id1",
123122
VPCID: "VPC-id1",
124-
Zone: "test-zone",
125123
ResourceGroup: &provider.ResourceGroup{ID: "default resource group id", Name: "default resource group"},
126124
},
127125

@@ -141,7 +139,6 @@ func TestCreateVolumeAccessPoint(t *testing.T) {
141139
providerVolumeAccessPointRequest: provider.VolumeAccessPointRequest{
142140
VolumeID: "volume-id1",
143141
VPCID: "VPC-id1",
144-
Zone: "test-zone",
145142
ResourceGroup: &provider.ResourceGroup{ID: "default resource group id", Name: "default resource group"},
146143
},
147144

‎file/provider/util.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -38,9 +38,8 @@ var retryGap = 10
3838

3939
// ConstantRetryGap ...
4040
const (
41-
ConstantRetryGap = 10 // seconds
42-
SecurityGroupMode = "security_group"
43-
VPCMode = "vpc"
41+
ConstantRetryGap = 10 // seconds
42+
SecurityGroup = "security_group"
4443
)
4544

4645
var volumeIDPartsCount = 5
@@ -61,6 +60,7 @@ var skipErrorCodes = map[string]bool{
6160
"shares_subnet_zone_mismatch": true,
6261
"targets_primary_ip_id_required": true,
6362
"targets_primary_ip_not_related_to_subnet": true,
63+
"shares_target_vpc_and_network_interface": true,
6464
"InvalidArgument": true,
6565
"shares_status_pending": false,
6666
"internal_error": false,

‎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.20230518103801-9f6369a64a54
8+
github.com/IBM/ibmcloud-volume-interface v1.1.5-0.20230601074012-3a09feec25bc
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.20230518103801-9f6369a64a54 h1:5vYb0mLIR/cn5D4DG4SrDyEFL5L1bfiAfJs/I8C7klA=
45-
github.com/IBM/ibmcloud-volume-interface v1.1.5-0.20230518103801-9f6369a64a54/go.mod h1:646HOeq8dAKbgpr7jRehGKckhgduJyII2uN5T6RDLww=
44+
github.com/IBM/ibmcloud-volume-interface v1.1.5-0.20230601074012-3a09feec25bc h1:glbW3s/T1/Q2sG9dfxqrCDiipCwZXZ67gK0UgpWjDd0=
45+
github.com/IBM/ibmcloud-volume-interface v1.1.5-0.20230601074012-3a09feec25bc/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=

0 commit comments

Comments
 (0)