metal-stack / csi-driver-lvm

MIT License
120 stars 25 forks source link

Read-only filesystems are not supported #86

Closed gerhard closed 1 year ago

gerhard commented 1 year ago

I am setting this up on Talos OS v1.4.0 & K8s v1.26.4. On this OS, /etc is not a writable.

I had to make this change to csi-driver-lvm chart so that it installs:

diff --git a/charts/csi-driver-lvm/templates/plugin.yaml b/charts/csi-driver-lvm/templates/plugin.yaml
index b347cfa..c1b0dc0 100644
--- a/charts/csi-driver-lvm/templates/plugin.yaml
+++ b/charts/csi-driver-lvm/templates/plugin.yaml
@@ -61,10 +61,10 @@ metadata:
 spec:
   allowedHostPaths:
   - pathPrefix: /lib/modules
-  - pathPrefix: /etc/lvm/cache
+  - pathPrefix: {{ .Values.lvm.hostWritePath }}etc/lvm/cache
   - pathPrefix: {{ .Values.kubernetes.kubeletPath }}/plugins/{{ .Values.lvm.storageClassStub }}
-  - pathPrefix: /etc/lvm/backup
-  - pathPrefix: /run/lock/lvm
+  - pathPrefix: {{ .Values.lvm.hostWritePath }}etc/lvm/backup
+  - pathPrefix: /var/run/lock/lvm
   - pathPrefix: {{ .Values.kubernetes.kubeletPath }}/plugins
   - pathPrefix: {{ .Values.kubernetes.kubeletPath }}/plugins_registry
   - pathPrefix: /dev
@@ -267,15 +267,15 @@ spec:
           path: /lib/modules
         name: mod-dir
       - hostPath:
-          path: /etc/lvm/backup
+          path: {{ .Values.lvm.hostWritePath }}etc/lvm/backup
           type: DirectoryOrCreate
         name: lvmbackup
       - hostPath:
-          path: /etc/lvm/cache
+          path: {{ .Values.lvm.hostWritePath }}etc/lvm/cache
           type: DirectoryOrCreate
         name: lvmcache
       - hostPath:
-          path: /run/lock/lvm
+          path: {{ .Values.lvm.hostWritePath }}/run/lock/lvm
           type: DirectoryOrCreate
         name: lvmlock
 ---
diff --git a/charts/csi-driver-lvm/values.yaml b/charts/csi-driver-lvm/values.yaml
index e954dfb..f95598c 100644
--- a/charts/csi-driver-lvm/values.yaml
+++ b/charts/csi-driver-lvm/values.yaml
@@ -3,6 +3,9 @@ lvm:
   # This one you should change
   devicePattern: /dev/nvme[0-9]n[0-9]

+  # You will want to change this for read-only filesystems, e.g. Talos OS
+  hostWritePath: /
+
   # these are primariliy for testing purposes
   vgName: csi-lvm
   driverName: lvm.csi.metal-stack.io

Note I can submit the above patch as a PR to https://github.com/metal-stack/helm-charts, no worries.

Once I got csi-driver-lvm installed on K8s on Talos OS, I am now hitting issues creating the pvc:

image

I tracked this issue down to these hard-coded /etc/lvm/backup, /etc/lvm/cache & /run/lock/lvm paths in the controller:

https://github.com/metal-stack/csi-driver-lvm/blob/999b3eead35f705d1751febeb2e9b9d65d3d68cc/pkg/lvm/lvm.go#L359-L385

Before I submit a PR that fixes it, I am wondering if this is something that you would be interested in accepting. I was thinking of adding a hostWritePath to the volumeAction, and then wire it through. Would you suggest a different approach? FTR:

https://github.com/metal-stack/csi-driver-lvm/blob/999b3eead35f705d1751febeb2e9b9d65d3d68cc/pkg/lvm/lvm.go#L67-L79

Thanks for a great LVM CSI! I tried a few other ones before I settled on this one. It's so close to what I am looking for. 💪


cc @smira @andrewrynhard

majst01 commented 1 year ago

certainly interested in this enhancement, your approach seems legit

gerhard commented 1 year ago

@majst01 WDYT?

majst01 commented 1 year ago

@majst01 WDYT?

Looks nice, thanks for the effort @mwennrich must also take a look.

gerhard commented 1 year ago

Resolved by https://github.com/metal-stack/csi-driver-lvm/pull/87