feat(stalwart): spread each node group over nodes and give each one a PDB
Validate Helm Charts / Resource quantities (pull_request) Successful in 20s
Validate Helm Charts / Selector match (pull_request) Successful in 14s
Validate Helm Charts / Detect changed charts (pull_request) Successful in 1m20s
Validate Helm Charts / Build stalwart (pull_request) Successful in 21s
Validate Helm Charts / Trivy config stalwart (pull_request) Successful in 42s
Install-test / Install-test stalwart (pull_request) Successful in 4m31s
Validate Helm Charts / Install-test (pull_request) Successful in 4m55s

A two-replica node group was redundancy on paper: nothing kept both pods off
one node, and no PodDisruptionBudget kept a drain from taking both at once.
Both objects are now rendered per node group, gated on that group's COMPUTED
replica count being >= 2.

The distribution is a promise, not an effort: maxSkew 1 on
kubernetes.io/hostname with whenUnsatisfiable DoNotSchedule, selecting only
this group's own pods, so an "mx" and a "full" pod may share a node. The
selector comes from the chart (stalwart.groupLabels), never from the operator -
a spread constraint whose selector matches no pod is trivially satisfied,
distributes nothing and reports nothing.

nodeAffinityPolicy: Ignore is set explicitly, and that is the load-bearing
detail. Its API default is Honor, under which a narrowing nodeSelector shrinks
the set of counted domains until both replicas fit into one: skew 0, both pods
co-located, no Pending, no event, and a selector that is correct - so no check
can see it. With Ignore the same narrowing gives skew 2 and the second pod is
Pending. A render abort was the alternative and would also have forbidden a
nodeSelector spanning five nodes, which is legal.

A user entry colliding with the chart's own on (topologyKey,
whenUnsatisfiable) aborts the render naming both values, because the API
rejects the duplicate with a message about a list index and an apply-based path
merges the two over patchMergeKey: topologyKey silently, dropping one selector.

Minor rather than major, proved with a diff rather than argued: with the
shipped defaults (full: 1, mx: 0) and with ci/stalwart/test-values.yaml,
helm template renders byte-for-byte what 1.0.1+upv0.16.24 rendered - both
diffs empty.

New in CI, over every chart on every PR: ci/run-selector-match-test.sh holds
every rendered PodDisruptionBudget selector and every topologySpreadConstraints
labelSelector against the labels of the pod templates in the same render. Helm
itself is the YAML parser (ci/yaml2json), because the runners have neither
pyyaml nor yq and a hand-written parser could read nothing and report nothing.
podAffinity/podAntiAffinity are out of scope; matchExpressions-only selectors
are reported as not checked rather than counted as passing. Two anti-vacuum
guards: a chart with an overlay under ci/selector-match/ that renders no
selector fails, and a run that checks no selector at all fails.

Fixes #96
This commit is contained in:
jklein committed 2026-10-07 11:24:37 +00:00
1 parent 6e196c26a5
commit 5ec60e86b6
14 files changed
+1125 -8

No files matched your search

