[IMPROVEMENT] RWX volume scheduling

[mazocode]

Is your improvement request related to a feature? Please describe (👍 if you like this request)

The RWX functionality is hardly usable for us because the share-manager does not honor the storage classes nodeSelector, topology, nor any other nodeSelector configured. This means that it gets scheduled on nodes that are not suitable for this kind of workload or do not match the requirements. In addition, placing it randomly might not be beneficial to the traffic flow.

Describe the solution you'd like

Add a nodeSelector derived from storage class topolgy configuration or nodeSelector tag.

Describe alternatives you've considered

So far I could not find a workaround.

Additional context

RWX storage class example

apiVersion: storage.k8s.io/v1
kind: StorageClass
provisioner: driver.longhorn.io
allowVolumeExpansion: true
allowedTopologies:
- matchLabelExpressions:
  - key: storage-provisioning/longhorn
    values:
    - ""
metadata:
  name: rep2x-rwx-hdd-fra4-b1c
parameters:
  diskSelector: hdd
  nfsOptions: vers=4.1,noresvport,softerr,timeo=600,retrans=5
  nodeSelector: fra4-b1c
  numberOfReplicas: "2"
  staleReplicaTimeout: "2880"
volumeBindingMode: WaitForFirstConsumer
reclaimPolicy: Delete

Neither topology nor nodeSelector tag gets honored when scheduling the share-manager pod.

[github-actions]

This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 5 days.

[github-actions]

This issue was closed because it has been stalled for 5 days with no activity.

[mazocode]

Can we please reopen this, so that I don't feel like I waste my time here?

[derekbit]

Just realized nodeSelector in StorageClass.Parameters is a bit confusing. The field is for selecting the candidates for replace placement rather than engine placement.

https://longhorn.io/docs/1.6.1/references/storage-class-parameters/

cc @shuo-wu @innobead

[derekbit]

@mazocode Thanks for raising the issue. I've implemented the feature. Feel free to check the design #8294 and welcome any feedback.

[shuo-wu]

Just realized nodeSelector in StorageClass.Parameters is a bit confusing. The field is for selecting the candidates for replace placement rather than engine placement.

For regular volumes, the engine scheduling is determined by the workload pods hence typically nodeSelector has no ambiguity. But for the RWX volume, YES it's confusing. I am fine with renaming it to replicaNodeSelector.

[longhorn-io-github-bot]

Pre Ready-For-Testing Checklist

• Where is the reproduce steps/test steps documented?
  The reproduce steps/test steps are at:

1. Configure the node selector, allowedTopologies or tolerations according to the doc https://github.com/longhorn/website/pull/891/files
2. Check the attached node of the rwx volume is expected.

• Does the PR include the explanation for the fix or the feature?

• Have the backend code been merged (Manager, Engine, Instance Manager, BackupStore etc) (including backport-needed/*)?
  The PR is at rwx: add "Configuring Volume Locality for an RWX Volume" website#891

• Which areas/issues this PR might have potential impacts on?
  Area: RWX volume, scheduling
  Issues

• If labeled: require/LEP Has the Longhorn Enhancement Proposal PR submitted?
  The LEP PR is at

#8294

• If labeled: area/ui Has the UI issue filed or ready to be merged (including backport-needed/*)?
  The UI issue/PR is at

• If labeled: require/doc Has the necessary document PR submitted or merged (including backport-needed/*)?
  The documentation issue/PR is at

longhorn/website#891

• If labeled: require/automation-e2e Has the end-to-end test plan been merged? Have QAs agreed on the automation test case? If only test case skeleton w/o implementation, have you created an implementation issue (including backport-needed/*)
  The automation skeleton PR is at
  The automation test case PR is at
  The issue of automation test case implementation is at (please create by the template)

• If labeled: require/automation-engine Has the engine integration test been merged (including backport-needed/*)?
  The engine automation PR is at

• If labeled: require/manual-test-plan Has the manual test plan been documented?
  The updated manual test plan is at

• If the fix introduces the code for backward compatibility Has a separate issue been filed with the label release/obsolete-compatibility?
  The compatibility issue is filed at

[davidfrickert]

Also interested in this feature. I've had a look at the design/implementation - it depends on storage class parameters.
Generally fine but as storage classes are not mutable this means anyone that wants to use this feature and is already using longhorn needs to create new storage classes and potentially migrate data, which is pretty annoying.

Would it be possible to implement a fallback by checking PV annotations if the new storageclass params are not present?

[derekbit]

Also interested in this feature. I've had a look at the design/implementation - it depends on storage class parameters. Generally fine but as storage classes are not mutable this means anyone that wants to use this feature and is already using longhorn needs to create new storage classes and potentially migrate data, which is pretty annoying.

Would it be possible to implement a fallback by checking PV annotations if the new storageclass params are not present?

@davidfrickert
checking PV annotations => Sounds good to me. Can you open a ticket for the feature request? Thank you.

[chriscchien]

Verified pass on longhorn master(longhorn-manager ad7420) with test steps

Share manager followed storageclass parameter shareManagerNodeSelector, allowedTopologies and shareManagerTolerations to attach node.