Skip to content

fix(chart): label the Services so the VMServiceScrape can select them - #35

Merged
Balevka merged 1 commit into
deploy/mcp-helmfrom
fix/vmservicescrape-selector
Aug 7, 2026
Merged

fix(chart): label the Services so the VMServiceScrape can select them#35
Balevka merged 1 commit into
deploy/mcp-helmfrom
fix/vmservicescrape-selector

Conversation

@mikhailak

Copy link
Copy Markdown
Contributor

Both planes have been unobserved in production since ~13:45 UTC today, while both pods stayed Healthy and kept serving. This is an observability outage, not a service outage, which is exactly why nothing paged.

What broke. A VMServiceScrape selects Services by their own metadata.labels. The merged chart carries app: <plane> only under the Services' spec.selector, which selects pods and is a different field. With no labels on the Service objects, neither scrape matched anything. The chart this replaced did label its Service, which is why this worked before the merge.

Evidence, measured:

up{namespace="agent-rpc-mcp"} no series at all
sum(up{job=~"agent-rpc-mcp|agent-rpc-mgmt-mcp"}) steady 2 until 13:45 UTC, nothing after
mcp_ankr_build_info absent, so "which build is running" is no longer answerable from a dashboard
the service itself fine: initialize answers 0.2.0+e9a0b572…, /healthz and /readyz 200, POST /mcp 401

The fix is metadata.labels on both Services, rendered from the same helper the selector already uses so the two cannot drift apart again.

Verified by rendering both ways, not by reading: comparing each VMServiceScrape.spec.selector.matchLabels against the matching Service.metadata.labels reports False for both planes before this change and True for both after.

To reach the cluster this needs helmChartVersion re-pinned to 0.1.1 in the agent-rpc-mcp AppSet in infrastructure-k8s.

Follow-up worth doing. SHARK-3608 already contains the alert for exactly this — McpNoScrapeTarget = absent(up{namespace="agent-rpc-mcp"}) for 10m, whose annotation names the VMServiceScrape and the metrics port as what to check. It is unmerged in infrastructure-observability #310. Had it been merged, this would have paged instead of being found by hand.

Both planes have been UNOBSERVED in production since roughly 13:45 UTC on
2026-08-07, while both pods stayed Healthy and both served traffic normally.

WHAT BROKE. A `VMServiceScrape` selects SERVICES by their own
`metadata.labels`. The merged chart carries `app: <plane>` only under the
Services' `spec.selector`, which selects PODS and is a different field. With no
labels on the Service objects themselves, neither scrape matched anything, so
no target existed to scrape.

The chart this replaced did label the Services, which is why this worked before
the merge and stopped after it. Nothing else changed: the ports are named
`metrics` on both Services and both Deployments, and the pods emit on 9464 as
before.

EVIDENCE, measured rather than inferred:
  - `up{namespace="agent-rpc-mcp"}` returns no series at all;
  - `sum(up{job=~"agent-rpc-mcp|agent-rpc-mgmt-mcp"})` held a steady 2 until
    13:45 UTC and has nothing after it;
  - `mcp_ankr_build_info` is absent, so "which build is running" is no longer
    answerable from a dashboard;
  - the service itself is fine: `initialize` still answers
    `0.2.0+e9a0b572…`, `/healthz` and `/readyz` are 200, `POST /mcp` is 401.
    This is an observability outage, not a service outage, which is precisely
    why nothing paged.

THE FIX is `metadata.labels` on both Services, rendered from the same helper the
selector already uses, so the two cannot drift apart again.

VERIFIED BY RENDERING BOTH WAYS, not by reading: rendering the templates and
comparing each `VMServiceScrape.spec.selector.matchLabels` against the matching
`Service.metadata.labels` reports False for both planes before this change and
True for both after it.

Chart version bumped to 0.1.1. `helmChartVersion` in the `agent-rpc-mcp` AppSet
in `infrastructure-k8s` needs re-pinning to 0.1.1 for this to reach the cluster.

