Race condition in attaching/detaching volumes when compute manager is unreachable

[Loganathan Parthipan]

When a compute manager is offline, or if it cannot pick up messages for some reason, a race condition exists in attaching/detaching volumes.

Try attach and detach a volume and then bring the compute manager online. Then the reserve_block_device_name message gets delivered and a block_device_mapping is created for this instance/volume regardless of the state of the volume. This will result in the following issues.

1. The mountpoint is no longer be usable.
2. os-volume_attachments API will list the volume as attached to the instance.

Steps to reproduce (This was recreated in Devstack with nova trunk 75af47a.)

1. Spawn an instance (Mine is a multinode devstack setup, so I spawn it to a different machine than the api, but the race condition should be reproducible in a single-node setup too)
2. Create a volume
3. Stop the compute manager (n-cpu)
4. Try to attach the volume to the instance, it should fail after a while
5. Try to detach the volume
6. List the volumes. The volume should be in 'available' state. Optionally you can delete it at this point
7. Check db for block_device_mapping. It shouldn't have any reference to this volume
8. Start compute manager on the node that the instance is running
9. Check db for block_device_mapping and it should now have a new entry associating this volume and instance regardless of the state of the volume

[Michael Still (mikal)]

That's for the detailed bug report!

[OpenStack Infra (hudson-openstack)]

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

[Nikola Đipanov (ndipanov)]

After looking at this a bit more closely - I immediately thought that we need to keep track of the volume attach requests and make sure that it is not possible that a detach on a volume that is still not attached should not really succeede. I realized that we already keep track of volume states in cinder so why not use cinder to make sure that we are handling this as expected.

What the previous attempt proposed was IMHO not ideal as the device name decision really should be up to the virt driver ultimately, while the proposed solution moved it (back) into the database layer.

I propose that we add two things to the volume attach code that will remove this race.

1) Fail the attach if the compute node is not available.
2) Fail the detach if there is a pending attach call that is not completed (we can use cinder API for this by reserving the volume before we do the reserve_block_device rpc call).

[Loganathan Parthipan (parthipan)]

When you say 'Fail the attach' in (1) do you mean we fail-fast the API request itself?

[Nikola Đipanov (ndipanov)]

Hi, yes - exactly that.

I posted a patch, and again I have no idea why it didn't get picked up by LP.

https://review.openstack.org/#/c/81256/

[John Garbutt (johngarbutt)]

Not sure this blocks RC1, seems like a good fix, but its not a regression.

[Nikola Đipanov (ndipanov)]

Also after looking some more into this - this bug may be invalid after switching to oslo.messaging (actually this commit that fixes it might have got synced over before the switch https://github.com/openstack/oslo-incubator/commit/30a50c8a6c534f01d518eb3ce4cf0d35877d9a7f) as each message TTL is now equal to the call timeout, so as long as we timeout - we should be OK and the message won't be delivered after that.

We can still make this a bit less racy by reserving the volumes before making any RPC calls, but in most cases everything should clean up correctly now.

[Loganathan Parthipan (parthipan)]

I saw this issue almost 10 months ago. :) Haven't tried reproducing it recently.

[Tracy Jones (tjones-i)]

based on comments - removing from rc1

[OpenStack Infra (hudson-openstack)]

Related fix proposed to branch: master
Review: https://review.opendev.org/692940

[OpenStack Infra (hudson-openstack)]

Change abandoned by Matt Riedemann (<email address hidden>) on branch: master
Review: https://review.opendev.org/692940