Skip to content
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

diskmetrics: increase timeout of qemu img convert #3509

Merged

Conversation

christoph-zededa
Copy link
Contributor

diskmetrics: increase timeout of qemu img convert

this partially reverts 193147f889660068db07f61714dbf8899ab55ea0

during testing it has been found out that the timeout is too short,
therefore it is now set again to 5 days

this partially reverts 193147f

during testing it has been found out that the timeout is too short,
therefore it is now set again to 5 days

Signed-off-by: Christoph Ostarek <christoph@zededa.com>
@codecov
Copy link

codecov bot commented Oct 20, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (c79d3fe) 20.51% compared to head (e9f67c6) 20.49%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #3509      +/-   ##
==========================================
- Coverage   20.51%   20.49%   -0.02%     
==========================================
  Files         203      203              
  Lines       45430    45430              
==========================================
- Hits         9319     9311       -8     
- Misses      35430    35438       +8     
  Partials      681      681              

see 3 files with indirect coverage changes

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

@rouming rouming added the stable Should be backported to stable release(s) label Oct 20, 2023
@christoph-zededa christoph-zededa changed the title Increase qemu convert timeout again diskmetrics: increase timeout of qemu img convert Oct 20, 2023
Copy link
Contributor

@rouming rouming left a comment

Choose a reason for hiding this comment

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

Ack

@christoph-zededa
Copy link
Contributor Author

cc @zedi-pramodh

Copy link

@zedi-pramodh zedi-pramodh left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@eriknordmark eriknordmark left a comment

Choose a reason for hiding this comment

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

LGTM

@rouming rouming merged commit 196c3cf into lf-edge:master Oct 22, 2023
41 of 55 checks passed
@eriknordmark
Copy link
Contributor

@rouming did someone check whether the failure in the storage test suite (zfs) in https://github.com/lf-edge/eve/actions/runs/6592602265/job/17923425921?pr=3509 can be due to the changes in this PR? I see that test passing on other PRs but here it failed even after a re-run

@rouming
Copy link
Contributor

rouming commented Nov 6, 2023

@eriknordmark I did not check, but IMO 8 minutes (the timeout set before this fix) should be enough for test images. I assume in our tests we don't use terabytes images which require >8min conversion.

@eriknordmark
Copy link
Contributor

@rouming the issue ended up being that the code was structured so that we always ran with the default timeout. That was fixed in #3518 and #3523

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stable Should be backported to stable release(s)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants