Pass mountpoint to volume attachment_create with connector

Similar to I11ba269c3f7a2e7707b2b7e27d26eb7a2c948a82, when
we create a volume attachment with a connector, we need to
also provide the mountpoint via the connector to Cinder,
because internally within Cinder the attachment_create code
is calling attachment_update if a connector is provided.

Change-Id: If3afe8d8bd6b8c327ccc7d1140053bccaf7e1ad7
Closes-Bug: #1737779

nova/compute/manager.py

@@ -5844,7 +5844,7 @@ class ComputeManager(manager.Manager):
                     # are providing the connector in the create call.
                     attach_ref = self.volume_api.attachment_create(
                         context, bdm.volume_id, bdm.instance_uuid,
-                        connector=connector)
+                        connector=connector, mountpoint=bdm.device_name)
 
                     # save current attachment so we can detach it on success,
                     # or restore it on a rollback.

nova/tests/fixtures.py

@@ -1575,7 +1575,7 @@ class CinderFixtureNewAttachFlow(fixtures.Fixture):
             return {'save_volume_id': new_volume_id}
 
         def fake_attachment_create(_self, context, volume_id, instance_uuid,
-                                   connector=None):
+                                   connector=None, mountpoint=None):
             attachment_id = uuidutils.generate_uuid()
             if self.attachment_error_id is not None:
                 attachment_id = self.attachment_error_id

nova/tests/unit/compute/test_compute_mgr.py

@@ -6511,7 +6511,8 @@ class ComputeManagerMigrationTestCase(test.NoDBTestCase):
             self.assertIsInstance(mock_plm.call_args_list[0][0][5],
                                   migrate_data_obj.LiveMigrateData)
             mock_attach.assert_called_once_with(
-                self.context, volume_id, instance.uuid, connector=connector)
+                self.context, volume_id, instance.uuid, connector=connector,
+                mountpoint=vol_bdm.device_name)
             self.assertEqual(vol_bdm.attachment_id, new_attachment_id)
             self.assertEqual(migrate_data.old_vol_attachment_ids[volume_id],
                              orig_attachment_id)

nova/tests/unit/volume/test_cinder.py

@@ -300,7 +300,9 @@ class CinderApiTestCase(test.NoDBTestCase):
 
     @mock.patch('nova.volume.cinder.cinderclient')
     def test_attachment_create(self, mock_cinderclient):
-        """Tests the happy path for creating a volume attachment."""
+        """Tests the happy path for creating a volume attachment without a
+        mountpoint.
+        """
         attachment_ref = {'id': uuids.attachment_id,
                           'connection_info': {}}
         expected_attachment_ref = {'id': uuids.attachment_id,
@@ -313,6 +315,30 @@ class CinderApiTestCase(test.NoDBTestCase):
         mock_cinderclient.return_value.attachments.create.\
             assert_called_once_with(uuids.volume_id, None, uuids.instance_id)
 
+    @mock.patch('nova.volume.cinder.cinderclient')
+    def test_attachment_create_with_mountpoint(self, mock_cinderclient):
+        """Tests the happy path for creating a volume attachment with a
+        mountpoint.
+        """
+        attachment_ref = {'id': uuids.attachment_id,
+                          'connection_info': {}}
+        expected_attachment_ref = {'id': uuids.attachment_id,
+                                   'connection_info': {}}
+        mock_cinderclient.return_value.attachments.create.return_value = (
+            attachment_ref)
+        original_connector = {'host': 'fake-host'}
+        updated_connector = dict(original_connector, mountpoint='/dev/vdb')
+        result = self.api.attachment_create(
+            self.ctx, uuids.volume_id, uuids.instance_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.create.\
+            assert_called_once_with(uuids.volume_id, updated_connector,
+                                    uuids.instance_id)
+
     @mock.patch('nova.volume.cinder.cinderclient')
     def test_attachment_create_volume_not_found(self, mock_cinderclient):
         """Tests that the translate_volume_exception decorator is used."""

nova/volume/cinder.py

@@ -558,7 +558,7 @@ class API(object):
 
     @translate_volume_exception
     def attachment_create(self, context, volume_id, instance_id,
-                          connector=None):
+                          connector=None, mountpoint=None):
         """Create a volume attachment. This requires microversion >= 3.44.
 
         The attachment_create call was introduced in microversion 3.27. We
@@ -571,13 +571,28 @@ class API(object):
             attached.
         :param connector: host connector dict; if None, the attachment will
             be 'reserved' but not yet attached.
+        :param mountpoint: Optional mount device name for the attachment,
+            e.g. "/dev/vdb". This is only used if a connector is provided.
         :returns: a dict created from the
             cinderclient.v3.attachments.VolumeAttachment object with a backward
             compatible connection_info dict
         """
+        # NOTE(mriedem): Due to a limitation in the POST /attachments/
+        # API in Cinder, we have to pass the mountpoint in via the
+        # host connector rather than pass it in as a top-level parameter
+        # like in the os-attach volume action API. Hopefully this will be
+        # fixed some day with a new Cinder microversion but until then we
+        # work around it client-side.
+        _connector = connector
+        if _connector and mountpoint and 'mountpoint' not in _connector:
+            # Make a copy of the connector so we don't modify it by
+            # reference.
+            _connector = copy.deepcopy(connector)
+            _connector['mountpoint'] = mountpoint
+
         try:
             attachment_ref = cinderclient(context, '3.44').attachments.create(
-                volume_id, connector, instance_id)
+                volume_id, _connector, instance_id)
             return _translate_attachment_ref(attachment_ref)
         except cinder_exception.ClientException as ex:
             with excutils.save_and_reraise_exception():