STOR-2141: add support for maxAllowedBlockVolumesPerNode #287

RomanBednar · 2025-02-04T11:32:38Z

Depends on

openshift/api: STOR-2141: add MaxAllowedBlockVolumesPerNode field to VSphereCSIDriverConfigSpec api#2190
openshift/client-go: STOR-2141: API bump for maxAllowedBlockVolumesPerNode client-go#313

Manual verification

Test value limits for maxAllowedBlockVolumesPerNode field:

oc patch clustercsidriver/csi.vsphere.vmware.com --type=merge -p '{"spec":{"driverConfig":{"vSphere":{"maxAllowedBlockVolumesPerNode":-1}}}}'
The ClusterCSIDriver "csi.vsphere.vmware.com" is invalid: spec.driverConfig.vSphere.maxAllowedBlockVolumesPerNode: Invalid value: -1: spec.driverConfig.vSphere.maxAllowedBlockVolumesPerNode in body should be greater than or equal to 1

$ oc patch clustercsidriver/csi.vsphere.vmware.com --type=merge -p '{"spec":{"driverConfig":{"vSphere":{"maxAllowedBlockVolumesPerNode":0}}}}'
The ClusterCSIDriver "csi.vsphere.vmware.com" is invalid: spec.driverConfig.vSphere.maxAllowedBlockVolumesPerNode: Invalid value: 0: spec.driverConfig.vSphere.maxAllowedBlockVolumesPerNode in body should be greater than or equal to 1

$ oc patch clustercsidriver/csi.vsphere.vmware.com --type=merge -p '{"spec":{"driverConfig":{"vSphere":{"maxAllowedBlockVolumesPerNode":256}}}}'
The ClusterCSIDriver "csi.vsphere.vmware.com" is invalid: spec.driverConfig.vSphere.maxAllowedBlockVolumesPerNode: Invalid value: 256: spec.driverConfig.vSphere.maxAllowedBlockVolumesPerNode in body should be less than or equal to 255

Validate maxAllowedBlockVolumesPerNode value propagation to driver deployment as MAX_VOLUMES_PER_NODE:

$ oc patch clustercsidriver/csi.vsphere.vmware.com --type=merge -p '{"spec":{"driverConfig":{"vSphere":{"maxAllowedBlockVolumesPerNode":60}}}}'
clustercsidriver.operator.openshift.io/csi.vsphere.vmware.com patched

$ oc -n openshift-cluster-csi-drivers get deployment.apps/vmware-vsphere-csi-driver-controller -o jsonpath='{.specontainers[0].env}'
[{"name":"CSI_ENDPOINT","value":"unix:///var/lib/csi/sockets/pluginproxy/csi.sock"},{"name":"X_CSI_MODE","value":"controller"},{"name":"VSPHERE_CSI_CONFIG","value":"/etc/kubernetes/vsphere-csi-config/cloud.conf"},{"name":"INCLUSTER_CLIENT_QPS","value":"100"},{"name":"INCLUSTER_CLIENT_BURST","value":"100"},{"name":"CSI_NAMESPACE","valueFrom":{"fieldRef":{"apiVersion":"v1","fieldPath":"metadata.namespace"}}},{"name":"X_CSI_SERIAL_VOL_ACCESS_TIMEOUT","value":"3m"},{"name":"X_CSI_SPEC_DISABLE_LEN_CHECK","value":"true"},{"name":"MAX_VOLUMES_PER_NODE","value":"60"}]

oc -n openshift-cluster-csi-drivers get daemonset.apps/vmware-vsphere-csi-driver-node -o jsonpath='{.spec.template.spec.containers[0].env}'
[{"name":"NODE_NAME","valueFrom":{"fieldRef":{"apiVersion":"v1","fieldPath":"spec.nodeName"}}},{"name":"CSI_ENDPOINT","value":"unix:///csi/csi.sock"},{"name":"X_CSI_MODE","value":"node"},{"name":"X_CSI_SPEC_DISABLE_LEN_CHECK","value":"true"},{"name":"CSI_NAMESPACE","valueFrom":{"fieldRef":{"apiVersion":"v1","fieldPath":"metadata.namespace"}}},{"name":"MAX_VOLUMES_PER_NODE","value":"60"}]

Validate propagation to CSINode as allocatable count:

$ oc get csinode/ci-ln-k30mn5t-c1627-2tk2k-worker-0-72mfn -o jsonpath='{.spec.drivers[0].allocatable.count}'
60

openshift-ci-robot · 2025-02-04T11:32:43Z

@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.

assets/vsphere_features_config.yaml

openshift-ci · 2025-02-06T10:57:28Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: RomanBednar

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [RomanBednar]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

openshift-ci-robot · 2025-03-18T14:48:16Z

@RomanBednar: This pull request references STOR-2141 which is a valid jira issue.

