Skip to content
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

Use HostToContainer mount propagation instead of /host/proc/mounts #321

Merged
merged 10 commits into from
Dec 20, 2024
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,9 @@
# Unreleased
[Documentation](https://github.com/awslabs/mountpoint-s3-csi-driver/blob/main/README.md)

unexge marked this conversation as resolved.
Show resolved Hide resolved
### 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)

Expand Down
13 changes: 7 additions & 6 deletions charts/aws-mountpoint-s3-csi-driver/templates/node.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
1 change: 0 additions & 1 deletion charts/aws-mountpoint-s3-csi-driver/values.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
11 changes: 5 additions & 6 deletions deploy/kubernetes/base/node-daemonset.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
83 changes: 0 additions & 83 deletions pkg/driver/node/mounter/lister.go

This file was deleted.

39 changes: 0 additions & 39 deletions pkg/driver/node/mounter/mocks/mock_mount.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 0 additions & 1 deletion pkg/driver/node/mounter/mount_credentials.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand Down
5 changes: 0 additions & 5 deletions pkg/driver/node/mounter/mounter.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
20 changes: 11 additions & 9 deletions pkg/driver/node/mounter/systemd_mounter.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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()
muddyfish marked this conversation as resolved.
Show resolved Hide resolved
muddyfish marked this conversation as resolved.
Show resolved Hide resolved
if err != nil {
return false, fmt.Errorf("Failed to list mounts: %w", err)
}
Expand Down Expand Up @@ -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{}
Expand Down
Loading
Loading