Skip to content

Updated PowerMax Documentation#1450

Closed
rishabhatdell wants to merge 2 commits into
mainfrom
bugfix-1689
Closed

Updated PowerMax Documentation#1450
rishabhatdell wants to merge 2 commits into
mainfrom
bugfix-1689

Conversation

@rishabhatdell

Copy link
Copy Markdown
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 #
https://github.com/dell/csm/issues/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?

@github-actions

github-actions Bot commented Feb 11, 2025

Copy link
Copy Markdown

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.


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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
Contributor

Please add one screenshot of the preview.

@adarsh-dell adarsh-dell left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@rishabhatdell

Copy link
Copy Markdown
Contributor Author

Please add one screenshot of the preview.

image

@rajkumar-palani rajkumar-palani left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

lgtm

@sakshi-garg1

Copy link
Copy Markdown
Contributor

Raised another PR #1457 against this as it had too many conflicts.

@suryagupta4 suryagupta4 deleted the bugfix-1689 branch February 27, 2025 08:23
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.

5 participants