In response to this:

Manual verification

Test value limits for maxAllowedBlockVolumesPerNode field:

oc patch clustercsidriver/csi.vsphere.vmware.com --type=merge -p '{"spec":{"driverConfig":{"vSphere":{"maxAllowedBlockVolumesPerNode":-1}}}}'
The ClusterCSIDriver "csi.vsphere.vmware.com" is invalid: spec.driverConfig.vSphere.maxAllowedBlockVolumesPerNode: Invalid value: -1: spec.driverConfig.vSphere.maxAllowedBlockVolumesPerNode in body should be greater than or equal to 1

$ oc patch clustercsidriver/csi.vsphere.vmware.com --type=merge -p '{"spec":{"driverConfig":{"vSphere":{"maxAllowedBlockVolumesPerNode":0}}}}'
The ClusterCSIDriver "csi.vsphere.vmware.com" is invalid: spec.driverConfig.vSphere.maxAllowedBlockVolumesPerNode: Invalid value: 0: spec.driverConfig.vSphere.maxAllowedBlockVolumesPerNode in body should be greater than or equal to 1

$ oc patch clustercsidriver/csi.vsphere.vmware.com --type=merge -p '{"spec":{"driverConfig":{"vSphere":{"maxAllowedBlockVolumesPerNode":256}}}}'
The ClusterCSIDriver "csi.vsphere.vmware.com" is invalid: spec.driverConfig.vSphere.maxAllowedBlockVolumesPerNode: Invalid value: 256: spec.driverConfig.vSphere.maxAllowedBlockVolumesPerNode in body should be less than or equal to 255

Validate maxAllowedBlockVolumesPerNode value propagation to driver deployment as MAX_VOLUMES_PER_NODE:

$ oc patch clustercsidriver/csi.vsphere.vmware.com --type=merge -p '{"spec":{"driverConfig":{"vSphere":{"maxAllowedBlockVolumesPerNode":60}}}}'
clustercsidriver.operator.openshift.io/csi.vsphere.vmware.com patched

$ oc -n openshift-cluster-csi-drivers get deployment.apps/vmware-vsphere-csi-driver-controller -o jsonpath='{.specontainers[0].env}'
[{"name":"CSI_ENDPOINT","value":"unix:///var/lib/csi/sockets/pluginproxy/csi.sock"},{"name":"X_CSI_MODE","value":"controller"},{"name":"VSPHERE_CSI_CONFIG","value":"/etc/kubernetes/vsphere-csi-config/cloud.conf"},{"name":"INCLUSTER_CLIENT_QPS","value":"100"},{"name":"INCLUSTER_CLIENT_BURST","value":"100"},{"name":"CSI_NAMESPACE","valueFrom":{"fieldRef":{"apiVersion":"v1","fieldPath":"metadata.namespace"}}},{"name":"X_CSI_SERIAL_VOL_ACCESS_TIMEOUT","value":"3m"},{"name":"X_CSI_SPEC_DISABLE_LEN_CHECK","value":"true"},{"name":"MAX_VOLUMES_PER_NODE","value":"60"}]

oc -n openshift-cluster-csi-drivers get daemonset.apps/vmware-vsphere-csi-driver-node -o jsonpath='{.spec.template.spec.containers[0].env}'
[{"name":"NODE_NAME","valueFrom":{"fieldRef":{"apiVersion":"v1","fieldPath":"spec.nodeName"}}},{"name":"CSI_ENDPOINT","value":"unix:///csi/csi.sock"},{"name":"X_CSI_MODE","value":"node"},{"name":"X_CSI_SPEC_DISABLE_LEN_CHECK","value":"true"},{"name":"CSI_NAMESPACE","valueFrom":{"fieldRef":{"apiVersion":"v1","fieldPath":"metadata.namespace"}}},{"name":"MAX_VOLUMES_PER_NODE","value":"60"}]

Validate propagation to CSINode as allocatable count:

$ oc get csinode/ci-ln-k30mn5t-c1627-2tk2k-worker-0-72mfn -o jsonpath='{.spec.drivers[0].allocatable.count}'
60

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.

openshift-ci-robot · 2025-03-20T10:26:46Z

@RomanBednar: This pull request references STOR-2141 which is a valid jira issue.

In response to this:

Depends on

openshift/api: STOR-2141: add MaxAllowedBlockVolumesPerNode field to VSphereCSIDriverConfigSpec api#2190
openshift/client-go: STOR-2141: API bump for maxAllowedBlockVolumesPerNode client-go#313

Manual verification

Test value limits for maxAllowedBlockVolumesPerNode field:

