fix(vm-instance): emit empty disk object so KubeVirt accepts disks without bus by myasnikovdaniil · Pull Request #2643 · cozystack/cozystack

myasnikovdaniil · 2026-05-13T09:49:29Z

Problem

A VMInstance with disks whose bus field is unset (the schema default — bus is optional, no default value) fails to deploy. The KubeVirt admission webhook rejects the rendered VirtualMachine with:

admission webhook "virtualmachines-mutator.kubevirt.io" denied the request:
spec.template.spec.domain.devices.disks.disk in body must be of type object: "null"

Root cause

packages/apps/vm-instance/templates/vm.yaml renders the disk entry as:

- name: disk-foo
  disk:                  # ← no value
  bootOrder: 1

YAML parses disk: (no value) as disk: null. KubeVirt's DiskTarget schema requires an object, so the mutator rejects it.

The template only attached a child (bus: …) when $disk.bus was non-empty, so the empty-bus path produced a key with no value.

Reproduction

On dev10:

spec:
  disks:
  - name: test-ui

→ kubectl get vminstance test -n tenant-root shows InstallFailed with the exact admission-webhook message above.

Fix

Restructure the disk loop so the disk / cdrom value is always a valid object:

bus set → disk: { bus: <x> }
bus empty → disk: {}

This mirrors how the same template already renders cloudinitdisk. Both the disk and cdrom branches get the fix.

Verification

helm install --dry-run=server against dev10 (KubeVirt v1.6.3) with disks: [{ name: test-ui }] (no bus):

Before fix: rendered disk: null — admission webhook rejects on real install.
After fix: rendered disk: {} — install succeeds; dry-run=server returns the full manifest, no admission error.

helm install --dry-run=server with disks: [{ name: test-ui, bus: sata }]:

After fix: rendered disk:\n bus: sata — install succeeds.

Release note

```release-note
fix(vm-instance): emit "disk: {}" instead of "disk: null" so VMInstances with disks lacking an explicit bus type are accepted by the KubeVirt admission webhook.
```

Summary by CodeRabbit

Refactor
- Optimized VM disk device configuration handling to improve code maintainability and reliability in disk type selection.

…thout bus When a disk entry has no bus set (the schema default), the previous template rendered `disk:` with no value — YAML parses that as `disk: null` and KubeVirt's virtualmachines-mutator webhook denies the request with: "spec.template.spec.domain.devices.disks.disk in body must be of type object: null". Restructure the disk loop so the disk/cdrom value is always a valid object: `disk: {}` when bus is empty, `disk: {bus: <x>}` when set. Mirrors how `cloudinitdisk` already does it in the same template. Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>

gemini-code-assist · 2026-05-13T09:49:37Z

Summary of Changes

Hello, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request addresses a deployment failure where VMInstances with disks lacking an explicit bus configuration were rejected by the KubeVirt admission webhook. By ensuring that the disk and cdrom fields are rendered as empty objects rather than null when the bus is unset, the generated manifests now comply with the expected KubeVirt schema.

Highlights

KubeVirt Compatibility: Updated the VMInstance template to ensure disk and cdrom objects are always explicitly defined, preventing KubeVirt admission webhook rejections caused by null values.
Template Refactoring: Restructured the disk loop logic to use a ternary operator for device key selection and ensured that an empty object is emitted when no bus type is specified.

New Features

🧠 You can now enable Memory (public preview) to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	`/gemini review`	Performs a code review for the current pull request in its current state.
Pull Request Summary	`/gemini summary`	Provides a summary of the current pull request in its current state.
Comment	Gemini (@gemini-code-assist)	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	`/gemini help`	Displays a list of available commands.

Customization

To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on Gemini (@gemini-code-assist) comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here.

Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

gemini-code-assist

Code Review

This pull request refactors the disk configuration logic in the VM instance template by introducing variables for device types and simplifying the conditional blocks. Feedback was provided regarding the safe access of Kubernetes annotations to prevent template rendering errors when the field is missing. Additionally, it was recommended to quote the disk bus value to ensure correct YAML parsing and string representation.

gemini-code-assist · 2026-05-13T09:50:38Z

            {{- if $dv }}
-            {{- if and (hasKey $dv.metadata.annotations "vm-disk.cozystack.io/optical") (eq (index $dv.metadata.annotations "vm-disk.cozystack.io/optical") "true") }}
-            cdrom:
+            {{- $isOptical := and (hasKey $dv.metadata.annotations "vm-disk.cozystack.io/optical") (eq (index $dv.metadata.annotations "vm-disk.cozystack.io/optical") "true") }}


Using hasKey on metadata.annotations can be risky because the annotations field is optional in Kubernetes objects. If it is missing (nil), hasKey will cause a template rendering error. It is safer to use the dig function to navigate the object path with a default value.

{{- $isOptical := eq (dig "metadata" "annotations" "vm-disk.cozystack.io/optical" "" $dv) "true" }}

References

Use defensive programming to safely handle potentially missing fields in Kubernetes objects (like annotations) to avoid template rendering panics.

