lvm-localpv
lvm-localpv copied to clipboard
fix(localpv): ensure kubeletDir trailing slash
Pull Request template
Please, go through these steps before you submit a PR.
Why is this PR required? What issue does it fix?: This PR avoids subtle problems when installing on microk8s or other k8s distributions with a different kubelet path. If the user types the path into the values file without a trailing slash, all the openebs pods would be running and even generate PVs and LVs, but no pods using the PVs would start because the CSI plugin was not installed correctly on the node.
What this PR does?: The lvm-node template is updated to ensure that there is always a slash between the kubeletDir value and subdirectories that are appended. Additionally, the paths are now quoted, in case they contain some characters that would cause problems when interpreting the rendered template as YAML.
Does this PR require any upgrade changes?: No. The user sets the same value, and the current values with trailing slashes produce the same output as before.
If the changes in this PR are manually verified, list down the scenarios covered::
helm template . -s templates/lvm-node.yaml --set lvmNode.kubeletDir=/var/lib/kubelet/
and helm template . -s templates/lvm-node.yaml --set lvmNode.kubeletDir=/var/lib/kubelet
now produce the same result.
Any additional information for your reviewer? : Mention if this PR is part of any design or a continuation of previous PRs
Checklist:
- [ ] Fixes #
- [ ] PR Title follows the convention of
<type>(<scope>): <subject>
- [ ] Has the change log section been updated?
- [ ] Commit has unit tests
- [ ] Commit has integration tests
- [ ] (Optional) Are upgrade changes included in this PR? If not, mention the issue/PR to track:
- [ ] (Optional) If documentation changes are required, which issue on https://github.com/openebs/openebs-docs is used to track them:
@mdonoughe could you please sign your commit? https://github.com/openebs/lvm-localpv/blob/develop/CONTRIBUTING.md#sign-your-work
@mdonoughe Thanks for the contribution. This PR would be reviewed and would eventually be cherry-picked into the next helm release once we achieve the approval quorum.
@Abhinandan-Purkait Can this be merged now? I see the label hold
was put on this earlier.
@dsharma-dc no, merging this will create a helm release. We will cherry-pick this to a single PR before release.
Closing this PR as this change has been included in #322. cc: @abhilashshetty04 @dsharma-dc