Use HostToContainer mount propagation instead of /host/proc/mounts (

#321)

*Description of changes:*
Currently, we spawn Mountpoint processes on the host using systemd. As a
result, the mounts created by Mountpoint are not visible inside the CSI
Driver Pod. To work around this, we were mounting `/proc/mounts` from
host and parsing this file to check existing mounts on the host.
Mounting `/proc/mounts` causes problems with Karpenter sometimes and its
also its blocked by some SELinux policies. Such as this
[issue](#284).

This commit instead uses `HostToContainer` mount propagation on
`hostPath` mount for `/var/lib/kubelet`. Thanks to `HostToContainer`,
any new mounts created inside `/var/lib/kubelet` gets automatically
propagated to our Pod from the host.

By submitting this pull request, I confirm that you can use, modify,
copy, and redistribute this contribution, under the terms of your
choice.

---------

Signed-off-by: Burak Varlı <[email protected]>
Co-authored-by: Burak Varlı <[email protected]>
Co-authored-by: Jiayi Nie <[email protected]>

CHANGELOG.md

@@ -1,3 +1,9 @@
+# Unreleased
+[Documentation](https://github.com/awslabs/mountpoint-s3-csi-driver/blob/main/README.md)
+
+### Notable changes
+* Add `HostToContainer` mount propagation, replacing the previous method of reading mount points via `/host/proc/mounts`. ([#321](https://github.com/awslabs/mountpoint-s3-csi-driver/pull/321))
+
 # v1.11.0
 [Documentation](https://github.com/awslabs/mountpoint-s3-csi-driver/blob/v1.11.0/README.md)
 

charts/aws-mountpoint-s3-csi-driver/templates/node.yaml

@@ -93,6 +93,8 @@ spec:
             # mount-s3 runs in systemd context, so this is relative to the host
             - name: MOUNT_S3_PATH
               value: {{ default "/opt/mountpoint-s3-csi/bin/" .Values.node.mountpointInstallPath }}mount-s3
+            - name: KUBELET_PATH
+              value: {{ .Values.node.kubeletPath }}
             - name: CSI_NODE_NAME
               valueFrom:
                 fieldRef:
@@ -122,14 +124,17 @@ spec:
           volumeMounts:
             - name: kubelet-dir
               mountPath: {{ .Values.node.kubeletPath }}
+              # Currently we spawn Mountpoint instances on the host using systemd,
+              # "HostToContainer" allows any newly created mounts inside kubelet path to propagated to the container.
+              # Thanks to this, we can do "is mount point?" checks for volumes provided by the CSI Driver
+              # without needing to mount "/proc/mounts" from host.
+              mountPropagation: HostToContainer
             - name: plugin-dir
               mountPath: /csi
             - name: systemd-bus
               mountPath: /run/systemd/private
             - name: host-dev
               mountPath: /host/dev
-            - name: proc-mounts
-              mountPath: /host/proc/mounts
           ports:
             - name: healthz
               containerPort: 9808
@@ -206,10 +211,6 @@ spec:
           hostPath:
             path: {{ default "/opt/mountpoint-s3-csi/bin/" .Values.node.mountpointInstallPath }}
             type: DirectoryOrCreate
-        - name: proc-mounts
-          hostPath:
-            path: {{ default "/proc/mounts" .Values.node.procMountPath }}
-            type: File
         - name: systemd-bus
           hostPath:
             path: /run/systemd/private

charts/aws-mountpoint-s3-csi-driver/values.yaml

@@ -14,7 +14,6 @@ image:
 node:
   kubeletPath: /var/lib/kubelet
   mountpointInstallPath: /opt/mountpoint-s3-csi/bin/ # should end with "/"
-  procMountPath: /proc/mounts
   logLevel: 4
   seLinuxOptions:
     user: system_u

deploy/kubernetes/base/node-daemonset.yaml

@@ -107,14 +107,17 @@ spec:
           volumeMounts:
             - name: kubelet-dir
               mountPath: /var/lib/kubelet
+              # Currently we spawn Mountpoint instances on the host using systemd,
+              # "HostToContainer" allows any newly created mounts inside kubelet path to propagated to the container.
+              # Thanks to this, we can do "is mount point?" checks for volumes provided by the CSI Driver
+              # without needing to mount "/proc/mounts" from host.
+              mountPropagation: HostToContainer
             - name: plugin-dir
               mountPath: /csi
             - name: systemd-bus
               mountPath: /run/systemd/private
             - name: host-dev
               mountPath: /host/dev
-            - name: proc-mounts
-              mountPath: /host/proc/mounts
           ports:
             - name: healthz
               containerPort: 9810
@@ -198,10 +201,6 @@ spec:
           hostPath:
             path: /opt/mountpoint-s3-csi/bin/
             type: DirectoryOrCreate
-        - name: proc-mounts
-          hostPath:
-            path: /proc/mounts
-            type: File
         - name: systemd-bus
           hostPath:
             path: /run/systemd/private

pkg/driver/node/mounter/mount_credentials.go

@@ -19,7 +19,6 @@ const (
 	awsMaxAttemptsEnv           = "AWS_MAX_ATTEMPTS"
 	MountpointCacheKey          = "UNSTABLE_MOUNTPOINT_CACHE_KEY"
 	defaultMountS3Path          = "/usr/bin/mount-s3"
-	procMounts                  = "/host/proc/mounts"
 	userAgentPrefix             = "--user-agent-prefix"
 	awsMaxAttemptsOption        = "--aws-max-attempts"
 )

pkg/driver/node/mounter/mounter.go

@@ -6,13 +6,8 @@ import (
 	"os"
 
 	"github.com/awslabs/aws-s3-csi-driver/pkg/system"
-	"k8s.io/mount-utils"
 )
 
-type MountLister interface {
-	ListMounts() ([]mount.MountPoint, error)
-}
-
 type ServiceRunner interface {
 	StartService(ctx context.Context, config *system.ExecConfig) (string, error)
 	RunOneshot(ctx context.Context, config *system.ExecConfig) (string, error)

pkg/driver/node/mounter/systemd_mounter.go

@@ -21,7 +21,7 @@ const mountpointDeviceName = "mountpoint-s3"
 type SystemdMounter struct {
 	Ctx               context.Context
 	Runner            ServiceRunner
-	MountLister       MountLister
+	Mounter           mount.Interface
 	MpVersion         string
 	MountS3Path       string
 	kubernetesVersion string
@@ -36,20 +36,23 @@ func NewSystemdMounter(mpVersion string, kubernetesVersion string) (*SystemdMoun
 	return &SystemdMounter{
 		Ctx:               ctx,
 		Runner:            runner,
-		MountLister:       &ProcMountLister{ProcMountPath: procMounts},
+		Mounter:           mount.New(""),
 		MpVersion:         mpVersion,
 		MountS3Path:       MountS3Path(),
 		kubernetesVersion: kubernetesVersion,
 	}, nil
 }
 
 // IsMountPoint returns whether given `target` is a `mount-s3` mount.
+// We implement the IsMountPoint interface instead of using Kubernetes' implementation
+// because we need to verify not only that the target is a mount point but also that it is specifically a mount-s3 mount point.
+// This is achieved by calling the mounter.List() method to enumerate all mount points.
 func (m *SystemdMounter) IsMountPoint(target string) (bool, error) {
 	if _, err := os.Stat(target); os.IsNotExist(err) {
 		return false, err
 	}
 
-	mountPoints, err := m.MountLister.ListMounts()
+	mountPoints, err := m.Mounter.List()
 	if err != nil {
 		return false, fmt.Errorf("Failed to list mounts: %w", err)
 	}
@@ -114,15 +117,14 @@ func (m *SystemdMounter) Mount(bucketName string, target string, credentials *Mo
 		}
 	}
 
-	mounts, err := m.MountLister.ListMounts()
+	isMountPoint, err := m.IsMountPoint(target)
 	if err != nil {
 		return fmt.Errorf("Could not check if %q is a mount point: %v, %v", target, statErr, err)
 	}
-	for _, m := range mounts {
-		if m.Path == target {
-			klog.V(4).Infof("NodePublishVolume: Target path %q is already mounted", target)
-			return nil
-		}
+
+	if isMountPoint {
+		klog.V(4).Infof("NodePublishVolume: Target path %q is already mounted", target)
+		return nil
 	}
 
 	env := []string{}