docs(backups): updated about restore VMI/VMDisk related PR#2251#466
docs(backups): updated about restore VMI/VMDisk related PR#2251#466
Conversation
✅ Deploy Preview for cozystack ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
📝 WalkthroughWalkthroughDocumentation updates for Velero backup/restore: refined Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the documentation for Velero backup and recovery, reflecting improvements in the restore controller which now automates several manual steps such as halting VMs, renaming PVCs, and deleting DataVolumes. It also updates the backup configuration examples to include ConfigMaps and detailed restore specifications. Feedback suggests restoring descriptive comments in the YAML examples for better clarity and verifying if the documentation for cross-resource restores using targetApplicationRef should be retained.
| spec: | ||
| includedNamespaces: | ||
| - '{{ .Application.metadata.namespace }}' | ||
| orLabelSelectors: | ||
| # VM resources (VirtualMachine, DataVolume, PVC, etc.) | ||
| - matchLabels: | ||
| app.kubernetes.io/instance: 'vm-instance-{{ .Application.metadata.name }}' | ||
| # HelmRelease (the Cozystack app object) | ||
| - matchLabels: |
There was a problem hiding this comment.
The descriptive comments in the YAML example were removed. These comments provided helpful context for users to understand the purpose of the spec fields and the label selectors. It is recommended to keep them to maintain documentation clarity.
| spec: | |
| includedNamespaces: | |
| - '{{ .Application.metadata.namespace }}' | |
| orLabelSelectors: | |
| # VM resources (VirtualMachine, DataVolume, PVC, etc.) | |
| - matchLabels: | |
| app.kubernetes.io/instance: 'vm-instance-{{ .Application.metadata.name }}' | |
| # HelmRelease (the Cozystack app object) | |
| - matchLabels: | |
| spec: # see https://velero.io/docs/v1.17/api-types/backup/ | |
| includedNamespaces: | |
| - '{{ .Application.metadata.namespace }}' | |
| orLabelSelectors: | |
| # VM resources (VirtualMachine, DataVolume, PVC, etc.) | |
| - matchLabels: | |
| app.kubernetes.io/instance: 'vm-instance-{{ .Application.metadata.name }}' | |
| # HelmRelease (the Cozystack app object) | |
| - matchLabels: |
| virtctl restart vm-instance-my-vm -n tenant-user | ||
| # Delete them when no longer needed (replace with actual PVC names from the list above) | ||
| kubectl delete pvc -n tenant-user <name>-orig-<hash> | ||
| ``` |
There was a problem hiding this comment.
The note about using targetApplicationRef to restore into a different VMInstance was removed in this update. If this feature is still supported by the restore controller, it should be retained in the documentation to ensure users are aware of how to perform cross-resource restores.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
content/en/docs/v1/virtualization/backup-and-recovery.md (1)
291-293: Add a concrete command for retrieving original IP/MAC from Backup status.Line 292 says users can fetch original IP/MAC from
Backup .status.underlyingResources, but without a command this is hard to execute during incident recovery. Add akubectl get backup ... -o jsonpath=...example here.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@content/en/docs/v1/virtualization/backup-and-recovery.md` around lines 291 - 293, Add a concrete kubectl example showing how to extract the original IP and MAC from the Backup resource's status; specifically update the block referencing "Backup .status.underlyingResources" to include a sample command using kubectl get backup <backup-name> -n <namespace> -o jsonpath=... (or a jq alternative) that queries the .status.underlyingResources JSON path to pull the saved VM network annotations/fields, so operators can copy the exact jsonpath to retrieve the original IP/MAC during recovery.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@content/en/docs/v1/virtualization/backup-and-recovery.md`:
- Around line 291-293: Add a concrete kubectl example showing how to extract the
original IP and MAC from the Backup resource's status; specifically update the
block referencing "Backup .status.underlyingResources" to include a sample
command using kubectl get backup <backup-name> -n <namespace> -o jsonpath=...
(or a jq alternative) that queries the .status.underlyingResources JSON path to
pull the saved VM network annotations/fields, so operators can copy the exact
jsonpath to retrieve the original IP/MAC during recovery.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 8e67eb12-fcb2-45b5-aea4-a194100811c3
📒 Files selected for processing (2)
content/en/docs/v1/operations/services/velero-backup-configuration.mdcontent/en/docs/v1/virtualization/backup-and-recovery.md
Signed-off-by: Andrey Kolkov <androndo@gmail.com>
androndo
force-pushed
the
resrore-updates
branch
from
9cca6e8 to
cc96abf
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (2)
content/en/docs/v1/virtualization/backup-and-recovery.md (2)
196-203: Consider documenting HelmRelease re-suspension behavior.The alert clearly documents that the restore controller suspends HelmReleases during preparation, but doesn't mention whether they are automatically unsuspended after the restore completes, or if users need to manually unsuspend them to allow Flux reconciliation.
📝 Suggested addition for clarity
After line 203, consider adding a note about post-restore HelmRelease state:
- **Deletes DataVolumes** so CDI does not recreate PVCs after the rename. +- **Re-suspends HelmReleases** after Velero completes the restore to prevent Flux from overwriting restored state. {{% /alert %}}(Adjust the text based on the actual controller behavior - if they remain suspended, if they auto-unsuspend, or if manual intervention is needed.)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@content/en/docs/v1/virtualization/backup-and-recovery.md` around lines 196 - 203, The doc explains that the restore controller suspends HelmReleases for the VMInstance and its VMDisks but omits what happens afterward; add a brief note after the alert stating the post-restore HelmRelease state (whether the restore controller automatically unsuspends HelmRelease resources, leaves them suspended, or requires the user to manually unsuspend them) and, if relevant, include the actions users should take (e.g., manually unsuspend Flux reconciliation on HelmRelease resources) and reference the relevant resources by name (HelmRelease, VMInstance, VMDisks, restore controller) so readers can find the behavior in the controller code or operator docs.
289-323: Clarify when MAC preservation succeeds vs. when manual intervention is needed.The alert at line 292 states the controller "automatically attempts to preserve the VM's OVN IP and MAC addresses" via annotations, but the following paragraph (line 295) says "After restore, the VM gets a new virtual NIC with a different MAC address." This creates confusion about when the automatic preservation works versus when users need the
cloudInitSeedworkaround.📝 Suggested clarification
Consider revising the flow to clarify the conditions:
{{% alert color="info" %}} -The restore controller automatically attempts to preserve the VM's **OVN IP and MAC addresses** from backup time by patching the restored VirtualMachine with the original annotations. If this fails, you can retrieve the original IP/MAC manually from `Backup .status.underlyingResources` and assign them. +The restore controller automatically attempts to preserve the VM's **OVN IP and MAC addresses** from backup time by patching the restored VirtualMachine with the original annotations. When preservation succeeds, the guest OS should maintain network connectivity. If preservation fails or the VM is restored to a different network, you can retrieve the original IP/MAC manually from `Backup .status.underlyingResources` and assign them. {{% /alert %}} -After a VMInstance is restored, the guest OS may lose network connectivity. This is known to happen on **Ubuntu Server**, where cloud-init generates a netplan configuration bound to the old VM's MAC address. After restore, the VM gets a new virtual NIC with a different MAC address, but the guest OS still has the old netplan config bound to the previous MAC — so the network interface is never configured. Other operating systems that do not pin network configuration to a specific MAC address may not be affected by this issue. +If MAC preservation fails and the VM gets a new virtual NIC with a different MAC address, the guest OS may lose network connectivity. This is known to happen on **Ubuntu Server**, where cloud-init generates a netplan configuration bound to the original MAC address but the guest OS now has a different MAC — so the network interface is never configured. Other operating systems that do not pin network configuration to a specific MAC address may not be affected by this issue.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@content/en/docs/v1/virtualization/backup-and-recovery.md` around lines 289 - 323, Clarify the documentation flow by stating exactly when the restore controller's attempt to preserve OVN IP and MAC (via annotations on the restored VirtualMachine and referenced in Backup .status.underlyingResources) typically succeeds (e.g., when the guest OS accepts the restored NIC/MAC as unchanged) and when it can fail (e.g., on Ubuntu Server where cloud-init/netplan binds to the original MAC and the guest creates a new NIC with a different MAC), then instruct users to use the cloudInitSeed workaround only in the failure cases; update the alert text and the paragraph that follows to mention the specific condition (Ubuntu Server/cloud-init netplan binding) that triggers manual intervention and note that other OSes normally do not require reseeding, referencing cloudInitSeed and virtctl restart in the guidance.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@content/en/docs/v1/virtualization/backup-and-recovery.md`:
- Around line 196-203: The doc explains that the restore controller suspends
HelmReleases for the VMInstance and its VMDisks but omits what happens
afterward; add a brief note after the alert stating the post-restore HelmRelease
state (whether the restore controller automatically unsuspends HelmRelease
resources, leaves them suspended, or requires the user to manually unsuspend
them) and, if relevant, include the actions users should take (e.g., manually
unsuspend Flux reconciliation on HelmRelease resources) and reference the
relevant resources by name (HelmRelease, VMInstance, VMDisks, restore
controller) so readers can find the behavior in the controller code or operator
docs.
- Around line 289-323: Clarify the documentation flow by stating exactly when
the restore controller's attempt to preserve OVN IP and MAC (via annotations on
the restored VirtualMachine and referenced in Backup
.status.underlyingResources) typically succeeds (e.g., when the guest OS accepts
the restored NIC/MAC as unchanged) and when it can fail (e.g., on Ubuntu Server
where cloud-init/netplan binds to the original MAC and the guest creates a new
NIC with a different MAC), then instruct users to use the cloudInitSeed
workaround only in the failure cases; update the alert text and the paragraph
that follows to mention the specific condition (Ubuntu Server/cloud-init netplan
binding) that triggers manual intervention and note that other OSes normally do
not require reseeding, referencing cloudInitSeed and virtctl restart in the
guidance.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7da91a72-589e-4bfe-b1a9-e4a784817ed7
📒 Files selected for processing (2)
content/en/docs/v1/operations/services/velero-backup-configuration.mdcontent/en/docs/v1/virtualization/backup-and-recovery.md
2 participants
Summary by CodeRabbit