Skip to content

Commit b4f5cd8

Browse files
reixdRainer 'rei' Schuth
andauthored
fix(cronjobs): fix EFS PV/PVC naming collisions for long namespace names (#163)
Co-authored-by: Rainer 'rei' Schuth <rainer.schuth@digital-results-international.com>
1 parent d6f20dd commit b4f5cd8

5 files changed

Lines changed: 63 additions & 29 deletions

File tree

charts/cronjobs/Chart.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ apiVersion: v2
33
name: cronjobs
44
description: A generic helm cronjob chart for kubernetes
55
type: application
6-
version: 1.7.1
6+
version: 2.0.0
77
appVersion: latest
88
home: https://github.com/klicktipp/helm-charts
99
keywords:

charts/cronjobs/templates/_helpers.tpl

Lines changed: 38 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -74,37 +74,51 @@ Return the fully qualified startup jitter image reference.
7474
{{- end -}}
7575

7676
{{/*
77-
Joins a list of strings with "-" and makes the result safe for use as a
78-
Kubernetes resource name (lowercase, no special chars, max 63 chars).
77+
Given a list of strings, concatenate all of them with a dash ("-")
78+
and slugify the string to be DNS name compatible
79+
*/}}
80+
{{- define "com.klicktipp.slugify-volume-name" -}}
81+
{{- $r := (join "-" .) | lower | replace "." "-" | replace "/" "-" | replace "_" "-" | replace "--" "-" | replace " " "-" | trimPrefix "-" | trunc 63 | trimSuffix "-" }}
82+
{{- $r }}
83+
{{- end -}}
84+
85+
{{/*
86+
Collision-free PV / PVC names. Accepts the same list as slugify-volume-name:
87+
[prefix, infix..., suffix]
88+
prefix – first element (namespace for PVs, release name for PVCs); kept as-is
89+
infix – all middle elements; joined and sha256-hashed to 8 hex chars
90+
suffix – last element (EFS access-point ID without "fsap-" prefix); kept as-is
7991
80-
The last element is always kept in full — it is the unique part (e.g. an EFS
81-
access point ID like "0e702cdce44153d31"). If the combined string would exceed
82-
63 chars, the prefix (everything before the last element) is shortened from the
83-
right to make room. This way two different access points for the same job always
84-
get two different PV names, even in namespaces with long names.
92+
Result: prefix-hash(infix)-suffix (≤ 63 chars)
8593
86-
Without this fix, the entire string was cut off at 63 chars from the right,
87-
which silently dropped the access point ID for long namespace names, making
88-
every access point of a job produce the same PV name — causing Kubernetes to
89-
reject the update because the PV's volume source is immutable after creation.
94+
When the result would exceed 63 chars the prefix is right-truncated to fit;
95+
any trailing dash introduced by truncation is stripped.
96+
97+
Because the hash covers the full infix and the access-point ID sits verbatim in
98+
the suffix, names are globally unique even if the same access point is mounted
99+
more than once or two jobs share the same name prefix.
100+
101+
Enable per release with .Values.slugifyVolumeNamesV2: true.
90102
*/}}
91-
{{- define "com.klicktipp.slugify-volume-name" -}}
92-
{{- if and (kindIs "slice" .) (gt (len .) 1) -}}
93-
{{- $last := last . | lower | replace "." "-" | replace "/" "-" | replace "_" "-" | replace "--" "-" | replace " " "-" | trimPrefix "-" | trimSuffix "-" -}}
94-
{{- if ge (len $last) 63 -}}
95-
{{- $last | trunc 63 | trimSuffix "-" | trimPrefix "-" -}}
103+
{{- define "com.klicktipp.slugify-volume-name-v2" -}}
104+
{{- $first := first . | lower | replace "." "-" | replace "/" "-" | replace "_" "-" | replace "--" "-" | replace " " "-" | trimPrefix "-" | trimSuffix "-" -}}
105+
{{- $last := last . | lower | replace "." "-" | replace "/" "-" | replace "_" "-" | replace "--" "-" | replace " " "-" | trimPrefix "-" | trimSuffix "-" -}}
106+
{{- $hash := join "-" (rest (initial .)) | sha256sum | trunc 8 -}}
107+
{{- $result := printf "%s-%s-%s" $first $hash $last -}}
108+
{{- if le (len $result) 63 -}}
109+
{{- $result -}}
110+
{{- else -}}
111+
{{- $max := int (sub 53 (len $last)) -}}
112+
{{- if le $max 0 -}}
113+
{{- printf "%s-%s" $hash $last | trunc 63 | trimSuffix "-" -}}
96114
{{- else -}}
97-
{{- $prefix := join "-" (initial .) | lower | replace "." "-" | replace "/" "-" | replace "_" "-" | replace "--" "-" | replace " " "-" | trimPrefix "-" | trimSuffix "-" -}}
98-
{{- $max := int (sub 63 (add 1 (len $last))) -}}
99-
{{- $p := $prefix | trunc $max | trimSuffix "-" | trimPrefix "-" -}}
100-
{{- if eq $p "" -}}
101-
{{- $last -}}
115+
{{- $f := $first | trunc $max | trimSuffix "-" | trimPrefix "-" -}}
116+
{{- if eq $f "" -}}
117+
{{- printf "%s-%s" $hash $last -}}
102118
{{- else -}}
103-
{{- printf "%s-%s" $p $last | trimSuffix "-" -}}
119+
{{- printf "%s-%s-%s" $f $hash $last -}}
104120
{{- end -}}
105121
{{- end -}}
106-
{{- else -}}
107-
{{- (join "-" .) | lower | replace "." "-" | replace "/" "-" | replace "_" "-" | replace "--" "-" | replace " " "-" | trimPrefix "-" | trunc 63 | trimSuffix "-" -}}
108122
{{- end -}}
109123
{{- end -}}
110124

charts/cronjobs/templates/cronjob.yaml

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -140,7 +140,8 @@ spec:
140140
{{- range $efs_mount := $job.storage.efs.mounts }}
141141
{{- range $efs_ap := $efs_mount.access_points }}
142142
{{- $EFS_AP_ID_TRIMMED := trimPrefix "fsap-" $efs_ap.id }}
143-
{{- $PVC_NAME := include "com.klicktipp.slugify-volume-name" (list $.Release.Name $JOB_NAME_SLUG (coalesce $efs_ap.name $efs_ap.path) $EFS_AP_ID_TRIMMED) }}
143+
{{- $slugify := ternary "com.klicktipp.slugify-volume-name-v2" "com.klicktipp.slugify-volume-name" (default false $.Values.slugifyVolumeNamesV2) -}}
144+
{{- $PVC_NAME := include $slugify (list $.Release.Name $JOB_NAME_SLUG (coalesce $efs_ap.name $efs_ap.path) $EFS_AP_ID_TRIMMED) }}
144145
- name: {{ $PVC_NAME }}
145146
mountPath: {{ required "ERROR: the EFS mount point requires a mount_dest." $efs_ap.mount_dest }}
146147
{{- end }} {{/* # end range $efs_ap */}}
@@ -185,7 +186,8 @@ spec:
185186
{{- range $efs_mount := $job.storage.efs.mounts }}
186187
{{- range $efs_ap := $efs_mount.access_points }}
187188
{{- $EFS_AP_ID_TRIMMED := trimPrefix "fsap-" $efs_ap.id }}
188-
{{- $PVC_NAME := include "com.klicktipp.slugify-volume-name" (list $.Release.Name $JOB_NAME_SLUG (coalesce $efs_ap.name $efs_ap.path) $EFS_AP_ID_TRIMMED) }}
189+
{{- $slugify := ternary "com.klicktipp.slugify-volume-name-v2" "com.klicktipp.slugify-volume-name" (default false $.Values.slugifyVolumeNamesV2) -}}
190+
{{- $PVC_NAME := include $slugify (list $.Release.Name $JOB_NAME_SLUG (coalesce $efs_ap.name $efs_ap.path) $EFS_AP_ID_TRIMMED) }}
189191
- name: {{ $PVC_NAME }}
190192
persistentVolumeClaim:
191193
claimName: {{ $PVC_NAME }}

charts/cronjobs/templates/volumes.yaml

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,18 +5,25 @@
55
{{- range $efs_mount := $job.storage.efs.mounts }}
66
{{- range $efs_ap := $efs_mount.access_points }}
77
{{- $EFS_AP_ID_TRIMMED := trimPrefix "fsap-" $efs_ap.id }}
8-
{{- $PV_NAME := include "com.klicktipp.slugify-volume-name" (list $.Release.Namespace $.Release.Name $JOB_NAME_SLUG $EFS_AP_ID_TRIMMED) }}
9-
{{- $PVC_NAME := include "com.klicktipp.slugify-volume-name" (list $.Release.Name $JOB_NAME_SLUG (coalesce $efs_ap.name $efs_ap.path) $EFS_AP_ID_TRIMMED) }}
8+
{{- $slugify := ternary "com.klicktipp.slugify-volume-name-v2" "com.klicktipp.slugify-volume-name" (default false $.Values.slugifyVolumeNamesV2) -}}
9+
{{- $PV_NAME := include $slugify (list $.Release.Namespace $.Release.Name $JOB_NAME_SLUG $EFS_AP_ID_TRIMMED) }}
10+
{{- $PVC_NAME := include $slugify (list $.Release.Name $JOB_NAME_SLUG (coalesce $efs_ap.name $efs_ap.path) $EFS_AP_ID_TRIMMED) }}
11+
{{- $PV_FULL := join "-" (list $.Release.Namespace $.Release.Name $JOB_NAME_SLUG $EFS_AP_ID_TRIMMED) | lower | replace "." "-" | replace "/" "-" | replace "_" "-" | replace "--" "-" | replace " " "-" | trimPrefix "-" | trimSuffix "-" -}}
12+
{{- $PVC_FULL := join "-" (list $.Release.Name $JOB_NAME_SLUG (coalesce $efs_ap.name $efs_ap.path) $EFS_AP_ID_TRIMMED) | lower | replace "." "-" | replace "/" "-" | replace "_" "-" | replace "--" "-" | replace " " "-" | trimPrefix "-" | trimSuffix "-" -}}
1013
---
1114
apiVersion: v1
1215
kind: PersistentVolume
1316
metadata:
1417
name: {{ $PV_NAME }}
18+
labels:
19+
storage.ktsys.cloud/efs_fs_id: {{ $efs_mount.fs_id | quote }}
20+
storage.ktsys.cloud/efs_ap_id: {{ $efs_ap.id | quote }}
1521
annotations:
1622
storage.ktsys.cloud/efs_fs_id: {{ $efs_mount.fs_id | quote }}
1723
storage.ktsys.cloud/efs_name: {{ $efs_mount.fs_name | quote }}
1824
storage.ktsys.cloud/efs_ap_id: {{ $efs_ap.id | quote }}
1925
storage.ktsys.cloud/efs_path: {{ $efs_ap.path | quote }}
26+
storage.ktsys.cloud/volume-name-full: {{ $PV_FULL | quote }}
2027
{{- with $.commonAnnotations }}
2128
{{- toYaml . | nindent 4 }}
2229
{{- end }}
@@ -40,11 +47,15 @@ apiVersion: v1
4047
kind: PersistentVolumeClaim
4148
metadata:
4249
name: {{ $PVC_NAME }}
50+
labels:
51+
storage.ktsys.cloud/efs_fs_id: {{ $efs_mount.fs_id | quote }}
52+
storage.ktsys.cloud/efs_ap_id: {{ $efs_ap.id | quote }}
4353
annotations:
4454
storage.ktsys.cloud/efs_fs_id: {{ $efs_mount.fs_id | quote }}
4555
storage.ktsys.cloud/efs_name: {{ $efs_mount.fs_name | quote }}
4656
storage.ktsys.cloud/efs_ap_id: {{ $efs_ap.id | quote }}
4757
storage.ktsys.cloud/efs_path: {{ $efs_ap.path | quote }}
58+
storage.ktsys.cloud/volume-name-full: {{ $PVC_FULL | quote }}
4859
{{- with $.commonAnnotations }}
4960
{{- toYaml . | nindent 4 }}
5061
{{- end }}

charts/cronjobs/values.yaml

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,13 @@ nameOverride: ""
33
# -- Fully override generated resource names.
44
fullnameOverride: ""
55

6+
# -- Use collision-free EFS PV/PVC naming (com.klicktipp.slugify-volume-name-v2).
7+
# When enabled, PV names are globally unique and PVC names are namespace-unique
8+
# by hashing the infix (chart + job name) to 8 hex chars while keeping the
9+
# namespace/release prefix and EFS access-point ID verbatim.
10+
# Disabled by default to preserve backward compatibility with existing PVs/PVCs.
11+
slugifyVolumeNamesV2: false
12+
613
# -- Image pull secrets.
714
imagePullSecrets: []
815

0 commit comments

Comments
 (0)