mds: dump formatter even for errors by batrick · Pull Request #57673 · ceph/ceph

batrick · 2024-05-23T17:37:02Z

The admin_socket framework only dumps the formatter by default if the command succeeds.

Fixes: a4dc881
Fixes: https://tracker.ceph.com/issues/66184

Contribution Guidelines

To sign and title your commits, please refer to Submitting Patches to Ceph.
If you are submitting a fix for a stable branch (e.g. "quincy"), please refer to Submitting Patches to Ceph - Backports for the proper workflow.
When filling out the below checklist, you may click boxes directly in the GitHub web UI. When entering or editing the entire PR message in the GitHub web UI editor, you may also select a checklist item by adding an x between the brackets: [x]. Spaces and capitalization matter when checking off items this way.

Checklist

Tracker (select at least one)
- References tracker ticket
- Very recent bug; references commit where it was introduced
- New feature (ticket optional)
- Doc update (no ticket needed)
- Code cleanup (no ticket needed)
Component impact
- Affects Dashboard, opened tracker ticket
- Affects Orchestrator, opened tracker ticket
- No impact that needs to be tracked
Documentation (select at least one)
- Updates relevant documentation
- No doc update is appropriate
Tests (select at least one)
- Includes unit test(s)
- Includes integration test(s)
- Includes bug reproducer
- No tests

Show available Jenkins commands

jenkins retest this please
jenkins test classic perf
jenkins test crimson perf
jenkins test signed
jenkins test make check
jenkins test make check arm64
jenkins test submodules
jenkins test dashboard
jenkins test dashboard cephadm
jenkins test api
jenkins test docs
jenkins render docs
jenkins test ceph-volume all
jenkins test ceph-volume tox
jenkins test windows
jenkins test rook e2e

batrick · 2024-05-23T17:44:26Z

asok_command always prepends and "ERROR" message so we just can't/shouldn't use it for QA testing (where the command may fail).

batrick · 2024-05-23T17:44:43Z

(still more to fix...)

src/mds/MDSRank.cc

leonid-s-usov

Please note that tell_command will assert for non-zero status. You should use check_status=False kwarg if you want to process the rc externally, the asok_command had it

leonid-s-usov

Looks good!

vshankar · 2024-05-28T08:08:13Z

jenkins retest this please

batrick · 2024-05-28T12:46:58Z

jenkins test api

batrick · 2024-05-28T18:46:36Z

This PR is under test in https://tracker.ceph.com/issues/66261.

batrick · 2024-05-28T18:48:08Z

jenkins test api

batrick · 2024-05-30T18:01:31Z

jenkins test api

batrick · 2024-05-30T18:01:37Z

jenkins test make check

batrick · 2024-05-30T18:01:44Z

jenkins test make check arm64

batrick · 2024-06-04T17:59:59Z

jenkins test api

batrick · 2024-06-04T18:27:43Z

https://tracker.ceph.com/issues/66261#note-3

And change second argument to std::string_view to have more flexible conversions. Signed-off-by: Patrick Donnelly <pdonnell@redhat.com>

The admin_socker framework only dumps the formatter by default if the command succeeds. Fixes: a4dc881 Fixes: https://tracker.ceph.com/issues/66184 Signed-off-by: Patrick Donnelly <pdonnell@redhat.com>

Existing convention is that "css" is short for CachedStackStringStream while "ss" is stringstream. Signed-off-by: Patrick Donnelly <pdonnell@redhat.com>

It's unpleasant to test for existence in json. Just dump an empty string if not present. Signed-off-by: Patrick Donnelly <pdonnell@redhat.com>

As it's being used for error output. Signed-off-by: Patrick Donnelly <pdonnell@redhat.com>

The asok interface will mangle stdout if the command actually fails. The reason `flush path` is done via the asok interface is because the tell/asok interfaces were unified after these tests were written and `flush path` was only available via the asok interface. Fixes: https://tracker.ceph.com/issues/66184 Signed-off-by: Patrick Donnelly <pdonnell@redhat.com>

batrick · 2024-06-04T19:45:10Z