oc patch clustercsidriver/csi.vsphere.vmware.com --type=merge -p '{"spec":{"driverConfig":{"vSphere":{"maxAllowedBlockVolumesPerNode":-1}}}}'
The ClusterCSIDriver "csi.vsphere.vmware.com" is invalid: spec.driverConfig.vSphere.maxAllowedBlockVolumesPerNode: Invalid value: -1: spec.driverConfig.vSphere.maxAllowedBlockVolumesPerNode in body should be greater than or equal to 1

$ oc patch clustercsidriver/csi.vsphere.vmware.com --type=merge -p '{"spec":{"driverConfig":{"vSphere":{"maxAllowedBlockVolumesPerNode":0}}}}'
The ClusterCSIDriver "csi.vsphere.vmware.com" is invalid: spec.driverConfig.vSphere.maxAllowedBlockVolumesPerNode: Invalid value: 0: spec.driverConfig.vSphere.maxAllowedBlockVolumesPerNode in body should be greater than or equal to 1

$ oc patch clustercsidriver/csi.vsphere.vmware.com --type=merge -p '{"spec":{"driverConfig":{"vSphere":{"maxAllowedBlockVolumesPerNode":256}}}}'
The ClusterCSIDriver "csi.vsphere.vmware.com" is invalid: spec.driverConfig.vSphere.maxAllowedBlockVolumesPerNode: Invalid value: 256: spec.driverConfig.vSphere.maxAllowedBlockVolumesPerNode in body should be less than or equal to 255

Validate maxAllowedBlockVolumesPerNode value propagation to driver deployment as MAX_VOLUMES_PER_NODE:

$ oc patch clustercsidriver/csi.vsphere.vmware.com --type=merge -p '{"spec":{"driverConfig":{"vSphere":{"maxAllowedBlockVolumesPerNode":60}}}}'
clustercsidriver.operator.openshift.io/csi.vsphere.vmware.com patched

$ oc -n openshift-cluster-csi-drivers get deployment.apps/vmware-vsphere-csi-driver-controller -o jsonpath='{.specontainers[0].env}'
[{"name":"CSI_ENDPOINT","value":"unix:///var/lib/csi/sockets/pluginproxy/csi.sock"},{"name":"X_CSI_MODE","value":"controller"},{"name":"VSPHERE_CSI_CONFIG","value":"/etc/kubernetes/vsphere-csi-config/cloud.conf"},{"name":"INCLUSTER_CLIENT_QPS","value":"100"},{"name":"INCLUSTER_CLIENT_BURST","value":"100"},{"name":"CSI_NAMESPACE","valueFrom":{"fieldRef":{"apiVersion":"v1","fieldPath":"metadata.namespace"}}},{"name":"X_CSI_SERIAL_VOL_ACCESS_TIMEOUT","value":"3m"},{"name":"X_CSI_SPEC_DISABLE_LEN_CHECK","value":"true"},{"name":"MAX_VOLUMES_PER_NODE","value":"60"}]

oc -n openshift-cluster-csi-drivers get daemonset.apps/vmware-vsphere-csi-driver-node -o jsonpath='{.spec.template.spec.containers[0].env}'
[{"name":"NODE_NAME","valueFrom":{"fieldRef":{"apiVersion":"v1","fieldPath":"spec.nodeName"}}},{"name":"CSI_ENDPOINT","value":"unix:///csi/csi.sock"},{"name":"X_CSI_MODE","value":"node"},{"name":"X_CSI_SPEC_DISABLE_LEN_CHECK","value":"true"},{"name":"CSI_NAMESPACE","valueFrom":{"fieldRef":{"apiVersion":"v1","fieldPath":"metadata.namespace"}}},{"name":"MAX_VOLUMES_PER_NODE","value":"60"}]

Validate propagation to CSINode as allocatable count:

$ oc get csinode/ci-ln-k30mn5t-c1627-2tk2k-worker-0-72mfn -o jsonpath='{.spec.drivers[0].allocatable.count}'
60

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.

In order to validate maximum volume attachment limit set by user NodeChecker needs access to ClusterCSIDriver which is where users set the value.

We need to check versions of all ESXI hosts in the cluster and if we detect that users set a custom volume attachment limit that is incorrect we degrade the cluster. Incorrect value is any value above 59 if any of the vSphere hosts in a cluster is not on ESXI version 8 or higher

Since NodeChecker now checks max attachment limit value it is now safe to add hooks for reflecting maxAllowedBlockVolumesPerNode field of clusterCSIDriver into deployment and daemonset as env variable.

RomanBednar · 2025-03-21T15:20:15Z

/retest-required

openshift-ci · 2025-03-21T16:38:45Z

@RomanBednar: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
ci/prow/okd-scos-e2e-aws-ovn	`3942bf2`	link	false	`/test okd-scos-e2e-aws-ovn`

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.

RomanBednar · 2025-03-25T14:44:54Z

/assign @gnufied

For review - feel free to reassign to other candidate.

gnufied · 2025-03-25T15:12:02Z