gemini-code-assist · 2026-05-13T09:50:38Z

+            {{- $deviceKey := ternary "cdrom" "disk" $isOptical }}
+            {{- if $disk.bus }}
+            {{ $deviceKey }}:
+              bus: {{ $disk.bus }}


The bus value should be quoted to ensure it is always treated as a string in the rendered YAML, especially to avoid issues with values that might be interpreted as booleans or numbers by some YAML parsers.

bus: {{ $disk.bus | quote }}

References

Helm template correctness: missing quote ^(link)

coderabbitai · 2026-05-13T09:52:35Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 3cfab3d5-f14d-4498-aa2c-c4c430dd02eb

📥 Commits

Reviewing files that changed from the base of the PR and between 70693ff and 0f3d2fa.

📒 Files selected for processing (1)

packages/apps/vm-instance/templates/vm.yaml

📝 Walkthrough

Walkthrough

This PR refactors the KubeVirt disk device stanza generation in the VM Helm template to use computed variables for cleaner device type selection and conditional bus field inclusion.

Changes

Disk device generation refactoring

Layer / File(s)	Summary
Disk device stanza generation logic `packages/apps/vm-instance/templates/vm.yaml`	The disk device type selection (`cdrom` vs `disk`) and bus field emission are refactored from an inline `if/else` block into computed `$isOptical` and `$deviceKey` variables. Device stanzas now emit either `<deviceKey>: { bus: ... }` or `<deviceKey>: {}` depending on whether bus is set.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Poem

🐰 A rabbit hops through templates clean,
Variables dance where conditions had been,
Optical disks now dance with a key,
The bus field blooms when it's meant to be! 🚌

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately describes the main fix: changing how disk objects are rendered in KubeVirt templates to emit empty objects instead of null values.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch fix/vm-instance-disk-null

Warning

Review ran into problems

🔥 Problems

Git: Failed to clone repository. Please run the @coderabbitai full review command to re-trigger a full review. If the issue persists, set path_filters to include or exclude specific files.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

Aleksei Sviridkin (lexfrei)

NOT LGTM — there is nothing left to merge: this branch's only commit already landed on main verbatim, so merging now would only add an empty merge commit and a duplicate history entry. This PR should be closed as superseded, not merged.

Business context: a VMInstance disk without bus rendered disk: with no value (disk: null), which the KubeVirt admission webhook rejects; the fix renders disk: {} / cdrom: {} instead.

Why close

The identical change shipped to main as commit 2e3e7bdb3 (authorship preserved: Myasnikov Daniil), picked up by #2602's strict-SSA fix series.

Evidence: git patch-id --stable yields the same hash (1f10c2bf...) for this branch's 0f3d2fa3a and main's 2e3e7bdb3; 2e3e7bdb3 is an ancestor of origin/main; git diff 0f3d2fa3a origin/main -- packages/apps/vm-instance/templates/vm.yaml is empty.

The fix itself is correct and has been live on main for ~4 weeks: the $isOptical + ternary refactor is semantically equivalent to the old branch, and disk: {} lets KubeVirt default the bus.

Follow-up material (against main, not this PR)

packages/apps/vm-instance ships no helm-unittest coverage at all (unlike postgres/kafka/kubernetes/mongodb/etc.). The null-vs-{} regression is exactly what a render test pins; helm-unittest can mock lookup via kubernetesProvider.objects (required here — without it the template hits the fail "Specified disk not exists in cluster" branch). Minimal set: disk without bus → disk == {}; disk with bus: virtio → disk.bus: virtio; DataVolume annotated vm-disk.cozystack.io/optical: "true" → cdrom key.
hasKey $dv.metadata.annotations ... (vm.yaml:70) is nil-unsafe: a DataVolume with no annotations at all fails the whole render with wrong type for value; expected map[string]interface {}. DVs created by the vm-disk chart always carry the annotation, so in-platform risk is low, but a manually created vm-disk-* DV breaks the VM render. Nil-safe form: eq (dig "metadata" "annotations" "vm-disk.cozystack.io/optical" "" $dv) "true".

Aleksei Sviridkin (lexfrei)

NOT LGTM — there is nothing left to merge. This branch's only commit is byte-identical to current main, and the same change already shipped to main independently, so merging now would add only a no-op merge commit and a duplicate history entry. Close as superseded.

Business context: a VMInstance disk without an explicit bus rendered disk: with no value (disk: null), which the KubeVirt admission webhook rejects; the fix emits disk: {} / cdrom: {} instead so KubeVirt can default the bus.

Blockers

The fix is already on main; this PR is a no-op. Evidence: git diff origin/main HEAD -- packages/apps/vm-instance/templates/vm.yaml is empty (head content equals current main at 374cac106); git patch-id --stable yields the same hash 1f10c2bf… for this branch's 0f3d2fa3a and main's 2e3e7bdb3; git merge-base --is-ancestor 2e3e7bdb3 origin/main succeeds. Commit 2e3e7bdb3 (same author, 2026-05-13) is already contained in main.