+76
View File
@@ -217,6 +217,82 @@ WHAT A GREEN POD PROVES HERE, AND WHAT IT DOES NOT
the node IP, and so every pod on a node shares one bucket. The defaults cost
6 requests/min/pod and leave room for about 16 pods per node. Tightening them
produces 429s, and a 429 on liveness is a restart loop.
{{- $anySpread := false }}
{{- range $group, $spec := $nodes }}
{{- $r := int (include "stalwart.replicas" (dict "scaleDown" $.Values.scaleDown "replicas" $spec.replicas)) }}
{{- $s := fromYaml (include "stalwart.groupSettings" (dict "group" $group "given" $spec)) }}
{{- if and $s.spreadAcrossNodes (ge $r 2) }}{{ $anySpread = true }}{{ end }}
{{- end }}
DISTRIBUTION AND DISRUPTION BUDGET — WHAT EACH GROUP ACTUALLY GOT
Both objects are conditional, so this is the list rather than something to
infer from `kubectl get pdb` and an `-o yaml`.
{{- range $group, $spec := $nodes }}
{{- $r := int (include "stalwart.replicas" (dict "scaleDown" $.Values.scaleDown "replicas" $spec.replicas)) }}
{{- $s := fromYaml (include "stalwart.groupSettings" (dict "group" $group "given" $spec)) }}
{{ $group }} — {{ $r }} effective replica(s)
{{- if and $s.spreadAcrossNodes (ge $r 2) }}
SPREAD ENFORCED. maxSkew 1 on kubernetes.io/hostname, whenUnsatisfiable
DoNotSchedule, nodeAffinityPolicy Ignore, selecting this group's
own pods only — a pod of "{{ $group }}" sharing a node with a pod of
another group stays fine. This is a PROMISE, so the price is a Pending pod
rather than a co-located one: if fewer nodes are available than
this group has replicas, the surplus replica does not schedule and
there is NO scheduling event to read. Check with
kubectl -n {{ $.Release.Namespace }} get pod -l stalwart.jk-dev.de/node-group={{ $group }} -o wide
{{- else if $s.spreadAcrossNodes }}
SPREAD NOT RENDERED, because this group has {{ $r }} effective replica(s) and the
rule needs two. {{ if eq $r 0 }}With no pod at all there is nothing to
distribute{{ else }}With one pod every placement satisfies it, so the object
would describe nothing{{ end }}. Raise nodes.{{ $group }}.replicas to 2 to get it —
and note that doing so changes the POD TEMPLATE, so {{ if eq $r 0 }}the group
starts with the constraint already in place{{ else }}the pod running now is
replaced in the same rollout round, not merely joined{{ end }}.
{{- else }}
SPREAD OFF — nodes.{{ $group }}.spreadAcrossNodes is false. Nothing keeps two
pods of this group off the same node, so its replicas are
redundancy on paper only unless your own
nodes.{{ $group }}.topologySpreadConstraints or affinity does the job.
{{- end }}
{{- if and $s.podDisruptionBudget.enabled (ge $r 2) }}
PDB {{ $release }}-{{ $group }}-pdb, minAvailable {{ $s.podDisruptionBudget.minAvailable | int64 }}. A node drain cannot take
all {{ $r }} replicas of this group at once.
{{- else if $s.podDisruptionBudget.enabled }}
PDB NONE, because the effective replica count is {{ $r }} and a budget needs
two. Deliberate: {{ if eq $r 0 }}with no pod at all the object would only
describe a promise about nothing{{ else }}minAvailable 1 with a single replica
forbids every voluntary eviction, so `kubectl drain` would block
forever on a pod nothing may move — and the error would name the
PDB rather than the replica count{{ end }}.
{{- else }}
PDB NONE — nodes.{{ $group }}.podDisruptionBudget.enabled is false. A node
drain may take every replica of this group at once.
{{- end }}
{{- end }}
{{- if $anySpread }}
ONE VERSION CAVEAT, AND IT FAILS SILENTLY: nodeAffinityPolicy is a beta field
— on by default from Kubernetes 1.26, GA with the feature gate
NodeInclusionPolicyInPodTopologySpread in 1.33. An older API server, or one
with the gate off, PRUNES IT WITHOUT A WORD, and then a nodeSelector or
affinity that narrows this group onto a single node no longer produces a
Pending pod but two CO-LOCATED ones, with the constraint reported as
satisfied. Compare the live object against what was rendered:
kubectl -n {{ .Release.Namespace }} get sts -l stalwart.jk-dev.de/release={{ $release }} \
-o jsonpath='{range .items[*]}{.metadata.name}{"\t"}{.spec.template.spec.topologySpreadConstraints[*].nodeAffinityPolicy}{"\n"}{end}'
An empty column where "Ignore" should be is the whole symptom.
AND A CEILING THAT IS A RULE, NOT A NUMBER: this chart leaves
nodeTaintsPolicy at its API default of Ignore, so every node in the cluster
counts as a domain — control-plane nodes included, where no Stalwart pod ever
runs. They hold the minimum at 0 permanently, so with maxSkew 1 a group can
never place more replicas than there are SCHEDULABLE WORKERS. It moves with
the cluster: add a worker and it rises, remove one and it falls. Past that
point a group needs spreadAcrossNodes: false or an own constraint with
ScheduleAnyway. This ceiling comes from nodeTaintsPolicy and existed before
nodeAffinityPolicy was set — the two fields are not a package.
{{- end }}
{{- if .Values.ingress.enabled }}
WEB UI AND JMAP