Skip to content

Fix VMNonRecoverableOSPanic to reflect both >0 and >5 cases#390

Open
avlitman wants to merge 1 commit into
kubevirt:mainfrom
avlitman:fix-panic-runbook
Open

Fix VMNonRecoverableOSPanic to reflect both >0 and >5 cases#390
avlitman wants to merge 1 commit into
kubevirt:mainfrom
avlitman:fix-panic-runbook

Conversation

@avlitman

@avlitman avlitman commented Jul 1, 2026

Copy link
Copy Markdown
Collaborator

Remove the specific > 5 panics threshold from the runbook's Meaning section, making it applicable to both the KubeVirt (critical, > 5) and HCO (warning, > 0) variants of the VMNonRecoverableOSPanic alert.

Release note:

none

Signed-off-by: avlitman <alitman@redhat.com>
@kubevirt-prow kubevirt-prow Bot added the dco-signoff: yes Indicates the PR's author has DCO signed all their commits. label Jul 1, 2026
@kubevirt-prow kubevirt-prow Bot requested review from ousleyp and sradco July 1, 2026 10:32
@kubevirt-prow kubevirt-prow Bot added the size/XS label Jul 1, 2026
@coderabbitai

coderabbitai Bot commented Jul 1, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

The change updates the alert description in the VMNonRecoverableOSPanic runbook, removing the specific "more than 5" panic threshold wording and replacing it with a general statement that the alert triggers when non-recoverable guest OS panics have occurred within the last 24 hours. The rest of the metric-based explanation is unchanged.

Estimated code review effort: 1 (Trivial) | ~2 minutes

Possibly related PRs

  • kubevirt/monitoring#387: Originally added this same runbook with the threshold-based description that this PR now revises.

Review notes: Solid, focused edit — docs/runbooks/VMNonRecoverableOSPanic.md lines 5-6 read cleanly and match the underlying alert logic. One thing worth double-checking: confirm the actual PrometheusRule query for this alert doesn't still reference a specific panic count threshold (e.g., > 5), since dropping the number from the doc could create a mismatch if the alert expression itself still uses a fixed count rather than a pure "occurred in last 24h" condition. If the expression truly has no numeric threshold, this is a clean accuracy fix and good to merge as-is.

Warning

Review ran into problems

🔥 Problems

Linked repositories: Your configuration references 10 linked repositories, but your current plan allows 0. Analyzed ``, skipped kubevirt/kubevirt, `kubevirt/containerized-data-importer`, `kubevirt/hyperconverged-cluster-operator`, `kubevirt/ssp-operator`, `kubevirt/cluster-network-addons-operator`, `kubevirt/hostpath-provisioner-operator`, `kubevirt/hostpath-provisioner`, `kubevirt/application-aware-quota`, `kubevirt/monitoring`, `k8snetworkplumbingwg/kubemacpool`.


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.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
docs/runbooks/VMNonRecoverableOSPanic.md (1)

37-38: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Prefix shell examples with $ per runbook style.

The kubectl commands lack the required $ prefix. As per path instructions, shell examples must be prefixed with $ to distinguish commands from output and maintain consistency with other runbooks.

-   kubectl get vmi -n $NAMESPACE $VM_NAME -o wide
-   kubectl describe vmi -n $NAMESPACE $VM_NAME
+   $ kubectl get vmi -n $NAMESPACE $VM_NAME -o wide
+   $ kubectl describe vmi -n $NAMESPACE $VM_NAME
-   POD=$(kubectl get pod -n $NAMESPACE -l kubevirt.io/domain=$VM_NAME -o name | head -n1)
-   kubectl describe $POD -n $NAMESPACE
-   kubectl logs $POD -n $NAMESPACE -c compute --previous
-   kubectl logs $POD -n $NAMESPACE -c compute
+   $ POD=$(kubectl get pod -n $NAMESPACE -l kubevirt.io/domain=$VM_NAME -o name | head -n1)
+   $ kubectl describe $POD -n $NAMESPACE
+   $ kubectl logs $POD -n $NAMESPACE -c compute --previous
+   $ kubectl logs $POD -n $NAMESPACE -c compute

Also applies to: 60-63

🤖 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/runbooks/VMNonRecoverableOSPanic.md` around lines 37 - 38, The shell
command examples in the runbook are missing the required `$ ` prefix, so update
the affected examples to match runbook style and consistency. Fix the command
blocks containing kubectl usage by adding `$ ` before each shell command in the
relevant sections, including the examples around the VM inspection commands and
the later related commands, so they are clearly distinguished from output.

Source: Path instructions

🤖 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.

Nitpick comments:
In `@docs/runbooks/VMNonRecoverableOSPanic.md`:
- Around line 37-38: The shell command examples in the runbook are missing the
required `$ ` prefix, so update the affected examples to match runbook style and
consistency. Fix the command blocks containing kubectl usage by adding `$ `
before each shell command in the relevant sections, including the examples
around the VM inspection commands and the later related commands, so they are
clearly distinguished from output.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: a082c031-b115-4b7f-ab9a-61aa51b8904a

📥 Commits

Reviewing files that changed from the base of the PR and between 4d7fc0f and 53c148e.

📒 Files selected for processing (1)
  • docs/runbooks/VMNonRecoverableOSPanic.md

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

dco-signoff: yes Indicates the PR's author has DCO signed all their commits. size/XS

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant