cinder sends old db object when delete a attachment

Bug #1916980 reported by wu.chunyang
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Cinder
Fix Released
High
Gorka Eguileor

Bug Description

when we delete an attachment. cinder sends a notification with old db object. it looks like:

Volume(_name_id=None,admin_metadata={},attach_status='attached',availability_zone='nova',bootable=True,cluster=<?>,cluster_name=None,consistencygroup=<?>,consistencygroup_id=None,created_at=2021-02-26T03:10:10Z,deleted=False,deleted_at=None,display_description='',display_name='',ec2_id=None,encryption_key_id=None,glance_metadata=<?>,group=<?>,group_id=None,host='bj-openstack@lvm-1#lvm-1',id=c98d214e-c11f-409d-982f-6d837d71fc20,launched_at=2021-02-26T03:10:14Z,metadata={},migration_status=None,multiattach=False,previous_status=None,project_id='b1cfcab46a144088863f3d9f73f0aeb0',provider_auth='CHAP xyr4vY3v2aNeZQ42Z8Nt Mx5Xcm5LgX4ufhoW',provider_geometry=None,provider_id=None,provider_location='172.16.0.8:3260,1 iqn.2010-10.org.openstack:volume-c98d214e-c11f-409d-982f-6d837d71fc20 1',replication_driver_data=None,replication_extended_status=None,replication_status=None,scheduled_at=2021-02-26T03:10:10Z,service_uuid='bf7b0e2d-3687-4bc6-8c6b-5862664301ef',shared_targets=False,size=1,snapshot_id=None,snapshots=<?>,source_volid=None,status='in-use',terminated_at=None,updated_at=2021-02-26T03:10:18Z,user_id='3bbb57fa55d64b98893894ccc1855640',volume_attachment=VolumeAttachmentList,volume_type=VolumeType(9494ec26-1f36-4011-8c4f-c651ccb636b5),volume_type_id=9494ec26-1f36-4011-8c4f-c651ccb636b5)

we should call refresh() function to refresh the object before sending to notification. after running the refresh function. the status changes to detached. and the refreshed db object is following:

Volume(_name_id=None,admin_metadata={},attach_status='detached',availability_zone='nova',bootable=True,cluster=<?>,cluster_name=None,consistencygroup=<?>,consistencygroup_id=None,created_at=2021-02-26T03:10:10Z,deleted=False,deleted_at=None,display_description='',display_name='',ec2_id=None,encryption_key_id=None,glance_metadata=<?>,group=<?>,group_id=None,host='bj-openstack@lvm-1#lvm-1',id=c98d214e-c11f-409d-982f-6d837d71fc20,launched_at=2021-02-26T03:10:14Z,metadata={},migration_status=None,multiattach=False,previous_status=None,project_id='b1cfcab46a144088863f3d9f73f0aeb0',provider_auth='CHAP xyr4vY3v2aNeZQ42Z8Nt Mx5Xcm5LgX4ufhoW',provider_geometry=None,provider_id=None,provider_location='172.16.0.8:3260,1 iqn.2010-10.org.openstack:volume-c98d214e-c11f-409d-982f-6d837d71fc20 1',replication_driver_data=None,replication_extended_status=None,replication_status=None,scheduled_at=2021-02-26T03:10:10Z,service_uuid='bf7b0e2d-3687-4bc6-8c6b-5862664301ef',shared_targets=False,size=1,snapshot_id=None,snapshots=<?>,source_volid=None,status='available',terminated_at=None,updated_at=2021-02-26T03:10:18Z,user_id='3bbb57fa55d64b98893894ccc1855640',volume_attachment=<?>,volume_type=VolumeType(9494ec26-1f36-4011-8c4f-c651ccb636b5),volume_type_id=9494ec26-1f36-4011-8c4f-c651ccb636b5)

wu.chunyang (wuchunyang)
Changed in cinder:
assignee: nobody → wu.chunyang (wuchunyang)
Revision history for this message
Sofia Enriquez (lsofia-enriquez) wrote :

Hi wu.chunyang, Does this problem occur on all backends or on a specific one?

tags: added: attachment
tags: added: attach
tags: added: volume
Changed in cinder:
status: New → Incomplete
Revision history for this message
Gorka Eguileor (gorka) wrote :

I'm looking at the code and there seem to be 2 bugs in that part of the code, both of them will happen with all drivers. These issues happen in `cinder.volume.api.API.attachment_delete`

The notification issue happens for two reasons:

- We are mixing OVO and DB calls, so when we use DB methods directly they don't know about the existence of the OVO so its fields don't get updated. The proper solution is to fix that OVO and DB calls mix and make everything OVO calls.
- We are calling the notification too soon, since we are also changing the volume's status after the notification has been sent.

The other issue I see in the code is that we are calling DB method `volume_detached` which modifies the volume object itself, and API method `attachment_delete` then goes and modifies it as well ignoring any changes that the DB method may have done. Once again the solution is is only using OVO methods that update the reference and also taking into account changes made by `volume_detached` DB method.

Revision history for this message
Gorka Eguileor (gorka) wrote :

Another part of the code that is wrong is cinder.volume.manager.VolumeManager._do_attachment_delete where we have the same issue, we call DB methods instead of OVO methods and the pass the OVO to the notification method.

