Repository navigation
introduce VMMCP CRD - #2740
introduce VMMCP CRD#2740megative wants to merge 7 commits into
Conversation
The CRD deploys the MCP server for VictoriaMetrics, the same as the official victoria-metrics-mcp helm chart. For each VMMCP the operator creates a Deployment, Service, ServiceAccount and VMServiceScrape, optionally PodDisruptionBudget, VPA and NetworkPolicy. The MCP server is configured with env vars: the queried instance is set at spec.entrypoint, credentials are passed as Secret references and spec.extraEnvs override env vars set by the operator. Default resources and the startup probe account for the in-memory documentation index, which takes ~700Mi and slows down the start with low CPU limits. The common part of the spec and the factory are shared, so MCP servers for VictoriaLogs and VictoriaTraces can be added on top of them. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
1 issue found across 42 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="internal/webhook/operator/v1/vmmcp_webhook.go">
<violation number="1" location="internal/webhook/operator/v1/vmmcp_webhook.go:38">
P1: This marker is not accompanied by a deployed `ValidatingWebhookConfiguration` rule for `vmmcps`, so installs using the tracked webhook manifests will never call this validator on VMMCP create/update requests. Regenerate and commit the webhook manifest so the new path and resource rule are included.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| Complete() | ||
| } | ||
|
|
||
| // +kubebuilder:webhook:path=/validate-operator-victoriametrics-com-v1-vmmcp,mutating=false,failurePolicy=fail,sideEffects=None,groups=operator.victoriametrics.com,resources=vmmcps,verbs=create;update,versions=v1,name=vvmmcps-v1.kb.io,admissionReviewVersions=v1 |
There was a problem hiding this comment.
P1: This marker is not accompanied by a deployed ValidatingWebhookConfiguration rule for vmmcps, so installs using the tracked webhook manifests will never call this validator on VMMCP create/update requests. Regenerate and commit the webhook manifest so the new path and resource rule are included.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At internal/webhook/operator/v1/vmmcp_webhook.go, line 38:
<comment>This marker is not accompanied by a deployed `ValidatingWebhookConfiguration` rule for `vmmcps`, so installs using the tracked webhook manifests will never call this validator on VMMCP create/update requests. Regenerate and commit the webhook manifest so the new path and resource rule are included.</comment>
<file context>
@@ -0,0 +1,68 @@
+ Complete()
+}
+
+// +kubebuilder:webhook:path=/validate-operator-victoriametrics-com-v1-vmmcp,mutating=false,failurePolicy=fail,sideEffects=None,groups=operator.victoriametrics.com,resources=vmmcps,verbs=create;update,versions=v1,name=vvmmcps-v1.kb.io,admissionReviewVersions=v1
+type VMMCPCustomValidator struct{}
+
</file context>
There was a problem hiding this comment.
Done, the VMMCP validator is in config/webhook/manifests.yaml now. make manifests doesn't write that file, so the entry is added by hand in the same format as the others.
| return s.Validate() | ||
| } | ||
|
|
||
| // validateMCPListItems checks items of the list, which is passed to the MCP server as a comma-separated env var value |
There was a problem hiding this comment.
| // validateMCPListItems checks items of the list, which is passed to the MCP server as a comma-separated env var value | |
| // validateMCPListItems checks items of the list - headers or tools, which is passed to the MCP server as a comma-separated env var value |
| entrypoint: http://vmselect-example:8481 | ||
| instanceType: cluster | ||
| defaultTenantID: "0" | ||
| replicaCount: 2 |
There was a problem hiding this comment.
Lets use a more realistic scenario - single replica (I'm not even sure whether two replicas make sense at all)
There was a problem hiding this comment.
Done, the example runs a single replica now. The High availability section of the docs describes when several replicas work.
| // mcpCR is implemented by CRDs of MCP servers | ||
| type mcpCR interface { | ||
| client.Object | ||
| crObject | ||
| PrefixedName() string | ||
| GetServiceAccountName() string | ||
| } | ||
|
|
||
| // OnVMMCPDelete deletes all VMMCP related resources | ||
| func OnVMMCPDelete(ctx context.Context, rclient client.Client, cr *vmv1.VMMCP) error { | ||
| return onMCPDelete(ctx, rclient, cr, &cr.Spec.CommonMCPSpec) | ||
| } |
There was a problem hiding this comment.
We don't need an extra wrapper and this interface
There was a problem hiding this comment.
Done, OnVMMCPDelete removes the finalizers directly now, like the other finalizers.
| crMeta := metav1.ObjectMeta{ | ||
| Name: "test", | ||
| Namespace: "default", | ||
| Finalizers: []string{vmv1beta1.FinalizerName}, |
There was a problem hiding this comment.
Set OwnerRefs here too and check that those are retained on finalize
There was a problem hiding this comment.
Done, the test sets owner references on the VMMCP and its child objects and checks that they're retained.
| "github.com/VictoriaMetrics/operator/internal/controller/operator/factory/k8stools" | ||
| ) | ||
|
|
||
| func quantityBytes(s string) int64 { |
There was a problem hiding this comment.
Use resource.MustParse instead
There was a problem hiding this comment.
Done. It compares with resource.MustParse(...).Equal(), since quantities read back from the fake client keep their string form and don't match with assert.Equal.
| // +kubebuilder:rbac:groups=operator.victoriametrics.com,resources=vmmcps,verbs=get;list;watch;create;update;patch;delete | ||
| // +kubebuilder:rbac:groups=operator.victoriametrics.com,resources=vmmcps/status,verbs=get;update;patch | ||
| // +kubebuilder:rbac:groups=operator.victoriametrics.com,resources=vmmcps/finalizers,verbs=update | ||
| // +kubebuilder:rbac:groups="networking.k8s.io",resources=networkpolicies,verbs=* |
There was a problem hiding this comment.
| // +kubebuilder:rbac:groups="networking.k8s.io",resources=networkpolicies,verbs=* | |
| // +kubebuilder:rbac:groups="networking.k8s.io",resources=networkpolicies,verbs=get;create;update;patch;delete |
# Conflicts: # config/crd/overlay/crd.descriptionless.yaml # config/crd/overlay/crd.specless.yaml # config/crd/overlay/crd.yaml # docs/CHANGELOG.md
- OnVMMCPDelete removes finalizers directly, without an interface and a wrapper - the finalizer test checks that owner references are retained - the examples run a single replica - tests compare memory with resource.MustParse - the networkpolicies RBAC lists the verbs the controller uses
- spec.entrypoint must have a host, since the CRD pattern checks only the scheme - the unit test checks that the NetworkPolicy is created and removed - the e2e test sends notifications/initialized after initialize, as MCP requires, and checks that all child objects are deleted - docs: when the ServiceAccount and VMServiceScrape aren't created, VMAuth in the security example, session affinity with serviceSpec.useAsDefault
There was a problem hiding this comment.
All reported issues were addressed across 11 files (changes from recent commits).
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
vrutkovs
left a comment
There was a problem hiding this comment.
Three things left to fix:
- proper wiring for the webhook - #2740 (comment)
- don't delete the serviceaccount if we don't own it - #2740 (comment)
- listen to more changes - #2740 (comment)
| resources: | ||
| - ingresses/finalizers | ||
| verbs: | ||
| - '*' |
There was a problem hiding this comment.
| - '*' | |
| - get | |
| - create | |
| - update | |
| - patch | |
| - delete |
There was a problem hiding this comment.
config/rbac/role.yaml is generated by make manifests (Makefile) from the +kubebuilder:rbac markers, so a manual change here gets reverted:
$ git diff --stat config/rbac/role.yaml # after applying the suggestion
config/rbac/role.yaml | 6 +++++-
$ make manifests
$ git diff --stat config/rbac/role.yaml # no changes: the suggestion is gone
This '*' is for ingresses/finalizers and comes from the marker in finalize/common.go. networkpolicies get '*' from the markers of 13 other controllers, e.g. vmagent and vmauth. Should I change those markers to explicit verbs too, or keep this PR to the VMMCP marker?
…ld objects - the ValidatingWebhookConfiguration calls the VMMCP validator on create and update - deleting a VMMCP doesn't touch a ServiceAccount set in spec.serviceAccountName - the controller watches the ServiceAccount, PodDisruptionBudget, NetworkPolicy, VMServiceScrape and, with VPA API enabled, VerticalPodAutoscaler, so it restores deleted or changed objects right away - the e2e test checks that a deleted PodDisruptionBudget is restored before the periodic resync
# Conflicts: # docs/CHANGELOG.md # docs/env.md
Summary
Adds the
VMMCPCRD (operator.victoriametrics.com/v1) for MCP server for VictoriaMetrics, which gives AI assistants access to VictoriaMetrics data and documentation over Model Context Protocol.Part of #2739.
VLMCPandVTMCPfor MCP servers of VictoriaLogs and VictoriaTraces are prepared on top of this PR and will be opened one by one after it, to keep each PR small.Design
VMMCPmirrorsvictoria-metrics-mcp, objects are namedvmmcp-<name>, askindOverridein the chart. The common part of the spec (CommonMCPSpec) and theinternal/controller/operator/factory/mcppackage are shared, soVLMCPandVTMCPonly add settings of their instances.Deployment,Service,ServiceAccountandVMServiceScrapefor/metrics. Optional:PodDisruptionBudget,VerticalPodAutoscaler,NetworkPolicy, additional service,updateStrategy/rollingUpdate.spec.extraArgsis rejected by validation, and env vars fromspec.extraEnvsoverride env vars set by the operator.spec.entrypoint(VMSingle or vmselect) withspec.instanceType: single|cluster, as in the chart, orspec.cloudAPIKeyfor VictoriaMetrics Cloud. Bearer token and Cloud API key are Secret references passed viavalueFrom.secretKeyRef, the operator doesn't read them. Custom headers, default tenant, passthrough headers, disabled tools and resources, heartbeat interval and logging are supported.spec.serverMode:http(default, Streamable HTTP at/mcp) orsse(/sseand/message).stdioisn't allowed, it can't be used in Kubernetes./health/readiness, liveness at/health/livenessand a TCP startup probe withfailureThreshold: 60. The MCP server loads an in-memory index of the built-in documentation before it starts listening; measured start time: 11s with 1 CPU, 22s with 0.5 CPU, 85s with 0.2 CPU.documentationtool disabled), so requests are 100m / 768Mi and limits are 500m / 1Gi.VM_MCP_METRICS_VERSIONenv var (v1.20.2) with a renovate annotation. It uses thegithub-releasesdatasource, sinceghcr.io/victoriametrics/mcp-victoriametricshas a mistakenv1.120.1tag (built from the same commit asv1.20.1), which thedockerdatasource would pick as the latest version.ghcr.io, soVM_CONTAINERREGISTRYdoesn't apply to it,VM_VMMCPDEFAULT_IMAGEcan point to a mirror.Review notes
docs/api.md,docs/env.md, RBAC)Core logic:
api/operator/v1/mcp_types.go,api/operator/v1/vmmcp_types.go- API types and validationinternal/controller/operator/factory/mcp/- reconciliation and env vars of the MCP serverinternal/controller/operator/factory/build/defaults.go,internal/config/config.go- defaultsinternal/controller/operator/factory/finalize/mcp.go,internal/controller/operator/vmmcp_controller.go,internal/webhook/operator/v1/vmmcp_webhook.goFollow-ups
VLMCPandVTMCP- prepared, will be opened after this PR.VMUsertargetRef.crdkinds for MCP servers: MCP servers don't authenticate clients, so the docs recommend puttingVMAuthin front of them.Testing
Automated
extraEnvsoverrides; default probes and resources; deployment params; optional objects creation and removal; finalizer. Coverage of the factory package is 84%.test/e2e/mcp_test.go, labelvmmcp):VMSingleandVMMCPwith strict security are created, a real MCP session (initialize+tools/callof thequerytool) is made through the Kubernetes API server proxy and returns data of VMSingle; thenlogLevelandpodDisruptionBudgetare updated, and objects are removed after CR deletion.Manual, at local kind cluster
The operator image was built from this branch (dd1be89) and deployed to kind via
config/default. The stand already hadVMSingle+VMAgent, which scrapes allVMServiceScrapeobjects.VMMCPwith default settings was created for the existingVMSingle:The MCP server was reached via
kubectl port-forwardand called over Streamable HTTP with curl (initialize, thentools/callwith theMcp-Session-Id):operationalin 27s, since the MCP server starts listening after loading the documentation index.Deployment,Service,ServiceAccountandVMServiceScrapeare created, the container has the default resources (requests 100m / 768Mi, limits 500m / 1Gi), a TCP startup probe, liveness at/health/livenessand readiness at/health/readiness. The pod uses 713Mi of memoryquerytool,upup=1, includingjob="vmmcp-demo": the MCP server itself is scraped via the createdVMServiceScrape. Its first scrape isup=0, since vmagent discovers the pod while the documentation index is loadingdocumentationtool,retention periodclaude mcp add --transport http, question "what targets are in VictoriaMetrics"serverMode: sse, then back tohttpDeploymentis rolled out,/ssereturnsevent: endpointVMMCPdeletionvmmcp-*objects are removedResponse of the query tool, labels trimmed
{"metric": {"__name__": "up", "job": "vmagent-example", "pod": "vmagent-example-67cc47cbcc-xh2dr"}, "value": [1792357571, "1"]} {"metric": {"__name__": "up", "job": "vmagent-example-reloader-http", "pod": "vmagent-example-67cc47cbcc-xh2dr"}, "value": [1792357571, "1"]} {"metric": {"__name__": "up", "job": "vmestimator-example", "pod": "vmestimator-select-example-6b76455f9f-c7574"}, "value": [1792357571, "1"]} {"metric": {"__name__": "up", "job": "vmestimator-storage-example", "pod": "vmestimator-storage-example-0"}, "value": [1792357571, "1"]} {"metric": {"__name__": "up", "job": "vmestimator-storage-example", "pod": "vmestimator-storage-example-1"}, "value": [1792357571, "1"]} {"metric": {"__name__": "up", "job": "vmmcp-demo", "pod": "vmmcp-demo-65ffd4b88c-wxrwn"}, "value": [1792357571, "1"]} {"metric": {"__name__": "up", "job": "vmsingle-example", "pod": "vmsingle-example-d8d859bbb-gzhfc"}, "value": [1792357571, "1"]}🤖 Generated with Claude Code