qa/iscsi_test: test also user:rbd (tcmu-runner) backstore #699
Conversation
Signed-off-by: Mykola Golub <mgolub@suse.com>
tserong
left a comment
tserong
left a comment
There was a problem hiding this comment.
LGTM, I just have one thing I'm confused about...
| ssh ${node} rbd showmapped | grep "${rbd_pool} *${rbd_image}" | ||
| ;; | ||
| user:rbd) | ||
| ssh ${node} rbd showmapped | grep "${rbd_pool} *${rbd_image}" && false |
There was a problem hiding this comment.
Why && false here? I realise that construct is also present in a couple other places in _iscsi_test(), but looking at it I feel like that should cause those lines to just always fail and abort the script. What am I missing?
There was a problem hiding this comment.
Sorry for the delayed response, my sesdev github notifications seem to be filtered-out some how, will need to check the filters.
Yes, the idea to check that it is not present in rbd showmapped output. We do this several times, e.g. at the beginning of the test to check the state clean and at the end of the test. But in this particular case (backstore=user:rbd) we check that is not in the list, because rbd map is used only for 'rbd' (kernel) backstore. And this check is to test that the 'backstore' setting is not ignored and is actually applied, and the default 'rbd' backstore is not used. It is not strictly necessary, because it would fail on the next check anyway (where we test that is is indeed tcmu), on the other hand the cause would not be so clear. Anyway, it is not mandatory and may safely remove it if you think it is confusing or unnecessary.
There was a problem hiding this comment.
Ah, actually now I understand what you mean. It looks like it should fail for both case (i.e. if the image is present in output or not). Hm, interesting why the test did not fail for me. I will need to recheck this
There was a problem hiding this comment.
I just checked, and it works as it was intended, i.e. if there is no the image in the list (grep fails) it does not abort and proceeds. And if the image is in the list (grep succeeds) then it aborts due to && false. Although if you run it from the command line and check $? it is 1 in both cases. I suppose it is a peculiarity of how set -e works that I am not aware of. I just saw this idiom in ceph tests many times and used it here not thinking much about it, so for me it did not look confusing, but if you think it is we may change it to something more clear.
There was a problem hiding this comment.
Actually the man bash explains this, and I remember I read it in the past (probably several times) but forgot after some time had passed.
-e Exit immediately if a pipeline (which may consist of a single simple command), a list, or a compound command (see SHELL GRAMMAR above), exits with a non-zero sta-
tus. The shell does not exit if the command that fails is part of the command list immediately following a while or until keyword, part of the test following the
if or elif reserved words, part of any command executed in a && or || list except the command following the final && or ||, any command in a pipeline but the last,
or if the command's return value is being inverted with !. If a compound command other than a subshell returns a non-zero status because a command failed while -e
was being ignored, the shell does not exit. A trap on ERR, if set, is executed before the shell exits. This option applies to the shell environment and each sub-
shell environment separately (see COMMAND EXECUTION ENVIRONMENT above), and may cause subshells to exit before executing all the commands in the subshell.
There was a problem hiding this comment.
Just looking at http://see.prv.suse.net:8080/blue/organizations/jenkins/sesdev-integration/detail/PR-699/1/pipeline, that CI test seems to have failed because:
master: + ssh master rbd showmapped
master: 0 rbd iscsi_disk_user - /dev/rbd0
So with backstore=user:rbd the volume is (incorrectly) mapped in this CI test. Note that the CI does a single node deployment on openSUSE 15.3, in case that makes any difference. I'll try to reproduce locally myself and poke around a bit...
There was a problem hiding this comment.
Hm, interesting. I will try to reproduce it with one node scenario, although I can't imaging how it could be related.
There was a problem hiding this comment.
Ah, it is failing for openSUSE. I think it is more important here. Right now I have no idea how iscsi support is built for openSUSE, if it has necessary bits. I will investigate. As the last resort we may enable this test for SES only.
There was a problem hiding this comment.
Hrm... openSUSE 15.3 is EOL since the end of December 2022. Under the circumstances, I wonder if we should maybe switch the CI to deploying ses7p on SLE, rather than pacific on openSUSE? I've opened #702 to see if that might work.
There was a problem hiding this comment.
I tried sesdev pacific deploy and reproduced the issue. The backstore=user:rbd fails because of this tcmu-runner error:
2023-06-23 10:46:30.521 7 [ERROR] add_device:456: could not find handler for uio0
And it is because tcmu-runner is built without rbd support here:
[ceph: root@node1 /]# ls /usr/lib64/tcmu-runner/
handler_qcow.so
There is no handler_rbd.so here.
I will modify the test to test backstore=user:rbd only if it is running on ses deployment.
which has tcmu-runner built with rbd support Signed-off-by: Mykola Golub <mgolub@suse.com>
|
Added a change that enables user:rbd backstore test only for SLES. |
Cool, LGTM. I've tested this locally as well against both ses7p (SLES) and pacific (openSUSE) and it works fine, so I'm going to go ahead and merge this. The CI failure is unrelated (see #703). |
2 participants
No description provided.