Cinder shouldn't fail a detach call for a volume that's not attached

Bug #1484194 reported by John Griffith
28
This bug affects 6 people
Affects Status Importance Assigned to Milestone
Cinder
Fix Released
Medium
Walt Boring
Kilo
Fix Released
Undecided
Unassigned

Bug Description

Currently Cinder-manage.detach does a db status check and if it finds the volume is not in-use/attached it logs and error and raises an exception, failing the call.

This probably isn't valuable, if the volume isn't attached, the caller is getting what they want in the end and we basically jam everything up by doing this. It's similar to the case of delete on the backend when the device doesn't have the volume.

We should just log a warning and return and not have a cow.

Changed in cinder:
assignee: nobody → Walt Boring (walter-boring)
status: New → Confirmed
importance: Undecided → Medium
Changed in cinder:
status: Confirmed → In Progress
Revision history for this message
Walt Boring (walter-boring) wrote :

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

Not sure why this didn't get added her automatically

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to cinder (master)

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

commit 3b4e8c31a2c94cac3e1a5c071eb38c7d5cb9a239
Author: Walter A. Boring IV <email address hidden>
Date: Wed Aug 12 14:00:54 2015 -0700

    Don't return Exception when volume is detached

    This patch changes the way we handle volume detach
    attempts when the attachment_id is already detached
    and/or when the volume has no attachments. We now
    handle this the same way we do with deleting volumes
    that don't exist. We return success.

    This patch also takes care to make sure we safely reset
    the volume status to what it should be depending on if
    there are other attachments. If the attachment_id is
    passed in and that attachment is detached, but there are
    other attachments, we want to make sure that the volume is
    left in an in-use state, not available.

    Change-Id: I11b7c45bb6570ce11e13e487cf1136ca2551036b
    Closes-Bug: #1484194
    Related-Bug: #1476806

Changed in cinder:
status: In Progress → Fix Committed
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to cinder (stable/kilo)

Fix proposed to branch: stable/kilo
Review: https://review.openstack.org/213761

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to cinder (stable/kilo)

Reviewed: https://review.openstack.org/213761
Committed: https://git.openstack.org/cgit/openstack/cinder/commit/?id=fd38f1d7b7a8d7f09b40dae1190d5a7330bd18d6
Submitter: Jenkins
Branch: stable/kilo

commit fd38f1d7b7a8d7f09b40dae1190d5a7330bd18d6
Author: Walter A. Boring IV <email address hidden>
Date: Wed Aug 12 14:00:54 2015 -0700

    Don't return Exception when volume is detached

    This patch changes the way we handle volume detach
    attempts when the attachment_id is already detached
    and/or when the volume has no attachments. We now
    handle this the same way we do with deleting volumes
    that don't exist. We return success.

    This patch also takes care to make sure we safely reset
    the volume status to what it should be depending on if
    there are other attachments. If the attachment_id is
    passed in and that attachment is detached, but there are
    other attachments, we want to make sure that the volume is
    left in an in-use state, not available.

    Conflicts:
            cinder/tests/test_volume.py
            cinder/volume/manager.py

    NOTE(mriedem): The conflicts are due to commit
    386e2858a4e76fafc1d78737154859a3c1618b20 not being in stable/kilo
    and I don't plan on backporting that since it's based on the level
    of oslo.log used in Liberty. This does mean the info messages in
    the detach_volume method in the volume manager are not exactly
    the same as what is in the original change in Liberty. Rather than
    using the resource=volume kwarg, we just log the volume_id like the
    error messages were before this change.

    Change-Id: I11b7c45bb6570ce11e13e487cf1136ca2551036b
    Closes-Bug: #1484194
    Related-Bug: #1476806
    (cherry picked from commit 3b4e8c31a2c94cac3e1a5c071eb38c7d5cb9a239)

tags: added: in-stable-kilo
Thierry Carrez (ttx)
Changed in cinder:
milestone: none → liberty-3
status: Fix Committed → Fix Released
Thierry Carrez (ttx)
Changed in cinder:
milestone: liberty-3 → 7.0.0
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.