Share OSMO's Redis for MCP proxy state and allow scaling out - #1331
Share OSMO's Redis for MCP proxy state and allow scaling out#1331jiaenren wants to merge 1 commit into
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Comment |
24b388b to
cc93b0d
Compare
The chart refused to render more than one MCP replica whenever the OIDC proxy was enabled, on the premise that FastMCP serializes token refresh within a process. FastMCP says the opposite: "All state is stored in the configured client_storage backend (Redis, disk, etc.) enabling horizontal scaling across multiple instances" (oauth_proxy/proxy.py:212-213), and :1903-1912 handles the distributed refresh race explicitly. The rule also contradicted itself, since sharing state through Redis only matters across replicas that were forbidden from existing. MCP also carried its own Redis host, port and TLS settings. serviceName and port already fell back to services.redis, but tlsEnabled did not, so a deployment whose Redis requires TLS could satisfy every other service and still CrashLoop MCP. Take all three from services.redis like every sibling and keep only dbNumber, which exists to isolate proxy state from OSMO's other Redis users; its default moves off 0 for the same reason. The CI negative test that asserted the replica ban is replaced by a positive assertion that two replicas render, negative-tested by reintroducing a ban and confirming the check fails. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
cc93b0d to
103360d
Compare
|
Consolidated into #1329 to reduce the number of PRs in this stack for review. Sharing OSMO's Redis and allowing replicas is the same theme: MCP inherits what every sibling service already inherits. No content is lost: the cumulative diff of the stack against |
1 participant
Stacked on #1330. Fourth of the MCP authentication simplification stack.
Issue #None
Two fixes
1. The single-replica rule was based on a misreading of FastMCP.
The chart hard-failed on
replicas != 1with the OIDC proxy enabled, citing process-local refresh serialization. FastMCP says the opposite:and
:1903-1912handles the distributed refresh race explicitly ("another worker may have already refreshed and rotated the token"). The rule was also self-contradictory: sharing state through Redis only matters across replicas the chart forbade from existing.2.
oidcProxy.redis.tlsEnableddid not inherit — and that one can CrashLoop the pod.serviceNameandportalready fell back toservices.redis, buttlsEnableddid not. A deployment whose Redis requires TLS could satisfy every other OSMO service and still fail on MCP, with a connection error rather than a config error. All three now come fromservices.redis.Only
dbNumberstays MCP-local, which is its actual job — isolating proxy state from OSMO's other Redis users. Its default moves off0for the same reason.Test change
The negative test asserting the replica ban is replaced by a positive assertion that two replicas render. I negative-tested the replacement by reintroducing a ban and confirming the check fails:
Verification
bazel test //src/service/mcp/...— 72/72 passrender-tests.sh,validate-mcp-chart.sh— passrediss://redis:6379/14🤖 Generated with Claude Code
Checklist