Delete attachment doesn't fail on remove_export errors

[Gorka Eguileor]

When deleting an attachment, if the remove_export method call fails in the cinder driver, then the attachment status is changed to error_detaching but the REST API call doesn't fail.

The end result is:
- Volume status is "available"
- Volume attach_status is "detached"
- There is a volume_attachment record for the volume
- The volume may still be exported in the backend

Cinder's REST API call should fail on remove export failures, like the old API did.

[Gorka Eguileor (gorka)]

In last week's cinder meeting we agreed to not fail and just delete the volume_attachment record from the DB instead.

While the volume exists in the system any new attach-detach cycle will be a new opportunity for the remove_export method to succeed, and the delete operation is already ensuring that the remove_export is called.

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: master
Review: https://review.opendev.org/c/openstack/cinder/+/801912

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.opendev.org/c/openstack/cinder/+/801912
Committed: https://opendev.org/openstack/cinder/commit/3aa00b087884a278e747e839db0707cf39561382
Submitter: "Zuul (22348)"
Branch: master

commit 3aa00b087884a278e747e839db0707cf39561382
Author: Gorka Eguileor <email address hidden>
Date: Tue Jul 20 17:15:54 2021 +0200

    Delete attachment on remove_export failure

    When deleting an attachment, if the driver's remove_export or the
    detach_volume method call fails in the cinder driver, then the
    attachment status is changed to error_detaching but the REST API call
    doesn't fail.

    The end result is:
    - Volume status is "available"
    - Volume attach_status is "detached"
    - There is a volume_attachment record for the volume
    - The volume may still be exported in the backend

    The volume still being exported in the storage array is not a problem,
    since the next attach-detach cycle will give it another opportunity for
    it to succeed, and we also do the export on volume deletion.

    So in the end leaving the attachment in error_detaching status doesn't
    have any use and creates confusion.

    This patch removes the attachment record when on an attachment delete
    request if the error happens on remove_export or detach_volume calls.

    This doesn't change how the REST API attachment delete operation
    behaves, the change is that there will not be a leftover attachment
    record with the volume in available and detached status.

    Closes-Bug: #1935057
    Change-Id: I442a42b0c098775935a799876ad8efbe141829ad

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: stable/wallaby
Review: https://review.opendev.org/c/openstack/cinder/+/818884

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: stable/xena
Review: https://review.opendev.org/c/openstack/cinder/+/824977

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: stable/victoria
Review: https://review.opendev.org/c/openstack/cinder/+/825101

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.opendev.org/c/openstack/cinder/+/824977
Committed: https://opendev.org/openstack/cinder/commit/1328c68a8cc6db0e4bd6b145daa788570908ed36
Submitter: "Zuul (22348)"
Branch: stable/xena

commit 1328c68a8cc6db0e4bd6b145daa788570908ed36
Author: Gorka Eguileor <email address hidden>
Date: Tue Jul 20 17:15:54 2021 +0200

    Delete attachment on remove_export failure

    When deleting an attachment, if the driver's remove_export or the
    detach_volume method call fails in the cinder driver, then the
    attachment status is changed to error_detaching but the REST API call
    doesn't fail.

    The end result is:
    - Volume status is "available"
    - Volume attach_status is "detached"
    - There is a volume_attachment record for the volume
    - The volume may still be exported in the backend

    The volume still being exported in the storage array is not a problem,
    since the next attach-detach cycle will give it another opportunity for
    it to succeed, and we also do the export on volume deletion.

    So in the end leaving the attachment in error_detaching status doesn't
    have any use and creates confusion.

    This patch removes the attachment record when on an attachment delete
    request if the error happens on remove_export or detach_volume calls.

    This doesn't change how the REST API attachment delete operation
    behaves, the change is that there will not be a leftover attachment
    record with the volume in available and detached status.

    Closes-Bug: #1935057
    Change-Id: I442a42b0c098775935a799876ad8efbe141829ad
    (cherry picked from commit 3aa00b087884a278e747e839db0707cf39561382)

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.opendev.org/c/openstack/cinder/+/818884
Committed: https://opendev.org/openstack/cinder/commit/be7b001290ab56632aaf5d108aeccb367adea55c
Submitter: "Zuul (22348)"
Branch: stable/wallaby

