Repository navigation
fix(probes): required_modules is a roll gate — a missing required module holds readiness only - #5754
Conversation
…ule holds READINESS only Policy required-modules-readiness-only: the same rule as bake-gate-readiness-only (#5749). ServiceDefaults.RollGateChecks now names ProbeEndpoints.RequiredModulesCheckName ("required_modules", the name the Plugins host registers), so TagRollGates tags it roll-gate and strips any live tag whatever the host's registration carries. A missing required module stalls the roll and keeps the pod out of the Service; it never fails the startup probe or liveness. /health still runs and prints it. RollGateReadinessOnlyTest: three new cases (startup 200 / readiness 503 / liveness 200 for an untagged Unhealthy required_modules; a live tag is stripped; the constant equals the host's literal). Negative control run: with the name dropped from the set, both behavioural cases fail. Docs: TheBakeGateOnlyStallsARoll table + wiring; PolicyNotProse row; chart comment. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Readiness behavior is not fully enforced for all required-module states or slow checks, and integration and operational documentation gaps remain.
Review effort: Lite
Findings: 1
Open (2)
What changed in this PR
Updates required_modules to act as a readiness-only roll gate rather than affecting startup or liveness probes.
Changes:
- Adds roll-gate tagging and a shared check-name constant.
- Extends probe behavior tests.
- Updates policy, operational documentation, and Helm guidance.
| File | Description |
|---|---|
test/Memex.Portal.Shared.Test/RollGateReadinessOnlyTest.cs |
Tests required-module probe behavior. |
src/MeshWeaver.Documentation/Data/Architecture/TheBakeGateOnlyStallsARoll.md |
Documents roll-gate semantics and operational impact. |
src/MeshWeaver.Documentation/Data/Architecture/PolicyNotProse.md |
Records the readiness-only policy. |
memex/aspire/Memex.Portal.ServiceDefaults/ServiceDefaults.cs |
Tags required modules as a roll gate. |
memex/aspire/Memex.Portal.ServiceDefaults/ProbeEndpoints.cs |
Adds the required-modules check-name constant. |
deploy/helm/templates/memex-portal/config.yaml |
Updates deployment guidance. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| ImmutableHashSet.Create(StringComparer.Ordinal, | ||
| NodeTypeBakeGateExtensions.HealthCheckName, | ||
| ProbeEndpoints.RequiredModulesCheckName); |
There was a problem hiding this comment.
Correct. The host maps ExpectedLater to Degraded, and Degraded is a 200 on every probe, so it holds nothing (the same as before this PR). Fixed in 8124c19 by narrowing the claim. The policy row, the doc table, and the XML docs on ProbeEndpoints.RequiredModulesCheckName and RollGateChecks now say that only the host's Unhealthy verdicts (a required module the image should ship is absent, or a present one did not install against this platform) hold readiness. A new case, ARequiredModuleStillExpectedFromTheStore_HoldsNothing, pins the Degraded boundary at 200 on startup and readiness. I did not change the host's mapping; that would be a separate decision in MeshWeaver.Plugins.
| public void TheRequiredModulesCheckName_IsTheNameTheHostRegisters() => | ||
| Assert.Equal("required_modules", ProbeEndpoints.RequiredModulesCheckName); |
There was a problem hiding this comment.
Agreed. This test sees the core side only. In 8124c19 it is renamed TheRollGateAllowList_NamesTheLiteralThePluginsHostRegistersToday, and its XML doc says it cannot see the host: a rename there leaves it green while TagRollGates stops matching. The doc page states the same limit. Closing the gap needs the Plugins host to register under ProbeEndpoints.RequiredModulesCheckName. That can only happen once an image carrying this constant is the Plugins pin, so it is a follow-up there and not part of this PR.
…althy verdicts Review findings on #5754: (1) a store-delivered module still expected (ExpectedLater) is reported Degraded by the host, a 200 on every probe, so it holds nothing — the policy, docs and XML docs now say only the Unhealthy verdicts (absent / not installable here) hold readiness, and a new case pins the Degraded boundary. (2) the name pin sees the core side only; renamed and documented as such. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
What (Refs #5544, #5704)
Maintainer decision, new policy
required-modules-readiness-only: the required-modules health check (required_modules, registered by the Plugins host) becomes a roll gate, under the same rule #5749 applied to the NodeType bake gate. A missing required module stalls a roll and keeps the pod out of the Service. It never fails the startup probe or liveness.Why. A required module missing from the shelf is a property of what the pod was given. When the registry cannot serve it, it is missing for every image. On the startup probe that "no" killed containers at the end of the startup budget, including restarted pods of the previous image, which is the same failure shape as the 2026-09-25/26 bake-gate outage. The old table in TheBakeGateOnlyStallsARoll listed exactly this as an open question.
How (the #5749 mechanism, extended)
ProbeEndpoints.RequiredModulesCheckName = "required_modules": a new public const, the name the host registers today as a literal.ServiceDefaults.RollGateChecksnow names it next tonodetype_bake.TagRollGates(aPostConfigureover every host's registrations) addsroll-gateand stripslive. Core applies the tag whatever the host's registration carries, so the Plugins host needs no change./healthstill runs the check and prints its reading. Its status and line one exclude it./readyreads it.Verification
RollGateReadinessOnlyTestgains three cases, driven over HTTP on the chart's own probe paths:required_modulesgives startup 200, readiness 503 (named in the body), liveness 200, and still prints on/health;livetag on it is stripped;db_versionnegative control still fails the startup probe.RollGateChecks, both behavioural cases fail (2 of 6). With the name restored, 6 of 6 pass.dotnet build -c Release -warnaserror:Memex.Portal.ServiceDefaults,Memex.Portal.Shared.TestandMeshWeaver.Documentation.Testall report 0 warnings and 0 errors.MeshWeaver.Documentation.Test: 649 of 649 pass.Docs
TheBakeGateOnlyStallsARoll.md: the table row forrequired_modulesis now "No: roll gate". It states the cost: a restart no longer re-runs the bundle-fetch init container by itself, so a pod whose modules arrive later needs a governedRestart. The wiring and guard paragraphs are updated.PolicyNotProse.md: a newrequired-modules-readiness-onlyrow. Thebake-gate-readiness-onlyrow no longer listsrequired_modulesas startup-critical.deploy/helm/templates/memex-portal/config.yaml: a comment that described the old startup-probe consequence.Not established
"required_modules"as a literal. A follow-up there should switch it toProbeEndpoints.RequiredModulesCheckName. Until it does, the equality test pins the two together from this side only.Pairs-with: none — additive only; no public type or member removed.
🤖 Generated with Claude Code