Pulled vstart_runner change out as it was failing api tests. It's now at #57881

batrick requested a review from leonid-s-usov May 23, 2024 17:37

github-actions bot added the cephfs Ceph File System label May 23, 2024

batrick marked this pull request as draft May 23, 2024 17:43

batrick marked this pull request as ready for review May 23, 2024 17:48

github-actions bot added the tests label May 23, 2024

leonid-s-usov reviewed May 23, 2024

View reviewed changes

src/mds/MDSRank.cc Outdated Show resolved Hide resolved

leonid-s-usov approved these changes May 23, 2024

View reviewed changes

batrick force-pushed the i66184 branch from ac68bba to 6381063 Compare May 23, 2024 20:01

leonid-s-usov suggested changes May 23, 2024

View reviewed changes

batrick force-pushed the i66184 branch from 6381063 to 9c58de7 Compare May 25, 2024 18:34

batrick requested a review from a team as a code owner May 25, 2024 18:34

batrick requested a review from leonid-s-usov May 25, 2024 18:34

github-actions bot added common core labels May 25, 2024

batrick force-pushed the i66184 branch from 9c58de7 to 6316895 Compare May 26, 2024 00:41

leonid-s-usov approved these changes May 27, 2024

View reviewed changes

vshankar approved these changes May 28, 2024

View reviewed changes

batrick force-pushed the i66184 branch from 6316895 to bd9340b Compare May 28, 2024 17:51

batrick added wip-pdonnell-testing needs-qa labels May 28, 2024

batrick force-pushed the i66184 branch from bd9340b to 01f645a Compare May 31, 2024 16:34

batrick added 6 commits June 4, 2024 15:44

common/admin_socket: create type for finisher callback

bd52d8c

And change second argument to std::string_view to have more flexible conversions. Signed-off-by: Patrick Donnelly <pdonnell@redhat.com>

mds: dump formatter even for errors

55a88a0

The admin_socker framework only dumps the formatter by default if the command succeeds. Fixes: a4dc881 Fixes: https://tracker.ceph.com/issues/66184 Signed-off-by: Patrick Donnelly <pdonnell@redhat.com>

mds: use appropriate abbrev. for variable name

4229c27

Existing convention is that "css" is short for CachedStackStringStream while "ss" is stringstream. Signed-off-by: Patrick Donnelly <pdonnell@redhat.com>

mds: unconditionally dump message in formatter

48a25e3

It's unpleasant to test for existence in json. Just dump an empty string if not present. Signed-off-by: Patrick Donnelly <pdonnell@redhat.com>

mds: dump AsyncHandler ss to stderr if present

b89e7d4

As it's being used for error output. Signed-off-by: Patrick Donnelly <pdonnell@redhat.com>

batrick force-pushed the i66184 branch from 01f645a to 715c951 Compare June 4, 2024 19:44

batrick merged commit dddc23d into ceph:main Jun 5, 2024

batrick deleted the i66184 branch June 5, 2024 00:49

batrick mentioned this pull request Jun 27, 2024

squid: mds: dump formatter even for errors #58319

Merged

Conversation

batrick commented May 23, 2024 • edited by leonid-s-usov Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Contribution Guidelines

Checklist

Uh oh!

batrick commented May 23, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

batrick commented May 23, 2024

Uh oh!

Uh oh!

leonid-s-usov left a comment

Choose a reason for hiding this comment

Uh oh!

leonid-s-usov left a comment

Choose a reason for hiding this comment

Uh oh!

vshankar commented May 28, 2024

Uh oh!

batrick commented May 28, 2024

Uh oh!

batrick commented May 28, 2024

Uh oh!

batrick commented May 28, 2024

Uh oh!

batrick commented May 30, 2024

Uh oh!

batrick commented May 30, 2024

Uh oh!

batrick commented May 30, 2024

Uh oh!

batrick commented Jun 4, 2024

Uh oh!

batrick commented Jun 4, 2024

Uh oh!

batrick commented Jun 4, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

batrick commented May 23, 2024 •

edited by leonid-s-usov

Loading

batrick commented May 23, 2024 •

edited

Loading