Skip to content

vpc acadia profile support #221

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 5 commits into from
Apr 21, 2025
Merged

vpc acadia profile support #221

merged 5 commits into from
Apr 21, 2025

Conversation

arahamad
Copy link
Contributor

@arahamad arahamad commented Apr 21, 2025 •
edited
Loading

new vpc profile sdp support

arahamad added 5 commits April 18, 2025 22:48
@arahamad
Added acadia block support with bandwidth support in volume
7e62b8b 
Signed-off-by: Arashad Ahamad <[email protected]>
@arahamad
Updated variable name
a8bb40f 
Signed-off-by: Arashad Ahamad <[email protected]>
@arahamad
Updated iops value
3591267 
Signed-off-by: Arashad Ahamad <[email protected]>
@arahamad
fixed unit tests
c3d959e 
Signed-off-by: Arashad Ahamad <[email protected]>
@arahamad
updated lib version and fix fmt
5946014 
Signed-off-by: Arashad Ahamad <[email protected]>
@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Apr 21, 2025
@k8s-ci-robot k8s-ci-robot added approved Indicates a PR has been approved by an approver from all required OWNERS files. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Apr 21, 2025
@coveralls
Copy link

Coverage Status

coverage: 79.495% (-0.3%) from 79.805%
when pulling 5946014 on acadia
into a1a6dbc on master.

@arahamad
Copy link
Contributor Author

Following tests has been done:

  • PVC Creation/writing/deletion/expansion/snapshot working fine for sdp profile based class
  • VPC creation/writing/deletion/expansion/snapshot Verified for tier one profile based storage class

sameshai
sameshai reviewed Apr 21, 2025
View reviewed changes
pkg/ibmcsidriver/constants.go
@@ -128,6 +128,9 @@ const (

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

// Throughput ...
Throughput = "throughput"
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need to call throughput on CSI side and bandwidht on RIAAS side ? Do we need to keep it same ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@sameshai , actually I align with volumeattributeclass and what k8s talk about so it looks we need to have this mapping as k8s always talk about throughput

Copy link
Contributor

Choose a reason for hiding this comment

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

ok

sameshai
sameshai reviewed Apr 21, 2025
View reviewed changes
vendor/github.com/IBM/ibmcloud-volume-interface/lib/utils/messages.go
@@ -39,5 +42,9 @@ func (msg Message) Error() string {

// Info ...
func (msg Message) Info() string {
return fmt.Sprintf("{Code:%s, Type:%s, Description:%s, BackendError:%s, RC:%d}", msg.Code, msg.Type, msg.Description, msg.BackendError, msg.RC)
Copy link
Contributor

Choose a reason for hiding this comment

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

Test this for couple of errors like invalid iops , throughput and see the format of error message your are getting. Test 2 scenarios one actual RIAAS error and other is the error from Driver itself.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@sameshai I verified both tests, here is the results
Invalid iops:

 Warning  ProvisioningFailed    68s                 vpc.block.csi.ibm.io_ibm-vpc-block-csi-controller-6d55756ddf-pg8bh_7ecb9acb-25a6-423c-9b2e-3e111a2357e5  failed to provision volume with StorageClass "ibmc-vpc-block-sdp": rpc error: code = Internal desc = {RequestID: c12e52e9-6a96-43f5-84cb-6e84cc33c987 , Code: InternalError, Description: Internal error occurred%!(EXTRA string=creation), BackendError: {Trace Code:c12e52e9-6a96-43f5-84cb-6e84cc33c987, The volume profile specified in the request is not valid for the provided capacity and/or IOPS. Please check .Failed to create volume with the storage provider}, Action: Please check 'BackendError' tag for more details}

Invalid throughput/bandwidth:

 Warning  ProvisioningFailed    58s                 vpc.block.csi.ibm.io_ibm-vpc-block-csi-controller-6d55756ddf-pg8bh_7ecb9acb-25a6-423c-9b2e-3e111a2357e5  failed to provision volume with StorageClass "ibmc-vpc-block-sdp": rpc error: code = Internal desc = {RequestID: a2326fa7-d1ff-46a7-8ccf-1964fe3e023d , Code: InternalError, Description: Internal error occurred%!(EXTRA string=creation), BackendError: {Trace Code:a2326fa7-d1ff-46a7-8ccf-1964fe3e023d, The volume profile specified in the request is not valid for the provided capacity and/or Bandwidth. Please check .Failed to create volume with the storage provider}, Action: Please check 'BackendError' tag for more details}

sameshai
sameshai requested changes Apr 21, 2025
View reviewed changes
Copy link
Contributor

@sameshai sameshai left a comment

Choose a reason for hiding this comment

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

Post the sample error messages we get today as we are including new interface changes but not the csi common

prankulmahajan
prankulmahajan approved these changes Apr 21, 2025
View reviewed changes
Copy link
Member

@prankulmahajan prankulmahajan left a comment

Choose a reason for hiding this comment

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

lgtm

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: arahamad, prankulmahajan

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@prankulmahajan
Copy link
Member

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Apr 21, 2025
@k8s-ci-robot k8s-ci-robot merged commit b0c9ebd into master Apr 21, 2025
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sameshai sameshai sameshai requested changes

@prankulmahajan prankulmahajan prankulmahajan approved these changes

@ambiknai ambiknai Awaiting requested review from ambiknai

@bradtopol bradtopol Awaiting requested review from bradtopol

Assignees

@prankulmahajan prankulmahajan

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@arahamad @coveralls @k8s-ci-robot @prankulmahajan @sameshai