feat(paperless-ngx)!: quote every operator scalar, pin the consume mover, drop oidcSecretRef
Three issues on one chart, bundled because they share values.yaml, values.schema.json and the version line, and because the first half of #11 lives in the same template as #101. Gitea #11, first half: the consume CronJob's schedule was rendered bare, so a schedule in the commonest cron notation there is - a star, a slash, an interval - aborted the render with a YAML parse error pointing at the template while the operator's value appeared nowhere in the message. Quoting it in the values file does not help; Helm passes the content through. Reproduced before the fix, at the line the issue names, and fixed with | quote. Gitea #11, second half: secrets.oidcSecretRef is removed, values and schema both. No template ever read it, so no OnePasswordItem was created and nothing reached the pod - the quiet kind of defect that looks like successful configuration. It is not reimplemented, and the reason is in paperless: the whole OIDC configuration travels in one json environment variable with no _FILE variant, so a secretKeyRef cannot substitute a fragment into it. The operational question is answered by #83 - Keycloak marks a client public per client, so nothing forces a confidential one here. In the same breath, oidc.provider.tokenAuthMethod is restricted to "none". Its comment named client_secret_basic as the example, which asks the provider for an authentication this chart cannot perform: it configures no client secret and cannot. The two values read together looked like a working secret setup, and the symptom was a login dying at the callback with invalid_client. No live release sets it (measured against all three instances). Gitea #112 asked for an extraEnv and the chart already had the standard additive env block - the issue was measured against a deployed 12.2.0 and read only the top-level keys. What was actually broken is the block itself: it rendered its operand bare, so a "*"-leading value aborted the render and "true" or "0755" arrived as a bool or an integer, which the API server rejects after a green render. It is brought to the repository's documented quoted form (template-chart carries the rule), which is what makes an arbitrary PAPERLESS_* variable settable in practice. PAPERLESS_LOGGING_DIR is documented as the worked example, with the cross-reference it needs: the root cause is the NFS-exported RWX data volume in jk-cluster-config #360 and this does not fix it - it only moves the log off that volume. The two Ingress upload-size annotations had the same bare render and are quoted too. Gitea #101: the consume mover now runs on the node of the StatefulSet pod, via a required podAffinity on its label. It mounts the RWX consume volume, and Longhorn's CSI driver refuses that mount unless it finds a workload pod in Pending, so a mover on another node left the StatefulSet node's kubelet retrying every two minutes - 291 events in 10 hours in both releases with SMB on, none in the release without the CronJob. A static nodeSelector would be wrong because the StatefulSet pod is not pinned either, and preferred would reproduce the bug more rarely and harder to find. The cost of a required term is handled rather than left open: concurrencyPolicy: Forbid, a startingDeadlineSeconds that also keeps the controller away from its 100-missed-schedules stop, and an activeDeadlineSeconds that fails a job which cannot be placed instead of leaving one Pending pod per schedule. scaleDown already suspends the CronJob; what remains is paperless.replicas: 0 with SMB on, which now reports a bounded, visible failure. successfulJobsHistoryLimit is deliberately untouched: Longhorn's check asks for the existence of a Pending pod, not the absence of Succeeded ones, so those three pods are the error message's evidence and not its cause. Side defect fixed on the way: every read of smbConnection.cron dereferenced the block directly, so an override that had erased it aborted with a nil-pointer error naming an internal template path instead of falling back to the defaults this chart promises everywhere else. Measured on origin/main; all reads now go through a merged-defaults helper that preserves an explicit false. Major: a value is removed and fails the strict schema afterwards, and runtime placement changes. Fixes #11 Fixes #101 Fixes #112
This commit is contained in:
8 files changed
+427
-42
No files matched your search
@@ -1,26 +1,165 @@
|
||||
{{- if .Values.smbConnection.enabled }}
|
||||
{{- /*
|
||||
The cron block, merged onto its defaults, so that an override which erased it
|
||||
falls back instead of aborting with a nil-pointer error naming an internal
|
||||
template path - which is what every direct .Values.smbConnection.cron.<key>
|
||||
read did before 13.0.0. The helper carries the measurement.
|
||||
*/}}
|
||||
{{- $cron := fromYaml (include "paperless-ngx.cron.scheduling" .Values.smbConnection.cron) -}}
|
||||
apiVersion: batch/v1
|
||||
kind: CronJob
|
||||
metadata:
|
||||
name: {{ .Release.Name }}-{{ .Chart.Name }}-consume-cronjob
|
||||
spec:
|
||||
schedule: {{ .Values.smbConnection.cronJob }}
|
||||
{{- /*
|
||||
QUOTED, AND THAT IS THE WHOLE OF GITEA #11's FIRST HALF. The default
|
||||
"0 0,12 * * *" renders safely only because it begins with a digit. The most
|
||||
common cron notation of all - a star, a slash and an interval, every 15
|
||||
minutes - begins with "*", which YAML reads as an alias indicator, and the
|
||||
render ABORTS:
|
||||
|
||||
(and the notation cannot be written out in this comment, because the two
|
||||
characters that start it would end a Go template comment right here)
|
||||
|
||||
Error: YAML parse error on paperless-ngx/templates/paperless-consume.cronjob.yaml:
|
||||
error converting YAML to JSON: yaml: line 6: did not find expected
|
||||
alphabetic or numeric character
|
||||
|
||||
The trap is that quoting it in the VALUES does not help: Helm passes the
|
||||
string's content through, and the star stands bare in the manifest again. So
|
||||
the operator writes a correctly quoted value and gets an error that points at
|
||||
this template instead of at their value. "@daily" fails the same way ("@" is
|
||||
a reserved YAML indicator).
|
||||
|
||||
ci/run-yaml-scalar-render-test.sh is the test that would have caught it, and
|
||||
ci/yaml-scalar/paperless-ngx.yaml is the overlay that feeds it a star-leading
|
||||
schedule - needed because this whole template sits behind
|
||||
smbConnection.enabled, which is false in every other fixture, so nothing else
|
||||
in the repository renders this object at all.
|
||||
*/}}
|
||||
schedule: {{ .Values.smbConnection.cronJob | quote }}
|
||||
{{- /*
|
||||
Forbid, hard-wired rather than a value: two movers running at once race on
|
||||
the same files ("mv /mnt/smb/*.pdf" twice over one directory), and the loser
|
||||
reports a file that is already gone. There is no configuration in which
|
||||
overlapping runs of this job are wanted, so there is nothing to configure.
|
||||
*/}}
|
||||
concurrencyPolicy: Forbid
|
||||
{{- /*
|
||||
Bounds how late a missed run may still start, and it is here for a second
|
||||
reason that has nothing to do with lateness: while the CronJob is suspended
|
||||
(scaleDown) or its runs are being suppressed, the controller counts missed
|
||||
schedules, and above 100 it STOPS SCHEDULING ALTOGETHER with only
|
||||
"Cannot determine if job needs to be started: too many missed start times" in
|
||||
the controller log. With this field set, the controller only counts misses
|
||||
inside this window, so that state cannot be reached. The cost is stated in
|
||||
values.yaml: a run that could not start inside the window is skipped, not
|
||||
queued.
|
||||
*/}}
|
||||
startingDeadlineSeconds: {{ include "paperless-ngx.formatNumber" $cron.startingDeadlineSeconds }}
|
||||
{{- if .Values.scaleDown }}
|
||||
suspend: true
|
||||
{{- end }}
|
||||
jobTemplate:
|
||||
spec:
|
||||
{{- /*
|
||||
THE SAFETY NET UNDER THE AFFINITY BELOW (Gitea #101). activeDeadlineSeconds
|
||||
counts from the job's start, not from the pod's, so it also bounds a pod
|
||||
that NEVER gets scheduled: when the StatefulSet pod is absent and the
|
||||
required affinity cannot be satisfied, the job fails after this many
|
||||
seconds with reason DeadlineExceeded instead of sitting Pending forever
|
||||
and gaining a sibling every schedule. That turns the one new failure mode
|
||||
this issue's fix could introduce into a visible, self-clearing one -
|
||||
failedJobsHistoryLimit (Kubernetes' default, 1) keeps the evidence.
|
||||
*/}}
|
||||
activeDeadlineSeconds: {{ include "paperless-ngx.formatNumber" $cron.activeDeadlineSeconds }}
|
||||
template:
|
||||
spec:
|
||||
{{- if $cron.pinToServerNode }}
|
||||
{{- /*
|
||||
GITEA #101: this pod mounts the consume volume, which is RWX and comes
|
||||
from Longhorn. Without a placement rule the scheduler puts it on any
|
||||
node, and Longhorn's CSI driver then refuses the mount on the node
|
||||
where the StatefulSet pod runs:
|
||||
|
||||
MountVolume.SetUp failed ... code = Aborted desc = no Pending
|
||||
workload pods for volume pvc-... to be mounted:
|
||||
map[Running:[...-sfs-0] Succeeded:[...-consume-cronjob-...]]
|
||||
|
||||
Measured at every two minutes, 291 events in 10 hours, in both
|
||||
releases that have SMB switched on; the release with
|
||||
smbConnection.enabled: false and therefore no CronJob produced none of
|
||||
them on identically built volumes.
|
||||
|
||||
WHY podAffinity AND NOT nodeSelector: the StatefulSet pod is not
|
||||
pinned either - it sits wherever the scheduler put it and moves on
|
||||
every restart. A static nodeSelector would be correct exactly until
|
||||
then and silently wrong afterwards. The affinity follows the pod.
|
||||
|
||||
WHY required AND NOT preferred: preferred falls back to another node
|
||||
in the very situation this exists for, which reproduces the bug more
|
||||
rarely and therefore harder to find. The fallback is what went
|
||||
unnoticed for ten hours.
|
||||
|
||||
The label is the one the StatefulSet puts on its pod template
|
||||
(paperless-server.sfs.yaml, metadata.labels.app) and is read off the
|
||||
running pod, not assumed.
|
||||
|
||||
WHAT THIS DOES NOT FIX: the volume stays RWX and keeps its
|
||||
share-manager; co-locating the two consumers removes the CROSS-NODE
|
||||
attach, not the second consumer. The design that removes it is moving
|
||||
the mv into the StatefulSet pod, which cannot be done for the running
|
||||
installations because RWX cannot be taken back off an existing PVC.
|
||||
Gitea #101 names it as the alternative and not as the proposal.
|
||||
*/}}
|
||||
affinity:
|
||||
podAffinity:
|
||||
requiredDuringSchedulingIgnoredDuringExecution:
|
||||
- labelSelector:
|
||||
matchLabels:
|
||||
app: {{ .Release.Name }}-{{ .Chart.Name }}-sfs
|
||||
topologyKey: kubernetes.io/hostname
|
||||
{{- end }}
|
||||
automountServiceAccountToken: false
|
||||
securityContext:
|
||||
{{- include "paperless-ngx.cron.podSecurityContext" .Values.smbConnection.cron.securityContext | nindent 12 }}
|
||||
{{- include "paperless-ngx.cron.podSecurityContext" $cron.securityContext | nindent 12 }}
|
||||
containers:
|
||||
- name: paperless-consume-cronjob
|
||||
image: "{{ .Values.paperless.busybox.image.repository }}:{{ .Values.paperless.busybox.image.tag }}"
|
||||
imagePullPolicy: IfNotPresent
|
||||
securityContext:
|
||||
{{- include "paperless-ngx.cron.containerSecurityContext" .Values.smbConnection.cron.securityContext | nindent 16 }}
|
||||
{{- include "paperless-ngx.cron.containerSecurityContext" $cron.securityContext | nindent 16 }}
|
||||
{{- if $cron.env }}
|
||||
env:
|
||||
{{- range $value := $cron.env }}
|
||||
{{- if $value.value }}
|
||||
{{- /*
|
||||
quote is required, not cosmetic: a value like "8080" or "true"
|
||||
renders unquoted as a YAML number/bool, and the API server then
|
||||
rejects the whole manifest with "cannot unmarshal number into
|
||||
Go struct field EnvVar.value of type string". A template
|
||||
comment and not a "#" one, because a "#" line here is part of
|
||||
the rendered manifest.
|
||||
*/}}
|
||||
- name: {{ $value.name | quote }}
|
||||
value: {{ tpl $value.value $ | quote }}
|
||||
{{- else if $value.valueFrom }}
|
||||
{{- if $value.valueFrom.secretKeyRef }}
|
||||
- name: {{ $value.name }}
|
||||
valueFrom:
|
||||
secretKeyRef:
|
||||
name: {{ tpl $value.valueFrom.secretKeyRef.name $ | quote }}
|
||||
key: {{ tpl $value.valueFrom.secretKeyRef.key $ | quote }}
|
||||
{{- else if $value.valueFrom.configMapKeyRef }}
|
||||
- name: {{ $value.name }}
|
||||
valueFrom:
|
||||
configMapKeyRef:
|
||||
name: {{ tpl $value.valueFrom.configMapKeyRef.name $ | quote }}
|
||||
key: {{ tpl $value.valueFrom.configMapKeyRef.key $ | quote }}
|
||||
{{- end }}
|
||||
{{- end }}
|
||||
{{- end }}
|
||||
{{- end }}
|
||||
command:
|
||||
- /bin/sh
|
||||
- -c
|
||||
@@ -33,7 +172,7 @@ spec:
|
||||
mountPath: "/mnt/consume"
|
||||
readOnly: false
|
||||
resources:
|
||||
{{- include "paperless-ngx.cron.effectiveResources" .Values.smbConnection.cron.resources | nindent 16 }}
|
||||
{{- include "paperless-ngx.cron.effectiveResources" $cron.resources | nindent 16 }}
|
||||
restartPolicy: Never
|
||||
volumes:
|
||||
- name: smb-share
|
||||
@@ -42,4 +181,4 @@ spec:
|
||||
- name: consume-folder
|
||||
persistentVolumeClaim:
|
||||
claimName: {{ .Release.Name }}-{{ .Chart.Name }}-consume-volume-claim
|
||||
{{- end }}
|
||||
{{- end }}
|
||||
Reference in new issue
Block a user