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.

Sign up for GitHub

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

Making helm charts configurable #116

Merged
merged 4 commits into from
Jan 3, 2024
Merged

Making helm charts configurable #116

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
b4abcde
Making helm charts configurable
dlakhaws Dec 28, 2023
467933d
Read the token path from env in the driver
dlakhaws Dec 28, 2023
f09e9ad
Only make the kubelet path configurable
dlakhaws Jan 2, 2024
61370d9
Trim suffix /
dlakhaws Jan 3, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 21 additions & 7 deletions charts/aws-mountpoint-s3-csi-driver/templates/node.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,11 +61,13 @@ spec:
value: /host/dev/ptmx
# mount-s3 runs in systemd context, so this is relative to the host
- name: MOUNT_S3_PATH
value: /opt/mountpoint-s3-csi/bin/mount-s3
value: {{ default "/opt/mountpoint-s3-csi/bin/" .Values.node.mountpointInstallPath }}mount-s3
- name: CSI_NODE_NAME
valueFrom:
fieldRef:
fieldPath: spec.nodeName
- name: HOST_TOKEN_PATH
value: {{ printf "%s/plugins/%s" .Values.node.kubeletPath (default "s3.csi.aws.com/token" .Values.node.hostTokenPath) }}
{{- with .Values.awsAccessSecret }}
- name: AWS_ACCESS_KEY_ID
valueFrom:
Expand All @@ -82,7 +84,7 @@ spec:
{{- end }}
volumeMounts:
- name: kubelet-dir
mountPath: /var/lib/kubelet
mountPath: {{ .Values.node.kubeletPath }}
- name: plugin-dir
mountPath: /csi
- name: systemd-bus
Expand All @@ -103,6 +105,10 @@ spec:
timeoutSeconds: 3
periodSeconds: 2
failureThreshold: 5
{{- with .Values.node.resources }}
resources:
{{- toYaml . | nindent 12 }}
{{- end }}
- name: node-driver-registrar
image: {{ printf "%s:%s" .Values.sidecars.nodeDriverRegistrar.image.repository .Values.sidecars.nodeDriverRegistrar.image.tag }}
imagePullPolicy: {{ default .Values.image.pullPolicy .Values.sidecars.nodeDriverRegistrar.image.pullPolicy }}
Expand All @@ -113,7 +119,7 @@ spec:
- name: ADDRESS
value: /csi/csi.sock
- name: DRIVER_REG_SOCK_PATH
value: /var/lib/kubelet/plugins/s3.csi.aws.com/csi.sock
value: {{ .Values.node.kubeletPath }}/plugins/s3.csi.aws.com/csi.sock
- name: KUBE_NODE_NAME
valueFrom:
fieldRef:
Expand All @@ -132,6 +138,10 @@ spec:
mountPath: /csi
- name: registration-dir
mountPath: /registration
{{- with default .Values.node.resources .Values.sidecars.nodeDriverRegistrar.resources }}
resources:
{{- toYaml . | nindent 12 }}
{{- end }}
- name: liveness-probe
image: {{ printf "%s:%s" .Values.sidecars.livenessProbe.image.repository .Values.sidecars.livenessProbe.image.tag }}
imagePullPolicy: {{ default .Values.image.pullPolicy .Values.sidecars.livenessProbe.image.pullPolicy }}
Expand All @@ -140,14 +150,18 @@ spec:
volumeMounts:
- name: plugin-dir
mountPath: /csi
{{- with default .Values.node.resources .Values.sidecars.livenessProbe.resources }}
resources:
{{- toYaml . | nindent 12 }}
{{- end }}
volumes:
- name: host-dev
hostPath:
path: /dev/
type: Directory
- name: mp-install
hostPath:
path: /opt/mountpoint-s3-csi/bin/
path: {{ default "/opt/mountpoint-s3-csi/bin/" .Values.node.mountpointInstallPath }}
type: DirectoryOrCreate
- name: proc-mounts
hostPath:
Expand All @@ -159,15 +173,15 @@ spec:
type: Socket
- name: kubelet-dir
hostPath:
path: /var/lib/kubelet
path: {{ .Values.node.kubeletPath }}
type: Directory
- name: plugin-dir
hostPath:
path: /var/lib/kubelet/plugins/s3.csi.aws.com/
path: {{ .Values.node.kubeletPath }}/plugins/s3.csi.aws.com/
type: DirectoryOrCreate
- name: registration-dir
hostPath:
path: /var/lib/kubelet/plugins_registry/
path: {{ .Values.node.kubeletPath }}/plugins_registry/
type: Directory
{{- with .Values.node.volumes }}
{{- toYaml . | nindent 8 }}
Expand Down
11 changes: 10 additions & 1 deletion charts/aws-mountpoint-s3-csi-driver/values.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,21 @@ image:
node:
resources: {}
kubeletPath: /var/lib/kubelet
mountpointInstallPath: /opt/mountpoint-s3-csi/bin/ # should end with "/"
hostTokenPath: s3.csi.aws.com/token # the end path will be `kubeletPath + "/plugins/" + hostTokenpath`
logLevel: 4
containerSecurityContext:
privileged: true
serviceAccount:
# Specifies whether a service account should be created
create: true
name: s3-csi-driver-sa

