fix(kubevirt): drop deprecated spec.running when setting runStrategy by fjvbn2003 · Pull Request #1518 · containers/kubernetes-mcp-server

fjvbn2003 · 2026-10-16T17:11:13Z

vm_lifecycle (start, stop, restart) reads and writes only spec.runStrategy
(pkg/kubevirt/vm.go). KubeVirt still accepts VirtualMachines that use the
deprecated spec.running field. KubeVirt v1.9 admits them and only adds the
warning spec.running is deprecated, please use spec.runStrategy instead.
For such a VM:

stop / restart / start (with running: false) write spec.runStrategy
while spec.running is still set. The KubeVirt VM admitter then rejects the
update, because validateRunStrategy refuses a spec that has both fields:
Running and RunStrategy are mutually exclusive. Note that Running is deprecated, please use RunStrategy instead
(vms-admitter.go).
No mutating webhook removes spec.running first.
start on a VM with running: true sees no runStrategy, so it updates the VM
and returns "VirtualMachine started successfully" although the VM was already running.

The fix follows KubeVirt's own interpretation in
VirtualMachine.RunStrategy():

GetVMRunStrategy maps spec.running: true/false to Always/Halted when
spec.runStrategy is absent.
SetVMRunStrategy removes spec.running before it writes spec.runStrategy.
This migrates the VM to the non-deprecated field instead of failing.

VMs that already use runStrategy behave the same as before, and the existing
TestStartVM, TestStopVM and TestRestartVM cases are unchanged.

Testing

I added TestLifecycleWithDeprecatedRunning (pkg/kubevirt/vm_test.go), which uses
the fake dynamic client:

On the current main it fails in all four subtests: spec.running is still set
×3, and wasStarted = true, want false for a VM that is already running.
With the change, the whole pkg/kubevirt vm_test.go passes:
TestStartVM, TestStopVM, TestRestartVM, TestPauseVM, TestUnpauseVM,
and the new test.

Caveat: I could not fetch the module set pinned in go.mod on this machine. So I ran
vm.go and vm_test.go in a scratch module against k8s.io/apimachinery and
k8s.io/client-go v0.36.0 instead of v0.37.0. The helpers used here
(unstructured.Nested*, RemoveNestedField, dynamic/fake) are the same in both.
CI will run the real module set.

The admission rejection itself comes from reading the KubeVirt v1.9.0 source
linked above. I have not reproduced it end to end with the MCP server against a
cluster. A cluster running KubeVirt v1.9.0 still has a VM that uses spec.running,
so the case does happen in practice.

This change was drafted with AI assistance and reviewed by me.

🤖 Generated with Claude Code

Cali0707 · 2026-10-16T17:53:00Z

cc @lyarwood @ksimon1 - mind reviewing this one?

fjvbn2003 · 2026-10-16T18:05:15Z

Follow-up on "I have not reproduced it end to end": I have now confirmed the admission rejection on a KubeVirt v1.9.0 cluster with a server-side dry run of a VM that sets both fields:

spec:
  running: false
  runStrategy: Halted

$ kubectl create --dry-run=server -f vm.yaml
The request is invalid: spec.running: Running and RunStrategy are mutually exclusive. Note that Running is deprecated, please use RunStrategy instead

This is the message from vms-admitter.go#L235. So a VM created with spec.running can't be switched by only adding spec.runStrategy, which is what this PR fixes by removing spec.running in the same update. (Still not run through the MCP server itself against a cluster.)

ksimon1

The change looks good. Below are some small findings. Please also add Assisted by: note to the commit message

ksimon1 · 2026-10-22T08:20:26Z

Please adjust this function instead of adding a new one (createTestVMWithRunning).

ksimon1 · 2026-10-22T08:34:10Z

