From 283e1c2194b9fda1a6116df29b0644f449a69a76 Mon Sep 17 00:00:00 2001
From: Yikun Jiang <yikunkero@gmail.com>
Date: Tue, 18 Dec 2018 17:21:29 +0800
Subject: [PATCH] Move 'attach.end' notify to attachment_complete

Since microversion 3.44, we update `attachemnt_update` to return
connector and make volume to 'attaching', and add a new api
`attachment_complete` to change volume status to 'in-use', that means
the end of attachment is changed to `attachment_complete`.

But the 'attach.end' notify is still send in attachment_update, which
is unexpected. So, in this patch we move 'attach.end' notify to
`attachment_complete`.

Change-Id: I86dad7afb52d2d253799b543f484e2d55f6e9e6d
Closes-bug: #1808941
---
 cinder/api/v3/attachments.py                 |  3 +++
 cinder/tests/unit/api/v3/test_attachments.py | 22 ++++++++++++++++++++
 cinder/volume/manager.py                     |  1 -
 3 files changed, 25 insertions(+), 1 deletion(-)

diff --git a/cinder/api/v3/attachments.py b/cinder/api/v3/attachments.py
index 0a209176f4e..d17a5ce4b40 100644
--- a/cinder/api/v3/attachments.py
+++ b/cinder/api/v3/attachments.py
@@ -28,6 +28,7 @@ from cinder.objects import fields
 from cinder.policies import attachments as attachment_policy
 from cinder import utils
 from cinder.volume import api as volume_api
+from cinder.volume import utils as volume_utils
 
 
 LOG = logging.getLogger(__name__)
@@ -286,6 +287,8 @@ class AttachmentsController(wsgi.Controller):
         attachment_ref.save()
         volume_ref.update({'status': 'in-use', 'attach_status': 'attached'})
         volume_ref.save()
+        volume_utils.notify_about_volume_usage(context, volume_ref,
+                                               "attach.end")
 
 
 def create_resource(ext_mgr):
diff --git a/cinder/tests/unit/api/v3/test_attachments.py b/cinder/tests/unit/api/v3/test_attachments.py
index 8d38e1c56ce..9ffc5f98cb5 100644
--- a/cinder/tests/unit/api/v3/test_attachments.py
+++ b/cinder/tests/unit/api/v3/test_attachments.py
@@ -30,6 +30,7 @@ from cinder.tests.unit.api import fakes
 from cinder.tests.unit import fake_constants as fake
 from cinder.volume import api as volume_api
 from cinder.volume import rpcapi as volume_rpcapi
+from cinder.volume import utils as volume_utils
 
 
 @ddt.ddt
@@ -341,3 +342,24 @@ class AttachmentsAPITestCase(test.TestCase):
 
         self.assertEqual(1, len(res_dict))
         self.assertEqual(count, len(res_dict['attachments']))
+
+    @mock.patch.object(volume_utils, 'notify_about_volume_usage')
+    def test_complete_attachment(self, mock_notify):
+
+        def fake_notify(context, volume, event_suffix,
+                        extra_usage_info=None, host=None):
+            # Check the notify content is in-use volume and 'attach.end'
+            self.assertEqual('in-use', volume['status'])
+            self.assertEqual('attach.end', event_suffix)
+
+        mock_notify.side_effect = fake_notify
+        req = fakes.HTTPRequest.blank('/v3/%s/attachments/%s/action' %
+                                      (fake.PROJECT_ID, self.attachment1.id),
+                                      version=mv.NEW_ATTACH_COMPLETION,
+                                      use_admin_context=True)
+        body = {"os-complete": {}}
+
+        self.controller.complete(req, self.attachment1.id,
+                                 body=body)
+        # Check notify has been called once
+        mock_notify.assert_called_once()
diff --git a/cinder/volume/manager.py b/cinder/volume/manager.py
index 5e4ba4f087a..89aafc71c56 100644
--- a/cinder/volume/manager.py
+++ b/cinder/volume/manager.py
@@ -4450,7 +4450,6 @@ class VolumeManager(manager.CleanableManager,
                                 False)
         vref.refresh()
         attachment_ref.refresh()
-        self._notify_about_volume_usage(context, vref, "attach.end")
         LOG.info("attachment_update completed successfully.",
                  resource=vref)
         return connection_info