resources:
requests:
cpu: 10m
memory: 40Mi
limits:
memory: 256Mi
sidecars:
nodeDriverRegistrar:
image:
Expand All @@ -35,6 +42,7 @@ sidecars:
mountPath: /csi
- name: registration-dir
mountPath: /registration
resources: {}
livenessProbe:
image:
repository: public.ecr.aws/eks-distro/kubernetes-csi/livenessprobe
Expand All @@ -43,6 +51,7 @@ sidecars:
volumeMounts:
- mountPath: /csi
name: plugin-dir
resources: {}

nameOverride: ""
fullnameOverride: ""
Expand Down
20 changes: 20 additions & 0 deletions deploy/kubernetes/base/node-daemonset.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,8 @@ spec:
valueFrom:
fieldRef:
fieldPath: spec.nodeName
- name: HOST_TOKEN_PATH
value: /var/lib/kubelet/plugins/s3.csi.aws.com/token
volumeMounts:
- name: kubelet-dir
mountPath: /var/lib/kubelet
Expand All @@ -95,6 +97,12 @@ spec:
initialDelaySeconds: 10
timeoutSeconds: 3
periodSeconds: 2
resources:
limits:
memory: 256Mi
requests:
cpu: 10m
memory: 40Mi
- name: node-driver-registrar
image: public.ecr.aws/eks-distro/kubernetes-csi/node-driver-registrar:v2.7.0-eks-1-23-13
imagePullPolicy: IfNotPresent
Expand All @@ -115,6 +123,12 @@ spec:
mountPath: /csi
- name: registration-dir
mountPath: /registration
resources:
limits:
memory: 256Mi
requests:
cpu: 10m
memory: 40Mi
- name: liveness-probe
image: public.ecr.aws/eks-distro/kubernetes-csi/livenessprobe:v2.9.0-eks-1-23-13
imagePullPolicy: IfNotPresent
Expand All @@ -124,6 +138,12 @@ spec:
volumeMounts:
- mountPath: /csi
name: plugin-dir
resources:
limits:
memory: 256Mi
requests:
cpu: 10m
memory: 40Mi
volumes:
- name: host-dev
hostPath:
Expand Down
7 changes: 6 additions & 1 deletion pkg/driver/mount.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,11 +39,11 @@ const (
defaultRegionEnv = "AWS_DEFAULT_REGION"
stsEndpointsEnv = "AWS_STS_REGIONAL_ENDPOINTS"
MountS3PathEnv = "MOUNT_S3_PATH"
hostTokenPathEnv = "HOST_TOKEN_PATH"
defaultMountS3Path = "/usr/bin/mount-s3"
procMounts = "/host/proc/mounts"
userAgentPrefix = "--user-agent-prefix"
csiDriverPrefix = "s3-csi-driver/"
hostTokenPath = "/var/lib/kubelet/plugins/s3.csi.aws.com/token"
)

// Mounter is an interface for mount operations
Expand Down Expand Up @@ -195,6 +195,11 @@ func passthroughEnv() []string {
}
webIdentityFile := os.Getenv(webIdentityTokenEnv)
awsRoleArn := os.Getenv(roleArnEnv)
hostTokenPath := os.Getenv(hostTokenPathEnv)
if hostTokenPath == "" {
// set the default in case the env variable isn't found
hostTokenPath = "/var/lib/kubelet/plugins/s3.csi.aws.com/token"
}
if webIdentityFile != "" {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should this be == in place of != ? if webIdentityFile is set should we not use it?

Copy link
Contributor Author

@dlakhaws dlakhaws Jan 3, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not quite, we're trying to set the env variable here based on the driver env (if you look at webIdentityFileEnv it comes from driver.go, this isn't something set by the customer) so if the driver is running with the web token, that should be passed here as well (but with different paths)

Copy link

@iostreamdoth iostreamdoth Jan 3, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see, so this line.
https://github.com/awslabs/mountpoint-s3-csi-driver/blob/main/pkg/driver/driver.go#L78C4-L82
I was little confused as it was pointing to /csi/token as path in driver. I assumed this might be some mechanism to override it. Thanks for clarification.

env = append(env, webIdentityTokenEnv+"="+hostTokenPath)
env = append(env, roleArnEnv+"="+awsRoleArn)
Expand Down