Skip to content

rptest: better error propagation from rp_storage_tool #27071

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

Open
wants to merge 2 commits into
base: dev
Choose a base branch
from

Conversation

nvartolomei
Copy link
Contributor

Backports Required

  • none - not a bug fix
  • none - this is a backport
  • none - issue does not exist in previous branches
  • none - papercut/not impactful enough to backport
  • v25.2.x
  • v25.1.x
  • v24.3.x
  • v24.2.x

Release Notes

  • none

@nvartolomei nvartolomei requested review from Lazin and oleiman July 30, 2025 21:36
@nvartolomei nvartolomei force-pushed the nv/rptest-rp-storage-tool-raise branch from 12e3658 to 6ce49d3 Compare July 30, 2025 21:39
dotnwat
dotnwat previously approved these changes Jul 31, 2025
Copy link
Member

@dotnwat dotnwat left a comment

Choose a reason for hiding this comment

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

lgtm

Less abstract failure messages.

Currently we get something like

    json.decoder.JSONDecodeError: Expecting value: line 1 column 1 (char 0)

Logs contain stdout/stderr but it is better to surface the error to the
user. Raise an exception with stderr message.
The service name is meaningless and we can infer it from stacktrace even
if it needed. The node on which the command failed is much more
relevant. It can also contain (temporary) files relevant for debugging
failures.
@nvartolomei nvartolomei force-pushed the nv/rptest-rp-storage-tool-raise branch from 6ce49d3 to a0bfb23 Compare July 31, 2025 11:01
@nvartolomei nvartolomei requested a review from dotnwat July 31, 2025 11:02
@nvartolomei
Copy link
Contributor Author

Looks like i was wrong on the first try. rp_storage_tool actually exits with non-zero and json to stdout which should be parsed. Reverted allow_fail change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants