Merge "Fix detach notification"

cinder/tests/unit/attachments/test_attachments_manager.py

@@ -166,6 +166,12 @@ class AttachmentManagerTestCase(test.TestCase):
                   mock_db_detached, mock_db_meta_delete, mock_get_attachment):
             mock_elevated.return_value = self.context
             mock_con_term.return_value = False
+            mock_db_detached.return_value = (
+                {'status': 'available',
+                 'attach_status': fields.VolumeAttachStatus.DETACHED},
+                {'attach_status': fields.VolumeAttachStatus.DETACHED,
+                 'deleted': True}
+            )
 
             # test single attachment. This should call
             # detach and remove_export

cinder/volume/api.py

@@ -2259,17 +2259,17 @@ class API(base.Base):
         ctxt.authorize(attachment_policy.DELETE_POLICY,
                        target_obj=attachment)
         volume = attachment.volume
+
         if attachment.attach_status == fields.VolumeAttachStatus.RESERVED:
-            self.db.volume_detached(ctxt.elevated(), attachment.volume_id,
+            volume_utils.notify_about_volume_usage(ctxt, volume,
-                                    attachment.get('id'))
+                                                   "detach.start")
-            self.db.volume_admin_metadata_delete(ctxt.elevated(),
+            volume.finish_detach(attachment.id)
-                                                 attachment.volume_id,
+            do_notify = True
-                                                 'attached_mode')
-            volume_utils.notify_about_volume_usage(ctxt, volume, "detach.end")
         else:
             self.volume_rpcapi.attachment_delete(ctxt,
                                                  attachment.id,
                                                  volume)
+            do_notify = False
         status_updates = {'status': 'available',
                           'attach_status': 'detached'}
         remaining_attachments = AO_LIST.get_all_by_volume_id(ctxt, volume.id)
@@ -2305,6 +2305,8 @@ class API(base.Base):
         volume.attach_status = status_updates['attach_status']
         volume.save()
 
+        if do_notify:
+            volume_utils.notify_about_volume_usage(ctxt, volume, "detach.end")
         return remaining_attachments
 
 

cinder/volume/manager.py

@@ -4953,15 +4953,11 @@ class VolumeManager(manager.CleanableManager,
             # represent a single failed attach out of multiple attachments
 
             # TODO(jdg): object method here
-            self.db.volume_attachment_update(
+            attachment.attach_status = \
-                context, attachment.get('id'),
+                fields.VolumeAttachStatus.ERROR_DETACHING
-                {'attach_status': fields.VolumeAttachStatus.ERROR_DETACHING})
+            attachment.save()
         else:
-            self.db.volume_detached(context.elevated(), vref.id,
+            vref.finish_detach(attachment_id)
-                                    attachment.get('id'))
-            self.db.volume_admin_metadata_delete(context.elevated(),
-                                                 vref.id,
-                                                 'attached_mode')
         self._notify_about_volume_usage(context, vref, "detach.end")
 
     # Replication group API (Tiramisu)

releasenotes/notes/detach-notification-31ae15dafdef36c1.yaml

@@ -0,0 +1,9 @@
+---
+fixes:
+  - |
+    `Bug #1916980 <https://bugs.launchpad.net/cinder/+bug/1916980>`_: Fixed
+    stale volume notification information on volume detach.
+  - |
+    `Bug #1935011 <https://bugs.launchpad.net/cinder/+bug/1935011>`_: Fixed
+    missing detach.start notification when deleting an attachment in reserved
+    state.