Re: [openstack-dev] [nova][cinder] what are the key errors with volume detach

2015-12-18 Thread Matt Riedemann



On 12/17/2015 9:24 AM, Matt Riedemann wrote:



On 12/17/2015 8:51 AM, Andrea Rosa wrote:



The communication with cinder is async, Nova doesn't wait or check if
the detach on cinder side has been executed correctly.


Yeah, I guess nova gets the 202 back:

http://logs.openstack.org/18/258118/2/check/gate-tempest-dsvm-full-ceph/7a5290d/logs/screen-n-cpu.txt.gz#_2015-12-16_03_30_43_990



Should nova be waiting for detach to complete before it tries deleting
the volume (in the case that delete_on_termination=True in the bdm)?

Should nova be waiting (regardless of volume delete) for the volume
detach to complete - or timeout and fail the instance delete if it
doesn't?


I'll revisit this change next year trying to look at the problem in a
different way.
Thank you all for your time and all the suggestions.
--
Andrea Rosa

__

OpenStack Development Mailing List (not for usage questions)
Unsubscribe:
openstack-dev-requ...@lists.openstack.org?subject:unsubscribe
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev



I had a quick discussion with hemna this morning and he confirmed that
nova should be waiting for os-detach to complete before we try to delete
the volume, because if the volume status isn't 'available' the delete
will fail.

Also, if nova is hitting a failure to delete the volume it's swallowing
it by passing raise_exc=False to _cleanup_volumes here [1]. Then we go
on our merry way and delete the bdms in the nova database [2]. But I'd
think at that point we're orphaning volumes in cinder that think they
are still attached.

If this is passing today it's probably just luck that we're getting the
volume detached fast enough before we try to delete it.

[1]
https://github.com/openstack/nova/blob/master/nova/compute/manager.py#L2425-L2426

[2]
https://github.com/openstack/nova/blob/master/nova/compute/manager.py#L909



I've confirmed that we definitely race in the gate with detach of the 
volume and then deleting it, we fail to delete the volume about 28K 
times in a week in the gate [1].


I've opened a bug [2] to track fixing this.

[1] 
http://logstash.openstack.org/#dashboard/file/logstash.json?query=message:%5C%22Failed%20to%20delete%20volume%5C%22%20AND%20message:%5C%22due%20to%5C%22%20AND%20tags:%5C%22screen-n-cpu.txt%5C%22

[2] https://bugs.launchpad.net/nova/+bug/1527623

--

Thanks,

Matt Riedemann


__
OpenStack Development Mailing List (not for usage questions)
Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev


Re: [openstack-dev] [nova][cinder] what are the key errors with volume detach

2015-12-17 Thread Matt Riedemann



On 12/17/2015 8:51 AM, Andrea Rosa wrote:



The communication with cinder is async, Nova doesn't wait or check if
the detach on cinder side has been executed correctly.


Yeah, I guess nova gets the 202 back:

http://logs.openstack.org/18/258118/2/check/gate-tempest-dsvm-full-ceph/7a5290d/logs/screen-n-cpu.txt.gz#_2015-12-16_03_30_43_990


Should nova be waiting for detach to complete before it tries deleting
the volume (in the case that delete_on_termination=True in the bdm)?

Should nova be waiting (regardless of volume delete) for the volume
detach to complete - or timeout and fail the instance delete if it doesn't?


I'll revisit this change next year trying to look at the problem in a
different way.
Thank you all for your time and all the suggestions.
--
Andrea Rosa

__
OpenStack Development Mailing List (not for usage questions)
Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev



I had a quick discussion with hemna this morning and he confirmed that 
nova should be waiting for os-detach to complete before we try to delete 
the volume, because if the volume status isn't 'available' the delete 
will fail.


Also, if nova is hitting a failure to delete the volume it's swallowing 
it by passing raise_exc=False to _cleanup_volumes here [1]. Then we go 
on our merry way and delete the bdms in the nova database [2]. But I'd 
think at that point we're orphaning volumes in cinder that think they 
are still attached.


If this is passing today it's probably just luck that we're getting the 
volume detached fast enough before we try to delete it.


[1] 
https://github.com/openstack/nova/blob/master/nova/compute/manager.py#L2425-L2426
[2] 
https://github.com/openstack/nova/blob/master/nova/compute/manager.py#L909


--

Thanks,

Matt Riedemann


__
OpenStack Development Mailing List (not for usage questions)
Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev


Re: [openstack-dev] [nova][cinder] what are the key errors with volume detach

2015-12-17 Thread Andrea Rosa

>> The communication with cinder is async, Nova doesn't wait or check if
>> the detach on cinder side has been executed correctly.
> 
> Yeah, I guess nova gets the 202 back:
> 
> http://logs.openstack.org/18/258118/2/check/gate-tempest-dsvm-full-ceph/7a5290d/logs/screen-n-cpu.txt.gz#_2015-12-16_03_30_43_990
> 
> 
> Should nova be waiting for detach to complete before it tries deleting
> the volume (in the case that delete_on_termination=True in the bdm)?
> 
> Should nova be waiting (regardless of volume delete) for the volume
> detach to complete - or timeout and fail the instance delete if it doesn't?

I'll revisit this change next year trying to look at the problem in a
different way.
Thank you all for your time and all the suggestions.
--
Andrea Rosa

__
OpenStack Development Mailing List (not for usage questions)
Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev


Re: [openstack-dev] [nova][cinder] what are the key errors with volume detach

2015-12-16 Thread Matt Riedemann



On 12/14/2015 11:24 AM, Andrea Rosa wrote:



On 10/12/15 15:29, Matt Riedemann wrote:


In a simplified view of a detach volume we can say that the nova code
does:
1 detach the volume from the instance
2 Inform cinder about the detach and call the terminate_connection on
the cinder API.
3 delete the dbm recod in the nova DB


We actually:

1. terminate the connection in cinder:

https://github.com/openstack/nova/blob/c4ca1abb4a49bf0bce765acd3ce906bd117ce9b7/nova/compute/manager.py#L2312


2. detach the volume

https://github.com/openstack/nova/blob/c4ca1abb4a49bf0bce765acd3ce906bd117ce9b7/nova/compute/manager.py#L2315


3. delete the volume (if marked for delete_on_termination):

https://github.com/openstack/nova/blob/c4ca1abb4a49bf0bce765acd3ce906bd117ce9b7/nova/compute/manager.py#L2348


4. delete the bdm in the nova db:

https://github.com/openstack/nova/blob/c4ca1abb4a49bf0bce765acd3ce906bd117ce9b7/nova/compute/manager.py#L908




I am confused here, why are are you referring to the _shutdown_instance
code?


Because that's the code in the compute manager that calls cinder to 
terminate the connection to the storage backend and detaches the volume 
from the instance, which you pointed out in your email as part of 
terminating the instance.






So if terminate_connection fails, we shouldn't get to detach. And if
detach fails, we shouldn't get to delete.



If 2 fails the volumes get stuck in a detaching status and any further
attempt to delete or detach the volume will fail:
"Delete for volume  failed: Volume  is still
attached, detach volume first. (HTTP 400)"

And if you try to detach:
"EROR (BadRequest): Invalid input received: Invalid volume: Unable to
detach volume. Volume status must be 'in-use' and attach_status must
be 'attached' to detach. Currently: status: 'detaching',
attach_status: 'attached.' (HTTP 400)"

at the moment the only way to clean up the situation is to hack the
nova DB for deleting the bdm record and do some hack on the cinder
side as well.
We wanted a way to clean up the situation avoiding the manual hack to
the nova DB.


Can't cinder rollback state somehow if it's bogus or failed an
operation? For example, if detach failed, shouldn't we not be in
'detaching' state? This is like auto-reverting task_state on server
instances when an operation fails so that we can reset or delete those
servers if needed.


