Skip to content

vmcp: operational.timeouts and partialFailureMode are validated and documented but never read #6164

Description

@jerm-dro

Bug description

Two operational config fields on VirtualMCPServer are fully plumbed as configuration surface — declared in the Go config structs, validated on input, defaulted, present in the CRD schema, and documented — but no production code ever reads them. Setting them has no effect, and there is no warning or error to indicate that.

  • operational.failureHandling.partialFailureMode (fail | best_effort)
  • operational.timeouts.default and operational.timeouts.perWorkload

The failure mode is silence: the operator accepts the value, the pod starts cleanly, the CRD documents what it should do, and nothing changes. This is arguably worse than an unimplemented feature that is absent, because the documented presence of the knob actively misleads.

Evidence

partialFailureMode — referenced only by config plumbing:

  • Declared: pkg/vmcp/config/config.go:651
  • Validated: pkg/vmcp/config/validator.go:466-468
  • Defaulted to fail: pkg/vmcp/config/defaults.go:32,62
  • In the CRD: deploy/charts/operator-crds/files/crds/toolhive.stacklok.dev_virtualmcpservers.yaml:1779
  • Documented: docs/operator/crd-api.md:502, and docs/operator/virtualmcpserver-kubernetes-guide.md:562 recommends best_effort
  • Consumers: none. A repo-wide search for the field outside pkg/vmcp/config/ and generated deepcopy code returns no production hits.

Separately, capability aggregation is hardcoded to best-effort behaviour regardless of the setting: QueryAllCapabilities logs and continues past a failing backend (pkg/vmcp/aggregator/default_aggregator.go:173-175), failing only when every backend fails (:192). So the effective behaviour is always best_effort, and a user who explicitly sets fail — the default — does not get it.

operational.timeouts — same pattern:

  • Declared: pkg/vmcp/config/config.go:611,615
  • Validated: pkg/vmcp/config/validator.go:416-423
  • Defaulted to 30s: pkg/vmcp/config/defaults.go:35,55
  • In the CRD: ...virtualmcpservers.yaml:1820; documented at docs/operator/crd-api.md:749
  • Consumers: none. Only *_test.go files reference them.

Note there is a ready-made seam for the timeout: vmcpsession.WithBackendInitTimeout (pkg/vmcp/session/factory.go:155) exists and has no production caller. The session factory currently uses a compiled-in defaultBackendInitTimeout = 30 * time.Second (factory.go:31).

Steps to reproduce

apiVersion: toolhive.stacklok.dev/v1beta1
kind: VirtualMCPServer
spec:
  config:
    operational:
      failureHandling:
        partialFailureMode: best_effort
      timeouts:
        default: 5s

Apply it. The resource is accepted and the pod starts. No behaviour changes, and nothing in the logs indicates the settings were ignored.

Expected behavior

Either the fields take effect, or the system tells you they don't. Any of:

  1. Implement them.
  2. Remove them from the config structs, CRD, and docs.
  3. Keep the schema for compatibility but log a clear warning at startup when a non-default value is set, and mark them unimplemented in the docs.

Actual behavior

Silently ignored.

Why this is worth fixing beyond tidiness

This has already misled users twice.

1. It set a false premise in #5861. That report opens by assuming these are working features:

"health monitoring + circuit breaker + partial-failure-mode (best_effort) already exist as designed features for exactly this kind of scenario"

2. A user in #5861 reasoned carefully about a tradeoff for a knob that does nothing:

"We considered raising operational.timeouts.perWorkload for just this backend, but since its own worst-case latency is 15-25+s, doing so would only convert 'occasional hard failure' into 'every session against this tenant reliably takes 15-25s'"

They rejected it on a cost/benefit analysis of behaviour that does not exist. Had they decided the other way, they would have set it, observed no change, and had no way to find out why — while debugging a production incident.

3. #4856 is related but did not fix this. That issue reported a CrashLoopBackOff when setting partialFailureMode: best_effort, caused by a validator/CRD enum mismatch. It was fixed in #4865 and closed as completed. That fix corrected the validation so the pod no longer crashes — it did not make the field do anything. So the current state is strictly more confusing than before: previously setting best_effort failed loudly; now it succeeds and silently does nothing.

Additional context

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workingneeds-triageIssue needs initial triage by a maintaineroperatorvmcpVirtual MCP Server related issues

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions