Race condition in quickly attaching / deleting volumes

[Joseph Lanoux]

It seems that there is a race condition in the stress volume_attach_delete test. It creates VMs and volumes, attaches volumes and deletes everything.

The test is waiting for volumes to be in 'in-use' state before deleting VMs. It seems that Nova/Cinder don't have time to register volumes as attached in their databases before VMs get deleted. Volumes are then left attached to deleted VMs and unable to be deleted.

[David Kranz (david-kranz)]

I am not seeing how this is a bug in tempest. Tempest is deleting the vm only after nova reports that the volume is 'in-use' which seems fine. It would be nice if there was a backtrace, log, or something associated with this ticket. This might be a cinder issue but more likely nova and the test is making a nova call.

[Sean Dague (sdague)]

So I believe this is the same basic issue that exists between cinder and nova in the fact that it is assumed the event will happen within a reasonable amount of time, and if it doesn't... sadness.

[Trung Trinh (trung-t-trinh)]

Initial analysis of Nova code is as follows.

The operation of deleting VM doesn't check if the VM is being attached with volume or not.
The operation of attaching volume to VM can't receive any status response from Cinder .

There is no sync between Nova and Cinder under this scenario.
Nova has no mechanism of pending an operation. Indeed, it just raises some Exception if invalid state is detected.
Cinder has no mechanism of responding the operation status.

Therefore, it's nearly impossible to resolve this problem completely. So, we ought to live with this problem.

A work-around is that Cinder should allow to delete an orphaned volume (i.e. the volume attached to an already-deleted VM).

Any feedback/comment is always appreciated

[Trung Trinh (trung-t-trinh)]

Proposed work-around of Cinder source code to allow deleting of volume attached to the already-deleted VM:

In the function cinder.volume.api.API.delete(), there exists an if-clause as follows:

        if volume['attach_status'] == "attached":
            MY_LOG.debug("%s - API::delete - Volume is still attached, "\
                         "need to detach first" % __name__)

            # Volume is still attached, need to detach first
            raise exception.VolumeAttached(volume_id=volume_id)

Based on the above if-clause, it's possible to add further code to check if volume['instance_uuid'] results in a non-existed instance, then the deleting operation is still passed through.
The question is how to check if some instance_uuid exists or not from Cinder side?

[Trung Trinh (trung-t-trinh)]

Based on debugging, it can be seen that Nova's deleting of a VM always triggers Cinder's detaching of the associated volume.

[Trung Trinh (trung-t-trinh)]

The work-around of allowing to delete volume attached to an already-deleted VM is successful as below:

xtrutri@ubuntu:~/Work/devstack-stable-juno$ cinder list
+--------------------------------------+--------+--------------+------+-------------+----------+--------------------------------------+
| ID | Status | Display Name | Size | Volume Type | Bootable | Attached to |
+--------------------------------------+--------+--------------+------+-------------+----------+--------------------------------------+
| 5fc4746e-bc31-4764-8877-cf32de9b4e22 | in-use | new_vol | 1 | lvmdriver-1 | false | 8e6033c7-1450-4526-95c1-84879a51af5e |
+--------------------------------------+--------+--------------+------+-------------+----------+--------------------------------------+

xtrutri@ubuntu:~/Work/devstack-stable-juno$ cinder delete 5fc4746e-bc31-4764-8877-cf32de9b4e22

We can see that now it's possible to delete the volume whose status is still "in-use"

Please be noted the Cinder's detaching of the associated volume, which is triggered by Nova's deleting of a VM, is temporarily disabled for having the scenario that the volume is attached to an already-deleted VM

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: master
Review: https://review.openstack.org/142006

[OpenStack Infra (hudson-openstack)]

Change abandoned by Mike Perez (<email address hidden>) on branch: master
Review: https://review.openstack.org/142006
Reason: Over a month with no update.

[Matt Riedemann (mriedem)]

There is a change here https://review.openstack.org/#/c/184537/ that adds a nova-manage command to force-delete a volume attachment from a server instance.

[Walt Boring (walter-boring)]

I think changing the cinder API to just allow a volume to be deleted, without a force flag, when it's 'in-use' is a dangerous and wrong to do. We'll get users who do it by mistake and then nuke their volumes attached to live VMs.