+		}
+	}
+
+	t.Run("Start VM with running: false", func(t *testing.T) {


Nit: I would use similar pattern for these tests as e.g. in TestStopVM(t *testing.T) function, then we could get rid of some code duplication.

fjvbn2003 · 2026-10-22T08:58:22Z

Thanks for the review! Updated: createTestVM now takes an optional running value (and leaves runStrategy unset when it is empty) instead of a separate helper, the deprecated-running cases are table-driven like TestStopVM, and the commit message has an Assisted-by trailer. The four cases still fail against the previous vm.go and pass with the fix; go test ./pkg/kubevirt/... and go vet are clean.

ksimon1 · 2026-10-30T10:05:16Z

/lgtm
@lyarwood can you please review this PR?

ychampion

Reviewed at 4fd2f67345cef952e7358c3a490abe9675c8600c.

The legacy fallback matches KubeVirt's running semantics, and removing running in the same update avoids persisting the mutually exclusive fields together. Matching legacy states correctly remain no-ops.

I ran the helper package with -race and the complete TestKubevirt MCP suite with -race. An additional six-case public-MCP probe used a disposable Kubernetes 1.37 API server with a CEL rule enforcing the documented field exclusion. The pre-change helper fails all six cases with that validation error; this revision passes all six. Start/stop no-ops retain their resource versions, changed objects contain only the expected strategy, and unrelated template fields/annotations survive.

No KubeVirt controller or admission webhook was installed for this probe, so it verifies MCP responses and persisted update payloads, rather than VM reconciliation or restart timing. LGTM for this focused compatibility fix.

vm_lifecycle start/stop/restart only looked at spec.runStrategy. A VirtualMachine that still uses the deprecated spec.running field (KubeVirt v1.9 accepts it with a deprecation warning) ended up with both fields set after the update, which the KubeVirt VM admitter rejects with "Running and RunStrategy are mutually exclusive". start on a VM with running: true also reported "started" although it was already running. GetVMRunStrategy now maps spec.running to Always/Halted when runStrategy is absent, the same way KubeVirt's VirtualMachine.RunStrategy() does, and SetVMRunStrategy removes spec.running before writing spec.runStrategy. Assisted-by: Claude <[email protected]> Signed-off-by: YoungjuKim <[email protected]>

lyarwood · 2026-11-12T11:20:52Z

/run-mcpchecker kubevirt

lyarwood

/run-mcpchecker kubevirt

github-actions Bot added the toolset/kubevirt label Oct 16, 2026

ksimon1 reviewed Oct 22, 2026

View reviewed changes

fjvbn2003 force-pushed the fix-kubevirt-drop-spec-running branch from 382478a to 4fd2f67 Compare October 22, 2026 08:58

fjvbn2003 requested review from 2uasimojo, Cali0707, manusa, matzew and nader-ziada as code owners October 22, 2026 08:58

Cali0707 requested a review from ksimon1 October 26, 2026 20:01

ychampion approved these changes Nov 11, 2026

View reviewed changes

fjvbn2003 force-pushed the fix-kubevirt-drop-spec-running branch from 4fd2f67 to 554a31c Compare November 12, 2026 01:00

fjvbn2003 requested a review from lyarwood as a code owner November 12, 2026 01:00

lyarwood approved these changes Nov 12, 2026

View reviewed changes

lyarwood reviewed Nov 12, 2026

View reviewed changes

lyarwood added this pull request to the merge queue Nov 12, 2026

lyarwood mentioned this pull request Nov 12, 2026

CI: expose KubeVirt mcpchecker failures and preserve revision-linked pass-rate history #1673

Open

Merged via the queue into containers:main with commit 2bb8c6a Nov 12, 2026
18 checks passed

openshift-ci-robot mentioned this pull request Nov 13, 2026

NO-JIRA: sync with upstream 2026-11-13 openshift/openshift-mcp-server#584

Open

BrewTestBot mentioned this pull request Nov 13, 2026

kubernetes-mcp-server 0.1.3 Homebrew/homebrew-core#325406

Merged

Navigation Menu

Repository navigation

fix(kubevirt): drop deprecated spec.running when setting runStrategy - #1518

fix(kubevirt): drop deprecated spec.running when setting runStrategy#1518
lyarwood merged 1 commit into
containers:mainfrom
fjvbn2003:fix-kubevirt-drop-spec-running

fjvbn2003 commented Oct 16, 2026

Uh oh!

Cali0707 commented Oct 16, 2026

Uh oh!

fjvbn2003 commented Oct 16, 2026

Uh oh!

ksimon1 left a comment

Uh oh!

ksimon1 Oct 22, 2026

Uh oh!

ksimon1 Oct 22, 2026

Uh oh!

fjvbn2003 commented Oct 22, 2026

Uh oh!

ksimon1 commented Oct 30, 2026

Uh oh!

ychampion left a comment

Uh oh!

lyarwood commented Nov 12, 2026

Uh oh!

lyarwood left a comment

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Conversation

fjvbn2003 commented Oct 16, 2026

Testing

Uh oh!

Cali0707 commented Oct 16, 2026

Uh oh!

fjvbn2003 commented Oct 16, 2026

Uh oh!

ksimon1 left a comment

Choose a reason for hiding this comment

Uh oh!

ksimon1 Oct 22, 2026

Choose a reason for hiding this comment

Uh oh!

ksimon1 Oct 22, 2026

Choose a reason for hiding this comment

Uh oh!

fjvbn2003 commented Oct 22, 2026

Uh oh!

ksimon1 commented Oct 30, 2026

Uh oh!

ychampion left a comment

Choose a reason for hiding this comment

Uh oh!

lyarwood commented Nov 12, 2026

Uh oh!

lyarwood left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants