mon: MDSMonitor: proper error output if pool DNE on 'add_data_pool' #2773
Conversation
Signed-off-by: Joao Eduardo Luis <joao@redhat.com>
Fixes: #9852 Signed-off-by: Joao Eduardo Luis <joao@redhat.com>
Makes for readable, expectable, prettier code. Signed-off-by: Joao Eduardo Luis <joao@redhat.com>
ghost
added
the
| @@ -620,6 +620,10 @@ function test_mon_mds() | |||
| data3_pool=$(ceph osd dump | grep 'pool.*data3' | awk '{print $2;}') | |||
| ceph mds add_data_pool $data2_pool | |||
| ceph mds add_data_pool $data3_pool | |||
| ceph mds add_data_pool 100 >& $TMPFILE || true | |||
| check_response "Error ENOENT" | |||
There was a problem hiding this comment.
Shouldn't this be the ss << "pool '" << poolname << "' does not exist"; message that's set in filesystem_command?
There was a problem hiding this comment.
could be, but it should start with 'Error ENOENT' so unless we change the error outputting of the ceph tool (where I think the parsing happens, and which will break most if not all the tests on this file) it's irrelevant whether we test for this string or the string you pointed out. In a way, it's fairly more probable that the string from filesystem_command() is subjected to change than the 'Error ENOENT' part. Keeping 'Error ENOENT' will safeguard us against the test erroring out due to a string that changed, when we only want to test is that the command fails with an ENOENT.
There was a problem hiding this comment.
Oh right, you get both strings in the output. Never mind.
|
wip-sam-testing |
|
I'm going to put this in wip-sam-testing, but this actually needs to go through I'm guessing an mds suite before merge? |
|
wip-sam-testing |
|
Went through a multimds suite over the weekend and all failures are also happening on next, master and giant, being unrelated to the changes on this branch. I'm confident enough to merge. |
mon: MDSMonitor: proper error output if pool DNE on 'add_data_pool' Reviewed-by: Joao Eduardo Luis <joao@redhat.com>
Pull request also has two minor patches: one adding proper docs to a function
added in a previous pull request for #9794, another refactoring a function to
make it a bit saner according to the changes introduced when fixing #9794.
Fixes: #9852
Signed-off-by: Joao Eduardo Luis joao@redhat.com