So, it seems the issue is related to the fact that in Nova's code we nuke the VM first and then try and delete the volumes.
https://github.com/openstack/nova/blob/master/nova/compute/manager.py#L2365-L2375

Shouldn't nova ensure that it detaches the volume first before deleting it?
https://github.com/openstack/nova/blob/master/nova/compute/manager.py#L2307-L2315

Or check the exception that comes back from the volume_api.delete call and see if it complains because it's attached, then call terminate_connection, then delete.

[vishal yadav (vishalcdac07)]

>> So, it seems the issue is related to the fact that in Nova's code we nuke the VM first and then try and delete the volumes.
>> Shouldn't nova ensure that it detaches the volume first before deleting it?

I think above behavior likely to be changed in nova considering volume muti-attach feature. So nova should detach the volume while nuking the VM and should only delete the volume if it is not attached to other live VM/s.

>> Or check the exception that comes back from the volume_api.delete call and see if it complains because it's attached, then call terminate_connection, then delete.
This lazy verification might be used to avoid some race-conditions but can have performance penalty for VM terminate operation.

[Gourav Singla (gsingla294)]

I opine that we should detach the volume first in nova code and then delete the VM. if we get an error while detaching volume then don't delete the VM and raise the exception.

We can implement a verification check at the end of API whether the volume is detached successfully or not, but it will have some performance penalty.

Any thoughts!

[Prateek Arora (parora)]

One thing we can do here is have a periodic task in nova or cinder and find all the volumes that are attached to instances that do not exist and delete those volumes. Opinions are welcome on this solution.

[Sujitha (sujitha-neti)]

@zhaolihui Please change the status to InProgress if you are currently working on it.

[Markus Zoeller (markus_z) (mzoeller)]

This is an automated cleanup. This bug report has been closed because it
is older than 18 months and there is no open code change to fix this.
After this time it is unlikely that the circumstances which lead to
the observed issue can be reproduced.

If you can reproduce the bug, please:
* reopen the bug report (set to status "New")
* AND add the detailed steps to reproduce the issue (if applicable)
* AND leave a comment "CONFIRMED FOR: <RELEASE_NAME>"
  Only still supported release names are valid (LIBERTY, MITAKA, OCATA, NEWTON).
  Valid example: CONFIRMED FOR: LIBERTY

[Sean McGinnis (sean-mcginnis)]

I think things have changed enough that this particular failure isn't an issue anymore. Can anyone confirm?

[Michael Dovgal (mdovgal)]

This bug can be reproduced by cinder tempest tests on which i'm working now.

Also the way to reproduce it described in this bug [0] that i marked as duplicated

https://bugs.launchpad.net/cinder/+bug/1650277

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: master
Review: https://review.openstack.org/413141

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/413141
Committed: https://git.openstack.org/cgit/openstack/cinder/commit/?id=d0abb60fe7798c6fea4bfb9247db25cf15591340
Submitter: Jenkins
Branch: master

commit d0abb60fe7798c6fea4bfb9247db25cf15591340
Author: Michael Dovgal <email address hidden>
Date: Tue Dec 20 15:16:03 2016 +0200

    Attach/Delete volume for tgt driver race condition fix

    We have race condition when remove target operation still
    isn't completed and we try to run create target operation.
    This patch fixed this race condition by retrying create
    iscsi target operation run.

    Change-Id: I151990cc5148eb3f56e27e01fce71d87da2d9759
    Closes-Bug: #1335889

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/411277
Committed: https://git.openstack.org/cgit/openstack/cinder/commit/?id=4f5adf435eaf1f6f8b5ff8195a8f371afe3fbf21
Submitter: Jenkins
Branch: master

commit 4f5adf435eaf1f6f8b5ff8195a8f371afe3fbf21
Author: Michael Dovgal <email address hidden>
Date: Thu Dec 8 17:05:29 2016 +0200

    Add volume backup tempest tests

    Tempest tests that are added:
    1) test volume snapshot backup
    2) test backup create and restore to an existing volume
    This test can reproduce the bug that is related.
    3) test incremental backup

    Co-Authored-By: Oleksii Butenko <email address hidden>
    Related-Bug: #1335889

    Change-Id: Iff857ee11ed15665315077a0ba22cf31f92efe4b

[OpenStack Infra (hudson-openstack)]

This issue was fixed in the openstack/cinder 10.0.0.0b3 development milestone.