Merge "Check for multiattach before removing connections" into stable/queens

nova/tests/unit/virt/libvirt/test_driver.py

@@ -6745,6 +6745,84 @@ class LibvirtConnTestCase(test.NoDBTestCase,
         mock_encryptor.detach_volume.called_once_with(self.context,
                                                       **encryption)
 
+    @mock.patch.object(libvirt_driver.LibvirtDriver, '_detach_encryptor')
+    @mock.patch('nova.objects.InstanceList.get_uuids_by_host')
+    @mock.patch('nova.virt.libvirt.driver.LibvirtDriver._get_volume_driver')
+    @mock.patch('nova.volume.cinder.API.get')
+    def test_disconnect_multiattach_single_connection(
+            self, mock_volume_get, mock_get_volume_driver,
+            mock_get_instances, mock_detach_encryptor):
+
+        drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), True)
+        mock_volume_driver = mock.MagicMock(
+            spec=volume_drivers.LibvirtBaseVolumeDriver)
+        mock_get_volume_driver.return_value = mock_volume_driver
+
+        attachments = (
+            [('70ab645f-6ffc-406a-b3d2-5007a0c01b82',
+              {'mountpoint': u'/dev/vdb',
+               'attachment_id': u'9402c249-99df-4f72-89e7-fd611493ee5d'}),
+             ('00803490-f768-4049-aa7d-151f54e6311e',
+              {'mountpoint': u'/dev/vdb',
+               'attachment_id': u'd6128a7b-19c8-4a3e-8036-011396df95ac'})])
+
+        mock_volume_get.return_value = (
+            {'attachments': OrderedDict(attachments), 'multiattach': True,
+             'id': 'd30559cf-f092-4693-8589-0d0a1e7d9b1f'})
+
+        fake_connection_info = {
+            'multiattach': True,
+            'volume_id': 'd30559cf-f092-4693-8589-0d0a1e7d9b1f'}
+        fake_instance_1 = fake_instance.fake_instance_obj(
+            self.context,
+            host='fake-host-1')
+
+        mock_get_instances.return_value = (
+            ['00803490-f768-4049-aa7d-151f54e6311e'])
+        drvr._disconnect_volume(
+            self.context, fake_connection_info, fake_instance_1)
+        mock_volume_driver.disconnect_volume.assert_called_once_with(
+            fake_connection_info, fake_instance_1)
+
+    @mock.patch.object(libvirt_driver.LibvirtDriver, '_detach_encryptor')
+    @mock.patch('nova.objects.InstanceList.get_uuids_by_host')
+    @mock.patch('nova.virt.libvirt.driver.LibvirtDriver._get_volume_driver')
+    @mock.patch('nova.volume.cinder.API.get')
+    def test_disconnect_multiattach_multi_connection(
+            self, mock_volume_get, mock_get_volume_driver,
+            mock_get_instances, mock_detach_encryptor):
+
+        drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), True)
+        mock_volume_driver = mock.MagicMock(
+            spec=volume_drivers.LibvirtBaseVolumeDriver)
+        mock_get_volume_driver.return_value = mock_volume_driver
+
+        attachments = (
+            [('70ab645f-6ffc-406a-b3d2-5007a0c01b82',
+              {'mountpoint': u'/dev/vdb',
+               'attachment_id': u'9402c249-99df-4f72-89e7-fd611493ee5d'}),
+             ('00803490-f768-4049-aa7d-151f54e6311e',
+              {'mountpoint': u'/dev/vdb',
+               'attachment_id': u'd6128a7b-19c8-4a3e-8036-011396df95ac'})])
+
+        mock_volume_get.return_value = (
+            {'attachments': OrderedDict(attachments), 'multiattach': True,
+             'id': 'd30559cf-f092-4693-8589-0d0a1e7d9b1f'})
+
+        fake_connection_info = {
+            'multiattach': True,
+            'volume_id': 'd30559cf-f092-4693-8589-0d0a1e7d9b1f'}
+        fake_instance_1 = fake_instance.fake_instance_obj(
+            self.context,
+            host='fake-host-1')
+
+        mock_get_instances.return_value = (
+            ['00803490-f768-4049-aa7d-151f54e6311e',
+             '70ab645f-6ffc-406a-b3d2-5007a0c01b82'])
+        drvr._disconnect_volume(
+            self.context, fake_connection_info, fake_instance_1)
+        mock_volume_driver.disconnect_volume.assert_not_called()
+
     def test_attach_invalid_volume_type(self):
         self.create_fake_libvirt_mock()
         libvirt_driver.LibvirtDriver._conn.lookupByUUIDString \

nova/virt/libvirt/driver.py

@@ -1237,11 +1237,53 @@ class LibvirtDriver(driver.ComputeDriver):
         self._attach_encryptor(context, connection_info, encryption,
                                allow_native_luks)
 
+    def _should_disconnect_target(self, context, connection_info, instance):
+        connection_count = 0
+
+        # NOTE(jdg): Multiattach is a special case (not to be confused
+        # with shared_targets). With multiattach we may have a single volume
+        # attached multiple times to *this* compute node (ie Server-1 and
+        # Server-2).  So, if we receive a call to delete the attachment for
+        # Server-1 we need to take special care to make sure that the Volume
+        # isn't also attached to another Server on this Node.  Otherwise we
+        # will indiscriminantly delete the connection for all Server and that's
+        # no good.  So check if it's attached multiple times on this node
+        # if it is we skip the call to brick to delete the connection.
+        if connection_info.get('multiattach', False):
+            volume = self._volume_api.get(
+                context,
+                driver_block_device.get_volume_id(connection_info))
+            attachments = volume.get('attachments', {})
+            if len(attachments) > 1:
+                # First we get a list of all Server UUID's associated with
+                # this Host (Compute Node).  We're going to use this to
+                # determine if the Volume being detached is also in-use by
+                # another Server on this Host, ie just check to see if more
+                # than one attachment.server_id for this volume is in our
+                # list of Server UUID's for this Host
+                servers_this_host = objects.InstanceList.get_uuids_by_host(
+                    context, instance.host)
+
+                # NOTE(jdg): nova.volume.cinder translates the
+                # volume['attachments'] response into a dict which includes
+                # the Server UUID as the key, so we're using that
+                # here to check against our server_this_host list
+                for server_id, data in attachments.items():
+                    if server_id in servers_this_host:
+                        connection_count += 1
+        return (False if connection_count > 1 else True)
+
     def _disconnect_volume(self, context, connection_info, instance,
                            encryption=None):
         self._detach_encryptor(context, connection_info, encryption=encryption)
-        vol_driver = self._get_volume_driver(connection_info)
-        vol_driver.disconnect_volume(connection_info, instance)
+        if self._should_disconnect_target(context, connection_info, instance):
+            vol_driver = self._get_volume_driver(connection_info)
+            vol_driver.disconnect_volume(connection_info, instance)
+        else:
+            LOG.info("Detected multiple connections on this host for volume: "
+                     "%s, skipping target disconnect.",
+                     driver_block_device.get_volume_id(connection_info),
+                     instance=instance)
 
     def _extend_volume(self, connection_info, instance):
         vol_driver = self._get_volume_driver(connection_info)