The fix itself is correct, which is why closing rather than reworking is the right move: disk: {} is a valid DiskTarget and lets KubeVirt default the bus, whereas disk: null fails the object-type schema check. All four shapes render valid output — disk/cdrom × bus-set/bus-unset — and the $isOptical + ternary refactor is behavior-preserving. Evidence: helm template over disks: [{name: a}, {name: b, bus: sata}] (one annotated optical) renders disk: {} for the no-bus disk and cdrom: + bus: sata for the optical one.

Non-blocking (these apply to the live `main` code, not introduced by this PR)

packages/apps/vm-instance ships no helm-unittest coverage, so the null-vs-{} regression has no pinning test. Evidence: no tests/ directory and no *_test.yaml exists under packages/apps/vm-instance. A render test can mock lookup via kubernetesProvider.objects. Minimal set: disk without bus → disk == {}; disk with bus: virtio → disk.bus == virtio; DataVolume annotated vm-disk.cozystack.io/optical: "true" → cdrom key emitted.
hasKey $dv.metadata.annotations … is nil-unsafe: a DataVolume with no annotations at all aborts the whole render with wrong type for value; expected map[string]interface {}. In-platform DataVolumes always carry the annotation, so risk is low, but a manually created vm-disk-* DataVolume breaks the VM render. Evidence: packages/apps/vm-instance/templates/vm.yaml:70 on main. Nil-safe form: eq (dig "metadata" "annotations" "vm-disk.cozystack.io/optical" "" $dv) "true".

myasnikovdaniil · 2026-06-25T06:45:53Z

Confirmed superseded — this branch's only commit (0f3d2fa3) is patch-identical to 2e3e7bdb3, already on main (landed via the #2602 strict-SSA series). git diff of packages/apps/vm-instance/templates/vm.yaml against origin/main is empty, so there's nothing left to merge. Closing. Thanks Aleksei Sviridkin (@lexfrei) for the catch.

myasnikovdaniil requested review from IvanHunters, Andrey Kolkov (androndo), Andrei Kvapil (kvaps), Aleksei Sviridkin (lexfrei), Timofei Larkin (lllamnyp) and Kirill Ilin (sircthulhu) as code owners May 13, 2026 09:49

github-actions Bot added the size/S This PR changes 10-29 lines, ignoring generated files label May 13, 2026

dosubot Bot added area/virtualization Issues or PRs related to virtualization (kubevirt, cdi, vmi, vm-import) kind/bug Categorizes issue or PR as related to a bug labels May 13, 2026

github-actions Bot added the area/uncategorized PR auto-labeler could not map title scope to a known area/*; please review label May 13, 2026

gemini-code-assist Bot reviewed May 13, 2026

View reviewed changes

Aleksei Sviridkin (lexfrei) removed the area/uncategorized PR auto-labeler could not map title scope to a known area/*; please review label May 25, 2026

myasnikovdaniil self-assigned this May 28, 2026

Aleksei Sviridkin (lexfrei) requested changes Jun 10, 2026

View reviewed changes

Aleksei Sviridkin (lexfrei) requested changes Jun 24, 2026

View reviewed changes

myasnikovdaniil closed this Jun 25, 2026

Navigation Menu

Uh oh!

fix(vm-instance): emit empty disk object so KubeVirt accepts disks without bus - #2643

fix(vm-instance): emit empty disk object so KubeVirt accepts disks without bus#2643
myasnikovdaniil wants to merge 1 commit into
mainfrom
fix/vm-instance-disk-null

myasnikovdaniil commented May 13, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

gemini-code-assist Bot commented May 13, 2026

Uh oh!

gemini-code-assist Bot left a comment

Uh oh!

gemini-code-assist Bot May 13, 2026

Uh oh!

gemini-code-assist Bot May 13, 2026

Uh oh!

coderabbitai Bot commented May 13, 2026 •

edited

Loading

Walkthrough

Changes

Estimated code review effort

Poem

Review ran into problems

Uh oh!

Aleksei Sviridkin (lexfrei) left a comment

Uh oh!

Aleksei Sviridkin (lexfrei) left a comment

Uh oh!

myasnikovdaniil commented Jun 25, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

myasnikovdaniil commented May 13, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem

Root cause

Reproduction

Fix

Verification

Release note

Summary by CodeRabbit

Uh oh!

gemini-code-assist Bot commented May 13, 2026

Summary of Changes

Highlights

Footnotes

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist Bot May 13, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot May 13, 2026

Choose a reason for hiding this comment

Uh oh!

coderabbitai Bot commented May 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Poem

Review ran into problems

Uh oh!

Aleksei Sviridkin (lexfrei) left a comment

Choose a reason for hiding this comment

Why close

Follow-up material (against main, not this PR)

Uh oh!

Aleksei Sviridkin (lexfrei) left a comment

Choose a reason for hiding this comment

Blockers

Non-blocking (these apply to the live main code, not introduced by this PR)

Uh oh!

myasnikovdaniil commented Jun 25, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

myasnikovdaniil commented May 13, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented May 13, 2026 •

edited

Loading

Non-blocking (these apply to the live `main` code, not introduced by this PR)