search

alibaba / open-local

cloud-native local storage management system for stateful workload, low-latency with simplicity
Apache License 2.0
460 stars 81 forks source link

feat: add `MaxVolumesPerNode` setting #242

Closed dongjiang1989 closed 9 months ago

dongjiang1989 commented 9 months ago

This PR adds support for returning max volume limit in CSI NodeGetInfo call, It returns MaxVolumesPerNode in CSINodeGetInfo response, which defaults to 64, This can be configured via MAX_VOLUMES_PERNODE from ENV config.

codecov-commenter commented 9 months ago

Codecov Report

Attention: 15 lines in your changes are missing coverage. Please review.

Comparison is base (3f84fda) 32.31% compared to head (3b4688a) 32.24%.

Files Patch % Lines
pkg/csi/nodeserver.go 0.00% 15 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #242 +/- ## ========================================== - Coverage 32.31% 32.24% -0.08% ========================================== Files 41 41 Lines 6411 6426 +15 ========================================== Hits 2072 2072 - Misses 4050 4065 +15 Partials 289 289 ``` | [Flag](https://app.codecov.io/gh/alibaba/open-local/pull/242/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=alibaba) | Coverage Δ | | |---|---|---| | [unittests](https://app.codecov.io/gh/alibaba/open-local/pull/242/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=alibaba) | `32.24% <0.00%> (-0.08%)` | :arrow_down: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=alibaba#carryforward-flags-in-the-pull-request-comment) to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

dongjiang1989 commented 9 months ago

@peter-wangxu addressed comments, please review :)

peter-wangxu commented 9 months ago

BTW, any reason to set this limit, do you meet any limitation on lvm/linux?

dongjiang1989 commented 9 months ago

BTW, any reason to set this limit, do you meet any limitation on lvm/linux?

Setting a reasonable number of volumes is related to Node disk pressure and Pod balanced deployment. In my case: lvm2 on aws ebs

@peter-wangxu Please re-check

dongjiang1989 commented 9 months ago

@peter-wangxu Please release patch version 🤔️

peter-wangxu commented 9 months ago

I have not verify this version in production env, do you mind a pre-relase for current version?

dongjiang1989 commented 9 months ago

I have not verify this version in production env, do you mind a pre-relase for current version?

Yes. Pre-release versions are helpful to me