-
Notifications
You must be signed in to change notification settings - Fork 559
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
rbd: call undoStagingTransaction() when NodeStageVolume() fails #2618
Conversation
/retest ci/centos/k8s-e2e-external-storage/1.21 |
https://jenkins-ceph-csi.apps.ocp.ci.centos.org/job/mini-e2e-helm_k8s-1.20/2991/display/redirect
|
This does not seem to fix #2610, but maybe it is still an improvement? |
@Mergifyio rebase |
✅ Branch has been successfully rebased |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Fails
|
ci/centos/mini-e2e/k8s-1.20 failed with logs:
|
@Mergifyio rebase |
✅ Branch has been successfully rebased |
@Mergifyio rebase |
✅ Branch has been successfully rebased |
/retest ci/centos/mini-e2e/k8s-1.21 |
/retest ci/centos/mini-e2e/k8s-1.21 |
/retest ci/centos/mini-e2e/k8s-1.21 |
/retest ci/centos/mini-e2e/k8s-1.21 |
/retest ci/centos/mini-e2e/k8s-1.21 |
On line 341 a `transaction` is created. This is passed to the deferred `undoStagingTransaction()` function when an error in the `NodeStageVolume` procedure is detected. So far, so good. However, on line 356 a new `transaction` is returned. This new `transaction` is not used for the defer call. By removing the empty `transaction` that is used in the defer call, and calling `undoStagingTransaction()` on an error of `stageTransaction()`, the code is a little simpler, and the cleanup of the transaction should be done correctly now. Updates: ceph#2610 Signed-off-by: Niels de Vos <ndevos@redhat.com>
On line 341 a
transaction
is created. This is passed to the deferredundoStagingTransaction()
function when an error in theNodeStageVolume
procedure is detected. So far, so good.However, on line 356 a new
transaction
is returned. This newtransaction
is not used for the defer call.By removing the empty
transaction
that is used in the defer call, andcalling
undoStagingTransaction()
on an error ofstageTransaction()
,the code is a little simpler, and the cleanup of the transaction should
be done correctly now.
Updates: #2610 (does not fix it)
Show available bot commands
These commands are normally not required, but in case of issues, leave any of
the following bot commands in an otherwise empty comment in this PR:
/retest ci/centos/<job-name>
: retest the<job-name>
after unrelatedfailure (please report the failure too!)
/retest all
: run this in case the CentOS CI failed to start/report any testprogress or results