pkg/operator/vspherecontroller/checks/check_nodes.go

+
+	// Check if a custom value is set in the ClusterCSIDriver
+	if clusterCSIDriver != nil && clusterCSIDriver.Spec.DriverConfig.VSphere != nil {
+		maxVolumesPerNode = int(clusterCSIDriver.Spec.DriverConfig.VSphere.MaxAllowedBlockVolumesPerNode)


What does this return when no limit is set? 0 ??

gnufied · 2025-03-25T15:13:04Z

Shouldn't all this code be behind a feature gate?

openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Feb 4, 2025

openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Feb 4, 2025

openshift-ci bot requested review from dobsonj and gnufied February 4, 2025 11:34

openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Feb 4, 2025

RomanBednar force-pushed the STOR-2141 branch from 4002654 to 087f333 Compare February 4, 2025 15:49

gnufied reviewed Feb 5, 2025

View reviewed changes

assets/vsphere_features_config.yaml Outdated Show resolved Hide resolved

RomanBednar force-pushed the STOR-2141 branch from 087f333 to 1a23c7d Compare February 6, 2025 10:57

RomanBednar force-pushed the STOR-2141 branch from 1a23c7d to 4da2904 Compare March 17, 2025 08:52

RomanBednar changed the title ~~WIP: STOR-2141: add daemonset hook to allow setting custom volume limit~~ WIP: STOR-2141: add support for maxAllowedBlockVolumesPerNode Mar 17, 2025

RomanBednar force-pushed the STOR-2141 branch 3 times, most recently from 623109c to ea8a387 Compare March 18, 2025 13:48

RomanBednar force-pushed the STOR-2141 branch from ea8a387 to 62766d2 Compare March 20, 2025 10:25

RomanBednar added 4 commits March 21, 2025 10:40

bump api and client-go

5cf704f

let NodeChecker access ClusterCSIDriverLister

9c1ef14

In order to validate maximum volume attachment limit set by user NodeChecker needs access to ClusterCSIDriver which is where users set the value.

add daemonset and deployment hooks for per node volume limit

3942bf2

Since NodeChecker now checks max attachment limit value it is now safe to add hooks for reflecting maxAllowedBlockVolumesPerNode field of clusterCSIDriver into deployment and daemonset as env variable.

RomanBednar force-pushed the STOR-2141 branch from 62766d2 to 3942bf2 Compare March 21, 2025 09:41

RomanBednar changed the title ~~WIP: STOR-2141: add support for maxAllowedBlockVolumesPerNode~~ STOR-2141: add support for maxAllowedBlockVolumesPerNode Mar 21, 2025

openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Mar 21, 2025

openshift-ci bot assigned gnufied Mar 25, 2025

gnufied reviewed Mar 25, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

STOR-2141: add support for maxAllowedBlockVolumesPerNode #287

STOR-2141: add support for maxAllowedBlockVolumesPerNode #287

RomanBednar commented Feb 4, 2025 •

edited

Loading

openshift-ci-robot commented Feb 4, 2025 •

edited by openshift-ci bot

Loading

openshift-ci bot commented Feb 6, 2025

openshift-ci-robot commented Mar 18, 2025 •

edited by openshift-ci bot

Loading

Manual verification

openshift-ci-robot commented Mar 20, 2025 •

edited by openshift-ci bot

Loading

Depends on

Manual verification

RomanBednar commented Mar 21, 2025

openshift-ci bot commented Mar 21, 2025

RomanBednar commented Mar 25, 2025

gnufied Mar 25, 2025

gnufied commented Mar 25, 2025

STOR-2141: add support for maxAllowedBlockVolumesPerNode #287

Are you sure you want to change the base?

STOR-2141: add support for maxAllowedBlockVolumesPerNode #287

Conversation

RomanBednar commented Feb 4, 2025 • edited Loading

Depends on

Manual verification

openshift-ci-robot commented Feb 4, 2025 • edited by openshift-ci bot Loading

openshift-ci bot commented Feb 6, 2025

openshift-ci-robot commented Mar 18, 2025 • edited by openshift-ci bot Loading

Manual verification

openshift-ci-robot commented Mar 20, 2025 • edited by openshift-ci bot Loading

Depends on

Manual verification

RomanBednar commented Mar 21, 2025

openshift-ci bot commented Mar 21, 2025

RomanBednar commented Mar 25, 2025

gnufied Mar 25, 2025

Choose a reason for hiding this comment

gnufied commented Mar 25, 2025

RomanBednar commented Feb 4, 2025 •

edited

Loading

openshift-ci-robot commented Feb 4, 2025 •

edited by openshift-ci bot

Loading

openshift-ci-robot commented Mar 18, 2025 •

edited by openshift-ci bot

Loading

openshift-ci-robot commented Mar 20, 2025 •

edited by openshift-ci bot

Loading