-
Notifications
You must be signed in to change notification settings - Fork 1.2k
agent: increase timeout for host arch retrieval #11254
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
Conversation
Signed-off-by: Abhishek Kumar <[email protected]>
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #11254 +/- ##
============================================
+ Coverage 16.15% 16.57% +0.42%
- Complexity 13268 14059 +791
============================================
Files 5657 5772 +115
Lines 497772 512939 +15167
Branches 60364 62305 +1941
============================================
+ Hits 80406 85013 +4607
- Misses 408415 418450 +10035
- Partials 8951 9476 +525
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
@blueorangutan package |
@sureshanaparti a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress. |
Packaging result [SF]: ✔️ el8 ✔️ el9 ✖️ debian ✔️ suse15. SL-JID 14319 |
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.
Pull Request Overview
This PR increases the timeout for agent/host architecture retrieval from 500ms to 1000ms and standardizes the approach used across different components. The changes address timeout errors that were occurring when retrieving CPU architecture information.
- Increased timeout for architecture retrieval in Agent.java from 500ms to 1000ms
- Standardized arch command execution using
Script.getExecutableAbsolutePath("arch")
instead of hardcoded paths - Added debug logging for better troubleshooting
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
File | Description |
---|---|
agent/src/main/java/com/cloud/agent/Agent.java | Updated getAgentArch() method to use standardized script execution with increased timeout and added debug logging |
plugins/hypervisors/kvm/src/main/java/org/apache/cloudstack/utils/linux/KVMHostInfo.java | Renamed variable and updated to use Script.getExecutableAbsolutePath() for consistency |
Comments suppressed due to low confidence (1)
plugins/hypervisors/kvm/src/main/java/org/apache/cloudstack/utils/linux/KVMHostInfo.java:61
- [nitpick] The variable name 'cpuArchRetrieveExecutable' is inconsistent with the naming pattern used elsewhere. Consider renaming to 'cpuArchCommand' or 'archExecutable' for better consistency with the codebase.
private static String cpuArchRetrieveExecutable = "arch";
@blueorangutan test |
@sureshanaparti a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests |
@@ -248,6 +248,6 @@ private void getHostInfoFromLibvirt() { | |||
|
|||
private String getCPUArchFromCommand() { | |||
LOGGER.info("Fetching host CPU arch"); | |||
return Script.runSimpleBashScript(cpuArchCommand); | |||
return Script.runSimpleBashScript(Script.getExecutableAbsolutePath(cpuArchRetrieveExecutable)); |
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.
return Script.runSimpleBashScript(Script.getExecutableAbsolutePath(cpuArchRetrieveExecutable)); | |
return Script.runSimpleBashScript(Script.getExecutableAbsolutePath(cpuArchRetrieveExecutable), 1000); |
increase timeout required here as well?
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.
@sureshanaparti as this was already running without a timeout and I didn't face an issue here, I have not changed this
[SF] Trillian test result (tid-13906)
|
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.
clgtm
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.
LGTM, manually checked it.
Signed-off-by: Abhishek Kumar <[email protected]>
Description
Increases timeout for agent/host arch retrieval
Types of changes
Feature/Enhancement Scale or Bug Severity
Feature/Enhancement Scale
Bug Severity
Screenshots (if appropriate):
How Has This Been Tested?
Without change (getting errors like the following):
With change (no errors):
How did you try to break this feature and the system with this change?