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

OSDOCS-12804#Increase vSphere max vols per node #91472

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

lpettyjo
Copy link
Contributor

@lpettyjo lpettyjo commented Apr 1, 2025

Sorry, something went wrong.

@openshift-ci openshift-ci bot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Apr 1, 2025
@lpettyjo lpettyjo added this to the Planned for 4.19 GA milestone Apr 1, 2025
@lpettyjo lpettyjo added the peer-review-needed Signifies that the peer review team needs to review this PR label Apr 1, 2025
@ocpdocs-previewbot
Copy link

Copy link

openshift-ci bot commented Apr 1, 2025

@lpettyjo: all tests passed!

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

@stevsmit stevsmit added the peer-review-in-progress Signifies that the peer review team is reviewing this PR label Apr 1, 2025
Copy link
Member

@stevsmit stevsmit left a comment

Choose a reason for hiding this comment

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

Looks good. Just a few things to consider.

.Limitations
* You must be running VMware vSphere version 8 or later.
* You must have an homogeneous vSphere 8 environment that only contains ESXi 8 hypervisors. Heterogeneous environment that contain a mix of ESXi 7 and 8 are not allowed.
Copy link
Member

Choose a reason for hiding this comment

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

Should environment be plural here?

Suggested change
* You must have an homogeneous vSphere 8 environment that only contains ESXi 8 hypervisors. Heterogeneous environment that contain a mix of ESXi 7 and 8 are not allowed.
* You must have an homogeneous vSphere 8 environment that only contains ESXi 8 hypervisors. Heterogeneous environments that contain a mix of ESXi 7 and 8 are not allowed.

However, for vSphere version 8 or later, you can increase the allowable number of volumes per node to a maximum of 255. Otherwise, the default value remains at 59.

.Limitations
* You must be running VMware vSphere version 8 or later.
Copy link
Member

Choose a reason for hiding this comment

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

Just food for thought: is this really a limitation? Or a prerequisite?

I do think that the next bullet point is a limitation. 👍

= Increasing the maximum allowable volumes per node for vSphere

.Prerequisites
* Access to the {product-title} web console.
Copy link
Member

Choose a reason for hiding this comment

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

If you find that having "* You must be running VMware vSphere version 8 or later." is more of a prerequisite, you could move it here.

.Procedure

To increase the maximum number of volumes per node for vSphere:
Copy link
Member

Choose a reason for hiding this comment

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

Could this be moved to line 9 and revised as maybe something like:

Suggested change
To increase the maximum number of volumes per node for vSphere:
Use the following procedure to increase the maximum number of volumes per node for vSphere.

That way we have an introductory sentence under the heading.

include::modules/persistent-storage-csi-vsphere-increase-max-vols-per-node-overview.adoc[leveloffset=+1]

:FeatureName: Increasing volumes per node
include::snippets/technology-preview.adoc[leveloffset=+2]
Copy link
Member

Choose a reason for hiding this comment

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

I'm hesitant to say that you should create a "Limitations" module for 1-2 limitations, but I can't help but wonder if the snippet would look better under the overview, and not under "Limitations." I guess TP could technically be a limitation, though (even though having it listed under "Limitations" might not be your intention).

@stevsmit stevsmit added peer-review-done Signifies that the peer review team has reviewed this PR and removed peer-review-in-progress Signifies that the peer review team is reviewing this PR labels Apr 1, 2025
@stevsmit
Copy link
Member

stevsmit commented Apr 1, 2025

Nice work!

@stevsmit stevsmit removed the peer-review-needed Signifies that the peer review team needs to review this PR label Apr 1, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
branch/enterprise-4.19 peer-review-done Signifies that the peer review team has reviewed this PR size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants