feat(stalwart): spread each node group over nodes and give each one a PodDisruptionBudget #97
No Assignees
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: jlab-cluster/helm-charts#97
Reference in new issue
Block a user
Renders a distribution rule and a
PodDisruptionBudgetper node group forstalwart, and adds a CI assurance over all charts that every rendered selector really matches the pods it is meant to select.Fixes #96
What is rendered now
Per node group, both gated on that group's computed replica count being
>= 2(soscaleDown: trueremoves them):plus
<release>-stalwart-<group>-pdbwithminAvailable: 1and the same selector. New per-group values:spreadAcrossNodes,podDisruptionBudget.{enabled,minAvailable}, and a generic pass-throughaffinity/nodeSelector/tolerations/topologySpreadConstraints. NoupdateStrategyand nopodManagementPolicy— the Kubernetes defaults already do the right thing, as the issue says.The selector is the chart's, never the operator's.
stalwart.groupLabelsis the one definition behind the pod template's labels, the spread selector and the PDB selector — precedent in this chart isstalwart.env.dbPassword, for the same reason: a mismatch between two copies is not a render error. A spread constraint whose selector matches no pod is trivially satisfied (a condition over zero pods), so it distributes nothing with a green rollout and no event; a PDB whose selector matches no pod never blocks an eviction while looking like protection.nodeAffinityPolicy: Ignoreis set explicitly, and that is the load-bearing detail. Its API default isHonor, under which a narrowingnodeSelectorshrinks the set of counted domains until both replicas fit into one: skew 0, both pods co-located, noPending, no event, and a selector that is correct — so no CI check can see it either. WithIgnorethe same narrowing gives2,0,0, skew 2, and the second pod isPending. A render abort was the alternative and was rejected because it would also have forbidden anodeSelectorspanning five nodes, which is legal and works. Verified against the rendered output rather than taken from the reasoning: the constraint carriesnodeAffinityPolicy: Ignoreand nonodeTaintsPolicy, so the latter is at its API default ofIgnore— which is where the replica ceiling comes from, and it predates this change.A collision aborts the render naming both values. A user entry on
kubernetes.io/hostnamewithwhenUnsatisfiableDoNotScheduleor omitted (which is the API default) collides with the chart's own on(topologyKey, whenUnsatisfiable).kubectl createrejects the duplicate with a message about a list index instead of a value, and an apply-based path merges the two overpatchMergeKey: topologyKeysilently, dropping onelabelSelectorfor a green result. The abort fires only while the chart's own entry is actually rendered.Minor, and the diff is the evidence
1.0.1+upv0.16.24→1.1.0+upv0.16.24. Both objects are gated on>= 2and the chart shipsfull: 1/mx: 0, so a default install renders exactly what it rendered before:Both empty, byte-for-byte, against
origin/main=6e196c2. One rendered YAML comment in the pod template was deliberately kept as a YAML comment rather than folded into a template comment, because turning it into one was the only thing that made the diff non-empty.The CI assurance, and what its fail path caught
New job
selector-matchinvalidate-charts.yml, runningci/run-selector-match-test.shover every chart on every PR — the same argument as the quantity render test: the mistake arrives by copy into a chart whose own diff nobody reads for it.It pulls every
PodDisruptionBudgetselector and everytopologySpreadConstraintslabelSelectorout of a render and holds it against the labels of the pod templates in that same render — a spread constraint against its own pod template, a PDB against any pod template present. Measured: 30 charts, 5 selectors verified, ~14 s, no cluster.Helm is the YAML parser, through the one-template chart
ci/yaml2json({{ .Values | toJson }}): the runners have neitherpyyamlnoryq, and a parser written inawkhas the one failure mode this repository removes rather than adds — reading nothing, finding no selector, reporting no mismatch, green. The only textual step is the document split on a column-one---, and getting that wrong does not produce a false pass: the fragment stops being valid YAML andhelmexits 1.The fail path was exercised, not assumed. Replacing the PDB's
includewith a hand-written second copy and renamingnode-grouptonodegroup:The same was done for the spread constraint's
labelSelector(twoFAILlines, exit 1). Also exercised: thematchExpressions-only path reports "gap" and is not counted as passing (2 selector(s) matched, 2 not checked), and the overlay guard — a chart with a file underci/selector-match/that renders no selector fails, probed with a throwawaycyberchef.yaml.Deliberately out of scope and named so the gap is not mistaken for coverage:
podAffinity/podAntiAffinity(a term matching nothing is a legitimate configuration there), and set-basedmatchExpressions.What is NOT measured, and will not be by this PR
The acceptance list of #96 asks for render statements only, and every one of them is demonstrated above. These are not:
fullpods really land on two different nodes, and that the third would goPendingon a three-worker cluster, is reasoning from the API semantics plus the rendered object — no cluster was involved. The four skew cases from the decision comment were re-checked against the rendered constraint, not re-derived.kubectl drain. That the budget actually holds one pod of a group during a node drain is untested here.nodeTaintsPolicy. "As many replicas per group as there are schedulable workers" follows from control-plane domains holding the minimum at 0 withmaxSkew: 1; it was not observed on a cluster.nodeAffinityPolicybeing pruned on an old API server. Documented as the pruning behaviour for a field behind a disabled gate, explicitly flagged invalues.yamlandNOTES.txtas not measured against a pre-1.26 server.ci/stalwart/test-values.yamlruns one replica infulland zero inmx, deliberately (a second pod needs a real in-memory store, and a runningmxpod would fail-open into a second full node against an empty database). So the install-test renders neither the PDB nor the spread constraint. That is exactly whyci/selector-match/stalwart.yamlexists as a render-only overlay, and why a missing-selector result with an overlay present is a failure rather than a shrug.helm lint --strict, both render tests, the schema in both directions and every fail path were run locally.Also in this PR
README.md: a new "Selector-match render test" section, and three bullets in §11 (PDB per group, the distribution-rule convention with its three traps, and the CI assurance).ci/helm-dependencies.shnow runs in six places, not five.stalwart(values + operating notes),CI-CD(the new job),Chart-Style-Guide(the convention),Lessons-Learned(the asymmetric policy defaults, and the silent merge overpatchMergeKey).NOTES.txtprints, per group, which case applied — spread enforced / not rendered / off, PDB rendered with itsminAvailable/ not rendered and why — plus the version caveat and the ceiling, once, when any group has the constraint.