Merge "Disconnecting volume from the compute host" into stable/2023.1

Author: Zuul

doc/source/cli/nova-manage.rst

@@ -1531,7 +1531,9 @@ command.
    * - 5
      - Instance state invalid (must be stopped and unlocked)
    * - 6
-     - Instance is not attached to volume
+     - Volume is not attached to the instance
+   * - 7
+     - Connector host is not correct
 
 
 Libvirt Commands

nova/cmd/manage.py

@@ -161,18 +161,14 @@ def locked_instance(cell_mapping, instance, reason):
     initial_state = 'locked' if instance.locked else 'unlocked'
     if not instance.locked:
         with context.target_cell(
-                    context.get_admin_context(),
-                    cell_mapping
-                ) as cctxt:
+                context.get_admin_context(), cell_mapping) as cctxt:
             compute_api.lock(cctxt, instance, reason=reason)
     try:
         yield
     finally:
         if initial_state == 'unlocked':
             with context.target_cell(
-                    context.get_admin_context(),
-                    cell_mapping
-                ) as cctxt:
+                    context.get_admin_context(), cell_mapping) as cctxt:
                 compute_api.unlock(cctxt, instance)
 
 
@@ -3078,8 +3074,15 @@ def _do_refresh(self, cctxt, instance,
             # TODO(lyarwood): Add delete_attachment as a kwarg to
             # remove_volume_connection as is available in the private
             # method within the manager.
-            compute_rpcapi.remove_volume_connection(
-                cctxt, instance, volume_id, instance.host)
+            if instance.host == connector['host']:
+                compute_rpcapi.remove_volume_connection(
+                    cctxt, instance, volume_id, instance.host)
+            else:
+                msg = (
+                    f"The compute host '{connector['host']}' in the "
+                    f"connector does not match the instance host "
+                    f"'{instance.host}'.")
+                raise exception.HostConflict(_(msg))
 
             # Delete the existing volume attachment if present in the bdm.
             # This isn't present when the original attachment was made
@@ -3161,6 +3164,7 @@ def refresh(self, instance_uuid=None, volume_id=None, connector_path=None):
         * 4: Instance does not exist.
         * 5: Instance state invalid.
         * 6: Volume is not attached to instance.
+        * 7: Connector host is not correct.
         """
         try:
             # TODO(lyarwood): Make this optional and provide a rpcapi capable
@@ -3176,6 +3180,12 @@ def refresh(self, instance_uuid=None, volume_id=None, connector_path=None):
             # Refresh the volume attachment
             return self._refresh(instance_uuid, volume_id, connector)
 
+        except exception.HostConflict as e:
+            print(
+                f"The command 'nova-manage volume_attachment get_connector' "
+                f"may have been run on the wrong compute host. Or the "
+                f"instance host may be wrong and in need of repair.\n{e}")
+            return 7
         except exception.VolumeBDMNotFound as e:
             print(str(e))
             return 6

nova/exception.py

@@ -2530,3 +2530,7 @@ class NotSupportedComputeForEvacuateV295(NotSupported):
                 "instance on destination. To evacuate before upgrades are "
                 "complete please use an older microversion. Required version "
                 "for compute %(expected), current version %(currently)s")
+
+
+class HostConflict(Exception):
+    pass

nova/tests/unit/cmd/test_manage.py

@@ -3622,6 +3622,50 @@ def test_refresh_attachment_unknown_failure(
         mock_action_start.assert_called_once()
         mock_action.finish.assert_called_once()
 
+    @mock.patch('nova.compute.rpcapi.ComputeAPI', autospec=True)
+    @mock.patch('nova.volume.cinder.API', autospec=True)
+    @mock.patch('nova.compute.api.API', autospec=True)
+    @mock.patch.object(objects.BlockDeviceMapping, 'save')
+    @mock.patch.object(
+        objects.BlockDeviceMapping, 'get_by_volume_and_instance')
+    @mock.patch.object(objects.Instance, 'get_by_uuid')
+    @mock.patch.object(objects.InstanceAction, 'action_start')
+    def test_refresh_invalid_connector_host(
+        self, mock_action_start, mock_get_instance,
+        mock_get_bdm, mock_save_bdm, mock_compute_api, mock_volume_api,
+        mock_compute_rpcapi
+    ):
+        """Test refresh with a old host not disconnected properly
+        and connector host info is not correct, a fake-host is
+        passed.
+        """
+
+        fake_volume_api = mock_volume_api.return_value
+        device_name = '/dev/vda'
+
+        mock_get_instance.return_value = objects.Instance(
+            uuid=uuidsentinel.instance,
+            vm_state=obj_fields.InstanceState.STOPPED,
+            host='old-host', locked=False)
+        mock_get_bdm.return_value = objects.BlockDeviceMapping(
+            uuid=uuidsentinel.bdm, volume_id=uuidsentinel.volume,
+            attachment_id=uuidsentinel.instance,
+            device_name=device_name)
+        mock_action = mock.Mock(spec=objects.InstanceAction)
+        mock_action_start.return_value = mock_action
+
+        fake_volume_api.attachment_create.return_value = {
+            'id': uuidsentinel.new_attachment,
+        }
+        # in instance we have host as 'old-host'
+        # but here 'fake-host' is passed in connector info.
+        fake_volume_api.attachment_update.return_value = {
+            'connection_info': self._get_fake_connector_info(),
+        }
+
+        ret = self._test_refresh()
+        self.assertEqual(7, ret)
+
     @mock.patch('nova.compute.rpcapi.ComputeAPI', autospec=True)
     @mock.patch('nova.volume.cinder.API', autospec=True)
     @mock.patch('nova.compute.api.API', autospec=True)
@@ -3644,7 +3688,7 @@ def test_refresh(
         mock_get_instance.return_value = objects.Instance(
             uuid=uuidsentinel.instance,
             vm_state=obj_fields.InstanceState.STOPPED,
-            host='foo', locked=False)
+            host='fake-host', locked=False)
         mock_get_bdm.return_value = objects.BlockDeviceMapping(
             uuid=uuidsentinel.bdm, volume_id=uuidsentinel.volume,
             attachment_id=uuidsentinel.instance,