Revision history for this message
Gorka Eguileor (gorka) wrote :
Revision history for this message
Gorka Eguileor (gorka) wrote :

fix from wu.chunyang doing a refresh: https://review.opendev.org/c/openstack/cinder/+/723179

Revision history for this message
wu.chunyang (wuchunyang) wrote :

thanks gorka for fixing this bug. i will abandon my change.

Changed in cinder:
assignee: wu.chunyang (wuchunyang) → nobody
Changed in cinder:
status: Incomplete → In Progress
importance: Undecided → High
assignee: nobody → Gorka Eguileor (gorka)
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to cinder (master)

Reviewed: https://review.opendev.org/c/openstack/cinder/+/778534
Committed: https://opendev.org/openstack/cinder/commit/68d49445778ef486e3ff656929405ab270a5a65d
Submitter: "Zuul (22348)"
Branch: master

commit 68d49445778ef486e3ff656929405ab270a5a65d
Author: Gorka Eguileor <email address hidden>
Date: Fri Jul 2 15:11:53 2021 +0200

    Fix detach notification

    Our current `attachment_delete` methods in the volume API and the
    manager are using DB methods directly, which makes the OVOs present in
    those methods get out of sync with the latest data, which leads to
    notifications having the wrong data when we send them on volume detach.

    This patch replaces DB method calls with OVO calls and moves the
    notification call to the end of the method, where we have the final
    status on the volume.

    It also adds the missing detach.start notification when deleting an
    attachment in the reserved state.

    Closes-Bug: #1916980
    Closes-Bug: #1935011
    Change-Id: Ie48cf55deacd08e7716201dac00ede8d57e6632f

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

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

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

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

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

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

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

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

commit c0197c6f1ad13e3a63615ceb667a53d3bbca2d77
Author: Gorka Eguileor <email address hidden>
Date: Fri Jul 2 15:11:53 2021 +0200

    Fix detach notification

    Our current `attachment_delete` methods in the volume API and the
    manager are using DB methods directly, which makes the OVOs present in
    those methods get out of sync with the latest data, which leads to
    notifications having the wrong data when we send them on volume detach.

    This patch replaces DB method calls with OVO calls and moves the
    notification call to the end of the method, where we have the final
    status on the volume.

    It also adds the missing detach.start notification when deleting an
    attachment in the reserved state.

    Closes-Bug: #1916980
    Closes-Bug: #1935011
    Change-Id: Ie48cf55deacd08e7716201dac00ede8d57e6632f
    (cherry picked from commit 68d49445778ef486e3ff656929405ab270a5a65d)

tags: added: in-stable-xena
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to cinder (stable/wallaby)

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

commit ed06fc7452699d76c22b288e1cc8e8bc2afa1849
Author: Gorka Eguileor <email address hidden>
Date: Fri Jul 2 15:11:53 2021 +0200

    Fix detach notification

    Our current `attachment_delete` methods in the volume API and the
    manager are using DB methods directly, which makes the OVOs present in
    those methods get out of sync with the latest data, which leads to
    notifications having the wrong data when we send them on volume detach.

    This patch replaces DB method calls with OVO calls and moves the
    notification call to the end of the method, where we have the final
    status on the volume.

    It also adds the missing detach.start notification when deleting an
    attachment in the reserved state.

    Closes-Bug: #1916980
    Closes-Bug: #1935011
    Change-Id: Ie48cf55deacd08e7716201dac00ede8d57e6632f
    (cherry picked from commit 68d49445778ef486e3ff656929405ab270a5a65d)
    Conflicts:
            cinder/volume/api.py
    Changes:
            cinder/volume/manager.py
    (cherry picked from commit c0197c6f1ad13e3a63615ceb667a53d3bbca2d77)

tags: added: in-stable-wallaby
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to cinder (stable/victoria)

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

commit c9d3cc966cc4856b9fcc8625ce3ce86061df1d87
Author: Gorka Eguileor <email address hidden>
Date: Fri Jul 2 15:11:53 2021 +0200

    Fix detach notification

    Our current `attachment_delete` methods in the volume API and the
    manager are using DB methods directly, which makes the OVOs present in
    those methods get out of sync with the latest data, which leads to
    notifications having the wrong data when we send them on volume detach.

    This patch replaces DB method calls with OVO calls and moves the
    notification call to the end of the method, where we have the final
    status on the volume.

    It also adds the missing detach.start notification when deleting an
    attachment in the reserved state.

    Closes-Bug: #1916980
    Closes-Bug: #1935011
    Change-Id: Ie48cf55deacd08e7716201dac00ede8d57e6632f
    (cherry picked from commit 68d49445778ef486e3ff656929405ab270a5a65d)
    Conflicts:
            cinder/volume/api.py
    Changes:
            cinder/volume/manager.py
    (cherry picked from commit c0197c6f1ad13e3a63615ceb667a53d3bbca2d77)
    (cherry picked from commit ed06fc7452699d76c22b288e1cc8e8bc2afa1849)

tags: added: in-stable-victoria
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix included in openstack/cinder 19.1.0

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

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix included in openstack/cinder 18.2.0

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

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix included in openstack/cinder 17.3.0

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

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix included in openstack/cinder 20.0.0.0rc1

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

To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers