NO-JIRA: docs(kubevirt): add how-to for configuring VMs with JSON patches - #8473
Conversation
|
Pipeline controller notification For optional jobs, comment This repository is configured in: LGTM mode |
|
@qinqon: This pull request explicitly references no jira issue. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
Skipping CI for Draft Pull Request. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughA new documentation page explains how to customize KubeVirt 🚥 Pre-merge checks | ✅ 12✅ Passed checks (12 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/content/how-to/kubevirt/configuring-vm-with-jsonpatch.md`:
- Around line 32-34: Clarify that HyperShift's JSON Patch `add` differs from RFC
6902 by auto-creating parent paths: update the paragraph describing the `add`
operation to state that HyperShift sets EnsurePathExistsOnAdd=true in the
evanphx/json-patch library, so intermediate path elements will be created
automatically (unlike RFC 6902 where parent paths must exist), and warn readers
that patches relying on this behavior are HyperShift-specific and may not be
portable to other implementations.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 8a6285e2-f5b8-4fd1-a9e4-c31bc5089d7a
📒 Files selected for processing (2)
docs/content/how-to/kubevirt/configuring-vm-with-jsonpatch.mddocs/mkdocs.yml
f0428d8 to
9c61db2
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (1)
docs/content/how-to/kubevirt/configuring-vm-with-jsonpatch.md (1)
32-34:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winClarify this
addbehavior as HyperShift-specific, not generic RFC 6902.Current wording reads like standard JSON Patch behavior, but portability depends on HyperShift’s patch apply options. Please call out that intermediate-path auto-creation is implementation-specific.
💡 Suggested wording update
- The `add` operation automatically creates intermediate path elements if they - do not exist. This means you can add deeply nested fields without worrying - about whether parent objects are already present. + In HyperShift, the `add` operation can automatically create intermediate path + elements when they do not exist. This is implementation-specific behavior and + differs from strict RFC 6902 expectations where parent paths must already + exist. Patches that rely on this convenience may not be portable to other + JSON Patch implementations.To verify against implementation and tests:
#!/bin/bash set -euo pipefail # Verify patch-apply options in KubeVirt patching code. fd -i "kubevirt.go" | xargs -r rg -n -C3 "EnsurePathExistsOnAdd|ApplyWithOptions|NewApplyOptions|json-patch|jsonpatch" # Verify coverage/expectations in KubeVirt tests. fd -i "kubevirt_test.go" | xargs -r rg -n -C3 "EnsurePathExistsOnAdd|jsonpatch|intermediate|requiredDuringSchedulingIgnoredDuringExecution|/spec/template/spec/affinity"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/content/how-to/kubevirt/configuring-vm-with-jsonpatch.md` around lines 32 - 34, Update the sentence about the JSON Patch "add" operation to clearly state this intermediate-path auto-creation is HyperShift/KubeVirt-specific (driven by patch-apply options) and not guaranteed by RFC 6902; mention the relevant implementation knobs such as EnsurePathExistsOnAdd, ApplyWithOptions/NewApplyOptions and jsonpatch so readers know where to verify behavior and link to tests that assert it (e.g., references in kubevirt.go and kubevirt_test.go).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In `@docs/content/how-to/kubevirt/configuring-vm-with-jsonpatch.md`:
- Around line 32-34: Update the sentence about the JSON Patch "add" operation to
clearly state this intermediate-path auto-creation is
HyperShift/KubeVirt-specific (driven by patch-apply options) and not guaranteed
by RFC 6902; mention the relevant implementation knobs such as
EnsurePathExistsOnAdd, ApplyWithOptions/NewApplyOptions and jsonpatch so readers
know where to verify behavior and link to tests that assert it (e.g., references
in kubevirt.go and kubevirt_test.go).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: e1771fd5-4474-4eac-9ccf-eecd07afa0cd
📒 Files selected for processing (2)
docs/content/how-to/kubevirt/configuring-vm-with-jsonpatch.mddocs/mkdocs.yml
✅ Files skipped from review due to trivial changes (1)
- docs/mkdocs.yml
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/content/how-to/kubevirt/configuring-vm-with-jsonpatch.md`:
- Line 62: Replace the fenced code blocks (instances of the marker ```yaml
linenums="1" and similar fenced backtick blocks) with indented code block style
to satisfy markdownlint MD046; locate occurrences where the doc uses fenced
blocks (the example marker and the other flagged occurrences noted) and convert
them to the project’s expected indented format, ensuring language/linenumber
metadata is preserved or removed per project style so CI linting no longer flags
MD046.
- Around line 17-24: The table of patch operation fields is missing the required
"from" field used by the RFC 6902 "move" and "copy" operations; update the
fields table in configuring-vm-with-jsonpatch.md to add a row for `from` that
explains it is a JSON Pointer referencing the source location within the
VirtualMachineTemplateSpec and mark it as "Required" for `move`/`copy`
operations (not required otherwise), matching RFC 6902 semantics and clarifying
how to construct valid `move`/`copy` patches alongside the existing `op`,
`path`, and `value` descriptions.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 11035656-9a3b-4921-a39e-094b71d1484d
📒 Files selected for processing (2)
docs/content/how-to/kubevirt/configuring-vm-with-jsonpatch.mddocs/mkdocs.yml
✅ Files skipped from review due to trivial changes (1)
- docs/mkdocs.yml
| `node-type=kubevirt-worker`. This uses `requiredDuringSchedulingIgnoredDuringExecution` | ||
| to enforce strict placement. | ||
|
|
||
| ```yaml linenums="1" |
There was a problem hiding this comment.
Address markdownlint MD046 warnings for code block style.
These fenced blocks are currently flagged by markdownlint (Expected: indented; Actual: fenced). If docs lint is enforced in CI, this will keep warning/failing until aligned with project markdown style config.
Also applies to: 106-106, 118-118, 166-166, 214-214, 252-252, 267-267
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 62-62: Code block style
Expected: indented; Actual: fenced
(MD046, code-block-style)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/content/how-to/kubevirt/configuring-vm-with-jsonpatch.md` at line 62,
Replace the fenced code blocks (instances of the marker ```yaml linenums="1" and
similar fenced backtick blocks) with indented code block style to satisfy
markdownlint MD046; locate occurrences where the doc uses fenced blocks (the
example marker and the other flagged occurrences noted) and convert them to the
project’s expected indented format, ensuring language/linenumber metadata is
preserved or removed per project style so CI linting no longer flags MD046.
There was a problem hiding this comment.
Skipping this one. All existing KubeVirt docs in this project use fenced code blocks with linenums="1" (e.g., gpu-devices.md, performance-tuning.md, configuring-storage.md). This is the established project style for MkDocs Material. The MD046 warning is a false positive from markdownlint's default config not matching the project conventions -- markdownlint is not enforced in CI for this repo.
There was a problem hiding this comment.
@qinqon, understood — thanks for the context. Since the entire KubeVirt docs section consistently uses fenced code blocks with linenums="1" as the MkDocs Material project convention, and markdownlint is not enforced in CI, this warning can be safely ignored. I'll remember this for future reviews.
📝 Learnings were identified and not saved (knowledge base disabled). Enable
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jparrill, qinqon The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
You will need to execute the |
9c61db2 to
9f30291
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (1)
docs/content/how-to/kubevirt/configuring-vm-with-jsonpatch.md (1)
19-23:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
move/copypatch schema is still incomplete in the field table.Line 21 includes
moveandcopy, but the table still omitsfromand overstates whenvalueis required. This can lead to invalid patch examples for those operations.Suggested doc fix
| Field | Description | Required | |---------|---------------------------------------------------------------------|------------------------| | `op` | The operation to perform: `add`, `replace`, `remove`, `move`, `copy`, `test` | Yes | | `path` | A JSON Pointer path into the `VirtualMachineTemplateSpec` | Yes | -| `value` | The value to use for the operation | Yes (except `remove`) | +| `from` | Source JSON Pointer path (used by `move` and `copy`) | Yes (`move`, `copy`) | +| `value` | The value to use for the operation | Yes (`add`, `replace`, `test`) |🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/content/how-to/kubevirt/configuring-vm-with-jsonpatch.md` around lines 19 - 23, Update the table describing JSONPatch fields so it correctly documents the `move` and `copy` operations: add a `from` row/column describing the JSON Pointer source used by `move` and `copy`, and adjust the `value` requirement to state that `value` is required for `add`, `replace`, and `test` but not for `remove`, `move`, or `copy`; ensure the `op`, `path`, `value`, and newly documented `from` entries in the table match the JSON Patch spec and the behavior expected by VirtualMachineTemplateSpec.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In `@docs/content/how-to/kubevirt/configuring-vm-with-jsonpatch.md`:
- Around line 19-23: Update the table describing JSONPatch fields so it
correctly documents the `move` and `copy` operations: add a `from` row/column
describing the JSON Pointer source used by `move` and `copy`, and adjust the
`value` requirement to state that `value` is required for `add`, `replace`, and
`test` but not for `remove`, `move`, or `copy`; ensure the `op`, `path`,
`value`, and newly documented `from` entries in the table match the JSON Patch
spec and the behavior expected by VirtualMachineTemplateSpec.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 59a46c7f-c97b-400c-b22b-ef909b635aae
⛔ Files ignored due to path filters (1)
docs/content/reference/aggregated-docs.mdis excluded by!docs/content/reference/aggregated-docs.md
📒 Files selected for processing (2)
docs/content/how-to/kubevirt/configuring-vm-with-jsonpatch.mddocs/mkdocs.yml
✅ Files skipped from review due to trivial changes (1)
- docs/mkdocs.yml
9f30291 to
23ecc13
Compare
|
/verified by e2e(build docs) |
There was a problem hiding this comment.
♻️ Duplicate comments (1)
docs/content/how-to/kubevirt/configuring-vm-with-jsonpatch.md (1)
63-63:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winResolve markdownlint MD046 code block style warnings.
markdownlint-cli2reports fenced code blocks where indented style is expected at these lines. Please align these examples with the repo’s configured markdown style to avoid recurring lint warnings/failures.Also applies to: 107-107, 119-119, 167-167, 215-215, 253-253, 268-268
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/content/how-to/kubevirt/configuring-vm-with-jsonpatch.md` at line 63, The markdown uses fenced code blocks like ```yaml linenums="1" which triggers markdownlint MD046; replace each fenced block with the repository's expected indented code-block style by removing the backtick fences and attributes (e.g., ```yaml linenums="1") and indenting every line of the example by four spaces (ensure a blank line before and after the indented block); update all occurrences of these fenced YAML examples in this document (the blocks starting with the ```yaml linenums="1" markers and the other instances noted in the review) so they use the indented style and no fenced-language or linenums attributes.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In `@docs/content/how-to/kubevirt/configuring-vm-with-jsonpatch.md`:
- Line 63: The markdown uses fenced code blocks like ```yaml linenums="1" which
triggers markdownlint MD046; replace each fenced block with the repository's
expected indented code-block style by removing the backtick fences and
attributes (e.g., ```yaml linenums="1") and indenting every line of the example
by four spaces (ensure a blank line before and after the indented block); update
all occurrences of these fenced YAML examples in this document (the blocks
starting with the ```yaml linenums="1" markers and the other instances noted in
the review) so they use the indented style and no fenced-language or linenums
attributes.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: f01421fe-c7b2-481d-ab79-653582157a5d
📒 Files selected for processing (2)
docs/content/how-to/kubevirt/configuring-vm-with-jsonpatch.mddocs/mkdocs.yml
🚧 Files skipped from review as they are similar to previous changes (1)
- docs/mkdocs.yml
|
@qinqon: This PR has been marked as verified by DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
/lgtm |
|
Pipeline controller notification No second-stage tests were triggered for this PR. This can happen when:
Use |
Test Failure Analysis CompleteJob Information
Test Failure AnalysisErrorSummaryThe PR adds a new documentation file ( Root CauseThe repository contains an automated docs aggregation tool at When PR #8473 added This is a standard "generated file not committed" failure — the PR author needs to run Recommendations
The fix is a single command: make generate update
git add docs/content/reference/aggregated-docs.md
git commit -m "docs: regenerate aggregated-docs.md"Evidence
|
Document the hypershift.openshift.io/kubevirt-vm-jsonpatch annotation for advanced VM customization, focusing on node affinity configuration. Covers required and preferred affinity rules, HostedCluster vs NodePool scope, precedence behavior, and additional examples for CPU and network patches. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Enrique Llorente <ellorent@redhat.com>
|
CI check |
23ecc13 to
938cef5
Compare
|
/lgtm |
|
Pipeline controller notification No second-stage tests were triggered for this PR. This can happen when:
Use |
|
/verified by e2e(build docs) |
|
@qinqon: This PR has been marked as verified by DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
@qinqon: all tests passed! Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
What this PR does / why we need it:
Adds a new how-to documentation page for the
hypershift.openshift.io/kubevirt-vm-jsonpatchannotation, which was previously undocumented. The doc focuses on configuring node affinity for KubeVirt VMs but also covers the general JSON patch mechanism.Content includes:
oc annotateexamplesWhich issue(s) this PR fixes:
N/A — this is net-new documentation for an existing but undocumented feature.
Special notes for your reviewer:
JSONPatchAnnotationconstant was introduced but had no user-facing documentation.hypershift-operator/controllers/nodepool/kubevirt/kubevirt_test.goand the implementation inkubevirt.go.Checklist:
Summary by CodeRabbit
addbehavior (auto-creation of intermediate paths).