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

Updated PowerMax Documentation #1450

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Updated PowerMax Documentation #1450

wants to merge 2 commits into from

Conversation

rishabhatdell
Copy link
Contributor

Description

This PR updates the priority of CSI PowerMax driver when its set to Auto

GitHub Issues

List the GitHub issues impacted by this PR:

GitHub Issue #
dell/csm#1689

Checklist:

  • Have you run a grammar and spell checks against your submission?
  • Have you tested the changes locally?
  • Have you tested whether the hyperlinks are working properly?
  • Did you add the examples wherever applicable?
  • Have you added high-resolution images?

Copy link

github-actions bot commented Feb 11, 2025

Test Results

77 tests  ±0   77 ✅ ±0   3s ⏱️ ±0s
 3 suites ±0    0 💤 ±0 
 1 files   ±0    0 ❌ ±0 

Results for commit 993bf6a. ± Comparison against base commit c3ceefc.

♻️ This comment has been updated with latest results.

@@ -670,6 +670,8 @@ This feature is also supported for limiting the volume provisioning on Kubernete

The CSI Driver for Dell PowerMax supports NVMeTCP from v2.11.0. To enable NVMe/TCP provisioning, blockProtocol in settings file should be specified as NVMETCP.

>**NOTE:** <br>If `X_CSI_TRANSPORT_PROTOCOL` is not specified in the powermax-array-config ConfigMap, the driver will detect the available initiators on the host and choose the protocol. Priority is given to NVMe/TCP, followed by FC, then iSCSI.
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we specify both i.e. AUTO and the empty value as both cases are handled in our driver code base?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think so, because here
https://github.com/dell/csi-powermax/blob/1c59cc178a2af03fe436bdd9c1ec60159de7be5f/service/service.go#L730
it just accept these values

FIBRE : Returns FC
FC : Returns FC
ISCSI : Returns ISCSI
NVMETCP : Returns NVMETCP
Empty string ("") : Returns an empty string
Invalid value (e.g., AUTO) : Logs an error and returns an empty string

Copy link
Contributor

Choose a reason for hiding this comment

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

image

Auto is a valid input or not, please refer this?

Copy link
Contributor

Choose a reason for hiding this comment

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

According to the documentation, if no value is specified, it defaults to "Auto." This means "Auto" is a valid input and should be supported. Supporting "Auto" will ensure consistency with other drivers, such as PowerStore, which also recognize "Auto" as a valid input.

image

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 it to csi-powermax and documentation

@adarsh-dell
Copy link
Contributor

Please add one screenshot of the preview.

Copy link
Contributor

@adarsh-dell adarsh-dell left a comment

Choose a reason for hiding this comment

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

LGTM

@rishabhatdell
Copy link
Contributor Author

Please add one screenshot of the preview.

image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants