fix(vm): handle PVE node availability in VM datasource #1715
Conversation
- Added error handling for HTTP error code 595 in vmsRead, providing a warning if a node is not available, which may result in an incomplete VM list. Signed-off-by: Pavel Boldyrev <[email protected]>
WalkthroughThe changes modify the VM listing functionality in the Proxmox Terraform provider to handle scenarios where a node in a cluster is offline. Specifically, the code now captures a specific HTTP error code (595) during VM listing operations, allowing the function to continue processing and append a warning diagnostic instead of completely failing. This enhances the provider's resilience when working with multi-node Proxmox clusters where some nodes might be temporarily unavailable. Changes
Assessment against linked issues
Possibly related PRs
Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
proxmoxtf/datasource/vms.go (2)
91-91: Enhance function documentation.While the updated comment is more accurate, it could be more descriptive about the function's behavior, especially regarding node availability handling.
-// vmRead reads the VMs. +// vmRead reads VMs from available Proxmox nodes. If a node is unavailable (HTTP 595), +// it will be skipped with a warning, and the function will continue with other nodes.
126-134: Enhance error handling and user feedback.The error handling implementation is good, but could be improved with more detailed feedback and logging.
var httpError *proxmoxapi.HTTPError if errors.As(e, &httpError) && httpError.Code == 595 { + // Log the original error for debugging + config.Logger.Debug("failed to list VMs", "node", nodeName, "error", e) + diags = append(diags, diag.Diagnostic{ Severity: diag.Warning, - Summary: fmt.Sprintf("node %q is not available - VM list may be incomplete", nodeName), + Summary: fmt.Sprintf("node %q is not available (HTTP 595) - VM list may be incomplete", nodeName), + Detail: "Please check the node's status in the Proxmox web interface and ensure it's running and accessible.", }) continue }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
proxmoxtf/datasource/vms.go(4 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: qodana
🔇 Additional comments (2)
proxmoxtf/datasource/vms.go (2)
11-11: LGTM! Import changes are appropriate.The added imports for
errorsandproxmoxapipackages are necessary for the new error handling logic.Also applies to: 24-24
Line range hint
91-135: Overall implementation looks great! 👍The changes successfully implement graceful handling of node unavailability in the VM datasource. The code:
- Properly handles HTTP 595 errors
- Provides clear user feedback
- Continues processing available nodes
- Uses correct error handling patterns
This enhancement will improve the user experience when working with partially available Proxmox clusters.
Contributor's Note
/docsfor any user-facing features or additions./fwprovider/testsfor any new or updated resources / data sources.make exampleto verify that the change works as expected.Proof of Work
3 VMs provisioned in a test cluster of 3 nodes, one node with one VM is down.
This simple config
produced this output on
apply:Community Note
Closes #692
Summary by CodeRabbit
Bug Fixes
Documentation