Repository navigation
fix: wait-for-postgres probes every database host the pod will open (#4173) - #4219
Conversation
…onfig value The `wait-for-postgres` init container exists to make one thing deterministic: "Postgres and the portal start concurrently on a fresh install; the portal's hub init FAILS (and the host aborts) if Postgres isn't yet accepting connections." It probed ONE host, and that host came from a different input than the connections the process opens. The probe read `config.<half>.MEMEX_HOST` — a CONFIG value. The boot opens `ConnectionStrings__memex` and, under AdoNet clustering, `ConnectionStrings__orleans` — SECRETS rendered from the Key Vault values half that no repository holds. Nothing rendered, asserted or logged that those hosts were the same one. So an explicit `ConnectionStrings__orleans` naming another host — a dedicated orleans server, or the same server under another name — was gated by NOTHING. The pod passes Init:1/1, the silo reaches lifecycle stage RuntimeGrainServices, and MembershipTableManager fails on a name that never resolved (#3780), and the container exits because the portal fails closed. The reading that costs the time is the init container's PASS: it says nothing about the connection that failed, so a gate that was never covering that host reads as a transient resolver blip. The probe is now derived FROM the connection strings — every distinct host:port the half will open, deduplicated, each named in the log as it is waited for. The strings themselves move into templates/_database.tpl, so the portal Secret, the migration Secret and both probes read ONE definition; memex-migration/secrets.yaml already CLAIMED that property ("both are derived by the same rule from the same input rather than typed twice") while the rule was in fact written out twice. No skip-trapdoor: `memex.dbProbeTargets` calls `fail` when no host can be derived, because an init container running `until nc -z ; do …` would pass having probed nothing. Invariant 16 of check-chart-invariants.sh asserts the coverage at render time for BOTH halves, on a new fixture whose mesh and orleans databases are on DIFFERENT servers — the shape nothing in this repo rendered, and #4173's own stated verification. Negative controls, both measured: * With the old probe restored and everything else unchanged, invariant 16 fails on TWO combinations — the shipped `two plain replicas, KEDA off (the memex shape)` and the new fixture: portal: ConnectionStrings__orleans names host 'orleans-db.example.test:6432' that wait-for-postgres does not probe …while the six combinations whose hosts already agreed stay green, so the invariant is not trivially true. * Rendering all seven previously-shipped combinations before and after the refactor, the ONLY substantive difference is the init-container command. Every rendered Secret is byte-identical — the connection strings did not move. Fixes #4173 Refs #3780 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Test Results (shard 0)262 tests 262 ✅ 2m 15s ⏱️ Results for commit 5d3aaf0. ♻️ This comment has been updated with latest results. |
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved critical and moderate findings affect external-AKS probes, runtime Secret overrides, multi-host coverage, and invariant enforcement.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
This PR centralizes Helm database connection derivation and makes portal and migration probes cover their database endpoints.
Changes:
- Adds shared connection and probe helpers.
- Updates workloads, secrets, fixtures, and invariant checks.
- Documents the revised startup behavior.
File summaries
| File | Summary |
|---|---|
src/MeshWeaver.Documentation/Data/WhatsNew/2026-09-13-a-portal-waits-for-every-database-it-will-open.md |
Adds release-note documentation. |
src/MeshWeaver.Documentation/Data/Architecture/DeploymentAKS.md |
Documents database probe coverage. |
deploy/helm/values.yaml |
Updates probe guidance; nit (1 vote) remains due to contradictory Orleans documentation. |
deploy/helm/templates/memex-portal/secrets.yaml |
Uses shared connection helpers. |
deploy/helm/templates/memex-portal/deployment.yaml |
Uses generated probes; moderate (1 vote) runtime override and critical (1 vote) post-render patch findings remain. |
deploy/helm/templates/memex-migration/secrets.yaml |
Uses shared connection helpers. |
deploy/helm/templates/memex-migration/job.yaml |
Uses generated probes; moderate (1 vote) runtime override and critical (1 vote) post-render patch findings remain. |
deploy/helm/templates/_database.tpl |
Defines connection parsing and probes; unresolved moderate and critical findings cover multi-host values, runtime overrides, and external-AKS patching. |
deploy/aks/scripts/testdata/values.dedicated-orleans-host.yaml |
Adds dedicated Orleans-host fixture values. |
deploy/aks/scripts/check-chart-invariants.sh |
Includes the new fixture. |
deploy/aks/scripts/check-chart-invariants.py |
Implements invariant 16; moderate findings cover silently skipped halves (3 votes) and ignored SecretRef overrides (1 vote). |
Review details
Suppressed comments (7)
deploy/aks/scripts/check-chart-invariants.py:582
- Although
_cs_hostreturnshost:port, this comparison discards the port withendpoint.split(":")[0]and only checks a substring. A rendered connection string usingHost=orleans-db.example.test;Port=6432therefore passes if the command probes the same host on5432, so invariant 16 cannot catch a wrong-port regression (and IPv6 hosts are also misread). Compare the exact generated endpoint as a command token.
if endpoint.split(":")[0] not in probed:
deploy/aks/scripts/check-chart-invariants.py:570
_probe_coverageinspects onlystringData/datafrom the chart-owned Secret. It never accounts for the later SecretRefs that can overrideConnectionStrings__memex/__orleans, so invariant 16 can pass while the pod gets a different endpoint and the init misses it. Include every declared envFrom source in the effective-value check, or make the chart forbid those keys outside this Secret.
data = secret_obj.get("stringData") or secret_obj.get("data") or {}
opened = {
key: _cs_host(value)
for key, value in data.items()
if key.startswith("ConnectionStrings__")
deploy/helm/templates/_database.tpl:104
- The empty-list guard only fails when no endpoint is found anywhere. If
ConnectionStrings__memexis a nonempty string withoutHost/Serverwhile an explicit Orleans string supplies a host,$targetsis nonempty and rendering succeeds even though the mesh connection the process will open is not covered by the probe. Track hostless connection strings per connection and fail (or explicitly handle the runtime default), and make invariant 16 reject thoseNoneendpoints.
{{- if not $targets -}}
{{- fail (printf "memex.dbProbeTargets: no database host could be derived for '%s'. Its connection strings name no Host= (or Server=), so the wait-for-postgres init container has nothing to wait for — and an init container that probes nothing would PASS, which is the exact failure #4173 removes. Set secrets.%s.ConnectionStrings__memex to a connection string naming a host." .half .half) -}}
deploy/helm/templates/_database.tpl:114
waitForDatabasesCommandis rendered from.Values.secrets, but the actual container's effectiveConnectionStrings__*can be overridden by the laterkeyVaultSecretClasses/extraEnvFromSecretRefs in the workload templates. This init container has no matching envFrom, so a Key Vault connection string can leave it probing the chart/default host while the process opens the injected host—reintroducing #4173 (or making AKS wait forever on the unrendered service). Make the init read the same effective Secret/env source at runtime, including CSI mount ordering, or explicitly reject connection-string overrides.
{{- define "memex.waitForDatabasesCommand" -}}
{{- printf "for t in %s; do h=${t%%:*}; p=${t##*:}; until nc -z $h $p; do echo 'waiting for postgres at '$h:$p; sleep 2; done; echo 'postgres ready at '$h:$p; done" (include "memex.dbProbeTargets" .) -}}
deploy/helm/templates/memex-migration/job.yaml:75
- The migration container also loads the Key Vault/extra
envFromsources aftermemex-migration-secrets, but this waiter is rendered only from.Values.secrets.memex_migration. If a synced Secret supplies the real externalConnectionStrings__memex/__orleans, the Job waits on the chart fallback instead of the connection it will actually use; with no in-cluster Postgres this can hold every migration forever. The init container and migration container need one authoritative runtime connection-string source, or the chart must reject overrides invisible to the probe.
- {{ include "memex.waitForDatabasesCommand" (dict "root" $ "half" "memex_migration") | quote }}
deploy/helm/templates/memex-portal/deployment.yaml:118
- This command is generated from
.Values.secrets, but the portal's actualConnectionStrings__*environment is loaded later from the Key Vault/extraenvFromsources in this same template, where the last source wins. A supported mapping isConnectionStrings__memexin a synced Key Vault Secret (test/MeshWeaver.Deployment.Contract.Test/Fixtures/memex.json:293-296); when that runtime value contains the external host while the values block is blank/default, this init container probesmemex-postgres-serviceand either hangs withpostgres.enabled=falseor fails to cover the host the process opens. Make the init container consume the same runtime secret sources, or reject connection-string overrides that the probe cannot see.
- {{ include "memex.waitForDatabasesCommand" (dict "root" $ "half" "memex_portal") | quote }}
deploy/helm/values.yaml:926
- The new documentation says external connections can omit
ConnectionStrings__orleansbecause the chart derives it, butvalues.yamlstill says above (847-852) that Helm refuses that exact shape (anddeploy/aks/README.md:1488-1492repeats it). Those instructions now contradictmemex.orleansConnectionString, so operators may supply an unnecessary secret or misread a valid render. Update the operator documentation in the same change.
# 🚨 That external host is now derived from the CONNECTION STRINGS the process opens
# (secrets.<half>.ConnectionStrings__memex and, under AdoNet, __orleans — see
# templates/_database.tpl), not from config.<half>.MEMEX_HOST. The probe read MEMEX_HOST until
# MeshWeaver#4173, and the two come from different halves of the values with nothing comparing
# them: an explicit ConnectionStrings__orleans naming another host was gated by nothing, which is
- Files reviewed: 11/11 changed files
- Comments generated: 5
- Review effort level: Lite
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| {{- define "memex.waitForDatabasesCommand" -}} | ||
| {{- printf "for t in %s; do h=${t%%:*}; p=${t##*:}; until nc -z $h $p; do echo 'waiting for postgres at '$h:$p; sleep 2; done; echo 'postgres ready at '$h:$p; done" (include "memex.dbProbeTargets" .) -}} |
| {{- $pgHost := .Values.postgres.enabled | ternary "memex-postgres-service" (.Values.config.memex_migration.MEMEX_HOST | default "memex-postgres-service") }} | ||
| {{- $pgPort := .Values.postgres.enabled | ternary "5432" (.Values.config.memex_migration.MEMEX_PORT | default "5432") }} | ||
| - "until nc -z {{ $pgHost }} {{ $pgPort }}; do echo 'waiting for postgres'; sleep 2; done" | ||
| - {{ include "memex.waitForDatabasesCommand" (dict "root" $ "half" "memex_migration") | quote }} |
| {{- $pgHost := .Values.postgres.enabled | ternary "memex-postgres-service" (.Values.config.memex_portal.MEMEX_HOST | default "memex-postgres-service") }} | ||
| {{- $pgPort := .Values.postgres.enabled | ternary "5432" (.Values.config.memex_portal.MEMEX_PORT | default "5432") }} | ||
| - "until nc -z {{ $pgHost }} {{ $pgPort }}; do echo 'waiting for postgres'; sleep 2; done" | ||
| - {{ include "memex.waitForDatabasesCommand" (dict "root" $ "half" "memex_portal") | quote }} |
| if obj is None or secret_obj is None: | ||
| return |
| {{- /* `host:port` for one ADO.NET connection string, or the empty string when it names no host. | ||
| Npgsql accepts `Host=` and `Server=`, case-insensitively, and defaults the port to 5432. A | ||
| multi-host value (`Host=a,b`) contributes its FIRST entry — a probe of one member of a | ||
| failover pair is the honest lower bound, and probing a comma-joined string would probe | ||
| nothing at all. */ -}} |
Test Results (shard 1)1 425 tests 1 425 ✅ 1m 41s ⏱️ Results for commit 5d3aaf0. ♻️ This comment has been updated with latest results. |
Test Results (shard 2)883 tests 883 ✅ 3m 22s ⏱️ Results for commit 5d3aaf0. ♻️ This comment has been updated with latest results. |
Test Results (shard 4)1 829 tests 1 637 ✅ 4m 49s ⏱️ Results for commit 5d3aaf0. ♻️ This comment has been updated with latest results. |
Test Results (shard 5) 6 files 6 suites 5m 7s ⏱️ Results for commit 5d3aaf0. ♻️ This comment has been updated with latest results. |
Test Results (shard 3)1 047 tests 1 045 ✅ 6m 27s ⏱️ Results for commit 5d3aaf0. ♻️ This comment has been updated with latest results. |
Test Results 18 files 18 suites 23m 44s ⏱️ Results for commit 5d3aaf0. ♻️ This comment has been updated with latest results. |
|
ℹ️ Merge-queue steward: no action — removed from the queue with reason |
All three are cases where the thing that was supposed to guarantee coverage
could quietly cover nothing.
1. THE GATE COULD SKIP A HALF. `_probe_coverage` returned early — WITHOUT
counting a check — when the object or its Secret was absent. Both the portal
Deployment and the migration Job are unconditional in this chart, so either
going missing is a template regression; making it a silent no-op is the
skip-trapdoor shape this whole script is written against. It now counts the
check first and REPORTS the absence. Measured: suppressing
memex-migration-secrets fails all 8 combinations ("migration:
memex-migration-secrets was not rendered, so there is nothing to compare the
probe to"); before, it failed none.
2. A MULTI-HOST CONNECTION STRING WAS COVERED BY ITS FIRST ENTRY ONLY. Npgsql
accepts `Host=a,b` and may open either, so a probe of `a` could pass while
the process connects to `b` — the same "the gate passed says nothing about
the connection that failed" defect one level down. `memex.dbHostGroup` keeps
every member and the waiter treats the group as "any ONE of these answers",
which is the process's own requirement; demanding a standby be up would block
a valid deployment. The invariant requires every member to be probed. The
dedicated-orleans fixture now carries a failover pair, so the new coverage is
itself exercised: with the old probe restored it reports 6 contradictions
there, naming orleans-a AND orleans-b.
The rendered waiter was EXECUTED against a stub `nc`, twice: it skips an
unreachable failover member and accepts the reachable one, and it retries a
host that is down and then proceeds — so the loop is neither a no-op nor a
hang.
3. deploy.sh HANDED THE CONNECTION STRING OVER AFTER THE RENDER. It patched
both Secrets with MEMEX_PG_CONN once `helm upgrade` had finished, under the
note "Chart-gen gap: the secret template hardcodes the in-cluster pg
connection string". That gap closed when the template started taking the
value from `secrets.<half>.ConnectionStrings__memex`; the patch outlived its
reason and was, by then, harmful twice over — helm replaces a Secret
wholesale, so any later upgrade overwrote the working string (the hazard
memex-portal/secrets.yaml documents and this script was still creating), and
a value arriving after the render cannot reach the init container the render
produced, so that path would have kept probing memex-postgres-service while
the process opened the Flexible Server. It now goes in through a values file
(not `--set`: a connection string carries `;`, `=` and `,`), deleted on exit.
Refs #4173
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
All five review findings acted on — and two of them were real holes in the gate I had just writtenPushed as 1–3.
|
The
|
Fixes #4173
Refs #3780 — see "What this means for #3780" below; that issue is not closed here.
The mechanism, at file:line on
origin/maindeploy/helm/templates/memex-portal/deployment.yaml:104-111(and the identical shape atmemex-migration/job.yaml:70):The probe's host is a config value. The connections the boot actually opens are secrets
(
memex-portal/secrets.yaml:29and:74/:94), rendered from the Key Vault values half that norepository holds.
grep -rn MEMEX_HOST deploy/helm/templates/ scripts/returns four hits — the twoprobes and the two ConfigMaps that publish the value. No comparison anywhere.
So an explicit
ConnectionStrings__orleansnaming another host was gated by nothing, and aderived one was covered only if its inherited host happened to equal
MEMEX_HOST, which nothingchecks. What an ungated host produces at boot is #3780's exact fingerprint:
Init:1/1, the siloreaches
RuntimeGrainServices, andMembershipTableManagerdies onName or service not knownwith no TCP attempt ever made.
🚨 The expensive part is the init container's PASS. It says nothing about the connection that
failed, so a gate that was never covering that host reads as a transient resolver blip. That is the
same shape as a skipped CI job wearing a passing tick.
The fix
deploy/helm/templates/_database.tpl— every database connection string the chart renders,and the endpoints derived from those strings, as ONE definition consumed by the Secret that
carries them and the init container that waits for them.
memex-migration/secrets.yamlalreadyclaimed this property — "both are derived by the same rule from the same input rather than
typed twice" — while the rule was in fact written out twice; now it is written once.
wait-for-postgresinit containers probe every distincthost:portthe half will open(mesh always, orleans under AdoNet), deduplicated, naming each endpoint as they wait — so the
log says which host is not answering.
memex.dbProbeTargetscallsfailwhen no host can be derived. An initcontainer running
until nc -z ; do …would pass having probed nothing, and "the gate could notdetermine what to wait for" must never render as "the gate passed".
check-chart-invariants.sh/.py, for BOTH halves: every host the renderedSecret's connection strings name must appear in that half's
wait-for-postgrescommand. Themigration Job is matched by name prefix (its name carries
.Release.Revision), so the lookupcannot silently find nothing and make half the check vacuous — and an absent object or Secret is
a finding, not an early return, so the check cannot evaluate nothing while the gate reports
success.
deploy/aks/scripts/deploy.shhands the connection string to helm instead of patching bothSecrets afterwards. A value that arrives after the render cannot reach the init container the
render produced — so on that path the probe would have kept naming
memex-postgres-servicewhilethe process opened the Flexible Server. It also removes the hazard
memex-portal/secrets.yamldocuments and this script was still creating: helm replaces a Secret wholesale, so any later
helm upgradeoverwrote the working connection string.Host=a,bfailover list is covered in full, and the waiter treats the group asany ONE of these answers — the process's own requirement. Requiring all of them would block a
deployment whose standby is legitimately down; covering only the first would let the gate pass
having waited for
awhile the process connects tob.values.dedicated-orleans-host.yaml: mesh onmesh-db.example.test:5432,Orleans membership on
orleans-db.example.test:6432,MEMEX_HOSTdeliberately left at the chartdefault naming neither. That is the shape nothing in this repo rendered, and it is wait-for-postgres gates only MEMEX_HOST — an explicit ConnectionStrings__orleans naming another host is gated by nothing #4173's own
stated verification.
🚨 Negative controls — both measured
1. The invariant can fail. Old probe restored, everything else unchanged:
(With the failover pair in the fixture it now reports 6 contradictions there, naming
orleans-aand
orleans-b.) Meanwhile the six combinations whose hosts already agreed stay green, so theinvariant is not trivially true.
1b. The gate cannot skip a half. Suppressing
memex-migration-secretsbehind{{- if false }}now fails all 8 combinations (
migration: memex-migration-secrets was not rendered, so there is nothing to compare the probe to). Before5d3aaf0that suppression failed none of them — thecheck returned early without counting anything, which is the skip-trapdoor shape this script exists
to prevent.
1c. The waiter was EXECUTED, against a stub
nc, because a shell loop nobody ran is anassumption: it skips an unreachable failover member and accepts the reachable one, and it retries a
host that is down for two attempts and then proceeds. Neither a no-op nor a hang. Note that
two plain replicas, KEDA offis the memex.systemorph.com shape — acombination this repo already shipped was un-gated, not just the new fixture. With the fix in place:
All 8 values combinations render a self-consistent deployment.2. The refactor moved no connection string. Rendering all seven previously-shipped combinations
before and after, the only substantive line-level difference across the entire manifest set is the
init-container command:
Every rendered Secret is byte-identical.
What this means for #3780
#3780's cause was never established — the issue named two candidates (a transient resolver blip in
the boot window, or a
ConnectionStrings__orleansnaming a host that does not exist) and its valueslive in Key Vault, which neither I nor the issue's author can read. This change makes the posture
question in it moot rather than deciding it, which is why it is
Refs, notFixes:holds the pod at
Init:0/1until the orleans host resolves, before the silo starts, so a blipcosts a delayed pod rather than a crash loop.
Init:0/1with the offending host named in itsown log, instead of passing and dying in
MembershipTableManageron an error that names nothing.Either way the silo's fail-closed stays correct: by the time it runs, every host it will open
has already resolved and accepted TCP, so a failure there is a real answer rather than a race. No
bounded-resolution-retry is added to the silo, no timer, no watchdog.
I am not closing #3780 on this: it has not recurred, so there is no measurement that would confirm
the cure, and it should be closed after this ships if the boot shape is what the change predicts.
Verified locally
What's New:
Category: Fix— an operator sees a roll that stalled visibly, naming the host, insteadof a pod that passed its gate and then died.
Pairs-with: none — no
publictop-level type or member is removed fromsrc/; the C# side of thisdiff is documentation only.
Mirror-sync: none — no
strings.{en,de}.jsonkey is added or re-worded.🤖 Generated with Claude Code