-
Notifications
You must be signed in to change notification settings - Fork 21
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
Fix Node Driver Crash Caused by Auto-Select Protocol #421
base: main
Are you sure you want to change the base?
Conversation
service/features/service.feature
Outdated
@@ -735,7 +738,7 @@ Feature: PowerMax CSI interface | |||
Then 0 nvmetcp targets are returned | |||
|
|||
@v1.4.0 | |||
Scenario: Validate nodeHostSetup with temporary failure | |||
Scenario: Validate nodeHostSetup with temporary fc failure |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fc -> FC ?
@v2.13.0 | ||
Scenario: Validate nodeHostSetup with temporary nvme failure | ||
Given a PowerMax service | ||
And I set transport protocol to "NVME" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
NVMe and iSCSI ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Applicable in all other files as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Switch case has this:
csi-powermax/service/step_defs_test.go
Line 3691 in 66cd85c
case "NVME": |
csi-powermax/service/step_defs_test.go
Line 3687 in 66cd85c
case "ISCSI": |
@rishabhatdell @sakshi-garg1 , have we documented it properly in csm-doc that what is the priority order in case of Auto mode selection? If yes, please share the link of that section, would like to review that section to make sure that doc aligns with the code logic written here. |
Please update the copyright year as well for all files that are being modified in this PR. |
Added: dell/csm-docs#1450 |
switch tp { | ||
case "FIBRE": | ||
return "FC" | ||
case "FC", "ISCSI", "NVMETCP", "": |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is nice.
Description
This PR addresses the issue of the node pod crashing when the X_CSI_TRANSPORT_PROTOCOL is empty in the powermax-array-config. The fix ensures that when the protocol is unspecified, the driver will prioritize available protocols in the following order:
The driver will automatically select the first available and working protocol from this list, establishing a connection based on these preferences.
GitHub Issues
List the GitHub issues impacted by this PR:
Checklist:
How Has This Been Tested?
Please describe the tests that you ran to verify your changes. Please also list any relevant details for your test configuration