-
Notifications
You must be signed in to change notification settings - Fork 450
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
Feature/multiple storage classes #361
Feature/multiple storage classes #361
Conversation
Can you help rebase the commit and |
8f748bb
to
a68a074
Compare
Rebased. Can you elaborate on what sort of update to the README you're looking for? I did update the config.json definition section to include the new field I added. FYSA I will be away from home for the next week, so if you'd like to put this on hold until then, I don't mind. |
@meln5674 |
Sorry for the delay on this, I ended up falling seriously ill after getting back from my trip. I can't seem to find any information on how the tests here are supposed to work. My default guess of I did in fact write tests while writing this, though since I ran into the above issues since the start, I just wrote them in a shell script, which I've put into this gist. If you give me any pointers on how to actually run the tests, as well as any on how best to integrate what I've written into the existing test setup, I'd really appreciate it. |
Hey @meln5674 , hope you are well again. I don't have any experience with Go and am still somewhat new to kubernetes, but as I'm looking forward to this feature I played around with the repo a bit. I checked how the CI is running the tests using the Edit: Just ran the tests again. Seem to be passing now. |
@timvahlbrock Thanks for the detective work, and I am in fact in better health again, thank you for your concern. @derekbit I think I've worked out how the tests are structured now, and I've added a test which creates two storage classes, two matching PVC's, and mounts them to a single pod to match how the other tests work so I wouldn't have to re-work how the tests are structured. Let me know if you'd prefer I go back and do a 2-pod test instead. |
@meln5674 is this pr possibly not being reviewed because the pipeline is red (even though that's because it was killed...)? |
I saw there were a few other PR builds that were killed, each because they never started their s390x build, so I assumed the project was working through some CI/CD issues. |
Can you set @derekbit as a reviewer explicitly or something? Or try to push something again to get the pipeline green? I don't wanna be too annoying or something, but other PRs seem to be getting merged. |
@meln5674 do you have permission to request a review on this PR? |
@timvahlbrock Sorry for the late reply. Busy at Longhorn project.
The error is due to that the s390x machine doesn't work. |
Yes, everything should be ready for review. |
@derekbit Slipped out of ToDo's again? |
In addition `volumeBindingMode: Immediate` can be used in StorageClass definition. | ||
|
||
Please note that `nodePathMap` and `sharedFileSystemPath` are mutually exclusive. If `sharedFileSystemPath` is used, then `nodePathMap` must be set to `[]`. | ||
Please note that `nodePathMap`, `sharedFileSystemPath`, and `storageClassConfigs` are mutually exclusive. If `sharedFileSystemPath` or `stroageClassConfigs` are used, then `nodePathMap` must be set to `[]`. |
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.
IIUC, the backward compatibility of an existing configmap with nodePathMap
is also addressed, right?
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.
Yes. The ConfigMap under this patch can contain one and only one of nodePathMap, sharedFileSystemPath, or storageClassConfigs. An existing ConfigMap with one of nodePathMap and sharedFileSystemPath should continue to operate as-is, which I believe the tests demonstrate.
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.
The implementation LGTM.
Only one question for the deploy manifest.
Is there anything I can help with to progress this further? |
@timvahlbrock |
how nice, thanks! I'm waiting for this to be released! |
When is v0.0.27 version getting released with this change..?
|
In the end of this May |
…7 ) (#4774) This PR contains the following updates: | Package | Update | Change | |---|---|---| | [local-path-provisioner](https://github.com/rancher/local-path-provisioner) | patch | `v0.0.26` -> `v0.0.27` | --- > [!WARNING] > Some dependencies could not be looked up. Check the Dependency Dashboard for more information. --- ### Release Notes <details> <summary>rancher/local-path-provisioner (local-path-provisioner)</summary> ### [`v0.0.27`](https://github.com/rancher/local-path-provisioner/releases/tag/v0.0.27): Local Path Provisioner v0.0.27 [Compare Source](https://github.com/rancher/local-path-provisioner/compare/v0.0.26...v0.0.27) #### What's Changed - Repair code example in storageClass description. by [@​c4lliope](https://github.com/c4lliope) in [rancher/local-path-provisioner#368 - Update README.md to 0.26 by [@​e-minguez](https://github.com/e-minguez) in [rancher/local-path-provisioner#373 - Update README.md,rm /manager by [@​terryzwt](https://github.com/terryzwt) in [rancher/local-path-provisioner#379 - Fix duplicate labels by [@​runningman84](https://github.com/runningman84) in [rancher/local-path-provisioner#393 - drone: remove s390x support by [@​derekbit](https://github.com/derekbit) in [rancher/local-path-provisioner#391 - Feature/multiple storage classes by [@​meln5674](https://github.com/meln5674) in [rancher/local-path-provisioner#361 - Remove duplicate labels and add ability to set helperpod resource requests/limits by [@​visokoo](https://github.com/visokoo) in [rancher/local-path-provisioner#394 - Automatic reloading of the helper pod manifest by the provisioner by [@​js185692](https://github.com/js185692) in [rancher/local-path-provisioner#399 - Add support for custom path patterns by [@​AlbanBedel](https://github.com/AlbanBedel) in [rancher/local-path-provisioner#385 - adding pvc with node name example by [@​sebastianohl](https://github.com/sebastianohl) in [rancher/local-path-provisioner#382 - Add e2e test for custom path patterns by [@​derekbit](https://github.com/derekbit) in [rancher/local-path-provisioner#404 - Give the helper pod more range of MCS categories by [@​galal-hussein](https://github.com/galal-hussein) in [rancher/local-path-provisioner#402 - Fix: Chart.yaml file is missing on helm install by [@​jamshidi799](https://github.com/jamshidi799) in [rancher/local-path-provisioner#388 - drone: disable e2e test by [@​derekbit](https://github.com/derekbit) in [rancher/local-path-provisioner#405 - Allow customizing helper pod by [@​justusbunsi](https://github.com/justusbunsi) in [rancher/local-path-provisioner#365 - test: use reclaimPolicy Delete instead by [@​derekbit](https://github.com/derekbit) in [rancher/local-path-provisioner#406 - chart: fix pathPattern by [@​derekbit](https://github.com/derekbit) in [rancher/local-path-provisioner#409 #### New Contributors - [@​c4lliope](https://github.com/c4lliope) made their first contribution in [rancher/local-path-provisioner#368 - [@​e-minguez](https://github.com/e-minguez) made their first contribution in [rancher/local-path-provisioner#373 - [@​terryzwt](https://github.com/terryzwt) made their first contribution in [rancher/local-path-provisioner#379 - [@​runningman84](https://github.com/runningman84) made their first contribution in [rancher/local-path-provisioner#393 - [@​visokoo](https://github.com/visokoo) made their first contribution in [rancher/local-path-provisioner#394 - [@​AlbanBedel](https://github.com/AlbanBedel) made their first contribution in [rancher/local-path-provisioner#385 - [@​sebastianohl](https://github.com/sebastianohl) made their first contribution in [rancher/local-path-provisioner#382 - [@​galal-hussein](https://github.com/galal-hussein) made their first contribution in [rancher/local-path-provisioner#402 - [@​jamshidi799](https://github.com/jamshidi799) made their first contribution in [rancher/local-path-provisioner#388 - [@​justusbunsi](https://github.com/justusbunsi) made their first contribution in [rancher/local-path-provisioner#365 **Full Changelog**: rancher/local-path-provisioner@v0.0.26...v0.0.27 </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy4zODEuNiIsInVwZGF0ZWRJblZlciI6IjM3LjM4MS42IiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6WyJyZW5vdmF0ZS9naXRodWItcmVsZWFzZSIsInR5cGUvcGF0Y2giXX0=--> Co-authored-by: lumiere-bot[bot] <98047013+lumiere-bot[bot]@users.noreply.github.com>
…7 ) (#4773) This PR contains the following updates: | Package | Update | Change | |---|---|---| | [local-path-provisioner](https://github.com/rancher/local-path-provisioner) | patch | `v0.0.26` -> `v0.0.27` | --- > [!WARNING] > Some dependencies could not be looked up. Check the Dependency Dashboard for more information. --- ### Release Notes <details> <summary>rancher/local-path-provisioner (local-path-provisioner)</summary> ### [`v0.0.27`](https://github.com/rancher/local-path-provisioner/releases/tag/v0.0.27): Local Path Provisioner v0.0.27 [Compare Source](https://github.com/rancher/local-path-provisioner/compare/v0.0.26...v0.0.27) #### What's Changed - Repair code example in storageClass description. by [@​c4lliope](https://github.com/c4lliope) in [rancher/local-path-provisioner#368 - Update README.md to 0.26 by [@​e-minguez](https://github.com/e-minguez) in [rancher/local-path-provisioner#373 - Update README.md,rm /manager by [@​terryzwt](https://github.com/terryzwt) in [rancher/local-path-provisioner#379 - Fix duplicate labels by [@​runningman84](https://github.com/runningman84) in [rancher/local-path-provisioner#393 - drone: remove s390x support by [@​derekbit](https://github.com/derekbit) in [rancher/local-path-provisioner#391 - Feature/multiple storage classes by [@​meln5674](https://github.com/meln5674) in [rancher/local-path-provisioner#361 - Remove duplicate labels and add ability to set helperpod resource requests/limits by [@​visokoo](https://github.com/visokoo) in [rancher/local-path-provisioner#394 - Automatic reloading of the helper pod manifest by the provisioner by [@​js185692](https://github.com/js185692) in [rancher/local-path-provisioner#399 - Add support for custom path patterns by [@​AlbanBedel](https://github.com/AlbanBedel) in [rancher/local-path-provisioner#385 - adding pvc with node name example by [@​sebastianohl](https://github.com/sebastianohl) in [rancher/local-path-provisioner#382 - Add e2e test for custom path patterns by [@​derekbit](https://github.com/derekbit) in [rancher/local-path-provisioner#404 - Give the helper pod more range of MCS categories by [@​galal-hussein](https://github.com/galal-hussein) in [rancher/local-path-provisioner#402 - Fix: Chart.yaml file is missing on helm install by [@​jamshidi799](https://github.com/jamshidi799) in [rancher/local-path-provisioner#388 - drone: disable e2e test by [@​derekbit](https://github.com/derekbit) in [rancher/local-path-provisioner#405 - Allow customizing helper pod by [@​justusbunsi](https://github.com/justusbunsi) in [rancher/local-path-provisioner#365 - test: use reclaimPolicy Delete instead by [@​derekbit](https://github.com/derekbit) in [rancher/local-path-provisioner#406 - chart: fix pathPattern by [@​derekbit](https://github.com/derekbit) in [rancher/local-path-provisioner#409 #### New Contributors - [@​c4lliope](https://github.com/c4lliope) made their first contribution in [rancher/local-path-provisioner#368 - [@​e-minguez](https://github.com/e-minguez) made their first contribution in [rancher/local-path-provisioner#373 - [@​terryzwt](https://github.com/terryzwt) made their first contribution in [rancher/local-path-provisioner#379 - [@​runningman84](https://github.com/runningman84) made their first contribution in [rancher/local-path-provisioner#393 - [@​visokoo](https://github.com/visokoo) made their first contribution in [rancher/local-path-provisioner#394 - [@​AlbanBedel](https://github.com/AlbanBedel) made their first contribution in [rancher/local-path-provisioner#385 - [@​sebastianohl](https://github.com/sebastianohl) made their first contribution in [rancher/local-path-provisioner#382 - [@​galal-hussein](https://github.com/galal-hussein) made their first contribution in [rancher/local-path-provisioner#402 - [@​jamshidi799](https://github.com/jamshidi799) made their first contribution in [rancher/local-path-provisioner#388 - [@​justusbunsi](https://github.com/justusbunsi) made their first contribution in [rancher/local-path-provisioner#365 **Full Changelog**: rancher/local-path-provisioner@v0.0.26...v0.0.27 </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy4zODEuNiIsInVwZGF0ZWRJblZlciI6IjM3LjM4MS42IiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6WyJyZW5vdmF0ZS9naXRodWItcmVsZWFzZSIsInR5cGUvcGF0Y2giXX0=--> Co-authored-by: lumiere-bot[bot] <98047013+lumiere-bot[bot]@users.noreply.github.com>
[![Mend Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com) This PR contains the following updates: | Package | Update | Change | |---|---|---| | [local-path-provisioner](https://github.com/rancher/local-path-provisioner) | patch | `v0.0.26` -> `v0.0.28` | --- ### Release Notes <details> <summary>rancher/local-path-provisioner (local-path-provisioner)</summary> ### [`v0.0.28`](https://github.com/rancher/local-path-provisioner/releases/tag/v0.0.28): Local Path Provisioner v0.0.28 [Compare Source](https://github.com/rancher/local-path-provisioner/compare/v0.0.27...v0.0.28-rc1) #### What's Changed - Migrate CI to github Actions by [@​mantissahz](https://github.com/mantissahz) in [rancher/local-path-provisioner#403 - fix(ci): allow to read docker hub secret by [@​mantissahz](https://github.com/mantissahz) in [rancher/local-path-provisioner#412 - Revert "Give the helper pod more range of MCS categories" by [@​derekbit](https://github.com/derekbit) in [rancher/local-path-provisioner#421 - Temporarily disable TestPodWithMultipleStorageClasses by [@​derekbit](https://github.com/derekbit) in [rancher/local-path-provisioner#423 - Move helperPod namespace into metadata by [@​justusbunsi](https://github.com/justusbunsi) in [rancher/local-path-provisioner#425 #### New Contributors - [@​justusbunsi](https://github.com/justusbunsi) made their first contribution in [rancher/local-path-provisioner#365 - [@​mantissahz](https://github.com/mantissahz) made their first contribution in [rancher/local-path-provisioner#403 **Full Changelog**: rancher/local-path-provisioner@v0.0.27...v0.0.28 ### [`v0.0.27`](https://github.com/rancher/local-path-provisioner/releases/tag/v0.0.27): Local Path Provisioner v0.0.27 [Compare Source](https://github.com/rancher/local-path-provisioner/compare/v0.0.26...v0.0.27) #### What's Changed - Repair code example in storageClass description. by [@​c4lliope](https://github.com/c4lliope) in [rancher/local-path-provisioner#368 - Update README.md to 0.26 by [@​e-minguez](https://github.com/e-minguez) in [rancher/local-path-provisioner#373 - Update README.md,rm /manager by [@​terryzwt](https://github.com/terryzwt) in [rancher/local-path-provisioner#379 - Fix duplicate labels by [@​runningman84](https://github.com/runningman84) in [rancher/local-path-provisioner#393 - drone: remove s390x support by [@​derekbit](https://github.com/derekbit) in [rancher/local-path-provisioner#391 - Feature/multiple storage classes by [@​meln5674](https://github.com/meln5674) in [rancher/local-path-provisioner#361 - Remove duplicate labels and add ability to set helperpod resource requests/limits by [@​visokoo](https://github.com/visokoo) in [rancher/local-path-provisioner#394 - Automatic reloading of the helper pod manifest by the provisioner by [@​js185692](https://github.com/js185692) in [rancher/local-path-provisioner#399 - Add support for custom path patterns by [@​AlbanBedel](https://github.com/AlbanBedel) in [rancher/local-path-provisioner#385 - adding pvc with node name example by [@​sebastianohl](https://github.com/sebastianohl) in [rancher/local-path-provisioner#382 - Add e2e test for custom path patterns by [@​derekbit](https://github.com/derekbit) in [rancher/local-path-provisioner#404 - Give the helper pod more range of MCS categories by [@​galal-hussein](https://github.com/galal-hussein) in [rancher/local-path-provisioner#402 - Fix: Chart.yaml file is missing on helm install by [@​jamshidi799](https://github.com/jamshidi799) in [rancher/local-path-provisioner#388 - drone: disable e2e test by [@​derekbit](https://github.com/derekbit) in [rancher/local-path-provisioner#405 - Allow customizing helper pod by [@​justusbunsi](https://github.com/justusbunsi) in [rancher/local-path-provisioner#365 - test: use reclaimPolicy Delete instead by [@​derekbit](https://github.com/derekbit) in [rancher/local-path-provisioner#406 - chart: fix pathPattern by [@​derekbit](https://github.com/derekbit) in [rancher/local-path-provisioner#409 #### New Contributors - [@​c4lliope](https://github.com/c4lliope) made their first contribution in [rancher/local-path-provisioner#368 - [@​e-minguez](https://github.com/e-minguez) made their first contribution in [rancher/local-path-provisioner#373 - [@​terryzwt](https://github.com/terryzwt) made their first contribution in [rancher/local-path-provisioner#379 - [@​runningman84](https://github.com/runningman84) made their first contribution in [rancher/local-path-provisioner#393 - [@​visokoo](https://github.com/visokoo) made their first contribution in [rancher/local-path-provisioner#394 - [@​AlbanBedel](https://github.com/AlbanBedel) made their first contribution in [rancher/local-path-provisioner#385 - [@​sebastianohl](https://github.com/sebastianohl) made their first contribution in [rancher/local-path-provisioner#382 - [@​galal-hussein](https://github.com/galal-hussein) made their first contribution in [rancher/local-path-provisioner#402 - [@​jamshidi799](https://github.com/jamshidi799) made their first contribution in [rancher/local-path-provisioner#388 - [@​justusbunsi](https://github.com/justusbunsi) made their first contribution in [rancher/local-path-provisioner#365 **Full Changelog**: rancher/local-path-provisioner@v0.0.26...v0.0.27 </details> --- ### Configuration 📅 **Schedule**: Branch creation - "on saturday" (UTC), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR has been generated by [Mend Renovate](https://www.mend.io/free-developer-tools/renovate/). View repository job log [here](https://developer.mend.io/github/sp3nx0r/homelab). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy4zNzcuOCIsInVwZGF0ZWRJblZlciI6IjM3LjQxMy4yIiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6WyJyZW5vdmF0ZS9naXRodWItcmVsZWFzZSIsInR5cGUvcGF0Y2giXX0=--> Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
While is is currently possible to deploy multiple instances of the provisioner to support multiple storage classes, as well as to support both RWO, RWX, and ROX volumes at the same time, the additional resource consumption and deployment complexity are undesirable.
This patch adds an additional field to the
config.json
namedstorageClassConfigs
, which is mutually exclusive with the existingnodePathMap
andsharedFilesystemPath
fields. This field is an object, whose keys are the names of storage classes, and its values are nested objects that contain eithernodePathMap
orsharedFilesystemPath
. When a volume request is received, if this field is set, it is consulted for the configuration to use based on the name of the storage class, otherwise the original configuration fields are used.This patch also updates the helm chart to add a matching option, which also deploys multiple storage class objects if selected.
This change is backwards compatible, installing this version of the provisioner should not break any existing installs.
One advantage of this change is that single-node k3s/k3d/kind clusters will be able to provide RWX and ROX storage classes out of the box.