Pass mountpoint to volume attachment_update

The old volume attach flow would pass the mountpoint,
which is the BlockDeviceMapping.device_name, to the
os-attach volume action API. With the new flow, apparently
the mountpoint is expected to be passed to the update
attachment API via the connector dict, which is not obvious
and if not provided, causes Cinder to default the mountpoint
to "/dev/na" which is wrong.

We work around this in Nova by providing the mountpoint in a
copy of the connector dict and pass that to attachment_update,
and update the two places in the code that are updating an
attachment (the normal driver block device attach code and
swap volume). Long-term this should be fixed in the Cinder
attachments update API, but that requires a microversion so
we need to handle it client-side for now.

Change-Id: I11ba269c3f7a2e7707b2b7e27d26eb7a2c948a82
Partial-Bug: #1737779

1 files changed, 43 insertions, 3 deletions

diff --git a/nova/tests/unit/volume/test_cinder.py b/nova/tests/unit/volume/test_cinder.py
index 8131eb01e5..6d0e6fa5f6 100644
--- a/nova/tests/unit/volume/test_cinder.py
+++ b/nova/tests/unit/volume/test_cinder.py
@@ -337,15 +337,49 @@ class CinderApiTestCase(test.NoDBTestCase):
 
     @mock.patch('nova.volume.cinder.cinderclient')
     def test_attachment_update(self, mock_cinderclient):
-        """Tests the happy path for updating a volume attachment."""
+        """Tests the happy path for updating a volume attachment without
+        a mountpoint.
+        """
+        fake_attachment = FakeAttachment()
+        connector = {'host': 'fake-host'}
+        expected_attachment_ref = {
+             'id': uuids.attachment_id,
+             'volume_id': fake_attachment.volume_id,
+             'connection_info': {
+                 'attach_mode': 'rw',
+                 'attached_at': fake_attachment.attached_at,
+                 'connector': connector,
+                 'data': {'foo': 'bar', 'target_lun': '1'},
+                 'detached_at': None,
+                 'driver_volume_type': 'fake_type',
+                 'instance': fake_attachment.instance,
+                 'status': 'attaching',
+                 'volume_id': fake_attachment.volume_id}}
+        mock_cinderclient.return_value.attachments.update.return_value = (
+            fake_attachment)
+        result = self.api.attachment_update(
+            self.ctx, uuids.attachment_id, connector=connector)
+        self.assertEqual(expected_attachment_ref, result)
+        # Make sure the connector wasn't modified.
+        self.assertNotIn('mountpoint', connector)
+        mock_cinderclient.return_value.attachments.update.\
+            assert_called_once_with(uuids.attachment_id, connector)
+
+    @mock.patch('nova.volume.cinder.cinderclient')
+    def test_attachment_update_with_mountpoint(self, mock_cinderclient):
+        """Tests the happy path for updating a volume attachment with
+        a mountpoint.
+        """
         fake_attachment = FakeAttachment()
+        original_connector = {'host': 'fake-host'}
+        updated_connector = dict(original_connector, mountpoint='/dev/vdb')
         expected_attachment_ref = {
              'id': uuids.attachment_id,
              'volume_id': fake_attachment.volume_id,
              'connection_info': {
                  'attach_mode': 'rw',
                  'attached_at': fake_attachment.attached_at,
-                 'connector': {'host': 'fake-host'},
+                 'connector': updated_connector,
                  'data': {'foo': 'bar', 'target_lun': '1'},
                  'detached_at': None,
                  'driver_volume_type': 'fake_type',
@@ -355,8 +389,14 @@ class CinderApiTestCase(test.NoDBTestCase):
         mock_cinderclient.return_value.attachments.update.return_value = (
             fake_attachment)
         result = self.api.attachment_update(
-            self.ctx, uuids.attachment_id, connector={'host': 'fake-host'})
+            self.ctx, uuids.attachment_id, connector=original_connector,
+            mountpoint='/dev/vdb')
         self.assertEqual(expected_attachment_ref, result)
+        # Make sure the original connector wasn't modified.
+        self.assertNotIn('mountpoint', original_connector)
+        # Make sure the mountpoint was passed through via the connector.
+        mock_cinderclient.return_value.attachments.update.\
+            assert_called_once_with(uuids.attachment_id, updated_connector)
 
     @mock.patch('nova.volume.cinder.cinderclient')
     def test_attachment_update_attachment_not_found(self, mock_cinderclient):