Skip to content

Commit ef79bda

Browse files
resourceGroupID in VSC params (#244)
* RG in VSC params Signed-off-by: Mahantesh-R <mahantesh.r@ibm.com> * snapshot param changes Signed-off-by: Mahantesh-R <mahantesh.r@ibm.com> * vol vpc changes Signed-off-by: Mahantesh-R <mahantesh.r@ibm.com> * vpc module changes pull Signed-off-by: Mahantesh-R <mahantesh.r@ibm.com> * vol vpc RG param name change Signed-off-by: Mahantesh-R <mahantesh.r@ibm.com> * remove exec file Signed-off-by: Mahantesh-R <mahantesh.r@ibm.com> * renames var Signed-off-by: Mahantesh-R <mahantesh.r@ibm.com> * adds RG validation for snapshot and cleaning up Signed-off-by: Mahantesh-R <mahantesh.r@ibm.com> * refactors changes Signed-off-by: Mahantesh-R <mahantesh.r@ibm.com> * adds unit test for getResourceGroup Signed-off-by: Mahantesh-R <mahantesh.r@ibm.com> * passes rgID for snapshot get by name Signed-off-by: Mahantesh-R <mahantesh.r@ibm.com> * rem rg len check Signed-off-by: Mahantesh-R <mahantesh.r@ibm.com> * removes logger Signed-off-by: Mahantesh-R <mahantesh.r@ibm.com> * handled error case in get vol Signed-off-by: Mahantesh-R <mahantesh.r@ibm.com> * Update the interface and library version * Update the interface and library version * Update the interface and library version --------- Signed-off-by: Mahantesh-R <mahantesh.r@ibm.com> Co-authored-by: Sameer Shaikh <sameer.shaikh@ibm.com>
1 parent eb45013 commit ef79bda

File tree

14 files changed

+230
-81
lines changed

14 files changed

+230
-81
lines changed

go.mod

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,8 @@ go 1.25.3
44

55
require (
66
github.com/IBM/ibm-csi-common v1.1.24
7-
github.com/IBM/ibmcloud-volume-interface v1.2.18
8-
github.com/IBM/ibmcloud-volume-vpc v1.1.21
7+
github.com/IBM/ibmcloud-volume-interface v1.2.19
8+
github.com/IBM/ibmcloud-volume-vpc v1.1.22
99
github.com/IBM/secret-utils-lib v1.1.15
1010
github.com/container-storage-interface/spec v1.11.0
1111
github.com/golang/glog v1.2.4

go.sum

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,10 +8,10 @@ github.com/IBM/go-sdk-core/v5 v5.17.4 h1:VGb9+mRrnS2HpHZFM5hy4J6ppIWnwNrw0G+tLSg
88
github.com/IBM/go-sdk-core/v5 v5.17.4/go.mod h1:KsAAI7eStAWwQa4F96MLy+whYSh39JzNjklZRbN/8ns=
99
github.com/IBM/ibm-csi-common v1.1.24 h1:uHgWTtXcQO7FxEEoRpBcLUD1yNFmdLi4mEC5zZdPFXw=
1010
github.com/IBM/ibm-csi-common v1.1.24/go.mod h1:QW28zxMAqx8O1iMK57rK/C4CBnLuAol+0mNDZ3asOA8=
11-
github.com/IBM/ibmcloud-volume-interface v1.2.18 h1:JwoZe4mP7GsNoAywagf48h/65FXHi2NKn9SLD6ZYiIo=
12-
github.com/IBM/ibmcloud-volume-interface v1.2.18/go.mod h1:2gOMNp05rLXkfWtL9wvPHuQph2kFFwbczPpFFThAy38=
13-
github.com/IBM/ibmcloud-volume-vpc v1.1.21 h1:jRziFUPS25S+/hhuj0GPt1y+jvUz2Yy/apsLGrVlHRU=
14-
github.com/IBM/ibmcloud-volume-vpc v1.1.21/go.mod h1:Zrlx5rr2r+dg2Iz+YfZ5dpFWGkQdIeauYHG9xZyEExI=
11+
github.com/IBM/ibmcloud-volume-interface v1.2.19 h1:Ud95Y1XlgkEz14im3IAUsJNTpKde+I7S2vqIW7JR4iQ=
12+
github.com/IBM/ibmcloud-volume-interface v1.2.19/go.mod h1:2gOMNp05rLXkfWtL9wvPHuQph2kFFwbczPpFFThAy38=
13+
github.com/IBM/ibmcloud-volume-vpc v1.1.22 h1:O/h4z+DQejiYEAFXBhJCsWS+GZC1Q/0WolU2f+d+LgA=
14+
github.com/IBM/ibmcloud-volume-vpc v1.1.22/go.mod h1:ld8mHYUbbJhPzZ/mOiY6as/lTGcJ61j+/mh83ceZAj4=
1515
github.com/IBM/secret-common-lib v1.1.14 h1:EdEFOdhYrDRc2+2u9SLQznUnbnDDq4f9RHx0eAuFd+E=
1616
github.com/IBM/secret-common-lib v1.1.14/go.mod h1:bSCfMHVgnl31H0CDxz5qxlHH7dtcDxTU703TniZzAsw=
1717
github.com/IBM/secret-utils-lib v1.1.15 h1:wW6/TdsjQVz/WDg7yQjD8Jmvb2NqYE851pDYxJVOo3s=

pkg/ibmcsidriver/controller.go

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -467,6 +467,8 @@ func (csiCS *CSIControllerServer) CreateSnapshot(ctx context.Context, req *csi.C
467467
return nil, commonError.GetCSIError(ctxLogger, commonError.MissingSourceVolumeID, requestID, nil)
468468
}
469469

470+
resourceGroupID := getResourceGroup(ctxLogger, req.GetParameters(), csiCS.CSIProvider.GetConfig())
471+
470472
// Validate if volume Already Exists
471473
session, err := csiCS.CSIProvider.GetProviderSession(ctx, ctxLogger)
472474
if err != nil {
@@ -479,20 +481,26 @@ func (csiCS *CSIControllerServer) CreateSnapshot(ctx context.Context, req *csi.C
479481
return nil, commonError.GetCSIError(ctxLogger, commonError.InternalError, requestID, err)
480482
}
481483

482-
snapshot, err := session.GetSnapshotByName(snapshotName)
484+
snapshot, err := session.GetSnapshotByName(snapshotName, resourceGroupID)
485+
if err != nil {
486+
return nil, commonError.GetCSIError(ctxLogger, commonError.ListSnapshotsFailed, requestID, err)
487+
}
488+
483489
if snapshot != nil {
484490
if snapshot.VolumeID != sourceVolumeID {
485491
return nil, commonError.GetCSIError(ctxLogger, commonError.SnapshotAlreadyExists, requestID, err, snapshotName, sourceVolumeID)
486492
}
487493
ctxLogger.Info("Snapshot with name already exist for volume", zap.Reflect("SnapshotName", snapshotName), zap.Reflect("VolumeID", sourceVolumeID))
488494
return createCSISnapshotResponse(*snapshot), nil
489495
}
496+
490497
snapshotParameters := provider.SnapshotParameters{}
491498
snapshotParameters.Name = snapshotName
492499
snapshotTags := map[string]string{
493500
"name": snapshotName,
494501
}
495502
snapshotParameters.SnapshotTags = snapshotTags
503+
snapshotParameters.ResourceGroup = resourceGroupID
496504

497505
snapshot, err = session.CreateSnapshot(sourceVolumeID, snapshotParameters)
498506

pkg/ibmcsidriver/controller_helper.go

Lines changed: 77 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -97,61 +97,63 @@ func areVolumeCapabilitiesSupported(volCaps []*csi.VolumeCapability, driverVolum
9797
return allSupported
9898
}
9999

100-
// getVolumeParameters this function get the parameters from storage class, this also validate
101-
// all parameters passed in storage class or not which are mandatory.
100+
// getVolumeParameters gets the parameters from storage class and validates them
102101
func getVolumeParameters(logger *zap.Logger, req *csi.CreateVolumeRequest, config *config.Config) (*provider.Volume, error) {
103102
var encrypt = "undef"
104-
var err error
105-
volume := &provider.Volume{}
106-
volume.Name = &req.Name
103+
volume := &provider.Volume{Name: &req.Name}
104+
105+
// Process storage class parameters
107106
for key, value := range req.GetParameters() {
107+
var err error
108108
switch key {
109109
case Profile:
110110
if utils.ListContainsSubstr(SupportedProfile, value) {
111111
volume.Profile = &provider.Profile{Name: value}
112112
} else {
113113
err = fmt.Errorf("%s:<%v> unsupported profile. Supported profiles are: %v", key, value, SupportedProfile)
114114
}
115+
115116
case Zone:
116117
if len(value) > ZoneNameMaxLen {
117118
err = fmt.Errorf("%s:<%v> exceeds %d chars", key, value, ZoneNameMaxLen)
118119
} else {
119120
volume.Az = value
120121
}
122+
121123
case Region:
122124
if len(value) > RegionMaxLen {
123125
err = fmt.Errorf("%s:<%v> exceeds %d chars", key, value, RegionMaxLen)
124126
} else {
125127
volume.Region = value
126128
}
129+
127130
case Tag:
128131
if len(value) != 0 {
129-
tagstr := strings.TrimSpace(value)
130-
volume.Tags = strings.Split(tagstr, ",")
132+
volume.Tags = strings.Split(strings.TrimSpace(value), ",")
131133
}
132134

133135
case ResourceGroup:
134136
if len(value) > ResourceGroupIDMaxLen {
135137
err = fmt.Errorf("%s:<%v> exceeds %d chars", key, value, ResourceGroupIDMaxLen)
138+
} else {
139+
volume.ResourceGroup = &provider.ResourceGroup{ID: value}
136140
}
137-
volume.ResourceGroup = &provider.ResourceGroup{ID: value}
138141

139142
case BillingType:
140-
// Its not supported by RIaaS, but this is just information for the user
143+
// Not supported by RIaaS, informational only
141144

142145
case Encrypted:
143146
if value != TrueStr && value != FalseStr {
144147
err = fmt.Errorf("'<%v>' is invalid, value of '%s' should be [true|false]", value, key)
145148
} else {
146149
encrypt = value
147150
}
151+
148152
case EncryptionKey:
149153
if len(value) > EncryptionKeyMaxLen {
150154
err = fmt.Errorf("%s: exceeds %d bytes", key, EncryptionKeyMaxLen)
151-
} else {
152-
if len(value) != 0 {
153-
volume.VolumeEncryptionKey = &provider.VolumeEncryptionKey{CRN: value}
154-
}
155+
} else if len(value) != 0 {
156+
volume.VolumeEncryptionKey = &provider.VolumeEncryptionKey{CRN: value}
155157
}
156158

157159
case ClassVersion:
@@ -168,35 +170,40 @@ func getVolumeParameters(logger *zap.Logger, req *csi.CreateVolumeRequest, confi
168170
iops := value
169171
volume.Iops = &iops
170172
}
171-
case Throughput: // getting throughput value from storage class if it is provided
173+
174+
case Throughput:
172175
if len(value) != 0 {
173-
bandwidth, errParse := strconv.ParseInt(value, 10, 32)
174-
if errParse != nil {
176+
bandwidth, parseErr := strconv.ParseInt(value, 10, 32)
177+
if parseErr != nil {
175178
err = fmt.Errorf("'<%v>' is invalid, value of '%s' should be an int32 type", value, key)
176179
} else {
177180
volume.Bandwidth = int32(bandwidth)
178181
}
179182
}
183+
180184
default:
181185
err = fmt.Errorf("<%s> is an invalid parameter", key)
182186
}
187+
183188
if err != nil {
184189
logger.Error("getVolumeParameters", zap.NamedError("SC Parameters", err))
185190
return volume, err
186191
}
187192
}
188-
// If encripted is set to false
193+
194+
// If encrypted is set to false, clear encryption key
189195
if encrypt == FalseStr {
190196
volume.VolumeEncryptionKey = nil
191197
}
192198

199+
// Validate required profile
193200
if volume.Profile == nil {
194-
err = fmt.Errorf("volume profile is empty, you need to pass valid profile name")
201+
err := fmt.Errorf("volume profile is empty, you need to pass valid profile name")
195202
logger.Error("getVolumeParameters", zap.NamedError("InvalidRequest", err))
196203
return volume, err
197204
}
198205

199-
// Get the requested capacity from the request
206+
// Get requested capacity
200207
capacityRange := req.GetCapacityRange()
201208
capBytes, err := getRequestedCapacity(capacityRange, volume.Profile.Name)
202209
if err != nil {
@@ -206,23 +213,23 @@ func getVolumeParameters(logger *zap.Logger, req *csi.CreateVolumeRequest, confi
206213
}
207214
logger.Info("Volume size in bytes", zap.Any("capacity", capBytes))
208215

209-
// Convert size/capacity in GiB, as this is needed by RIaaS
216+
// Convert capacity to GiB
210217
fsSize := utils.BytesToGiB(capBytes)
211218
// Assign the size to volume object
212219
volume.Capacity = &fsSize
213220
logger.Info("Volume size in GiB", zap.Any("capacity", fsSize))
214221

215-
// volume.Capacity should be set before calling overrideParams
222+
// Override with secret parameters (volume.Capacity must be set before this)
216223
err = overrideParams(logger, req, config, volume)
217224
if err != nil {
218225
return volume, err
219226
}
220227

221-
// Check if the provided fstype is supported one
228+
// Validate and set filesystem type
222229
volumeCapabilities := req.GetVolumeCapabilities()
223230
if volumeCapabilities == nil {
224231
err = fmt.Errorf("volume capabilities are empty")
225-
logger.Error("overrideParams", zap.NamedError("invalid parameter", err))
232+
logger.Error("getVolumeParameters", zap.NamedError("invalid parameter", err))
226233
return volume, err
227234
}
228235

@@ -231,28 +238,26 @@ func getVolumeParameters(logger *zap.Logger, req *csi.CreateVolumeRequest, confi
231238
if mnt == nil {
232239
continue
233240
}
241+
234242
if len(mnt.FsType) == 0 {
235243
volume.VolumeType = provider.VolumeType(defaultFsType)
244+
} else if utils.ListContainsSubstr(SupportedFS, mnt.FsType) {
245+
volume.VolumeType = provider.VolumeType(mnt.FsType)
236246
} else {
237-
if utils.ListContainsSubstr(SupportedFS, mnt.FsType) {
238-
volume.VolumeType = provider.VolumeType(mnt.FsType)
239-
} else {
240-
err = fmt.Errorf("unsupported fstype <%s>. Supported types: %v", mnt.FsType, SupportedFS)
241-
}
247+
err = fmt.Errorf("unsupported fstype <%s>. Supported types: %v", mnt.FsType, SupportedFS)
248+
249+
logger.Error("getVolumeParameters", zap.NamedError("InvalidParameter", err))
250+
return volume, err
242251
}
243252
break
244253
}
245-
if err != nil {
246-
logger.Error("getVolumeParameters", zap.NamedError("InvalidParameter", err))
247-
return volume, err
248-
}
249254

250-
if volume.Profile != nil && (volume.Profile.Name != CustomProfile && volume.Profile.Name != SDPProfile) {
251-
// Specify IOPS only for custom or SDP class
255+
// Clear IOPS for non-custom/SDP profiles
256+
if volume.Profile != nil && volume.Profile.Name != CustomProfile && volume.Profile.Name != SDPProfile {
252257
volume.Iops = nil
253258
}
254259

255-
//If zone not provided in storage class parameters then we pick from the Topology
260+
// Set zone from topology if not specified
256261
if len(strings.TrimSpace(volume.Az)) == 0 {
257262
zones, err := pickTargetTopologyParams(req.GetAccessibilityRequirements())
258263
if err != nil {
@@ -261,95 +266,105 @@ func getVolumeParameters(logger *zap.Logger, req *csi.CreateVolumeRequest, confi
261266
return volume, err
262267
}
263268
volume.Az = zones[utils.NodeZoneLabel]
264-
265269
}
266270

267271
return volume, nil
268272
}
269273

274+
// overrideParams overrides volume parameters with values from secrets and sets default resource group
270275
func overrideParams(logger *zap.Logger, req *csi.CreateVolumeRequest, config *config.Config, volume *provider.Volume) error {
271276
var encrypt = "undef"
272-
var err error
277+
273278
if volume == nil {
274279
return fmt.Errorf("invalid volume parameter")
275280
}
276281

282+
// Process secret parameters
277283
for key, value := range req.GetSecrets() {
284+
var err error
278285
switch key {
279286
case ResourceGroup:
280287
if len(value) > ResourceGroupIDMaxLen {
281-
err = fmt.Errorf("%s:<%v> exceeds %d bytes ", key, value, ResourceGroupIDMaxLen)
288+
err = fmt.Errorf("%s:<%v> exceeds %d bytes", key, value, ResourceGroupIDMaxLen)
282289
} else {
283290
logger.Info("override", zap.Any(ResourceGroup, value))
284291
volume.ResourceGroup = &provider.ResourceGroup{ID: value}
285292
}
293+
286294
case Encrypted:
287295
if value != TrueStr && value != FalseStr {
288296
err = fmt.Errorf("<%v> is invalid, value for '%s' should be [true|false]", value, key)
289297
} else {
290298
logger.Info("override", zap.Any(Encrypted, value))
291299
encrypt = value
292300
}
301+
293302
case EncryptionKey:
294303
if len(value) > EncryptionKeyMaxLen {
295304
err = fmt.Errorf("%s exceeds %d bytes", key, EncryptionKeyMaxLen)
296-
} else {
297-
if len(value) != 0 {
298-
logger.Info("override", zap.String("parameter", EncryptionKey))
299-
volume.VolumeEncryptionKey = &provider.VolumeEncryptionKey{CRN: value}
300-
}
305+
} else if len(value) != 0 {
306+
logger.Info("override", zap.String("parameter", EncryptionKey))
307+
volume.VolumeEncryptionKey = &provider.VolumeEncryptionKey{CRN: value}
301308
}
309+
302310
case Tag:
303311
if len(value) != 0 {
304312
logger.Info("append", zap.Any(Tag, value))
305-
tagstr := strings.TrimSpace(value)
306-
secretTags := strings.Split(tagstr, ",")
313+
secretTags := strings.Split(strings.TrimSpace(value), ",")
307314
volume.Tags = append(volume.Tags, secretTags...)
308315
}
309316

310317
case Zone:
311318
if len(value) > ZoneNameMaxLen {
312319
err = fmt.Errorf("%s:<%v> exceeds %d chars", key, value, ZoneNameMaxLen)
313320
} else {
314-
logger.Info("override", zap.Any(Zone, value))
315321
volume.Az = value
316322
}
323+
317324
case Region:
318325
if len(value) > RegionMaxLen {
319326
err = fmt.Errorf("%s:<%v> exceeds %d chars", key, value, RegionMaxLen)
320327
} else {
321328
volume.Region = value
322329
}
330+
323331
case IOPS:
324332
// Override IOPS only for custom or sdp class
325333
if len(value) != 0 {
326334
iops := value
327335
volume.Iops = &iops
328336
}
329-
case Throughput: // getting throughput value from storage class if it is provided
337+
338+
case Throughput:
330339
if len(value) != 0 {
331-
bandwidth, errParse := strconv.ParseInt(value, 10, 32)
332-
if errParse != nil {
340+
bandwidth, parseErr := strconv.ParseInt(value, 10, 32)
341+
if parseErr != nil {
333342
err = fmt.Errorf("'<%v>' is invalid, value of '%s' should be an int32 type", value, key)
334343
} else {
335344
volume.Bandwidth = int32(bandwidth)
336345
}
337346
}
347+
338348
default:
339349
err = fmt.Errorf("<%s> is an invalid parameter", key)
340350
}
351+
341352
if err != nil {
342353
logger.Error("overrideParams", zap.NamedError("Secret Parameters", err))
343354
return err
344355
}
345356
}
346-
// Assign ResourceGroupID from config
357+
358+
// Set cluster's resource group if not specified
347359
if volume.ResourceGroup == nil || len(volume.ResourceGroup.ID) < 1 {
348360
volume.ResourceGroup = &provider.ResourceGroup{ID: config.VPC.G2ResourceGroupID}
349361
}
362+
363+
// If encrypted is set to false, clear encryption key
350364
if encrypt == FalseStr {
351365
volume.VolumeEncryptionKey = nil
352366
}
367+
353368
return nil
354369
}
355370

@@ -450,6 +465,18 @@ func getAccountID(input string) string {
450465
}
451466
}
452467

468+
func getResourceGroup(logger *zap.Logger, snapshotParameters map[string]string, config *config.Config) string {
469+
if rg, ok := snapshotParameters[ResourceGroup]; ok && len(strings.TrimSpace(rg)) > 0 {
470+
logger.Info("Using resource group from VolumeSnapshotClass", zap.String("resourceGroup", rg))
471+
return rg
472+
473+
}
474+
475+
// return cluster's resource group
476+
logger.Info("Using cluster's resource group", zap.String("resourceGroup", config.VPC.G2ResourceGroupID))
477+
return config.VPC.G2ResourceGroupID
478+
}
479+
453480
// getSnapshotAndAccountIDsFromCRN ...
454481
func getSnapshotAndAccountIDsFromCRN(crn string) (string, string) {
455482
// This method will be able to handle either crn is actual crn or caller passed snapshot ID also

0 commit comments

Comments
 (0)