PMM-14738 Validate PMM HA replica counts and publish the supported range - #936
Open
theTibi wants to merge 20 commits into
Open
PMM-14738 Validate PMM HA replica counts and publish the supported range#936theTibi wants to merge 20 commits into
theTibi wants to merge 20 commits into
Conversation
…rview-dashboard PMM-13860: Add PMM_HA_NAMESPACE to statefulset
…ce-user PMM-15309 Add read-only ClickHouse data source user
theTibi
requested review from
a team,
DhruthiKV,
egegunes,
eleo007,
gkech,
hors,
jvpasinatto,
mayankshah1607,
nmarukovich,
oksana-grishchenko,
pooknull and
valmiranogueira
as code owners
August 27, 2026 15:37
theTibi
requested review from
4nte and
ademidoff
and removed request for
a team
August 27, 2026 15:37
This was referenced Aug 27, 2026
Raft elects by majority, so an even replica count needs more votes to elect a leader without surviving more failures, and replicas=2 survives none. HAProxy renders only maxReplicas server-template slots and marks a backend UP only when it answers /v1/server/leaderHealthCheck, so a replica above that ceiling is invisible to HAProxy - and if the Raft leader lands there, no backend is UP at all. Neither was checked or documented. pmm.replicas.validate and pmm.keeper.validate reject both, along with malformed input: sprig's int parses base 0, so "010" would have validated as 8, and anything wider than int64 overflows to 0. Also fixes pmm.haPeers and the HAProxy readiness probe, which built pod hostnames from .Release.Name while the StatefulSet is named after pmm.fullname. The two are equal only when the release name already contains the chart name, so any other release never formed a quorum. Release "pmm-ha" renders identically, so existing installs see no change. maxReplicas now has a single definition, pmm.maxReplicas, shared by the validator and the ConfigMap that consumes it. ci-values.yaml set clickhouse.keeper.replicas, a key nothing reads, so CI ran three Keepers instead of one. Covered by helm unittest suites under charts/pmm-ha/tests: 28 cases, no cluster required, runnable with make test-pmm-ha. Signed-off-by: theTibi <tkorocz@gmail.com>
The helm-unittest plugin.yaml now carries a platformHooks field that Helm 3.15 cannot parse, so the plugin installs but is unusable and make helm-unittest exits 1. test.yaml already runs its unittest jobs on azure/setup-helm@v5.0.0 with Helm v3.21.0; match it rather than pinning the plugin to an older release, since the Makefile target is shared with the pxc suites. Signed-off-by: theTibi <tkorocz@gmail.com>
percona#868 (PMM-15151) and percona#919 (PMM-13858) carry the same .Release.Name -> pmm.fullname fix for the Raft peer list. All three were already functionally identical; this PR differed only in where the $fullname assignment sat and in the wording of the comment, which made _helpers.tpl conflict four ways for no functional reason. Identical text on both sides of a merge resolves cleanly, so the three PRs no longer collide here whichever one lands first. tests/ha-peers_test.yaml still covers the fix. Signed-off-by: theTibi <tkorocz@gmail.com>
percona#937 (PMM-15394) removes templates/haproxy-init-configmap.yaml and the haproxy.initContainers readiness gate. This PR referenced it in three places, so merging both would have failed the ha-peers suite and left stale docs: - tests/ha-peers_test.yaml: dropped the template from the suite and the data["wait-for-pmm.sh"] assertion. - values.yaml, README.md: the HAProxy ConfigMap note now describes the one ConfigMap left after percona#937 - haproxy.cfg, which carries maxReplicas. After percona#937 only one value is embedded in an HAProxy ConfigMap, so the guidance gets simpler rather than weaker. Signed-off-by: theTibi <tkorocz@gmail.com>
percona#938 (PMM-15393) removes the required podAntiAffinity that capped haproxy.replicaCount at the worker-node count, so stating that cap here becomes wrong the moment percona#938 lands - and it conflicted with percona#938, which leaves this line at its original text and documents the new behaviour beside it. Restores the base wording. Signed-off-by: theTibi <tkorocz@gmail.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: ademidoff <81549+ademidoff@users.noreply.github.com>
) The pmm-client StatefulSet added in PMM-14665 (percona#913) has no override in ci/ci-values.yaml, so CI runs 3 Client pods at 100m CPU each. On the single-node Kind cluster that leaves nothing for the third PMM replica: pmm-ha-<release>-2 stays Pending with "Insufficient cpu" and the install hits the 600s timeout. Cap the Clients at 1 and shrink their requests. Also fix clickhouse.keeper.replicas, a key the chart never reads - it is replicasCount - so CI has been running 3 Keepers instead of 1 since January. Together this frees ~450m of requests; the stuck replica needs 200m. Signed-off-by: theTibi <tkorocz@gmail.com>
…cona#868) * PMM-15151: make PG and ClickHouse operators watch all namespaces The pmm-ha-dependencies chart installed the PostgreSQL (Percona PGO) and ClickHouse (Altinity) operators scoped to their own namespace, so they did not reconcile PerconaPGCluster / ClickHouseInstallation CRs created in other namespaces. Installing pmm-ha into a second namespace (e.g. a DR restore target) left PostgreSQL and ClickHouse with no pods, while only VictoriaMetrics came up — its operator is already cluster-scoped. - pg-operator: set watchAllNamespaces=true. This renders WATCH_NAMESPACE="" (all namespaces) and switches the operator RBAC from a namespaced Role/RoleBinding to ClusterRole/ClusterRoleBinding. - altinity-clickhouse-operator: RBAC is already cluster-wide, but the operator only watched its own namespace; set WATCH_NAMESPACES=.* so it watches every namespace. (An empty watch list, documented as "all", did not take effect in 0.25.4.) Lets a DR/secondary namespace be added without manual operator edits. Validated by upgrading pmm-operators with these values: the PG and ClickHouse operators then reconciled CRs in a second namespace, with no disruption to the existing namespace. Signed-off-by: theTibi <tkorocz@gmail.com> * PMM-15151: fix PMM-HA peer DNS to use the StatefulSet name, not the release name The pmm.haPeers helper built the HA memberlist peer FQDNs from .Release.Name (<release>-<ordinal>.monitoring-service...), but the PMM pods are named after the StatefulSet (pmm.fullname): <release>-pmm-ha-<ordinal>. These only coincide when the release is named "pmm-ha". With any other release name (e.g. a DR install named "pmm-dr"), every peer name resolved to a non-existent host, the memberlist could not form, and pmm-managed panicked on startup ("cannot start high availability service: failed to join memberlist cluster ... no such host") — so PMM-HA never came up. Build the peers from include "pmm.fullname" so they match the actual pod hostnames for any release name. No change for a release named "pmm-ha" (fullname == release name). Part of multi-namespace support: lets pmm-ha be installed under any release name (e.g. a secondary/DR namespace) with a working HA cluster. Signed-off-by: theTibi <tkorocz@gmail.com> * PMM-15151: fix HAProxy wait-for-pmm-ready to use the StatefulSet name, not the release name The HAProxy init script (haproxy-init-configmap.yaml) probes each PMM instance at <release>-<ordinal>.monitoring-service..., but the PMM pods are named after the StatefulSet (pmm.fullname): <release>-pmm-ha-<ordinal>. As with pmm.haPeers, these only coincide when the release is named "pmm-ha". With any other release name the probe targets non-existent hosts (HTTPS status 000), wait-for-pmm-ready never passes, and HAProxy never starts — even though PMM itself is healthy. Build pmm_host from include "pmm.fullname" so it matches the actual pod hostnames for any release name. Companion to the pmm.haPeers fix; completes multi-namespace support for PMM-HA. Signed-off-by: theTibi <tkorocz@gmail.com> * Enhance multi-namespace support for ClickHouse and PostgreSQL operators in PMM HA dependencies Signed-off-by: theTibi <tkorocz@gmail.com> * PMM-15151 Match the ClickHouse scrape jobs to the CR names The ClickHouse and Keeper scrape jobs matched the pod's chi/chk label against pmm.fullname, but clickhouse-cluster.yaml and clickhouse-keeper.yaml name those CRs from Release.Name. The two are equal only when the release name already contains the chart name, so with the distinct release name this branch enables (e.g. "pmm-2" -> CHI "pmm-2", fullname "pmm-2-pmm-ha") neither job matched anything and ClickHouse and Keeper metrics were silently dropped. Match on Release.Name, which is what the label actually carries. This is the same naming split the haPeers fix addresses, in the opposite direction - the rest of the ClickHouse wiring (sticky Service selector, pmm.clickhouse.nodes, pmm.clickhouse.keeper.nodes) already uses Release.Name consistently. Renaming the CRs to pmm.fullname instead would recreate the ClickHouse cluster on existing releases, so the regexes move rather than the resources. Signed-off-by: theTibi <tkorocz@gmail.com> * PMM-15151 Document the privilege change and the operator upgrade path - Privileges: the cluster-wide RBAC requirement applies to every install, not only multi-namespace ones, and fails outright where cluster-scoped RBAC cannot be created. Spell out the single-namespace opt-out instead of alluding to it. - Existing installs: the cluster-wide watch is a values default, so operators installed before this version stay namespace-scoped until helm upgrade. Add the upgrade command, note that reinstalling the chart into the new namespace fails on VM CRD ownership, and warn that installing pmm-ha first is not recoverable by retrying - PMM boots without PostgreSQL and Grafana then keeps the default admin password. Signed-off-by: theTibi <tkorocz@gmail.com> * Fix helm upgrade command for pmm-ha-operators * PMM-15151 Address review feedback on multi-namespace support Templates: - Escape the interpolated vmagent scrape-config regexes with regexQuoteMeta. They are fully-anchored RE2 and Helm permits "." in release names, so release "pmm.ha" rendered regex 'pmm.ha' and could keep a different CHI's pods. Applied to all seven, not only the two this PR already touched. - Move the chart-maintainer notes out of the inlineScrapeConfig block scalar into {{- /* */}}, matching _helpers.tpl. They were shipping inside the VMAgent CR, so helm upgrade rewrote the spec and made the operator regenerate the scrape-config Secret and reload every vmagent replica. - Bump haproxy config-version to "4". The init-script ConfigMap fix in this PR is not covered by any checksum, so without the bump helm upgrade reported success while running pods kept the pre-upgrade script. Docs: - Rewrite the multi-namespace section around the real rule: one pmm-ha per namespace (six namespaced objects have fixed names), each with a distinct release name. Note that every instance's kube-state-metrics reads cluster-wide, so namespaces are not a tenancy boundary. Drop the install example, which could not render because pmm-secret must pre-exist, and which duplicated the worked example in the pmm-ha chart README. - Use kubectl rollout status instead of a readiness wait when upgrading the operators. helm upgrade runs without --wait and this is an env-only change, so the readiness wait matched the still-Ready pre-upgrade pod. - Correct the "order matters" box. An unreconciled ClickHouseInstallation parks the PMM pod at Init:1/2 on wait-for-clickhouse, which loops with no timeout, so Grafana never initializes and the default admin password state described previously cannot occur. - Warn that uninstalling this chart stops reconciliation in every namespace, and that retained CRDs block reinstalling it into a different namespace. - Rename the remaining pmm-operators references to pmm-ha-operators. Signed-off-by: theTibi <tkorocz@gmail.com> * PMM-15151 Fix instance discovery and tenancy caveats in docs Address the remaining review comments on the multi-namespace section: - The pre-uninstall check used `helm list -A -f pmm-ha`, but `-f` is a regex over the release name, not the chart. It misses any instance not named "pmm-ha" - which is exactly the convention this document establishes - and returns a plausible non-empty list, so the reader concludes there are no other instances and proceeds with the uninstall. Filter on the chart column instead, anchored with `[0-9]` so it does not also match pmm-ha-dependencies. Same fix in prose form in NOTES.txt. - Drop the forward reference to the worked two-namespace example, which lives in percona#865 and is not on this branch. The host port 9100 constraint is now stated inline, and only the pmm-secret pointer goes to the pmm-ha README, where that section actually exists. The two PRs can now merge in either order. - Extend the "namespaces are not a tenancy boundary" note to cover RBAC. Each instance's ServiceAccount holds cluster-wide read/delete on Secrets, so instances are not isolated from each other's credentials. The ClusterRole predates this PR and is unchanged here; what changed is that this section now recommends the configuration that makes the grant reachable. Docs only - no templates, values or RBAC touched. Signed-off-by: theTibi <tkorocz@gmail.com> * PMM-15151 Address Copilot review feedback - Upgrade docs: tell existing installs to use their own release name (earlier docs used pmm-operators, so `helm upgrade pmm-ha-operators` aborts with `release: not found` for exactly the audience that section targets). - Correct the rollout explanation: the upgrade also swaps pg-operator's namespaced Role/RoleBinding for cluster-scoped ones, and both the pg and ClickHouse operators use `strategy: Recreate`, so the false green comes from the pre-upgrade pod still being Ready while Terminating, not from an overlap window. Only victoria-metrics-operator rolls with overlap. - NOTES.txt: derive the SCOPE message from the values instead of always claiming cluster-wide, so a narrowed install (pg-operator.watchAllNamespaces false, or WATCH_NAMESPACES other than ".*") is reported as such. - Bump pmm-ha-dependencies to 1.1.0: chart-releaser runs with skip_existing and 1.0.0 is already published, so the changed defaults and RBAC would silently never be published. - CI: add a render check for the release-name-sensitive names this PR fixes. `ct install` generates a pmm-ha-prefixed release name, so pmm.fullname equals Release.Name there and the whole bug class is invisible. Renders as release `pmm-2` and asserts the peer list, HAProxy hostnames, CH/Keeper scrape selectors and CHI name, plus `pmm.ha` -> `regex: 'pmm\.ha'`. Needs --dry-run=server because pg-user-credentials-secrets.yaml does an unconditional lookup+fail on pmm-secret. Signed-off-by: theTibi <tkorocz@gmail.com> * PMM-15151 Clarify the release-name requirement for multi-namespace installs The multi-namespace section said only that a distinct release name "flows into the correct pod/peer DNS", which reads as if the name itself has to follow a pattern. QA hit this and asked whether a release name not containing the chart name (e.g. "pmm-shruti") is supposed to work. It is: pod and peer DNS are derived from pmm.fullname, not Release.Name, so any name works and uniqueness in the cluster is the only requirement. Say that explicitly, with an example. Signed-off-by: theTibi <tkorocz@gmail.com> --------- Signed-off-by: theTibi <tkorocz@gmail.com>
…a#865) * PMM-15149: support installing PMM-HA into multiple namespaces The cluster-wide monitoring sub-charts (kube-state-metrics, prometheus-node-exporter) are already conditional on main, which makes it possible to run a second PMM-HA instance in another namespace (e.g. a DR / restore target or an isolated test instance) by disabling them and using a distinct Helm release name. This closes the remaining gaps so that path is clean and documented: - vmagent: gate the kube-state-metrics and node-exporter scrape jobs on the same `kube-state-metrics.enabled` / `prometheus-node-exporter.enabled` toggles. They were rendered unconditionally, so disabling the sub-charts left dangling scrape configs with no targets. Other jobs (kubelet, cadvisor, kube-apiserver, and the PMM components) are untouched. - README: add an "Installing into multiple namespaces" section - operators installed once (cluster-wide), a distinct release name per instance, and monitoring sub-charts disabled on secondary instances (node-exporter uses hostNetwork:9100, so only one set can run per node), plus per-namespace prerequisites (pmm-secret, backup-storage access). - .gitignore: ignore local-only value overrides (values-dev.yaml, values-*.local.yaml) so per-environment files containing real bucket/account/IRSA details are never committed. Backward compatible: both monitoring sub-charts default to enabled, so existing single-namespace installs are unchanged. Verified with `helm template` (default keeps the jobs; disabled drops only the two gated jobs) and `helm lint`. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: theTibi <tkorocz@gmail.com> * Apply suggestion from @ademidoff Co-authored-by: Alex Demidoff <a@demidoff.me> Signed-off-by: theTibi <tkorocz@gmail.com> * PMM-15149: correct multi-namespace monitoring docs (review feedback) Disabling kube-state-metrics / prometheus-node-exporter on secondary instances does NOT reuse the first instance's agents: each vmagent's KSM/node-exporter scrape jobs are scoped to namespaces.names=[.Release.Namespace], so a secondary instance collects no kube-state-metrics or node-exporter metrics at all (those live only in the first instance's VictoriaMetrics). Reword the section + code-block comment to say so; note kubelet/cadvisor (role: node) still provide basic node/container metrics, and that this is fine for a DR target. Addresses review comments on PR percona#865. Signed-off-by: theTibi <tkorocz@gmail.com> * PMM-15149: document namespace creation in multi-namespace install example Signed-off-by: theTibi <tkorocz@gmail.com> * PMM-15149 Gate the kube-state-metrics scrape job via a bundledEnabled helper Mirrors pmm.nodeExporter.bundledEnabled from PMM-15070 for kube-state-metrics, so both gates follow Helm's `condition:` semantics: only a boolean false disables the subchart, while null / non-bool `enabled` values leave it enabled (Helm renders the subchart in those cases, so the scrape job must stay too). Replaces the hand-rolled `index .Values "kube-state-metrics" "enabled"` form. Signed-off-by: theTibi <tkorocz@gmail.com> * PMM-15149 Note OpenShift node-exporter mode in the multi-namespace section With nodeExporter.mode=openshift the scrape job targets the platform exporter in openshift-monitoring rather than the release namespace, so every instance still collects node metrics - only kube-state-metrics stays first-instance-only. Signed-off-by: theTibi <tkorocz@gmail.com> * PMM-15149 Point the multi-namespace docs at the dependencies chart The operator side (install once, cluster-wide watch) and the distinct-release-name requirement are documented in pmm-ha-dependencies' "Multi-namespace support" section (PMM-15151), so link there instead of restating them. This section keeps only the per-instance settings that belong to the pmm-ha chart. Also splits the monitoring sub-chart guidance, which conflated two different cases: - prometheus-node-exporter must be disabled on additional instances - it uses host networking on port 9100, so a second DaemonSet's pods cannot start. - kube-state-metrics is optional. Its resources are named after the release (pmm-dr-kube-state-metrics), so a second instance can run its own copy without colliding, and collect real object-state metrics rather than none. Signed-off-by: theTibi <tkorocz@gmail.com> * PMM-15149 Widen the local value-override ignore pattern values-dev.yaml matched only that exact basename, so the per-environment files this is meant to catch - values-dev-demo2.yaml, values-dev-eks.yaml - were still tracked. values-dev*.yaml covers them; the charts' own values.yaml is unaffected. Signed-off-by: theTibi <tkorocz@gmail.com> * PMM-15149 Note the required node-exporter toggle in OpenShift mode Signed-off-by: theTibi <tkorocz@gmail.com> --------- Signed-off-by: theTibi <tkorocz@gmail.com> Co-authored-by: Alex Demidoff <a@demidoff.me>
* PMM-14403 Finalize the HA resource limitations
Sizes the chart's defaults for ~100 monitored nodes at 30-day retention and
gives every container explicit requests and limits. Several components had
none at all: the PostgreSQL instances, pgBouncer, the PMM client sidecar,
kube-state-metrics and node-exporter all ran as BestEffort, making the pods
that hold PMM's inventory, Grafana state and alert rules the first thing the
kubelet evicts under node pressure. HAProxy carries every client
remote_write, QAN payload and UI request, and inherited a request but no
limit from its subchart.
Adds dataRetentionDays, which drives both vmstorage's -retentionPeriod and
PMM's PMM_DATA_RETENTION. The two were previously independent: vmstorage kept
90 days while Query Analytics silently expired on PMM's own 30-day default.
An explicit victoriaMetrics.vmstorage.retentionPeriod still wins, so installs
already pinning it keep their behaviour.
ClickHouse storage moves from 20Gi to 50Gi per replica. Each replica holds a
full copy of Query Analytics, and 20Gi does not cover 100 services at 30-day
retention once ClickHouse's own system.* tables are accounted for. This is a
PVC expansion on upgrade and needs a storage class with allowVolumeExpansion.
Documents the model behind the numbers in docs/SIZING.md, along with ready
profiles for 500 and 1000 nodes in examples/. The sizing constants there are
measured from a running cluster rather than estimated, and the document shows
how to re-measure them against your own workload.
Verified with helm lint and helm template against the default values, both
example profiles and ci/ci-values.yaml.
Signed-off-by: theTibi <tkorocz@gmail.com>
* PMM-14403 Make dataRetentionDays opt-in so upgrades never shorten retention
Defaulting dataRetentionDays to 30 shortened metrics retention from 90 to 30
days on any install that had taken the chart default, deleting TSDB data that
does not come back. The commit message claimed only installs explicitly
pinning victoriaMetrics.vmstorage.retentionPeriod were affected, which was
wrong.
Two things about how retention works in HA make the single-value design worse
than it looked:
* PMM_DATA_RETENTION governs Query Analytics only. The built-in
VictoriaMetrics that would otherwise consume it does not run in an HA
deployment - supervisord starts only qan-api2, which prunes ClickHouse.
Metrics retention belongs entirely to the operator.
* Setting PMM_DATA_RETENTION locks the setting. PMM's ChangeSettings
returns FailedPrecondition for any retention change while the variable is
present, so the chart was silently removing a control users had in the UI.
dataRetentionDays is now empty by default. Left empty the chart does not touch
either mechanism: vmstorage keeps victoriaMetrics.vmstorage.retentionPeriod,
restored to its original "90d", and PMM_DATA_RETENTION is not emitted, so
Query Analytics stays on PMM's own setting and remains adjustable from the UI.
Upgrades are a no-op. Set it and it drives both, taking precedence over
vmstorage.retentionPeriod, which is what a declaratively managed deployment
wants.
Documents the split in SIZING.md, including that the default 50Gi vmstorage
volume holds roughly 30 days at 100 nodes rather than the 90 the default
retention asks for. Neither default is changed to resolve that, because one
choice deletes data and the other forces a PVC expansion on every install.
Verified across six configurations: defaults, dataRetentionDays at 30/45/90,
an explicitly pinned retentionPeriod, and both set together.
Signed-off-by: theTibi <tkorocz@gmail.com>
* PMM-14403 Keep the CI install within the single-node kind budget
ct install failed with "0/1 nodes are available: 1 Insufficient cpu" - two of
the three PMM pods stayed Pending. ci-values.yaml shrinks resources to fit the
single-node kind cluster, but it did not cover the components this branch gave
requests to for the first time, so those requests landed at their production
defaults:
vmagent 2 x 250m (never overridden, previously 2 x 100m)
kube-state-metrics 100m (previously BestEffort)
node-exporter 50m (previously BestEffort)
PG PMM sidecar 100m (previously BestEffort)
That is +550m against a base branch that was already at 2.75 of roughly 4
available CPU.
Adds CI overrides for those four, plus a 5Gi ClickHouse volume now that the
default is 50Gi. Also fixes clickhouse.keeper.replicas, which is not a key the
chart reads - the value is replicasCount, so CI has been running three Keepers
where it meant to run one.
Request totals rendered from ci-values.yaml:
base branch (CI green) 2.75 CPU 7.28 Gi
this branch, before 3.30 CPU 8.09 Gi
this branch, after 2.49 CPU 6.06 Gi
Signed-off-by: theTibi <tkorocz@gmail.com>
---------
Signed-off-by: theTibi <tkorocz@gmail.com>
…rcona#937) * PMM-15394 Let HAProxy start without waiting for every PMM replica The HAProxy pod ran an init container that required every PMM replica to answer HTTP 200 before HAProxy could start, and exited 1 after five minutes - so the pod CrashLoopBackOffs at Init:0/1 and HAProxy never starts at all. Every fresh install races this, and the chart guarantees it will. The StatefulSet uses the default OrderedReady policy, so pmm-ha-1 is created only after pmm-ha-0 is Ready and pmm-ha-2 only after pmm-ha-1. For most of the five-minute budget the later replicas do not exist, their names do not resolve, and curl returns 000. PMM Server is a multi-GB image that also runs database migrations on first boot, so three replicas started one after another routinely exceed the budget. PMM-15202 is this failure, closed as Not a Bug. The gate was also redundant. haproxy.cfg carries "init-addr last,libc,none", so a backend whose name does not resolve is held with no address rather than aborting startup, and "http-check expect status 200" against /v1/server/leaderHealthCheck brings it UP only once it is genuinely serving. HAProxy running before any PMM pod exists is the designed behaviour: it returns 503 until a backend is up, which beats not running. The init container added no capability - it only turned "HAProxy up, no backends yet" into "no HAProxy". Verified on a live 3-replica cluster. Before the change HAProxy was already serving with seven of ten server-template slots in MAINT (resolution), i.e. unresolved names. After removing the init container, deleting a follower and restarting HAProxy in the same moment brought all three pods to 1/1 Running in under ten seconds with zero restarts, and PMM stayed reachable throughout. The chart's own init script, run unmodified against two absent replicas, exits 1 while three healthy replicas are serving. Removing it also drops the alpine:latest image and its "apk add --no-cache curl" at every HAProxy start, which required egress to the Alpine mirrors. Signed-off-by: theTibi <tkorocz@gmail.com> * PMM-15394 Drop the CI assertions on the deleted HAProxy init script PMM-15151 (percona#868) added a lint-test step asserting that the wait-for-pmm.sh host loop renders pmm_host="<fullname>-$i..." - an assertion on the very file this branch deletes, so it failed the moment PMM-HA-GA was merged in. Git saw no conflict because the assertion and the deletion live in different files. HAProxy no longer builds per-pod names at all: it resolves backends through the headless service with server-template + init-addr, so there is nothing left to assert. The PMM_HA_PEERS half of the check, which does still build per-pod names from pmm.fullname, is kept as-is. Signed-off-by: theTibi <tkorocz@gmail.com> --------- Signed-off-by: theTibi <tkorocz@gmail.com>
Two conflicts, both from work that landed on PMM-HA-GA after this branch forked: - charts/pmm-ha/Chart.yaml: percona#937 bumped 1.6.1 -> 1.6.2 and appVersion to "3.9.1" while this branch bumped to 1.7.0. Kept 1.7.0 (still ahead of 1.6.2, and the render-breaking even-replicas check justifies the minor) with appVersion "3.9.1". - charts/pmm-ha/templates/haproxy-init-configmap.yaml: percona#937 (PMM-15394) deleted the file; this branch had fixed its pmm_host loop to use pmm.fullname. Took the deletion - the fix has nothing left to apply to. Nothing references the ConfigMap, the init container or wait-for-pmm.sh any more. Silent, non-conflicting overlaps checked by hand: - _helpers.tpl pmm.haPeers: percona#868 (PMM-15151) landed the same .Release.Name -> pmm.fullname fix, and this branch had already adopted that block verbatim, so the merge is a no-op there. The peer-addressing suite stays as a regression guard. - README.md: dropped the upgrade bullet announcing the PMM_HA_PEERS change. It ships in 1.6.2 via percona#868, so attributing it to 1.7.0 is now wrong. The even-replicas breaking change is unchanged. - ci/ci-values.yaml: percona#946 (PMM-15441) rewrote the file and already sets clickhouse.keeper.replicasCount, so this branch's fix for the ignored clickhouse.keeper.replicas key is subsumed. - The new examples/ and docs/SIZING.md from percona#935 set no PMM replicas value, so the new validators do not reject them. helm unittest: 3 suites, 27 tests passed. Renders verified against the merged tree - replicas=4, replicas=12 and clickhouse.keeper.replicasCount=2 all fail with the intended messages. Signed-off-by: theTibi <tkorocz@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
PMM-14738
Adds the validation and documentation for PMM HA scaling that the chart never had, plus the test suite that guards it.
Nothing checked the two values that can break a cluster
Rendered at
replicas1 through 12, everything was accepted silently:replicasreplicas=2survives none — one pod restart stops the cluster.maxReplicas. HAProxy rendersserver-template pmm 1-<maxReplicas>and fills the slots from a headless-service DNS answer in arbitrary order, then marks a backend UP only when it answers/v1/server/leaderHealthCheck. A pod without a slot is invisible to HAProxy — and if the Raft leader lands there, every backend is DOWN and PMM serves 503. Not a degraded pod: an outage.pmm.replicas.validateandpmm.keeper.validatenow reject both, plus malformed input. The regex is strict on purpose: sprig'sintparses base 0, so"010"would have validated as 8, and anything wider than int64 overflows to 0 — either way the error would quote a number nobody typed.Suggestions are clamped to the ceiling, so the error never names a value the next check rejects:
clickhouse.keeper.replicasCountis a Raft ensemble too.values.yamlhas always said it "should be odd"; that is now enforceable rather than advisory.Smaller fixes
maxReplicashad its| default 10duplicated between the validator andhaproxy-configmap.yaml. Both now readpmm.maxReplicas, so they cannot drift — and becausedefaulttreats 0 as empty,maxReplicas: 0used to become 10 silently.paths:filter did not include the workflow file, so a PR editing only its assertions ran no job.azure/setup-helmv4.2.0 / Helm 3.15.4 -> v5.0.0 / Helm 3.21.0, which is what thehelm-unittestplugin needs to load.Tests
charts/pmm-ha/tests/— 27 helm-unittest cases, no cluster required, runnable withmake test-pmm-ha:Scoping a suite to
templates/statefulset.yamlskips thelookupinpg-user-credentials-secrets.yamlentirely, so these do not depend on Helm's template ordering the way ahelm templateassertion would.Mutation-tested against the merged tree — every regression is caught:
pmm.haPeersto.Release.Nameserver-templateslot count to a literal10pmm.validate.oddCountalways rejectmaxReplicasceiling checkMerged with PMM-HA-GA
Merging the current base branch dropped two changes this PR originally carried, because equivalent work landed first:
PMM_HA_PEERSfix.pmm.haPeersbuilt peer hostnames from.Release.Namewhile the StatefulSet is named afterpmm.fullname, and the two are equal only when the release name already contains the chart name — sohelm install pmm-prod percona/pmm-hagave Raft peers that never resolved. PMM-15151: make PG and ClickHouse operators watch all namespaces #868 (PMM-15151) shipped the same fix, so there is nothing left to change here.tests/ha-peers_test.yamlstays as the regression guard, including a case asserting that a release namedpmm-hastill renders byte-identically.ci/ci-values.yaml. This PR removedclickhouse.keeper.replicas, a key nothing reads (the template readsreplicasCount). PMM-15441 Fit the pmm-ha CI install back onto the Kind node #946 (PMM-15441) rewrote the file and already setsreplicasCount.templates/haproxy-init-configmap.yamlwas deleted by #937 (PMM-15394), and this PR'spmm.fullnamefix to its host loop went with it. The chart version conflict resolved to1.7.0, keepingappVersion "3.9.1"from #937.Upgrade impact
Breaking for anyone running an even
replicas.2or4was accepted before and now fails the render; those users must set an odd value in the samehelm upgrade, which recreates the pods. The README carries an upgrade note.Chart
1.6.2->1.7.0. Reviewers: a case can be made for2.0.0— a^1.6.2consumer will otherwise adopt a render-breaking change automatically. Happy to bump if you prefer.Related