I think that is an option but probably it is part of the redesign of the
cinder API (see the solution proposed #3), but It would be nice to get
cinder guys commenting here.


Solution proposed #3
Ok, so the solution is to fix the Cinder API and makes the interaction
between Nova volume manager and that API robust.
This time I was right (YAY) but as you can imagine this fix is not
going to be an easy one and after talking with Cinder guys they
clearly told me that thatt is going to be a massive change in the
Cinder API and it is unlikely to land in the N(utella) or O(melette)
release.



As Sean pointed out in another reply, I feel like what we're really
missing here is some rollback code in the case that delete fails so we
don't get in this stuck state and have to rely on deleting the BDMs
manually in the database just to delete the instance.

We should rollback on delete fail 1 so that delete request 2 can pass
the 'check attach' checks again.


The communication with cinder is async, Nova doesn't wait or check if
the detach on cinder side has been executed correctly.


Yeah, I guess nova gets the 202 back:

http://logs.openstack.org/18/258118/2/check/gate-tempest-dsvm-full-ceph/7a5290d/logs/screen-n-cpu.txt.gz#_2015-12-16_03_30_43_990

Should nova be waiting for detach to complete before it tries deleting 
the volume (in the case that delete_on_termination=True in the bdm)?


Should nova be waiting (regardless of volume delete) for the volume 
detach to complete - or timeout and fail the instance delete if it doesn't?




Thanks
--
Andrea Rosa

__
OpenStack Development Mailing List (not for usage questions)
Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev



--

Thanks,

Matt Riedemann


__
OpenStack Development Mailing List (not for usage questions)
Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev


Re: [openstack-dev] [nova][cinder] what are the key errors with volume detach

2015-12-14 Thread Andrea Rosa


On 10/12/15 15:29, Matt Riedemann wrote:

>> In a simplified view of a detach volume we can say that the nova code
>> does:
>> 1 detach the volume from the instance
>> 2 Inform cinder about the detach and call the terminate_connection on
>> the cinder API.
>> 3 delete the dbm recod in the nova DB
> 
> We actually:
> 
> 1. terminate the connection in cinder:
> 
> https://github.com/openstack/nova/blob/c4ca1abb4a49bf0bce765acd3ce906bd117ce9b7/nova/compute/manager.py#L2312
> 
> 
> 2. detach the volume
> 
> https://github.com/openstack/nova/blob/c4ca1abb4a49bf0bce765acd3ce906bd117ce9b7/nova/compute/manager.py#L2315
> 
> 
> 3. delete the volume (if marked for delete_on_termination):
> 
> https://github.com/openstack/nova/blob/c4ca1abb4a49bf0bce765acd3ce906bd117ce9b7/nova/compute/manager.py#L2348
> 
> 
> 4. delete the bdm in the nova db:
> 
> https://github.com/openstack/nova/blob/c4ca1abb4a49bf0bce765acd3ce906bd117ce9b7/nova/compute/manager.py#L908
> 
> 

I am confused here, why are are you referring to the _shutdown_instance
code?


> So if terminate_connection fails, we shouldn't get to detach. And if
> detach fails, we shouldn't get to delete.
> 
>>
>> If 2 fails the volumes get stuck in a detaching status and any further
>> attempt to delete or detach the volume will fail:
>> "Delete for volume  failed: Volume  is still
>> attached, detach volume first. (HTTP 400)"
>>
>> And if you try to detach:
>> "EROR (BadRequest): Invalid input received: Invalid volume: Unable to
>> detach volume. Volume status must be 'in-use' and attach_status must
>> be 'attached' to detach. Currently: status: 'detaching',
>> attach_status: 'attached.' (HTTP 400)"
>>
>> at the moment the only way to clean up the situation is to hack the
>> nova DB for deleting the bdm record and do some hack on the cinder
>> side as well.
>> We wanted a way to clean up the situation avoiding the manual hack to
>> the nova DB.
> 
> Can't cinder rollback state somehow if it's bogus or failed an
> operation? For example, if detach failed, shouldn't we not be in
> 'detaching' state? This is like auto-reverting task_state on server
> instances when an operation fails so that we can reset or delete those
> servers if needed.

I think that is an option but probably it is part of the redesign of the
cinder API (see the solution proposed #3), but It would be nice to get
cinder guys commenting here.

>> Solution proposed #3
>> Ok, so the solution is to fix the Cinder API and makes the interaction
>> between Nova volume manager and that API robust.
>> This time I was right (YAY) but as you can imagine this fix is not
>> going to be an easy one and after talking with Cinder guys they
>> clearly told me that thatt is going to be a massive change in the
>> Cinder API and it is unlikely to land in the N(utella) or O(melette) 
>> release.

> As Sean pointed out in another reply, I feel like what we're really
> missing here is some rollback code in the case that delete fails so we
> don't get in this stuck state and have to rely on deleting the BDMs
> manually in the database just to delete the instance.
> 
> We should rollback on delete fail 1 so that delete request 2 can pass
> the 'check attach' checks again.

The communication with cinder is async, Nova doesn't wait or check if
the detach on cinder side has been executed correctly.

Thanks
--
Andrea Rosa

__
OpenStack Development Mailing List (not for usage questions)
Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev


Re: [openstack-dev] [nova][cinder] what are the key errors with volume detach

2015-12-14 Thread Andrea Rosa


On 10/12/15 14:42, Sean Dague wrote:
> On 12/02/2015 12:37 PM, Rosa, Andrea (HP Cloud Services) wrote:
>> Hi
>>
>> thanks Sean for bringing this point, I have been working on the change and 
>> on the (abandoned) spec.
>> I'll try here to summarize all the discussions we had and what we decided.
>>
>>> From: Sean Dague [mailto:s...@dague.net]
>>> Sent: 02 December 2015 13:31
>>> To: OpenStack Development Mailing List (not for usage questions)
>>> Subject: [openstack-dev] [nova] what are the key errors with volume detach
>>>
>>> This patch to add a bunch of logic to nova-manage for forcing volume detach
>>> raised a bunch of questions
>>> https://review.openstack.org/#/c/184537/24/nova/cmd/manage.py,cm
>>
>> On this specific review there are some valid concerns that I am happy to 
>> address, but first we need to understand if we want this change.
>> FWIW I think it is still a valid change, please see below.
>>
>>> In thinking about this for the last day, I think the real concern is that 
>>> we have
>>> so many safety checks on volume delete, that if we failed with a partially
>>> setup volume, we have too many safety latches to tear it down again.
>>>
>>> Do we have some detailed bugs about how that happens? Is it possible to
>>> just fix DELETE to work correctly even when we're in these odd states?
>>
>> In a simplified view of a detach volume we can say that the nova code does:
>> 1 detach the volume from the instance
>> 2 Inform cinder about the detach and call the terminate_connection on the 
>> cinder API. 
>> 3 delete the dbm recod in the nova DB
>>
>> If 2 fails the volumes get stuck in a detaching status and any further 
>> attempt to delete or detach the volume will fail:
>> "Delete for volume  failed: Volume  is still attached, 
>> detach volume first. (HTTP 400)"
> 
> So why isn't this handled in a "finally" pattern.
> 
> Ensure that you always do 2 (a) & (b) and 3, collect errors that happen
> during 2 (a) & (b), report them back to the user.
> What state does that leave things in? Both from the server and the volume.
> 

The detach volume in cinder (2.a 2.b) is an async call, if Nova can talk
to the Cinder API it sends the request and if the detach on the Cinder
side fails Nova doesn't know about it.
--
Andrea Rosa



__
OpenStack Development Mailing List (not for usage questions)
Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev


Re: [openstack-dev] [nova][cinder] what are the key errors with volume detach

2015-12-10 Thread Matt Riedemann



On 12/2/2015 11:37 AM, Rosa, Andrea (HP Cloud Services) wrote:

Hi

thanks Sean for bringing this point, I have been working on the change and on 
the (abandoned) spec.
I'll try here to summarize all the discussions we had and what we decided.


From: Sean Dague [mailto:s...@dague.net]
Sent: 02 December 2015 13:31
To: OpenStack Development Mailing List (not for usage questions)
Subject: [openstack-dev] [nova] what are the key errors with volume detach

This patch to add a bunch of logic to nova-manage for forcing volume detach
raised a bunch of questions
https://review.openstack.org/#/c/184537/24/nova/cmd/manage.py,cm


On this specific review there are some valid concerns that I am happy to 
address, but first we need to understand if we want this change.
FWIW I think it is still a valid change, please see below.


In thinking about this for the last day, I think the real concern is that we 
have
so many safety checks on volume delete, that if we failed with a partially
setup volume, we have too many safety latches to tear it down again.

Do we have some detailed bugs about how that happens? Is it possible to
just fix DELETE to work correctly even when we're in these odd states?


In a simplified view of a detach volume we can say that the nova code does:
1 detach the volume from the instance
2 Inform cinder about the detach and call the terminate_connection on the 
cinder API.
3 delete the dbm recod in the nova DB


We actually:

1. terminate the connection in cinder:

https://github.com/openstack/nova/blob/c4ca1abb4a49bf0bce765acd3ce906bd117ce9b7/nova/compute/manager.py#L2312

2. detach the volume

https://github.com/openstack/nova/blob/c4ca1abb4a49bf0bce765acd3ce906bd117ce9b7/nova/compute/manager.py#L2315

3. delete the volume (if marked for delete_on_termination):

https://github.com/openstack/nova/blob/c4ca1abb4a49bf0bce765acd3ce906bd117ce9b7/nova/compute/manager.py#L2348

4. delete the bdm in the nova db:

https://github.com/openstack/nova/blob/c4ca1abb4a49bf0bce765acd3ce906bd117ce9b7/nova/compute/manager.py#L908

So if terminate_connection fails, we shouldn't get to detach. And if 
detach fails, we shouldn't get to delete.




If 2 fails the volumes get stuck in a detaching status and any further attempt 
to delete or detach the volume will fail:
"Delete for volume  failed: Volume  is still attached, detach 
volume first. (HTTP 400)"

And if you try to detach:
"EROR (BadRequest): Invalid input received: Invalid volume: Unable to detach volume. 
Volume status must be 'in-use' and attach_status must be 'attached' to detach. Currently: 
status: 'detaching', attach_status: 'attached.' (HTTP 400)"

at the moment the only way to clean up the situation is to hack the nova DB for 
deleting the bdm record and do some hack on the cinder side as well.
We wanted a way to clean up the situation avoiding the manual hack to the nova 
DB.


Can't cinder rollback state somehow if it's bogus or failed an 
operation? For example, if detach failed, shouldn't we not be in 
'detaching' state? This is like auto-reverting task_state on server 
instances when an operation fails so that we can reset or delete those 
servers if needed.




Solution proposed #1
Move the deletion of the bdm record so as it happens before calling cinder, I 
thought that was ok as from nova side we have done, no leaking bdm and the 
problem was just in the cinder side, but I was wrong.
We have to call the terminate_connection otherwise the device may show back on 
the nova host, for example that is true for iSCSI volumes:
  "if an iSCSI session from the compute host to the storage backend still exists 
(because other volumes are connected), then the volume you just removed will show back up 
on the next scsi bus rescan."
The key point here is that Nova must call the terminate_connection because just Nova has 
the "connector info" to call the terminate connection method, so Cinder can't 
fix it.

Solution proposed #2
Then I thought, ok, so let's expose a new nova API called force delete volume which skips 
all the check and allow to detach a volume in "detaching" status, I thought it 
was ok but I was wrong (again).
The main concern here is that we do not want to have the concept of "force 
delete", the user already asked for detaching the volume and the call should be 
idempotent and just work.
So adding a new API was just adding a technical debt in the RESP API for a 
buggy/weak interaction between the Cinder API and Nova, or in other words we 
are adding a Nova API for fixing a bug in Cinder, which is very odd.

Solution proposed #3
Ok, so the solution is to fix the Cinder API and makes the interaction between 
Nova volume manager and that API robust.
This time I was right (YAY) but as you can imagine this fix is not going to be 
an easy one and after talking with Cinder guys they clearly told me that thatt 
is going to be a massive change in the Cinder API and it is unlikely to land in 
the N(utella) or O(melette)  release.

Solution propo

Re: [openstack-dev] [nova][cinder] what are the key errors with volume detach

2015-12-10 Thread Sean Dague
On 12/02/2015 12:37 PM, Rosa, Andrea (HP Cloud Services) wrote:
> Hi
> 
> thanks Sean for bringing this point, I have been working on the change and on 
> the (abandoned) spec.
> I'll try here to summarize all the discussions we had and what we decided.
> 
>> From: Sean Dague [mailto:s...@dague.net]
>> Sent: 02 December 2015 13:31
>> To: OpenStack Development Mailing List (not for usage questions)
>> Subject: [openstack-dev] [nova] what are the key errors with volume detach
>>
>> This patch to add a bunch of logic to nova-manage for forcing volume detach
>> raised a bunch of questions
>> https://review.openstack.org/#/c/184537/24/nova/cmd/manage.py,cm
> 
> On this specific review there are some valid concerns that I am happy to 
> address, but first we need to understand if we want this change.
> FWIW I think it is still a valid change, please see below.
> 
>> In thinking about this for the last day, I think the real concern is that we 
>> have
>> so many safety checks on volume delete, that if we failed with a partially
>> setup volume, we have too many safety latches to tear it down again.
>>
>> Do we have some detailed bugs about how that happens? Is it possible to
>> just fix DELETE to work correctly even when we're in these odd states?
> 
> In a simplified view of a detach volume we can say that the nova code does:
> 1 detach the volume from the instance
> 2 Inform cinder about the detach and call the terminate_connection on the 
> cinder API. 
> 3 delete the dbm recod in the nova DB
> 
> If 2 fails the volumes get stuck in a detaching status and any further 
> attempt to delete or detach the volume will fail:
> "Delete for volume  failed: Volume  is still attached, 
> detach volume first. (HTTP 400)"

So why isn't this handled in a "finally" pattern.

Ensure that you always do 2 (a) & (b) and 3, collect errors that happen
during 2 (a) & (b), report them back to the user.

What state does that leave things in? Both from the server and the volume.

-Sean

-- 
Sean Dague
http://dague.net

__
OpenStack Development Mailing List (not for usage questions)
Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev


Re: [openstack-dev] [nova][cinder] what are the key errors with volume detach

2015-12-08 Thread hao wang
Thanks to Andrea for clarifying this complex and long history problem.

As he says, we have talked about many solutions, and finally we found
this using nova-mange to call the terminate_connection and clean the
bdm entry.
It's simple and clear IMO and no bad impact to Nova API.

2015-12-03 1:37 GMT+08:00 Rosa, Andrea (HP Cloud Services)
:
> Hi
>
> thanks Sean for bringing this point, I have been working on the change and on 
> the (abandoned) spec.
> I'll try here to summarize all the discussions we had and what we decided.
>
>> From: Sean Dague [mailto:s...@dague.net]
>> Sent: 02 December 2015 13:31
>> To: OpenStack Development Mailing List (not for usage questions)
>> Subject: [openstack-dev] [nova] what are the key errors with volume detach
>>
>> This patch to add a bunch of logic to nova-manage for forcing volume detach
>> raised a bunch of questions
>> https://review.openstack.org/#/c/184537/24/nova/cmd/manage.py,cm
>
> On this specific review there are some valid concerns that I am happy to 
> address, but first we need to understand if we want this change.
> FWIW I think it is still a valid change, please see below.
>
>> In thinking about this for the last day, I think the real concern is that we 
>> have
>> so many safety checks on volume delete, that if we failed with a partially
>> setup volume, we have too many safety latches to tear it down again.
>>
>> Do we have some detailed bugs about how that happens? Is it possible to
>> just fix DELETE to work correctly even when we're in these odd states?
>
> In a simplified view of a detach volume we can say that the nova code does:
> 1 detach the volume from the instance
> 2 Inform cinder about the detach and call the terminate_connection on the 
> cinder API.
> 3 delete the dbm recod in the nova DB
>
> If 2 fails the volumes get stuck in a detaching status and any further 
> attempt to delete or detach the volume will fail:
> "Delete for volume  failed: Volume  is still attached, 
> detach volume first. (HTTP 400)"
>
> And if you try to detach:
> "EROR (BadRequest): Invalid input received: Invalid volume: Unable to detach 
> volume. Volume status must be 'in-use' and attach_status must be 'attached' 
> to detach. Currently: status: 'detaching', attach_status: 'attached.' (HTTP 
> 400)"
>
> at the moment the only way to clean up the situation is to hack the nova DB 
> for deleting the bdm record and do some hack on the cinder side as well.
> We wanted a way to clean up the situation avoiding the manual hack to the 
> nova DB.
>
> Solution proposed #1
> Move the deletion of the bdm record so as it happens before calling cinder, I 
> thought that was ok as from nova side we have done, no leaking bdm and the 
> problem was just in the cinder side, but I was wrong.
> We have to call the terminate_connection otherwise the device may show back 
> on the nova host, for example that is true for iSCSI volumes:
>  "if an iSCSI session from the compute host to the storage backend still 
> exists (because other volumes are connected), then the volume you just 
> removed will show back up on the next scsi bus rescan."
> The key point here is that Nova must call the terminate_connection because 
> just Nova has the "connector info" to call the terminate connection method, 
> so Cinder can't fix it.
>
> Solution proposed #2
> Then I thought, ok, so let's expose a new nova API called force delete volume 
> which skips all the check and allow to detach a volume in "detaching" status, 
> I thought it was ok but I was wrong (again).
> The main concern here is that we do not want to have the concept of "force 
> delete", the user already asked for detaching the volume and the call should 
> be idempotent and just work.
> So adding a new API was just adding a technical debt in the RESP API for a 
> buggy/weak interaction between the Cinder API and Nova, or in other words we 
> are adding a Nova API for fixing a bug in Cinder, which is very odd.
>
> Solution proposed #3
> Ok, so the solution is to fix the Cinder API and makes the interaction 
> between Nova volume manager and that API robust.
> This time I was right (YAY) but as you can imagine this fix is not going to 
> be an easy one and after talking with Cinder guys they clearly told me that 
> thatt is going to be a massive change in the Cinder API and it is unlikely to 
> land in the N(utella) or O(melette)  release.
>
> Solution proposed #4: The trade-off
> This solution is the solution I am proposing in the patch you mentioned, the 
> idea is to have a temporary solution which allows us to give a handy tool for 
> the operators to easily fix a problem which can occur quite often.
> The solution should be something with a low impact on the nova code and easy 
> to remove when we will have the proper solution for the root cause.
> "quick", "useful", "dirty", "tool", "trade-off",  "db"we call it 
> nova-manage!
> The idea is to put a new method in the compute/api.py which skips all the 
> checks on the volume sta

Re: [openstack-dev] [nova][cinder] what are the key errors with volume detach

2015-12-02 Thread Rosa, Andrea (HP Cloud Services)
Hi

thanks Sean for bringing this point, I have been working on the change and on 
the (abandoned) spec.
I'll try here to summarize all the discussions we had and what we decided.

> From: Sean Dague [mailto:s...@dague.net]
> Sent: 02 December 2015 13:31
> To: OpenStack Development Mailing List (not for usage questions)
> Subject: [openstack-dev] [nova] what are the key errors with volume detach
> 
> This patch to add a bunch of logic to nova-manage for forcing volume detach
> raised a bunch of questions
> https://review.openstack.org/#/c/184537/24/nova/cmd/manage.py,cm

On this specific review there are some valid concerns that I am happy to 
address, but first we need to understand if we want this change.
FWIW I think it is still a valid change, please see below.

> In thinking about this for the last day, I think the real concern is that we 
> have
> so many safety checks on volume delete, that if we failed with a partially
> setup volume, we have too many safety latches to tear it down again.
> 
> Do we have some detailed bugs about how that happens? Is it possible to
> just fix DELETE to work correctly even when we're in these odd states?

In a simplified view of a detach volume we can say that the nova code does:
1 detach the volume from the instance
2 Inform cinder about the detach and call the terminate_connection on the 
cinder API. 
3 delete the dbm recod in the nova DB

If 2 fails the volumes get stuck in a detaching status and any further attempt 
to delete or detach the volume will fail:
"Delete for volume  failed: Volume  is still attached, 
detach volume first. (HTTP 400)"

And if you try to detach:
"EROR (BadRequest): Invalid input received: Invalid volume: Unable to detach 
volume. Volume status must be 'in-use' and attach_status must be 'attached' to 
detach. Currently: status: 'detaching', attach_status: 'attached.' (HTTP 400)"

at the moment the only way to clean up the situation is to hack the nova DB for 
deleting the bdm record and do some hack on the cinder side as well.
We wanted a way to clean up the situation avoiding the manual hack to the nova 
DB.

Solution proposed #1
Move the deletion of the bdm record so as it happens before calling cinder, I 
thought that was ok as from nova side we have done, no leaking bdm and the 
problem was just in the cinder side, but I was wrong.
We have to call the terminate_connection otherwise the device may show back on 
the nova host, for example that is true for iSCSI volumes:
 "if an iSCSI session from the compute host to the storage backend still exists 
(because other volumes are connected), then the volume you just removed will 
show back up on the next scsi bus rescan."
The key point here is that Nova must call the terminate_connection because just 
Nova has the "connector info" to call the terminate connection method, so 
Cinder can't fix it.

Solution proposed #2
Then I thought, ok, so let's expose a new nova API called force delete volume 
which skips all the check and allow to detach a volume in "detaching" status, I 
thought it was ok but I was wrong (again).
The main concern here is that we do not want to have the concept of "force 
delete", the user already asked for detaching the volume and the call should be 
idempotent and just work. 
So adding a new API was just adding a technical debt in the RESP API for a 
buggy/weak interaction between the Cinder API and Nova, or in other words we 
are adding a Nova API for fixing a bug in Cinder, which is very odd.

Solution proposed #3
Ok, so the solution is to fix the Cinder API and makes the interaction between 
Nova volume manager and that API robust. 
This time I was right (YAY) but as you can imagine this fix is not going to be 
an easy one and after talking with Cinder guys they clearly told me that thatt 
is going to be a massive change in the Cinder API and it is unlikely to land in 
the N(utella) or O(melette)  release.

Solution proposed #4: The trade-off
This solution is the solution I am proposing in the patch you mentioned, the 
idea is to have a temporary solution which allows us to give a handy tool for 
the operators to easily fix a problem which can occur quite often.
The solution should be something with a low impact on the nova code and easy to 
remove when we will have the proper solution for the root cause.
"quick", "useful", "dirty", "tool", "trade-off",  "db"we call it 
nova-manage!
The idea is to put a new method in the compute/api.py which skips all the 
checks on the volume status and go ahead with calling the detach_volume on the 
compute manager to detach the volume, call the terminate_connection and clean 
the bdm entry.
Nova-manage will have a command to call directly that new method.

That is a recap that I hope can help to understand how we decided to use 
nova-manage instead of other solutions, it was a bit long but I tried to 
condensate the comments from 53 spec patches + 24 code change patches (and 
counting).

PS: I added the cinder tag as they are