NOTE FOR THE FOLLOW-UP. SHARK-3608 contains an alert for exactly this,
`McpNoScrapeTarget` = `absent(up{namespace="agent-rpc-mcp"})` for 10m, whose
annotation names the VMServiceScrape and the `metrics` port as the things to
check. It is sitting unmerged in infrastructure-observability PR #310. Had it
been merged, this would have paged instead of being found by hand.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@codacy-production

Copy link
Copy Markdown

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.

mikhailak added a commit that referenced this pull request Aug 7, 2026
The two per-plane ArgoCD applications and their two Helm charts were merged into
one application and one chart on 2026-08-07 at about 13:45 UTC, after most of
section 4b was written. Rather than let the file describe yesterday's shape, this
re-reads it against the cluster and the manifests now in `main`.

WHAT IMPROVED, and both were open items in this file:

- The two VirtualServices are now ONE, carrying both routes in written order.
  That closes the ordering risk 4b flagged, and it is verified as SHIPPED rather
  than agreed: the manifest quoted in 4b is the one applied.
- The two superseded chart branches take the pending `0.4.0` release decision
  with them, and PRs #30 and #31 against them are moot. The merged chart carries
  no ingress template at all and requests 256Mi for the data plane by default, so
  two of the three "artifacts describing a deployment nobody runs" are gone.

WHAT BROKE, and it is the more instructive half. The merged chart labels its
Services only under `spec.selector`, which selects PODS. A VMServiceScrape
selects SERVICES by their own `metadata.labels`, which were dropped. Since about
13:45 UTC `up{namespace="agent-rpc-mcp"}` has returned no series and
`mcp_ankr_build_info` has been absent, so "which build is running" stopped being
answerable from a dashboard on the same day it started being answerable.

The images did not move: data is still `e9a0b572…`, mgmt still `9176d12c…`, and
`initialize` still answers `0.2.0+e9a0b572…` on the wire, so the code under
review is the code serving. Fix in PR #35, needs `helmChartVersion` re-pinned to
`0.1.1`.

The point recorded for the reviewer is not about Helm. The pods stayed Healthy,
ArgoCD stayed green and traffic kept flowing, so an observability outage is
invisible to every signal a deployment normally offers. The only thing that
catches it is an alert on the absence of the metrics themselves, and SHARK-3608
already contains exactly that alert, sitting unmerged in
infrastructure-observability #310.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@Balevka
Balevka merged commit a313f7c into deploy/mcp-helm Aug 7, 2026
1 of 2 checks passed
@Balevka
Balevka deleted the fix/vmservicescrape-selector branch August 7, 2026 14:32
mikhailak added a commit that referenced this pull request Aug 7, 2026
Two things settled after the previous commit, both by the SRE who owns the
deploy, and both recorded here as verified rather than as reported.

THE SCRAPE OUTAGE IS OVER. PR #35 merged at 14:32Z, chart 0.1.1, re-pinned and
rolled. Checked rather than accepted, which is the whole point given how the
outage was found in the first place: `up{namespace="agent-rpc-mcp"}` is 1 for
both jobs, `mcp_ankr_build_info` again carries `e9a0b572…` and `9176d12c…`, and
the pod names have changed, so a rollout genuinely happened. The window was about
13:45 to 14:32 UTC.

EDGE RATE LIMITING IS DECIDED, which empties section 6. The option space was
narrower than this file assumed, and the wrong assumption was ours: an Istio
local rate limit is a BLANKET limit in the sidecar with no per-client key, not
the per-IP control we had written it up as. Per-IP at the edge needs a Global
Rate Limit service with a Redis backend and a per-request gRPC hop. Decision
taken jointly: neither, for now. The Global service is disproportionate to
current traffic, the abuse shapes that worried us are already bounded in the
application, and a blanket backstop was considered and also declined. The point
to revisit is the one 4.2 already names: before GA, or when either plane moves
past one replica.

That is a decision a reviewer can disagree with, and the file now says so
explicitly, because disagreeing with it would not change a line of this branch.

Also refreshes the gate battery: 1714 tests pass after SHARK-3619/3620/3622
merged, and the live e2e is 26/26 against the deployment as it stands now.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants