Skip to content

agent: increase timeout for host arch retrieval#11822

Merged
weizhouapache merged 1 commit intoapache:4.20from
shapeblue:arm64-arch-backport
Oct 14, 2025
Merged

agent: increase timeout for host arch retrieval#11822
weizhouapache merged 1 commit intoapache:4.20from
shapeblue:arm64-arch-backport

Conversation

@yadvr
Copy link
Member

@yadvr yadvr commented Oct 10, 2025

Backport 44f8064 from #11254

Types of changes

  • Breaking change (fix or feature that would cause existing functionality to change)
  • New feature (non-breaking change which adds functionality)
  • Bug fix (non-breaking change which fixes an issue)
  • Enhancement (improves an existing feature and functionality)
  • Cleanup (Code refactoring and cleanup, that may add test cases)
  • Build/CI
  • Test (unit or integration test code)

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

  • Major
  • Minor

Bug Severity

  • BLOCKER
  • Critical
  • Major
  • Minor
  • Trivial

Screenshots (if appropriate):

How Has This Been Tested?

How did you try to break this feature and the system with this change?

@shwstppr @yadvr
agent: increase timeout for host arch retrieval (apache#11254)
7b604fc 
Cherry-picked from 44f8064

Signed-off-by: Abhishek Kumar <abhishek.mrt22@gmail.com>
@yadvr yadvr added this to the 4.20.2 milestone Oct 10, 2025
@yadvr
Copy link
Member Author

yadvr commented Oct 10, 2025

@blueorangutan package

@yadvr yadvr mentioned this pull request Oct 10, 2025
Closed
vishesh92
vishesh92 approved these changes Oct 10, 2025
View reviewed changes
Copy link
Member

@vishesh92 vishesh92 left a comment

Choose a reason for hiding this comment

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

clgtm

@codecov
Copy link

codecov bot commented Oct 10, 2025 •
edited
Loading

Codecov Report

❌ Patch coverage is 40.00000% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 16.17%. Comparing base (4d95f08) to head (7b604fc).
⚠️ Report is 2 commits behind head on 4.20.

Files with missing lines Patch % Lines
agent/src/main/java/com/cloud/agent/Agent.java 0.00% 3 Missing ⚠️
Additional details and impacted files 
@@             Coverage Diff              @@
##               4.20   #11822      +/-   ##
============================================
- Coverage     16.17%   16.17%   -0.01%     
- Complexity    13297    13298       +1     
============================================
  Files          5656     5656              
  Lines        498331   498331              
  Branches      60476    60476              
============================================
- Hits          80591    80590       -1     
- Misses       408767   408769       +2     
+ Partials       8973     8972       -1
Flag Coverage Δ
uitests 4.00% <ø> (ø)
unittests 17.02% <40.00%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

shwstppr
shwstppr requested changes Oct 10, 2025
View reviewed changes
agent/src/main/java/com/cloud/agent/Agent.java
final Script command = new Script("/usr/bin/arch", 500, logger);
final OutputInterpreter.OneLineParser parser = new OutputInterpreter.OneLineParser();
return command.execute(parser);
String arch = Script.runSimpleBashScript(Script.getExecutableAbsolutePath("arch"), 2000);
Copy link
Contributor

@shwstppr shwstppr Oct 10, 2025

Choose a reason for hiding this comment

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

original PR had timeout as 1000 (base on my tests with OL8 x86_64 #11251 (comment)), this has 2000. Is there a new finding? Would it make sense to make this configurable then?

Copy link
Member

@weizhouapache weizhouapache Oct 13, 2025

Choose a reason for hiding this comment

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

@rohityadavcloud
would it be better same as #11254 ?

Copy link
Member Author

@yadvr yadvr Oct 14, 2025

Choose a reason for hiding this comment

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

I've explained my thinking but feel free to revert otherwise @weizhouapache @shwstppr

@weizhouapache
Copy link
Member

weizhouapache commented Oct 10, 2025

@tverkade @scottsignal I

can you test the changes ? thanks

harikrishna-patnala
harikrishna-patnala approved these changes Oct 14, 2025
View reviewed changes
Copy link
Member

@harikrishna-patnala harikrishna-patnala left a comment

Choose a reason for hiding this comment

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

code LGTM

@weizhouapache
Copy link
Member

weizhouapache commented Oct 14, 2025

@shwstppr
is the timeout (2000 ms) ok to you ?

@shwstppr shwstppr self-requested a review October 14, 2025 08:06
@shwstppr
Copy link
Contributor

shwstppr commented Oct 14, 2025

@shwstppr is the timeout (2000 ms) ok to you ?

If that is something we've agreed then fine with me

@yadvr
Copy link
Member Author

yadvr commented Oct 14, 2025

The increased timeout just increases the chance without failure, that being max. timeout shouldn't affect other systems where the command returns quickly. @weizhouapache @shwstppr I had increased it as usually timeouts can be larger than it would rather take for a command.

@weizhouapache
Copy link
Member

weizhouapache commented Oct 14, 2025

The increased timeout just increases the chance without failure, that being max. timeout shouldn't affect other systems where the command returns quickly. @weizhouapache @shwstppr I had increased it as usually timeouts can be larger than it would rather take for a command.

thanks @rohityadavcloud @shwstppr for your comments.
if there is no objection, let's merge ?

@weizhouapache weizhouapache merged commit 6f931db into apache:4.20 Oct 14, 2025
24 of 25 checks passed
@DaanHoogland DaanHoogland deleted the arm64-arch-backport branch October 14, 2025 09:52
dhslove pushed a commit to ablecloud-team/ablestack-cloud that referenced this pull request Oct 17, 2025
@yadvr @shwstppr @dhslove
agent: increase timeout for host arch retrieval (apache#11254) (apach…
a08c0d9 
…e#11822)

Cherry-picked from 44f8064

Signed-off-by: Abhishek Kumar <abhishek.mrt22@gmail.com>
Co-authored-by: Abhishek Kumar <abhishek.mrt22@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@harikrishna-patnala harikrishna-patnala harikrishna-patnala approved these changes

@vishesh92 vishesh92 vishesh92 approved these changes

@DaanHoogland DaanHoogland Awaiting requested review from DaanHoogland

@weizhouapache weizhouapache Awaiting requested review from weizhouapache

@shwstppr shwstppr Awaiting requested review from shwstppr

Assignees

No one assigned

Labels

component:agent component:kvm

Projects

None yet

Milestone

4.20.2

Development

Successfully merging this pull request may close these issues.

5 participants

@yadvr @weizhouapache @shwstppr @harikrishna-patnala @vishesh92