cinder sends old db object when delete a attachment

[wu.chunyang]

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)

[Sofia Enriquez (lsofia-enriquez)]

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

[Gorka Eguileor (gorka)]

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.

[Gorka Eguileor (gorka)]

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.

[Gorka Eguileor (gorka)]

fix: https://review.opendev.org/c/openstack/cinder/+/778534

[Gorka Eguileor (gorka)]

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

[wu.chunyang (wuchunyang)]

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

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

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)

[OpenStack Infra (hudson-openstack)]

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)

[OpenStack Infra (hudson-openstack)]

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)

[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.