Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support for acadia profile #205

Merged
merged 10 commits into from
Dec 3, 2024
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ go 1.22.0
require (
github.com/IBM/ibm-csi-common v1.1.18
github.com/IBM/ibmcloud-volume-interface v1.2.6
github.com/IBM/ibmcloud-volume-vpc v1.1.11
github.com/IBM/ibmcloud-volume-vpc v1.1.12
github.com/IBM/secret-utils-lib v1.1.11
github.com/container-storage-interface/spec v1.9.0
github.com/golang/glog v1.2.1
Expand Down Expand Up @@ -51,7 +51,7 @@ require (
github.com/gofrs/uuid v4.4.0+incompatible // indirect
github.com/gogo/protobuf v1.3.2 // indirect
github.com/golang-jwt/jwt v3.2.2+incompatible // indirect
github.com/golang-jwt/jwt/v4 v4.5.0 // indirect
github.com/golang-jwt/jwt/v4 v4.5.1 // indirect
github.com/golang/groupcache v0.0.0-20210331224755-41bb18bfe9da // indirect
github.com/golang/protobuf v1.5.4 // indirect
github.com/google/gnostic-models v0.6.8 // indirect
Expand Down
8 changes: 4 additions & 4 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@ github.com/IBM/ibm-csi-common v1.1.18 h1:CbzoONFN6vdmPLoXGxygiq3sbr2xtAcVUb0Vqj3
github.com/IBM/ibm-csi-common v1.1.18/go.mod h1:bDs9CLfr09kFpSMcR35e9AmyR2pSydx8goHjagFPrHs=
github.com/IBM/ibmcloud-volume-interface v1.2.6 h1:OLumrSQ0XTOp6gW+k0z2X53uTYOIt1wWSkTCXzK/vAM=
github.com/IBM/ibmcloud-volume-interface v1.2.6/go.mod h1:sDeQiPuN8k9yTRl9FbE2GZCXPNg4cV3oldUfL8wwGNA=
github.com/IBM/ibmcloud-volume-vpc v1.1.11 h1:HnDPi7ZN+iAPVLZ29sLeQnJ7y0lvorc8/q4csDnxht4=
github.com/IBM/ibmcloud-volume-vpc v1.1.11/go.mod h1:V4Xw+ETu4nnIHn8lZ8T/tD6UlCTSDLt4kT8oEs4o80w=
github.com/IBM/ibmcloud-volume-vpc v1.1.12 h1:DSC+YfddhegnixdX3stOgsIqyKlbc5wF5ofWomyMIsI=
github.com/IBM/ibmcloud-volume-vpc v1.1.12/go.mod h1:bdVgIWQmiq4ehs4buHpMYwFh6nlIaKBOxRGJoTh6bwY=
github.com/IBM/secret-common-lib v1.1.11 h1:EpfEe1gT1bnFQ3bxQPrh6bzTPeGjUo1NReVkCCP+TOc=
github.com/IBM/secret-common-lib v1.1.11/go.mod h1:7YJF0ipT979nHIPkiCpvjFboFoIhrmYnIliE1vjCjZM=
github.com/IBM/secret-utils-lib v1.1.11 h1:w87BzkddoFFlhRuWRteuGj3/561lEUg6Oo0RajVC87A=
Expand Down Expand Up @@ -87,8 +87,8 @@ github.com/gogo/protobuf v1.3.2 h1:Ov1cvc58UF3b5XjBnZv7+opcTcQFZebYjWzi34vdm4Q=
github.com/gogo/protobuf v1.3.2/go.mod h1:P1XiOD3dCwIKUDQYPy72D8LYyHL2YPYrpS2s69NZV8Q=
github.com/golang-jwt/jwt v3.2.2+incompatible h1:IfV12K8xAKAnZqdXVzCZ+TOjboZ2keLg81eXfW3O+oY=
github.com/golang-jwt/jwt v3.2.2+incompatible/go.mod h1:8pz2t5EyA70fFQQSrl6XZXzqecmYZeUEB8OUGHkxJ+I=
github.com/golang-jwt/jwt/v4 v4.5.0 h1:7cYmW1XlMY7h7ii7UhUyChSgS5wUJEnm9uZVTGqOWzg=
github.com/golang-jwt/jwt/v4 v4.5.0/go.mod h1:m21LjoU+eqJr34lmDMbreY2eSTRJ1cv77w39/MY0Ch0=
github.com/golang-jwt/jwt/v4 v4.5.1 h1:JdqV9zKUdtaa9gdPlywC3aeoEsR681PlKC+4F5gQgeo=
github.com/golang-jwt/jwt/v4 v4.5.1/go.mod h1:m21LjoU+eqJr34lmDMbreY2eSTRJ1cv77w39/MY0Ch0=
github.com/golang/glog v0.0.0-20160126235308-23def4e6c14b/go.mod h1:SBH7ygxi8pfUlaOkMMuAQtPIUF8ecWP5IEl/CR7VP2Q=
github.com/golang/glog v1.2.1 h1:OptwRhECazUx5ix5TTWC3EZhsZEHWcYWY4FQHTIubm4=
github.com/golang/glog v1.2.1/go.mod h1:6AhwSGph0fcJtXVM/PEHPqZlFeoLxhs7/t5UDAwmO+w=
Expand Down
9 changes: 8 additions & 1 deletion pkg/ibmcsidriver/constants.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ limitations under the License.
package ibmcsidriver

import (
"github.com/IBM/ibm-csi-common/pkg/utils"
"github.com/kubernetes-sigs/ibm-vpc-block-csi-driver/config"
)

Expand Down Expand Up @@ -65,6 +66,9 @@ const (
// CustomProfile ...
CustomProfile = "custom"

// SDPProfile ...
SDPProfile = "sdp"

// ClassVersion ...
ClassVersion = "classVersion"

Expand Down Expand Up @@ -121,10 +125,13 @@ const (

// MAX_SNAPSHOT_CREATE_DELAY ... This is max timeout value for csi-snapshotter
MAX_SNAPSHOT_CREATE_DELAY = 900 //900 seconds

// MinimumSDPVolumeSizeInBytes ... This is minimum size require for sdp (acadia profile)
MinimumSDPVolumeSizeInBytes int64 = 1 * utils.GiB
)

// SupportedFS the supported FS types
var SupportedFS = []string{"ext2", "ext3", "ext4", "xfs"}

// SupportedProfile the supported profile names
var SupportedProfile = []string{"custom", "general-purpose", "5iops-tier", "10iops-tier"}
var SupportedProfile = []string{"custom", "general-purpose", "5iops-tier", "10iops-tier", "sdp"}
23 changes: 15 additions & 8 deletions pkg/ibmcsidriver/controller_helper.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,12 +33,16 @@ import (
)

// normalize the requested capacity(in GiB) to what is supported by the driver
func getRequestedCapacity(capRange *csi.CapacityRange) (int64, error) {
func getRequestedCapacity(capRange *csi.CapacityRange, profileName string) (int64, error) {
// Input is in bytes from csi
var capBytes int64
// Default case where nothing is set
if capRange == nil {
capBytes = utils.MinimumVolumeSizeInBytes
if profileName == SDPProfile { // SDP profile minimum size is 1GB
capBytes = MinimumSDPVolumeSizeInBytes
} else {
capBytes = utils.MinimumVolumeSizeInBytes // tierd and custom profile minimum size is 10 GB
}
// returns in GiB
return capBytes, nil
}
Expand All @@ -65,7 +69,8 @@ func getRequestedCapacity(capRange *csi.CapacityRange) (int64, error) {

// Limit is more than Required, but larger than Minimum. So we just set capcity to Minimum
// Too small, default
if capBytes < utils.MinimumVolumeSizeInBytes {
// If profile is SDP profile then no need to check for minimum size as the RoundUpBytes will giving minimum value as 1 GiB
if capBytes < utils.MinimumVolumeSizeInBytes && profileName != SDPProfile {
capBytes = utils.MinimumVolumeSizeInBytes
}

Expand Down Expand Up @@ -99,10 +104,12 @@ func getVolumeParameters(logger *zap.Logger, req *csi.CreateVolumeRequest, confi
var err error
volume := &provider.Volume{}
volume.Name = &req.Name
var profileName string
for key, value := range req.GetParameters() {
switch key {
case Profile:
if utils.ListContainsSubstr(SupportedProfile, value) {
profileName = value
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why we can't use volume.VPCVolume.Profile.Name instead of creating new variable?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@arahamad I had initially used that only but wanted to avoid multiple nil checks. I have now put generic nil check to avoid this. Thanks for the comments.

volume.VPCVolume.Profile = &provider.Profile{Name: value}
} else {
err = fmt.Errorf("%s:<%v> unsupported profile. Supported profiles are: %v", key, value, SupportedProfile)
Expand Down Expand Up @@ -158,7 +165,7 @@ func getVolumeParameters(logger *zap.Logger, req *csi.CreateVolumeRequest, confi
logger.Info("Ignoring storage class parameter, for backward compatibility", zap.Any("ClassParameter", Generation))

case IOPS:
// Default IOPS can be specified in Custom class
// Default IOPS can be specified in Custom or sdp class
if len(value) != 0 {
iops := value
volume.Iops = &iops
Expand All @@ -179,7 +186,7 @@ func getVolumeParameters(logger *zap.Logger, req *csi.CreateVolumeRequest, confi

// Get the requested capacity from the request
capacityRange := req.GetCapacityRange()
capBytes, err := getRequestedCapacity(capacityRange)
capBytes, err := getRequestedCapacity(capacityRange, profileName)
if err != nil {
err = fmt.Errorf("invalid PVC capacity size: '%v'", err)
logger.Error("getVolumeParameters", zap.NamedError("invalid parameter", err))
Expand Down Expand Up @@ -228,8 +235,8 @@ func getVolumeParameters(logger *zap.Logger, req *csi.CreateVolumeRequest, confi
return volume, err
}

if volume.VPCVolume.Profile != nil && volume.VPCVolume.Profile.Name != CustomProfile {
// Specify IOPS only for custom class
if volume.VPCVolume.Profile != nil && (volume.VPCVolume.Profile.Name != CustomProfile && volume.VPCVolume.Profile.Name != SDPProfile) {
// Specify IOPS only for custom or SDP class
volume.Iops = nil
}

Expand Down Expand Up @@ -302,7 +309,7 @@ func overrideParams(logger *zap.Logger, req *csi.CreateVolumeRequest, config *co
volume.Region = value
}
case IOPS:
// Override IOPS only for custom class
// Override IOPS only for custom or sdp class
if len(value) != 0 {
iops := value
volume.Iops = &iops
Expand Down
23 changes: 22 additions & 1 deletion pkg/ibmcsidriver/controller_helper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,54 +40,75 @@ func TestGetRequestedCapacity(t *testing.T) {
testCases := []struct {
testCaseName string
capRange *csi.CapacityRange
profileName string
expectedValue int64
expectedError error
}{
{
testCaseName: "Check minimum size supported by volume provider in case of nil passed as input for sdp profile",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please add negative tests as well

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added few cases covering nil but we cannot pass negative value to this parameter. IDeally this will come from the k8s parsing form PVC.It is not allowed

capRange: nil,
profileName: "sdp",
expectedValue: MinimumSDPVolumeSizeInBytes,
expectedError: nil,
},
{
testCaseName: "Check minimum size supported by volume provider in case of lower value passed as input for sdp profile",
capRange: &csi.CapacityRange{RequiredBytes: 1024},
profileName: "sdp",
expectedValue: MinimumSDPVolumeSizeInBytes,
expectedError: nil,
},
{
testCaseName: "Check minimum size supported by volume provider in case of nil passed as input",
capRange: &csi.CapacityRange{},
profileName: "custom",
expectedValue: utils.MinimumVolumeSizeInBytes,
expectedError: nil,
},
{
testCaseName: "Capacity range is nil",
capRange: nil,
profileName: "general-purpose",
expectedValue: utils.MinimumVolumeSizeInBytes,
expectedError: nil,
},
{
testCaseName: "Check minimum size supported by volume provider",
capRange: &csi.CapacityRange{RequiredBytes: 1024,
LimitBytes: utils.MinimumVolumeSizeInBytes},
profileName: "custom",
expectedValue: utils.MinimumVolumeSizeInBytes,
expectedError: nil,
},
{
testCaseName: "Check size passed as actual value",
capRange: &csi.CapacityRange{RequiredBytes: 11811160064,
LimitBytes: utils.MinimumVolumeSizeInBytes + utils.MinimumVolumeSizeInBytes}, // MinimumVolumeSizeInBytes->10737418240
profileName: "custom",
expectedValue: 11811160064,
expectedError: nil,
},
{
testCaseName: "Expected error check-success",
capRange: &csi.CapacityRange{RequiredBytes: 1073741824 * 30,
LimitBytes: utils.MinimumVolumeSizeInBytes}, // MinimumVolumeSizeInBytes->10737418240
profileName: "custom",
expectedValue: 0,
expectedError: fmt.Errorf("limit bytes %v is less than required bytes %v", utils.MinimumVolumeSizeInBytes, 1073741824*30),
},
{
testCaseName: "Expected error check against limit byte-success",
capRange: &csi.CapacityRange{RequiredBytes: utils.MinimumVolumeSizeInBytes - 100,
LimitBytes: 10737418230}, // MinimumVolumeSizeInBytes->10737418240
profileName: "custom",
expectedValue: 0,
expectedError: fmt.Errorf("limit bytes %v is less than minimum volume size: %v", 10737418230, utils.MinimumVolumeSizeInBytes),
},
}

for _, testcase := range testCases {
t.Run(testcase.testCaseName, func(t *testing.T) {
sizeCap, err := getRequestedCapacity(testcase.capRange)
sizeCap, err := getRequestedCapacity(testcase.capRange, testcase.profileName)
if testcase.expectedError != nil {
assert.Equal(t, err, testcase.expectedError)
} else {
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

41 changes: 20 additions & 21 deletions vendor/github.com/golang-jwt/jwt/v4/parser.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions vendor/modules.txt
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ github.com/IBM/ibmcloud-volume-interface/lib/utils/reasoncode
github.com/IBM/ibmcloud-volume-interface/provider/auth
github.com/IBM/ibmcloud-volume-interface/provider/iam
github.com/IBM/ibmcloud-volume-interface/provider/local
# github.com/IBM/ibmcloud-volume-vpc v1.1.11
# github.com/IBM/ibmcloud-volume-vpc v1.1.12
## explicit; go 1.22.0
github.com/IBM/ibmcloud-volume-vpc/block/provider
github.com/IBM/ibmcloud-volume-vpc/block/utils
Expand Down Expand Up @@ -135,7 +135,7 @@ github.com/gogo/protobuf/sortkeys
# github.com/golang-jwt/jwt v3.2.2+incompatible
## explicit
github.com/golang-jwt/jwt
# github.com/golang-jwt/jwt/v4 v4.5.0
# github.com/golang-jwt/jwt/v4 v4.5.1
## explicit; go 1.16
github.com/golang-jwt/jwt/v4
# github.com/golang/glog v1.2.1
Expand Down