commit be7b001290ab56632aaf5d108aeccb367adea55c
Author: Gorka Eguileor <email address hidden>
Date: Tue Jul 20 17:15:54 2021 +0200

    Delete attachment on remove_export failure

    When deleting an attachment, if the driver's remove_export or the
    detach_volume method call fails in the cinder driver, then the
    attachment status is changed to error_detaching but the REST API call
    doesn't fail.

    The end result is:
    - Volume status is "available"
    - Volume attach_status is "detached"
    - There is a volume_attachment record for the volume
    - The volume may still be exported in the backend

    The volume still being exported in the storage array is not a problem,
    since the next attach-detach cycle will give it another opportunity for
    it to succeed, and we also do the export on volume deletion.

    So in the end leaving the attachment in error_detaching status doesn't
    have any use and creates confusion.

    This patch removes the attachment record when on an attachment delete
    request if the error happens on remove_export or detach_volume calls.

    This doesn't change how the REST API attachment delete operation
    behaves, the change is that there will not be a leftover attachment
    record with the volume in available and detached status.

    Closes-Bug: #1935057
    Change-Id: I442a42b0c098775935a799876ad8efbe141829ad
    (cherry picked from commit 3aa00b087884a278e747e839db0707cf39561382)
    Conflicts:
            cinder/volume/manager.py
    (cherry picked from commit 1328c68a8cc6db0e4bd6b145daa788570908ed36)

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.opendev.org/c/openstack/cinder/+/825101
Committed: https://opendev.org/openstack/cinder/commit/362b7e14f17d9b0aa751df3016b27c536e1c0bc8
Submitter: "Zuul (22348)"
Branch: stable/victoria

commit 362b7e14f17d9b0aa751df3016b27c536e1c0bc8
Author: Gorka Eguileor <email address hidden>
Date: Tue Jul 20 17:15:54 2021 +0200

    Delete attachment on remove_export failure

    When deleting an attachment, if the driver's remove_export or the
    detach_volume method call fails in the cinder driver, then the
    attachment status is changed to error_detaching but the REST API call
    doesn't fail.

    The end result is:
    - Volume status is "available"
    - Volume attach_status is "detached"
    - There is a volume_attachment record for the volume
    - The volume may still be exported in the backend

    The volume still being exported in the storage array is not a problem,
    since the next attach-detach cycle will give it another opportunity for
    it to succeed, and we also do the export on volume deletion.

    So in the end leaving the attachment in error_detaching status doesn't
    have any use and creates confusion.

    This patch removes the attachment record when on an attachment delete
    request if the error happens on remove_export or detach_volume calls.

    This doesn't change how the REST API attachment delete operation
    behaves, the change is that there will not be a leftover attachment
    record with the volume in available and detached status.

    Closes-Bug: #1935057
    Change-Id: I442a42b0c098775935a799876ad8efbe141829ad
    (cherry picked from commit 3aa00b087884a278e747e839db0707cf39561382)
    Conflicts:
            cinder/volume/manager.py
    (cherry picked from commit 1328c68a8cc6db0e4bd6b145daa788570908ed36)
    (cherry picked from commit be7b001290ab56632aaf5d108aeccb367adea55c)

[OpenStack Infra (hudson-openstack)]

This issue was fixed in the openstack/cinder 19.1.0 release.

[OpenStack Infra (hudson-openstack)]

This issue was fixed in the openstack/cinder 18.2.0 release.

[OpenStack Infra (hudson-openstack)]

This issue was fixed in the openstack/cinder 17.3.0 release.

[OpenStack Infra (hudson-openstack)]

This issue was fixed in the openstack/cinder 20.0.0.0rc1 release candidate.