-
Notifications
You must be signed in to change notification settings - Fork 1.5k
fix(proxmox): handle AttributeError when Qemu Agent is not available #21399
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(proxmox): handle AttributeError when Qemu Agent is not available #21399
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files🚀 New features to boost your workflow:
|
|
The following files, which will be shipped with the agent, were modified in this PR and You can ignore this if you are sure the changes in this PR do not require QA. Otherwise, consider removing the label. List of modified files that will be shipped with the agent |
19ffa61 to
5d4b286
Compare
sarah-witt
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks great, thanks for the fix!
Review from sarah-witt is dismissed. Related teams and files:
- agent-integrations
- proxmox/datadog_checks/proxmox/check.py
- proxmox/tests/test_unit.py
Fixes DataDog#21300 Signed-off-by: puretension <[email protected]>
Signed-off-by: puretension <[email protected]>
8afb64b to
8f59804
Compare
- Add AttributeError to existing exception handling in _get_vm_hostname() - Fall back to using vm_name when Qemu Agent is unavailable - Add test case for AttributeError scenario - Maintain existing logging behavior for debugging Fixes DataDog#21300 Signed-off-by: puretension <[email protected]>
24be028 to
e4715eb
Compare
- Move hostname parsing logic inside try block for cleaner error handling - Maintain fallback to vm_name when API call fails - Improve code readability and exception flow Co-authored-by: sarah-witt <[email protected]> Signed-off-by: puretension <[email protected]> Signed-off-by: sarah-witt <[email protected]>
…zed test Move test_get_hostname_attribute_error test case to a parameter in test_get_hostname_error and use MockResponse class as requested in review. Signed-off-by: puretension <[email protected]>
…not configured
- Replace AttributeErrorMockResponse with MockResponse using status_code=200
- Use actual JSON response: {"data": null, "message": "No QEMU guest agent configured\n"}
- Update test case ID to qemu_agent_not_configured for clarity
Signed-off-by: puretension <[email protected]>
Remove multiple consecutive blank lines that were causing linting failures Signed-off-by: puretension <[email protected]>
- Add required blank lines between functions - Fix trailing comma spacing in MockResponse Signed-off-by: puretension <[email protected]>
sarah-witt
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, thanks!
|
Thanks a lot, @sarah-witt! 🙏 |
|
When will this be included into the actual Datadog agent? When I updated it e.g. to 7.72.2 it is still the old check.py in the Proxmox integration? |
This should be included in v7.73.0 of the Agent which includes v2.2.0 of the Proxmox integration.
|
What does this PR do?
Fixes AttributeError when Proxmox VMs don't have Qemu Agent installed or available.
Motivation
Fixes #21300 - When Qemu Agent is not installed on Proxmox VMs, the
/agent/get-host-nameendpoint fails and causes an AttributeError. This breaks hostname collection for VMs without the agent.Description of the changes
AttributeErrorto the existing exception handling in_get_vm_hostname()vm_namewhen Qemu Agent is unavailableHow to test the changes?
Test with Proxmox VMs that don't have Qemu Agent installed - the integration should continue working and use VM names as hostnames.
Review checklist