diff --git a/doc/source/cli/nova-manage.rst b/doc/source/cli/nova-manage.rst index a2398f71d00e..94022b1c8061 100644 --- a/doc/source/cli/nova-manage.rst +++ b/doc/source/cli/nova-manage.rst @@ -1572,7 +1572,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 diff --git a/nova/cmd/manage.py b/nova/cmd/manage.py index c0957fef53a4..a9b5ade5fb5e 100644 --- a/nova/cmd/manage.py +++ b/nova/cmd/manage.py @@ -164,18 +164,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) @@ -3139,8 +3135,15 @@ class VolumeAttachmentCommands(object): # 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 @@ -3222,6 +3225,7 @@ class VolumeAttachmentCommands(object): * 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 @@ -3237,6 +3241,12 @@ class VolumeAttachmentCommands(object): # 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 diff --git a/nova/exception.py b/nova/exception.py index c869b0b375b3..801033b33e7f 100644 --- a/nova/exception.py +++ b/nova/exception.py @@ -2556,3 +2556,7 @@ class EphemeralEncryptionSecretNotFound(Invalid): class EphemeralEncryptionCleanupFailed(NovaException): msg_fmt = _("Failed to clean up ephemeral encryption secrets: " "%(error)s") + + +class HostConflict(Exception): + pass diff --git a/nova/tests/unit/cmd/test_manage.py b/nova/tests/unit/cmd/test_manage.py index 253263827954..0756624f6e63 100644 --- a/nova/tests/unit/cmd/test_manage.py +++ b/nova/tests/unit/cmd/test_manage.py @@ -3642,6 +3642,50 @@ class VolumeAttachmentCommandsTestCase(test.NoDBTestCase): 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) @@ -3664,7 +3708,7 @@ class VolumeAttachmentCommandsTestCase(test.NoDBTestCase): 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,