-
Notifications
You must be signed in to change notification settings - Fork 484
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
STOR-2141: add enhancement for vSphere configurable max number of volumes per node #1748
base: master
Are you sure you want to change the base?
Conversation
@RomanBednar: This pull request references STOR-2141 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the epic to target the "4.19.0" version, but no target version was set. In response to this: 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 openshift-eng/jira-lifecycle-plugin repository. |
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
enhancements/storage/vsphere-configurable-maximum-allowed-number-of-block-volumes-per-node.md
Show resolved
Hide resolved
enhancements/storage/vsphere-configurable-maximum-allowed-number-of-block-volumes-per-node.md
Outdated
Show resolved
Hide resolved
78b4a2a
to
8062fa5
Compare
@RomanBednar: This pull request references STOR-2141 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the epic to target the "4.19.0" version, but no target version was set. In response to this:
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 openshift-eng/jira-lifecycle-plugin repository. |
@RomanBednar: This pull request references STOR-2141 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the epic to target the "4.19.0" version, but no target version was set. In response to this:
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 openshift-eng/jira-lifecycle-plugin repository. |
/assign @gnufied For review, feel free to reassign to someone else if needed. |
8062fa5
to
7cef5ef
Compare
- Enabling FSS without checking vSphere and ESXi versions poses a risk because cluster can have different versions of | ||
ESXi per host. If higher limit is enabled CSI driver reports this limit that is later checked by pod scheduler. The | ||
scheduler might assign a pod to a node that can be already at its limit due to ESXi limitations, failing all | ||
volume attachments. |
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.
I think this feature-gate is already enabled in driver now and there is no way to disable/enable it. This is called a released feature now - https://github.com/openshift/vmware-vsphere-csi-driver/blob/master/pkg/csi/service/common/commonco/k8sorchestrator/k8sorchestrator.go#L354
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.
Correct, I've updated the enhancement to reflect this.
|
||
### Risks and Mitigations | ||
|
||
1. Unlimited volume attachments per node: |
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.
We could also block users from setting this value to 0
via some kind of validation in CRD.
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.
Absolutely, more on this in API PR: openshift/api#2190 (comment)
7cef5ef
to
1e497db
Compare
@RomanBednar: 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. |
- API will not allow `0` value to be set `MAX_VOLUMES_PER_NODE` or allow the field to be unset. This would lead to | ||
disabling the limit. |
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.
looks like a copy-paste error?
- API will not allow `0` value to be set `MAX_VOLUMES_PER_NODE` or allow the field to be unset. This would lead to | |
disabling the limit. | |
- API will not allow `0` value to be set or allow the field to be unset. This would lead to | |
disabling the limit. |
|
||
- Introduce a new field `spec.driverConfig.vSphere.maxAllowedBlockVolumesPerNode` in ClusterCSIDriver API to allow | ||
administrators to configure the desired maximum number of volumes per node. | ||
- This field should default to the current maximum limit of 59 volumes per node for vSphere 7. |
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.
Just to be clear: in my 4.18 cluster, I don't have maxAllowedBlockVolumesPerNode
field. When I upgrade to 4.19, the field appears and will have 59 as the value?
openshift/api PR: openshift/api#2190
cc @openshift/storage